[PATCH v1 0/2] perf: RISC-V: fix SBI PMU masks for RV32

[PATCH v1 0/2] perf: RISC-V: fix SBI PMU masks for RV32

[Xixin Liu]

Hi,

This series fixes two RV32 mask-width issues in the SBI PMU driver:

  1) Overflow status / restart tracking is u64 but used BIT(), which is
     an unsigned long shift and breaks for indices >= 32 on RV32
  2) The available-counter mask was a single unsigned long while
     iteration uses RISCV_MAX_COUNTERS (64), so RV32 can read past the
     object; store it as a DECLARE_BITMAP

Patches are independent. Please review.

Thanks,
Xixin Liu

---

Xixin Liu (2):
  perf: RISC-V: use BIT_ULL for u64 overflow masks
  perf: RISC-V: store available counter mask as bitmap

 drivers/perf/riscv_pmu_legacy.c |  5 +++--
 drivers/perf/riscv_pmu_sbi.c    | 35 ++++++++++++++++++++++------------
 include/linux/perf/riscv_pmu.h  |  2 +-
 3 files changed, 27 insertions(+), 15 deletions(-)

-- 
2.43.0

[PATCH v1 2/2] perf: RISC-V: store available counter mask as bitmap

[Xixin Liu]

The available-counter mask was a single unsigned long, but iteration
uses RISCV_MAX_COUNTERS (64). On RV32 that reads past the object. Filling
with BIT(i) is also wrong for i >= 32.

Use DECLARE_BITMAP, set_bit/bitmap helpers, and stop counters one word
at a time.

Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/perf/riscv_pmu_legacy.c |  5 +++--
 drivers/perf/riscv_pmu_sbi.c    | 29 ++++++++++++++++++-----------
 include/linux/perf/riscv_pmu.h  |  2 +-
 3 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/perf/riscv_pmu_legacy.c b/drivers/perf/riscv_pmu_legacy.c
index 4d6461d6a..1b8e4789c 100644
--- a/drivers/perf/riscv_pmu_legacy.c
+++ b/drivers/perf/riscv_pmu_legacy.c
@@ -110,8 +110,9 @@ static void pmu_legacy_init(struct riscv_pmu *pmu)
 {
 	pr_info("Legacy PMU implementation is available\n");
 
-	pmu->cmask = BIT(RISCV_PMU_LEGACY_CYCLE) |
-		BIT(RISCV_PMU_LEGACY_INSTRET);
+	bitmap_zero(pmu->cmask, RISCV_MAX_COUNTERS);
+	set_bit(RISCV_PMU_LEGACY_CYCLE, pmu->cmask);
+	set_bit(RISCV_PMU_LEGACY_INSTRET, pmu->cmask);
 	pmu->ctr_start = pmu_legacy_ctr_start;
 	pmu->ctr_stop = NULL;
 	pmu->event_map = pmu_legacy_event_map;
diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
index d6c66e375..c913d73f8 100644
--- a/drivers/perf/riscv_pmu_sbi.c
+++ b/drivers/perf/riscv_pmu_sbi.c
@@ -97,7 +97,7 @@ static unsigned int riscv_pmu_irq_mask;
 static unsigned int riscv_pmu_irq;
 
 /* Cache the available counters in a bitmask */
-static unsigned long cmask;
+static DECLARE_BITMAP(cmask, RISCV_MAX_COUNTERS);
 
 static int pmu_event_find_cache(u64 config);
 struct sbi_pmu_event_data {
@@ -364,7 +364,7 @@ static void pmu_sbi_check_event(struct sbi_pmu_event_data *edata)
 	struct sbiret ret;
 
 	ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_CFG_MATCH,
-			0, cmask, 0, edata->event_idx, 0, 0);
+			0, cmask[0], 0, edata->event_idx, 0, 0);
 	if (!ret.error) {
 		sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP,
 			  ret.value, 0x1, SBI_PMU_STOP_FLAG_RESET, 0, 0, 0);
@@ -488,10 +488,10 @@ int riscv_pmu_get_hpm_info(u32 *hw_ctr_width, u32 *num_hw_ctr)
 	union sbi_pmu_ctr_info *info;
 	u32 hpm_width = 0, hpm_count = 0;
 
-	if (!cmask)
+	if (bitmap_empty(cmask, RISCV_MAX_COUNTERS))
 		return -EINVAL;
 
