linux-kernel - Re: [PATCH 3/3] hwmon: (coretemp) Use a model-specific bitmask to read registers

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: <d0b5ae04b4d08e2003114c4d6b6d3a040f585995.camel@intel.com>
Date: Sun, 7 Apr 2024 08:39:51 +0000
From: "Zhang, Rui" <rui.zhang@...el.com>
To: "linux@...ck-us.net" <linux@...ck-us.net>,
	"ricardo.neri-calderon@...ux.intel.com"
	<ricardo.neri-calderon@...ux.intel.com>
CC: "Neri, Ricardo" <ricardo.neri@...el.com>, "linux-hwmon@...r.kernel.org"
	<linux-hwmon@...r.kernel.org>, "lukasz.luba@....com" <lukasz.luba@....com>,
	"daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>, "jdelvare@...e.com"
	<jdelvare@...e.com>, "srinivas.pandruvada@...ux.intel.com"
	<srinivas.pandruvada@...ux.intel.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Wysocki, Rafael J"
	<rafael.j.wysocki@...el.com>, "linux-pm@...r.kernel.org"
	<linux-pm@...r.kernel.org>
Subject: Re: [PATCH 3/3] hwmon: (coretemp) Use a model-specific bitmask to
 read registers

On Sat, 2024-04-06 at 06:17 -0700, Guenter Roeck wrote:
> On Fri, Apr 05, 2024 at 06:04:16PM -0700, Ricardo Neri wrote:
> > The Intel Software Development manual defines states the
> > temperature
> > digital readout as the bits [22:16] of the
> > IA32_[PACKAGE]_THERM_STATUS
> > registers. In recent processor, however, the range is [23:16]. Use
> > a
> > model-specific bitmask to extract the temperature readout
> > correctly.
> > 
> > Instead of re-implementing model checks, extract the correct
> > bitmask
> > using the intel_tcc library. Add an 'imply' weak reverse dependency
> > on
> > CONFIG_INTEL_TCC. This captures the dependency and lets user to
> > unselect
> > them if they are so inclined. In such case, the bitmask used for
> > the
> > digital readout is [22:16] as specified in the Intel Software
> > Developer's
> > manual.
> > 
> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
> > ---
> > Cc: Daniel Lezcano <daniel.lezcano@...aro.org>
> > Cc: Lukasz Luba <lukasz.luba@....com>
> > Cc: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
> > Cc: linux-hwmon@...r.kernel.org
> > Cc: linux-pm@...r.kernel.org
> > Cc: linux-kernel@...r.kernel.org
> > Cc: stable@...r.kernel.org # v6.7+
> > ---
> >  drivers/hwmon/Kconfig    | 1 +
> >  drivers/hwmon/coretemp.c | 6 +++++-
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 83945397b6eb..11d72b3009bf 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -847,6 +847,7 @@ config SENSORS_I5500
> >  config SENSORS_CORETEMP
> >         tristate "Intel Core/Core2/Atom temperature sensor"
> >         depends on X86
> > +       imply INTEL_TCC
> 
> I do not think it is appropriate to make a hardware monitoring driver
> depend on the thermal subsystem.
> 
> NAK in the current form.
> 
Hi, Guenter,

Thanks for reviewing.

We've seen a couple of hwmon drivers depends on or imply THERMAL.
That's why we think this is an applicable solution.
Using the intel_tcc APIs can effectively reduce the future maintenance
burden because we don't need to duplicate the model list in multiple
places.

or do you have any suggestions?

thanks,
rui