Re: [PATCH v5 09/12] x86/tdx: Wire up KVM hypercalls

Borislav Petkov
2021-08-24T16:34:16Z

On Wed, Aug 04, 2021 at 11:13:26AM -0700, Kuppuswamy Sathyanarayanan wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> KVM hypercalls use the "vmcall" or "vmmcall" instructions.

Write instruction mnemonics in all caps pls.

> +# This option enables KVM specific hypercalls in TDX guest.
> +config INTEL_TDX_GUEST_KVM

What is that config option really for? IOW, can't you use
CONFIG_KVM_GUEST instead?

> + def_bool y
> + depends on KVM_GUEST && INTEL_TDX_GUEST
> +
> endif #HYPERVISOR_GUEST
>
> source "arch/x86/Kconfig.cpu"
> diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h
> index 4cb726c71ed8..9855a9ff2924 100644
> --- a/arch/x86/include/asm/asm-prototypes.h
> +++ b/arch/x86/include/asm/asm-prototypes.h
> @@ -17,6 +17,10 @@
> extern void cmpxchg8b_emu(void);
> #endif
>
> +#ifdef CONFIG_INTEL_TDX_GUEST
> +#include <asm/tdx.h>
> +#endif

What "ASM sysmbol generation issue" forced this?

...

> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 846fe58f0426..8fa33e2c98db 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -6,8 +6,9 @@
> #include <linux/cpufeature.h>
> #include <linux/types.h>
>
> -#define TDX_CPUID_LEAF_ID 0x21
> -#define TDX_HYPERCALL_STANDARD 0
> +#define TDX_CPUID_LEAF_ID 0x21
> +#define TDX_HYPERCALL_STANDARD 0
> +#define TDX_HYPERCALL_VENDOR_KVM 0x4d564b2e584454

"TDX.KVM"

Yeah, you can put it in a comment so that people don't have to do the
CTRL-V game in vim insert mode, i.e., ":help i_CTRL-V_digit" :-)

> /*
> * Used in __tdx_module_call() helper function to gather the
> @@ -80,4 +81,29 @@ static inline bool tdx_prot_guest_has(unsigned long flag) { return false; }
>
> #endif /* CONFIG_INTEL_TDX_GUEST */
>
> +#ifdef CONFIG_INTEL_TDX_GUEST_KVM

I don't think that even needs the ifdeffery. If it is not used, the
inline will simply get discarded so why bother?

> +
> +static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
> + unsigned long p2, unsigned long p3,
> + unsigned long p4)
> +{
> + struct tdx_hypercall_output out;
> + u64 err;
> +
> + err = __tdx_hypercall(TDX_HYPERCALL_VENDOR_KVM, nr, p1, p2,
> + p3, p4, &out);
> +
> + BUG_ON(err);
> +
> + return out.r10;
> +}
> +#else
> +static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
> + unsigned long p2, unsigned long p3,
> + unsigned long p4)
> +{
> + return -ENODEV;
> +}
> +#endif /* CONFIG_INTEL_TDX_GUEST_KVM */
> +
> #endif /* _ASM_X86_TDX_H */
> diff --git a/arch/x86/kernel/tdcall.S b/arch/x86/kernel/tdcall.S
> index 9df94f87465d..1823bac4542d 100644
> --- a/arch/x86/kernel/tdcall.S
> +++ b/arch/x86/kernel/tdcall.S
> @@ -3,6 +3,7 @@
> #include <asm/asm.h>
> #include <asm/frame.h>
> #include <asm/unwind_hints.h>
> +#include <asm/export.h>
>
> #include <linux/linkage.h>
> #include <linux/bits.h>
> @@ -309,3 +310,4 @@ skip_sti:
>
> retq
> SYM_FUNC_END(__tdx_hypercall)
> +EXPORT_SYMBOL(__tdx_hypercall);

EXPORT_SYMBOL_GPL, of course.

--
Regards/Gruss,
Boris.