[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHBxVyEtxoRHLO791r6uFzHnX0vAcpeWMOcS+0tbVN+p6Oh2tQ@mail.gmail.com>
Date: Fri, 7 Feb 2025 08:53:59 -0800
From: Atish Kumar Patra <atishp@...osinc.com>
To: Will Deacon <will@...nel.org>
Cc: Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Anup Patel <anup@...infault.org>, Atish Patra <atishp@...shpatra.org>,
Mark Rutland <mark.rutland@....com>, Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>, Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>, weilin.wang@...el.com,
linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
Conor Dooley <conor@...nel.org>, devicetree@...r.kernel.org, kvm@...r.kernel.org,
kvm-riscv@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v4 11/21] RISC-V: perf: Restructure the SBI PMU code
On Thu, Feb 6, 2025 at 2:51 AM Will Deacon <will@...nel.org> wrote:
>
> On Wed, Feb 05, 2025 at 11:23:16PM -0800, Atish Patra wrote:
> > With Ssccfg/Smcdeleg, we no longer need SBI PMU extension to program/
> > access hpmcounter/events. However, we do need it for firmware counters.
> > Rename the driver and its related code to represent generic name
> > that will handle both sbi and ISA mechanism for hpmcounter related
> > operations. Take this opportunity to update the Kconfig names to
> > match the new driver name closely.
> >
> > No functional change intended.
> >
> > Signed-off-by: Atish Patra <atishp@...osinc.com>
> > ---
> > MAINTAINERS | 4 +-
> > arch/riscv/include/asm/kvm_vcpu_pmu.h | 4 +-
> > arch/riscv/include/asm/kvm_vcpu_sbi.h | 2 +-
> > arch/riscv/kvm/Makefile | 4 +-
> > arch/riscv/kvm/vcpu_sbi.c | 2 +-
> > drivers/perf/Kconfig | 16 +-
> > drivers/perf/Makefile | 4 +-
> > drivers/perf/{riscv_pmu.c => riscv_pmu_common.c} | 0
> > drivers/perf/{riscv_pmu_sbi.c => riscv_pmu_dev.c} | 214 +++++++++++++---------
>
> This seems... gratuitous? It feels like renaming the file could be a pain
> for managing backports and renaming the driver might cause some headaches
> in userspace.
>
> What do you gain from such an invasive change?
>
The current upstream driver was developed only for SBI PMU[1] due to
lack of any ISA extension support that allows PMU programming from the
kernel.
Thus, the name of the file and the functions reflect those assumptions.
IMO, it would be confusing to use a driver named sbi that also uses
the new ISA extensions that allows direct PMU programming from the
kernel.
This patch allows the internal functions and structure to clearly
identify two ways of PMU programming.
There are more PMU related ISA extensions planned in the future as
well. Thus, I felt it would be best to do these changes now rather
than later.
I understand that managing the backport would be a pain as a result
but I didn't quite understand the concern about the userspace
headances.
Can you please clarify that ?
[1] https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/src/ext-pmu.adoc
> Will
Powered by blists - more mailing lists