Skip to content

Commit 283182c

Browse files
Leo-Yanwilldeacon
authored andcommitted
perf: arm_spe: Properly set hw.state on failures
When arm_spe_pmu_next_off() fails to calculate a valid limit, it returns zero to indicate that tracing should not start. However, the caller arm_spe_perf_aux_output_begin() does not propagate this failure by updating hwc->state, cause the error to be silently ignored by upper layers. Because hwc->state remains zero after a failure, arm_spe_pmu_start() continues to programs filter registers unnecessarily. The driver still reports success to the perf core, so the core assumes the SPE event was enabled and proceeds to enable other events. This breaks event group semantics: SPE is already stopped while other events in the same group are enabled. Fix this by updating arm_spe_perf_aux_output_begin() to return a status code indicating success (0) or failure (-EIO). Both the interrupt handler and arm_spe_pmu_start() check the return value and call arm_spe_pmu_stop() to set PERF_HES_STOPPED in hwc->state. In the interrupt handler, the period (e.g., period_left) needs to be updated, so PERF_EF_UPDATE is passed to arm_spe_pmu_stop(). When the error occurs during event start, the trace unit is not yet enabled, so a flag '0' is used to drain buffer and update state only. Fixes: d5d9696 ("drivers/perf: Add support for ARMv8.2 Statistical Profiling Extension") Signed-off-by: Leo Yan <leo.yan@arm.com> Signed-off-by: Will Deacon <will@kernel.org>
1 parent 53c9985 commit 283182c

1 file changed

Lines changed: 12 additions & 6 deletions

File tree

drivers/perf/arm_spe_pmu.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ struct arm_spe_pmu {
106106
/* Keep track of our dynamic hotplug state */
107107
static enum cpuhp_state arm_spe_pmu_online;
108108

109+
static void arm_spe_pmu_stop(struct perf_event *event, int flags);
110+
109111
enum arm_spe_pmu_buf_fault_action {
110112
SPE_PMU_BUF_FAULT_ACT_SPURIOUS,
111113
SPE_PMU_BUF_FAULT_ACT_FATAL,
@@ -607,8 +609,8 @@ static u64 arm_spe_pmu_next_off(struct perf_output_handle *handle)
607609
return limit;
608610
}
609611

610-
static void arm_spe_perf_aux_output_begin(struct perf_output_handle *handle,
611-
struct perf_event *event)
612+
static int arm_spe_perf_aux_output_begin(struct perf_output_handle *handle,
613+
struct perf_event *event)
612614
{
613615
u64 base, limit;
614616
struct arm_spe_pmu_buf *buf;
@@ -622,7 +624,6 @@ static void arm_spe_perf_aux_output_begin(struct perf_output_handle *handle,
622624
/* Start a new aux session */
623625
buf = perf_aux_output_begin(handle, event);
624626
if (!buf) {
625-
event->hw.state |= PERF_HES_STOPPED;
626627
/*
627628
* We still need to clear the limit pointer, since the
628629
* profiler might only be disabled by virtue of a fault.
@@ -642,6 +643,7 @@ static void arm_spe_perf_aux_output_begin(struct perf_output_handle *handle,
642643

643644
out_write_limit:
644645
write_sysreg_s(limit, SYS_PMBLIMITR_EL1);
646+
return (limit & PMBLIMITR_EL1_E) ? 0 : -EIO;
645647
}
646648

647649
static void arm_spe_perf_aux_output_end(struct perf_output_handle *handle)
@@ -781,7 +783,10 @@ static irqreturn_t arm_spe_pmu_irq_handler(int irq, void *dev)
781783
* when we get to it.
782784
*/
783785
if (!(handle->aux_flags & PERF_AUX_FLAG_TRUNCATED)) {
784-
arm_spe_perf_aux_output_begin(handle, event);
786+
if (arm_spe_perf_aux_output_begin(handle, event)) {
787+
arm_spe_pmu_stop(event, PERF_EF_UPDATE);
788+
break;
789+
}
785790
isb();
786791
}
787792
break;
@@ -880,9 +885,10 @@ static void arm_spe_pmu_start(struct perf_event *event, int flags)
880885
struct perf_output_handle *handle = this_cpu_ptr(spe_pmu->handle);
881886

882887
hwc->state = 0;
883-
arm_spe_perf_aux_output_begin(handle, event);
884-
if (hwc->state)
888+
if (arm_spe_perf_aux_output_begin(handle, event)) {
889+
arm_spe_pmu_stop(event, 0);
885890
return;
891+
}
886892

887893
reg = arm_spe_event_to_pmsfcr(event);
888894
write_sysreg_s(reg, SYS_PMSFCR_EL1);

0 commit comments

Comments
 (0)