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

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

On 10/16/2026 9:47 AM, Sean Christopherson wrote:
> On Tue, Oct 13, 2026, Binbin Wu wrote:
>> On 2/25/2026 5:29 AM, Changyuan Lyu wrote:
>>
>> [...]
>>
>>> -----------------------
>>>
>>> >From 18b967b718911c09872c3717d8ab083fa59c4a70 Mon Sep 17 00:00:00 2001
>>> From: Changyuan Lyu <changyuanl@google.com>
>>> Date: Fri, 20 Feb 2026 09:55:28 -0800
>>> Subject: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
>>>
>>> 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.
>>>
>>> Suggested-by: Sean Christopherson <seanjc@google.com>
>>> Signed-off-by: Changyuan Lyu <changyuanl@google.com>
>>
>>
>> Hi Sean,
>>
>> When I was preparing the v5 of the TDX CPUID patch series, the internal sashiko
>> reported a issue:
>>   > @@ -2359,6 +2359,15 @@ static int setup_tdparams_cpuids(struct kvm_cpuid2 *cpuid,
>>   >  		if (!entry)
>>   >  			continue;
>>   >  
>>   > +		/*
>>   > +		 * Reject entries whose index doesn't match the expected one.
>>   > +		 * This catches userspace passing a CPUID entry with the
>>   > +		 * KVM_CPUID_FLAG_SIGNIFCANT_INDEX flag cleared when the index
>>   > +		 * is significant.
>>   > +		 */
>>   > +		if (entry->index != tmp.index)
>>   > +			return -EINVAL;
>>   
>>   [Severity: Medium]
>>   Does this unconditional check inadvertently break userspace VMMs that provide
>>   valid CPUID entries but leave the index field uninitialized for
>>   non-significant leaves?
>>   
>>   Since kvm_find_cpuid_entry2() ignores the index field when
>>   KVM_CPUID_FLAG_SIGNIFCANT_INDEX is cleared, VMMs are not architecturally
>>   required to zero the index for non-significant leaves. This check forces
>>   the index to be exactly 0 (since tmp.index is normalized to 0) even for
>>   leaves that do not have architecturally significant subleaves.
>>
>>
>> I think the issue is valid,
> 
> Yeah, requiring the index to be a specific value without SIGNIFCANT_INDEX set
> could break usrspace.
> 
>> so it needs to know whether the entry is a subleaf or not. I think it's more
> 
> By "it", I assume you mean userspace?

I meant the code to check the index mismatch in my previous implementation when
preparing the v5 of TDX CPUID validation patch series.

But I changed the implementation, which doesn't rely on the flag.
I.e. KVM validates the unsupported features against the same subleaf that the
values are copied into.

> 
>> reliable to do it via the information reported by the TDX module. i.e. my
>> patch set will rely on this patch to set KVM_CPUID_FLAG_SIGNIFCANT_INDEX for
>> entries that are indexed.
>>
>> Will you apply this patch?
>> Or I can include this patch in my patch set. Also, 0x23 is still missing in
>> cpuid_function_is_indexed(), I can add a patch to add 0x23 as well.
>>
>>> ---
>>>  arch/x86/kvm/vmx/tdx.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>>> index 2d7a4d52ccfb4..1c039eab2f3d8 100644
>>> --- a/arch/x86/kvm/vmx/tdx.c
>>> +++ b/arch/x86/kvm/vmx/tdx.c
>>> @@ -167,6 +167,7 @@ static void td_init_cpuid_entry2(struct kvm_cpuid_entry2 *entry, unsigned char i
>>>
>>>  	entry->function = (u32)td_conf->cpuid_config_leaves[idx];
>>>  	entry->index = td_conf->cpuid_config_leaves[idx] >> 32;
>>> +	entry->flags = 0;
>>>  	entry->eax = (u32)td_conf->cpuid_config_values[idx][0];
>>>  	entry->ebx = td_conf->cpuid_config_values[idx][0] >> 32;
>>>  	entry->ecx = (u32)td_conf->cpuid_config_values[idx][1];
>>> @@ -174,6 +175,9 @@ static void td_init_cpuid_entry2(struct kvm_cpuid_entry2 *entry, unsigned char i
>>>
>>>  	if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF)
>>>  		entry->index = 0;
>>> +	else
>>> +		entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> 
> Hrm, I'm getting deja vu.  Oh, this is an old thread.  *sigh*  I was expecting
> a new patch/thread, but it got "posted" inline.
> 
> I'll send a small v2.

Thanks.