-	for_each_set_bit(i, &cmask, RISCV_MAX_COUNTERS) {
+	for_each_set_bit(i, cmask, RISCV_MAX_COUNTERS) {
 		info = &pmu_ctr_list[i];
 		if (!info)
 			continue;
@@ -541,7 +541,7 @@ static int pmu_sbi_ctr_get_idx(struct perf_event *event)
 	struct cpu_hw_events *cpuc = this_cpu_ptr(rvpmu->hw_events);
 	struct sbiret ret;
 	int idx;
-	uint64_t cbase = 0, cmask = rvpmu->cmask;
+	uint64_t cbase = 0, cmask = rvpmu->cmask[0];
 	unsigned long cflags = 0;
 
 	cflags = pmu_sbi_get_filter_flags(event);
@@ -577,7 +577,7 @@ static int pmu_sbi_ctr_get_idx(struct perf_event *event)
 	}
 
 	idx = ret.value;
-	if (!test_bit(idx, &rvpmu->cmask) || !pmu_ctr_list[idx].value)
+	if (!test_bit(idx, rvpmu->cmask) || !pmu_ctr_list[idx].value)
 		return -ENOENT;
 
 	/* Additional sanity check for the counter id */
@@ -881,7 +881,7 @@ static int pmu_sbi_get_ctrinfo(int nctr, unsigned long *mask)
 			/* The logical counter ids are not expected to be contiguous */
 			continue;
 
-		*mask |= BIT(i);
+		set_bit(i, mask);
 
 		cinfo.value = ret.value;
 		if (cinfo.type == SBI_PMU_CTR_TYPE_FW)
@@ -898,12 +898,19 @@ static int pmu_sbi_get_ctrinfo(int nctr, unsigned long *mask)
 
 static inline void pmu_sbi_stop_all(struct riscv_pmu *pmu)
 {
+	int i;
+
 	/*
 	 * No need to check the error because we are disabling all the counters
 	 * which may include counters that are not enabled yet.
 	 */
-	sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP,
-		  0, pmu->cmask, SBI_PMU_STOP_FLAG_RESET, 0, 0, 0);
+	for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
+		if (!pmu->cmask[i])
+			continue;
+		sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP,
+			  i * BITS_PER_LONG, pmu->cmask[i],
+			  SBI_PMU_STOP_FLAG_RESET, 0, 0, 0);
+	}
 }
 
 static inline void pmu_sbi_stop_hw_ctrs(struct riscv_pmu *pmu)
@@ -1442,7 +1449,7 @@ static int pmu_sbi_device_probe(struct platform_device *pdev)
 	}
 
 	/* cache all the information about counters now */
-	if (pmu_sbi_get_ctrinfo(num_counters, &cmask))
+	if (pmu_sbi_get_ctrinfo(num_counters, cmask))
 		goto out_free;
 
 	ret = pmu_sbi_setup_irqs(pmu, pdev);
@@ -1454,7 +1461,7 @@ static int pmu_sbi_device_probe(struct platform_device *pdev)
 
 	pmu->pmu.attr_groups = riscv_pmu_attr_groups;
 	pmu->pmu.parent = &pdev->dev;
-	pmu->cmask = cmask;
+	bitmap_copy(pmu->cmask, cmask, RISCV_MAX_COUNTERS);
 	pmu->ctr_start = pmu_sbi_ctr_start;
 	pmu->ctr_stop = pmu_sbi_ctr_stop;
 	pmu->event_map = pmu_sbi_event_map;
diff --git a/include/linux/perf/riscv_pmu.h b/include/linux/perf/riscv_pmu.h
index f82a28040..ecaa40370 100644
--- a/include/linux/perf/riscv_pmu.h
+++ b/include/linux/perf/riscv_pmu.h
@@ -55,7 +55,7 @@ struct riscv_pmu {
 
 	irqreturn_t	(*handle_irq)(int irq_num, void *dev);
 
-	unsigned long	cmask;
+	DECLARE_BITMAP(cmask, RISCV_MAX_COUNTERS);
 	u64		(*ctr_read)(struct perf_event *event);
 	int		(*ctr_get_idx)(struct perf_event *event);
 	int		(*ctr_get_width)(int idx);

Re: [PATCH v1 2/2] perf: RISC-V: store available counter mask as bitmap

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] The patch fails to fully support matching 64 counters on RV32, as it hardcodes `cmask[0]` and `cbase = 0` in SBI counter match queries, effectively ignoring counters 32-63.

Pre-existing issues:
- [High] An out-of-bounds counter base index is passed to `SBI_EXT_PMU_COUNTER_START` in the snapshot overflow handler.
- [High] Global `cmask` is not cleared on probe failure, leading to a NULL pointer dereference in `riscv_pmu_get_hpm_info()`.
- [High] Incorrect snapshot array indexing and overflow mask shifting for PMU counters >= 32 on RV32.
--

--- Patch [2]: [PATCH v1 2/2] perf: RISC-V: store available counter mask as bitmap ---
commit 0c40d37d24749711315dcc35e091383a23b8eeb6
Author: Xixin Liu <liuxixin@kylinos.cn>
Subject: perf: RISC-V: store available counter mask as bitmap

The available-counter mask was previously a single unsigned long, which
caused out-of-bounds reads on RV32 when iterating up to RISCV_MAX_COUNTERS.
This commit transitions the mask to use DECLARE_BITMAP and related helpers
to prevent reads past the object and properly stop counters across words.

> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -364,7 +364,7 @@ static void pmu_sbi_check_event(struct sbi_pmu_event_data *edata)
>  	struct sbiret ret;
>  
>  	ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_CFG_MATCH,
> -			0, cmask, 0, edata->event_idx, 0, 0);
> +			0, cmask[0], 0, edata->event_idx, 0, 0);

[Severity: High]
Does this code fail to match all 64 counters on RV32?

