Re: [PATCH v2] perf/arm_cspmu: Support 64-bit programmers' model

On Fri, 31 Jul 2026, Robin Murphy wrote:
> The 64-bit Programmers' model extension, now named FEAT_CSPMU_EXT64,
> makes all the non-counter registers 64-bit as well, of which we most
> need to care PMEVFILTR<n>, PMEVTYPER<n>, and PMCR since it changes
> location. Our event config fields are ready for this internally, but
> we need a few more tweaks to propagate 64-bit values properly from
> end to end.
>
> Since 64-bit events leave no room for our existing "cycles" encoding,
> and we don't really expect to see implementations of it anyway, we
> deliberately omit generic support for the architectural fixed cycle
> counter in this case to save some bother. At worst, vendor modules
> can still implement their own encoding if they really want to.
>
> Signed-off-by: Robin Murphy <[email protected]>

The patch looks good to me and v2 seemed to work fine on Ampere platform 
still.

Reviewed-by: Ilkka Koskinen <[email protected]>

Cheers, Ilkka

> ---
>
> v2: Fix off-by-one in the event format
> ---
> drivers/perf/arm_cspmu/arm_cspmu.c    | 121 +++++++++++++++++++++-----
> drivers/perf/arm_cspmu/arm_cspmu.h    |  25 +++---
> drivers/perf/arm_cspmu/nvidia_cspmu.c |   2 +-
> 3 files changed, 114 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index ad82de166dd1..3b48a35b6241 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -78,13 +78,13 @@ static struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
> }
>
> /*
> - * In CoreSight PMU architecture, all of the MMIO registers are 32-bit except
> - * counter register. The counter register can be implemented as 32-bit or 64-bit
> - * register depending on the value of PMCFGR.SIZE field. For 64-bit access,
> - * single-copy 64-bit atomic support is implementation defined. APMT node flag
> - * is used to identify if the PMU supports 64-bit single copy atomic. If 64-bit
> - * single copy atomic is not supported, the driver treats the register as a pair
> - * of 32-bit register.
> + * With FEAT_CSPMU_EXT32, all of the MMIO registers are 32-bit except the
> + * counter registers, which are either 32-bit or 64-bit depending on the value
> + * of PMCFGR.SIZE. It is implementation-defined whether single-copy-atomic
> + * 64-bit accesses are supported, so we rely on a firmware flag to identify
> + * that, and otherwise treat a 64-bit counter as a non-atomic pair of 32-bit
> + * registers. With FEAT_CSPMU_EXT64, everything is 64-bit, but we may still
> + * have to deal with atomicity being broken.
>  */
>
> /*
> @@ -173,13 +173,30 @@ arm_cspmu_event_attr_is_visible(struct kobject *kobj,
> 	eattr = container_of(attr, typeof(*eattr), attr.attr);
>
> 	/* Hide cycle event if not supported */
> -	if (!supports_cycle_counter(cspmu) &&
> +	if ((cspmu->has_ext64 || !supports_cycle_counter(cspmu)) &&
> 	    eattr->id == ARM_CSPMU_EVT_CYCLES_DEFAULT)
> 		return 0;
>
> 	return attr->mode;
> }
>
> +ssize_t arm_cspmu_default_format_show(struct device *dev,
> +				      struct device_attribute *attr, char *buf)
> +{
> +	struct perf_pmu_events_attr *fmt = container_of(attr, typeof(*fmt), attr);
> +	struct arm_cspmu *cspmu = to_arm_cspmu(dev_get_drvdata(dev));
> +	u64 field = cspmu->has_ext64 ? U64_MAX : U32_MAX;
> +	DECLARE_BITMAP(bits, 64) = { BITMAP_FROM_U64(field) };
> +
> +	if (!fmt->id) {
> +		set_bit(32, bits); /* For 32-bit "cycles" event */
> +		return sysfs_emit(buf, "config:%*pbl\n", 64, bits);
> +	}
> +
> +	return sysfs_emit(buf, "config%lld:%*pbl\n", fmt->id, 64, bits);
> +}
> +EXPORT_SYMBOL_GPL(arm_cspmu_default_format_show);
> +
> static struct attribute *arm_cspmu_format_attrs[] = {
> 	ARM_CSPMU_FORMAT_EVENT_ATTR,
> 	ARM_CSPMU_FORMAT_FILTER_ATTR,
> @@ -198,9 +215,9 @@ arm_cspmu_get_format_attrs(const struct arm_cspmu *cspmu)
> 	return attrs;
> }
>
> -static u32 arm_cspmu_event_type(const struct perf_event *event)
> +static u64 arm_cspmu_event_type(const struct perf_event *event)
> {
> -	return event->attr.config & ARM_CSPMU_EVENT_MASK;
> +	return event->attr.config;
> }
>
> static bool arm_cspmu_is_cycle_counter_event(const struct perf_event *event)
> @@ -208,6 +225,16 @@ static bool arm_cspmu_is_cycle_counter_event(const struct perf_event *event)
> 	return (event->attr.config == ARM_CSPMU_EVT_CYCLES_DEFAULT);
> }
>
> +static u64 arm_cspmu_filter(const struct perf_event *event)
> +{
> +	return event->attr.config1;
> +}
> +
> +static u64 arm_cspmu_filter2(const struct perf_event *event)
> +{
> +	return event->attr.config2;
> +}
> +
> static ssize_t arm_cspmu_identifier_show(struct device *dev,
> 					 struct device_attribute *attr,
> 					 char *page)
> @@ -417,6 +444,17 @@ static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
> 		DEFAULT_IMPL_OP(event_attr_is_visible),
> 	};
>
> +	/*
> +	 * With 64-bit events, since our default "cycles" encoding won't work,
> +	 * and the architecture recommends against implementing it anyway, we
> +	 * choose to effectively ignore FEAT_CSPMU_CCNTR, unless a vendor
> +	 * module really wants to provide its own encoding and ops.
> +	 */
> +	if (cspmu->has_ext64) {
> +		cspmu->impl.ops.is_cycle_counter_event = NULL;
> +		cspmu->impl.ops.set_cc_filter = NULL;
> +	}
> +
> 	/* Firmware may override implementer/product ID from PMIIDR */
> 	if (apmt_node && apmt_node->impl_id)
> 		cspmu->impl.pmiidr = apmt_node->impl_id;
> @@ -518,19 +556,24 @@ static int arm_cspmu_alloc_attr_groups(struct arm_cspmu *cspmu)
> 	return 0;
> }
>
> +static inline int arm_cspmu_pmcr(struct arm_cspmu *cspmu)
> +{
> +	return cspmu->has_ext64 ? PMCR_64 : PMCR;
> +}
> +
> static inline void arm_cspmu_reset_counters(struct arm_cspmu *cspmu)
> {
> -	writel(PMCR_C | PMCR_P, cspmu->base0 + PMCR);
> +	writel(PMCR_C | PMCR_P, cspmu->base0 + arm_cspmu_pmcr(cspmu));
> }
>
> static inline void arm_cspmu_start_counters(struct arm_cspmu *cspmu)
> {
> -	writel(PMCR_E, cspmu->base0 + PMCR);
> +	writel(PMCR_E, cspmu->base0 + arm_cspmu_pmcr(cspmu));
> }
>
> static inline void arm_cspmu_stop_counters(struct arm_cspmu *cspmu)
> {
> -	writel(0, cspmu->base0 + PMCR);
> +	writel(0, cspmu->base0 + arm_cspmu_pmcr(cspmu));
> }
>
> static void arm_cspmu_enable(struct pmu *pmu)
> @@ -561,7 +604,8 @@ static int arm_cspmu_get_event_idx(struct arm_cspmu_hw_events *hw_events,
> 	struct arm_cspmu *cspmu = to_arm_cspmu(event->pmu);
>
> 	if (supports_cycle_counter(cspmu)) {
> -		if (cspmu->impl.ops.is_cycle_counter_event(event)) {
> +		if (cspmu->impl.ops.is_cycle_counter_event &&
> +		    cspmu->impl.ops.is_cycle_counter_event(event)) {
> 			/* Search for available cycle counter. */
> 			if (test_and_set_bit(cspmu->cycle_counter_logical_idx,
> 					     hw_events->used_ctrs))
> @@ -804,26 +848,33 @@ static void arm_cspmu_event_update(struct perf_event *event)
> static inline void arm_cspmu_set_event(struct arm_cspmu *cspmu,
> 					struct hw_perf_event *hwc)
> {
> -	u32 offset = PMEVTYPER + (4 * hwc->idx);
> -
> -	writel(hwc->config, cspmu->base0 + offset);
> +	if (cspmu->has_ext64)
> +		writeq(hwc->config, cspmu->base0 + PMEVTYPER + (8 * hwc->idx));
> +	else
> +		writel(hwc->config, cspmu->base0 + PMEVTYPER + (4 * hwc->idx));
> }
>
> static void arm_cspmu_set_ev_filter(struct arm_cspmu *cspmu,
> 				    const struct perf_event *event)
> {
> -	u32 filter = event->attr.config1 & ARM_CSPMU_FILTER_MASK;
> -	u32 filter2 = event->attr.config2 & ARM_CSPMU_FILTER_MASK;
> -	u32 offset = 4 * event->hw.idx;
> +	u64 filter = arm_cspmu_filter(event);
> +	u64 filter2 = arm_cspmu_filter2(event);
> +	int n = event->hw.idx;
>
> -	writel(filter, cspmu->base0 + PMEVFILTR + offset);
> -	writel(filter2, cspmu->base0 + PMEVFILT2R + offset);
> +	if (cspmu->has_ext64) {
> +		writeq(filter, cspmu->base0 + PMEVFILTR + (8 * n));
> +		writeq(filter2, cspmu->base0 + PMEVFILT2R + (8 * n));
> +	} else {
> +		writel(filter, cspmu->base0 + PMEVFILTR + (4 * n));
> +		writel(filter2, cspmu->base0 + PMEVFILT2R + (4 * n));
> +	}
> }
>
> +/* Note we deliberately don't expect 64-bit filters here; see init_impl_ops */
> static void arm_cspmu_set_cc_filter(struct arm_cspmu *cspmu,
> 				    const struct perf_event *event)
> {
> -	u32 filter = event->attr.config1 & ARM_CSPMU_FILTER_MASK;
> +	u32 filter = arm_cspmu_filter(event);
>
> 	writel(filter, cspmu->base0 + PMCCFILTR);
> }
> @@ -976,6 +1027,30 @@ static int arm_cspmu_init_mmio(struct arm_cspmu *cspmu)
> 		}
> 	}
>
> +	/*
> +	 * We can infer FEAT_CSPMU_EXT64 from PMCNTEN, or hope that anything
> +	 * that failed to get that right has at least implemented the optional
> +	 * PMDEVARCH correctly...
> +	 *
> +	 * Note that architecturally, has_ext64 *should* imply has_atomic_dword,
> +	 * but enough implementations have ignored that already that we'll just
> +	 * have to still rely on the firmware flag.
> +	 */
> +	writel(~0U, cspmu->base0 + PMCNTENCLR);
> +	writel(~0U, cspmu->base0 + PMCNTEN);
> +	if (readl(cspmu->base0 + PMCNTENCLR)) {
> +		cspmu->has_ext64 = true;
> +		writel(0, cspmu->base0 + PMCNTEN);
> +	} else {
> +		u32 reg = readl(cspmu->base0 + PMDEVARCH);
> +
> +		if (reg & ARM_CSPMU_PMDEVARCH_PRESENT) {
> +			reg &= ARM_CSPMU_PMDEVARCH_ARCHPART;
> +			if (reg == 0xaf4 || reg == 0xaf5)
> +				cspmu->has_ext64 = true;
> +		}
> +	}
> +
> 	cspmu->pmcfgr = readl(cspmu->base0 + PMCFGR);
>
> 	cspmu->num_logical_ctrs = FIELD_GET(PMCFGR_N, cspmu->pmcfgr) + 1;
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
> index 3fc5c8d77266..c4058d602477 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.h
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.h
> @@ -35,21 +35,15 @@
> 	PMU_EVENT_ATTR_ID(_name, arm_cspmu_sysfs_event_show, _config)
>
>
> -/* Default event id mask */
> -#define ARM_CSPMU_EVENT_MASK	GENMASK_ULL(63, 0)
> -
> -/* Default filter value mask */
> -#define ARM_CSPMU_FILTER_MASK	GENMASK_ULL(63, 0)
> -
> /* Default event format */
> #define ARM_CSPMU_FORMAT_EVENT_ATTR	\
> -	ARM_CSPMU_FORMAT_ATTR(event, "config:0-32")
> +	PMU_EVENT_ATTR_ID(event, arm_cspmu_default_format_show, 0)
>
> /* Default filter format */
> #define ARM_CSPMU_FORMAT_FILTER_ATTR	\
> -	ARM_CSPMU_FORMAT_ATTR(filter, "config1:0-31")
> +	PMU_EVENT_ATTR_ID(filter, arm_cspmu_default_format_show, 1)
> #define ARM_CSPMU_FORMAT_FILTER2_ATTR	\
> -	ARM_CSPMU_FORMAT_ATTR(filter2, "config2:0-31")
> +	PMU_EVENT_ATTR_ID(filter2, arm_cspmu_default_format_show, 2)
>
> /*
>  * This is the default event number for cycle count, if supported, since the
> @@ -78,6 +72,7 @@
> #define PMEVFILT2R			0x800
> #define PMEVFILTR			0xA00
> #define PMCNTENSET			0xC00
> +#define PMCNTEN				0xC10
> #define PMCNTENCLR			0xC20
> #define PMINTENSET			0xC40
> #define PMINTENCLR			0xC60
> @@ -87,6 +82,8 @@
> #define PMCFGR				0xE00
> #define PMCR				0xE04
> #define PMIIDR				0xE08
> +#define PMCR_64				0xE10
> +#define PMDEVARCH			0xFBC
> #define PMPIDR0				0xFE0
> #define PMPIDR1				0xFE4
> #define PMPIDR2				0xFE8
> @@ -154,6 +151,10 @@
> #define ARM_CSPMU_IMPL_ID_NVIDIA	0x36B
> #define ARM_CSPMU_IMPL_ID_AMPERE	0xA16
>
> +/* PMDEVARCH */
> +#define ARM_CSPMU_PMDEVARCH_PRESENT	BIT(20)
> +#define ARM_CSPMU_PMDEVARCH_ARCHPART	GENMASK(11, 0)
> +
> struct arm_cspmu;
>
> /* This tracks the events assigned to each counter in the PMU. */
> @@ -183,7 +184,7 @@ struct arm_cspmu_impl_ops {
> 	/* Check if the event corresponds to cycle count event */
> 	bool (*is_cycle_counter_event)(const struct perf_event *event);
> 	/* Decode event type/id from configs */
> -	u32 (*event_type)(const struct perf_event *event);
> +	u64 (*event_type)(const struct perf_event *event);
> 	/* Set/reset event filters */
> 	void (*set_cc_filter)(struct arm_cspmu *cspmu,
> 			      const struct perf_event *event);
> @@ -234,6 +235,7 @@ struct arm_cspmu {
> 	int irq;
>
> 	bool has_atomic_dword;
> +	bool has_ext64;
> 	u32 pmcfgr;
> 	u32 num_logical_ctrs;
> 	u32 num_set_clr_reg;
> @@ -250,6 +252,9 @@ ssize_t arm_cspmu_sysfs_event_show(struct device *dev,
> 				   struct device_attribute *attr,
> 				   char *buf);
>
> +ssize_t arm_cspmu_default_format_show(struct device *dev,
> +				      struct device_attribute *attr, char *buf);
> +
> /* Register vendor backend. */
> int arm_cspmu_impl_register(const struct arm_cspmu_impl_match *impl_match);
>
> diff --git a/drivers/perf/arm_cspmu/nvidia_cspmu.c b/drivers/perf/arm_cspmu/nvidia_cspmu.c
> index bac83e424d6d..a4c1ab886709 100644
> --- a/drivers/perf/arm_cspmu/nvidia_cspmu.c
> +++ b/drivers/perf/arm_cspmu/nvidia_cspmu.c
> @@ -762,7 +762,7 @@ static void pcie_tgt_pmu_reset_ev_filter(struct arm_cspmu *cspmu,
> 	pcie_tgt_pmu_config_addr_filter(cspmu, false, base, mask, idx);
> }
>
> -static u32 pcie_tgt_pmu_event_type(const struct perf_event *event)
> +static u64 pcie_tgt_pmu_event_type(const struct perf_event *event)
> {
> 	return event->attr.config & NV_PCIE_TGT_EV_TYPE_MASK;
> }
> -- 
> 2.54.0.dirty
>
>