lib: sbi_pmu: Add FW counter index validation when reading high bits on RV64
PATCH v2 1/1 lib: sbi_pmu: Add FW counter index validation when reading high bits on RV64
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.
---
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(-)
include/sbi/sbi_pmu.h +1 -1
@@ -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);
lib/sbi/sbi_ecall_pmu.c +2 -2
@@ -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 ;
lib/sbi/sbi_pmu.c +9 -1
@@ -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 ;
Signature · 7 lines
2.43.0 -- opensbi mailing list [email protected] http://lists.infradead.org/mailman/listinfo/opensbi
Quoted 10 lines · 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.valueto 0 without validating the counter index. The SBI specification requires us to setsbiret.errortoSBI_ERR_INVALID_PARAMif 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 <[email protected]>
LGTM.
Applied this patch to the riscv/opensbi repo.
Thanks,
Anup
Quoted 80 lines
---
v1 -> v2:
- Implemented Radim's suggestion to refactorsbi_pmu_ctr_fw_read()instead of adding a new function.
- Updated the type ofcidxin 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(-)include/sbi/sbi_pmu.h +1
-1@@ -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);lib/sbi/sbi_ecall_pmu.c +2
-2@@ -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 ;lib/sbi/sbi_pmu.c +9
-2@@ -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
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi
Signature · 3 lines
opensbi mailing list [email protected] http://lists.infradead.org/mailman/listinfo/opensbi