On 3/10/2026 1:55 PM, 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;
Nit: Also need to align with previous line ?
Otherwise,
Reviewed-by: Nutty Liu <[email protected]>
Thanks,
Nutty
+
+ 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);
+ }
}
}
