Re: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
From: Binbin Wu
Date: Thu Oct 15 2026 - 22:12:44 EST
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@xxxxxxxxxx>
>>> 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@xxxxxxxxxx>
>>> Signed-off-by: Changyuan Lyu <changyuanl@xxxxxxxxxx>
>>
>>
>> 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.