On Tue, Mar 10, 2026 at 05:55:57AM +0000, Jiakai Xu wrote: > The current sbi_pmu_test attempts to read firmware counters without > configuring them first with SBI_EXT_PMU_COUNTER_CFG_MATCH. > > Previously this did not fail because KVM incorrectly allowed the read > and accessed fw_event[] with an out-of-bounds index when the counter > was unconfigured. After fixing that bug, the read now correctly returns > SBI_ERR_INVALID_PARAM, causing the selftest to fail. > > Update the test to configure a firmware event before reading the > counter. Also add a negative test to ensure that attempting to read an > unconfigured firmware counter fails gracefully. > > Signed-off-by: Jiakai Xu <[email protected]> > Signed-off-by: Jiakai Xu <[email protected]> > --- > V2 -> V3: > - Removed unnecessary BIT(ret.value) & counter_mask_available check. > - Asserted ret.value == i after successful CFG_MATCH. > - Fixed eidx construction in SBI_EXT_PMU_COUNTER_CFG_MATCH. > --- > .../testing/selftests/kvm/include/riscv/sbi.h | 37 +++++++++++++++++++ > .../selftests/kvm/riscv/sbi_pmu_test.c | 20 +++++++++- > 2 files changed, 56 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/kvm/include/riscv/sbi.h > b/tools/testing/selftests/kvm/include/riscv/sbi.h > index 046b432ae896..16f1815ac48f 100644 > --- a/tools/testing/selftests/kvm/include/riscv/sbi.h > +++ b/tools/testing/selftests/kvm/include/riscv/sbi.h > @@ -97,6 +97,43 @@ enum sbi_pmu_hw_generic_events_t { > SBI_PMU_HW_GENERAL_MAX, > }; > > +enum sbi_pmu_fw_generic_events_t { > + SBI_PMU_FW_MISALIGNED_LOAD = 0, > + SBI_PMU_FW_MISALIGNED_STORE = 1, > + SBI_PMU_FW_ACCESS_LOAD = 2, > + SBI_PMU_FW_ACCESS_STORE = 3, > + SBI_PMU_FW_ILLEGAL_INSN = 4, > + SBI_PMU_FW_SET_TIMER = 5, > + SBI_PMU_FW_IPI_SENT = 6, > + SBI_PMU_FW_IPI_RCVD = 7, > + SBI_PMU_FW_FENCE_I_SENT = 8, > + SBI_PMU_FW_FENCE_I_RCVD = 9, > + SBI_PMU_FW_SFENCE_VMA_SENT = 10, > + SBI_PMU_FW_SFENCE_VMA_RCVD = 11, > + SBI_PMU_FW_SFENCE_VMA_ASID_SENT = 12, > + SBI_PMU_FW_SFENCE_VMA_ASID_RCVD = 13, > + > + SBI_PMU_FW_HFENCE_GVMA_SENT = 14, > + SBI_PMU_FW_HFENCE_GVMA_RCVD = 15, > + SBI_PMU_FW_HFENCE_GVMA_VMID_SENT = 16, > + SBI_PMU_FW_HFENCE_GVMA_VMID_RCVD = 17, > + > + SBI_PMU_FW_HFENCE_VVMA_SENT = 18, > + SBI_PMU_FW_HFENCE_VVMA_RCVD = 19, > + SBI_PMU_FW_HFENCE_VVMA_ASID_SENT = 20, > + SBI_PMU_FW_HFENCE_VVMA_ASID_RCVD = 21, > + SBI_PMU_FW_MAX, > +}; > + > +/* SBI PMU event types */ > +enum sbi_pmu_event_type { > + SBI_PMU_EVENT_TYPE_HW = 0x0, > + SBI_PMU_EVENT_TYPE_CACHE = 0x1, > + SBI_PMU_EVENT_TYPE_RAW = 0x2, > + SBI_PMU_EVENT_TYPE_RAW_V2 = 0x3, > + SBI_PMU_EVENT_TYPE_FW = 0xf, > +}; > + > /* SBI PMU counter types */ > enum sbi_pmu_ctr_type { > SBI_PMU_CTR_TYPE_HW = 0x0, > diff --git a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c > b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c > index 924a335d2262..9404577e4ad5 100644 > --- a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c > +++ b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c > @@ -436,6 +436,7 @@ static void test_pmu_basic_sanity(void) > struct sbiret ret; > int num_counters = 0, i; > union sbi_pmu_ctr_info ctrinfo; > + unsigned long fw_eidx; > > probe = guest_sbi_probe_extension(SBI_EXT_PMU, &out_val); > GUEST_ASSERT(probe && out_val == 1); > @@ -461,7 +462,24 @@ static void test_pmu_basic_sanity(void) > pmu_csr_read_num(ctrinfo.csr); > GUEST_ASSERT(illegal_handler_invoked); > } else if (ctrinfo.type == SBI_PMU_CTR_TYPE_FW) { > - read_fw_counter(i, ctrinfo); > + /* Read without configure should fail */ > + ret = sbi_ecall(SBI_EXT_PMU, > SBI_EXT_PMU_COUNTER_FW_READ, > + i, 0, 0, 0, 0, 0); > + GUEST_ASSERT(ret.error == SBI_ERR_INVALID_PARAM); > + > + /* > + * Try to configure with a common firmware event. > + * If configuration succeeds, verify we can read it. > + */ > + fw_eidx = ((unsigned long)SBI_PMU_EVENT_TYPE_FW << 16) | > + SBI_PMU_FW_ACCESS_LOAD; > + > + ret = sbi_ecall(SBI_EXT_PMU, > SBI_EXT_PMU_COUNTER_CFG_MATCH, > + i, 1, 0, fw_eidx, 0, 0); > + if (ret.error == 0) { > + GUEST_ASSERT(ret.value == i); > + read_fw_counter(i, ctrinfo); > + } > } > } > > -- > 2.34.1 >
Reviewed-by: Andrew Jones <[email protected]>
