b/drivers/hwmon/peci/cputemp.c | 10 ++++----- b/drivers/peci/core.c | 4 +-- b/drivers/peci/cpu.c | 16 +++++++-------- b/drivers/peci/device.c | 40 ++++++++++++--------------------------- b/drivers/peci/internal.h | 4 +-- b/include/linux/peci-cpu.h | 42 ++++++++++++++++------------------------- b/include/linux/peci.h | 2 - 7 files changed, 48 insertions(+), 70 deletions(-)
From: Dave Hansen <dave.hansen@linux.intel.com>
tl;dr: The non-x86 PECI driver #includes an arch/x86 header. This is
ostensibly to avoid duplicating CPU model number constants, but the
result is complexity and duplicated *code* which is a far worse fate
than duplicated constants.
Remove the PECI dependency on arch/x86 by adding a list of supported
"target" CPU models in the driver.
This is only compile tested.
Long version:
== Background ==
The "PECI" driver runs on non-x86 hardware inside an x86 system. It
talks to the x86 CPU. The PECI hardware has different features based on
platform generations and uses the CPU model to control feature
detections.
Basically, instead of a PCI or USB device ID that a USB or PCI driver
would use, the PECI driver uses the CPU model (and family).
The arch/x86 code unsurprisingly has a list of CPU model numbers and the
PECI code currently reuses that list. But the arch/x86 list is
maintained in the "Display" format which is different than the binary
format that CPUID (and PECI hardware) uses.
== Problem ==
The end result is that the PECI code #includes the arch/x86 constants
header and then duplicates some code that transforms the CPUID to the
"Display" format. This is fragile because it's easy for us x86 folks to
break the PECI driver when assuming that arch/x86 is x86-only.
== Solution ==
Remove the arch/x86 dependency. Instead of duplicating the
CPUID=>Display functionality, just duplicate the constants.
Also rename the formerly "x86_vfm" variables. They are not in the VFM
format any longer. They are purely device IDs. Name them appropriately.
The result is a net code removal. The only downside is that the PECI
folks need to add a #define whenever there is a new CPU model. But, they
need to go add new CPU model to the driver explicitly *anyway*.
== Notes ==
One little wrinkle in this is that the CPU identifier that comes back
from the PECI hardware contains the CPU stepping just like
CPUID.01H:EAX. But the stepping is ignored by the PECI driver.
So, the PECI_INTEL_* identifiers are just defined with the stepping
shifted off the beginning. They could have been defined with a 0 there
and then have the stepping masked somewhere.
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Iwona Winiarska <iwona.winiarska@intel.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: linux-hwmon@vger.kernel.org
Cc: openbmc@lists.ozlabs.org
Cc: Tony Luck <tony.luck@intel.com>
Cc: x86@kernel.org
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
b/drivers/hwmon/peci/cputemp.c | 10 ++++-----
b/drivers/peci/core.c | 4 +--
b/drivers/peci/cpu.c | 16 +++++++--------
b/drivers/peci/device.c | 40 ++++++++++++---------------------------
b/drivers/peci/internal.h | 4 +--
b/include/linux/peci-cpu.h | 42 ++++++++++++++++-------------------------
b/include/linux/peci.h | 2 -
7 files changed, 48 insertions(+), 70 deletions(-)
diff -puN drivers/peci/device.c~peci-sanity drivers/peci/device.c
--- a/drivers/peci/device.c~peci-sanity 2026-02-18 08:19:42.368396631 -0800
+++ b/drivers/peci/device.c 2026-02-18 08:55:48.107887339 -0800
@@ -57,39 +57,25 @@ static int peci_get_cpu_id(struct peci_d
if (ret)
goto out_req_free;
+ /*
+ * The id that comes back from the hardware is in the raw
+ * format of x86 CPUID.01H:EAX leaf and includes the CPU
+ * Model, Family and Stepping.
+ */
*cpu_id = peci_request_data_readl(req);
+
+ /*
+ * Remove the stepping (CPUID.01H:EAX[3:0]) to match the
+ * PECI_INTEL_* identifiers:
+ */
+ *cpu_id >>= 4;
+
out_req_free:
peci_request_free(req);
return ret;
}
-static unsigned int peci_x86_cpu_family(unsigned int sig)
-{
- unsigned int x86;
-
- x86 = (sig >> 8) & 0xf;
-
- if (x86 == 0xf)
- x86 += (sig >> 20) & 0xff;
-
- return x86;
-}
-
-static unsigned int peci_x86_cpu_model(unsigned int sig)
-{
- unsigned int fam, model;
-
- fam = peci_x86_cpu_family(sig);
-
- model = (sig >> 4) & 0xf;
-
- if (fam >= 0x6)
- model += ((sig >> 16) & 0xf) << 4;
-
- return model;
-}
-
static int peci_device_info_init(struct peci_device *device)
{
u8 revision;
@@ -100,7 +86,7 @@ static int peci_device_info_init(struct
if (ret)
return ret;
- device->info.x86_vfm = IFM(peci_x86_cpu_family(cpu_id), peci_x86_cpu_model(cpu_id));
+ device->info.device_id = cpu_id;
ret = peci_get_revision(device, &revision);
if (ret)
diff -puN drivers/peci/internal.h~peci-sanity drivers/peci/internal.h
--- a/drivers/peci/internal.h~peci-sanity 2026-02-18 08:19:42.370396706 -0800
+++ b/drivers/peci/internal.h 2026-02-18 08:19:42.388397383 -0800
@@ -66,11 +66,11 @@ struct peci_request *peci_xfer_ep_mmio64
/**
* struct peci_device_id - PECI device data to match
* @data: pointer to driver private data specific to device
- * @x86_vfm: device vendor-family-model
+ * @device_id: device identifier, includes CPU vendor-family-model
*/
struct peci_device_id {
const void *data;
- u32 x86_vfm;
+ u32 device_id;
};
extern const struct device_type peci_device_type;
diff -puN include/linux/peci.h~peci-sanity include/linux/peci.h
--- a/include/linux/peci.h~peci-sanity 2026-02-18 08:19:42.371396743 -0800
+++ b/include/linux/peci.h 2026-02-18 08:19:42.388397383 -0800
@@ -72,7 +72,7 @@ static inline struct peci_controller *to
struct peci_device {
struct device dev;
struct {
- u32 x86_vfm;
+ u32 device_id;
u8 peci_revision;
u8 socket_id;
} info;
diff -puN drivers/hwmon/peci/cputemp.c~peci-sanity drivers/hwmon/peci/cputemp.c
--- a/drivers/hwmon/peci/cputemp.c~peci-sanity 2026-02-18 08:19:42.373396819 -0800
+++ b/drivers/hwmon/peci/cputemp.c 2026-02-18 08:19:42.388397383 -0800
@@ -340,11 +340,11 @@ static int init_core_mask(struct peci_cp
int ret;
/* Get the RESOLVED_CORES register value */
- switch (peci_dev->info.x86_vfm) {
- case INTEL_ICELAKE_X:
- case INTEL_ICELAKE_D:
- case INTEL_SAPPHIRERAPIDS_X:
- case INTEL_EMERALDRAPIDS_X:
+ switch (peci_dev->info.device_id) {
+ case PECI_INTEL_ICELAKE_X:
+ case PECI_INTEL_ICELAKE_D:
+ case PECI_INTEL_SAPPHIRERAPIDS_X:
+ case PECI_INTEL_EMERALDRAPIDS_X:
ret = peci_ep_pci_local_read(peci_dev, 0, reg->bus, reg->dev,
reg->func, reg->offset + 4, &data);
if (ret)
diff -puN drivers/peci/core.c~peci-sanity drivers/peci/core.c
--- a/drivers/peci/core.c~peci-sanity 2026-02-18 08:19:42.375396894 -0800
+++ b/drivers/peci/core.c 2026-02-18 08:19:42.388397383 -0800
@@ -163,8 +163,8 @@ EXPORT_SYMBOL_NS_GPL(devm_peci_controlle
static const struct peci_device_id *
peci_bus_match_device_id(const struct peci_device_id *id, struct peci_device *device)
{
- while (id->x86_vfm != 0) {
- if (id->x86_vfm == device->info.x86_vfm)
+ while (id->device_id != 0) {
+ if (id->device_id == device->info.device_id)
return id;
id++;
}
diff -puN include/linux/peci-cpu.h~peci-sanity include/linux/peci-cpu.h
--- a/include/linux/peci-cpu.h~peci-sanity 2026-02-18 08:19:42.376396932 -0800
+++ b/include/linux/peci-cpu.h 2026-02-18 08:19:42.388397383 -0800
@@ -6,31 +6,23 @@
#include <linux/types.h>
-/* Copied from x86 <asm/processor.h> */
-#define X86_VENDOR_INTEL 0
-
-/* Copied from x86 <asm/cpu_device_id.h> */
-#define VFM_MODEL_BIT 0
-#define VFM_FAMILY_BIT 8
-#define VFM_VENDOR_BIT 16
-#define VFM_RSVD_BIT 24
-
-#define VFM_MODEL_MASK GENMASK(VFM_FAMILY_BIT - 1, VFM_MODEL_BIT)
-#define VFM_FAMILY_MASK GENMASK(VFM_VENDOR_BIT - 1, VFM_FAMILY_BIT)
-#define VFM_VENDOR_MASK GENMASK(VFM_RSVD_BIT - 1, VFM_VENDOR_BIT)
-
-#define VFM_MODEL(vfm) (((vfm) & VFM_MODEL_MASK) >> VFM_MODEL_BIT)
-#define VFM_FAMILY(vfm) (((vfm) & VFM_FAMILY_MASK) >> VFM_FAMILY_BIT)
-#define VFM_VENDOR(vfm) (((vfm) & VFM_VENDOR_MASK) >> VFM_VENDOR_BIT)
-
-#define VFM_MAKE(_vendor, _family, _model) ( \
- ((_model) << VFM_MODEL_BIT) | \
- ((_family) << VFM_FAMILY_BIT) | \
- ((_vendor) << VFM_VENDOR_BIT) \
-)
-/* End of copied code */
-
-#include "../../arch/x86/include/asm/intel-family.h"
+/*
+ * These are in the format of and match the values of the x86
+ * CPUID.01H:EAX[19:4]. They encode the model and family of
+ * the CPU with which the driver is interfacing.
+ *
+ * All driver functionality is common across all CPU steppings
+ * of a given model, so the lower 4 stepping bits are excluded
+ * from these IDs.
+ */
+#define PECI_INTEL_HASWELL_X 0x306C
+#define PECI_INTEL_BROADWELL_X 0x406F
+#define PECI_INTEL_BROADWELL_D 0x5066
+#define PECI_INTEL_SKYLAKE_X 0x5065
+#define PECI_INTEL_ICELAKE_X 0x606A
+#define PECI_INTEL_ICELAKE_D 0x606C
+#define PECI_INTEL_SAPPHIRERAPIDS_X 0x806F
+#define PECI_INTEL_EMERALDRAPIDS_X 0xC06F
#define PECI_PCS_PKG_ID 0 /* Package Identifier Read */
#define PECI_PKG_ID_CPU_ID 0x0000 /* CPUID Info */
diff -puN drivers/peci/cpu.c~peci-sanity drivers/peci/cpu.c
--- a/drivers/peci/cpu.c~peci-sanity 2026-02-18 08:19:42.380397082 -0800
+++ b/drivers/peci/cpu.c 2026-02-18 08:19:42.389397421 -0800
@@ -294,35 +294,35 @@ peci_cpu_probe(struct peci_device *devic
static const struct peci_device_id peci_cpu_device_ids[] = {
{ /* Haswell Xeon */
- .x86_vfm = INTEL_HASWELL_X,
+ .device_id = PECI_INTEL_HASWELL_X,
.data = "hsx",
},
{ /* Broadwell Xeon */
- .x86_vfm = INTEL_BROADWELL_X,
+ .device_id = PECI_INTEL_BROADWELL_X,
.data = "bdx",
},
{ /* Broadwell Xeon D */
- .x86_vfm = INTEL_BROADWELL_D,
+ .device_id = PECI_INTEL_BROADWELL_D,
.data = "bdxd",
},
{ /* Skylake Xeon */
- .x86_vfm = INTEL_SKYLAKE_X,
+ .device_id = PECI_INTEL_SKYLAKE_X,
.data = "skx",
},
{ /* Icelake Xeon */
- .x86_vfm = INTEL_ICELAKE_X,
+ .device_id = PECI_INTEL_ICELAKE_X,
.data = "icx",
},
{ /* Icelake Xeon D */
- .x86_vfm = INTEL_ICELAKE_D,
+ .device_id = PECI_INTEL_ICELAKE_D,
.data = "icxd",
},
{ /* Sapphire Rapids Xeon */
- .x86_vfm = INTEL_SAPPHIRERAPIDS_X,
+ .device_id = PECI_INTEL_SAPPHIRERAPIDS_X,
.data = "spr",
},
{ /* Emerald Rapids Xeon */
- .x86_vfm = INTEL_EMERALDRAPIDS_X,
+ .device_id = PECI_INTEL_EMERALDRAPIDS_X,
.data = "emr",
},
{ }
_On Wed, 18 Feb 2026 09:03:01 -0800 Dave Hansen <dave.hansen@linux.intel.com> wrote: > From: Dave Hansen <dave.hansen@linux.intel.com> > > tl;dr: The non-x86 PECI driver #includes an arch/x86 header. This is > ostensibly to avoid duplicating CPU model number constants, but the > result is complexity and duplicated *code* which is a far worse fate > than duplicated constants. Is is possible/sensible to add a check in one file that includes both headers that the constants match? That will help pick up any typos. David
On 2/19/26 02:20, David Laight wrote: >> tl;dr: The non-x86 PECI driver #includes an arch/x86 header. This is >> ostensibly to avoid duplicating CPU model number constants, but the >> result is complexity and duplicated *code* which is a far worse fate >> than duplicated constants. > Is is possible/sensible to add a check in one file that includes both > headers that the constants match? > > That will help pick up any typos. I don't think it's worth it. Honestly, the _only_ reason to share names is to avoid doubling the inevitable name bikeshedding that happens every time we try to name a CPU model. I honestly don't think it matters if the names diverge otherwise.
On 2/18/2026 9:03 AM, Dave Hansen wrote:
>
> b/drivers/hwmon/peci/cputemp.c | 10 ++++-----
> b/drivers/peci/core.c | 4 +--
> b/drivers/peci/cpu.c | 16 +++++++--------
> b/drivers/peci/device.c | 40 ++++++++++++---------------------------
> b/drivers/peci/internal.h | 4 +--
> b/include/linux/peci-cpu.h | 42 ++++++++++++++++-------------------------
> b/include/linux/peci.h | 2 -
> 7 files changed, 48 insertions(+), 70 deletions(-)
>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Everything mostly looks good. A few small suggestions below.
> diff -puN include/linux/peci.h~peci-sanity include/linux/peci.h
> --- a/include/linux/peci.h~peci-sanity 2026-02-18 08:19:42.371396743 -0800
> +++ b/include/linux/peci.h 2026-02-18 08:19:42.388397383 -0800
> @@ -72,7 +72,7 @@ static inline struct peci_controller *to
> struct peci_device {
> struct device dev;
> struct {
> - u32 x86_vfm;
> + u32 device_id;
There is a kernel-doc comment on top of this struct as well that needs
to reflect the change.
> u8 peci_revision;
> u8 socket_id;
> } info;
> diff -puN include/linux/peci-cpu.h~peci-sanity include/linux/peci-cpu.h
> -#include "../../arch/x86/include/asm/intel-family.h"
> +/*
> + * These are in the format of and match the values of the x86
> + * CPUID.01H:EAX[19:4]. They encode the model and family of
Can we include the extended family bits in the *comment* to say
EAX[27:4]? I expect Family 19 (DMR) will be added soonish, which will
make the comment stale.
> + * the CPU with which the driver is interfacing.
> + *
> + * All driver functionality is common across all CPU steppings
> + * of a given model, so the lower 4 stepping bits are excluded
> + * from these IDs.
> + */
> +#define PECI_INTEL_HASWELL_X 0x306C
> +#define PECI_INTEL_BROADWELL_X 0x406F
> +#define PECI_INTEL_BROADWELL_D 0x5066
> +#define PECI_INTEL_SKYLAKE_X 0x5065
> +#define PECI_INTEL_ICELAKE_X 0x606A
> +#define PECI_INTEL_ICELAKE_D 0x606C
> +#define PECI_INTEL_SAPPHIRERAPIDS_X 0x806F
> +#define PECI_INTEL_EMERALDRAPIDS_X 0xC06F
>
The _D has been used in Intel official product names such as "XEON D".
AFAIU, The _X notation is specific to intel-family.h. Should that be
explained in the comment above?
Something like:
* _X - regular server parts
* _D - micro server parts
> #define PECI_PCS_PKG_ID 0 /* Package Identifier Read */
> #define PECI_PKG_ID_CPU_ID 0x0000 /* CPUID Info */
On 2/18/26 11:26, Sohil Mehta wrote:
>> diff -puN include/linux/peci.h~peci-sanity include/linux/peci.h
>> --- a/include/linux/peci.h~peci-sanity 2026-02-18 08:19:42.371396743 -0800
>> +++ b/include/linux/peci.h 2026-02-18 08:19:42.388397383 -0800
>> @@ -72,7 +72,7 @@ static inline struct peci_controller *to
>> struct peci_device {
>> struct device dev;
>> struct {
>> - u32 x86_vfm;
>> + u32 device_id;
>
> There is a kernel-doc comment on top of this struct as well that needs
> to reflect the change.
Thanks, I'll fix it up.
>> diff -puN include/linux/peci-cpu.h~peci-sanity include/linux/peci-cpu.h
>> -#include "../../arch/x86/include/asm/intel-family.h"
>> +/*
>> + * These are in the format of and match the values of the x86
>> + * CPUID.01H:EAX[19:4]. They encode the model and family of
>
> Can we include the extended family bits in the *comment* to say
> EAX[27:4]? I expect Family 19 (DMR) will be added soonish, which will
> make the comment stale.
Right now, all the constants are 16 bits long, that's 19:4. If a future
CPU model needs more bits, they'll make all the constants bigger and can
change the comment.
>> + * the CPU with which the driver is interfacing.
>> + *
>> + * All driver functionality is common across all CPU steppings
>> + * of a given model, so the lower 4 stepping bits are excluded
>> + * from these IDs.
>> + */
>> +#define PECI_INTEL_HASWELL_X 0x306C
>> +#define PECI_INTEL_BROADWELL_X 0x406F
>> +#define PECI_INTEL_BROADWELL_D 0x5066
>> +#define PECI_INTEL_SKYLAKE_X 0x5065
>> +#define PECI_INTEL_ICELAKE_X 0x606A
>> +#define PECI_INTEL_ICELAKE_D 0x606C
>> +#define PECI_INTEL_SAPPHIRERAPIDS_X 0x806F
>> +#define PECI_INTEL_EMERALDRAPIDS_X 0xC06F
>>
>
> The _D has been used in Intel official product names such as "XEON D".
> AFAIU, The _X notation is specific to intel-family.h. Should that be
> explained in the comment above?
>
> Something like:
>
> * _X - regular server parts
> * _D - micro server parts
I think I just verbatim copied the intel-family.h names and added PECI_.
Are you seeing something different.
BTW, we should probably comment the naming scheme and at least mention
that it should be consistent with the x86 code.
On 2/18/2026 11:30 AM, Dave Hansen wrote: >>> +#define PECI_INTEL_HASWELL_X 0x306C >>> +#define PECI_INTEL_BROADWELL_X 0x406F >>> +#define PECI_INTEL_BROADWELL_D 0x5066 >>> +#define PECI_INTEL_SKYLAKE_X 0x5065 >>> +#define PECI_INTEL_ICELAKE_X 0x606A >>> +#define PECI_INTEL_ICELAKE_D 0x606C >>> +#define PECI_INTEL_SAPPHIRERAPIDS_X 0x806F >>> +#define PECI_INTEL_EMERALDRAPIDS_X 0xC06F >>> >> >> The _D has been used in Intel official product names such as "XEON D". >> AFAIU, The _X notation is specific to intel-family.h. Should that be >> explained in the comment above? >> >> Something like: >> >> * _X - regular server parts >> * _D - micro server parts > > I think I just verbatim copied the intel-family.h names and added PECI_. > Are you seeing something different. > No. > BTW, we should probably comment the naming scheme and at least mention > that it should be consistent with the x86 code. The naming scheme is already commented in intel-family.h. Yeah, this just needs some breadcrumbs to help the user understand the naming.
On 2/18/2026 11:36 AM, Sohil Mehta wrote: > On 2/18/2026 11:30 AM, Dave Hansen wrote: > >>>> +#define PECI_INTEL_HASWELL_X 0x306C >>>> +#define PECI_INTEL_BROADWELL_X 0x406F >>>> +#define PECI_INTEL_BROADWELL_D 0x5066 >>>> +#define PECI_INTEL_SKYLAKE_X 0x5065 >>>> +#define PECI_INTEL_ICELAKE_X 0x606A >>>> +#define PECI_INTEL_ICELAKE_D 0x606C >>>> +#define PECI_INTEL_SAPPHIRERAPIDS_X 0x806F >>>> +#define PECI_INTEL_EMERALDRAPIDS_X 0xC06F >>>> >>> >>> The _D has been used in Intel official product names such as "XEON D". >>> AFAIU, The _X notation is specific to intel-family.h. Should that be >>> explained in the comment above? >>> >>> Something like: >>> >>> * _X - regular server parts >>> * _D - micro server parts >> >> I think I just verbatim copied the intel-family.h names and added PECI_. >> Are you seeing something different. >> > > No. > Scratch that. I see a difference. #define INTEL_HASWELL IFM(6, 0x3C) #define INTEL_HASWELL_X IFM(6, 0x3F) PECI_INTEL_HASWELL_X (0x306C) doesn't match INTEL_HASWELL_X instead you seem to have copied INTEL_HASWELL.
On 2/18/26 11:43, Sohil Mehta wrote: > Scratch that. I see a difference. > > #define INTEL_HASWELL IFM(6, 0x3C) > #define INTEL_HASWELL_X IFM(6, 0x3F) > > PECI_INTEL_HASWELL_X (0x306C) doesn't match INTEL_HASWELL_X instead you > seem to have copied INTEL_HASWELL. Hah! Human error. I knew should not have done that by hand! Fixed for v2.
© 2016 - 2026 Red Hat, Inc.