public inbox for opensbi@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v2] lib: sbi_pmu: Add FW counter index validation when reading high bits on RV64
@ 2026-01-25  9:06 James Raphael Tiovalen
  2026-02-22 15:29 ` Anup Patel
  0 siblings, 1 reply; 2+ messages in thread
From: James Raphael Tiovalen @ 2026-01-25  9:06 UTC (permalink / raw)
  To: opensbi; +Cc: andrew.jones, atishp, radim.krcmar, James Raphael Tiovalen

Currently, when we attempt to read the upper 32 bits of a firmware
counter on RV64 or higher, we just set `sbiret.value` to 0 without
validating the counter index. The SBI specification requires us to set
`sbiret.error` to `SBI_ERR_INVALID_PARAM` if the counter index points to
a hardware counter or an invalid counter. Add a validation check to
ensure compliance with the specification on RV64 or higher.

Fixes: 51951d9e9af8 ("lib: sbi_pmu: Implement sbi_pmu_counter_fw_read_hi")
Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
---
v1 -> v2:
  - Implemented Radim's suggestion to refactor `sbi_pmu_ctr_fw_read()`
    instead of adding a new function.
  - Updated the type of `cidx` in the function signature of
    `sbi_pmu_ctr_fw_read()`.
---
 include/sbi/sbi_pmu.h   |  2 +-
 lib/sbi/sbi_ecall_pmu.c |  4 ++--
 lib/sbi/sbi_pmu.c       | 10 +++++++++-
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
index c0e25f5a..3fff4627 100644
--- a/include/sbi/sbi_pmu.h
+++ b/include/sbi/sbi_pmu.h
@@ -136,7 +136,7 @@ int sbi_pmu_add_hw_event_counter_map(u32 eidx_start, u32 eidx_end, u32 cmap);
 
 int sbi_pmu_add_raw_event_counter_map(uint64_t select, uint64_t select_mask, u32 cmap);
 
-int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval);
+int sbi_pmu_ctr_fw_read(unsigned long cidx, uint64_t *cval, bool high_bits);
 
 int sbi_pmu_ctr_stop(unsigned long cidx_base, unsigned long cidx_mask,
 		     unsigned long flag);
diff --git a/lib/sbi/sbi_ecall_pmu.c b/lib/sbi/sbi_ecall_pmu.c
index 868e8665..41bb1624 100644
--- a/lib/sbi/sbi_ecall_pmu.c
+++ b/lib/sbi/sbi_ecall_pmu.c
@@ -50,12 +50,12 @@ static int sbi_ecall_pmu_handler(unsigned long extid, unsigned long funcid,
 
 		break;
 	case SBI_EXT_PMU_COUNTER_FW_READ:
-		ret = sbi_pmu_ctr_fw_read(regs->a0, &temp);
+		ret = sbi_pmu_ctr_fw_read(regs->a0, &temp, false);
 		out->value = temp;
 		break;
 	case SBI_EXT_PMU_COUNTER_FW_READ_HI:
+		ret = sbi_pmu_ctr_fw_read(regs->a0, &temp, true);
 #if __riscv_xlen == 32
-		ret = sbi_pmu_ctr_fw_read(regs->a0, &temp);
 		out->value = temp >> 32;
 #else
 		out->value = 0;
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index e084005d..a310553e 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -227,7 +227,7 @@ static bool pmu_ctr_idx_validate(unsigned long cbase, unsigned long cmask)
 	return cmask && cbase + sbi_fls(cmask) < total_ctrs;
 }
 
-int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
+int sbi_pmu_ctr_fw_read(unsigned long cidx, uint64_t *cval, bool high_bits)
 {
 	int event_idx_type;
 	uint32_t event_code;
@@ -236,6 +236,14 @@ int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
 	if (unlikely(!phs))
 		return SBI_EINVAL;
 
+	if (cidx < num_hw_ctrs || cidx >= total_ctrs)
+		return SBI_EINVAL;
+
+#if __riscv_xlen > 32
+	if (high_bits)
+		return 0;
+#endif
+
 	event_idx_type = pmu_ctr_validate(phs, cidx, &event_code);
 	if (event_idx_type != SBI_PMU_EVENT_TYPE_FW)
 		return SBI_EINVAL;
-- 
2.43.0


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [PATCH v2] lib: sbi_pmu: Add FW counter index validation when reading high bits on RV64
  2026-01-25  9:06 [PATCH v2] lib: sbi_pmu: Add FW counter index validation when reading high bits on RV64 James Raphael Tiovalen
@ 2026-02-22 15:29 ` Anup Patel
  0 siblings, 0 replies; 2+ messages in thread
