From: Sean Christopherson <seanjc@google.com>
To: Binbin Wu <binbin.wu@linux.intel.com>
Cc: Rick P Edgecombe <rick.p.edgecombe@intel.com>,
Xiaoyao Li <xiaoyao.li@intel.com>,
"changyuanl@google.com" <changyuanl@google.com>,
"pbonzini@redhat.com" <pbonzini@redhat.com>,
Binbin Wu <binbin.wu@intel.com>,
Isaku Yamahata <isaku.yamahata@intel.com>,
"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 08:03:16 -0800 [thread overview]
Message-ID: <aZ3LxD5XMepnU8jh@google.com> (raw)
In-Reply-To: <fd3b58fd-a450-471a-89a3-541c3f88c874@linux.intel.com>
On Tue, Feb 24, 2026, Binbin Wu wrote:
> On 2/24/2026 9:57 AM, Edgecombe, Rick P 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.
+1, td_init_cpuid_entry2() should initialize flags to '0' and then set
KVM_CPUID_FLAG_SIGNIFCANT_INDEX as appropriate.
> >> + } 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?
>
> I guess because the list in cpuid_function_is_indexed() is hard-coded
> and 0x23 is not added into the list yet.
Yeah, I was anticipating that we'd run afoul of leaves that aren't known to
the kernel. FWIW, it looks like 0x24 is also indexed.
> It's fine for existing KVM code because cpuid_function_is_indexed() is
> only used to check that if a CPUID entry is queried without index, it
> shouldn't be included in the indexed list.
>
> But adding the consistency check here would cause compatibility issue.
> Generally, if a new CPUID indexed function is added for some new CPU and
> the TDX module reports it, KVM versions without the CPUID function in
> the list will trigger the warning.
IMO, that's a good thing and working as intended. WARNs aren't inherently evil.
While the goal is to be WARN-free, in this case triggering the WARN if the TDX
Module is updated (or new silicon arrives) is desirable, because it alerts us to
that new behavior, so that we can go update KVM.
But we should "fix" 0x23 and 0x24 before landing this patch.
next prev parent reply other threads:[~2026-02-24 16:03 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
2026-02-24 8:50 ` Binbin Wu
2026-02-24 16:03 ` Sean Christopherson [this message]
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=aZ3LxD5XMepnU8jh@google.com \
--to=seanjc@google.com \
--cc=binbin.wu@intel.com \
--cc=binbin.wu@linux.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=rick.p.edgecombe@intel.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