public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: "Carlos López" <clopez@suse.de>
Cc: Jim Mattson <jmattson@google.com>, Borislav Petkov <bp@alien8.de>,
	kvm@vger.kernel.org,  Paolo Bonzini <pbonzini@redhat.com>,
	Thomas Gleixner <tglx@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	 "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	 "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<linux-kernel@vger.kernel.org>, Babu Moger <bmoger@amd.com>
Subject: Re: [PATCH] KVM: x86: synthesize TSA CPUID bits via SCATTERED_F()
Date: Mon, 9 Feb 2026 07:06:36 -0800	[thread overview]
Message-ID: <aYn3_PhRvHPCJTo7@google.com> (raw)
In-Reply-To: <e19b9666-b224-4fbd-92c9-82c712916a07@suse.de>

On Mon, Feb 09, 2026, Carlos López wrote:
> Hi,
> 
> On 2/9/26 6:48 AM, Jim Mattson wrote:
> > On Sun, Feb 8, 2026 at 1:14 PM Borislav Petkov <bp@alien8.de> wrote:
> >>
> >> On Sun, Feb 08, 2026 at 12:50:18PM -0800, Jim Mattson wrote:
> >>>> /*
> >>>>  * Synthesized Feature - For features that are synthesized into boot_cpu_data,
> >>>>  * i.e. may not be present in the raw CPUID, but can still be advertised to
> >>>>  * userspace.  Primarily used for mitigation related feature flags.
> >>>>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >>>>  */
> >>>> #define SYNTHESIZED_F(name)
> >>>>
> >>>>> +             SCATTERED_F(TSA_SQ_NO),
> >>>>> +             SCATTERED_F(TSA_L1_NO),
> >>>>
> >>>> And scattered are of the same type.
> >>>>
> >>>> Sean, what's the subtle difference here?
> >>>
> >>> SYNTHESIZED_F() sets the bit unconditionally. SCATTERED_F() propagates
> >>> the bit (if set) from the host's cpufeature flags.

As noted below, SYNTHESIZED_F() isn't entirely unconditional.  kvm_cpu_cap_init()
factors in boot_cpu_data.x86_capability, the problem here is that SYNTHESIZED_F()
is now being used for KVM-only leafs, and so the common code doesn't work as
intended.

> >> Yah, and I was hinting at the scarce documentation.

Maybe I could add a table showing how the XXX_F() macros map to various controls?

> >> SYNTHESIZED_F() is "Primarily used for mitigation related feature flags."
> >> SCATTERED_F() is "For features that are scattered by cpufeatures.h."
> > 
> > Ugh. I have to rescind my Reviewed-by. IIUC, SCATTERED_F() implies a
> > logical and with hardware CPUID, which means that the current proposal
> > will never set the ITS_NO bits.
> 
> Right, I see what you mean now. SCATTERED_F() will set kvm_cpu_caps
> correctly, but then this will clear the bits, because
> kvm_cpu_cap_synthesized is now 0:
> 
>     kvm_cpu_caps[leaf] &= (raw_cpuid_get(cpuid) |
>         kvm_cpu_cap_synthesized);
> 
> So to me it seems like SYNTHESIZED_F() is just wrong,

It was right when I wrote it :-) 

> since it always enables bits for KVM-only leafs.

Yes, I didn't anticipate synthesizing flags into KVM-only leafs.  

> So how about the following
> (I think Binbin Wu suggests this in his other email):
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 819c176e02ff..5e863e213f54 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -769,7 +769,8 @@ do {                                                                        \
>   */
>  #define SYNTHESIZED_F(name)                                    \
>  ({                                                             \
> -       kvm_cpu_cap_synthesized |= feature_bit(name);           \
> +       if (boot_cpu_has(X86_FEATURE_##name))                   \
> +               kvm_cpu_cap_synthesized |= feature_bit(name);           \

I would rather do this:

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 88a5426674a1..5f41924987c7 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -770,7 +770,10 @@ do {                                                                       \
 #define SYNTHESIZED_F(name)                                    \
 ({                                                             \
        kvm_cpu_cap_synthesized |= feature_bit(name);           \
-       F(name);                                                \
+                                                               \
+       BUILD_BUG_ON(X86_FEATURE_##name >= MAX_CPU_FEATURES);   \
+       if (boot_cpu_has(X86_FEATURE_##name))                   \
+               F(name);                                        \
 })
 
 /*

because I'd like to keep kvm_cpu_cap_synthesized unconditional and have
kvm_cpu_cap_features reflect what is supported.  And with

  Fixes: 31272abd5974 ("KVM: SVM: Advertise TSA CPUID bits to guests")

because everything was fine before that commit (though it was set up to fail).

>         F(name);                                                \
>  })
> 

  reply	other threads:[~2026-02-09 15:06 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-08 16:42 [PATCH] KVM: x86: synthesize TSA CPUID bits via SCATTERED_F() Carlos López
2026-02-08 18:28 ` Borislav Petkov
2026-02-08 20:50   ` Jim Mattson
2026-02-08 21:13     ` Borislav Petkov
2026-02-09  5:48       ` Jim Mattson
2026-02-09 11:40         ` Carlos López
2026-02-09 15:06           ` Sean Christopherson [this message]
2026-02-09 15:32             ` Borislav Petkov
2026-02-09 16:29               ` Sean Christopherson
2026-02-09 17:45                 ` Borislav Petkov
2026-02-09 21:12                   ` Sean Christopherson
2026-02-10 20:07                     ` Borislav Petkov
2026-02-10 23:48                       ` Sean Christopherson
2026-02-11 13:32                         ` Borislav Petkov
2026-02-11 15:54                           ` Sean Christopherson
2026-02-11 16:17                             ` Borislav Petkov
2026-02-11 16:28                               ` Sean Christopherson
2026-02-09 10:02 ` Binbin Wu
2026-02-09 11:43   ` Carlos López

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=aYn3_PhRvHPCJTo7@google.com \
    --to=seanjc@google.com \
    --cc=bmoger@amd.com \
    --cc=bp@alien8.de \
    --cc=clopez@suse.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=tglx@kernel.org \
    --cc=x86@kernel.org \
    /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