From: Anup Patel @ 2026-02-22 15:29 UTC (permalink / raw)
  To: James Raphael Tiovalen; +Cc: opensbi, andrew.jones, atishp, radim.krcmar

On Sun, Jan 25, 2026 at 2:36 PM James Raphael Tiovalen
<jamestiotio@gmail.com> wrote:
>
> Currently, when we attempt to read the upper 32 bits of a firmware
> counter on RV64 or higher, we just set `sbiret.value` to 0 without
> validating the counter index. The SBI specification requires us to set
> `sbiret.error` to `SBI_ERR_INVALID_PARAM` if the counter index points to
> a hardware counter or an invalid counter. Add a validation check to
> ensure compliance with the specification on RV64 or higher.
>
> Fixes: 51951d9e9af8 ("lib: sbi_pmu: Implement sbi_pmu_counter_fw_read_hi")
> Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>

LGTM.

Reviewed-by: Anup Patel <anup@brainfault.org>

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup


> ---
> v1 -> v2:
>   - Implemented Radim's suggestion to refactor `sbi_pmu_ctr_fw_read()`
>     instead of adding a new function.
>   - Updated the type of `cidx` in the function signature of
>     `sbi_pmu_ctr_fw_read()`.
> ---
>  include/sbi/sbi_pmu.h   |  2 +-
>  lib/sbi/sbi_ecall_pmu.c |  4 ++--
>  lib/sbi/sbi_pmu.c       | 10 +++++++++-
>  3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
> index c0e25f5a..3fff4627 100644
> --- a/include/sbi/sbi_pmu.h
> +++ b/include/sbi/sbi_pmu.h
> @@ -136,7 +136,7 @@ int sbi_pmu_add_hw_event_counter_map(u32 eidx_start, u32 eidx_end, u32 cmap);
>
>  int sbi_pmu_add_raw_event_counter_map(uint64_t select, uint64_t select_mask, u32 cmap);
>
> -int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval);
> +int sbi_pmu_ctr_fw_read(unsigned long cidx, uint64_t *cval, bool high_bits);
>
>  int sbi_pmu_ctr_stop(unsigned long cidx_base, unsigned long cidx_mask,
>                      unsigned long flag);
> diff --git a/lib/sbi/sbi_ecall_pmu.c b/lib/sbi/sbi_ecall_pmu.c
> index 868e8665..41bb1624 100644
> --- a/lib/sbi/sbi_ecall_pmu.c
> +++ b/lib/sbi/sbi_ecall_pmu.c
> @@ -50,12 +50,12 @@ static int sbi_ecall_pmu_handler(unsigned long extid, unsigned long funcid,
>
>                 break;
>         case SBI_EXT_PMU_COUNTER_FW_READ:
> -               ret = sbi_pmu_ctr_fw_read(regs->a0, &temp);
> +               ret = sbi_pmu_ctr_fw_read(regs->a0, &temp, false);
>                 out->value = temp;
>                 break;
>         case SBI_EXT_PMU_COUNTER_FW_READ_HI:
> +               ret = sbi_pmu_ctr_fw_read(regs->a0, &temp, true);
>  #if __riscv_xlen == 32
> -               ret = sbi_pmu_ctr_fw_read(regs->a0, &temp);
>                 out->value = temp >> 32;
>  #else
>                 out->value = 0;
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index e084005d..a310553e 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -227,7 +227,7 @@ static bool pmu_ctr_idx_validate(unsigned long cbase, unsigned long cmask)
>         return cmask && cbase + sbi_fls(cmask) < total_ctrs;
>  }
>
> -int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
> +int sbi_pmu_ctr_fw_read(unsigned long cidx, uint64_t *cval, bool high_bits)
>  {
>         int event_idx_type;
>         uint32_t event_code;
> @@ -236,6 +236,14 @@ int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
>         if (unlikely(!phs))
>                 return SBI_EINVAL;
>
> +       if (cidx < num_hw_ctrs || cidx >= total_ctrs)
> +               return SBI_EINVAL;
> +
> +#if __riscv_xlen > 32
> +       if (high_bits)
> +               return 0;
> +#endif
> +
>         event_idx_type = pmu_ctr_validate(phs, cidx, &event_code);
>         if (event_idx_type != SBI_PMU_EVENT_TYPE_FW)
>                 return SBI_EINVAL;
> --
> 2.43.0
>
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-02-22 15:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-25  9:06 [PATCH v2] lib: sbi_pmu: Add FW counter index validation when reading high bits on RV64 James Raphael Tiovalen
2026-02-22 15:29 ` Anup Patel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox