From c5077d497d0f7c7c9a74218ef2c448bc1a6d8a31 Mon Sep 17 00:00:00 2001 From: "David E. Garcia Porras" Date: Tue, 18 Aug 2026 15:00:23 -0600 Subject: [PATCH] lib: sbi_pmu: Fix counter and event info error codes as per SBI v3.0 spec Align the PMU extension implementation with the error codes required by the SBI v3.0 specification, chapter 11: - sbi_pmu_counter_start and sbi_pmu_counter_stop (secs 11.9-11.10, tables 39-42): the start_flags/stop_flags bits 2:(XLEN-1) are reserved and must be zero, so return SBI_ERR_INVALID_PARAM when any reserved flag bit is set. Introduce SBI_PMU_START_FLAGS_MASK and SBI_PMU_STOP_FLAGS_MASK for the valid bits of each function. - sbi_pmu_counter_start and sbi_pmu_counter_stop (tables 40 and 42): return SBI_ERR_ALREADY_STARTED / SBI_ERR_ALREADY_STOPPED when the set of counters includes a counter which is already started or stopped, instead of ignoring the error returned for each counter. - sbi_pmu_event_get_info (sec 11.14, table 47): the output word must indicate whether the event is supported, but firmware events were only matched against the hardware event map and were always reported as unsupported. Report a validated firmware event as supported. Signed-off-by: David E. Garcia Porras Reviewed-by: Anup Patel Link: https://lore.kernel.org/r/20260818210023.466462-4-david.garcia@aheadcomputing.com Signed-off-by: Anup Patel --- include/sbi/sbi_ecall_interface.h | 12 ++++++++++++ lib/sbi/sbi_pmu.c | 15 +++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/include/sbi/sbi_ecall_interface.h b/include/sbi/sbi_ecall_interface.h index bfde25d0..fd4e77ca 100644 --- a/include/sbi/sbi_ecall_interface.h +++ b/include/sbi/sbi_ecall_interface.h @@ -306,10 +306,22 @@ struct sbi_pmu_event_info { /* Flags defined for counter start function */ #define SBI_PMU_START_FLAG_SET_INIT_VALUE (1 << 0) #define SBI_PMU_START_FLAG_INIT_FROM_SNAPSHOT (1 << 1) +/* Start flags valid mask */ +#define SBI_PMU_START_FLAGS_MASK \ + ( \ + SBI_PMU_START_FLAG_SET_INIT_VALUE | \ + SBI_PMU_START_FLAG_INIT_FROM_SNAPSHOT \ + ) /* Flags defined for counter stop function */ #define SBI_PMU_STOP_FLAG_RESET (1 << 0) #define SBI_PMU_STOP_FLAG_TAKE_SNAPSHOT (1 << 1) +/* Stop flags valid mask */ +#define SBI_PMU_STOP_FLAGS_MASK \ + ( \ + SBI_PMU_STOP_FLAG_RESET | \ + SBI_PMU_STOP_FLAG_TAKE_SNAPSHOT \ + ) /* SBI function IDs for DBCN extension */ #define SBI_EXT_DBCN_CONSOLE_WRITE 0x0 diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c index 676de9aa..49d52066 100644 --- a/lib/sbi/sbi_pmu.c +++ b/lib/sbi/sbi_pmu.c @@ -574,6 +574,9 @@ int sbi_pmu_ctr_start(unsigned long cbase, unsigned long cmask, if (!pmu_ctr_idx_validate(cbase, cmask)) return ret; + if (flags & ~SBI_PMU_START_FLAGS_MASK) + return SBI_ERR_INVALID_PARAM; + if (flags & SBI_PMU_STOP_FLAG_TAKE_SNAPSHOT) return SBI_ENO_SHMEM; @@ -592,6 +595,8 @@ int sbi_pmu_ctr_start(unsigned long cbase, unsigned long cmask, : 0x0; ret = pmu_ctr_start_fw(phs, cidx, event_code, edata, ival, bUpdate); + if (ret) + return ret; } else { if (cidx >= 3) { struct sbi_pmu_hw_event_config *ev_cfg = @@ -605,6 +610,8 @@ int sbi_pmu_ctr_start(unsigned long cbase, unsigned long cmask, return ret; } ret = pmu_ctr_start_hw(cidx, ival, bUpdate); + if (ret) + return ret; } } @@ -693,6 +700,9 @@ int sbi_pmu_ctr_stop(unsigned long cbase, unsigned long cmask, if (!pmu_ctr_idx_validate(cbase, cmask)) return ret; + if (flag & ~SBI_PMU_STOP_FLAGS_MASK) + return SBI_ERR_INVALID_PARAM; + if (flag & SBI_PMU_STOP_FLAG_TAKE_SNAPSHOT) return SBI_ENO_SHMEM; @@ -708,6 +718,9 @@ int sbi_pmu_ctr_stop(unsigned long cbase, unsigned long cmask, else ret = pmu_ctr_stop_hw(cidx); + if(ret) + return ret; + if (cidx > (CSR_INSTRET - CSR_CYCLE) && flag & SBI_PMU_STOP_FLAG_RESET) { phs->active_events[cidx] = SBI_PMU_EVENT_IDX_INVALID; pmu_reset_hw_mhpmevent(cidx); @@ -1104,6 +1117,8 @@ int sbi_pmu_event_get_info(unsigned long shmem_phys_lo, unsigned long shmem_phys event_type = pmu_event_validate(phs, event_idx, einfo[i].event_data); if (event_type < 0) { einfo[i].output = 0; + } else if (event_type == SBI_PMU_EVENT_TYPE_FW) { + einfo[i].output = 1; } else { for (j = 0; j < num_hw_events; j++) { temp = &hw_event_map[j];