linux-kernel - RE: [PATCH v3 50/74] x86/cpu/vfm: Update drivers/hwmon/peci/cputemp.c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <SJ1PR11MB6083D0DFBC7DE286F986A453FC082@SJ1PR11MB6083.namprd11.prod.outlook.com>
Date: Tue, 16 Apr 2024 23:05:15 +0000
From: "Luck, Tony" <tony.luck@...el.com>
To: Guenter Roeck <linux@...ck-us.net>
CC: Jean Delvare <jdelvare@...e.com>, "Winiarska, Iwona"
	<iwona.winiarska@...el.com>, "linux-hwmon@...r.kernel.org"
	<linux-hwmon@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "patches@...ts.linux.dev"
	<patches@...ts.linux.dev>
Subject: RE: [PATCH v3 50/74] x86/cpu/vfm: Update drivers/hwmon/peci/cputemp.c

Guenter,

Thanks for taking a look at this patch.

> > +   case VFM_MODEL(INTEL_ICELAKE_X):
> > +   case VFM_MODEL(INTEL_ICELAKE_D):
> > +   case VFM_MODEL(INTEL_SAPPHIRERAPIDS_X):
>
> $ git describe
> v6.9-rc4-31-g96fca68c4fbf
> $ git grep VFM_MODEL
> $
>
> So I guess this depends on some other patch ?
> That might be worth mentioning, especially since
>
> $ git describe
> next-20240416
> $ git grep VFM_MODEL
> $
>
> doesn't find it either.

Yes. This depends on parts 1,2,3 of this series. I should have added
that to the "---" meta comment part of these patches that I'm spraying
out to maintainers of random subsystems that use INTEL_FAM6 defines.

> On top of that, it looks like
>
> #include <asm/cpu_device_id.h>
>
> introduces a dependency on X86 which is not currently the case.
> CONFIG_PECI is typically used on BMCs. Many of those do not use
> Intel CPUs. It does not seem appropriate to make support for PECI
> based hardware monitoring dependent on it running on an Intel CPU.

It seems that some use (or need to know about) Icelake and Sapphire Rapids.

How does this code get the value "peci_dev->info.model" used in this switch?

Given that it is doing some magic just for some recent Intel CPUs, it seems
plausible that when non-family 6 CPUs appear in the market, it might have
to grab both the family and the model to reliably determine what to do?

Simple options to avoid including <asm/cpu_device_id.h> would be
to either:

1) provide a copy of the VFM_MODEL macro here.

or

2) Keep using the old INTEL_FAM6_* #define names, but define those for
the three CPU models peci needs locally instead of getting them from
<asm/intel-family.h>. It looks like there is a somewhat convoluted path to
include that. I see in <linux/peci.cpu.h>

  #include "../../arch/x86/include/asm/intel-family.h"

Or maybe some better option?

-Tony