By hardcoding cmask[0] and a base of 0, it appears to ignore counters 32-63
during the SBI counter match queries.

Should this issue subsequent sbi_ecall queries for cmask[1] with cbase = 32
to ensure the upper half of the counters are checked?

[ ... ]

> @@ -541,7 +541,7 @@ static int pmu_sbi_ctr_get_idx(struct perf_event *event)
>  	struct cpu_hw_events *cpuc = this_cpu_ptr(rvpmu->hw_events);
>  	struct sbiret ret;
>  	int idx;
> -	uint64_t cbase = 0, cmask = rvpmu->cmask;
> +	uint64_t cbase = 0, cmask = rvpmu->cmask[0];

[Severity: High]
Does this code also fail to support matching counters >= 32 on RV32?

Similar to pmu_sbi_check_event(), initializing cmask to rvpmu->cmask[0] and
using it as the sole mask for the match call will result in -ENOENT for
events that could be satisfied by the upper counters.

[ ... ]

> @@ -1451,7 +1458,7 @@ static int pmu_sbi_device_probe(struct platform_device *pdev)
>  	}
>  
>  	/* cache all the information about counters now */
> -	if (pmu_sbi_get_ctrinfo(num_counters, &cmask))
> +	if (pmu_sbi_get_ctrinfo(num_counters, cmask))
>  		goto out_free;

[Severity: High]
This is a pre-existing issue, but does the global cmask get left populated
with stale data on probe failure?

If pmu_sbi_device_probe() fails after pmu_sbi_get_ctrinfo() (for instance,
during IRQ setup), the out_free error path frees and nullifies
pmu_ctr_list:

drivers/perf/riscv_pmu_sbi.c:pmu_sbi_device_probe() {
    ...
out_free:
    free_percpu(pmu->hw_events);
    kfree(pmu_ctr_list);
    pmu_ctr_list = NULL;
    ...
}

However, the global cmask bitmap retains its bits. Later, if KVM calls
riscv_pmu_get_hpm_info(), it will bypass the bitmap_empty() check,
iterate over the stale bits, and dereference the NULL pmu_ctr_list array.

[Severity: High]
This is a pre-existing issue, but can the PMU counters fail to restart
after an overflow when snapshots are enabled?

In pmu_sbi_start_ovf_ctrs_snapshot(), the loop-exhausted idx variable is
used to calculate the base counter index instead of the outer loop
variable i:

drivers/perf/riscv_pmu_sbi.c:pmu_sbi_start_ovf_ctrs_snapshot() {
    ...
    for_each_set_bit(idx, &cpu_hw_evt->used_hw_ctrs[i], BITS_PER_LONG) {
        ...
    }
    
    sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, idx * BITS_PER_LONG,
              ...
}

The for_each_set_bit() loop terminates with idx equal to BITS_PER_LONG.
Passing idx * BITS_PER_LONG (e.g., 1024 on RV32) as the counter_idx_base
to SBI_EXT_PMU_COUNTER_START will cause the firmware to reject the
out-of-bounds base index.

[Severity: High]
This is a pre-existing issue, but does this code corrupt snapshot memory
for counters >= 32 on RV32?

When stopping hardware counters, the snapshot counter values are read using
relative indices instead of absolute ones:

drivers/perf/riscv_pmu_sbi.c:pmu_sbi_stop_hw_ctrs() {
    ...
    for_each_set_bit(idx, &cpu_hw_evt->used_hw_ctrs[i], BITS_PER_LONG)
        cpu_hw_evt->snapshot_cval_shcopy[i * BITS_PER_LONG + idx] =
                    sdata->ctr_values[idx];
    ...
}

Additionally, the absolute 64-bit overflow mask is incorrectly shifted:

    temp_ctr_overflow_mask |= sdata->ctr_overflow_mask << (i * BITS_PER_LONG);

For i = 1 (counters 32-63), this shifts the valid overflow bits out of
bounds.

Furthermore, in pmu_sbi_start_ovf_ctrs_snapshot(), the snapshot counter
values are written back using relative indices:

    sdata->ctr_values[idx] = ...

This overwrites the first 32 counters' values instead of writing to the
counters 32-63 region.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1786092600.git.liuxixin@kylinos.cn?part=2

Re: [PATCH v1 2/2] perf: RISC-V: store available counter mask as bitmap

[Paul Walmsley]

Hi,

On Fri, 7 Aug 2026, Xixin Liu wrote:

> The available-counter mask was a single unsigned long, but iteration
> uses RISCV_MAX_COUNTERS (64). On RV32 that reads past the object. Filling
> with BIT(i) is also wrong for i >= 32.
> 
> Use DECLARE_BITMAP, set_bit/bitmap helpers, and stop counters one word
> at a time.
> 
> Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>

This patch contains several array indexes to element 0 of the cmask 
bitmap, but that element is going to be XLEN bits wide.   Does that 
actually work on RV32 systems?


- Paul

Re: [PATCH v1 2/2] perf: RISC-V: store available counter mask as bitmap

[Xixin Liu]

Hi Paul,

On Fri, 7 Aug 2026, Paul Walmsley wrote:
> This patch contains several array indexes to element 0 of the cmask
> bitmap, but that element is going to be XLEN bits wide.   Does that
> actually work on RV32 systems?

