lkml.org 
[lkml]   [2024]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] x86/cpu: Fix x86_match_cpu() to match just X86_VENDOR_INTEL
On Thu, May 16, 2024 at 09:29:25AM -0700, Tony Luck wrote:
> -#define X86_VENDOR_INTEL 0
> #define X86_VENDOR_CYRIX 1
> #define X86_VENDOR_AMD 2
> #define X86_VENDOR_UMC 3
> +#define X86_VENDOR_INTEL 4
> #define X86_VENDOR_CENTAUR 5
> #define X86_VENDOR_TRANSMETA 7
> #define X86_VENDOR_NSC 8
> --

From my last review:

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

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

But nope, apparently that's too much to ask. :-(

modinfo ./arch/x86/events/intel/intel-uncore.ko
filename: ./arch/x86/events/intel/intel-uncore.ko
license: GPL
srcversion: ECE38449B18DD83223B93FD
alias: cpu:type:x86,ven0000fam0006mod00B6:feature:*
alias: cpu:type:x86,ven0000fam0006mod00AF:feature:*
alias: cpu:type:x86,ven0000fam0006mod00BE:feature:*
^^^^^^^^^

Would everything still work if it said "ven0004" now?

So tglx and I just did some poking and we think the best solution would
be to add a __u16 flags field to struct x86_cpu_id right...

struct x86_cpu_id {
__u16 vendor; /* 0 2 */
__u16 family; /* 2 2 */
__u16 model; /* 4 2 */
__u16 steppings; /* 6 2 */
__u16 feature; /* 8 2 */

/* XXX 6 bytes hole, try to pack */

<--- HERE

kernel_ulong_t driver_data; /* 16 8 */

/* size: 24, cachelines: 1, members: 6 */
/* sum members: 18, holes: 1, sum holes: 6 */
/* last cacheline: 24 bytes */
};

and the 32-bit version has the same hole:

struct x86_cpu_id {
__u16 vendor; /* 0 2 */
__u16 family; /* 2 2 */
__u16 model; /* 4 2 */
__u16 steppings; /* 6 2 */
__u16 feature; /* 8 2 */

/* XXX 2 bytes hole, try to pack */

<--- HERE

kernel_ulong_t driver_data; /* 12 4 */

/* size: 16, cachelines: 1, members: 6 */
/* sum members: 14, holes: 1, sum holes: 2 */
/* last cacheline: 16 bytes */
};

And then do:

struct x86_cpu_id {
__u16 vendor;
__u16 family;
__u16 model;
__u16 steppings;
__u16 feature; /* bit index */
__u16 flags;
kernel_ulong_t driver_data;
};

#define X86_CPU_ID_FLAG_VENDOR_VALID BIT(0)

and then have the macros in arch/x86/include/asm/cpu_device_id.h set
that valid flag and then have x86_match_cpu() check it.

Then you don't risk a userspace breakage and that x86_match_cpu() crap
thing is fixed.

Thx.

--
Regards/Gruss,
Boris.

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

\
 
 \ /
  Last update: 2024-05-27 18:30    [W:0.073 / U:0.648 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site