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

[Changyuan Lyu]

Hi Rick!

On Tue, 24 Feb 2026 01:57:46 +0000, Edgecombe, Rick P wrote:
> On Mon, 2026-02-23 at 13:43 -0800, Changyuan Lyu 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.

Thanks for the suggestion. I agree that initializing entry->flags to 0 at
the start of td_init_cpuid_entry2() is much cleaner.

> > +	} 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?

Leaf 0x23 is not in the list of cpuid_function_is_indexed.
Thanks Binbin for the explanation!

> 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 main purpose of this patch was to make the KVM_TDX_GET_CPUID API
more intuitive from userspace VMM's perspective.
Since both KVM_TDX_CAPABILITIES and KVM_GET_SUPPORTED_CPUID return
struct kvm_cpuid_entry2, I expected the semantic of the flag in both APIs
to be the same, as I didn't find any special notes to the contrary in the
TDX documentation Documentation/virt/kvm/x86/intel-tdx.rst .

> >  	/*
> >  	 * 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
> > --

Regarding the WARN_ON_ONCE, I understand it touches on the larger
consistency and compatibility questions that require more discussion
as you and Sean mentioned. Since I am new to TDX and lack the full context
on those prior debates, I removed the WARN_ON_ONCE check and focus only on
the KVM_CPUID_FLAG_SIGNIFCANT_INDEX consistency fix, which was the core of
this patch.

Best,
Changyuan

-----------------------