| Message ID | 20230413140219.189916-2-alexghiti@rivosinc.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,1/2] lib: sbi: Do not clear active_events for cycle/instret when stopping | expand |
On Thu, Apr 13, 2023 at 7:33 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote: > > As per the SBI specification, we should "unconditionally select the first > counter from the set of counters specified by the counter_idx_base and > counter_idx_mask", so implement this behaviour. > > Suggested-by: Atish Patra <atishp@atishpatra.org> > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > --- > lib/sbi/sbi_pmu.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c > index 2176cc7..f5dbe6d 100644 > --- a/lib/sbi/sbi_pmu.c > +++ b/lib/sbi/sbi_pmu.c > @@ -735,10 +735,15 @@ int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask, > /* The caller wants to skip the match because it already knows the > * counter idx for the given event. Verify that the counter idx > * is still valid. > + * As per the specification, we should "unconditionally select > + * the first counter from the set of counters specified by the > + * counter_idx_base and counter_idx_mask". > */ > - if (active_events[hartid][cidx_base] == SBI_PMU_EVENT_IDX_INVALID) > + unsigned long cidx_first = cidx_base + sbi_ffs(cidx_mask); > + > + if (active_events[hartid][cidx_first] == SBI_PMU_EVENT_IDX_INVALID) > return SBI_EINVAL; > - ctr_idx = cidx_base; > + ctr_idx = cidx_first; > goto skip_match; > } > > -- > 2.37.2 > Reviewed-by: Atish Patra <atishp@rivosinc.com>
On Thu, Apr 13, 2023 at 7:33 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote: > > As per the SBI specification, we should "unconditionally select the first > counter from the set of counters specified by the counter_idx_base and > counter_idx_mask", so implement this behaviour. > > Suggested-by: Atish Patra <atishp@atishpatra.org> > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> No issues with this patch but as a separate patch we might still want to check whether the specified counter actually supports the specified event. Applied this patch to the riscv/opesbi repo. Thanks, Anup > --- > lib/sbi/sbi_pmu.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c > index 2176cc7..f5dbe6d 100644 > --- a/lib/sbi/sbi_pmu.c > +++ b/lib/sbi/sbi_pmu.c > @@ -735,10 +735,15 @@ int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask, > /* The caller wants to skip the match because it already knows the > * counter idx for the given event. Verify that the counter idx > * is still valid. > + * As per the specification, we should "unconditionally select > + * the first counter from the set of counters specified by the > + * counter_idx_base and counter_idx_mask". > */ > - if (active_events[hartid][cidx_base] == SBI_PMU_EVENT_IDX_INVALID) > + unsigned long cidx_first = cidx_base + sbi_ffs(cidx_mask); > + > + if (active_events[hartid][cidx_first] == SBI_PMU_EVENT_IDX_INVALID) > return SBI_EINVAL; > - ctr_idx = cidx_base; > + ctr_idx = cidx_first; > goto skip_match; > } > > -- > 2.37.2 > > > -- > opensbi mailing list > opensbi@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/opensbi
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c index 2176cc7..f5dbe6d 100644 --- a/lib/sbi/sbi_pmu.c +++ b/lib/sbi/sbi_pmu.c @@ -735,10 +735,15 @@ int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask, /* The caller wants to skip the match because it already knows the * counter idx for the given event. Verify that the counter idx * is still valid. + * As per the specification, we should "unconditionally select + * the first counter from the set of counters specified by the + * counter_idx_base and counter_idx_mask". */ - if (active_events[hartid][cidx_base] == SBI_PMU_EVENT_IDX_INVALID) + unsigned long cidx_first = cidx_base + sbi_ffs(cidx_mask); + + if (active_events[hartid][cidx_first] == SBI_PMU_EVENT_IDX_INVALID) return SBI_EINVAL; - ctr_idx = cidx_base; + ctr_idx = cidx_first; goto skip_match; }
As per the SBI specification, we should "unconditionally select the first counter from the set of counters specified by the counter_idx_base and counter_idx_mask", so implement this behaviour. Suggested-by: Atish Patra <atishp@atishpatra.org> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> --- lib/sbi/sbi_pmu.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)