1. Patchew
  2. linux
  3. [PATCH v2 0/2] RISC-V: KVM: Fix array out-of-bounds in firmware counter reads
  4. View patch

[PATCH v2 2/2] RISC-V: KVM: selftests: Fix firmware counter read in sbi_pmu_test

Jiakai Xu posted 2 patches 1 month ago
There is a newer version of this series
[PATCH v2 2/2] RISC-V: KVM: selftests: Fix firmware counter read in sbi_pmu_test
Posted by Jiakai Xu 1 month ago 
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 <xujiakai2025@iscas.ac.cn>
Signed-off-by: Jiakai Xu <jiakaiPeanut@gmail.com>
---
 .../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);
 		}
 	}
 
-- 
2.34.1
Re: [PATCH v2 2/2] RISC-V: KVM: selftests: Fix firmware counter read in sbi_pmu_test
Posted by Andrew Jones 1 month ago 
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 <xujiakai2025@iscas.ac.cn>
> Signed-off-by: Jiakai Xu <jiakaiPeanut@gmail.com>
> ---
>  .../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
Re: Re: [PATCH v2 2/2] RISC-V: KVM: selftests: Fix firmware counter read in sbi_pmu_test
Posted by Jiakai Xu 1 month ago 
Thanks for the review! I have updated the patch accordingly and verified
the v3 version passes the test. Will send it out shortly.

Best regards,
Jiakai Xu


> -----Original Messages-----
> From: "Andrew Jones" <andrew.jones@oss.qualcomm.com>
> Sent Time: 2026-03-10 05:08:22 (Tuesday)
> To: "Jiakai Xu" <xujiakai2025@iscas.ac.cn>
> Cc: linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, "Shuah Khan" <shuah@kernel.org>, "Paolo Bonzini" <pbonzini@redhat.com>, "Andrew Jones" <ajones@ventanamicro.com>, "Alexandre Ghiti" <alex@ghiti.fr>, "Albert Ou" <aou@eecs.berkeley.edu>, "Palmer Dabbelt" <palmer@dabbelt.com>, "Paul Walmsley" <pjw@kernel.org>, "Atish Patra" <atish.patra@linux.dev>, "Anup Patel" <anup@brainfault.org>, "Jiakai Xu" <jiakaiPeanut@gmail.com>
> Subject: Re: [PATCH v2 2/2] RISC-V: KVM: selftests: Fix firmware counter read in sbi_pmu_test
> 
> 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 <xujiakai2025@iscas.ac.cn>
> > Signed-off-by: Jiakai Xu <jiakaiPeanut@gmail.com>
> > ---
> >  .../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

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.