I built with CONFIG_ARCH_RV32I=y and booted qemu-system-riscv32.
Probe reports 16 firmware and 18 hardware counters.  The available
bitmap is two words: 0xfffffffd then 0x7.  Three bits sit at index 32
and above.

When CFG_MATCH fails on the first word, the same call is retried with
base 32 and the second word.  That is the walk the stop-all path already
used.  Clearing the first word still issues that second call with
base 32.

I also booted qemu-system-riscv64.  Probe reports the same 16 firmware
and 18 hardware counters.  The available bitmap is one word:
0x7fffffffd.  CFG_MATCH and stop-all stay on that word.  Opening a
cycles event from userspace succeeds on both guests.

A legacy-only RV32 kernel sets cycle and instret in the bitmap.  Opening
a cycles event from userspace succeeds there too.

v2 walks every bitmap word for the remaining CFG_MATCH sites, same as
stop-all, with the counter base set to i times BITS_PER_LONG.

Thanks,
Xixin

[PATCH v1 1/2] perf: RISC-V: use BIT_ULL for u64 overflow masks

[Xixin Liu]

Overflow status and restart masks are u64, but bits were built with
BIT(). On RV32 that is an unsigned long shift, so indices >= 32 truncate
or wrap and corrupt the mask.

Use BIT_ULL() for those u64 bitops.

Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/perf/riscv_pmu_sbi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
index dfc886dee..93a31d5d5 100644
--- a/drivers/perf/riscv_pmu_sbi.c
+++ b/drivers/perf/riscv_pmu_sbi.c
@@ -1002,7 +1002,7 @@ static inline void pmu_sbi_start_ovf_ctrs_snapshot(struct cpu_hw_events *cpu_hw_
 	struct riscv_pmu_snapshot_data *sdata = cpu_hw_evt->snapshot_addr;
 
 	for_each_set_bit(idx, cpu_hw_evt->used_hw_ctrs, RISCV_MAX_COUNTERS) {
-		if (ctr_ovf_mask & BIT(idx)) {
+		if (ctr_ovf_mask & BIT_ULL(idx)) {
 			event = cpu_hw_evt->events[idx];
 			hwc = &event->hw;
 			max_period = riscv_pmu_ctr_get_width_mask(event);
@@ -1109,14 +1109,14 @@ static irqreturn_t pmu_sbi_ovf_handler(int irq, void *dev)
 			hidx = info->csr - CSR_CYCLE;
 
 		/* check if the corresponding bit is set in scountovf or overflow mask in shmem */
-		if (!(overflow & BIT(hidx)))
+		if (!(overflow & BIT_ULL(hidx)))
 			continue;
 
 		/*
 		 * Keep a track of overflowed counters so that they can be started
 		 * with updated initial value.
 		 */
-		overflowed_ctrs |= BIT(lidx);
+		overflowed_ctrs |= BIT_ULL(lidx);
 		hw_evt = &event->hw;
 		/* Update the event states here so that we know the state while reading */
 		hw_evt->state |= PERF_HES_STOPPED;

Re: [PATCH v1 1/2] perf: RISC-V: use BIT_ULL for u64 overflow masks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [High] Invalid SBI base index due to out-of-scope loop variable in pmu_sbi_start_ovf_ctrs_snapshot().
- [High] Incorrect mask arithmetic in pmu_sbi_start_ovf_ctrs_sbi() on RV32 mixes up counter indices.
- [High] Buffer over-read and undefined shift behavior due to global cmask being an unsigned long.
- [High] Early exit path in pmu_sbi_ovf_handler() triggers undefined shift and incorrect CSR access on Andes custom PMUs.
- [High] Absolute 64-bit SBI PMU snapshot shared memory structures are incorrectly shifted or indexed relative to cbase on RV32.
--

--- Patch [1]: [PATCH v1 1/2] perf: RISC-V: use BIT_ULL for u64 overflow masks ---
commit d452f12b31f1955f29f737a7fc0bc5084ebd71af
Author: Xixin Liu <liuxixin@kylinos.cn>
Subject: perf: RISC-V: use BIT_ULL for u64 overflow masks

This patch replaces BIT() with BIT_ULL() for u64 overflow status and
restart masks in the RISC-V SBI PMU driver. This fixes truncation and
wrapping issues on RV32 where BIT() resolves to an unsigned long shift.

> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index 50220f7b46d9b..8ea5ae6173471 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -1002,7 +1002,7 @@ static inline void pmu_sbi_start_ovf_ctrs_snapshot(struct cpu_hw_events *cpu_hw_
>  	struct riscv_pmu_snapshot_data *sdata = cpu_hw_evt->snapshot_addr;
>  
>  	for_each_set_bit(idx, cpu_hw_evt->used_hw_ctrs, RISCV_MAX_COUNTERS) {
> -		if (ctr_ovf_mask & BIT(idx)) {
> +		if (ctr_ovf_mask & BIT_ULL(idx)) {
>  			event = cpu_hw_evt->events[idx];
>  			hwc = &event->hw;
>  			max_period = riscv_pmu_ctr_get_width_mask(event);

[Severity: High]
This isn't a bug introduced by this patch, but since we're fixing issues in
pmu_sbi_start_ovf_ctrs_snapshot(), there is an SBI base index problem slightly
lower in this same function:

pmu_sbi_start_ovf_ctrs_snapshot() {
    ...
    for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
        for_each_set_bit(idx, &cpu_hw_evt->used_hw_ctrs[i], BITS_PER_LONG)
            ...
        sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, idx * BITS_PER_LONG,
                  cpu_hw_evt->used_hw_ctrs[i], flag, 0, 0, 0);
    }
}

Does this pass an invalid base index to the SBI firmware? The for_each_set_bit()
macro terminates with idx equal to BITS_PER_LONG. So idx * BITS_PER_LONG would
evaluate to something like 4096 on RV64, which seems like it would cause an
SBI_ERR_INVALID_PARAM error. Should this use i * BITS_PER_LONG instead?

[Severity: High]
This is another pre-existing issue in pmu_sbi_start_ovf_ctrs_snapshot() related
to RV32 shared memory structures:

pmu_sbi_start_ovf_ctrs_snapshot() {
    ...
    for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
        for_each_set_bit(idx, &cpu_hw_evt->used_hw_ctrs[i], BITS_PER_LONG)
            sdata->ctr_values[idx] =
                cpu_hw_evt->snapshot_cval_shcopy[idx + i * BITS_PER_LONG];
    ...
}

Since ctr_values is a 64-element array indexed by absolute counter IDs, does
using the relative idx (0-31) incorrectly overwrite the states of counters 0-31
when processing chunk i=1 (counters 32-63)?

[ ... ]

> @@ -1109,14 +1109,14 @@ static irqreturn_t pmu_sbi_ovf_handler(int irq, void *dev)
>  			hidx = info->csr - CSR_CYCLE;
>  
>  		/* check if the corresponding bit is set in scountovf or overflow mask in shmem */
> -		if (!(overflow & BIT(hidx)))
> +		if (!(overflow & BIT_ULL(hidx)))
>  			continue;
>  
>  		/*
>  		 * Keep a track of overflowed counters so that they can be started
>  		 * with updated initial value.
>  		 */
> -		overflowed_ctrs |= BIT(lidx);
> +		overflowed_ctrs |= BIT_ULL(lidx);
>  		hw_evt = &event->hw;
>  		/* Update the event states here so that we know the state while reading */
>  		hw_evt->state |= PERF_HES_STOPPED;

[Severity: High]
This is a pre-existing issue, but while looking at pmu_sbi_ovf_handler(), the
early exit path near the beginning of the function caught my eye:

pmu_sbi_ovf_handler() {
    ...
    fidx = find_first_bit(cpu_hw_evt->used_hw_ctrs, RISCV_MAX_COUNTERS);
    if (fidx == RISCV_MAX_COUNTERS) {
        csr_clear(CSR_SIP, BIT(riscv_pmu_irq_num));
        return IRQ_NONE;
    }
    ...
}

For the Andes custom PMU, riscv_pmu_irq_num can be assigned values like 274
(ANDES_SLI_CAUSE_BASE + ANDES_RV_IRQ_PMOVI). Does BIT(274) cause an undefined
out-of-bounds shift? Furthermore, does clearing CSR_SIP work for Andes PMUs
which use custom SLIE CSRs for these IRQs? Could this lead to an unacknowledged
interrupt storm?

[Severity: High]
This is a pre-existing issue, but I noticed another mask arithmetic problem on
RV32 that might cause broken PMU statistics. In pmu_sbi_start_ovf_ctrs_sbi():

pmu_sbi_start_ovf_ctrs_sbi() {
    ...
    for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
        ...
        ctr_start_mask = cpu_hw_evt->used_hw_ctrs[i] & ~ctr_ovf_mask;
        ...
}

On RV32, when i = 1 (counters 32-63), used_hw_ctrs[1] is a 32-bit unsigned long,
but ctr_ovf_mask is a 64-bit absolute mask. Since ~ctr_ovf_mask isn't shifted
down by i * BITS_PER_LONG, doesn't this evaluate the usage bits for counters
32-63 against the overflow status of counters 0-31?

[Severity: High]
This is also a pre-existing issue, but related to 32-bit mask truncation, is the
global cmask variable affected?

drivers/perf/riscv_pmu_sbi.c:
    static unsigned long cmask;

pmu_sbi_get_hpm_info() {
    ...
    for_each_set_bit(i, &cmask, RISCV_MAX_COUNTERS) {
    ...
}

On RV32, unsigned long is 32 bits, but RISCV_MAX_COUNTERS is 64. Does calling
for_each_set_bit() here force a 64-bit read that accesses out-of-bounds memory
adjacent to cmask? Should cmask be declared with DECLARE_BITMAP()?

[Severity: High]
Finally, this is a pre-existing issue, but similar to the ctr_values array, the
overflow mask is also shifted out-of-bounds on RV32 in pmu_sbi_stop_hw_ctrs():

pmu_sbi_stop_hw_ctrs() {
    ...
    for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
        ...
        temp_ctr_overflow_mask |= sdata->ctr_overflow_mask << (i * BITS_PER_LONG);
        ...
}

On RV32, shifting the 64-bit ctr_overflow_mask by 32 bits when i=1 pushes
the absolute bits 32-63 out of bounds. Doesn't this discard the overflow state
for counters 32-63?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1786092600.git.liuxixin@kylinos.cn?part=1

Re: [PATCH v1 1/2] perf: RISC-V: use BIT_ULL for u64 overflow masks

[Paul Walmsley]

Hi,

On Fri, 7 Aug 2026, Xixin Liu wrote:

> Overflow status and restart masks are u64, but bits were built with
> BIT(). On RV32 that is an unsigned long shift, so indices >= 32 truncate
> or wrap and corrupt the mask.
> 
> Use BIT_ULL() for those u64 bitops.
> 
> Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>

Thanks for the patch, but, same comments as on

  https://lore.kernel.org/linux-riscv/9109b689-1145-c714-ebe0-ec368f242a78@kernel.org/T/#m6fa97e46bde0d100fa0ec2854d47a57bd271c0b3


- Paul

[PATCH v2 0/2] perf: RISC-V: fix SBI PMU masks for RV32

[Xixin Liu]

Hi,

Thanks for the review, Paul.

v2 for 1/2: add Fixes and Assisted-by. Overflow masks are u64. On RV32
an unsigned-long bit at index 32 is 0, and BIT_ULL at index 32 is
0x100000000.

v2 for 2/2: same as v1, but do not pass only the first bitmap word into
CFG_MATCH. Walk each word like the stop-all path, with the counter base
set to i times BITS_PER_LONG. Share one helper for the 32-bit argument
split. Also add Fixes and Assisted-by.

Tested on qemu-system-riscv32 with CONFIG_ARCH_RV32I=y: 34 counters,
second word 0x7, CFG_MATCH retried at base 32 when the first word fails.
qemu-system-riscv64 still uses one word. A legacy-only RV32 kernel sets
cycle and instret bits and opens a cycles event. Overflow IRQ was not
taken on this QEMU virt guest, because sscof was not available to Linux.

Thanks,
Xixin Liu

---

Xixin Liu (2):
  perf: RISC-V: use BIT_ULL for u64 overflow masks
  perf: RISC-V: store available counter mask as bitmap

 drivers/perf/riscv_pmu_legacy.c |  5 ++-
 drivers/perf/riscv_pmu_sbi.c    | 88 ++++++++++++++++++++++++++++------------
 include/linux/perf/riscv_pmu.h  |  2 +-
 3 files changed, 66 insertions(+), 29 deletions(-)

-- 
2.43.0

[PATCH v2 2/2] perf: RISC-V: store available counter mask as bitmap

[Xixin Liu]

The available-counter mask was a single unsigned long, but iteration
uses RISCV_MAX_COUNTERS, which is 64. On RV32 that reads past the object.
Filling with an unsigned-long bit at index 32 and above is also wrong.

Use DECLARE_BITMAP and set_bit/bitmap helpers. Walk each bitmap word
into CFG_MATCH when checking events, when allocating an index, and when
stopping all counters. Set the counter base to i times BITS_PER_LONG.
Share the CFG_MATCH ecall through a small helper so the 32-bit argument
split is not duplicated. On qemu-system-riscv32 the probe bitmap has bits
above XLEN set, so the first word alone is not enough.

Fixes: e9991434596f ("RISC-V: Add perf platform driver based on SBI PMU extension")
Assisted-by: DeepSeek:deepseek-v3
Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/perf/riscv_pmu_legacy.c         | 5 +-
 drivers/perf/riscv_pmu_sbi.c            | 88 ++++++++++++++++++++++++++++------------
 include/linux/perf/riscv_pmu.h          | 2
 3 files changed, 66 insertions(+), 29 deletions(-)

diff --git a/drivers/perf/riscv_pmu_legacy.c b/drivers/perf/riscv_pmu_legacy.c
--- a/drivers/perf/riscv_pmu_legacy.c
+++ b/drivers/perf/riscv_pmu_legacy.c
@@ -111,8 +111,9 @@ static void pmu_legacy_init(struct riscv_pmu *pmu)
 {
 	pr_info("Legacy PMU implementation is available\n");
 
-	pmu->cmask = BIT(RISCV_PMU_LEGACY_CYCLE) |
-		BIT(RISCV_PMU_LEGACY_INSTRET);
+	bitmap_zero(pmu->cmask, RISCV_MAX_COUNTERS);
+	set_bit(RISCV_PMU_LEGACY_CYCLE, pmu->cmask);
+	set_bit(RISCV_PMU_LEGACY_INSTRET, pmu->cmask);
 	pmu->ctr_start = pmu_legacy_ctr_start;
 	pmu->ctr_stop = NULL;
 	pmu->event_map = pmu_legacy_event_map;
diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
--- a/drivers/perf/riscv_pmu_sbi.c
+++ b/drivers/perf/riscv_pmu_sbi.c
@@ -97,7 +97,7 @@ static unsigned int riscv_pmu_irq_mask;
 static unsigned int riscv_pmu_irq;
 
 /* Cache the available counters in a bitmask */
-static unsigned long cmask;
+static DECLARE_BITMAP(cmask, RISCV_MAX_COUNTERS);
 
 struct sbi_pmu_event_data {
 	union {
@@ -298,16 +298,38 @@ static struct sbi_pmu_event_data pmu_cache_event_map[PERF_COUNT_HW_CACHE_MAX]
 	},
 };
 
+static struct sbiret pmu_sbi_ctr_cfg_match(unsigned long cbase,
+					    unsigned long ctr_mask,
+					    unsigned long cflags,
+					    unsigned long event_idx,
+					    u64 config)
+{
+#if defined(CONFIG_32BIT)
+	return sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_CFG_MATCH, cbase,
+			 ctr_mask, cflags, event_idx, config, config >> 32);
+#else
+	return sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_CFG_MATCH, cbase,
+			 ctr_mask, cflags, event_idx, config, 0);
+#endif
+}
+
 static void pmu_sbi_check_event(struct sbi_pmu_event_data *edata)
 {
-	struct sbiret ret;
+	struct sbiret ret = { .error = SBI_ERR_NOT_SUPPORTED };
+	int i;
 
-	ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_CFG_MATCH,
-			0, cmask, 0, edata->event_idx, 0, 0);
-	if (!ret.error) {
-		sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP,
-			  ret.value, 0x1, SBI_PMU_STOP_FLAG_RESET, 0, 0, 0);
-	} else if (ret.error == SBI_ERR_NOT_SUPPORTED) {
+	for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
+		if (!cmask[i])
+			continue;
+		ret = pmu_sbi_ctr_cfg_match(i * BITS_PER_LONG, cmask[i], 0,
+					    edata->event_idx, 0);
+		if (!ret.error) {
+			sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP,
+				  ret.value, 0x1, SBI_PMU_STOP_FLAG_RESET, 0, 0, 0);
+			return;
+		}
+	}
+	if (ret.error == SBI_ERR_NOT_SUPPORTED) {
 		/* This event cannot be monitored by any counter */
 		edata->event_idx = -ENOENT;
 	}
@@ -353,10 +375,10 @@ int riscv_pmu_get_hpm_info(u32 *hw_ctr_width, u32 *num_hw_ctr)
 	union sbi_pmu_ctr_info *info;
 	u32 hpm_width = 0, hpm_count = 0;
 
-	if (!cmask)
+	if (bitmap_empty(cmask, RISCV_MAX_COUNTERS))
 		return -EINVAL;
 
-	for_each_set_bit(i, &cmask, RISCV_MAX_COUNTERS) {
+	for_each_set_bit(i, cmask, RISCV_MAX_COUNTERS) {
 		info = &pmu_ctr_list[i];
 		if (!info)
 			continue;
@@ -405,8 +427,8 @@ static int pmu_sbi_ctr_get_idx(struct perf_event *event)
 	struct riscv_pmu *rvpmu = to_riscv_pmu(event->pmu);
 	struct cpu_hw_events *cpuc = this_cpu_ptr(rvpmu->hw_events);
 	struct sbiret ret;
-	int idx;
-	uint64_t cbase = 0, cmask = rvpmu->cmask;
+	int idx, i;
+	uint64_t cbase = 0, cmask = 0;
 	unsigned long cflags = 0;
 
 	cflags = pmu_sbi_get_filter_flags(event);
@@ -427,14 +449,21 @@ static int pmu_sbi_ctr_get_idx(struct perf_event *event)
 	}
 
 	/* retrieve the available counter index */
-#if defined(CONFIG_32BIT)
-	ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_CFG_MATCH, cbase,
-			cmask, cflags, hwc->event_base, hwc->config,
-			hwc->config >> 32);
-#else
-	ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_CFG_MATCH, cbase,
-			cmask, cflags, hwc->event_base, hwc->config, 0);
-#endif
+	if (cmask) {
+		ret = pmu_sbi_ctr_cfg_match(cbase, cmask, cflags, hwc->event_base,
+					   hwc->config);
+	} else {
+		ret.error = SBI_ERR_NOT_SUPPORTED;
+		for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
+			if (!rvpmu->cmask[i])
+				continue;
+			cbase = i * BITS_PER_LONG;
+			ret = pmu_sbi_ctr_cfg_match(cbase, rvpmu->cmask[i], cflags,
+						   hwc->event_base, hwc->config);
+			if (!ret.error)
+				break;
+		}
+	}
 	if (ret.error) {
 		pr_debug("Not able to find a counter for event %lx config %llx\n",
 			hwc->event_base, hwc->config);
@@ -442,7 +471,7 @@ static int pmu_sbi_ctr_get_idx(struct perf_event *event)
 	}
 
 	idx = ret.value;
-	if (!test_bit(idx, &rvpmu->cmask) || !pmu_ctr_list[idx].value)
+	if (!test_bit(idx, rvpmu->cmask) || !pmu_ctr_list[idx].value)
 		return -ENOENT;
 
 	/* Additional sanity check for the counter id */
@@ -794,7 +823,7 @@ static int pmu_sbi_get_ctrinfo(int nctr, unsigned long *mask)
 			/* The logical counter ids are not expected to be contiguous */
 			continue;
 
-		*mask |= BIT(i);
+		set_bit(i, mask);
 
 		cinfo.value = ret.value;
 		if (cinfo.type == SBI_PMU_CTR_TYPE_FW)
@@ -811,12 +840,19 @@ static int pmu_sbi_get_ctrinfo(int nctr, unsigned long *mask)
 
 static inline void pmu_sbi_stop_all(struct riscv_pmu *pmu)
 {
+	int i;
+
 	/*
 	 * No need to check the error because we are disabling all the counters
 	 * which may include counters that are not enabled yet.
 	 */
-	sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP,
-		  0, pmu->cmask, SBI_PMU_STOP_FLAG_RESET, 0, 0, 0);
+	for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
+		if (!pmu->cmask[i])
+			continue;
+		sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP,
+			  i * BITS_PER_LONG, pmu->cmask[i],
+			  SBI_PMU_STOP_FLAG_RESET, 0, 0, 0);
+	}
 }
 
 static inline void pmu_sbi_stop_hw_ctrs(struct riscv_pmu *pmu)
