[PATCH 0/3] perf: arm_spe: Add support for a 'software' discard mode

[PATCH 0/3] perf: arm_spe: Add support for a 'software' discard mode

[James Clark]

Add a mode similar to discard mode that we get in SPE V1P2, but without
requiring hardware support. It works by allocating a single page in the
driver when opening the event so the buffer can still be programmed and
SPE enabled. The same page is mapped multiple times to make up a 4MB
buffer so the full interrupt is serviced less frequently.

Unfortunately we can't rely on userspace to do the allocation because
the existing HW discard mode came with the relaxation that an event can
be opened without mapping a buffer. Therefore adding SW discard mode
without doing the allocation in the driver would break old tools. It
will also be useful for very simple tools that only do raw
perf_event_open() calls and don't want to mess around with mmaps. Tools
also can't do the hack to present a 4MB buffer to hardware while only
being backed by a single page.

Discard mode is used to activate SPE related PMU events so that they can
be counted by the PMU, without the overhead of collecting any SPE data.
So it's likely that simple tools are using discard mode with only raw
perf_event_open() calls.

Signed-off-by: James Clark <james.clark@linaro.org>
---
James Clark (3):
      perf: arm_spe: Factor aux output and flags out of buffer fault handler
      perf: arm_spe: Factor HW discard mode out of arm_spe_perf_aux_output_begin()
      perf: arm_spe: Add support for a 'software' discard mode

 drivers/perf/arm_spe_pmu.c | 208 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 168 insertions(+), 40 deletions(-)
---
base-commit: 075b74841bd0065a3bda3440873c747938e69b68
change-id: 20260804-james-spe-software-discard-87691ce1a949

Best regards,
--  
James Clark <james.clark@linaro.org>

[PATCH 1/3] perf: arm_spe: Factor aux output and flags out of buffer fault handler

[James Clark]

Software discard mode will still want to handle buffer faults, but
doesn't have a real aux buffer to set flags or call
perf_aux_output_end() on. Factor the aux buffer parts out of
arm_spe_pmu_buf_get_fault_act() so it only decodes the fault, which
better matches the name and can be used by the new mode later.

This makes the arm_spe_pmu_stop() handling a bit simpler because it
doesn't need to conditionally call arm_spe_perf_aux_output_end() anymore
to handle the case when the fault handler didn't do it.

No functional changes intended.

Signed-off-by: James Clark <james.clark@linaro.org>
---
 drivers/perf/arm_spe_pmu.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
index dbd0da111639..10d28ad81256 100644
--- a/drivers/perf/arm_spe_pmu.c
+++ b/drivers/perf/arm_spe_pmu.c
@@ -677,11 +677,12 @@ static void arm_spe_pmu_disable_and_drain_local(void)
 
 /* IRQ handling */
 static enum arm_spe_pmu_buf_fault_action
-arm_spe_pmu_buf_get_fault_act(struct perf_output_handle *handle)
+arm_spe_pmu_buf_get_fault_act(u64 *aux_flags)
 {
 	const char *err_str;
 	u64 pmbsr;
-	enum arm_spe_pmu_buf_fault_action ret;
+
+	*aux_flags = 0;
 
 	/*
 	 * Ensure new profiling data is visible to the CPU and any external
@@ -703,12 +704,11 @@ arm_spe_pmu_buf_get_fault_act(struct perf_output_handle *handle)
 	 * flag to indicate that the last record is corrupted.
 	 */
 	if (FIELD_GET(PMBSR_EL1_DL, pmbsr))
-		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED |
-					     PERF_AUX_FLAG_PARTIAL);
+		*aux_flags |= PERF_AUX_FLAG_TRUNCATED | PERF_AUX_FLAG_PARTIAL;
 
 	/* Report collisions to userspace so that it can up the period */
 	if (FIELD_GET(PMBSR_EL1_COLL, pmbsr))
