Re: [PATCH] x86/tdx: Fix zero-extension for CPUID emulation

[Carlos López]

On 5/13/26 12:14 AM, Dave Hansen wrote:
> On 5/12/26 14:48, Edgecombe, Rick P wrote:
>>> -	regs->ax = args.r12;
>>> -	regs->bx = args.r13;
>>> -	regs->cx = args.r14;
>>> -	regs->dx = args.r15;
>>> +	regs->ax = lower_32_bits(args.r12);
>>> +	regs->bx = lower_32_bits(args.r13);
>>> +	regs->cx = lower_32_bits(args.r14);
>>> +	regs->dx = lower_32_bits(args.r15);
>>>  
>> Can you explain the impact here? Why should the guest fixup what the VMM
>> emulates?
> 
> Oh boy.
> 
> args.r12-15 come from the VMM, right? So the VMM Can put whatever it
> wants in there.

Yes, exactly.

> CPUID (the instruction) is defined to fill in eax/ebx/ecx/edx. Those are
> 32-bit registers so the normal register rules apply: "32-bit operands
> generate a 32-bit result, zero-extended to a 64-bit result in the
> destination general-purpose register."
> 
> So a properly-behaving CPUID implementation will always end up with the
> top 32 bits empty on the four CPUID registers after a CPUID is executed.
> 
> The VMM here obviously might be naughty and might put gunk in
> args.r12/r13/r14/r15 that gets copied to ptregs->ax/bx/cx/dx which are
> 'unsigned long' on 64-bit.
> 
> The end result is that a TDX guest can use CPUID and end up having bits
> set in rax/rbx/rcx/rdx that are architecturally impossible. This patch
> is effectively fixing up the VMM naughtiness before the guest CPUID
> instance can see it.
> 
> Does anybody disagree with any of that?
> 
> Do we *want* to fix this up silently? If we catch a malicious VMM trying
> to stuff garbage into the guest, shouldn't we be a bit more upset than
> silently papering over it?

Okay, how about this (on top of the changes I already sent)?

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 831475cf4313..cd33781c8d61 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -538,6 +538,13 @@ static int handle_cpuid(struct pt_regs *regs, struct ve_info *ve)
        if (__tdx_hypercall(&args))
                return -EIO;
 
+       /* Emit a warning if the hypervisor tries to inject architecturally
+        * invalid (non-zero-extended) output values for CPUID */
+       if (upper_32_bits(args.r12) || upper_32_bits(args.r13)
+           || upper_32_bits(args.r14) || upper_32_bits(args.r15))
+               pr_warn("detected invalid CPUID result from VMM: eax=%lld ebx=%lld ecx=%lld edx=%lld",
+                                       args.r12, args.r13, args.r14, args.r15);
+
        /*
         * As per TDX GHCI CPUID ABI, r12-r15 registers contain contents of
         * EAX, EBX, ECX, EDX registers after the CPUID instruction execution.