[PATCH 0/3] hwmon: (coretemp) Report unreliable temperature readings

[PATCH 0/3] hwmon: (coretemp) Report unreliable temperature readings

[Ricardo Neri]

Hi,

Intel CPUs indicate in IA32_[PACKAGE]_THERM_STATUS whether the digital
thermal readout they expose is valid. coretemp deliberately ignores that
indication, for the reason given in commit bf6ea084ebb5 ("hwmon:
(coretemp) Do not return -EAGAIN for low temperatures"): some CPUs clear
it while the temperature is too low to be measured, and the value reported
in that state is more useful to userspace than an error would be.

The consequence is that userspace cannot distinguish a genuinely low
temperature from one the CPU could not measure. This series exposes the
indication through the standard hwmon temp%d_fault attribute, leaving
temp%d_input exactly as it is.

One user-visible effect is worth mentioning: sensors(1) prints FAULT in
place of the temperature when temp%d_fault reads 1. On a CPU that clears
the valid bit at low temperature, that core stops showing a number in the
default output, although sensors -u and -j still report it, as does
anything that reads temp%d_input from sysfs directly. A driver-custom
attribute name would avoid this, but would be invisible to generic tools.
Reporting the condition through the documented attribute looks like a
better option, but please say if you prefer otherwise.

Patch 1 is pre-work: it fixes an existing bug where, on 32-bit kernels,
the first temperature read is skipped and temp%d_input reports zero for
the first 301 seconds of uptime. Both the bug and the fix were verified
with a 32-bit kernel. It carries a Fixes: tag and does not depend on the
rest of the series.

Patch 2 stops reading TjMax on a cache hit, where the value is discarded.
It is separate so that the behavioral change is not buried in the
restructuring patch 3 does.

Patch 3 adds temp%d_fault.

Thanks and BR,
Ricardo

Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Ricardo Neri (3):
      hwmon: (coretemp) Refresh the temperature on the first read
      hwmon: (coretemp) Read TjMax only when refreshing the temperature
      hwmon: (coretemp) Add temp%d_fault sysfs attribute

 Documentation/hwmon/coretemp.rst |  4 +++
 drivers/hwmon/coretemp.c         | 71 +++++++++++++++++++++++++++++-----------
 2 files changed, 55 insertions(+), 20 deletions(-)
---
base-commit: 93f51579e7df248780214094418f205253383cc5
change-id: 20260831-coretemp-temp-fault-a44c447936ec

Best regards,
-- 
Ricardo Neri <ricardo.neri-calderon@linux.intel.com>

[PATCH 1/3] hwmon: (coretemp) Refresh the temperature on the first read

[Ricardo Neri]

show_temp() reads the status MSR only when the cached temperature is
older than one second. Commit 5c0e64dde80f ("hwmon: (coretemp) Remove
obsolete temp_data->valid") dropped the tdata->valid check that used to
force the very first read, leaving that comparison as the only trigger.

A never-updated temp_data carries a zero timestamp, which does not look
stale on 32-bit kernels: jiffies starts 300 seconds short of wrapping,
so the comparison stays false until jiffies wraps and passes HZ. For the
first 301 seconds of uptime temp%d_input reports the zero left by the
allocator rather than the CPU temperature. 64-bit kernels are
unaffected: jiffies starts at a positive value there and does not wrap.

Backdate the timestamp when the temperature data is allocated. One jiffy
older than the caching interval is stale under either word size, and the
first refresh overwrites it.

Fixes: 5c0e64dde80f ("hwmon: (coretemp) Remove obsolete temp_data->valid")
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
 drivers/hwmon/coretemp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index 5095eb057680..ace51e08e72d 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -511,6 +511,11 @@ init_temp_data(struct platform_data *pdata, unsigned int cpu, int pkg_flag)
 	tdata->cpu = cpu;
 	tdata->cpu_core_id = topology_core_id(cpu);
 	tdata->attr_size = MAX_CORE_ATTRS;
+	/*
+	 * A zero timestamp does not look stale on 32-bit, where jiffies
+	 * starts just short of wrapping. Backdate it instead.
+	 */
+	tdata->last_updated = jiffies - HZ - 1;
 	mutex_init(&tdata->update_lock);
 	return tdata;
 }

-- 
2.43.0

[PATCH 2/3] hwmon: (coretemp) Read TjMax only when refreshing the temperature

[Ricardo Neri]

show_temp() reads TjMax before checking whether the cached temperature is
still fresh, but consumes it only when recomputing that temperature. On a
cache hit the value is discarded.

Discarding it is not free. On CPUs that report TjMax in
MSR_IA32_TEMPERATURE_TARGET, get_tjmax() returns the value it just read
without storing it in tdata->tjmax, so every call reads the MSR again.
That is a cross-CPU call whenever the CPU owning the sensor is not the
current one, and it is made while holding tdata->update_lock.

Read TjMax where it is used. The value cannot go stale in the meantime:
bits 23:16 of MSR_IA32_TEMPERATURE_TARGET are read-only, and the register
is package-scoped, so every CPU of the package reads the same value.

The temperature reported to userspace does not change.

Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
 drivers/hwmon/coretemp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index ace51e08e72d..0ab6bbff5637 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -402,9 +402,9 @@ static ssize_t show_temp(struct device *dev,
 
 	mutex_lock(&tdata->update_lock);
 
-	tjmax = get_tjmax(tdata, dev);
 	/* Check whether the time interval has elapsed */
 	if (time_after(jiffies, tdata->last_updated + HZ)) {
+		tjmax = get_tjmax(tdata, dev);
 		rdmsrq_on_cpu(tdata->cpu, tdata->status_reg, &val.q);
 		/*
 		 * Ignore the valid bit. In all observed cases the register

-- 
2.43.0

[PATCH 3/3] hwmon: (coretemp) Add temp%d_fault sysfs attribute

[Ricardo Neri]

The temp%d_input sysfs attribute reports the CPU core temperature
regardless of the valid bit (bit 31) in IA32_[PACKAGE]_THERM_STATUS.
Commit bf6ea084ebb5 ("hwmon: (coretemp) Do not return -EAGAIN for low
temperatures") established this behavior after observing that some Intel
CPUs do not set the valid bit while the temperature is too low to be
measured. The value reported is of more use than an error. Changing this is
not an option: the interface is long-established and userspace relies on
it never failing.

Userspace, however, cannot distinguish a genuinely low temperature from
one the CPU could not measure. Until that commit it could: reading
temp%d_input returned -EAGAIN. That signal was removed and nothing
replaced it. Report the inverse of the valid bit through the standard hwmon
temp%d_fault attribute. Its documented meaning is that the measurement
for that channel should not be trusted. Tools built on libsensors consume
the standard attribute.

temp%d_input continues to ignore the valid bit. A new update_temp() helper
refreshes the cached temperature and its fault state together, so both
attributes are served from the same reading of the status register.

Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
 Documentation/hwmon/coretemp.rst |  4 +++
 drivers/hwmon/coretemp.c         | 68 +++++++++++++++++++++++++++-------------
 2 files changed, 51 insertions(+), 21 deletions(-)

diff --git a/Documentation/hwmon/coretemp.rst b/Documentation/hwmon/coretemp.rst
index 349301683381..e3e3205b5155 100644
--- a/Documentation/hwmon/coretemp.rst
+++ b/Documentation/hwmon/coretemp.rst
@@ -50,6 +50,10 @@ All sysfs entries are named with their core_id (represented here by 'X').
 
 ================= ========================================================
 tempX_input	  Core temperature (in millidegrees Celsius).
+tempX_fault	  Set when the CPU reports the reading in tempX_input as
+		  invalid. Some CPUs do this while the temperature is too
+		  low to be measured. tempX_input still reports a value in
+		  this state; it is low or zero.
 tempX_max	  Maximum recommended operating temperature (Tcontrol).
 		  All cooling devices should be turned on.
 tempX_crit	  Maximum junction temperature (in millidegrees Celsius).
diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index 0ab6bbff5637..e0fb7c0ef4d0 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -45,6 +45,7 @@ enum coretemp_attr_index {
 	ATTR_LABEL,
 	ATTR_CRIT_ALARM,
 	ATTR_TEMP,
+	ATTR_TEMP_FAULT,
 	ATTR_TJMAX,
 	ATTR_TTARGET,
 	MAX_CORE_ATTRS = ATTR_TJMAX + 1,	/* Maximum no of basic attrs */
@@ -60,6 +61,9 @@ enum coretemp_attr_index {
 
 /*
  * Per-Core Temperature Data
+ * @temp_fault: The temperature in @temp is not to be trusted. Mirrors the
+ *		meaning of the tempX_fault attribute, and is therefore the
+ *		inverse of the valid bit the CPU reports in @status_reg.
  * @tjmax: The static tjmax value when tjmax cannot be retrieved from
  *		IA32_TEMPERATURE_TARGET MSR.
  * @last_updated: The time when the current temperature value was updated
@@ -72,6 +76,7 @@ enum coretemp_attr_index {
  */
 struct temp_data {
 	int temp;
+	bool temp_fault;
 	int tjmax;
 	unsigned long last_updated;
 	unsigned int cpu;
@@ -393,41 +398,62 @@ static ssize_t show_ttarget(struct device *dev,
 	return sprintf(buf, "%d\n", ttarget);
 }
 
-static ssize_t show_temp(struct device *dev,
-			 struct device_attribute *devattr, char *buf)
+/* Requires tdata->update_lock to be held. */
+static void update_temp(struct temp_data *tdata, struct device *dev)
 {
 	struct msr val;
-	struct temp_data *tdata = container_of(devattr, struct temp_data, sd_attrs[ATTR_TEMP]);
 	int tjmax;
 
-	mutex_lock(&tdata->update_lock);
-
 	/* Check whether the time interval has elapsed */
-	if (time_after(jiffies, tdata->last_updated + HZ)) {
-		tjmax = get_tjmax(tdata, dev);
-		rdmsrq_on_cpu(tdata->cpu, tdata->status_reg, &val.q);
-		/*
-		 * Ignore the valid bit. In all observed cases the register
-		 * value is either low or zero if the valid bit is 0.
-		 * Return it instead of reporting an error which doesn't
-		 * really help at all.
-		 */
-		tdata->temp = tjmax - ((val.l >> 16) & 0xff) * 1000;
-		tdata->last_updated = jiffies;
-	}
+	if (!time_after(jiffies, tdata->last_updated + HZ))
+		return;
+
+	tjmax = get_tjmax(tdata, dev);
+
+	rdmsrq_on_cpu(tdata->cpu, tdata->status_reg, &val.q);
+
+	/*
+	 * Cache the temperature even when the CPU reports it as invalid. In
+	 * all observed cases the register value is either low or zero if the
+	 * valid bit is 0, which is of more use than reporting an error.
+	 * tempX_fault tells userspace when the temperature is unreliable.
+	 */
+	tdata->temp_fault = !((val.l >> 31) & 1);
+	tdata->temp = tjmax - ((val.l >> 16) & 0xff) * 1000;
+	tdata->last_updated = jiffies;
+}
+
+static ssize_t show_temp(struct device *dev,
+			 struct device_attribute *devattr, char *buf)
+{
+	struct temp_data *tdata = container_of(devattr, struct temp_data, sd_attrs[ATTR_TEMP]);
+
+	guard(mutex)(&tdata->update_lock);
+	update_temp(tdata, dev);
 
-	mutex_unlock(&tdata->update_lock);
 	return sprintf(buf, "%d\n", tdata->temp);
 }
 
+static ssize_t show_temp_fault(struct device *dev,
+			       struct device_attribute *devattr, char *buf)
+{
+	struct temp_data *tdata = container_of(devattr, struct temp_data,
+					       sd_attrs[ATTR_TEMP_FAULT]);
+
+	guard(mutex)(&tdata->update_lock);
+	update_temp(tdata, dev);
+
+	return sprintf(buf, "%d\n", tdata->temp_fault);
+}
+
 static int create_core_attrs(struct temp_data *tdata, struct device *dev)
 {
 	static ssize_t (*const rd_ptr[TOTAL_ATTRS]) (struct device *dev,
 			struct device_attribute *devattr, char *buf) = {
-			show_label, show_crit_alarm, show_temp, show_tjmax,
-			show_ttarget };
+			show_label, show_crit_alarm, show_temp, show_temp_fault,
+			show_tjmax, show_ttarget };
 	static const char *const suffixes[TOTAL_ATTRS] = {
-		"label", "crit_alarm", "input", "crit", "max"
+		"label", "crit_alarm", "input", "fault", "crit", "max"
 	};
 	int i;
 

-- 
2.43.0

Re: [PATCH 0/3] hwmon: (coretemp) Report unreliable temperature readings

[Guenter Roeck]

On Thu, Sep 24, 2026 at 07:33:19PM -0700, Ricardo Neri wrote:
> Hi,
> 
> Intel CPUs indicate in IA32_[PACKAGE]_THERM_STATUS whether the digital
> thermal readout they expose is valid. coretemp deliberately ignores that
> indication, for the reason given in commit bf6ea084ebb5 ("hwmon:
> (coretemp) Do not return -EAGAIN for low temperatures"): some CPUs clear
> it while the temperature is too low to be measured, and the value reported
> in that state is more useful to userspace than an error would be.
> 
> The consequence is that userspace cannot distinguish a genuinely low
> temperature from one the CPU could not measure. This series exposes the
> indication through the standard hwmon temp%d_fault attribute, leaving
> temp%d_input exactly as it is.
> 
> One user-visible effect is worth mentioning: sensors(1) prints FAULT in
> place of the temperature when temp%d_fault reads 1. On a CPU that clears
> the valid bit at low temperature, that core stops showing a number in the
> default output, although sensors -u and -j still report it, as does
> anything that reads temp%d_input from sysfs directly. A driver-custom
> attribute name would avoid this, but would be invisible to generic tools.
> Reporting the condition through the documented attribute looks like a
> better option, but please say if you prefer otherwise.

I think it would be _much_ better to return -ENODATA for invalid readings.
This isn't really a fault, after all. The sensor is not defective,
it just can not provide valid data.

With -ENODATA the sensors command reports N/A for the temperature
measurement, which I also think would be better than reporting FAULT.

Thanks,
Guenter