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

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

On 2/25/2026 12:03 AM, Sean Christopherson wrote:
> On Tue, Feb 24, 2026, Binbin Wu wrote:
>> On 2/24/2026 9:57 AM, Edgecombe, Rick P wrote:
>>>> 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.
> 
> +1, td_init_cpuid_entry2() should initialize flags to '0' and then set
> KVM_CPUID_FLAG_SIGNIFCANT_INDEX as appropriate.
> 
>>>> +	} 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.
> 
> Yeah, I was anticipating that we'd run afoul of leaves that aren't known to
> the kernel.  FWIW, it looks like 0x24 is also indexed.
> 
>> 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.
> 
> IMO, that's a good thing and working as intended.  WARNs aren't inherently evil.
> While the goal is to be WARN-free, in this case triggering the WARN if the TDX
> Module is updated (or new silicon arrives) is desirable, because it alerts us to
> that new behavior, so that we can go update KVM.

So it effectively leverages the TDX module's interface to retrieve the hardware
information to validate the hard-coded list.

Do we need to consider the panic_on_warn case? I guess the option will not be
enabled in a production environment?

> 
> But we should "fix" 0x23 and 0x24 before landing this patch.
>