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

[Edgecombe, Rick P]

+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@xxxxxxxxxx>
> Signed-off-by: Changyuan Lyu <changyuanl@xxxxxxxxxx>
> ---
>  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?

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