| Message ID | 20240403080452.1007601-4-atishp@rivosinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v5,01/22] RISC-V: Fix the typo in Scountovf CSR name | expand |
On 4/4/24 04:02, Andrew Jones wrote: > On Wed, Apr 03, 2024 at 01:04:32AM -0700, Atish Patra wrote: >> SBI v2.0 introduced a explicit function to read the upper 32 bits >> for any firmware counter width that is longer than 32bits. >> This is only applicable for RV32 where firmware counter can be >> 64 bit. >> >> Reviewed-by: Andrew Jones <ajones@ventanamicro.com> >> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> >> Reviewed-by: Conor Dooley <conor.dooley@microchip.com> >> Reviewed-by: Anup Patel <anup@brainfault.org> >> Signed-off-by: Atish Patra <atishp@rivosinc.com> >> --- >> drivers/perf/riscv_pmu_sbi.c | 25 ++++++++++++++++++++----- >> 1 file changed, 20 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c >> index 3e44d2fb8bf8..babf1b9a4dbe 100644 >> --- a/drivers/perf/riscv_pmu_sbi.c >> +++ b/drivers/perf/riscv_pmu_sbi.c >> @@ -57,6 +57,8 @@ asm volatile(ALTERNATIVE( \ >> PMU_FORMAT_ATTR(event, "config:0-47"); >> PMU_FORMAT_ATTR(firmware, "config:63"); >> >> +static bool sbi_v2_available; >> + >> static struct attribute *riscv_arch_formats_attr[] = { >> &format_attr_event.attr, >> &format_attr_firmware.attr, >> @@ -511,19 +513,29 @@ static u64 pmu_sbi_ctr_read(struct perf_event *event) >> struct hw_perf_event *hwc = &event->hw; >> int idx = hwc->idx; >> struct sbiret ret; >> - union sbi_pmu_ctr_info info; >> u64 val = 0; >> + union sbi_pmu_ctr_info info = pmu_ctr_list[idx]; >> >> if (pmu_sbi_is_fw_event(event)) { >> ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_FW_READ, >> hwc->idx, 0, 0, 0, 0, 0); >> - if (!ret.error) >> - val = ret.value; >> + if (ret.error) >> + return 0; >> + >> + val = ret.value; >> + if (IS_ENABLED(CONFIG_32BIT) && sbi_v2_available && info.width >= 32) { >> + ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_FW_READ_HI, >> + hwc->idx, 0, 0, 0, 0, 0); >> + if (!ret.error) >> + val |= ((u64)ret.value << 32); >> + else >> + WARN_ONCE(1, "Unable to read upper 32 bits of firmware counter error: %d\n", >> + sbi_err_map_linux_errno(ret.error)); > > I don't think we should use sbi_err_map_linux_errno() in this case since > we don't have a 1:1 mapping of SBI errors to Linux errors and we don't > propagate the error as a Linux error. For warnings, it's better to output > the exact SBI error. > Sure. Fixed it. >> + } >> } else { >> - info = pmu_ctr_list[idx]; >> val = riscv_pmu_ctr_read_csr(info.csr); >> if (IS_ENABLED(CONFIG_32BIT)) >> - val = ((u64)riscv_pmu_ctr_read_csr(info.csr + 0x80)) << 31 | val; >> + val |= ((u64)riscv_pmu_ctr_read_csr(info.csr + 0x80)) << 32; >> } >> >> return val; >> @@ -1135,6 +1147,9 @@ static int __init pmu_sbi_devinit(void) >> return 0; >> } >> >> + if (sbi_spec_version >= sbi_mk_version(2, 0)) >> + sbi_v2_available = true; >> + >> ret = cpuhp_setup_state_multi(CPUHP_AP_PERF_RISCV_STARTING, >> "perf/riscv/pmu:starting", >> pmu_sbi_starting_cpu, pmu_sbi_dying_cpu); >> -- >> 2.34.1 >> > > Thanks, > drew
diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c index 3e44d2fb8bf8..babf1b9a4dbe 100644 --- a/drivers/perf/riscv_pmu_sbi.c +++ b/drivers/perf/riscv_pmu_sbi.c @@ -57,6 +57,8 @@ asm volatile(ALTERNATIVE( \ PMU_FORMAT_ATTR(event, "config:0-47"); PMU_FORMAT_ATTR(firmware, "config:63"); +static bool sbi_v2_available; + static struct attribute *riscv_arch_formats_attr[] = { &format_attr_event.attr, &format_attr_firmware.attr, @@ -511,19 +513,29 @@ static u64 pmu_sbi_ctr_read(struct perf_event *event) struct hw_perf_event *hwc = &event->hw; int idx = hwc->idx; struct sbiret ret; - union sbi_pmu_ctr_info info; u64 val = 0; + union sbi_pmu_ctr_info info = pmu_ctr_list[idx]; if (pmu_sbi_is_fw_event(event)) { ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_FW_READ, hwc->idx, 0, 0, 0, 0, 0); - if (!ret.error) - val = ret.value; + if (ret.error) + return 0; + + val = ret.value; + if (IS_ENABLED(CONFIG_32BIT) && sbi_v2_available && info.width >= 32) { + ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_FW_READ_HI, + hwc->idx, 0, 0, 0, 0, 0); + if (!ret.error) + val |= ((u64)ret.value << 32); + else + WARN_ONCE(1, "Unable to read upper 32 bits of firmware counter error: %d\n", + sbi_err_map_linux_errno(ret.error)); + } } else { - info = pmu_ctr_list[idx]; val = riscv_pmu_ctr_read_csr(info.csr); if (IS_ENABLED(CONFIG_32BIT)) - val = ((u64)riscv_pmu_ctr_read_csr(info.csr + 0x80)) << 31 | val; + val |= ((u64)riscv_pmu_ctr_read_csr(info.csr + 0x80)) << 32; } return val; @@ -1135,6 +1147,9 @@ static int __init pmu_sbi_devinit(void) return 0; } + if (sbi_spec_version >= sbi_mk_version(2, 0)) + sbi_v2_available = true; + ret = cpuhp_setup_state_multi(CPUHP_AP_PERF_RISCV_STARTING, "perf/riscv/pmu:starting", pmu_sbi_starting_cpu, pmu_sbi_dying_cpu);