From: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
To: "Li, Xiaoyao" <xiaoyao.li@intel.com>,
"changyuanl@google.com" <changyuanl@google.com>,
"pbonzini@redhat.com" <pbonzini@redhat.com>,
"seanjc@google.com" <seanjc@google.com>,
"Wu, Binbin" <binbin.wu@intel.com>,
"Yamahata, Isaku" <isaku.yamahata@intel.com>
Cc: "bp@alien8.de" <bp@alien8.de>, "x86@kernel.org" <x86@kernel.org>,
"kas@kernel.org" <kas@kernel.org>,
"hpa@zytor.com" <hpa@zytor.com>,
"mingo@redhat.com" <mingo@redhat.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
"tglx@kernel.org" <tglx@kernel.org>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"linux-coco@lists.linux.dev" <linux-coco@lists.linux.dev>
Subject: Re: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
Date: Tue, 24 Feb 2026 01:57:46 +0000 [thread overview]
Message-ID: <213d614fe73e183a230c8f4e0c8fa1cc3d45df39.camel@intel.com> (raw)
In-Reply-To: <20260223214336.722463-1-changyuanl@google.com>
+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@google.com>
> Signed-off-by: Changyuan Lyu <changyuanl@google.com>
> ---
> 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
> --
next prev parent reply other threads:[~2026-02-24 1:57 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-23 21:43 [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs Changyuan Lyu
2026-02-24 1:57 ` Edgecombe, Rick P [this message]
2026-02-24 8:50 ` Binbin Wu
2026-02-24 16:03 ` Sean Christopherson
2026-02-24 18:45 ` Edgecombe, Rick P
2026-02-24 20:42 ` Sean Christopherson
2026-02-24 21:44 ` Edgecombe, Rick P
2026-02-25 0:18 ` Binbin Wu
2026-02-25 3:23 ` Binbin Wu
2026-02-25 13:59 ` Sean Christopherson
2026-02-25 15:03 ` Binbin Wu
2026-02-24 21:29 ` Changyuan Lyu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=213d614fe73e183a230c8f4e0c8fa1cc3d45df39.camel@intel.com \
--to=rick.p.edgecombe@intel.com \
--cc=binbin.wu@intel.com \
--cc=bp@alien8.de \
--cc=changyuanl@google.com \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=isaku.yamahata@intel.com \
--cc=kas@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=linux-coco@lists.linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=tglx@kernel.org \
--cc=x86@kernel.org \
--cc=xiaoyao.li@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox