All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Jason Andryuk" <jason.andryuk@amd.com>,
	"Xenia Ragiadakou" <xenia.ragiadakou@amd.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	xen-devel@lists.xenproject.org
Subject: Re: [RFC PATCH 09/11] x86: Migrate spec_ctrl vendor checks to x86_vendor_is()
Date: Thu, 11 Dec 2025 11:31:33 +0100	[thread overview]
Message-ID: <DEVBH18RU4WL.2GFVGYVC8SWAC@amd.com> (raw)
In-Reply-To: <73146271-c849-4d16-8eb8-80e7d59f42f2@suse.com>

On Mon Dec 8, 2025 at 5:04 PM CET, Jan Beulich wrote:
> On 26.11.2025 17:44, Alejandro Vallejo wrote:
>> @@ -938,7 +938,7 @@ static bool __init retpoline_calculations(void)
>>   */
>>  static bool __init rsb_is_full_width(void)
>>  {
>> -    if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
>> +    if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) ||
>
> One other aspect: If already you touch lines still using the old (being
> phased out) field names, please rename at the same time. This may then
> also help with line length in some cases.
>
> Jan

Yes, of course. I didn't even notice the difference at first. On the note on
length, I'm revising the idea, keeping the same principle but making it less
verbose.

Seeing how both you and Andrew seem onboard with dropping cross-vendor support
and having these turn into constants, I'm leaning into transforming everything
to a single "cpu_vendor", both host checks and policy checks would become
something of the form:

    if ( cpu_vendor != X86_VENDOR_INTEL )
      ...

    if ( cpu_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON))
      ...

where cpu_vendor would be defined as a runtime check on boot_cpu_data.vendor 
masked with X86_ENABLED_VENDORS when multiple vendors are compiled in, and
the single vendor compiled-in. Perhaps something like... (untested)

#define cpu_vendor \
    ((!IS_ENABLED(CONFIG_UNKNOWN_CPU_VENDOR) && X86_ENABLED_VENDORS &&
      (X86_ENABLED_VENDORS == ISOLATE_LSB(X86_ENABLED_VENDORS)))
        ? X86_ENABLED_VENDORS
        : (boot_cpu_data.vendor & X86_ENABLED_VENDORS))

I _think_ it would have the same DCE implications, but it would be much nicer
to read at the callsites and cause far fewer line wraps.

This brings down complexity, allows for the switches to stay (DCE understands
unreachable labels) and improves the general code quality.

I'm busy atm chasing emulator woes, but a non-rfc series of this will reach
the mailing list at some point. I'll make sure to s/x86_vendor/vendor while at
it.

Cheers,
Alejandro


  reply	other threads:[~2025-12-11 10:31 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-26 16:44 [RFC PATCH 00/11] x86 vendor check optimisations Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig Alejandro Vallejo
2025-11-27  9:43   ` Jan Beulich
2025-11-27 12:36     ` Alejandro Vallejo
2025-11-27 13:19       ` Jan Beulich
2025-11-27 13:44         ` Alejandro Vallejo
2025-11-27 14:00           ` Jan Beulich
2025-11-27  9:59   ` Jan Beulich
2025-11-27 12:37     ` Alejandro Vallejo
2025-11-27 21:01   ` Andrew Cooper
2025-11-26 16:44 ` [RFC PATCH 02/11] x86: Reject CPU policies with vendors other than the host's Alejandro Vallejo
2025-11-27 20:15   ` Andrew Cooper
2025-11-26 16:44 ` [RFC PATCH 03/11] x86: Add x86_vendor_is() by itself before using it Alejandro Vallejo
2025-11-27 10:46   ` Jan Beulich
2025-11-27 13:15     ` Alejandro Vallejo
2025-11-27 13:37       ` Jan Beulich
2025-11-27 14:06         ` Alejandro Vallejo
2025-11-27 14:14           ` Jan Beulich
2025-11-27 20:36       ` Andrew Cooper
2025-11-28  9:08         ` Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 04/11] x86: Refactor early vendor lookup code to use x86_vendor_is() Alejandro Vallejo
2025-11-27 10:51   ` Jan Beulich
2025-11-27 13:35     ` Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 05/11] x86: Conditionalise the inclusion of Hygon/Centaur/Shanghai cpu/ files Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 06/11] x86: Migrate switch-based vendor checks to x86_vendor_is() Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 07/11] x86: Migrate MSR handler " Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 08/11] x86: Migrate insn emulator to use x86_vendor_is() Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 09/11] x86: Migrate spec_ctrl vendor checks to x86_vendor_is() Alejandro Vallejo
2025-12-08 16:01   ` Jan Beulich
2025-12-08 16:04   ` Jan Beulich
2025-12-11 10:31     ` Alejandro Vallejo [this message]
2025-12-11 15:38       ` Andrew Cooper
2025-12-11 16:58         ` Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 10/11] x86: Migrate everything under cpu/ to use x86_vendor_is() Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 11/11] x86: Migrate every remaining raw vendor check to x86_vendor_is() Alejandro Vallejo
2025-11-27 22:11 ` [RFC PATCH 00/11] x86 vendor check optimisations Andrew Cooper
2025-11-28  9:18   ` Alejandro Vallejo

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=DEVBH18RU4WL.2GFVGYVC8SWAC@amd.com \
    --to=alejandro.garciavallejo@amd.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=jason.andryuk@amd.com \
    --cc=jbeulich@suse.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    --cc=xenia.ragiadakou@amd.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 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.