Re: [PATCH v2 1/3] perf: marvell: Add MPAM partid filtering to CN10K TAD PMU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Inconsistent masking of `event->attr.config` causes valid MPAM partition ID filters to be silently bypassed (or hardware programming to be skipped entirely) when upper bits are set.
--

commit f4f2ecc176ec6e4641448761b6600d2344554e81
Author: Tanmay Jagdale <tanmay@marvell.com>

perf: marvell: Add MPAM partid filtering to CN10K TAD PMU

The TAD PMU exposes counters that can be filtered by MPAM partition id
for a subset of allocation and hit events.

> diff --git a/drivers/perf/marvell_cn10k_tad_pmu.c b/drivers/perf/marvell_cn10k_tad_pmu.c
> index 51ccb0befa05d..af706b890bf17 100644
> --- a/drivers/perf/marvell_cn10k_tad_pmu.c
> +++ b/drivers/perf/marvell_cn10k_tad_pmu.c
[ ... ]
> @@ -27,30 +35,92 @@ struct tad_region {
[ ... ]
> +static void tad_pmu_start_counter(struct tad_pmu *pmu,
> +				  struct perf_event *event)
> +{
> +	const struct tad_pmu_data *pdata = pmu->pdata;
> +	struct hw_perf_event *hwc = &event->hw;
> +	u32 event_idx = event->attr.config;

[Severity: Medium]
Does this cause valid MPAM partition ID filters to be silently bypassed if
upper bits are set in event->attr.config?

In tad_pmu_event_init(), the event_idx is masked to 8 bits:
u32 event_idx = (u32)(event->attr.config & GENMASK(7, 0));

Since it is masked there, an event with upper bits set successfully passes
the validation checks. However, because it is unmasked here in
tad_pmu_start_counter(), it will fail the bounds checks below while the
lower 8 bits are still eventually programmed into the hardware counter:

> +	u32 counter_idx = hwc->idx;
> +	u64 partid_filter = 0;
> +	u64 reg_val;
> +	u64 cfg1 = event->attr.config1;
> +	bool use_mpam = cfg1 & TAD_PARTID_FILTER_EN;
> +	u32 partid = (u32)(cfg1 & GENMASK(8, 0));
> +	int i;
> +
> +	for (i = 0; i < pmu->region_cnt; i++)
> +		writeq_relaxed(0, pmu->regions[i].base +
> +			       TAD_PFC(pdata->tad_pfc_offset, counter_idx));
> +
> +	if (use_mpam && event_idx > 0x19 && event_idx < 0x21) {
> +		partid_filter = TAD_PRF_MATCH_PARTID | TAD_PRF_PARTID_NS |
> +				((u64)partid << 11);
> +	}
> +
> +
> +	for (i = 0; i < pmu->region_cnt; i++) {
> +		reg_val = event_idx & 0xFF;
> +		reg_val |= partid_filter;
> +		writeq_relaxed(reg_val, pmu->regions[i].base +
> +			       TAD_PRF(pdata->tad_prf_offset, counter_idx));
> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612095746.19679-1-gakula@marvell.com?part=1