patches.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: "Luck, Tony" <tony.luck@intel.com>
Cc: "x86@kernel.org" <x86@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"patches@lists.linux.dev" <patches@lists.linux.dev>
Subject: Re: [PATCH] x86/cpu: Fix x86_match_cpu() to match just X86_VENDOR_INTEL
Date: Thu, 16 May 2024 09:17:59 +0200	[thread overview]
Message-ID: <20240516071759.GAZkWzJ7t83HW6BECA@fat_crate.local> (raw)
In-Reply-To: <SJ1PR11MB6083D455CE90AFC3251703B8FCEC2@SJ1PR11MB6083.namprd11.prod.outlook.com>

On Wed, May 15, 2024 at 10:07:56PM +0000, Luck, Tony wrote:
> >> There's a wildcard match in arch/x86/kernel/smpboot.c that wants
> >> to hit on any CPU made by Intel. The match used to work because
> >
> > intel_cod_cpu[] or which one?
> >
> > Please be more specific.
> 
> intel_cod_cpu[] is the only one in arch/x86/kernel/smpboot.c

Sorry Tony, commit messages are not a guessing game but a precise
explanation as to what the problem is. Please try again.

Also, this change looks like "oh, lemme change the Intel vendor number
so that my conditional works".

But the Intel vendor has been 0 for what, 30 years?

Are you sure no other code in the tree relies on that? Audited?

Also, those x86_match_cpu functions with for-loop termination
conditionals checking whether stuff is not 0 - i.e., that:

        for (m = match;
             m->vendor | m->family | m->model | m->steppings | m->feature;
	     ^^^^^^^^^^^
             m++) {

are technically wrong when a vendor 0 is valid.

So this has all worked by chance and you've hit the one case where it
didn't.

Because it's not like we don't have a "invalid vendor" define:

#define X86_VENDOR_UNKNOWN      0xff

So, *actually*, all those terminators at the end of those arrays should
have those invalid values.

And there are a gazillion of those in the tree... :-\

Lovely.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

      reply	other threads:[~2024-05-16  7:18 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-15 21:43 [PATCH] x86/cpu: Fix x86_match_cpu() to match just X86_VENDOR_INTEL Tony Luck
2024-05-15 22:00 ` Borislav Petkov
2024-05-15 22:07   ` Luck, Tony
2024-05-16  7:17     ` Borislav Petkov [this message]

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=20240516071759.GAZkWzJ7t83HW6BECA@fat_crate.local \
    --to=bp@alien8.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=tony.luck@intel.com \
    --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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).