-		perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
+		*aux_flags |= PERF_AUX_FLAG_COLLISION;
 
 	/* We only expect buffer management events */
 	switch (FIELD_GET(PMBSR_EL1_EC, pmbsr)) {
@@ -727,8 +727,7 @@ arm_spe_pmu_buf_get_fault_act(struct perf_output_handle *handle)
 	/* Buffer management event */
 	switch (FIELD_GET(PMBSR_EL1_BUF_BSC_MASK, pmbsr)) {
 	case PMBSR_EL1_BUF_BSC_FULL:
-		ret = SPE_PMU_BUF_FAULT_ACT_OK;
-		goto out_stop;
+		return SPE_PMU_BUF_FAULT_ACT_OK;
 	default:
 		err_str = "Unknown buffer status code";
 	}
@@ -738,11 +737,7 @@ arm_spe_pmu_buf_get_fault_act(struct perf_output_handle *handle)
 			   err_str, smp_processor_id(), pmbsr,
 			   read_sysreg_s(SYS_PMBPTR_EL1),
 			   read_sysreg_s(SYS_PMBLIMITR_EL1));
-	ret = SPE_PMU_BUF_FAULT_ACT_FATAL;
-
-out_stop:
-	arm_spe_perf_aux_output_end(handle);
-	return ret;
+	return SPE_PMU_BUF_FAULT_ACT_FATAL;
 }
 
 static irqreturn_t arm_spe_pmu_irq_handler(int irq, void *dev)
@@ -750,14 +745,18 @@ static irqreturn_t arm_spe_pmu_irq_handler(int irq, void *dev)
 	struct perf_output_handle *handle = dev;
 	struct perf_event *event = handle->event;
 	enum arm_spe_pmu_buf_fault_action act;
+	u64 aux_flags;
 
 	if (!perf_get_aux(handle))
 		return IRQ_NONE;
 
-	act = arm_spe_pmu_buf_get_fault_act(handle);
+	act = arm_spe_pmu_buf_get_fault_act(&aux_flags);
 	if (act == SPE_PMU_BUF_FAULT_ACT_SPURIOUS)
 		return IRQ_NONE;
 
+	perf_aux_output_flag(handle, aux_flags);
+	arm_spe_perf_aux_output_end(handle);
+
 	/*
 	 * Ensure perf callbacks have completed, which may disable the
 	 * profiling buffer in response to a TRUNCATION flag.
@@ -927,6 +926,7 @@ static void arm_spe_pmu_stop(struct perf_event *event, int flags)
 	struct arm_spe_pmu *spe_pmu = to_spe_pmu(event->pmu);
 	struct hw_perf_event *hwc = &event->hw;
 	struct perf_output_handle *handle = this_cpu_ptr(spe_pmu->handle);
+	u64 aux_flags;
 
 	/* If we're already stopped, then nothing to do */
 	if (hwc->state & PERF_HES_STOPPED)
@@ -944,10 +944,12 @@ static void arm_spe_pmu_stop(struct perf_event *event, int flags)
 		if (perf_get_aux(handle)) {
 			enum arm_spe_pmu_buf_fault_action act;
 
-			act = arm_spe_pmu_buf_get_fault_act(handle);
-			if (act == SPE_PMU_BUF_FAULT_ACT_SPURIOUS)
-				arm_spe_perf_aux_output_end(handle);
-			else
+			act = arm_spe_pmu_buf_get_fault_act(&aux_flags);
+			perf_aux_output_flag(handle, aux_flags);
+			arm_spe_perf_aux_output_end(handle);
+
+			/* Assume PMBSR only needs clearing for real faults */
+			if (act != SPE_PMU_BUF_FAULT_ACT_SPURIOUS)
 				write_sysreg_s(0, SYS_PMBSR_EL1);
 		}
 

-- 
2.43.0

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

[James Clark]

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.

Signed-off-by: James Clark <james.clark@linaro.org>
---
 drivers/perf/arm_spe_pmu.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
