[PATCH v3] perf: arm_spe: Relax period restriction

[PATCH v3] perf: arm_spe: Relax period restriction

[Leo Yan]

The minimum interval specified the PMSIDR_EL1.Interval field is a
hardware recommendation. However, this value is set by hardware designer
before the production. It is not actual hardware limitation but tools
currently have no way to test shorter periods.

This change relaxes the limitation by allowing any non-zero periods,
with simplifying code with clamp_t().

The downside is that small periods may increase the risk of AUX ring
buffer overruns. When an overrun occurs, the perf core layer will
trigger an irq work to disable the event and wake up the tool in user
space to read the trace data. After the tool finishes reading, it will
re-enable the AUX event.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---

Changes from v2:
- Updated comment for the difference between minimum interval
  recommendation and hardware limitation (Anshuman).
- Refactored with clamp_t().
- Removed review tag as the refactoring.

 drivers/perf/arm_spe_pmu.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
index 3efed8839a4e..369e77ad5f13 100644
--- a/drivers/perf/arm_spe_pmu.c
+++ b/drivers/perf/arm_spe_pmu.c
@@ -308,17 +308,21 @@ static u64 arm_spe_event_to_pmscr(struct perf_event *event)
 
 static void arm_spe_event_sanitise_period(struct perf_event *event)
 {
-	struct arm_spe_pmu *spe_pmu = to_spe_pmu(event->pmu);
 	u64 period = event->hw.sample_period;
 	u64 max_period = PMSIRR_EL1_INTERVAL_MASK;
 
-	if (period < spe_pmu->min_period)
-		period = spe_pmu->min_period;
-	else if (period > max_period)
-		period = max_period;
-	else
-		period &= max_period;
+	/*
+	 * The PMSIDR_EL1.Interval field (stored in spe_pmu->min_period) is a
+	 * recommendation for the minimum interval, not a hardware limitation.
+	 *
+	 * According to the Arm ARM (DDI 0487 L.a), section D24.7.12 PMSIRR_EL1,
+	 * Sampling Interval Reload Register, the INTERVAL field (bits [31:8])
+	 * states: "Software must set this to a nonzero value". Use 1 as the
+	 * minimum value.
+	 */
+	u64 min_period = FIELD_PREP(PMSIRR_EL1_INTERVAL_MASK, 1);
 
+	period = clamp_t(u64, period, min_period, max_period) & max_period;
 	event->hw.sample_period = period;
 }
 
-- 
2.34.1

Re: [PATCH v3] perf: arm_spe: Relax period restriction

[Anshuman Khandual]

On 27/06/25 10:00 PM, Leo Yan wrote:
> The minimum interval specified the PMSIDR_EL1.Interval field is a
> hardware recommendation. However, this value is set by hardware designer
> before the production. It is not actual hardware limitation but tools
> currently have no way to test shorter periods.
> 
> This change relaxes the limitation by allowing any non-zero periods,
> with simplifying code with clamp_t().
> 
> The downside is that small periods may increase the risk of AUX ring
> buffer overruns. When an overrun occurs, the perf core layer will
> trigger an irq work to disable the event and wake up the tool in user
> space to read the trace data. After the tool finishes reading, it will
> re-enable the AUX event.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>

LGTM

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
> 
> Changes from v2:
> - Updated comment for the difference between minimum interval
>   recommendation and hardware limitation (Anshuman).
> - Refactored with clamp_t().
> - Removed review tag as the refactoring.
> 
>  drivers/perf/arm_spe_pmu.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
> index 3efed8839a4e..369e77ad5f13 100644
> --- a/drivers/perf/arm_spe_pmu.c
> +++ b/drivers/perf/arm_spe_pmu.c
> @@ -308,17 +308,21 @@ static u64 arm_spe_event_to_pmscr(struct perf_event *event)
>  
>  static void arm_spe_event_sanitise_period(struct perf_event *event)
>  {
> -	struct arm_spe_pmu *spe_pmu = to_spe_pmu(event->pmu);
>  	u64 period = event->hw.sample_period;
>  	u64 max_period = PMSIRR_EL1_INTERVAL_MASK;
>  
> -	if (period < spe_pmu->min_period)
> -		period = spe_pmu->min_period;
> -	else if (period > max_period)
> -		period = max_period;
> -	else
> -		period &= max_period;
> +	/*
> +	 * The PMSIDR_EL1.Interval field (stored in spe_pmu->min_period) is a
> +	 * recommendation for the minimum interval, not a hardware limitation.
> +	 *
> +	 * According to the Arm ARM (DDI 0487 L.a), section D24.7.12 PMSIRR_EL1,
> +	 * Sampling Interval Reload Register, the INTERVAL field (bits [31:8])
> +	 * states: "Software must set this to a nonzero value". Use 1 as the
> +	 * minimum value.
> +	 */
> +	u64 min_period = FIELD_PREP(PMSIRR_EL1_INTERVAL_MASK, 1);
>  
> +	period = clamp_t(u64, period, min_period, max_period) & max_period;
>  	event->hw.sample_period = period;
>  }
>  

Re: [PATCH v3] perf: arm_spe: Relax period restriction

[Leo Yan]

On Fri, Jun 27, 2025 at 05:30:28PM +0100, Leo Yan wrote:
> The minimum interval specified the PMSIDR_EL1.Interval field is a
> hardware recommendation. However, this value is set by hardware designer
> before the production. It is not actual hardware limitation but tools
> currently have no way to test shorter periods.
> 
> This change relaxes the limitation by allowing any non-zero periods,
> with simplifying code with clamp_t().

Hi Will,

Would you mind checking if this small patch is okay to pick up?

Thanks,
Leo

Re: [PATCH v3] perf: arm_spe: Relax period restriction

[Will Deacon]

On Fri, 27 Jun 2025 17:30:28 +0100, Leo Yan wrote:
> The minimum interval specified the PMSIDR_EL1.Interval field is a
> hardware recommendation. However, this value is set by hardware designer
> before the production. It is not actual hardware limitation but tools
> currently have no way to test shorter periods.
> 
> This change relaxes the limitation by allowing any non-zero periods,
> with simplifying code with clamp_t().
> 
> [...]

Applied to will (for-next/perf), thanks!

[1/1] perf: arm_spe: Relax period restriction
      https://git.kernel.org/will/c/ba2ff3e1b640

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev