diff mbox series

[v1,2/2] lib: sbi_pmu: Fix the counter info function

Message ID 20231206010844.1739845-3-atishp@rivosinc.com
State Superseded
Headers show
Series SBI PMU improvements | expand

Commit Message

Atish Kumar Patra Dec. 6, 2023, 1:08 a.m. UTC
The counter info should only return valid hardware counters for the ones
set in the counter mask. Otherwise, it will report incorrect number of
hardware counters to the supervisor if the platform has discontiguous
counters.

Fixes: c744ed77b18c ("lib: sbi_pmu: Enable noncontigous hpm event and counters")

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 lib/sbi/sbi_pmu.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Xiang W Dec. 6, 2023, 7:46 a.m. UTC | #1
在 2023-12-05星期二的 17:08 -0800,Atish Patra写道:
> The counter info should only return valid hardware counters for the ones
> set in the counter mask. Otherwise, it will report incorrect number of
> hardware counters to the supervisor if the platform has discontiguous
> counters.
> 
> Fixes: c744ed77b18c ("lib: sbi_pmu: Enable noncontigous hpm event and counters")
> 
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
>  lib/sbi/sbi_pmu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index 185068bb1f43..ac5ae0af7da4 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -885,6 +885,7 @@ int sbi_pmu_ctr_get_info(uint32_t cidx, unsigned long *ctr_info)
>  	int width;
>  	union sbi_pmu_ctr_info cinfo = {0};
>  	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> +	unsigned long counter_mask = (unsigned long)sbi_hart_mhpm_mask(scratch) | 0x5;
>  
>  	/* Sanity check. Counter1 is not mapped at all */
>  	if (cidx >= total_ctrs || cidx == 1)
Because counter_mask does not contain counter1, the cidx==1 check can be removed here.

Regards,
Xiang W

> @@ -892,6 +893,8 @@ int sbi_pmu_ctr_get_info(uint32_t cidx, unsigned long *ctr_info)
>  
>  	/* We have 31 HW counters with 31 being the last index(MHPMCOUNTER31) */
>  	if (cidx < num_hw_ctrs) {
> +		if (!(__test_bit(cidx, &counter_mask)))
> +			return SBI_EINVAL;
>  		cinfo.type = SBI_PMU_CTR_TYPE_HW;
>  		cinfo.csr = CSR_CYCLE + cidx;
>  		/* mcycle & minstret are always 64 bit */
> -- 
> 2.34.1
> 
>
Atish Kumar Patra Dec. 6, 2023, 8:54 a.m. UTC | #2
On Tue, Dec 5, 2023 at 11:46 PM Xiang W <wxjstz@126.com> wrote:
>
> 在 2023-12-05星期二的 17:08 -0800,Atish Patra写道:
> > The counter info should only return valid hardware counters for the ones
> > set in the counter mask. Otherwise, it will report incorrect number of
> > hardware counters to the supervisor if the platform has discontiguous
> > counters.
> >
> > Fixes: c744ed77b18c ("lib: sbi_pmu: Enable noncontigous hpm event and counters")
> >
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > ---
> >  lib/sbi/sbi_pmu.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> > index 185068bb1f43..ac5ae0af7da4 100644
> > --- a/lib/sbi/sbi_pmu.c
> > +++ b/lib/sbi/sbi_pmu.c
> > @@ -885,6 +885,7 @@ int sbi_pmu_ctr_get_info(uint32_t cidx, unsigned long *ctr_info)
> >       int width;
> >       union sbi_pmu_ctr_info cinfo = {0};
> >       struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> > +     unsigned long counter_mask = (unsigned long)sbi_hart_mhpm_mask(scratch) | 0x5;
> >
> >       /* Sanity check. Counter1 is not mapped at all */
> >       if (cidx >= total_ctrs || cidx == 1)
> Because counter_mask does not contain counter1, the cidx==1 check can be removed here.
>

Yes. Thanks for pointing it out. Will send it a v2.

> Regards,
> Xiang W
>
> > @@ -892,6 +893,8 @@ int sbi_pmu_ctr_get_info(uint32_t cidx, unsigned long *ctr_info)
> >
> >       /* We have 31 HW counters with 31 being the last index(MHPMCOUNTER31) */
> >       if (cidx < num_hw_ctrs) {
> > +             if (!(__test_bit(cidx, &counter_mask)))
> > +                     return SBI_EINVAL;
> >               cinfo.type = SBI_PMU_CTR_TYPE_HW;
> >               cinfo.csr = CSR_CYCLE + cidx;
> >               /* mcycle & minstret are always 64 bit */
> > --
> > 2.34.1
> >
> >
>
diff mbox series

Patch

diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 185068bb1f43..ac5ae0af7da4 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -885,6 +885,7 @@  int sbi_pmu_ctr_get_info(uint32_t cidx, unsigned long *ctr_info)
 	int width;
 	union sbi_pmu_ctr_info cinfo = {0};
 	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
+	unsigned long counter_mask = (unsigned long)sbi_hart_mhpm_mask(scratch) | 0x5;
 
 	/* Sanity check. Counter1 is not mapped at all */
 	if (cidx >= total_ctrs || cidx == 1)
@@ -892,6 +893,8 @@  int sbi_pmu_ctr_get_info(uint32_t cidx, unsigned long *ctr_info)
 
 	/* We have 31 HW counters with 31 being the last index(MHPMCOUNTER31) */
 	if (cidx < num_hw_ctrs) {
+		if (!(__test_bit(cidx, &counter_mask)))
+			return SBI_EINVAL;
 		cinfo.type = SBI_PMU_CTR_TYPE_HW;
 		cinfo.csr = CSR_CYCLE + cidx;
 		/* mcycle & minstret are always 64 bit */