Messages in this thread | | | Subject | Re: [RFC v2 10/32] x86/tdx: Wire up KVM hypercalls | From | Dave Hansen <> | Date | Fri, 7 May 2021 14:46:41 -0700 |
| |
On 4/26/21 11:01 AM, Kuppuswamy Sathyanarayanan wrote: > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > > KVM hypercalls have to be wrapped into vendor-specific TDVMCALLs.
How about:
KVM hypercalls use the "vmcall" or "vmmcall" instructions. Although the ABI is similar, those instructions no longer function for TDX guests. Make TDVMCALLs instead of VMCALL/VMCALL.
> diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h > index 338119852512..2fa85481520b 100644 > --- a/arch/x86/include/asm/kvm_para.h > +++ b/arch/x86/include/asm/kvm_para.h > @@ -6,6 +6,7 @@ > #include <asm/alternative.h> > #include <linux/interrupt.h> > #include <uapi/asm/kvm_para.h> > +#include <asm/tdx.h> > > extern void kvmclock_init(void); > > @@ -34,6 +35,10 @@ static inline bool kvm_check_and_clear_guest_paused(void) > static inline long kvm_hypercall0(unsigned int nr) > { > long ret; > + > + if (is_tdx_guest()) > + return tdx_kvm_hypercall0(nr);
... all of these look OK.
> #endif /* _ASM_X86_TDX_H */ > diff --git a/arch/x86/kernel/tdcall.S b/arch/x86/kernel/tdcall.S > index 81af70c2acbd..964bfd7fc682 100644 > --- a/arch/x86/kernel/tdcall.S > +++ b/arch/x86/kernel/tdcall.S > @@ -11,6 +11,7 @@ > * refer to TDX GHCI specification). > */ > #define TDVMCALL_EXPOSE_REGS_MASK 0xfc00 > +#define TDVMCALL_VENDOR_KVM 0x4d564b2e584454 /* "TDX.KVM" */ > > /* > * TDX guests use the TDCALL instruction to make > @@ -198,3 +199,9 @@ SYM_FUNC_START(__tdvmcall) > call do_tdvmcall > retq > SYM_FUNC_END(__tdvmcall) > + > +SYM_FUNC_START(__tdvmcall_vendor_kvm) > + movq $TDVMCALL_VENDOR_KVM, %r10 > + call do_tdvmcall > + retq > +SYM_FUNC_END(__tdvmcall_vendor_kvm)
Granted, this is not a ton of assembly. But, it does look a bit weird. It needs a comment and/or a mention in the changelog.
R10 is not part of the function call ABI, but it is a part of the TDVMCALL ABI. This little assembly wrapper lets us reuse do_tdvmcall() for both KVM-specific hypercalls TDVMCALL_VENDOR_KVM and the more generic __tdvmcalls.
> --- a/arch/x86/kernel/tdx.c > +++ b/arch/x86/kernel/tdx.c > @@ -8,6 +8,10 @@ > > #include <linux/cpu.h> > > +#ifdef CONFIG_KVM_GUEST > +#include "tdx-kvm.c" > +#endif > + > static struct { > unsigned int gpa_width; > unsigned long attributes;
I know KVM does weird stuff. But, this is *really* weird. Why are we #including a .c file into another .c file?
| |