diff mbox series

[v2,1/3] lib: sbi_pmu: Return invalid param error for reserved event_idx bits

Message ID 20260818210023.466462-2-david.garcia@aheadcomputing.com
State Accepted
Headers show
Series lib: sbi_pmu: SBI v3.0 PMU error code fixes | expand

Commit Message

David E. Garcia Porras Aug. 18, 2026, 9 p.m. UTC
As per section 11.14 of the SBI specification (Function: Get PMU Event
Info, FID #8), Table 47, the event_idx word of an event info entry
only uses BIT[0:19]; BIT[20:31] are reserved for the future purpose
and must be zero. Table 48 further requires the SBI implementation to
return SBI_ERR_INVALID_PARAM if any reserved bit in an event_idx word
is set.

sbi_pmu_event_get_info() does not check the reserved bits, so a
malformed event_idx is silently passed on to pmu_event_validate()
instead of failing the call. Add SBI_PMU_EVENT_IDX_MBZ_MASK covering
the must-be-zero bits and return SBI_ERR_INVALID_PARAM when any of
them are set.

Fixes: e4345842168b ("lib: sbi_pmu: Implement SBI PMU event info function")
Signed-off-by: David E. Garcia Porras <david.garcia@aheadcomputing.com>
---
 include/sbi/sbi_ecall_interface.h | 2 ++
 lib/sbi/sbi_pmu.c                 | 3 +++
 2 files changed, 5 insertions(+)

Comments

Anup Patel Sept. 4, 2026, 7:56 a.m. UTC | #1
On Wed, Aug 19, 2026 at 2:30 AM David E. Garcia Porras
<david.garcia@aheadcomputing.com> wrote:
>
> As per section 11.14 of the SBI specification (Function: Get PMU Event
> Info, FID #8), Table 47, the event_idx word of an event info entry
> only uses BIT[0:19]; BIT[20:31] are reserved for the future purpose
> and must be zero. Table 48 further requires the SBI implementation to
> return SBI_ERR_INVALID_PARAM if any reserved bit in an event_idx word
> is set.
>
> sbi_pmu_event_get_info() does not check the reserved bits, so a
> malformed event_idx is silently passed on to pmu_event_validate()
> instead of failing the call. Add SBI_PMU_EVENT_IDX_MBZ_MASK covering
> the must-be-zero bits and return SBI_ERR_INVALID_PARAM when any of
> them are set.
>
> Fixes: e4345842168b ("lib: sbi_pmu: Implement SBI PMU event info function")
> Signed-off-by: David E. Garcia Porras <david.garcia@aheadcomputing.com>

LGTM.

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

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup


> ---
>  include/sbi/sbi_ecall_interface.h | 2 ++
>  lib/sbi/sbi_pmu.c                 | 3 +++
>  2 files changed, 5 insertions(+)
>
> diff --git a/include/sbi/sbi_ecall_interface.h b/include/sbi/sbi_ecall_interface.h
> index acb91a01..bfde25d0 100644
> --- a/include/sbi/sbi_ecall_interface.h
> +++ b/include/sbi/sbi_ecall_interface.h
> @@ -271,6 +271,8 @@ struct sbi_pmu_event_info {
>  #define SBI_PMU_EVENT_IDX_TYPE_OFFSET 16
>  #define SBI_PMU_EVENT_IDX_TYPE_MASK (0xF << SBI_PMU_EVENT_IDX_TYPE_OFFSET)
>  #define SBI_PMU_EVENT_IDX_CODE_MASK 0xFFFF
> +#define SBI_PMU_EVENT_IDX_MBZ_OFFSET 20
> +#define SBI_PMU_EVENT_IDX_MBZ_MASK (0xFFF << SBI_PMU_EVENT_IDX_MBZ_OFFSET)
>  #define SBI_PMU_EVENT_RAW_IDX 0x20000
>  #define SBI_PMU_EVENT_RAW_V2_IDX 0x30000
>
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index a0f6d2fa..4558876e 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -1098,6 +1098,9 @@ int sbi_pmu_event_get_info(unsigned long shmem_phys_lo, unsigned long shmem_phys
>         einfo = (struct sbi_pmu_event_info *)(shmem_phys_lo);
>         for (i = 0; i < num_events; i++) {
>                 event_idx = einfo[i].event_idx;
> +               /* Any must-be-zero event_idx bits set should return INVALID_PARAM per-spec */
> +               if (event_idx & SBI_PMU_EVENT_IDX_MBZ_MASK)
> +                       return SBI_ERR_INVALID_PARAM;
>                 event_type = pmu_event_validate(phs, event_idx, einfo[i].event_data);
>                 if (event_type < 0) {
>                         einfo[i].output = 0;
> --
> 2.43.0
>
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
diff mbox series

Patch

diff --git a/include/sbi/sbi_ecall_interface.h b/include/sbi/sbi_ecall_interface.h
index acb91a01..bfde25d0 100644
--- a/include/sbi/sbi_ecall_interface.h
+++ b/include/sbi/sbi_ecall_interface.h
@@ -271,6 +271,8 @@  struct sbi_pmu_event_info {
 #define SBI_PMU_EVENT_IDX_TYPE_OFFSET 16
 #define SBI_PMU_EVENT_IDX_TYPE_MASK (0xF << SBI_PMU_EVENT_IDX_TYPE_OFFSET)
 #define SBI_PMU_EVENT_IDX_CODE_MASK 0xFFFF
+#define SBI_PMU_EVENT_IDX_MBZ_OFFSET 20
+#define SBI_PMU_EVENT_IDX_MBZ_MASK (0xFFF << SBI_PMU_EVENT_IDX_MBZ_OFFSET)
 #define SBI_PMU_EVENT_RAW_IDX 0x20000
 #define SBI_PMU_EVENT_RAW_V2_IDX 0x30000
 
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index a0f6d2fa..4558876e 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -1098,6 +1098,9 @@  int sbi_pmu_event_get_info(unsigned long shmem_phys_lo, unsigned long shmem_phys
 	einfo = (struct sbi_pmu_event_info *)(shmem_phys_lo);
 	for (i = 0; i < num_events; i++) {
 		event_idx = einfo[i].event_idx;
+		/* Any must-be-zero event_idx bits set should return INVALID_PARAM per-spec */
+		if (event_idx & SBI_PMU_EVENT_IDX_MBZ_MASK)
+			return SBI_ERR_INVALID_PARAM;
 		event_type = pmu_event_validate(phs, event_idx, einfo[i].event_data);
 		if (event_type < 0) {
 			einfo[i].output = 0;