Re: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs

[Binbin Wu <binbin.wu@linux.intel.com>]

On 2/24/2026 9:57 AM, Edgecombe, Rick P wrote:
> +binbin
> 
> On Mon, 2026-02-23 at 13:43 -0800, Changyuan Lyu wrote:
>> Set the KVM_CPUID_FLAG_SIGNIFCANT_INDEX flag in the kvm_cpuid_entry2
>> structures returned by KVM_TDX_CAPABILITIES if the CPUID is indexed.
>> This ensures consistency with the CPUID entries returned by
>> KVM_GET_SUPPORTED_CPUID.
>>
>> Additionally, add a WARN_ON_ONCE() to verify that the TDX module's
>> reported entries align with KVM's expectations regarding indexed
>> CPUID functions.
>>
>> Suggested-by: Sean Christopherson <seanjc@google.com>
>> Signed-off-by: Changyuan Lyu <changyuanl@google.com>
>> ---
>>  arch/x86/kvm/vmx/tdx.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>> index 2d7a4d52ccfb4..0c524f9a94a6c 100644
>> --- a/arch/x86/kvm/vmx/tdx.c
>> +++ b/arch/x86/kvm/vmx/tdx.c
>> @@ -172,9 +172,15 @@ static void td_init_cpuid_entry2(struct
>> kvm_cpuid_entry2 *entry, unsigned char i
>>  	entry->ecx = (u32)td_conf->cpuid_config_values[idx][1];
>>  	entry->edx = td_conf->cpuid_config_values[idx][1] >> 32;
>>  
>> -	if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF)
>> +	if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF) {
>>  		entry->index = 0;
>> +		entry->flags &= ~KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> 
> There are two callers of this. One is already zeroed, and the other has
> stack garbage in flags. But that second caller doesn't look at the
> flags so it is harmless. Maybe it would be simpler and clearer to just
> zero init the entry struct in that caller. Then you don't need to clear
> it here. Or alternatively set flags to zero above, and then add
> KVM_CPUID_FLAG_SIGNIFCANT_INDEX if needed. Rather than manipulating a
> single bit in a field of garbage, which seems weird.
> 
>> +	} else {
>> +		entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
>> +	}
>>  
>> +	WARN_ON_ONCE(cpuid_function_is_indexed(entry->function) !=
>> +		     !!(entry->flags &
>> KVM_CPUID_FLAG_SIGNIFCANT_INDEX));
> 
> It warns on leaf 0x23 for me. Is it intentional?

I guess because the list in cpuid_function_is_indexed() is hard-coded
and 0x23 is not added into the list yet.

It's fine for existing KVM code because cpuid_function_is_indexed() is
only used to check that if a CPUID entry is queried without index, it
shouldn't be included in the indexed list.

But adding the consistency check here would cause compatibility issue.
Generally, if a new CPUID indexed function is added for some new CPU and
the TDX module reports it, KVM versions without the CPUID function in
the list will trigger the warning.


> 
> This warning kind of begs the question of how how much consistency
> there should be between KVM_TDX_CAPABILITIES and
> KVM_GET_SUPPORTED_CPUID. There was quite a bit of debate on this and in
> the end we moved forward with a solution that did the bare minimum
> consistency checking.
> 
> We actually have been looking at some potential TDX module changes to
> fix the deficiencies from not enforcing the consistency. But didn't
> consider this pattern. Can you explain more about the failure mode?  
> 
>>  	/*
>>  	 * The TDX module doesn't allow configuring the guest phys
>> addr bits
>>  	 * (EAX[23:16]).  However, KVM uses it as an interface to
>> the userspace
>> --
>