index 10d28ad81256..562a1d3be39f 100644
--- a/drivers/perf/arm_spe_pmu.c
+++ b/drivers/perf/arm_spe_pmu.c
@@ -363,6 +363,11 @@ static const struct attribute_group *arm_spe_pmu_attr_groups[] = {
 	NULL,
 };
 
+static bool arm_spe_discard_mode(struct perf_event *event)
+{
+	return ATTR_CFG_GET_FLD(&event->attr, discard);
+}
+
 /* Convert between user ABI and register values */
 static u64 arm_spe_event_to_pmscr(struct perf_event *event)
 {
@@ -609,18 +614,23 @@ static u64 arm_spe_pmu_next_off(struct perf_output_handle *handle)
 	return limit;
 }
 
+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;
 
-	if (ATTR_CFG_GET_FLD(&event->attr, discard)) {
-		limit = FIELD_PREP(PMBLIMITR_EL1_FM, PMBLIMITR_EL1_FM_DISCARD);
-		limit |= PMBLIMITR_EL1_E;
-		goto out_write_limit;
-	}
-
 	/* Start a new aux session */
 	buf = perf_aux_output_begin(handle, event);
 	if (!buf) {
@@ -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;
+		}
+
 		/*
 		 * We handled the fault (the buffer was full), so resume
 		 * profiling as long as we didn't detect truncation.
@@ -864,7 +880,7 @@ static int arm_spe_pmu_event_init(struct perf_event *event)
 	    !(spe_pmu->features & SPE_PMU_FEAT_EFT))
 		return -EOPNOTSUPP;
 
-	if (ATTR_CFG_GET_FLD(&event->attr, discard) &&
+	if (arm_spe_discard_mode(event) &&
 	    !(spe_pmu->features & SPE_PMU_FEAT_DISCARD))
 		return -EOPNOTSUPP;
 
@@ -884,7 +900,9 @@ static void arm_spe_pmu_start(struct perf_event *event, int flags)
 	struct perf_output_handle *handle = this_cpu_ptr(spe_pmu->handle);
 
 	hwc->state = 0;
-	if (arm_spe_perf_aux_output_begin(handle, event)) {
+	if (arm_spe_discard_mode(event)) {
+		arm_spe_pmu_begin_discard(handle, event);
+	} else if (arm_spe_perf_aux_output_begin(handle, event)) {
 		arm_spe_pmu_stop(event, 0);
 		return;
 	}

-- 
2.43.0

[PATCH 3/3] perf: arm_spe: Add support for a 'software' discard mode

[James Clark]

Currently discard mode needs hardware support, but we can get something
that appears pretty similar to userspace by writing to a single page
allocated by the driver and not emitting aux records.

Remove the -EOPNOTSUPP, as now we use the same discard mode format attr
as with HW support but fall back to the software version.

Don't call perf_get_aux() in discard mode as it unconditionally
dereferences a NULL event->rb when handle->event exists, which can now
be true.

Signed-off-by: James Clark <james.clark@linaro.org>
---
 drivers/perf/arm_spe_pmu.c | 160 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 134 insertions(+), 26 deletions(-)

diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
index 562a1d3be39f..093fa3cc5084 100644
--- a/drivers/perf/arm_spe_pmu.c
+++ b/drivers/perf/arm_spe_pmu.c
@@ -60,6 +60,7 @@ static bool get_spe_event_has_cx(struct perf_event *event)
 }
 
 #define ARM_SPE_BUF_PAD_BYTE			0
+#define ARM_SPE_DISCARD_BUF_SIZE		SZ_4M
 
 struct arm_spe_pmu_buf {
 	int					nr_pages;
@@ -67,6 +68,12 @@ struct arm_spe_pmu_buf {
 	void					*base;
 };
 
+struct arm_spe_pmu_sw_discard_buf {
+	struct page				*page;
+	void					*base;
+	refcount_t				refcount;
+};
+
 struct arm_spe_pmu {
 	struct pmu				pmu;
 	struct platform_device			*pdev;
@@ -85,7 +92,7 @@ struct arm_spe_pmu {
 #define SPE_PMU_FEAT_LDS			(1UL << 4)
 #define SPE_PMU_FEAT_ERND			(1UL << 5)
 #define SPE_PMU_FEAT_INV_FILT_EVT		(1UL << 6)
-#define SPE_PMU_FEAT_DISCARD			(1UL << 7)
+#define SPE_PMU_FEAT_HW_DISCARD			(1UL << 7)
 #define SPE_PMU_FEAT_EFT			(1UL << 8)
 #define SPE_PMU_FEAT_FDS			(1UL << 9)
 #define SPE_PMU_FEAT_DEV_PROBED			(1UL << 63)
@@ -218,7 +225,8 @@ static const struct attribute_group arm_spe_pmu_cap_group = {
 #define ATTR_CFG_FLD_store_filter_CFG		config	/* PMSFCR_EL1.ST */
 #define ATTR_CFG_FLD_store_filter_LO		34
 #define ATTR_CFG_FLD_store_filter_HI		34
