[PATCH] powercap: intel_rapl: Sign-extend the PMU delta on counter wraparound

[PATCH] powercap: intel_rapl: Sign-extend the PMU delta on counter wraparound

[Li Yifan]

From: "Li, Yifan" <yifan2.li@intel.com>

The RAPL PMU misreports energy when the hardware energy counter
overflows and wraps back to zero.  perf event counts are defined to
increase monotonically, but a single wraparound makes the PMU event
count jump backwards by nearly the full counter range, and consumers
that take the difference of two reads in unsigned arithmetic then
underflow and report an absurd value.

On a Panther Lake system (energy unit 61.035 uJ, counter range
262144 J) the package counter wraps every ~2.9 hours at 25 W, and
turbostat prints one bogus sample per wraparound, per domain:

  PkgTmp  PkgWatt         CorWatt GFXWatt RAMWatt SysWatt
  44      24.97           16.30   3.90    1.87    2145386370.35
  43      2145240612.10   16.13   4.02    1.91    40.46

The RAPL energy counters are 32-bit wide on every register interface:
MSR, MMIO and TPMI all describe ENERGY_COUNTER with a GENMASK(31, 0)
mask.  rapl_read_data_raw() applies that mask, so event_read_counter()
returns the counter zero-extended in a u64.

rapl_event_update() then computes

	delta = new_raw_count - prev_raw_count;

without reducing the result modulo 2^32.  While the counter does not
wrap this is correct, but once the hardware counter wraps,
new_raw_count < prev_raw_count and delta becomes (true_delta - 2^32),
a large negative value.  Declaring delta as s64 only makes that value
representable; it does not correct it.  That bogus delta is scaled and
added to event->count, which is where the backwards jump comes from.

Fix it the way arch/x86/events/rapl.c has done since the RAPL PMU was
first introduced: shift both values up so that the 64-bit subtraction
reduces modulo 2^32, then shift the difference back down with an
arithmetic shift to sign-extend it.

This is correct as long as at most one wraparound happens between two
updates, which the existing overflow hrtimer already guarantees: its
period is half of the counter range at the 200 W reference used in
rapl_package_add_pmu_locked().

