On Sun, Mar 08, 2026 at 02:38:03AM +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]>
> ---
> .../testing/selftests/kvm/include/riscv/sbi.h | 28 +++++++++++++++++++
> .../selftests/kvm/riscv/sbi_pmu_test.c | 16 ++++++++++-
> 2 files changed, 43 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..8c172422f386 100644
> --- a/tools/testing/selftests/kvm/include/riscv/sbi.h
> +++ b/tools/testing/selftests/kvm/include/riscv/sbi.h
> @@ -97,6 +97,34 @@ 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 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..9b7e28bec435 100644
> --- a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c
> +++ b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c
> @@ -461,7 +461,21 @@ 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.
> + */
> + ret = sbi_ecall(SBI_EXT_PMU,
> SBI_EXT_PMU_COUNTER_CFG_MATCH,
> + i, 1, 0, SBI_PMU_FW_ACCESS_LOAD, 0, 0);
> + if (ret.error == 0 &&
> + ret.value < RISCV_MAX_PMU_COUNTERS &&
> + (BIT(ret.value) & counter_mask_available))
> + read_fw_counter(i, ctrinfo);
We don't need the 'BIT(ret.value) & counter_mask_available' test. Instead
we should assert that ret.value == i.
if (ret.error == 0) {
GUEST_ASSERT(ret.value == i);
read_fw_counter(i, ctrinfo);
}
Thanks,
drew