-#define ATTR_CFG_FLD_discard_CFG		config	/* PMBLIMITR_EL1.FM = DISCARD */
+/* PMBLIMITR_EL1.FM = DISCARD, or scratch buffer if no HW support */
+#define ATTR_CFG_FLD_discard_CFG		config
 #define ATTR_CFG_FLD_discard_LO			35
 #define ATTR_CFG_FLD_discard_HI			35
 #define ATTR_CFG_FLD_branch_filter_mask_CFG	config	/* PMSFCR_EL1.Bm */
@@ -309,9 +317,6 @@ static umode_t arm_spe_pmu_format_attr_is_visible(struct kobject *kobj,
 	struct device *dev = kobj_to_dev(kobj);
 	struct arm_spe_pmu *spe_pmu = dev_get_drvdata(dev);
 
-	if (attr == &format_attr_discard.attr && !(spe_pmu->features & SPE_PMU_FEAT_DISCARD))
-		return 0;
-
 	if (attr == &format_attr_inv_event_filter.attr && !(spe_pmu->features & SPE_PMU_FEAT_INV_FILT_EVT))
 		return 0;
 
@@ -368,6 +373,82 @@ static bool arm_spe_discard_mode(struct perf_event *event)
 	return ATTR_CFG_GET_FLD(&event->attr, discard);
 }
 
