[PATCH 0/3] hwmon: (acpi_power_meter) Fix locking and unknown value handling

[PATCH 0/3] hwmon: (acpi_power_meter) Fix locking and unknown value handling

[Guenter Roeck]

Sashiko identified a number of issues in the acpi_power_meter driver.
I used gemini-3-carbon to analyze the bug reports and to implement fixes.
Subsequently, I used Sashiko again to review the fixes. This series is the
result of this effort.

This series fixes locking races and incomplete handling of ACPI unknown
sentinel values in the acpi_power_meter hwmon driver:

1. Fix missing locking in sysfs show callbacks and system resume.
   acpi_power_meter_resume() frees and reallocates capability strings
   without holding resource->lock, racing with concurrent
   METER_NOTIFY_CONFIG notifications (and sysfs readers when the freezer
   is disabled). In addition, the extra sysfs show callbacks read
   capability strings, capability integers, and 64-bit trip points
   without acquiring res->lock, exposing torn or uncommitted trip values
   when racing with set_trip(). Acquire resource->lock in resume and in
   all extra sysfs show callbacks, and return -ENODATA if a capability
   string pointer is NULL.

2. Prevent spurious power alarms when power is unknown.
   When _PMM returns an unknown sentinel (U32_MAX or U64_MAX),
   update_meter() stores it in resource->power and returns 0, causing
   get_power_alarm_state() to evaluate resource->power > resource->cap
   as true and report a false alarm. Convert get_power_alarm_state() to
   return a boolean alarm state so reading power1_alarm never returns an
   error code, preserve and clear any latched METER_NOTIFY_CAPPING
   notification when notifications are supported, and otherwise report 0
   (no alarm) when readings fail or power is unknown.

3. Check for both 32-bit and 64-bit unknown ACPI values across attributes.
   ACPI Power Meter objects report unknown or unavailable integer values
   as all-ones (U32_MAX or U64_MAX depending on DSDT revision), but most
   attributes in the driver either only checked 32-bit 0xffffffff or did
   not check for unknown sentinels at all. Introduce
   acpi_power_meter_is_unknown() and use it to return -ENODATA for
   unknown non-alarm numeric hwmon attributes, emit "unknown" in
   power1_cap_hyst_show() and power1_accuracy_show(), avoid comparing
   against an unknown cap in get_power_alarm_state(), and skip jiffies
   caching in update_meter() when sampling_time is unknown.

----------------------------------------------------------------
Guenter Roeck (3):
      hwmon: (acpi_power_meter) Fix missing locking in sysfs show callbacks and resume
      hwmon: (acpi_power_meter) Prevent spurious power alarm when power is unknown
      hwmon: (acpi_power_meter) Check for 32-bit and 64-bit unknown ACPI values

 drivers/hwmon/acpi_power_meter.c | 101 ++++++++++++++++++++++++++++-----------
 1 file changed, 74 insertions(+), 27 deletions(-)

[PATCH 1/3] hwmon: (acpi_power_meter) Fix missing locking in sysfs show callbacks and resume

[Guenter Roeck]

During system resume, acpi_power_meter_resume() calls free_capabilities()
and read_capabilities() without acquiring resource->lock. Because ACPI
notifications run on the non-freezable kacpi_notify_wq workqueue, a
concurrent METER_NOTIFY_CONFIG notification during resume can race with
acpi_power_meter_resume() and trigger a double-free or use-after-free on
the model_number, serial_number, and oem_info strings as well as
concurrently overwrite resource->caps. When CONFIG_SUSPEND_FREEZER is
disabled, concurrent sysfs readers can also dereference the freed
capability strings mid-update.

In addition, none of the extra sysfs attribute show callbacks acquire
res->lock:
- power1_average_min_show() and power1_average_max_show() read the 64-bit
  res->trip[] entries twice without locking while set_trip() modifies
  them under res->lock (including temporarily storing a candidate value
  before evaluating _PTP and rolling it back on failure). On 32-bit
  architectures this exposes torn 64-bit reads, and on all architectures
  it exposes uncommitted trip values and a TOCTOU race between the < 0
  check and multiplication (emitting "-1000" instead of "unknown" if
  set_trip() rolls back to -1).
- power1_model_number_show(), power1_oem_info_show(),
  power1_serial_number_show(), power1_cap_hyst_show(),
  power1_accuracy_show(), and power1_is_battery_show() read capability
  strings and 64-bit capability fields without synchronization against
  resume updates.

