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

[Dave Hansen]

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.

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?