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.