The problem has been present since the powercap RAPL PMU was added, but
only affected TPMI RAPL until commit 748d6ba43afd ("powercap:
intel_rapl: Enable MSR-based RAPL PMU support") routed MSR RAPL through
the same PMU, which exposed it on client platforms such as Panther Lake.

Fixes: 575024a8aa7c ("powercap: intel_rapl: Introduce APIs for PMU support")
Reported-by: Jyoti, Anand B <anand.b.jyoti@intel.com>
Signed-off-by: Li, Yifan <yifan2.li@intel.com>
Signed-off-by: Gao Jianfeng <jianfeng.gao@intel.com>
Tested-by: Jyoti, Anand B <anand.b.jyoti@intel.com>
---
 drivers/powercap/intel_rapl_common.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/powercap/intel_rapl_common.c b/drivers/powercap/intel_rapl_common.c
index 1006d183d508..6b7d11a0abc5 100644
--- a/drivers/powercap/intel_rapl_common.c
+++ b/drivers/powercap/intel_rapl_common.c
@@ -32,6 +32,9 @@
 
 #define ENERGY_STATUS_MASK		GENMASK(31, 0)
 
+/* Width of the RAPL energy counters, see the *_ENERGY_STATUS_MASK defines */
+#define RAPL_CNTR_WIDTH			32
+
 #define POWER_UNIT_OFFSET		0x00
 #define POWER_UNIT_MASK			GENMASK(3, 0)
 
@@ -1227,6 +1230,7 @@ static u64 rapl_event_update(struct perf_event *event)
 	struct rapl_package_pmu_data *data = event_to_pmu_data(event);
 	u64 prev_raw_count, new_raw_count;
 	s64 delta, sdelta;
+	int shift = 64 - RAPL_CNTR_WIDTH;
 
 	/*
 	 * Follow the generic code to drain hwc->prev_count.
@@ -1243,8 +1247,13 @@ static u64 rapl_event_update(struct perf_event *event)
 	 * Now we have the new raw value and have updated the prev
 	 * timestamp already. We can now calculate the elapsed delta
 	 * (event-)time and add that to the generic event.
+	 *
+	 * Careful, the counter is narrower than u64 and is not
+	 * sign-extended above its physical width.  Shift both values up
+	 * so that the subtraction wraps, then shift the result back down.
 	 */
-	delta = new_raw_count - prev_raw_count;
+	delta = (new_raw_count << shift) - (prev_raw_count << shift);
+	delta >>= shift;
 
 	/*
 	 * Scale delta to smallest unit (2^-32)

base-commit: f5bbbfec59b4e2fb7520a91de3df8a6174325d6a
-- 
2.43.0

Re: [PATCH] powercap: intel_rapl: Sign-extend the PMU delta on counter wraparound

[srinivas pandruvada]

On Fri, 2026-08-14 at 11:10 +0800, Li Yifan wrote:
> From: "Li, Yifan" <yifan2.li@intel.com>
> 
> The RAPL PMU misreports energy when the hardware energy counter
> overflows and wraps back to zero.  perf event counts are defined to
> increase monotonically, but a single wraparound makes the PMU event
> count jump backwards by nearly the full counter range, and consumers
> that take the difference of two reads in unsigned arithmetic then
> underflow and report an absurd value.
> 
> On a Panther Lake system (energy unit 61.035 uJ, counter range
> 262144 J) the package counter wraps every ~2.9 hours at 25 W, and
> turbostat prints one bogus sample per wraparound, per domain:
> 
>   PkgTmp  PkgWatt         CorWatt GFXWatt RAMWatt SysWatt
>   44      24.97           16.30   3.90    1.87    2145386370.35
>   43      2145240612.10   16.13   4.02    1.91    40.46
> 
> The RAPL energy counters are 32-bit wide on every register interface:
> MSR, MMIO and TPMI all describe ENERGY_COUNTER with a GENMASK(31, 0)
> mask.  rapl_read_data_raw() applies that mask, so
> event_read_counter()
> returns the counter zero-extended in a u64.
> 
> rapl_event_update() then computes
> 
> 	delta = new_raw_count - prev_raw_count;
> 
> without reducing the result modulo 2^32.  While the counter does not
> wrap this is correct, but once the hardware counter wraps,
> new_raw_count < prev_raw_count and delta becomes (true_delta - 2^32),
> a large negative value.  Declaring delta as s64 only makes that value
> representable; it does not correct it.  That bogus delta is scaled
> and
> added to event->count, which is where the backwards jump comes from.
> 
> Fix it the way arch/x86/events/rapl.c has done since the RAPL PMU was
> first introduced: shift both values up so that the 64-bit subtraction
> reduces modulo 2^32, then shift the difference back down with an
> arithmetic shift to sign-extend it.
> 
> This is correct as long as at most one wraparound happens between two
> updates, which the existing overflow hrtimer already guarantees: its
> period is half of the counter range at the 200 W reference used in
> rapl_package_add_pmu_locked().
> 
> The problem has been present since the powercap RAPL PMU was added,
> but
> only affected TPMI RAPL until commit 748d6ba43afd ("powercap:
> intel_rapl: Enable MSR-based RAPL PMU support") routed MSR RAPL
> through
> the same PMU, which exposed it on client platforms such as Panther
> Lake.
> 
> Fixes: 575024a8aa7c ("powercap: intel_rapl: Introduce APIs for PMU
> support")
> Reported-by: Jyoti, Anand B <anand.b.jyoti@intel.com>
> Signed-off-by: Li, Yifan <yifan2.li@intel.com>
> Signed-off-by: Gao Jianfeng <jianfeng.gao@intel.com>
> Tested-by: Jyoti, Anand B <anand.b.jyoti@intel.com>

Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

> ---
>  drivers/powercap/intel_rapl_common.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/powercap/intel_rapl_common.c
> b/drivers/powercap/intel_rapl_common.c
> index 1006d183d508..6b7d11a0abc5 100644
> --- a/drivers/powercap/intel_rapl_common.c
> +++ b/drivers/powercap/intel_rapl_common.c
> @@ -32,6 +32,9 @@
>  
>  #define ENERGY_STATUS_MASK		GENMASK(31, 0)
>  
> +/* Width of the RAPL energy counters, see the *_ENERGY_STATUS_MASK
> defines */
> +#define RAPL_CNTR_WIDTH			32
> +
>  #define POWER_UNIT_OFFSET		0x00
>  #define POWER_UNIT_MASK			GENMASK(3, 0)
>  
> @@ -1227,6 +1230,7 @@ static u64 rapl_event_update(struct perf_event
> *event)
>  	struct rapl_package_pmu_data *data =
> event_to_pmu_data(event);
>  	u64 prev_raw_count, new_raw_count;
>  	s64 delta, sdelta;
> +	int shift = 64 - RAPL_CNTR_WIDTH;
>  
>  	/*
>  	 * Follow the generic code to drain hwc->prev_count.
> @@ -1243,8 +1247,13 @@ static u64 rapl_event_update(struct perf_event
> *event)
>  	 * Now we have the new raw value and have updated the prev
>  	 * timestamp already. We can now calculate the elapsed delta
>  	 * (event-)time and add that to the generic event.
> +	 *
> +	 * Careful, the counter is narrower than u64 and is not
> +	 * sign-extended above its physical width.  Shift both
> values up
> +	 * so that the subtraction wraps, then shift the result back
> down.
>  	 */
> -	delta = new_raw_count - prev_raw_count;
> +	delta = (new_raw_count << shift) - (prev_raw_count <<
> shift);
> +	delta >>= shift;
>  
>  	/*
>  	 * Scale delta to smallest unit (2^-32)
> 
> base-commit: f5bbbfec59b4e2fb7520a91de3df8a6174325d6a

Re: [PATCH] powercap: intel_rapl: Sign-extend the PMU delta on counter wraparound

[Kuppuswamy Sathyanarayanan]

On 8/13/2026 8:10 PM, Li Yifan wrote:
> From: "Li, Yifan" <yifan2.li@intel.com>
> 
> The RAPL PMU misreports energy when the hardware energy counter
> overflows and wraps back to zero.  perf event counts are defined to
> increase monotonically, but a single wraparound makes the PMU event
> count jump backwards by nearly the full counter range, and consumers
> that take the difference of two reads in unsigned arithmetic then
> underflow and report an absurd value.
> 
> On a Panther Lake system (energy unit 61.035 uJ, counter range
> 262144 J) the package counter wraps every ~2.9 hours at 25 W, and
> turbostat prints one bogus sample per wraparound, per domain:
> 
>   PkgTmp  PkgWatt         CorWatt GFXWatt RAMWatt SysWatt
>   44      24.97           16.30   3.90    1.87    2145386370.35
>   43      2145240612.10   16.13   4.02    1.91    40.46
> 
> The RAPL energy counters are 32-bit wide on every register interface:
> MSR, MMIO and TPMI all describe ENERGY_COUNTER with a GENMASK(31, 0)
> mask.  rapl_read_data_raw() applies that mask, so event_read_counter()
> returns the counter zero-extended in a u64.
> 
> rapl_event_update() then computes
> 
> 	delta = new_raw_count - prev_raw_count;
> 
> without reducing the result modulo 2^32.  While the counter does not
> wrap this is correct, but once the hardware counter wraps,
> new_raw_count < prev_raw_count and delta becomes (true_delta - 2^32),
> a large negative value.  Declaring delta as s64 only makes that value
> representable; it does not correct it.  That bogus delta is scaled and
> added to event->count, which is where the backwards jump comes from.
> 
> Fix it the way arch/x86/events/rapl.c has done since the RAPL PMU was
> first introduced: shift both values up so that the 64-bit subtraction
> reduces modulo 2^32, then shift the difference back down with an
> arithmetic shift to sign-extend it.
> 
> This is correct as long as at most one wraparound happens between two
> updates, which the existing overflow hrtimer already guarantees: its
> period is half of the counter range at the 200 W reference used in
> rapl_package_add_pmu_locked().
> 
> The problem has been present since the powercap RAPL PMU was added, but
> only affected TPMI RAPL until commit 748d6ba43afd ("powercap:
> intel_rapl: Enable MSR-based RAPL PMU support") routed MSR RAPL through
> the same PMU, which exposed it on client platforms such as Panther Lake.
> 
> Fixes: 575024a8aa7c ("powercap: intel_rapl: Introduce APIs for PMU support")
> Reported-by: Jyoti, Anand B <anand.b.jyoti@intel.com>
> Signed-off-by: Li, Yifan <yifan2.li@intel.com>
> Signed-off-by: Gao Jianfeng <jianfeng.gao@intel.com>
> Tested-by: Jyoti, Anand B <anand.b.jyoti@intel.com>
> ---

Looks good to me.

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

>  drivers/powercap/intel_rapl_common.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/powercap/intel_rapl_common.c b/drivers/powercap/intel_rapl_common.c
> index 1006d183d508..6b7d11a0abc5 100644
> --- a/drivers/powercap/intel_rapl_common.c
> +++ b/drivers/powercap/intel_rapl_common.c
> @@ -32,6 +32,9 @@
>  
>  #define ENERGY_STATUS_MASK		GENMASK(31, 0)
>  
> +/* Width of the RAPL energy counters, see the *_ENERGY_STATUS_MASK defines */
> +#define RAPL_CNTR_WIDTH			32
> +
>  #define POWER_UNIT_OFFSET		0x00
>  #define POWER_UNIT_MASK			GENMASK(3, 0)
>  
> @@ -1227,6 +1230,7 @@ static u64 rapl_event_update(struct perf_event *event)
>  	struct rapl_package_pmu_data *data = event_to_pmu_data(event);
>  	u64 prev_raw_count, new_raw_count;
>  	s64 delta, sdelta;
> +	int shift = 64 - RAPL_CNTR_WIDTH;
>  
>  	/*
>  	 * Follow the generic code to drain hwc->prev_count.
> @@ -1243,8 +1247,13 @@ static u64 rapl_event_update(struct perf_event *event)
>  	 * Now we have the new raw value and have updated the prev
>  	 * timestamp already. We can now calculate the elapsed delta
>  	 * (event-)time and add that to the generic event.
> +	 *
> +	 * Careful, the counter is narrower than u64 and is not
> +	 * sign-extended above its physical width.  Shift both values up
> +	 * so that the subtraction wraps, then shift the result back down.
>  	 */
> -	delta = new_raw_count - prev_raw_count;
> +	delta = (new_raw_count << shift) - (prev_raw_count << shift);
> +	delta >>= shift;
>  
>  	/*
>  	 * Scale delta to smallest unit (2^-32)
> 
> base-commit: f5bbbfec59b4e2fb7520a91de3df8a6174325d6a

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer