All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.