* [PATCH v3 1/3] drivers/perf: riscv: Do not update the event data if uptodate
2024-06-26 7:23 [PATCH v3 0/3] Assorted fixes in RISC-V PMU driver Atish Patra
@ 2024-06-26 7:23 ` Atish Patra
2024-06-26 7:23 ` [PATCH v3 2/3] drivers/perf: riscv: Reset the counter to hpmevent mapping while starting cpus Atish Patra
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Atish Patra @ 2024-06-26 7:23 UTC (permalink / raw)
To: linux-riscv, kvm-riscv
Cc: Atish Patra, Anup Patel, Will Deacon, Mark Rutland, Paul Walmsley,
Palmer Dabbelt, Andrew Jones, Conor Dooley, Samuel Holland,
Palmer Dabbelt, Alexandre Ghiti, linux-arm-kernel, linux-kernel,
kvm, Atish Patra, garthlei
In case of an counter overflow, the event data may get corrupted
if called from an external overflow handler. This happens because
we can't update the counter without starting it when SBI PMU
extension is in use. However, the prev_count has been already
updated at the first pass while the counter value is still the
old one.
The solution is simple where we don't need to update it again
if it is already updated which can be detected using hwc state.
Fixes: a8625217a054 ("drivers/perf: riscv: Implement SBI PMU snapshot function")
Reported-by: garthlei@pku.edu.cn
Closes:https://lore.kernel.org/all/CC51D53B-846C-4D81-86FC-FBF969D0A0D6@pku.edu.cn/
Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
drivers/perf/riscv_pmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
index 78c490e0505a..0a02e85a8951 100644
--- a/drivers/perf/riscv_pmu.c
+++ b/drivers/perf/riscv_pmu.c
@@ -167,7 +167,7 @@ u64 riscv_pmu_event_update(struct perf_event *event)
unsigned long cmask;
u64 oldval, delta;
- if (!rvpmu->ctr_read)
+ if (!rvpmu->ctr_read || (hwc->state & PERF_HES_UPTODATE))
return 0;
cmask = riscv_pmu_ctr_get_width_mask(event);
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 2/3] drivers/perf: riscv: Reset the counter to hpmevent mapping while starting cpus
2024-06-26 7:23 [PATCH v3 0/3] Assorted fixes in RISC-V PMU driver Atish Patra
2024-06-26 7:23 ` [PATCH v3 1/3] drivers/perf: riscv: Do not update the event data if uptodate Atish Patra
@ 2024-06-26 7:23 ` Atish Patra
2024-06-26 13:24 ` Samuel Holland
2024-06-26 7:23 ` [PATCH v3 3/3] perf: RISC-V: Check standard event availability Atish Patra
2024-06-26 13:31 ` [PATCH v3 0/3] Assorted fixes in RISC-V PMU driver Samuel Holland
3 siblings, 1 reply; 12+ messages in thread
From: Atish Patra @ 2024-06-26 7:23 UTC (permalink / raw)
To: linux-riscv, kvm-riscv
Cc: Atish Patra, Anup Patel, Will Deacon, Mark Rutland, Paul Walmsley,
Palmer Dabbelt, Andrew Jones, Conor Dooley, Samuel Holland,
Palmer Dabbelt, Alexandre Ghiti, linux-arm-kernel, linux-kernel,
kvm, Atish Patra
From: Samuel Holland <samuel.holland@sifive.com>
Currently, we stop all the counters while a new cpu is brought online.
However, the hpmevent to counter mappings are not reset. The firmware may
have some stale encoding in their mapping structure which may lead to
undesirable results. We have not encountered such scenario though.
Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
drivers/perf/riscv_pmu_sbi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
index a2e4005e1fd0..94bc369a3454 100644
--- a/drivers/perf/riscv_pmu_sbi.c
+++ b/drivers/perf/riscv_pmu_sbi.c
@@ -762,7 +762,7 @@ static inline void pmu_sbi_stop_all(struct riscv_pmu *pmu)
* which may include counters that are not enabled yet.
*/
sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP,
- 0, pmu->cmask, 0, 0, 0, 0);
+ 0, pmu->cmask, SBI_PMU_STOP_FLAG_RESET, 0, 0, 0);
}
static inline void pmu_sbi_stop_hw_ctrs(struct riscv_pmu *pmu)
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/3] drivers/perf: riscv: Reset the counter to hpmevent mapping while starting cpus
2024-06-26 7:23 ` [PATCH v3 2/3] drivers/perf: riscv: Reset the counter to hpmevent mapping while starting cpus Atish Patra
@ 2024-06-26 13:24 ` Samuel Holland
2024-06-26 16:18 ` Atish Kumar Patra
0 siblings, 1 reply; 12+ messages in thread
From: Samuel Holland @ 2024-06-26 13:24 UTC (permalink / raw)
To: Atish Patra, linux-riscv, kvm-riscv
Cc: Atish Patra, Anup Patel, Will Deacon, Mark Rutland, Paul Walmsley,
Palmer Dabbelt, Andrew Jones, Conor Dooley, Palmer Dabbelt,
Alexandre Ghiti, linux-arm-kernel, linux-kernel, kvm
On 2024-06-26 2:23 AM, Atish Patra wrote:
> From: Samuel Holland <samuel.holland@sifive.com>
>
> Currently, we stop all the counters while a new cpu is brought online.
> However, the hpmevent to counter mappings are not reset. The firmware may
> have some stale encoding in their mapping structure which may lead to
> undesirable results. We have not encountered such scenario though.
>
This needs:
Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
otherwise your commit message looks fine to me.
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
> drivers/perf/riscv_pmu_sbi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index a2e4005e1fd0..94bc369a3454 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -762,7 +762,7 @@ static inline void pmu_sbi_stop_all(struct riscv_pmu *pmu)
> * which may include counters that are not enabled yet.
> */
> sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP,
> - 0, pmu->cmask, 0, 0, 0, 0);
> + 0, pmu->cmask, SBI_PMU_STOP_FLAG_RESET, 0, 0, 0);
> }
>
> static inline void pmu_sbi_stop_hw_ctrs(struct riscv_pmu *pmu)
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/3] drivers/perf: riscv: Reset the counter to hpmevent mapping while starting cpus
2024-06-26 13:24 ` Samuel Holland
@ 2024-06-26 16:18 ` Atish Kumar Patra
2024-06-26 16:37 ` Conor Dooley
0 siblings, 1 reply; 12+ messages in thread
From: Atish Kumar Patra @ 2024-06-26 16:18 UTC (permalink / raw)
To: Samuel Holland
Cc: linux-riscv, kvm-riscv, Atish Patra, Anup Patel, Will Deacon,
Mark Rutland, Paul Walmsley, Palmer Dabbelt, Andrew Jones,
Conor Dooley, Palmer Dabbelt, Alexandre Ghiti, linux-arm-kernel,
linux-kernel, kvm
On Wed, Jun 26, 2024 at 6:24 AM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> On 2024-06-26 2:23 AM, Atish Patra wrote:
> > From: Samuel Holland <samuel.holland@sifive.com>
> >
> > Currently, we stop all the counters while a new cpu is brought online.
> > However, the hpmevent to counter mappings are not reset. The firmware may
> > have some stale encoding in their mapping structure which may lead to
> > undesirable results. We have not encountered such scenario though.
> >
>
> This needs:
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
>
Oops. Sorry I missed that.
@Alexandre Ghiti @Palmer Dabbelt : Can you add that while picking up
the patch or should I respin a v4 ?
> otherwise your commit message looks fine to me.
>
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > ---
> > drivers/perf/riscv_pmu_sbi.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> > index a2e4005e1fd0..94bc369a3454 100644
> > --- a/drivers/perf/riscv_pmu_sbi.c
> > +++ b/drivers/perf/riscv_pmu_sbi.c
> > @@ -762,7 +762,7 @@ static inline void pmu_sbi_stop_all(struct riscv_pmu *pmu)
> > * which may include counters that are not enabled yet.
> > */
> > sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP,
> > - 0, pmu->cmask, 0, 0, 0, 0);
> > + 0, pmu->cmask, SBI_PMU_STOP_FLAG_RESET, 0, 0, 0);
> > }
> >
> > static inline void pmu_sbi_stop_hw_ctrs(struct riscv_pmu *pmu)
> >
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/3] drivers/perf: riscv: Reset the counter to hpmevent mapping while starting cpus
2024-06-26 16:18 ` Atish Kumar Patra
@ 2024-06-26 16:37 ` Conor Dooley
2024-06-26 16:39 ` Conor Dooley
0 siblings, 1 reply; 12+ messages in thread
From: Conor Dooley @ 2024-06-26 16:37 UTC (permalink / raw)
To: Atish Kumar Patra
Cc: Samuel Holland, linux-riscv, kvm-riscv, Atish Patra, Anup Patel,
Will Deacon, Mark Rutland, Paul Walmsley, Palmer Dabbelt,
Andrew Jones, Conor Dooley, Palmer Dabbelt, Alexandre Ghiti,
linux-arm-kernel, linux-kernel, kvm
[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]
On Wed, Jun 26, 2024 at 09:18:46AM -0700, Atish Kumar Patra wrote:
> On Wed, Jun 26, 2024 at 6:24 AM Samuel Holland
> <samuel.holland@sifive.com> wrote:
> >
> > On 2024-06-26 2:23 AM, Atish Patra wrote:
> > > From: Samuel Holland <samuel.holland@sifive.com>
> > >
> > > Currently, we stop all the counters while a new cpu is brought online.
> > > However, the hpmevent to counter mappings are not reset. The firmware may
> > > have some stale encoding in their mapping structure which may lead to
> > > undesirable results. We have not encountered such scenario though.
> > >
> >
> > This needs:
> >
> > Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> >
>
> Oops. Sorry I missed that.
>
> @Alexandre Ghiti
What's Alex going to be able to do?
> @Palmer Dabbelt : Can you add that while picking up
> the patch or should I respin a v4 ?
b4 should pick the signoff up though. "perf: RISC-V: Check standard
event availability" seems to be missing your signoff though...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/3] drivers/perf: riscv: Reset the counter to hpmevent mapping while starting cpus
2024-06-26 16:37 ` Conor Dooley
@ 2024-06-26 16:39 ` Conor Dooley
2024-06-26 20:40 ` Atish Kumar Patra
0 siblings, 1 reply; 12+ messages in thread
From: Conor Dooley @ 2024-06-26 16:39 UTC (permalink / raw)
To: Atish Kumar Patra
Cc: Samuel Holland, linux-riscv, kvm-riscv, Atish Patra, Anup Patel,
Will Deacon, Mark Rutland, Paul Walmsley, Palmer Dabbelt,
Andrew Jones, Conor Dooley, Palmer Dabbelt, Alexandre Ghiti,
linux-arm-kernel, linux-kernel, kvm
[-- Attachment #1: Type: text/plain, Size: 1311 bytes --]
On Wed, Jun 26, 2024 at 05:37:07PM +0100, Conor Dooley wrote:
> On Wed, Jun 26, 2024 at 09:18:46AM -0700, Atish Kumar Patra wrote:
> > On Wed, Jun 26, 2024 at 6:24 AM Samuel Holland
> > <samuel.holland@sifive.com> wrote:
> > >
> > > On 2024-06-26 2:23 AM, Atish Patra wrote:
> > > > From: Samuel Holland <samuel.holland@sifive.com>
> > > >
> > > > Currently, we stop all the counters while a new cpu is brought online.
> > > > However, the hpmevent to counter mappings are not reset. The firmware may
> > > > have some stale encoding in their mapping structure which may lead to
> > > > undesirable results. We have not encountered such scenario though.
> > > >
> > >
> > > This needs:
> > >
> > > Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> > >
> >
> > Oops. Sorry I missed that.
> >
> > @Alexandre Ghiti
>
> What's Alex going to be able to do?
>
> > @Palmer Dabbelt : Can you add that while picking up
> > the patch or should I respin a v4 ?
>
> b4 should pick the signoff up though. "perf: RISC-V: Check standard
> event availability" seems to be missing your signoff though...
Huh, this doesn't really make sense. I meant:
b4 should pick the signoff up, though "perf: RISC-V: Check standard
event availability" seems to be missing your signoff...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/3] drivers/perf: riscv: Reset the counter to hpmevent mapping while starting cpus
2024-06-26 16:39 ` Conor Dooley
@ 2024-06-26 20:40 ` Atish Kumar Patra
2024-06-26 22:11 ` Conor Dooley
0 siblings, 1 reply; 12+ messages in thread
From: Atish Kumar Patra @ 2024-06-26 20:40 UTC (permalink / raw)
To: Conor Dooley
Cc: Samuel Holland, linux-riscv, kvm-riscv, Atish Patra, Anup Patel,
Will Deacon, Mark Rutland, Paul Walmsley, Palmer Dabbelt,
Andrew Jones, Conor Dooley, Palmer Dabbelt, Alexandre Ghiti,
linux-arm-kernel, linux-kernel, kvm
On Wed, Jun 26, 2024 at 9:39 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Wed, Jun 26, 2024 at 05:37:07PM +0100, Conor Dooley wrote:
> > On Wed, Jun 26, 2024 at 09:18:46AM -0700, Atish Kumar Patra wrote:
> > > On Wed, Jun 26, 2024 at 6:24 AM Samuel Holland
> > > <samuel.holland@sifive.com> wrote:
> > > >
> > > > On 2024-06-26 2:23 AM, Atish Patra wrote:
> > > > > From: Samuel Holland <samuel.holland@sifive.com>
> > > > >
> > > > > Currently, we stop all the counters while a new cpu is brought online.
> > > > > However, the hpmevent to counter mappings are not reset. The firmware may
> > > > > have some stale encoding in their mapping structure which may lead to
> > > > > undesirable results. We have not encountered such scenario though.
> > > > >
> > > >
> > > > This needs:
> > > >
> > > > Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> > > >
> > >
> > > Oops. Sorry I missed that.
> > >
> > > @Alexandre Ghiti
> >
> > What's Alex going to be able to do?
> >
He is collecting the fixes patches in the RISC-V tree and pinged for
revision for this patch last week.
> > > @Palmer Dabbelt : Can you add that while picking up
> > > the patch or should I respin a v4 ?
> >
> > b4 should pick the signoff up though. "perf: RISC-V: Check standard
> > event availability" seems to be missing your signoff though...
>
> Huh, this doesn't really make sense. I meant:
> b4 should pick the signoff up, though "perf: RISC-V: Check standard
> event availability" seems to be missing your signoff...
Strange. I modified and sent the patch using b4 as well. It's missing
my sign off too.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/3] drivers/perf: riscv: Reset the counter to hpmevent mapping while starting cpus
2024-06-26 20:40 ` Atish Kumar Patra
@ 2024-06-26 22:11 ` Conor Dooley
2024-06-28 14:29 ` Konstantin Ryabitsev
0 siblings, 1 reply; 12+ messages in thread
From: Conor Dooley @ 2024-06-26 22:11 UTC (permalink / raw)
To: Atish Kumar Patra
Cc: Samuel Holland, linux-riscv, kvm-riscv, Atish Patra, Anup Patel,
Will Deacon, Mark Rutland, Paul Walmsley, Palmer Dabbelt,
Andrew Jones, Conor Dooley, Palmer Dabbelt, Alexandre Ghiti,
linux-arm-kernel, linux-kernel, kvm
[-- Attachment #1: Type: text/plain, Size: 821 bytes --]
On Wed, Jun 26, 2024 at 01:40:52PM -0700, Atish Kumar Patra wrote:
> > > > @Palmer Dabbelt : Can you add that while picking up
> > > > the patch or should I respin a v4 ?
> > >
> > > b4 should pick the signoff up though. "perf: RISC-V: Check standard
> > > event availability" seems to be missing your signoff though...
> >
> > Huh, this doesn't really make sense. I meant:
> > b4 should pick the signoff up, though "perf: RISC-V: Check standard
> > event availability" seems to be missing your signoff...
>
> Strange. I modified and sent the patch using b4 as well. It's missing
> my sign off too.
`b4 shazam` should pick up trailers provided on the list, signoffs
included. `b4 shazam -s` will add yours. TBH, I am not sure why that is
not the default behaviour.
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/3] drivers/perf: riscv: Reset the counter to hpmevent mapping while starting cpus
2024-06-26 22:11 ` Conor Dooley
@ 2024-06-28 14:29 ` Konstantin Ryabitsev
0 siblings, 0 replies; 12+ messages in thread
From: Konstantin Ryabitsev @ 2024-06-28 14:29 UTC (permalink / raw)
To: Conor Dooley
Cc: Atish Kumar Patra, Samuel Holland, linux-riscv, kvm-riscv,
Atish Patra, Anup Patel, Will Deacon, Mark Rutland, Paul Walmsley,
Palmer Dabbelt, Andrew Jones, Conor Dooley, Palmer Dabbelt,
Alexandre Ghiti, linux-arm-kernel, linux-kernel, kvm
On Wed, Jun 26, 2024 at 11:11:54PM GMT, Conor Dooley wrote:
> > Strange. I modified and sent the patch using b4 as well. It's missing
> > my sign off too.
>
> `b4 shazam` should pick up trailers provided on the list, signoffs
> included. `b4 shazam -s` will add yours. TBH, I am not sure why that is
> not the default behaviour.
Some projects don't use the DCO model, so they don't require Signed-off-by
trailers.
-K
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 3/3] perf: RISC-V: Check standard event availability
2024-06-26 7:23 [PATCH v3 0/3] Assorted fixes in RISC-V PMU driver Atish Patra
2024-06-26 7:23 ` [PATCH v3 1/3] drivers/perf: riscv: Do not update the event data if uptodate Atish Patra
2024-06-26 7:23 ` [PATCH v3 2/3] drivers/perf: riscv: Reset the counter to hpmevent mapping while starting cpus Atish Patra
@ 2024-06-26 7:23 ` Atish Patra
2024-06-26 13:31 ` [PATCH v3 0/3] Assorted fixes in RISC-V PMU driver Samuel Holland
3 siblings, 0 replies; 12+ messages in thread
From: Atish Patra @ 2024-06-26 7:23 UTC (permalink / raw)
To: linux-riscv, kvm-riscv
Cc: Atish Patra, Anup Patel, Will Deacon, Mark Rutland, Paul Walmsley,
Palmer Dabbelt, Andrew Jones, Conor Dooley, Samuel Holland,
Palmer Dabbelt, Alexandre Ghiti, linux-arm-kernel, linux-kernel,
kvm, Atish Patra
From: Samuel Holland <samuel.holland@sifive.com>
The RISC-V SBI PMU specification defines several standard hardware and
cache events. Currently, all of these events are exposed to userspace,
even when not actually implemented. They appear in the `perf list`
output, and commands like `perf stat` try to use them.
This is more than just a cosmetic issue, because the PMU driver's .add
function fails for these events, which causes pmu_groups_sched_in() to
prematurely stop scheduling in other (possibly valid) hardware events.
Add logic to check which events are supported by the hardware (i.e. can
be mapped to some counter), so only usable events are reported to
userspace. Since the kernel does not know the mapping between events and
possible counters, this check must happen during boot, when no counters
are in use. Make the check asynchronous to minimize impact on boot time.
Fixes: e9991434596f ("RISC-V: Add perf platform driver based on SBI PMU extension")
Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
Tested-by: Atish Patra <atishp@rivosinc.com>
---
arch/riscv/kvm/vcpu_pmu.c | 2 +-
drivers/perf/riscv_pmu_sbi.c | 42 ++++++++++++++++++++++++++++++++++++++++--
2 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
index 04db1f993c47..bcf41d6e0df0 100644
--- a/arch/riscv/kvm/vcpu_pmu.c
+++ b/arch/riscv/kvm/vcpu_pmu.c
@@ -327,7 +327,7 @@ static long kvm_pmu_create_perf_event(struct kvm_pmc *pmc, struct perf_event_att
event = perf_event_create_kernel_counter(attr, -1, current, kvm_riscv_pmu_overflow, pmc);
if (IS_ERR(event)) {
- pr_err("kvm pmu event creation failed for eidx %lx: %ld\n", eidx, PTR_ERR(event));
+ pr_debug("kvm pmu event creation failed for eidx %lx: %ld\n", eidx, PTR_ERR(event));
return PTR_ERR(event);
}
diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
index 94bc369a3454..4e842dcedfba 100644
--- a/drivers/perf/riscv_pmu_sbi.c
+++ b/drivers/perf/riscv_pmu_sbi.c
@@ -20,6 +20,7 @@
#include <linux/cpu_pm.h>
#include <linux/sched/clock.h>
#include <linux/soc/andes/irq.h>
+#include <linux/workqueue.h>
#include <asm/errata_list.h>
#include <asm/sbi.h>
@@ -114,7 +115,7 @@ struct sbi_pmu_event_data {
};
};
-static const struct sbi_pmu_event_data pmu_hw_event_map[] = {
+static struct sbi_pmu_event_data pmu_hw_event_map[] = {
[PERF_COUNT_HW_CPU_CYCLES] = {.hw_gen_event = {
SBI_PMU_HW_CPU_CYCLES,
SBI_PMU_EVENT_TYPE_HW, 0}},
@@ -148,7 +149,7 @@ static const struct sbi_pmu_event_data pmu_hw_event_map[] = {
};
#define C(x) PERF_COUNT_HW_CACHE_##x
-static const struct sbi_pmu_event_data pmu_cache_event_map[PERF_COUNT_HW_CACHE_MAX]
+static struct sbi_pmu_event_data pmu_cache_event_map[PERF_COUNT_HW_CACHE_MAX]
[PERF_COUNT_HW_CACHE_OP_MAX]
[PERF_COUNT_HW_CACHE_RESULT_MAX] = {
[C(L1D)] = {
@@ -293,6 +294,34 @@ static const struct sbi_pmu_event_data pmu_cache_event_map[PERF_COUNT_HW_CACHE_M
},
};
+static void pmu_sbi_check_event(struct sbi_pmu_event_data *edata)
+{
+ struct sbiret ret;
+
+ ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_CFG_MATCH,
+ 0, cmask, 0, edata->event_idx, 0, 0);
+ if (!ret.error) {
+ sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP,
+ ret.value, 0x1, SBI_PMU_STOP_FLAG_RESET, 0, 0, 0);
+ } else if (ret.error == SBI_ERR_NOT_SUPPORTED) {
+ /* This event cannot be monitored by any counter */
+ edata->event_idx = -EINVAL;
+ }
+}
+
+static void pmu_sbi_check_std_events(struct work_struct *work)
+{
+ for (int i = 0; i < ARRAY_SIZE(pmu_hw_event_map); i++)
+ pmu_sbi_check_event(&pmu_hw_event_map[i]);
+
+ for (int i = 0; i < ARRAY_SIZE(pmu_cache_event_map); i++)
+ for (int j = 0; j < ARRAY_SIZE(pmu_cache_event_map[i]); j++)
+ for (int k = 0; k < ARRAY_SIZE(pmu_cache_event_map[i][j]); k++)
+ pmu_sbi_check_event(&pmu_cache_event_map[i][j][k]);
+}
+
+static DECLARE_WORK(check_std_events_work, pmu_sbi_check_std_events);
+
static int pmu_sbi_ctr_get_width(int idx)
{
return pmu_ctr_list[idx].width;
@@ -478,6 +507,12 @@ static int pmu_sbi_event_map(struct perf_event *event, u64 *econfig)
u64 raw_config_val;
int ret;
+ /*
+ * Ensure we are finished checking standard hardware events for
+ * validity before allowing userspace to configure any events.
+ */
+ flush_work(&check_std_events_work);
+
switch (type) {
case PERF_TYPE_HARDWARE:
if (config >= PERF_COUNT_HW_MAX)
@@ -1359,6 +1394,9 @@ static int pmu_sbi_device_probe(struct platform_device *pdev)
if (ret)
goto out_unregister;
+ /* Asynchronously check which standard events are available */
+ schedule_work(&check_std_events_work);
+
return 0;
out_unregister:
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 0/3] Assorted fixes in RISC-V PMU driver
2024-06-26 7:23 [PATCH v3 0/3] Assorted fixes in RISC-V PMU driver Atish Patra
` (2 preceding siblings ...)
2024-06-26 7:23 ` [PATCH v3 3/3] perf: RISC-V: Check standard event availability Atish Patra
@ 2024-06-26 13:31 ` Samuel Holland
3 siblings, 0 replies; 12+ messages in thread
From: Samuel Holland @ 2024-06-26 13:31 UTC (permalink / raw)
To: Atish Patra, linux-riscv, kvm-riscv
Cc: Atish Patra, Anup Patel, Will Deacon, Mark Rutland, Paul Walmsley,
Palmer Dabbelt, Andrew Jones, Conor Dooley, Palmer Dabbelt,
Alexandre Ghiti, linux-arm-kernel, linux-kernel, kvm, garthlei
Hi Atish,
On 2024-06-26 2:23 AM, Atish Patra wrote:
> This series contains 3 fixes out of which the first one is a new fix
> for invalid event data reported in lkml[2]. The last two are v3 of Samuel's
> patch[1]. I added the RB/TB/Fixes tag and moved 1 unrelated change
> to its own patch. I also changed a error message in kvm vcpu_pmu from
> pr_err to pr_debug to avoid redundant failure error messages generated
> due to the boot time quering of events implemented in the patch[1]
Thanks for picking this up! The change in patch 2 isn't quite unrelated.
pmu_sbi_check_std_events() depends on pmu_sbi_stop_all() to ensure all counters
are free at the beginning of the function. Compare v1 of the patch where the
function contains an additional call to SBI_EXT_PMU_COUNTER_STOP. With the
current patch ordering, everything works out, so it all looks good to me.
Regards,
Samuel
> Here is the original cover letter for the patch[1]
>
> Before this patch:
> $ perf list hw
>
> List of pre-defined events (to be used in -e or -M):
>
> branch-instructions OR branches [Hardware event]
> branch-misses [Hardware event]
> bus-cycles [Hardware event]
> cache-misses [Hardware event]
> cache-references [Hardware event]
> cpu-cycles OR cycles [Hardware event]
> instructions [Hardware event]
> ref-cycles [Hardware event]
> stalled-cycles-backend OR idle-cycles-backend [Hardware event]
> stalled-cycles-frontend OR idle-cycles-frontend [Hardware event]
>
> $ perf stat -ddd true
>
> Performance counter stats for 'true':
>
> 4.36 msec task-clock # 0.744 CPUs utilized
> 1 context-switches # 229.325 /sec
> 0 cpu-migrations # 0.000 /sec
> 38 page-faults # 8.714 K/sec
> 4,375,694 cycles # 1.003 GHz (60.64%)
> 728,945 instructions # 0.17 insn per cycle
> 79,199 branches # 18.162 M/sec
> 17,709 branch-misses # 22.36% of all branches
> 181,734 L1-dcache-loads # 41.676 M/sec
> 5,547 L1-dcache-load-misses # 3.05% of all L1-dcache accesses
> <not counted> LLC-loads (0.00%)
> <not counted> LLC-load-misses (0.00%)
> <not counted> L1-icache-loads (0.00%)
> <not counted> L1-icache-load-misses (0.00%)
> <not counted> dTLB-loads (0.00%)
> <not counted> dTLB-load-misses (0.00%)
> <not counted> iTLB-loads (0.00%)
> <not counted> iTLB-load-misses (0.00%)
> <not counted> L1-dcache-prefetches (0.00%)
> <not counted> L1-dcache-prefetch-misses (0.00%)
>
> 0.005860375 seconds time elapsed
>
> 0.000000000 seconds user
> 0.010383000 seconds sys
>
> After this patch:
> $ perf list hw
>
> List of pre-defined events (to be used in -e or -M):
>
> branch-instructions OR branches [Hardware event]
> branch-misses [Hardware event]
> cache-misses [Hardware event]
> cache-references [Hardware event]
> cpu-cycles OR cycles [Hardware event]
> instructions [Hardware event]
>
> $ perf stat -ddd true
>
> Performance counter stats for 'true':
>
> 5.16 msec task-clock # 0.848 CPUs utilized
> 1 context-switches # 193.817 /sec
> 0 cpu-migrations # 0.000 /sec
> 37 page-faults # 7.171 K/sec
> 5,183,625 cycles # 1.005 GHz
> 961,696 instructions # 0.19 insn per cycle
> 85,853 branches # 16.640 M/sec
> 20,462 branch-misses # 23.83% of all branches
> 243,545 L1-dcache-loads # 47.203 M/sec
> 5,974 L1-dcache-load-misses # 2.45% of all L1-dcache accesses
> <not supported> LLC-loads
> <not supported> LLC-load-misses
> <not supported> L1-icache-loads
> <not supported> L1-icache-load-misses
> <not supported> dTLB-loads
> 19,619 dTLB-load-misses
> <not supported> iTLB-loads
> 6,831 iTLB-load-misses
> <not supported> L1-dcache-prefetches
> <not supported> L1-dcache-prefetch-misses
>
> 0.006085625 seconds time elapsed
>
> 0.000000000 seconds user
> 0.013022000 seconds sys
>
> Changes in v3:
> - Added one more fix
> - Separated an unrelated change to its own patch.
> - Rebase and Added RB/TB/Fixes tag.
> - Changed a error message in kvm code to avoid unnecessary failures
> at guest booting.
> Changes in v2:
> - Move the event checking to a workqueue to make it asynchronous
> - Add more details to the commit message based on the v1 discussion
>
> [1] https://lore.kernel.org/linux-riscv/20240418014652.1143466-1-samuel.holland@sifive.com/
> [2] https://lore.kernel.org/all/CC51D53B-846C-4D81-86FC-FBF969D0A0D6@pku.edu.cn/
>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
> Atish Patra (1):
> drivers/perf: riscv: Do not update the event data if uptodate
>
> Samuel Holland (2):
> drivers/perf: riscv: Reset the counter to hpmevent mapping while starting cpus
> perf: RISC-V: Check standard event availability
>
> arch/riscv/kvm/vcpu_pmu.c | 2 +-
> drivers/perf/riscv_pmu.c | 2 +-
> drivers/perf/riscv_pmu_sbi.c | 44 +++++++++++++++++++++++++++++++++++++++++---
> 3 files changed, 43 insertions(+), 5 deletions(-)
> ---
> base-commit: 55027e689933ba2e64f3d245fb1ff185b3e7fc81
> change-id: 20240625-misc_perf_fixes-5c57f555d828
> --
> Regards,
> Atish patra
>
^ permalink raw reply [flat|nested] 12+ messages in thread