Acquire resource->lock using guard(mutex) in acpi_power_meter_resume()
and in all extra sysfs show callbacks. Also return -ENODATA in the
capability string show callbacks if a string pointer is NULL after a
failed capability re-read.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: de584afa5e18 ("hwmon driver for ACPI 4.0 power meters")
Fixes: 16746ce8adfe ("hwmon: (acpi_power_meter) Replace the deprecated hwmon_device_register")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/acpi_power_meter.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
index 3891962f03db..38d6a1e56ffc 100644
--- a/drivers/hwmon/acpi_power_meter.c
+++ b/drivers/hwmon/acpi_power_meter.c
@@ -584,6 +584,8 @@ static ssize_t power1_average_min_show(struct device *dev,
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
 
+	guard(mutex)(&res->lock);
+
 	if (res->trip[POWER_METER_TRIP_AVERAGE_MIN_IDX] < 0)
 		return sysfs_emit(buf, "unknown\n");
 
@@ -596,6 +598,8 @@ static ssize_t power1_average_max_show(struct device *dev,
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
 
+	guard(mutex)(&res->lock);
+
 	if (res->trip[POWER_METER_TRIP_AVERAGE_MAX_IDX] < 0)
 		return sysfs_emit(buf, "unknown\n");
 
@@ -608,6 +612,8 @@ static ssize_t power1_cap_hyst_show(struct device *dev,
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
 
+	guard(mutex)(&res->lock);
+
 	if (res->caps.hysteresis == UNKNOWN_HYSTERESIS)
 		return sysfs_emit(buf, "unknown\n");
 
@@ -619,8 +625,11 @@ static ssize_t power1_accuracy_show(struct device *dev,
 				    char *buf)
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
-	unsigned int acc = res->caps.accuracy;
+	unsigned int acc;
 
+	guard(mutex)(&res->lock);
+
+	acc = res->caps.accuracy;
 	return sysfs_emit(buf, "%u.%u%%\n", acc / 1000, acc % 1000);
 }
 
@@ -630,6 +639,8 @@ static ssize_t power1_is_battery_show(struct device *dev,
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
 
+	guard(mutex)(&res->lock);
+
 	return sysfs_emit(buf, "%u\n",
 			  res->caps.flags & POWER_METER_IS_BATTERY ? 1 : 0);
 }
@@ -640,6 +651,11 @@ static ssize_t power1_model_number_show(struct device *dev,
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
 
+	guard(mutex)(&res->lock);
+
+	if (!res->model_number)
+		return -ENODATA;
+
 	return sysfs_emit(buf, "%s\n", res->model_number);
 }
 
@@ -649,6 +665,11 @@ static ssize_t power1_oem_info_show(struct device *dev,
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
 
+	guard(mutex)(&res->lock);
+
+	if (!res->oem_info)
+		return -ENODATA;
+
 	return sysfs_emit(buf, "%s\n", res->oem_info);
 }
 
@@ -658,6 +679,11 @@ static ssize_t power1_serial_number_show(struct device *dev,
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
 
+	guard(mutex)(&res->lock);
+
+	if (!res->serial_number)
+		return -ENODATA;
+
 	return sysfs_emit(buf, "%s\n", res->serial_number);
 }
 
@@ -984,6 +1010,8 @@ static int acpi_power_meter_resume(struct device *dev)
 {
 	struct acpi_power_meter_resource *resource = dev_get_drvdata(dev);
 
+	guard(mutex)(&resource->lock);
+
 	free_capabilities(resource);
 	read_capabilities(resource);
 
-- 
2.45.2

[PATCH 2/3] hwmon: (acpi_power_meter) Prevent spurious power alarm when power is unknown

[Guenter Roeck]

According to the ACPI specification (Section 10.4.3 _PMM), if an error
occurs while obtaining the power meter reading or if the value is not
available, _PMM returns an integer with all bits set (U32_MAX or
U64_MAX).

In get_power_alarm_state(), update_meter() stores this unknown sentinel
value in resource->power and returns 0. Unlike the hwmon_power_average
handler in power_meter_read(), get_power_alarm_state() does not check
resource->power against UNKNOWN_POWER_32 or UNKNOWN_POWER_64 before
comparing resource->power > resource->cap. Because the sentinel is a
large unsigned integer, the comparison evaluates to true for any valid
cap, falsely reporting an active power alarm to userspace and latching
resource->power_alarm = true.

In addition, alarm attributes should not return error codes when the
alarm status cannot be determined; an inconclusive alarm state should
report 0 (no alarm, no error).

Convert get_power_alarm_state() to return a boolean alarm state. Preserve
and clear any latched hardware capping notification when
POWER_METER_CAN_NOTIFY is set, and otherwise return false (0, no alarm)
when resource->power is unknown or when updating the meter or cap reading
fails.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: de584afa5e18 ("hwmon driver for ACPI 4.0 power meters")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/acpi_power_meter.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
index 38d6a1e56ffc..1f1d58995693 100644
--- a/drivers/hwmon/acpi_power_meter.c
+++ b/drivers/hwmon/acpi_power_meter.c
@@ -377,28 +377,30 @@ static int set_avg_interval(struct acpi_power_meter_resource *resource,
 	return 0;
 }
 
-static int get_power_alarm_state(struct acpi_power_meter_resource *resource,
-				 long *val)
+static bool get_power_alarm_state(struct acpi_power_meter_resource *resource)
 {
+	bool alarm = false;
 	int ret;
 
+	if (resource->caps.flags & POWER_METER_CAN_NOTIFY)
+		alarm = resource->power_alarm;
+	resource->power_alarm = false;
+
 	ret = update_meter(resource);
-	if (ret)
-		return ret;
+	if (ret ||
+	    resource->power == UNKNOWN_POWER_32 ||
+	    resource->power == UNKNOWN_POWER_64)
+		return alarm;
 
 	/* need to update cap if not to support the notification. */
 	if (!(resource->caps.flags & POWER_METER_CAN_NOTIFY)) {
 		ret = update_cap(resource);
 		if (ret)
-			return ret;
-		resource->power_alarm = resource->power > resource->cap;
-		*val = resource->power_alarm;
-	} else {
-		*val = resource->power_alarm || resource->power > resource->cap;
-		resource->power_alarm = resource->power > resource->cap;
+			return false;
 	}
 
-	return 0;
+	resource->power_alarm = resource->power > resource->cap;
+	return alarm || resource->power_alarm;
 }
 
 static umode_t power_meter_is_visible(const void *data,
@@ -481,9 +483,7 @@ static int power_meter_read(struct device *dev, enum hwmon_sensor_types type,
 		*val = res->caps.max_cap * 1000;
 		break;
 	case hwmon_power_alarm:
-		ret = get_power_alarm_state(res, val);
-		if (ret)
-			return ret;
+		*val = get_power_alarm_state(res);
 		break;
 	case hwmon_power_cap:
 		ret = update_cap(res);
-- 
2.45.2

[PATCH 3/3] hwmon: (acpi_power_meter) Check for 32-bit and 64-bit unknown ACPI values

[Guenter Roeck]

According to the ACPI specification (Section 10.4), ACPI Power Meter
objects (_PMC capability fields, _PMM, _GHL, _PAI) report unknown or
unavailable integer values using an integer with all bits set, which can
be either U32_MAX (0xffffffff) or U64_MAX (0xffffffffffffffff) depending
on the DSDT revision.

Currently, only hwmon_power_average checks both 32-bit and 64-bit
unknown sentinels:
- power1_cap_hyst_show() only compares res->caps.hysteresis against the
  32-bit UNKNOWN_HYSTERESIS (0xffffffff), so a 64-bit U64_MAX sentinel
  bypasses the check, overflows when multiplied by 1000, and emits a
  bogus number instead of "unknown".
- power1_accuracy_show() does not check for unknown sentinels at all,
  formatting 0xffffffff as "4294967.295%".
- power_meter_read() does not check min_avg_interval, max_avg_interval,
  avg_interval, min_cap, max_cap, or cap for unknown sentinels, exposing
  scaled or overflowed values (or negative values when cast to signed
  long) to userspace instead of returning -ENODATA.
- get_power_alarm_state() does not check whether resource->cap is
  unknown before comparing resource->power > resource->cap.
- update_meter() passes an unknown caps.sampling_time directly to
  msecs_to_jiffies(), caching sensor readings for weeks on 64-bit builds.

Introduce acpi_power_meter_is_unknown() to check for both U32_MAX and
U64_MAX, and use it across sysfs attribute handlers and meter updates:
- Return -ENODATA in power_meter_read() when numeric power, interval, or
  cap attributes report an unknown value.
- Emit "unknown" in power1_cap_hyst_show() and power1_accuracy_show()
  when hysteresis or accuracy is unknown.
- Avoid comparing against an unknown cap in get_power_alarm_state(),
  returning 0 (no alarm, no error) unless a hardware capping
  notification was latched.
- Skip the sampling_time cache window in update_meter() when
  sampling_time is unknown.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: de584afa5e18 ("hwmon driver for ACPI 4.0 power meters")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/acpi_power_meter.c | 49 ++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 15 deletions(-)

diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
index 1f1d58995693..b28ca6acf630 100644
--- a/drivers/hwmon/acpi_power_meter.c
+++ b/drivers/hwmon/acpi_power_meter.c
@@ -30,13 +30,6 @@
 #define POWER_METER_CAN_CAP	(1 << 2)
 #define POWER_METER_CAN_NOTIFY	(1 << 3)
 #define POWER_METER_IS_BATTERY	(1 << 8)
-#define UNKNOWN_HYSTERESIS	0xFFFFFFFF
-/*
- * ACPI integers can either be 32-bit or 64-bit wide, so
- * we have to check for both values.
- */
-#define UNKNOWN_POWER_32	U32_MAX
-#define UNKNOWN_POWER_64	U64_MAX
 
 #define METER_NOTIFY_CONFIG	0x80
 #define METER_NOTIFY_TRIP	0x81
@@ -54,6 +47,15 @@ static bool force_cap_on;
 
 static DEFINE_MUTEX(acpi_notify_lock);
 
+/*
+ * ACPI integers can either be 32-bit or 64-bit wide, so
+ * we have to check for both values.
+ */
+static bool acpi_power_meter_is_unknown(u64 val)
+{
+	return val == U32_MAX || val == U64_MAX;
+}
+
 static int can_cap_in_hardware(void)
 {
 	return force_cap_on || cap_in_hardware;
@@ -179,9 +181,10 @@ static int update_meter(struct acpi_power_meter_resource *resource)
 	acpi_status status;
 	unsigned long local_jiffies = jiffies;
 
-	if (time_before(local_jiffies, resource->sensors_last_updated +
+	if (!acpi_power_meter_is_unknown(resource->caps.sampling_time) &&
+	    time_before(local_jiffies, resource->sensors_last_updated +
 			msecs_to_jiffies(resource->caps.sampling_time)) &&
-			resource->sensors_valid)
+	    resource->sensors_valid)
 		return 0;
 
 	status = acpi_evaluate_integer(resource->acpi_dev->handle, "_PMM",
@@ -387,9 +390,7 @@ static bool get_power_alarm_state(struct acpi_power_meter_resource *resource)
 	resource->power_alarm = false;
 
 	ret = update_meter(resource);
-	if (ret ||
-	    resource->power == UNKNOWN_POWER_32 ||
-	    resource->power == UNKNOWN_POWER_64)
+	if (ret || acpi_power_meter_is_unknown(resource->power))
 		return alarm;
 
 	/* need to update cap if not to support the notification. */
@@ -399,6 +400,9 @@ static bool get_power_alarm_state(struct acpi_power_meter_resource *resource)
 			return false;
 	}
 
+	if (acpi_power_meter_is_unknown(resource->cap))
+		return alarm;
+
 	resource->power_alarm = resource->power > resource->cap;
 	return alarm || resource->power_alarm;
 }
@@ -460,26 +464,36 @@ static int power_meter_read(struct device *dev, enum hwmon_sensor_types type,
 		ret = update_meter(res);
 		if (ret)
 			return ret;
-		if (res->power == UNKNOWN_POWER_32 || res->power == UNKNOWN_POWER_64)
+		if (acpi_power_meter_is_unknown(res->power))
 			return -ENODATA;
 		*val = res->power * 1000;
 		break;
 	case hwmon_power_average_interval_min:
+		if (acpi_power_meter_is_unknown(res->caps.min_avg_interval))
+			return -ENODATA;
 		*val = res->caps.min_avg_interval;
 		break;
 	case hwmon_power_average_interval_max:
+		if (acpi_power_meter_is_unknown(res->caps.max_avg_interval))
+			return -ENODATA;
 		*val = res->caps.max_avg_interval;
 		break;
 	case hwmon_power_average_interval:
 		ret = update_avg_interval(res);
 		if (ret)
 			return ret;
-		*val = (res)->avg_interval;
+		if (acpi_power_meter_is_unknown(res->avg_interval))
+			return -ENODATA;
+		*val = res->avg_interval;
 		break;
 	case hwmon_power_cap_min:
+		if (acpi_power_meter_is_unknown(res->caps.min_cap))
+			return -ENODATA;
 		*val = res->caps.min_cap * 1000;
 		break;
 	case hwmon_power_cap_max:
+		if (acpi_power_meter_is_unknown(res->caps.max_cap))
+			return -ENODATA;
 		*val = res->caps.max_cap * 1000;
 		break;
 	case hwmon_power_alarm:
@@ -489,6 +503,8 @@ static int power_meter_read(struct device *dev, enum hwmon_sensor_types type,
 		ret = update_cap(res);
 		if (ret)
 			return ret;
+		if (acpi_power_meter_is_unknown(res->cap))
+			return -ENODATA;
 		*val = res->cap * 1000;
 		break;
 	default:
@@ -614,7 +630,7 @@ static ssize_t power1_cap_hyst_show(struct device *dev,
 
 	guard(mutex)(&res->lock);
 
-	if (res->caps.hysteresis == UNKNOWN_HYSTERESIS)
+	if (acpi_power_meter_is_unknown(res->caps.hysteresis))
 		return sysfs_emit(buf, "unknown\n");
 
 	return sysfs_emit(buf, "%llu\n", res->caps.hysteresis * 1000);
@@ -629,6 +645,9 @@ static ssize_t power1_accuracy_show(struct device *dev,
 
 	guard(mutex)(&res->lock);
 
+	if (acpi_power_meter_is_unknown(res->caps.accuracy))
+		return sysfs_emit(buf, "unknown\n");
+
 	acc = res->caps.accuracy;
 	return sysfs_emit(buf, "%u.%u%%\n", acc / 1000, acc % 1000);
 }
-- 
2.45.2

Re: [PATCH 1/3] hwmon: (acpi_power_meter) Fix missing locking in sysfs show callbacks and resume

[sashiko-bot]

> During system resume, acpi_power_meter_resume() calls free_capabilities()
> and read_capabilities() without acquiring resource->lock. Because ACPI
> notifications run on the non-freezable kacpi_notify_wq workqueue, a
> concurrent METER_NOTIFY_CONFIG notification during resume can race with
> acpi_power_meter_resume() and trigger a double-free or use-after-free on
> the model_number, serial_number, and oem_info strings as well as
> concurrently overwrite resource->caps. When CONFIG_SUSPEND_FREEZER is
> disabled, concurrent sysfs readers can also dereference the freed
> capability strings mid-update.
> 
> In addition, none of the extra sysfs attribute show callbacks acquire
> res->lock:
> - power1_average_min_show() and power1_average_max_show() read the 64-bit
>   res->trip[] entries twice without locking while set_trip() modifies
>   them under res->lock (including temporarily storing a candidate value
> [ ... ]
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Fixes: de584afa5e18 ("hwmon driver for ACPI 4.0 power meters")
> Fixes: 16746ce8adfe ("hwmon: (acpi_power_meter) Replace the deprecated hwmon_device_register")
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031165447.1771473-1-linux@roeck-us.net?part=1

Re: [PATCH 3/3] hwmon: (acpi_power_meter) Check for 32-bit and 64-bit unknown ACPI values

[sashiko-bot]

> According to the ACPI specification (Section 10.4), ACPI Power Meter
> objects (_PMC capability fields, _PMM, _GHL, _PAI) report unknown or
> unavailable integer values using an integer with all bits set, which can
> be either U32_MAX (0xffffffff) or U64_MAX (0xffffffffffffffff) depending
> on the DSDT revision.
> 
> Currently, only hwmon_power_average checks both 32-bit and 64-bit
> unknown sentinels:
> - power1_cap_hyst_show() only compares res->caps.hysteresis against the
>   32-bit UNKNOWN_HYSTERESIS (0xffffffff), so a 64-bit U64_MAX sentinel
>   bypasses the check, overflows when multiplied by 1000, and emits a
>   bogus number instead of "unknown".
> - power1_accuracy_show() does not check for unknown sentinels at all,
>   formatting 0xffffffff as "4294967.295%".
> - power_meter_read() does not check min_avg_interval, max_avg_interval,
> [ ... ]
>   sampling_time is unknown.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Fixes: de584afa5e18 ("hwmon driver for ACPI 4.0 power meters")
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031165447.1771473-1-linux@roeck-us.net?part=3

Re: [PATCH 2/3] hwmon: (acpi_power_meter) Prevent spurious power alarm when power is unknown

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] hwmon: acpi_power_meter: get_power_alarm_state() re-latches software alarm
--

commit f84e0084ffbbd96f84c563d4aa65b6dcc7bb7480
Author: Guenter Roeck <linux@roeck-us.net>

hwmon: (acpi_power_meter) Prevent spurious power alarm when power is unknown

This commit updates get_power_alarm_state() to return a boolean alarm state
and ignore unknown power readings (UNKNOWN_POWER_32 and UNKNOWN_POWER_64) as
well as meter or cap update errors when evaluating the power alarm.

> diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
> index 38d6a1e56ffcc..1f1d58995693c 100644
> --- a/drivers/hwmon/acpi_power_meter.c
> +++ b/drivers/hwmon/acpi_power_meter.c
> @@ -377,28 +377,30 @@ static int set_avg_interval(struct acpi_power_meter_resource *resource,
>  	return 0;
>  }
>  
> -static int get_power_alarm_state(struct acpi_power_meter_resource *resource,
> -				 long *val)
> +static bool get_power_alarm_state(struct acpi_power_meter_resource *resource)
>  {
> +	bool alarm = false;
>  	int ret;
>  
> +	if (resource->caps.flags & POWER_METER_CAN_NOTIFY)
> +		alarm = resource->power_alarm;
> +	resource->power_alarm = false;
> +
>  	ret = update_meter(resource);
> -	if (ret)
> -		return ret;
> +	if (ret ||
> +	    resource->power == UNKNOWN_POWER_32 ||
> +	    resource->power == UNKNOWN_POWER_64)
> +		return alarm;
>  
>  	/* need to update cap if not to support the notification. */
>  	if (!(resource->caps.flags & POWER_METER_CAN_NOTIFY)) {
>  		ret = update_cap(resource);
>  		if (ret)
> -			return ret;
> -		resource->power_alarm = resource->power > resource->cap;
> -		*val = resource->power_alarm;
> -	} else {
> -		*val = resource->power_alarm || resource->power > resource->cap;
> -		resource->power_alarm = resource->power > resource->cap;
> +			return false;
>  	}
>  
> -	return 0;
> +	resource->power_alarm = resource->power > resource->cap;
> +	return alarm || resource->power_alarm;
>  }

[Severity: Medium]
When POWER_METER_CAN_NOTIFY is set in resource->caps.flags, does assigning
resource->power_alarm = resource->power > resource->cap at the end of
get_power_alarm_state() re-latch the polled comparison result into the next
power1_alarm read?

On a first read via power_meter_read() -> get_power_alarm_state() where
resource->power > resource->cap, resource->power_alarm is set back to true
after having been cleared at the start of the function.

On the next read of power1_alarm (with no intervening METER_NOTIFY_CAPPING
ACPI notification):

	if (resource->caps.flags & POWER_METER_CAN_NOTIFY)
		alarm = resource->power_alarm;
	resource->power_alarm = false;

alarm becomes true from that prior software comparison. If _PMM now returns
UNKNOWN_POWER_32 or UNKNOWN_POWER_64, or if update_meter() fails, the early
return:

	if (ret ||
	    resource->power == UNKNOWN_POWER_32 ||
	    resource->power == UNKNOWN_POWER_64)
		return alarm;

returns true even though no hardware capping notification occurred since the
previous read. Likewise, if resource->power has since dropped to or below
resource->cap, get_power_alarm_state() still returns true for that second
read.

When POWER_METER_CAN_NOTIFY is not set, writing resource->power_alarm at the
end of get_power_alarm_state() has no effect because the next call
unconditionally resets resource->power_alarm = false. Should the end of
get_power_alarm_state() simply return alarm || resource->power > resource->cap
without writing resource->power_alarm?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031165447.1771473-1-linux@roeck-us.net?part=2