+static bool arm_spe_uses_sw_discard_buf(struct perf_event *event)
+{
+	struct arm_spe_pmu *spe_pmu = to_spe_pmu(event->pmu);
+
+	return arm_spe_discard_mode(event) &&
+	       !(spe_pmu->features & SPE_PMU_FEAT_HW_DISCARD);
+}
+
+static void arm_spe_pmu_free_discard_buf(struct perf_event *event)
+{
+	struct arm_spe_pmu_sw_discard_buf *buf = event->pmu_private;
+
+	event->pmu_private = NULL;
+	if (!refcount_dec_and_test(&buf->refcount))
+		return;
+
+	vunmap(buf->base);
+	__free_page(buf->page);
+	kfree(buf);
+}
+
+/*
+ * Map a single page multiple times to make up ARM_SPE_DISCARD_BUF_SIZE.
+ *
+ * This reduces the number of interrupts that have to be serviced in software
+ * discard mode, but at the same time only uses a page of memory. We don't need
+ * to worry about samples being overwritten because they're never read.
+ */
+static int arm_spe_pmu_alloc_discard_buf(struct perf_event *event)
+{
+	int node = (event->cpu == -1) ? -1 : cpu_to_node(event->cpu);
+	int nr_pages = ARM_SPE_DISCARD_BUF_SIZE / PAGE_SIZE;
+	struct arm_spe_pmu_sw_discard_buf *buf;
+	struct page **pglist;
+	int i;
+
+	if (event->parent) {
+		buf = event->parent->pmu_private;
+		refcount_inc(&buf->refcount);
+		event->pmu_private = buf;
+		event->destroy = arm_spe_pmu_free_discard_buf;
+		return 0;
+	}
+
+	buf = kzalloc_node(sizeof(*buf), GFP_KERNEL, node);
+	if (!buf)
+		return -ENOMEM;
+
+	buf->page = alloc_pages_node(node, GFP_KERNEL, 0);
+	if (!buf->page)
+		goto out_free_buf;
+
+	pglist = kvmalloc_array(nr_pages, sizeof(*pglist), GFP_KERNEL);
+	if (!pglist)
+		goto out_free_page;
+
+	for (i = 0; i < nr_pages; i++)
+		pglist[i] = buf->page;
+
+	buf->base = vmap(pglist, nr_pages, VM_MAP, PAGE_KERNEL);
+	kvfree(pglist);
+	if (!buf->base)
+		goto out_free_page;
+
+	refcount_set(&buf->refcount, 1);
+	event->pmu_private = buf;
+	event->destroy = arm_spe_pmu_free_discard_buf;
+	return 0;
+
+out_free_page:
+	__free_page(buf->page);
+out_free_buf:
+	kfree(buf);
+	return -ENOMEM;
+}
+
 /* Convert between user ABI and register values */
 static u64 arm_spe_event_to_pmscr(struct perf_event *event)
 {
@@ -617,10 +698,26 @@ static u64 arm_spe_pmu_next_off(struct perf_output_handle *handle)
 static void arm_spe_pmu_begin_discard(struct perf_output_handle *handle,
 				      struct perf_event *event)
 {
-	u64 limit;
+	struct arm_spe_pmu *spe_pmu = to_spe_pmu(event->pmu);
+	struct arm_spe_pmu_sw_discard_buf *discard_buf = event->pmu_private;
+	u64 base, limit;
+
+	if (spe_pmu->features & SPE_PMU_FEAT_HW_DISCARD) {
+		limit = FIELD_PREP(PMBLIMITR_EL1_FM, PMBLIMITR_EL1_FM_DISCARD);
+		limit |= PMBLIMITR_EL1_E;
+	} else {
+		base = (u64) discard_buf->base;
+		limit = base + ARM_SPE_DISCARD_BUF_SIZE;
+		limit |= PMBLIMITR_EL1_E;
+		write_sysreg_s(base, SYS_PMBPTR_EL1);
 
-	limit = FIELD_PREP(PMBLIMITR_EL1_FM, PMBLIMITR_EL1_FM_DISCARD);
-	limit |= PMBLIMITR_EL1_E;
+		/*
+		 * Track the event so we can access the discard buffer through
+		 * event->pmu_private. perf_aux_output_begin() normally handles
+		 * this, but we can do it manually if that's not being used.
+		 */
+		handle->event = event;
+	}
 
 	write_sysreg_s(limit, SYS_PMBLIMITR_EL1);
 }
@@ -757,15 +854,20 @@ static irqreturn_t arm_spe_pmu_irq_handler(int irq, void *dev)
 	enum arm_spe_pmu_buf_fault_action act;
 	u64 aux_flags;
 
-	if (!perf_get_aux(handle))
+	if (!event)
+		return IRQ_NONE;
+
+	if (!arm_spe_discard_mode(event) && !perf_get_aux(handle))
 		return IRQ_NONE;
 
 	act = arm_spe_pmu_buf_get_fault_act(&aux_flags);
 	if (act == SPE_PMU_BUF_FAULT_ACT_SPURIOUS)
 		return IRQ_NONE;
 
-	perf_aux_output_flag(handle, aux_flags);
-	arm_spe_perf_aux_output_end(handle);
+	if (!arm_spe_discard_mode(event)) {
+		perf_aux_output_flag(handle, aux_flags);
+		arm_spe_perf_aux_output_end(handle);
+	}
 
 	/*
 	 * Ensure perf callbacks have completed, which may disable the
@@ -819,6 +921,7 @@ static irqreturn_t arm_spe_pmu_irq_handler(int irq, void *dev)
 static int arm_spe_pmu_event_init(struct perf_event *event)
 {
 	u64 reg;
+	int ret;
 	struct perf_event_attr *attr = &event->attr;
 	struct arm_spe_pmu *spe_pmu = to_spe_pmu(event->pmu);
 
@@ -880,14 +983,16 @@ static int arm_spe_pmu_event_init(struct perf_event *event)
 	    !(spe_pmu->features & SPE_PMU_FEAT_EFT))
 		return -EOPNOTSUPP;
 
-	if (arm_spe_discard_mode(event) &&
-	    !(spe_pmu->features & SPE_PMU_FEAT_DISCARD))
-		return -EOPNOTSUPP;
-
 	set_spe_event_has_cx(event);
 	reg = arm_spe_event_to_pmscr(event);
-	if (reg & (PMSCR_EL1_PA | PMSCR_EL1_PCT))
-		return perf_allow_kernel();
+	if (reg & (PMSCR_EL1_PA | PMSCR_EL1_PCT)) {
+		ret = perf_allow_kernel();
+		if (ret)
+			return ret;
+	}
+
+	if (arm_spe_uses_sw_discard_buf(event))
+		return arm_spe_pmu_alloc_discard_buf(event);
 
 	return 0;
 }
@@ -954,23 +1059,26 @@ static void arm_spe_pmu_stop(struct perf_event *event, int flags)
 	arm_spe_pmu_disable_and_drain_local();
 
 	if (flags & PERF_EF_UPDATE) {
+		enum arm_spe_pmu_buf_fault_action act;
+
 		/*
 		 * If there's a fault pending then ensure we contain it
 		 * to this buffer, since we might be on the context-switch
 		 * path.
 		 */
-		if (perf_get_aux(handle)) {
-			enum arm_spe_pmu_buf_fault_action act;
-
-			act = arm_spe_pmu_buf_get_fault_act(&aux_flags);
+		act = arm_spe_pmu_buf_get_fault_act(&aux_flags);
+		if (!arm_spe_discard_mode(event) && perf_get_aux(handle)) {
 			perf_aux_output_flag(handle, aux_flags);
 			arm_spe_perf_aux_output_end(handle);
-
-			/* Assume PMBSR only needs clearing for real faults */
-			if (act != SPE_PMU_BUF_FAULT_ACT_SPURIOUS)
-				write_sysreg_s(0, SYS_PMBSR_EL1);
+		} else {
+			/* Discard mode tracks event outside of perf_aux_output_X() */
+			handle->event = NULL;
 		}
 
+		/* Assume PMBSR only needs clearing for real faults */
+		if (act != SPE_PMU_BUF_FAULT_ACT_SPURIOUS)
+			write_sysreg_s(0, SYS_PMBSR_EL1);
+
 		/*
 		 * This may also contain ECOUNT, but nobody else should
 		 * be looking at period_left, since we forbid frequency
@@ -1180,7 +1288,7 @@ static void __arm_spe_pmu_dev_probe(void *info)
 		spe_pmu->features |= SPE_PMU_FEAT_ERND;
 
 	if (spe_pmu->pmsver >= ID_AA64DFR0_EL1_PMSVer_V1P2)
-		spe_pmu->features |= SPE_PMU_FEAT_DISCARD;
+		spe_pmu->features |= SPE_PMU_FEAT_HW_DISCARD;
 
 	if (FIELD_GET(PMSIDR_EL1_EFT, reg))
 		spe_pmu->features |= SPE_PMU_FEAT_EFT;

-- 
2.43.0

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

[Will Deacon]

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.

> @@ -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

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

[James Clark]

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