@@ -1353,7 +1389,7 @@ static int pmu_sbi_device_probe(struct platform_device *pdev)
 	}
 
 	/* cache all the information about counters now */
-	if (pmu_sbi_get_ctrinfo(num_counters, &cmask))
+	if (pmu_sbi_get_ctrinfo(num_counters, cmask))
 		goto out_free;
 
 	ret = pmu_sbi_setup_irqs(pmu, pdev);
@@ -1365,7 +1401,7 @@ static int pmu_sbi_device_probe(struct platform_device *pdev)
 
 	pmu->pmu.attr_groups = riscv_pmu_attr_groups;
 	pmu->pmu.parent = &pdev->dev;
-	pmu->cmask = cmask;
+	bitmap_copy(pmu->cmask, cmask, RISCV_MAX_COUNTERS);
 	pmu->ctr_start = pmu_sbi_ctr_start;
 	pmu->ctr_stop = pmu_sbi_ctr_stop;
 	pmu->event_map = pmu_sbi_event_map;
diff --git a/include/linux/perf/riscv_pmu.h b/include/linux/perf/riscv_pmu.h
--- a/include/linux/perf/riscv_pmu.h
+++ b/include/linux/perf/riscv_pmu.h
@@ -55,7 +55,7 @@ struct riscv_pmu {
 
 	irqreturn_t	(*handle_irq)(int irq_num, void *dev);
 
-	unsigned long	cmask;
+	DECLARE_BITMAP(cmask, RISCV_MAX_COUNTERS);
 	u64		(*ctr_read)(struct perf_event *event);
 	int		(*ctr_get_idx)(struct perf_event *event);
 	int		(*ctr_get_width)(int idx);

-- 
2.43.0

[PATCH v2 1/2] perf: RISC-V: use BIT_ULL for u64 overflow masks

[Xixin Liu]

Overflow status and restart masks are u64, but bits were built with
BIT(). On RV32 that is an unsigned long shift, so indices >= 32 truncate
or wrap and corrupt the mask.

Use BIT_ULL() for those u64 bitops.

Fixes: a8625217a054 ("drivers/perf: riscv: Implement SBI PMU snapshot function")
Assisted-by: DeepSeek:deepseek-v3
Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/perf/riscv_pmu_sbi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
--- a/drivers/perf/riscv_pmu_sbi.c
+++ b/drivers/perf/riscv_pmu_sbi.c
@@ -913,7 +913,7 @@ static inline void pmu_sbi_start_ovf_ctrs_snapshot(struct cpu_hw_events *cpu_hw_
 	struct riscv_pmu_snapshot_data *sdata = cpu_hw_evt->snapshot_addr;
 
 	for_each_set_bit(idx, cpu_hw_evt->used_hw_ctrs, RISCV_MAX_COUNTERS) {
-		if (ctr_ovf_mask & BIT(idx)) {
+		if (ctr_ovf_mask & BIT_ULL(idx)) {
 			event = cpu_hw_evt->events[idx];
 			hwc = &event->hw;
 			max_period = riscv_pmu_ctr_get_width_mask(event);
@@ -1020,14 +1020,14 @@ static irqreturn_t pmu_sbi_ovf_handler(int irq, void *dev)
 			hidx = info->csr - CSR_CYCLE;
 
 		/* check if the corresponding bit is set in sscountovf or overflow mask in shmem */
-		if (!(overflow & BIT(hidx)))
+		if (!(overflow & BIT_ULL(hidx)))
 			continue;
 
 		/*
 		 * Keep a track of overflowed counters so that they can be started
 		 * with updated initial value.
 		 */
-		overflowed_ctrs |= BIT(lidx);
+		overflowed_ctrs |= BIT_ULL(lidx);
 		hw_evt = &event->hw;
 		/* Update the event states here so that we know the state while reading */
 		hw_evt->state |= PERF_HES_STOPPED;

-- 
2.43.0