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]
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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