* Re: [kvm-riscv/for-next 1/2] drivers/perf: riscv: Remove the warning from stop function
2024-04-25 23:29 ` [kvm-riscv/for-next 1/2] drivers/perf: riscv: Remove the warning from stop function Atish Patra
@ 2024-04-25 20:08 ` Samuel Holland
2024-04-25 20:22 ` Atish Patra
0 siblings, 1 reply; 6+ messages in thread
From: Samuel Holland @ 2024-04-25 20:08 UTC (permalink / raw)
To: Atish Patra, linux-kernel
Cc: Alexandre Ghiti, Andrew Jones, Anup Patel, Atish Patra,
Conor Dooley, linux-riscv, kvm-riscv, Mark Rutland,
Palmer Dabbelt, Paul Walmsley, Will Deacon
On 2024-04-25 6:29 PM, Atish Patra wrote:
> The warning message was initially added just to indicate that counter
> stop function is being called while the event is already stopped.
>
> However, we update the state to stopped case now in an overflow handler
> after stopping the counter. If there is another child overflow handler
> is registered (e.g kvm) it may call stop again which will trigger the
> warning.
>
> Fixes : commit 22f5dac41004d ("drivers/perf: riscv: Implement SBI PMU snapshot function")
This may be intentional, since you wanted these to be squashed, but this isn't
the right format for a Fixes: tag (no space before ":" and no "commit"). Otherwise,
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
> drivers/perf/riscv_pmu.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
> index 36d348753d05..78c490e0505a 100644
> --- a/drivers/perf/riscv_pmu.c
> +++ b/drivers/perf/riscv_pmu.c
> @@ -191,8 +191,6 @@ void riscv_pmu_stop(struct perf_event *event, int flags)
> struct hw_perf_event *hwc = &event->hw;
> struct riscv_pmu *rvpmu = to_riscv_pmu(event->pmu);
>
> - WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
> -
> if (!(hwc->state & PERF_HES_STOPPED)) {
> if (rvpmu->ctr_stop) {
> rvpmu->ctr_stop(event, 0);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-riscv/for-next 2/2] drivers/perf: riscv: Fix RV32 snapshot overflow use case
2024-04-25 23:29 ` [kvm-riscv/for-next 2/2] drivers/perf: riscv: Fix RV32 snapshot overflow use case Atish Patra
@ 2024-04-25 20:18 ` Samuel Holland
0 siblings, 0 replies; 6+ messages in thread
From: Samuel Holland @ 2024-04-25 20:18 UTC (permalink / raw)
To: Atish Patra
Cc: Alexandre Ghiti, Andrew Jones, Anup Patel, Atish Patra,
Conor Dooley, linux-riscv, kvm-riscv, Mark Rutland,
Palmer Dabbelt, Paul Walmsley, Will Deacon, linux-kernel
On 2024-04-25 6:29 PM, Atish Patra wrote:
> The shadow copy alogirthm is implemented incorrectly. This patch fixes
> the behavior by keeping a per cpu shadow copy of the counter values to
> avoid clobbering for the cases where system more than XLEN counters and
> the overflown counter index are beyond XLEN. This issue can only be
> observed only in RV32 if an SBI implementation assigns logical counters
> ids greater than XLEN or firmware counter overflow is supported in the
> future.
>
> Fixes : commit 22f5dac41004d ("drivers/perf: riscv: Implement SBI PMU snapshot function")
Same comment as for patch 1. The logic looks correct as far as I can tell, so:
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
One minor comment below.
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
> drivers/perf/riscv_pmu_sbi.c | 49 +++++++++++++++++++---------------
> include/linux/perf/riscv_pmu.h | 2 ++
> 2 files changed, 30 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index 2694110f1cff..98aaeb13e9db 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -588,6 +588,7 @@ static int pmu_sbi_snapshot_setup(struct riscv_pmu *pmu, int cpu)
> return sbi_err_map_linux_errno(ret.error);
> }
>
> + memset(cpu_hw_evt->snapshot_cval_shcopy, 0, sizeof(u64) * RISCV_MAX_COUNTERS);
> cpu_hw_evt->snapshot_set_done = true;
>
> return 0;
> @@ -605,7 +606,7 @@ static u64 pmu_sbi_ctr_read(struct perf_event *event)
> union sbi_pmu_ctr_info info = pmu_ctr_list[idx];
>
> /* Read the value from the shared memory directly only if counter is stopped */
> - if (sbi_pmu_snapshot_available() & (hwc->state & PERF_HES_STOPPED)) {
> + if (sbi_pmu_snapshot_available() && (hwc->state & PERF_HES_STOPPED)) {
> val = sdata->ctr_values[idx];
> return val;
> }
> @@ -769,36 +770,38 @@ static inline void pmu_sbi_stop_hw_ctrs(struct riscv_pmu *pmu)
> struct cpu_hw_events *cpu_hw_evt = this_cpu_ptr(pmu->hw_events);
> struct riscv_pmu_snapshot_data *sdata = cpu_hw_evt->snapshot_addr;
> unsigned long flag = 0;
> - int i;
> + int i, idx;
> struct sbiret ret;
> - unsigned long temp_ctr_values[64] = {0};
> - unsigned long ctr_val, temp_ctr_overflow_mask = 0;
> + u64 temp_ctr_overflow_mask = 0;
>
> if (sbi_pmu_snapshot_available())
> flag = SBI_PMU_STOP_FLAG_TAKE_SNAPSHOT;
>
> + /* Reset the shadow copy to avoid save/restore any value from previous overflow */
> + memset(cpu_hw_evt->snapshot_cval_shcopy, 0, sizeof(u64) * RISCV_MAX_COUNTERS);
> +
> for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
> /* No need to check the error here as we can't do anything about the error */
> ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP, i * BITS_PER_LONG,
> cpu_hw_evt->used_hw_ctrs[i], flag, 0, 0, 0);
> if (!ret.error && sbi_pmu_snapshot_available()) {
> /* Save the counter values to avoid clobbering */
> - temp_ctr_values[i * BITS_PER_LONG + i] = sdata->ctr_values[i];
> - /* Save the overflow mask to avoid clobbering */
> - if (BIT(i) & sdata->ctr_overflow_mask)
> - temp_ctr_overflow_mask |= BIT(i + i * BITS_PER_LONG);
> + for_each_set_bit(idx, &cpu_hw_evt->used_hw_ctrs[i], BITS_PER_LONG) {
> + cpu_hw_evt->snapshot_cval_shcopy[i * BITS_PER_LONG + idx] =
> + sdata->ctr_values[idx];
> + /* Save the overflow mask to avoid clobbering */
> + if (BIT(idx) & sdata->ctr_overflow_mask)
> + temp_ctr_overflow_mask |= BIT(idx + i * BITS_PER_LONG);
This is equivalent to doing
temp_ctr_overflow_mask |= sdata->ctr_overflow_mask << (i * BITS_PER_LONG);
outside the for_each_set_bit() loop.
> + }
> }
> }
>
> - /* Restore the counter values to the shared memory */
> + /* Restore the counter values to the shared memory for used hw counters */
> if (sbi_pmu_snapshot_available()) {
> - for (i = 0; i < 64; i++) {
> - ctr_val = temp_ctr_values[i];
> - if (ctr_val)
> - sdata->ctr_values[i] = ctr_val;
> - if (temp_ctr_overflow_mask)
> - sdata->ctr_overflow_mask = temp_ctr_overflow_mask;
> - }
> + for_each_set_bit(idx, cpu_hw_evt->used_hw_ctrs, RISCV_MAX_COUNTERS)
> + sdata->ctr_values[idx] = cpu_hw_evt->snapshot_cval_shcopy[idx];
> + if (temp_ctr_overflow_mask)
> + sdata->ctr_overflow_mask = temp_ctr_overflow_mask;
> }
> }
>
> @@ -850,7 +853,7 @@ static inline void pmu_sbi_start_ovf_ctrs_sbi(struct cpu_hw_events *cpu_hw_evt,
> static inline void pmu_sbi_start_ovf_ctrs_snapshot(struct cpu_hw_events *cpu_hw_evt,
> u64 ctr_ovf_mask)
> {
> - int idx = 0;
> + int i, idx = 0;
> struct perf_event *event;
> unsigned long flag = SBI_PMU_START_FLAG_INIT_SNAPSHOT;
> u64 max_period, init_val = 0;
> @@ -863,7 +866,7 @@ static inline void pmu_sbi_start_ovf_ctrs_snapshot(struct cpu_hw_events *cpu_hw_
> hwc = &event->hw;
> max_period = riscv_pmu_ctr_get_width_mask(event);
> init_val = local64_read(&hwc->prev_count) & max_period;
> - sdata->ctr_values[idx] = init_val;
> + cpu_hw_evt->snapshot_cval_shcopy[idx] = init_val;
> }
> /*
> * We do not need to update the non-overflow counters the previous
> @@ -871,10 +874,14 @@ static inline void pmu_sbi_start_ovf_ctrs_snapshot(struct cpu_hw_events *cpu_hw_
> */
> }
>
> - for (idx = 0; idx < BITS_TO_LONGS(RISCV_MAX_COUNTERS); idx++) {
> + for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
> + /* Restore the counter values to relative indices for used hw counters */
> + for_each_set_bit(idx, &cpu_hw_evt->used_hw_ctrs[i], BITS_PER_LONG)
> + sdata->ctr_values[idx] =
> + cpu_hw_evt->snapshot_cval_shcopy[idx + i * BITS_PER_LONG];
> /* Start all the counters in a single shot */
> sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, idx * BITS_PER_LONG,
> - cpu_hw_evt->used_hw_ctrs[idx], flag, 0, 0, 0);
> + cpu_hw_evt->used_hw_ctrs[i], flag, 0, 0, 0);
> }
> }
>
> @@ -898,7 +905,7 @@ static irqreturn_t pmu_sbi_ovf_handler(int irq, void *dev)
> int lidx, hidx, fidx;
> struct riscv_pmu *pmu;
> struct perf_event *event;
> - unsigned long overflow;
> + u64 overflow;
> u64 overflowed_ctrs = 0;
> struct cpu_hw_events *cpu_hw_evt = dev;
> u64 start_clock = sched_clock();
> diff --git a/include/linux/perf/riscv_pmu.h b/include/linux/perf/riscv_pmu.h
> index c3fa90970042..701974639ff2 100644
> --- a/include/linux/perf/riscv_pmu.h
> +++ b/include/linux/perf/riscv_pmu.h
> @@ -45,6 +45,8 @@ struct cpu_hw_events {
> phys_addr_t snapshot_addr_phys;
> /* Boolean flag to indicate setup is already done */
> bool snapshot_set_done;
> + /* A shadow copy of the counter values to avoid clobbering during multiple SBI calls */
> + u64 snapshot_cval_shcopy[RISCV_MAX_COUNTERS];
> };
>
> struct riscv_pmu {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-riscv/for-next 1/2] drivers/perf: riscv: Remove the warning from stop function
2024-04-25 20:08 ` Samuel Holland
@ 2024-04-25 20:22 ` Atish Patra
0 siblings, 0 replies; 6+ messages in thread
From: Atish Patra @ 2024-04-25 20:22 UTC (permalink / raw)
To: Samuel Holland
Cc: Atish Patra, linux-kernel, Alexandre Ghiti, Andrew Jones,
Anup Patel, Conor Dooley, linux-riscv, kvm-riscv, Mark Rutland,
Palmer Dabbelt, Paul Walmsley, Will Deacon
On Thu, Apr 25, 2024 at 1:08 PM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> On 2024-04-25 6:29 PM, Atish Patra wrote:
> > The warning message was initially added just to indicate that counter
> > stop function is being called while the event is already stopped.
> >
> > However, we update the state to stopped case now in an overflow handler
> > after stopping the counter. If there is another child overflow handler
> > is registered (e.g kvm) it may call stop again which will trigger the
> > warning.
> >
> > Fixes : commit 22f5dac41004d ("drivers/perf: riscv: Implement SBI PMU snapshot function")
>
> This may be intentional, since you wanted these to be squashed, but this isn't
> the right format for a Fixes: tag (no space before ":" and no "commit"). Otherwise,
>
Yeah. Just wanted to be explicit as the commit is based on kvm-riscv
queue and not based on upstream commit.
> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
>
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > ---
> > drivers/perf/riscv_pmu.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
> > index 36d348753d05..78c490e0505a 100644
> > --- a/drivers/perf/riscv_pmu.c
> > +++ b/drivers/perf/riscv_pmu.c
> > @@ -191,8 +191,6 @@ void riscv_pmu_stop(struct perf_event *event, int flags)
> > struct hw_perf_event *hwc = &event->hw;
> > struct riscv_pmu *rvpmu = to_riscv_pmu(event->pmu);
> >
> > - WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
> > -
> > if (!(hwc->state & PERF_HES_STOPPED)) {
> > if (rvpmu->ctr_stop) {
> > rvpmu->ctr_stop(event, 0);
>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 6+ messages in thread
* [kvm-riscv/for-next 0/2] Fixes for kvm-riscv
@ 2024-04-25 23:29 Atish Patra
2024-04-25 23:29 ` [kvm-riscv/for-next 1/2] drivers/perf: riscv: Remove the warning from stop function Atish Patra
2024-04-25 23:29 ` [kvm-riscv/for-next 2/2] drivers/perf: riscv: Fix RV32 snapshot overflow use case Atish Patra
0 siblings, 2 replies; 6+ messages in thread
From: Atish Patra @ 2024-04-25 23:29 UTC (permalink / raw)
To: linux-kernel
Cc: Atish Patra, Alexandre Ghiti, Andrew Jones, Anup Patel,
Atish Patra, Conor Dooley, linux-riscv, kvm-riscv, Mark Rutland,
Samuel Holland, Palmer Dabbelt, Paul Walmsley, Will Deacon
Here are two fixes for issues found during review/testing after the
series[1] has been queued for 6.10.
@Anup: Can you please squash them into the original source commit
22f5dac41004 that introduced this ?
[1] https://lore.kernel.org/all/20240420151741.962500-1-atishp@rivosinc.com/
Atish Patra (2):
drivers/perf: riscv: Remove the warning from stop function
drivers/perf: riscv: Fix RV32 snapshot overflow use case
drivers/perf/riscv_pmu.c | 2 --
drivers/perf/riscv_pmu_sbi.c | 49 +++++++++++++++++++---------------
include/linux/perf/riscv_pmu.h | 2 ++
3 files changed, 30 insertions(+), 23 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [kvm-riscv/for-next 1/2] drivers/perf: riscv: Remove the warning from stop function
2024-04-25 23:29 [kvm-riscv/for-next 0/2] Fixes for kvm-riscv Atish Patra
@ 2024-04-25 23:29 ` Atish Patra
2024-04-25 20:08 ` Samuel Holland
2024-04-25 23:29 ` [kvm-riscv/for-next 2/2] drivers/perf: riscv: Fix RV32 snapshot overflow use case Atish Patra
1 sibling, 1 reply; 6+ messages in thread
From: Atish Patra @ 2024-04-25 23:29 UTC (permalink / raw)
To: linux-kernel
Cc: Atish Patra, Alexandre Ghiti, Andrew Jones, Anup Patel,
Atish Patra, Conor Dooley, linux-riscv, kvm-riscv, Mark Rutland,
Samuel Holland, Palmer Dabbelt, Paul Walmsley, Will Deacon
The warning message was initially added just to indicate that counter
stop function is being called while the event is already stopped.
However, we update the state to stopped case now in an overflow handler
after stopping the counter. If there is another child overflow handler
is registered (e.g kvm) it may call stop again which will trigger the
warning.
Fixes : commit 22f5dac41004d ("drivers/perf: riscv: Implement SBI PMU snapshot function")
Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
drivers/perf/riscv_pmu.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
index 36d348753d05..78c490e0505a 100644
--- a/drivers/perf/riscv_pmu.c
+++ b/drivers/perf/riscv_pmu.c
@@ -191,8 +191,6 @@ void riscv_pmu_stop(struct perf_event *event, int flags)
struct hw_perf_event *hwc = &event->hw;
struct riscv_pmu *rvpmu = to_riscv_pmu(event->pmu);
- WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
-
if (!(hwc->state & PERF_HES_STOPPED)) {
if (rvpmu->ctr_stop) {
rvpmu->ctr_stop(event, 0);
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [kvm-riscv/for-next 2/2] drivers/perf: riscv: Fix RV32 snapshot overflow use case
2024-04-25 23:29 [kvm-riscv/for-next 0/2] Fixes for kvm-riscv Atish Patra
2024-04-25 23:29 ` [kvm-riscv/for-next 1/2] drivers/perf: riscv: Remove the warning from stop function Atish Patra
@ 2024-04-25 23:29 ` Atish Patra
2024-04-25 20:18 ` Samuel Holland
1 sibling, 1 reply; 6+ messages in thread
From: Atish Patra @ 2024-04-25 23:29 UTC (permalink / raw)
To: linux-kernel
Cc: Atish Patra, Alexandre Ghiti, Andrew Jones, Anup Patel,
Atish Patra, Conor Dooley, linux-riscv, kvm-riscv, Mark Rutland,
Samuel Holland, Palmer Dabbelt, Paul Walmsley, Will Deacon
The shadow copy alogirthm is implemented incorrectly. This patch fixes
the behavior by keeping a per cpu shadow copy of the counter values to
avoid clobbering for the cases where system more than XLEN counters and
the overflown counter index are beyond XLEN. This issue can only be
observed only in RV32 if an SBI implementation assigns logical counters
ids greater than XLEN or firmware counter overflow is supported in the
future.
Fixes : commit 22f5dac41004d ("drivers/perf: riscv: Implement SBI PMU snapshot function")
Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
drivers/perf/riscv_pmu_sbi.c | 49 +++++++++++++++++++---------------
include/linux/perf/riscv_pmu.h | 2 ++
2 files changed, 30 insertions(+), 21 deletions(-)
diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
index 2694110f1cff..98aaeb13e9db 100644
--- a/drivers/perf/riscv_pmu_sbi.c
+++ b/drivers/perf/riscv_pmu_sbi.c
@@ -588,6 +588,7 @@ static int pmu_sbi_snapshot_setup(struct riscv_pmu *pmu, int cpu)
return sbi_err_map_linux_errno(ret.error);
}
+ memset(cpu_hw_evt->snapshot_cval_shcopy, 0, sizeof(u64) * RISCV_MAX_COUNTERS);
cpu_hw_evt->snapshot_set_done = true;
return 0;
@@ -605,7 +606,7 @@ static u64 pmu_sbi_ctr_read(struct perf_event *event)
union sbi_pmu_ctr_info info = pmu_ctr_list[idx];
/* Read the value from the shared memory directly only if counter is stopped */
- if (sbi_pmu_snapshot_available() & (hwc->state & PERF_HES_STOPPED)) {
+ if (sbi_pmu_snapshot_available() && (hwc->state & PERF_HES_STOPPED)) {
val = sdata->ctr_values[idx];
return val;
}
@@ -769,36 +770,38 @@ static inline void pmu_sbi_stop_hw_ctrs(struct riscv_pmu *pmu)
struct cpu_hw_events *cpu_hw_evt = this_cpu_ptr(pmu->hw_events);
struct riscv_pmu_snapshot_data *sdata = cpu_hw_evt->snapshot_addr;
unsigned long flag = 0;
- int i;
+ int i, idx;
struct sbiret ret;
- unsigned long temp_ctr_values[64] = {0};
- unsigned long ctr_val, temp_ctr_overflow_mask = 0;
+ u64 temp_ctr_overflow_mask = 0;
if (sbi_pmu_snapshot_available())
flag = SBI_PMU_STOP_FLAG_TAKE_SNAPSHOT;
+ /* Reset the shadow copy to avoid save/restore any value from previous overflow */
+ memset(cpu_hw_evt->snapshot_cval_shcopy, 0, sizeof(u64) * RISCV_MAX_COUNTERS);
+
for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
/* No need to check the error here as we can't do anything about the error */
ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP, i * BITS_PER_LONG,
cpu_hw_evt->used_hw_ctrs[i], flag, 0, 0, 0);
if (!ret.error && sbi_pmu_snapshot_available()) {
/* Save the counter values to avoid clobbering */
- temp_ctr_values[i * BITS_PER_LONG + i] = sdata->ctr_values[i];
- /* Save the overflow mask to avoid clobbering */
- if (BIT(i) & sdata->ctr_overflow_mask)
- temp_ctr_overflow_mask |= BIT(i + i * BITS_PER_LONG);
+ for_each_set_bit(idx, &cpu_hw_evt->used_hw_ctrs[i], BITS_PER_LONG) {
+ cpu_hw_evt->snapshot_cval_shcopy[i * BITS_PER_LONG + idx] =
+ sdata->ctr_values[idx];
+ /* Save the overflow mask to avoid clobbering */
+ if (BIT(idx) & sdata->ctr_overflow_mask)
+ temp_ctr_overflow_mask |= BIT(idx + i * BITS_PER_LONG);
+ }
}
}
- /* Restore the counter values to the shared memory */
+ /* Restore the counter values to the shared memory for used hw counters */
if (sbi_pmu_snapshot_available()) {
- for (i = 0; i < 64; i++) {
- ctr_val = temp_ctr_values[i];
- if (ctr_val)
- sdata->ctr_values[i] = ctr_val;
- if (temp_ctr_overflow_mask)
- sdata->ctr_overflow_mask = temp_ctr_overflow_mask;
- }
+ for_each_set_bit(idx, cpu_hw_evt->used_hw_ctrs, RISCV_MAX_COUNTERS)
+ sdata->ctr_values[idx] = cpu_hw_evt->snapshot_cval_shcopy[idx];
+ if (temp_ctr_overflow_mask)
+ sdata->ctr_overflow_mask = temp_ctr_overflow_mask;
}
}
@@ -850,7 +853,7 @@ static inline void pmu_sbi_start_ovf_ctrs_sbi(struct cpu_hw_events *cpu_hw_evt,
static inline void pmu_sbi_start_ovf_ctrs_snapshot(struct cpu_hw_events *cpu_hw_evt,
u64 ctr_ovf_mask)
{
- int idx = 0;
+ int i, idx = 0;
struct perf_event *event;
unsigned long flag = SBI_PMU_START_FLAG_INIT_SNAPSHOT;
u64 max_period, init_val = 0;
@@ -863,7 +866,7 @@ static inline void pmu_sbi_start_ovf_ctrs_snapshot(struct cpu_hw_events *cpu_hw_
hwc = &event->hw;
max_period = riscv_pmu_ctr_get_width_mask(event);
init_val = local64_read(&hwc->prev_count) & max_period;
- sdata->ctr_values[idx] = init_val;
+ cpu_hw_evt->snapshot_cval_shcopy[idx] = init_val;
}
/*
* We do not need to update the non-overflow counters the previous
@@ -871,10 +874,14 @@ static inline void pmu_sbi_start_ovf_ctrs_snapshot(struct cpu_hw_events *cpu_hw_
*/
}
- for (idx = 0; idx < BITS_TO_LONGS(RISCV_MAX_COUNTERS); idx++) {
+ for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
+ /* Restore the counter values to relative indices for used hw counters */
+ for_each_set_bit(idx, &cpu_hw_evt->used_hw_ctrs[i], BITS_PER_LONG)
+ sdata->ctr_values[idx] =
+ cpu_hw_evt->snapshot_cval_shcopy[idx + i * BITS_PER_LONG];
/* Start all the counters in a single shot */
sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, idx * BITS_PER_LONG,
- cpu_hw_evt->used_hw_ctrs[idx], flag, 0, 0, 0);
+ cpu_hw_evt->used_hw_ctrs[i], flag, 0, 0, 0);
}
}
@@ -898,7 +905,7 @@ static irqreturn_t pmu_sbi_ovf_handler(int irq, void *dev)
int lidx, hidx, fidx;
struct riscv_pmu *pmu;
struct perf_event *event;
- unsigned long overflow;
+ u64 overflow;
u64 overflowed_ctrs = 0;
struct cpu_hw_events *cpu_hw_evt = dev;
u64 start_clock = sched_clock();
diff --git a/include/linux/perf/riscv_pmu.h b/include/linux/perf/riscv_pmu.h
index c3fa90970042..701974639ff2 100644
--- a/include/linux/perf/riscv_pmu.h
+++ b/include/linux/perf/riscv_pmu.h
@@ -45,6 +45,8 @@ struct cpu_hw_events {
phys_addr_t snapshot_addr_phys;
/* Boolean flag to indicate setup is already done */
bool snapshot_set_done;
+ /* A shadow copy of the counter values to avoid clobbering during multiple SBI calls */
+ u64 snapshot_cval_shcopy[RISCV_MAX_COUNTERS];
};
struct riscv_pmu {
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-25 20:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-25 23:29 [kvm-riscv/for-next 0/2] Fixes for kvm-riscv Atish Patra
2024-04-25 23:29 ` [kvm-riscv/for-next 1/2] drivers/perf: riscv: Remove the warning from stop function Atish Patra
2024-04-25 20:08 ` Samuel Holland
2024-04-25 20:22 ` Atish Patra
2024-04-25 23:29 ` [kvm-riscv/for-next 2/2] drivers/perf: riscv: Fix RV32 snapshot overflow use case Atish Patra
2024-04-25 20:18 ` Samuel Holland
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®