Re: [RFC v2-fix 1/1] x86/tdx: Wire up KVM hypercalls
From: Kuppuswamy, Sathyanarayanan
Date: Tue May 18 2021 - 16:12:23 EST
On 5/18/21 8:51 AM, Dave Hansen wrote:
Question for KVM folks: Should all of these guest patches say:
"x86/tdx/guest:" or something? It seems like that would put us all in
the right frame of mind as we review these. It's kinda easy (for me at
least) to get lost about which side I'm looking at sometimes.
On 5/17/21 5:15 PM, Kuppuswamy Sathyanarayanan wrote:
From: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>
KVM hypercalls use the "vmcall" or "vmmcall" instructions.
Although the ABI is similar, those instructions no longer
function for TDX guests. Make vendor specififc TDVMCALLs
"vendor-specific"
Hyphen and spelling ^
I will fix it next version.
instead of VMCALL.
This would also be a great place to say:
This enables TDX guests to run with KVM acting as the hypervisor. TDX
guests running under other hypervisors will continue to use those
hypervisors hypercalls.
I will include it.
[Isaku: proposed KVM VENDOR string]
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
Reviewed-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
This SoB chain is odd. Kirill wrote this, sent it to Isaku, who sent it
to Sathya?
Initially we have used "0" as vendor ID for KVM. But Isaku proposed a new
value for it and sent a patch to fix it. But, I did not want to carry it as
separate patch (for one line change). So I have merged his change with
this patch, and added his signed-off with comment ([Isaku: proposed KVM VENDOR string])
+#define TDVMCALL_VENDOR_KVM 0x4d564b2e584454 /* "TDX.KVM" */
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 9e0e0ff76bab..768df1b98487 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -886,6 +886,12 @@ config INTEL_TDX_GUEST
run in a CPU mode that protects the confidentiality of TD memory
contents and the TD’s CPU state from other software, including VMM.
+config INTEL_TDX_GUEST_KVM
+ def_bool y
+ depends on KVM_GUEST && INTEL_TDX_GUEST
+ help
+ This option enables KVM specific hypercalls in TDX guest.
For something that's not user-visible, I'd probably just add a Kconfig
comment rather than help text.
If it is the preferred approach, I can remove it.
...
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 7966c10ea8d1..a90fec004844 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -128,6 +128,7 @@ obj-$(CONFIG_X86_PMEM_LEGACY_DEVICE) += pmem.o
obj-$(CONFIG_JAILHOUSE_GUEST) += jailhouse.o
obj-$(CONFIG_INTEL_TDX_GUEST) += tdcall.o tdx.o
+obj-$(CONFIG_INTEL_TDX_GUEST_KVM) += tdx-kvm.o
Is the indentation consistent with the other items near "tdx-kvm.o" in
the Makefile?
Yes. For longer config names, common indentation is not maintained. Please
check the PMEM example.
126 obj-$(CONFIG_PARAVIRT_CLOCK) += pvclock.o
127 obj-$(CONFIG_X86_PMEM_LEGACY_DEVICE) += pmem.o
128
129 obj-$(CONFIG_JAILHOUSE_GUEST) += jailhouse.o
130 obj-$(CONFIG_INTEL_TDX_GUEST) += tdcall.o tdx.o
131 obj-$(CONFIG_INTEL_TDX_GUEST_KVM) += tdx-kvm.o
...
+/* Used by kvm_hypercall4() to trigger hypercall in TDX guest */
+long tdx_kvm_hypercall4(unsigned int nr, unsigned long p1, unsigned long p2,
+ unsigned long p3, unsigned long p4)
+{
+ return tdx_kvm_hypercall(nr, p1, p2, p3, p4);
+}
+EXPORT_SYMBOL_GPL(tdx_kvm_hypercall4);
I always forget that KVM code is goofy and needs to have things in C
files so you can export the symbols. Could you add a sentence to the
changelog to this effect?
Code-wise, this is fine. Just a few tweaks and I'll be happy to ack
this one.
Will add it.
Since KVM hypercall functions can be included and called
from kernel modules, export tdx_kvm_hypercall*() functions
to avoid symbol errors
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer