From: Erdem Aktas <erdemaktas@google.com>
Add support for TDX guests to issue TDCALLs to the TDX module.
Signed-off-by: Erdem Aktas <erdemaktas@google.com>
Co-developed-by: Sagi Shahar <sagis@google.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
tools/testing/selftests/kvm/Makefile.kvm | 8 ++
.../selftests/kvm/include/x86/tdx/tdcall.h | 34 +++++++
.../selftests/kvm/lib/x86/tdx/tdcall.S | 93 +++++++++++++++++++
.../kvm/lib/x86/tdx/tdcall_offsets.c | 16 ++++
4 files changed, 151 insertions(+)
create mode 100644 tools/testing/selftests/kvm/include/x86/tdx/tdcall.h
create mode 100644 tools/testing/selftests/kvm/lib/x86/tdx/tdcall.S
create mode 100644 tools/testing/selftests/kvm/lib/x86/tdx/tdcall_offsets.c
diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 86fe629f2e81..969338b66592 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -20,6 +20,7 @@ LIBKVM += lib/userfaultfd_util.c
LIBKVM_STRING += lib/string_override.c
LIBKVM_ASM_DEFS += lib/x86/tdx/td_boot_offsets.c
+LIBKVM_ASM_DEFS += lib/x86/tdx/tdcall_offsets.c
LIBKVM_x86 += lib/x86/apic.c
LIBKVM_x86 += lib/x86/handlers.S
@@ -33,6 +34,7 @@ LIBKVM_x86 += lib/x86/ucall.c
LIBKVM_x86 += lib/x86/vmx.c
LIBKVM_x86 += lib/x86/tdx/tdx_util.c
LIBKVM_x86 += lib/x86/tdx/td_boot.S
+LIBKVM_x86 += lib/x86/tdx/tdcall.S
LIBKVM_arm64 += lib/arm64/gic.c
LIBKVM_arm64 += lib/arm64/gic_v3.c
@@ -352,7 +354,13 @@ $(OUTPUT)/lib/x86/tdx/td_boot.o: $(OUTPUT)/include/x86/tdx/td_boot_offsets.h
$(OUTPUT)/include/x86/tdx/td_boot_offsets.h: $(OUTPUT)/lib/x86/tdx/td_boot_offsets.s FORCE
$(call filechk,offsets,__TDX_BOOT_OFFSETS_H__)
+$(OUTPUT)/lib/x86/tdx/tdcall.o: $(OUTPUT)/include/x86/tdx/tdcall_offsets.h
+
+$(OUTPUT)/include/x86/tdx/tdcall_offsets.h: $(OUTPUT)/lib/x86/tdx/tdcall_offsets.s FORCE
+ $(call filechk,offsets,__TDCALL__OFFSETS_H__)
+
EXTRA_CLEAN += $(OUTPUT)/include/x86/tdx/td_boot_offsets.h
+EXTRA_CLEAN += $(OUTPUT)/include/x86/tdx/tdcall_offsets.h
$(shell mkdir -p $(sort $(dir $(TEST_GEN_PROGS))))
$(SPLIT_TEST_GEN_OBJ): $(GEN_HDRS)
diff --git a/tools/testing/selftests/kvm/include/x86/tdx/tdcall.h b/tools/testing/selftests/kvm/include/x86/tdx/tdcall.h
new file mode 100644
index 000000000000..60c70646f876
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/x86/tdx/tdcall.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Adapted from arch/x86/include/asm/shared/tdx.h */
+
+#ifndef SELFTESTS_TDX_TDCALL_H
+#define SELFTESTS_TDX_TDCALL_H
+
+#include <linux/bits.h>
+
+#define TDX_TDCALL_HAS_OUTPUT BIT(0)
+
+#ifndef __ASSEMBLY__
+
+#include <linux/types.h>
+
+/*
+ * Used in __tdx_tdcall() to pass down and get back registers' values of
+ * the TDCALL instruction when requesting services from the VMM.
+ *
+ * This is a software only structure and not part of the TDX module/VMM ABI.
+ */
+struct tdx_tdcall_args {
+ u64 r10;
+ u64 r11;
+ u64 r12;
+ u64 r13;
+ u64 r14;
+ u64 r15;
+};
+
+/* Used to request services from the VMM */
+u64 __tdx_tdcall(struct tdx_tdcall_args *args, unsigned long flags);
+
+#endif // __ASSEMBLY__
+#endif // SELFTESTS_TDX_TDCALL_H
diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdcall.S b/tools/testing/selftests/kvm/lib/x86/tdx/tdcall.S
new file mode 100644
index 000000000000..05869e86b9d8
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdcall.S
@@ -0,0 +1,93 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Adapted from arch/x86/virt/vmx/tdx/tdxcall.S */
+
+#ifndef __ASSEMBLY__
+#define __ASSEMBLY__
+#endif
+
+#include <linux/bits.h>
+#include "tdx/tdcall.h"
+#include "tdx/tdcall_offsets.h"
+
+/*
+ * TDCALL is supported in Binutils >= 2.36, add it for older version.
+ */
+#define tdcall .byte 0x66,0x0f,0x01,0xcc
+
+/*
+ * Bitmasks of exposed registers (with VMM).
+ */
+#define TDX_R10 BIT(10)
+#define TDX_R11 BIT(11)
+#define TDX_R12 BIT(12)
+#define TDX_R13 BIT(13)
+#define TDX_R14 BIT(14)
+#define TDX_R15 BIT(15)
+
+/*
+ * These registers are clobbered to hold arguments for each
+ * TDVMCALL. They are safe to expose to the VMM.
+ * Each bit in this mask represents a register ID. Bit field
+ * details can be found in TDX GHCI specification, section
+ * titled "TDCALL [TDG.VP.VMCALL] leaf".
+ */
+#define TDVMCALL_EXPOSE_REGS_MASK \
+ (TDX_R10 | TDX_R11 | TDX_R12 | TDX_R13 | TDX_R14 | TDX_R15)
+
+.code64
+.section .text
+
+.globl __tdx_tdcall
+.type __tdx_tdcall, @function
+__tdx_tdcall:
+ /* Set up stack frame */
+ push %rbp
+ movq %rsp, %rbp
+
+ /* Save callee-saved GPRs as mandated by the x86_64 ABI */
+ push %r15
+ push %r14
+ push %r13
+ push %r12
+
+ /* Mangle function call ABI into TDCALL ABI: */
+ /* Set TDCALL leaf ID (TDVMCALL (0)) in RAX */
+ xor %eax, %eax
+
+ /* Copy tdcall registers from arg struct: */
+ movq TDX_TDCALL_R10(%rdi), %r10
+ movq TDX_TDCALL_R11(%rdi), %r11
+ movq TDX_TDCALL_R12(%rdi), %r12
+ movq TDX_TDCALL_R13(%rdi), %r13
+ movq TDX_TDCALL_R14(%rdi), %r14
+ movq TDX_TDCALL_R15(%rdi), %r15
+
+ movl $TDVMCALL_EXPOSE_REGS_MASK, %ecx
+
+ tdcall
+
+ /* TDVMCALL leaf return code is in R10 */
+ movq %r10, %rax
+
+ /* Copy tdcall result registers to arg struct if needed */
+ testq $TDX_TDCALL_HAS_OUTPUT, %rsi
+ jz .Lout
+
+ movq %r10, TDX_TDCALL_R10(%rdi)
+ movq %r11, TDX_TDCALL_R11(%rdi)
+ movq %r12, TDX_TDCALL_R12(%rdi)
+ movq %r13, TDX_TDCALL_R13(%rdi)
+ movq %r14, TDX_TDCALL_R14(%rdi)
+ movq %r15, TDX_TDCALL_R15(%rdi)
+.Lout:
+ /* Restore callee-saved GPRs as mandated by the x86_64 ABI */
+ pop %r12
+ pop %r13
+ pop %r14
+ pop %r15
+
+ pop %rbp
+ ret
+
+/* Disable executable stack */
+.section .note.GNU-stack,"",%progbits
diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdcall_offsets.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdcall_offsets.c
new file mode 100644
index 000000000000..dcd4457be6e5
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdcall_offsets.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+#define COMPILE_OFFSETS
+
+#include <linux/kbuild.h>
+
+#include "tdx/tdcall.h"
+
+static void __attribute__((used)) common(void)
+{
+ OFFSET(TDX_TDCALL_R10, tdx_tdcall_args, r10);
+ OFFSET(TDX_TDCALL_R11, tdx_tdcall_args, r11);
+ OFFSET(TDX_TDCALL_R12, tdx_tdcall_args, r12);
+ OFFSET(TDX_TDCALL_R13, tdx_tdcall_args, r13);
+ OFFSET(TDX_TDCALL_R14, tdx_tdcall_args, r14);
+ OFFSET(TDX_TDCALL_R15, tdx_tdcall_args, r15);
+}
--
2.51.1.851.g4ebd6896fd-googSagi Shahar wrote:
> From: Erdem Aktas <erdemaktas@google.com>
>
> Add support for TDX guests to issue TDCALLs to the TDX module.
Generally it is nice to have more details. As someone new to TDX I
have to remind myself what a TDCALL is. And any random kernel developer
reading this in the future will likely have even less clue than me.
Paraphrased from the spec:
TDCALL is the instruction used by the guest TD software (in TDX non-root
mode) to invoke guest-side TDX functions. TDG.VP.VMCALL helps invoke
services from the host VMM.
Add support for TDX guests to invoke services from the host VMM.
>
> Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> Co-developed-by: Sagi Shahar <sagis@google.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> ---
[snip]
> diff --git a/tools/testing/selftests/kvm/include/x86/tdx/tdcall.h b/tools/testing/selftests/kvm/include/x86/tdx/tdcall.h
> new file mode 100644
> index 000000000000..60c70646f876
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/include/x86/tdx/tdcall.h
[snip]
> +
> +/*
> + * Used in __tdx_tdcall() to pass down and get back registers' values of
> + * the TDCALL instruction when requesting services from the VMM.
> + *
> + * This is a software only structure and not part of the TDX module/VMM ABI.
This is a good comment.
> + */
> +struct tdx_tdcall_args {
> + u64 r10;
> + u64 r11;
> + u64 r12;
> + u64 r13;
> + u64 r14;
> + u64 r15;
> +};
> +
[snip]
> +
> +/*
> + * Bitmasks of exposed registers (with VMM).
> + */
> +#define TDX_R10 BIT(10)
> +#define TDX_R11 BIT(11)
> +#define TDX_R12 BIT(12)
> +#define TDX_R13 BIT(13)
> +#define TDX_R14 BIT(14)
> +#define TDX_R15 BIT(15)
> +
> +/*
> + * These registers are clobbered to hold arguments for each
> + * TDVMCALL. They are safe to expose to the VMM.
I'm not sure what this comment means by being 'safe to expose to the VMM'?
They are all overwritten per the data specified correct?
> + * Each bit in this mask represents a register ID. Bit field
> + * details can be found in TDX GHCI specification, section
> + * titled "TDCALL [TDG.VP.VMCALL] leaf".
TDX GHCI specification v1.5, March 2023
2.4.1 TDCALL [TDG.VP.VMCALL] leaf
This nails down any issues which may arise if the module/spec changes.
Ira
[snip]
On Fri, Oct 31, 2025, Ira Weiny wrote:
> Sagi Shahar wrote:
> > From: Erdem Aktas <erdemaktas@google.com>
> >
> > Add support for TDX guests to issue TDCALLs to the TDX module.
>
> Generally it is nice to have more details. As someone new to TDX I
> have to remind myself what a TDCALL is. And any random kernel developer
> reading this in the future will likely have even less clue than me.
>
> Paraphrased from the spec:
>
> TDCALL is the instruction used by the guest TD software (in TDX non-root
> mode) to invoke guest-side TDX functions. TDG.VP.VMCALL helps invoke
> services from the host VMM.
>
> Add support for TDX guests to invoke services from the host VMM.
Eh, at some point a baseline amount of knowledge is required. I highly doubt
regurgitating the spec is going to make a huge difference
I also dislike the above wording, because it doesn't help understand _why_ KVM
selftests need to support TDCALL, or _how_ the functionality will be utilized.
E.g. strictly speaking, we could write KVM selftests without ever doing a single
TDG.VP.VMCALL, because we control both sides (guest and VMM). And I have a hard
time belive name-dropping TDG.VP.VMCALL is going to connect the dots between
TDCALL and the "tunneling" scheme defined by the GHCI for requesting emulation
of "legacy" functionality".
What I would like to know is why selftests are copy-pasting the kernel's scheme
for marshalling data to/from the registers used by TDCALL, how selftests are
expected to utilize TDCALL, etc. I'm confident that if someone actually took the
time to write a changelog explaining those details, then what TDCALL "is" will
be fairly clear, even if the reader doesn't know exactly what it is.
E.g. IMO this is ugly and lazy on multiple fronts:
uint64_t tdg_vp_vmcall_ve_request_mmio_write(uint64_t address, uint64_t size,
uint64_t data_in)
{
struct tdx_tdcall_args args = {
.r10 = TDG_VP_VMCALL,
.r11 = TDG_VP_VMCALL_VE_REQUEST_MMIO,
.r12 = size,
.r13 = MMIO_WRITE,
.r14 = address,
.r15 = data_in,
};
return __tdx_tdcall(&args, 0);
}
First, these are KVM selftests, there's no need to provide a super fancy namespace
because we are "competing" with thousands upon thousands of lines of code from
other components and subsystems.
Similarly, tdg_vp_vmcall_ve_request_mmio_write() is absurdly verbose. Referencing
#VE in any way is also flat out wrong.
It's also far too specific to TDX, which is going to be problematic when full
support for SEV-ES+ selftests comes along. I.e. calling this from common code
is going to be a pain in the rear, bordering on unworkable.
And related to your comment about having enums for the sizes, there's absolutely
zero reason the caller should have to specify the size.
In short, don't simply copy what was done for the kernel. The kernel is operating
under constraints that do not and should not ever apply to KVM selftests. Except
for tests like set_memory_region_test.c that delete memslots while a vCPU is running
and thus _may_ generate MMIO accesses, our selftests should never, ever take a #VE
(or #VC) and then request MMIO in the handler. If a test wants to do MMIO, then
do MMIO.
So, I want to see GUEST_MMIO_WRITE() and GUEST_MMIO_READ(), or probably even just
MMIO_WRITE() and MMIO_READ(). And then under the hood, wire up kvm_arch_mmio_write()
and kvm_arch_mmio_read() in kvm_util_arch.h. And from there have x86 globally track
if it's TDX, SEV-ES+, or "normal". That'd also give us a good reason+way to assert
on s390 if a test attempts MMIO, as s390 doesn't support emulated MMIO.
One potential hiccup is if/when KVM selftests get access to actual MMIO, i.e. don't
want to trigger emulation, e.g. for VFIO related selftests when accessing BARs.
Though the answer there is probably to just use WRITE/READ_ONCE() and call it good.
E.g.
#define MMIO_WRITE(addr, val) \
kvm_arch_mmio_write(addr, val);
#define kvm_arch_mmio_write(addr, val) \
({ \
if (guest_needs_tdvmcall) \
tdx_mmio_write(addr, val, sizeof(val)); \
else if (guest_needs_vmgexit) \
sev_mmio_write(addr, val, sizeof(val)); \
else \
WRITE_ONCE(addr, val); \
})
#define MMIO_READ(addr, val) \
kvm_arch_mmio_read(addr, val);
#define kvm_arch_mmio_read(addr, val) \
({ \
if (guest_needs_tdvmcall) \
tdx_mmio_read(addr, &(val), sizeof(val)); \
else if (guest_needs_vmgexit) \
sev_mmio_write(addr, &(val), sizeof(val)); \
else \
(val) = READ_ONCE(addr); \
})
On Fri, Oct 31, 2025, Sean Christopherson wrote: > On Fri, Oct 31, 2025, Ira Weiny wrote: > > Sagi Shahar wrote: > > > From: Erdem Aktas <erdemaktas@google.com> > > > > > > Add support for TDX guests to issue TDCALLs to the TDX module. > > > > Generally it is nice to have more details. As someone new to TDX I > > have to remind myself what a TDCALL is. And any random kernel developer > > reading this in the future will likely have even less clue than me. > > > > Paraphrased from the spec: > > > > TDCALL is the instruction used by the guest TD software (in TDX non-root > > mode) to invoke guest-side TDX functions. TDG.VP.VMCALL helps invoke > > services from the host VMM. > > > > Add support for TDX guests to invoke services from the host VMM. > > Eh, at some point a baseline amount of knowledge is required. I highly doubt > regurgitating the spec is going to make a huge difference > > I also dislike the above wording, because it doesn't help understand _why_ KVM > selftests need to support TDCALL, or _how_ the functionality will be utilized. > E.g. strictly speaking, we could write KVM selftests without ever doing a single > TDG.VP.VMCALL, because we control both sides (guest and VMM). And I have a hard > time belive name-dropping TDG.VP.VMCALL is going to connect the dots between > TDCALL and the "tunneling" scheme defined by the GHCI for requesting emulation > of "legacy" functionality". > > What I would like to know is why selftests are copy-pasting the kernel's scheme > for marshalling data to/from the registers used by TDCALL, I almost forgot. I detest the "throw everything into a structure" approach, which the kernel used largely so that it could share code between SEAMCALLs and TDCALLs. Unless there's a good reason no to, I would much rather have prototypes like uint64_t __tdvmcall(<all the args>) uint64_t tdvmcall_1(uint64_t arg1); uint64_t tdvmcall_2(uint64_t arg1, uint64_t arg2); uint64_t tdvmcall_3(...); uint65_t tdvmcall_4(...); uint64_t tdvmcall_5(...); uint64_t tdvmcall_6(...);
On Fri, Oct 31, 2025 at 10:15 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Fri, Oct 31, 2025, Ira Weiny wrote:
> > Sagi Shahar wrote:
> > > From: Erdem Aktas <erdemaktas@google.com>
> > >
> > > Add support for TDX guests to issue TDCALLs to the TDX module.
> >
> > Generally it is nice to have more details. As someone new to TDX I
> > have to remind myself what a TDCALL is. And any random kernel developer
> > reading this in the future will likely have even less clue than me.
> >
> > Paraphrased from the spec:
> >
> > TDCALL is the instruction used by the guest TD software (in TDX non-root
> > mode) to invoke guest-side TDX functions. TDG.VP.VMCALL helps invoke
> > services from the host VMM.
> >
> > Add support for TDX guests to invoke services from the host VMM.
>
> Eh, at some point a baseline amount of knowledge is required. I highly doubt
> regurgitating the spec is going to make a huge difference
>
> I also dislike the above wording, because it doesn't help understand _why_ KVM
> selftests need to support TDCALL, or _how_ the functionality will be utilized.
> E.g. strictly speaking, we could write KVM selftests without ever doing a single
> TDG.VP.VMCALL, because we control both sides (guest and VMM). And I have a hard
> time belive name-dropping TDG.VP.VMCALL is going to connect the dots between
> TDCALL and the "tunneling" scheme defined by the GHCI for requesting emulation
> of "legacy" functionality".
>
> What I would like to know is why selftests are copy-pasting the kernel's scheme
> for marshalling data to/from the registers used by TDCALL, how selftests are
> expected to utilize TDCALL, etc. I'm confident that if someone actually took the
> time to write a changelog explaining those details, then what TDCALL "is" will
> be fairly clear, even if the reader doesn't know exactly what it is.
>
> E.g. IMO this is ugly and lazy on multiple fronts:
To give some context to why this was done this way: Part of the reason
for the selftests is to test the GHCI protocol itself. Some of the
selftests will issue calls with purposely invalid arguments to ensure
KVM handles these cases properly. For example, issuing a port IO calls
with sizes other than 1,2 or 4 and ensure we get an error on the guest
side.
The code was intentionally written to be specific to TDX so we can
test the TDX GHCI spec itself.
As I understand it, you want the selftests to operate at a higher
level and abstract away the specific GHCI details so that the code can
be shared between TDX and SEV. I can refactor the code to abstract
away implementation details. However, tests that want to exercise the
API at a fine-grained level to test different arguments will need to
define these TDCALLs themselves.
These calls were placed in a header that can be included in the guest
code. I can add higher level wrappers that can be used for common
code.
>
> uint64_t tdg_vp_vmcall_ve_request_mmio_write(uint64_t address, uint64_t size,
> uint64_t data_in)
> {
> struct tdx_tdcall_args args = {
> .r10 = TDG_VP_VMCALL,
> .r11 = TDG_VP_VMCALL_VE_REQUEST_MMIO,
> .r12 = size,
> .r13 = MMIO_WRITE,
> .r14 = address,
> .r15 = data_in,
> };
>
> return __tdx_tdcall(&args, 0);
> }
>
> First, these are KVM selftests, there's no need to provide a super fancy namespace
> because we are "competing" with thousands upon thousands of lines of code from
> other components and subsystems.
>
> Similarly, tdg_vp_vmcall_ve_request_mmio_write() is absurdly verbose. Referencing
> #VE in any way is also flat out wrong.
This name was taken from the GHCI spec: TDG.VP.VMCALL<#VE.RequestMMIO>
("Intel TDX Guest-Hypervisor Communication Interface v1.5" section 3.7)
>
> It's also far too specific to TDX, which is going to be problematic when full
> support for SEV-ES+ selftests comes along. I.e. calling this from common code
> is going to be a pain in the rear, bordering on unworkable.
>
> And related to your comment about having enums for the sizes, there's absolutely
> zero reason the caller should have to specify the size.
>
> In short, don't simply copy what was done for the kernel. The kernel is operating
> under constraints that do not and should not ever apply to KVM selftests. Except
> for tests like set_memory_region_test.c that delete memslots while a vCPU is running
> and thus _may_ generate MMIO accesses, our selftests should never, ever take a #VE
> (or #VC) and then request MMIO in the handler. If a test wants to do MMIO, then
> do MMIO.
>
> So, I want to see GUEST_MMIO_WRITE() and GUEST_MMIO_READ(), or probably even just
> MMIO_WRITE() and MMIO_READ(). And then under the hood, wire up kvm_arch_mmio_write()
> and kvm_arch_mmio_read() in kvm_util_arch.h. And from there have x86 globally track
> if it's TDX, SEV-ES+, or "normal". That'd also give us a good reason+way to assert
> on s390 if a test attempts MMIO, as s390 doesn't support emulated MMIO.
>
> One potential hiccup is if/when KVM selftests get access to actual MMIO, i.e. don't
> want to trigger emulation, e.g. for VFIO related selftests when accessing BARs.
> Though the answer there is probably to just use WRITE/READ_ONCE() and call it good.
>
> E.g.
>
> #define MMIO_WRITE(addr, val) \
> kvm_arch_mmio_write(addr, val);
>
> #define kvm_arch_mmio_write(addr, val) \
> ({ \
> if (guest_needs_tdvmcall) \
> tdx_mmio_write(addr, val, sizeof(val)); \
> else if (guest_needs_vmgexit) \
> sev_mmio_write(addr, val, sizeof(val)); \
> else \
> WRITE_ONCE(addr, val); \
> })
>
> #define MMIO_READ(addr, val) \
> kvm_arch_mmio_read(addr, val);
>
> #define kvm_arch_mmio_read(addr, val) \
> ({ \
> if (guest_needs_tdvmcall) \
> tdx_mmio_read(addr, &(val), sizeof(val)); \
> else if (guest_needs_vmgexit) \
> sev_mmio_write(addr, &(val), sizeof(val)); \
> else \
> (val) = READ_ONCE(addr); \
> })
>
On Fri, Oct 31, 2025, Sagi Shahar wrote:
> On Fri, Oct 31, 2025 at 10:15 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Fri, Oct 31, 2025, Ira Weiny wrote:
> > > Sagi Shahar wrote:
> > > > From: Erdem Aktas <erdemaktas@google.com>
> > > >
> > > > Add support for TDX guests to issue TDCALLs to the TDX module.
> > >
> > > Generally it is nice to have more details. As someone new to TDX I
> > > have to remind myself what a TDCALL is. And any random kernel developer
> > > reading this in the future will likely have even less clue than me.
> > >
> > > Paraphrased from the spec:
> > >
> > > TDCALL is the instruction used by the guest TD software (in TDX non-root
> > > mode) to invoke guest-side TDX functions. TDG.VP.VMCALL helps invoke
> > > services from the host VMM.
> > >
> > > Add support for TDX guests to invoke services from the host VMM.
> >
> > Eh, at some point a baseline amount of knowledge is required. I highly doubt
> > regurgitating the spec is going to make a huge difference
> >
> > I also dislike the above wording, because it doesn't help understand _why_ KVM
> > selftests need to support TDCALL, or _how_ the functionality will be utilized.
> > E.g. strictly speaking, we could write KVM selftests without ever doing a single
> > TDG.VP.VMCALL, because we control both sides (guest and VMM). And I have a hard
> > time belive name-dropping TDG.VP.VMCALL is going to connect the dots between
> > TDCALL and the "tunneling" scheme defined by the GHCI for requesting emulation
> > of "legacy" functionality".
> >
> > What I would like to know is why selftests are copy-pasting the kernel's scheme
> > for marshalling data to/from the registers used by TDCALL, how selftests are
> > expected to utilize TDCALL, etc. I'm confident that if someone actually took the
> > time to write a changelog explaining those details, then what TDCALL "is" will
> > be fairly clear, even if the reader doesn't know exactly what it is.
> >
> > E.g. IMO this is ugly and lazy on multiple fronts:
>
> To give some context to why this was done this way: Part of the reason
> for the selftests is to test the GHCI protocol itself.
The only part of the protocol that we're actually testing is the guest<=>KVM
interaction. Testing the guest<=>VMM interaction is self-referential, i.e. we're
validating that we implemented the guest and VMM sides correctly, which is all
kinds of silly.
> Some of the selftests will issue calls with purposely invalid arguments to
> ensure KVM handles these cases properly. For example, issuing a port IO calls
> with sizes other than 1,2 or 4 and ensure we get an error on the guest side.
That's fine, great in fact, but that's completely orthogonal to how selftests
implement the literal guest or VMM code.
> The code was intentionally written to be specific to TDX so we can
> test the TDX GHCI spec itself.
That's 100% possible without copy+pasting the kernel, and also 100% possible
while also providing sane, common interaces.
> As I understand it, you want the selftests to operate at a higher
> level and abstract away the specific GHCI details so that the code can
> be shared between TDX and SEV.
No, I want us to think critically about what we're actually doing, and I want us
to write code that is maintainable and as easy to follow as possible.
> I can refactor the code to abstract away implementation details. However,
> tests that want to exercise the API at a fine-grained level to test different
> arguments will need to define these TDCALLs themselves.
It's not so much about abstracting details as it is about making it easy to write
tests. Yes, some TDX-specific tests will need to know the gory details. But in
the grand scheme, those will be a very tiny percentage of all KVM selftests.
E.g. in all likelihood there should be literally _one_ test to validate that KVM
and the TDX-Module honor the guest<=>KVM GHCI contract. Or maybe one test per
instruction/operation. Everything else, even tests that are TDX specific, should
not care one whit about the GHCI.
> These calls were placed in a header that can be included in the guest
> code. I can add higher level wrappers that can be used for common
> code.
>
> >
> > uint64_t tdg_vp_vmcall_ve_request_mmio_write(uint64_t address, uint64_t size,
> > uint64_t data_in)
> > {
> > struct tdx_tdcall_args args = {
> > .r10 = TDG_VP_VMCALL,
> > .r11 = TDG_VP_VMCALL_VE_REQUEST_MMIO,
> > .r12 = size,
> > .r13 = MMIO_WRITE,
> > .r14 = address,
> > .r15 = data_in,
> > };
> >
> > return __tdx_tdcall(&args, 0);
> > }
> >
> > First, these are KVM selftests, there's no need to provide a super fancy namespace
> > because we are "competing" with thousands upon thousands of lines of code from
> > other components and subsystems.
> >
> > Similarly, tdg_vp_vmcall_ve_request_mmio_write() is absurdly verbose. Referencing
> > #VE in any way is also flat out wrong.
>
> This name was taken from the GHCI spec: TDG.VP.VMCALL<#VE.RequestMMIO>
> ("Intel TDX Guest-Hypervisor Communication Interface v1.5" section 3.7)
I know, and I'm saying throw away the GHCI except for when it's absolutely necessary
to care what the GHCI says.
© 2016 - 2025 Red Hat, Inc.