Messages in this thread | | | Subject | Re: [PATCH v2] KVM: x86: Assume a 64-bit hypercall for guests with protected state | From | Tom Lendacky <> | Date | Tue, 26 Oct 2021 13:03:00 -0500 |
| |
On 10/1/21 12:06 PM, Tom Lendacky wrote: > On 5/25/21 1:25 AM, Vitaly Kuznetsov wrote: >> Tom Lendacky <thomas.lendacky@amd.com> writes: >> >>> When processing a hypercall for a guest with protected state, currently >>> SEV-ES guests, the guest CS segment register can't be checked to >>> determine if the guest is in 64-bit mode. For an SEV-ES guest, it is >>> expected that communication between the guest and the hypervisor is >>> performed to shared memory using the GHCB. In order to use the GHCB, the >>> guest must have been in long mode, otherwise writes by the guest to the >>> GHCB would be encrypted and not be able to be comprehended by the >>> hypervisor. >>> >>> Create a new helper function, is_64_bit_hypercall(), that assumes the >>> guest is in 64-bit mode when the guest has protected state, and returns >>> true, otherwise invoking is_64_bit_mode() to determine the mode. Update >>> the hypercall related routines to use is_64_bit_hypercall() instead of >>> is_64_bit_mode(). >>> >>> Add a WARN_ON_ONCE() to is_64_bit_mode() to catch occurences of calls to >>> this helper function for a guest running with protected state. >>> >>> Fixes: f1c6366e3043 ("KVM: SVM: Add required changes to support >>> intercepts under SEV-ES") >>> Reported-by: Sean Christopherson <seanjc@google.com> >>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> >>> --- >>> >>> Changes since v1: >>> - Create a new helper routine, is_64_bit_hypercall(), and use it in place >>> of is_64_bit_mode() in hypercall related areas. >>> - Add a WARN_ON_ONCE() to is_64_bit_mode() to issue a warning if invoked >>> for a guest with protected state. >>> --- >>> arch/x86/kvm/hyperv.c | 4 ++-- >>> arch/x86/kvm/x86.c | 2 +- >>> arch/x86/kvm/x86.h | 12 ++++++++++++ >>> arch/x86/kvm/xen.c | 2 +- >>> 4 files changed, 16 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c >>> index f98370a39936..1cdf2b213f41 100644 >>> --- a/arch/x86/kvm/hyperv.c >>> +++ b/arch/x86/kvm/hyperv.c >>> @@ -1818,7 +1818,7 @@ static void kvm_hv_hypercall_set_result(struct >>> kvm_vcpu *vcpu, u64 result) >>> { >>> bool longmode; >>> - longmode = is_64_bit_mode(vcpu); >>> + longmode = is_64_bit_hypercall(vcpu); >>> if (longmode) >>> kvm_rax_write(vcpu, result); >>> else { >>> @@ -1895,7 +1895,7 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu) >>> } >>> #ifdef CONFIG_X86_64 >>> - if (is_64_bit_mode(vcpu)) { >>> + if (is_64_bit_hypercall(vcpu)) { >>> param = kvm_rcx_read(vcpu); >>> ingpa = kvm_rdx_read(vcpu); >>> outgpa = kvm_r8_read(vcpu); >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>> index 9b6bca616929..dc72f0a1609a 100644 >>> --- a/arch/x86/kvm/x86.c >>> +++ b/arch/x86/kvm/x86.c >>> @@ -8403,7 +8403,7 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) >>> trace_kvm_hypercall(nr, a0, a1, a2, a3); >>> - op_64_bit = is_64_bit_mode(vcpu); >>> + op_64_bit = is_64_bit_hypercall(vcpu); >>> if (!op_64_bit) { >>> nr &= 0xFFFFFFFF; >>> a0 &= 0xFFFFFFFF; >>> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h >>> index 521f74e5bbf2..3102caf689d2 100644 >>> --- a/arch/x86/kvm/x86.h >>> +++ b/arch/x86/kvm/x86.h >>> @@ -151,12 +151,24 @@ static inline bool is_64_bit_mode(struct kvm_vcpu >>> *vcpu) >>> { >>> int cs_db, cs_l; >>> + WARN_ON_ONCE(vcpu->arch.guest_state_protected); >>> + >>> if (!is_long_mode(vcpu)) >>> return false; >>> static_call(kvm_x86_get_cs_db_l_bits)(vcpu, &cs_db, &cs_l); >>> return cs_l; >>> } >>> +static inline bool is_64_bit_hypercall(struct kvm_vcpu *vcpu) >>> +{ >>> + /* >>> + * If running with protected guest state, the CS register is not >>> + * accessible. The hypercall register values will have had to been >>> + * provided in 64-bit mode, so assume the guest is in 64-bit. >>> + */ >>> + return vcpu->arch.guest_state_protected || is_64_bit_mode(vcpu); >>> +} >>> + >>> static inline bool is_la57_mode(struct kvm_vcpu *vcpu) >>> { >>> #ifdef CONFIG_X86_64 >>> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c >>> index ae17250e1efe..c58f6369e668 100644 >>> --- a/arch/x86/kvm/xen.c >>> +++ b/arch/x86/kvm/xen.c >>> @@ -680,7 +680,7 @@ int kvm_xen_hypercall(struct kvm_vcpu *vcpu) >>> kvm_hv_hypercall_enabled(vcpu)) >>> return kvm_hv_hypercall(vcpu); >>> - longmode = is_64_bit_mode(vcpu); >>> + longmode = is_64_bit_hypercall(vcpu); >>> if (!longmode) { >>> params[0] = (u32)kvm_rbx_read(vcpu); >>> params[1] = (u32)kvm_rcx_read(vcpu); >> >> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> >> >> Thanks! > > Paolo, > > This got lost in my stack of work... any comments? > > Thanks, > Tom
Ping
Thanks, Tom
> >>
| |