Re: [PATCH 2/3] perf: arm_spe: Factor HW discard mode out of arm_spe_perf_aux_output_begin()

[James Clark <james.clark@linaro.org>]

On 05/10/2026 09:27, Will Deacon wrote:
> On Wed, Aug 05, 2026 at 04:01:52PM +0100, James Clark wrote:
>> Discard mode doesn't call perf_aux_output_begin() or emit aux records,
>> and enabling it never fails. We'll add a second discard mode in the
>> next commit so it will be cleaner if there is a separate begin function
>> just for discard mode.
>>
>> No functional changes intended.
> 
> [...]
> 
>> +static void arm_spe_pmu_begin_discard(struct perf_output_handle *handle,
>> +				      struct perf_event *event)
>> +{
>> +	u64 limit;
>> +
>> +	limit = FIELD_PREP(PMBLIMITR_EL1_FM, PMBLIMITR_EL1_FM_DISCARD);
>> +	limit |= PMBLIMITR_EL1_E;
>> +
>> +	write_sysreg_s(limit, SYS_PMBLIMITR_EL1);
>> +}
>> +
>>   static int arm_spe_perf_aux_output_begin(struct perf_output_handle *handle,
>>   					 struct perf_event *event)
>>   {
>>   	u64 base, limit;
>>   	struct arm_spe_pmu_buf *buf;
> 
> Is it worth checking that we have a discard event in begin_discard() and
> similarly that we don't have a discard event in output_begin()? It looks
> like quite an error-prone interface for the caller, otherwise.
> 

Makes sense, I can add that.

>> @@ -775,6 +785,12 @@ static irqreturn_t arm_spe_pmu_irq_handler(int irq, void *dev)
>>   		arm_spe_pmu_disable_and_drain_local();
>>   		break;
>>   	case SPE_PMU_BUF_FAULT_ACT_OK:
>> +		if (arm_spe_discard_mode(event)) {
>> +			arm_spe_pmu_begin_discard(handle, event);
>> +			isb();
>> +			break;
>> +		}
> 
> This looks like new behaviour?
> 
> Will

I just extracted the discard part of arm_spe_perf_aux_output_begin() 
into here and arm_spe_pmu_begin_discard(). The PMBLIMITR_EL1 write and 
isb() should be the same.

Do you mean that now it does arm_spe_pmu_begin_discard() unconditionally 
before checking PERF_AUX_FLAG_TRUNCATED? Truncated would never be set in 
HW discard mode, so it should be equivalent. And the IRQ would never 
fire anyway so we don't get here until SW discard is added, and even if 
it did, handle->event is NULL so the handler would exit on the first 
line anyway.

I suppose there's another change that it can never call 
arm_spe_pmu_stop()? But that wouldn't have happened in discard mode either.

James