Messages in this thread Patch in this message | | | From | <> | Subject | Re: [PATCH v2] drivers/perf: riscv_pmu_sbi: add support for PMU variant on T-Head C9xx cores | Date | Fri, 26 Aug 2022 17:57:33 +0000 |
| |
On 26/08/2022 17:35, Heiko Stuebner wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > With the T-HEAD C9XX cores being designed before or during the ratification > to the SSCOFPMF extension, they implement functionality very similar but > not equal to it. So add some adaptions to allow the C9XX to still handle > its PMU through the regular SBI PMU interface instead of defining new > interfaces or drivers. > > To work properly, this requires a matching change in SBI, though the actual > interface between kernel and SBI does not change. > > The main differences are a the overflow CSR and irq number. > > As the reading of the overflow-csr is in the hot-path during irq handling
Hey Heiko,
Very nitpicky, but I had to read this twice to get it.. If you respin, it'd be worth adding a comma after "handling".
> use an errata and alternatives to not introduce new conditionals there. > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > --- > changes in v2: > - use alternatives for the CSR access > - make the irq num selection a bit nicer > > There is of course a matching opensbi-part whose current implementation can > be found on [0], but as comments show, this needs some more work still. > > > [0] https://patchwork.ozlabs.org/project/opensbi/cover/20220817112004.745776-1-heiko@sntech.de/ > > arch/riscv/Kconfig.erratas | 14 ++++++++++++ > arch/riscv/errata/thead/errata.c | 19 +++++++++++++++++ > arch/riscv/include/asm/errata_list.h | 16 +++++++++++++- > drivers/perf/riscv_pmu_sbi.c | 32 +++++++++++++++++++--------- > 4 files changed, 70 insertions(+), 11 deletions(-) > > diff --git a/arch/riscv/Kconfig.erratas b/arch/riscv/Kconfig.erratas > index 6850e9389930..f1eaac4c0073 100644 > --- a/arch/riscv/Kconfig.erratas > +++ b/arch/riscv/Kconfig.erratas > @@ -66,4 +66,18 @@ config ERRATA_THEAD_CMO > > If you don't know what to do here, say "Y". > > +config ERRATA_THEAD_PMU > + bool "Apply T-Head PMU errata" > + depends on ERRATA_THEAD > + depends on RISCV_PMU_SBI > + default y > + help > + The T-Head C9xx cores implement a PMU overflow extension very > + similar to the core SSCOFPMF extension. > + > + This will apply the overflow errata to handle the non-standard > + behaviour via the regular SBI PMU driver and interface. > + > + If you don't know what to do here, say "Y". > + > endmenu # "CPU errata selection" > diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c > index 202c83f677b2..e6101eab25c8 100644 > --- a/arch/riscv/errata/thead/errata.c > +++ b/arch/riscv/errata/thead/errata.c > @@ -44,6 +44,22 @@ static bool errata_probe_cmo(unsigned int stage, > #endif > } > > +static bool errata_probe_pmu(unsigned int stage, > + unsigned long arch_id, unsigned long impid) > +{ > +#ifdef CONFIG_ERRATA_THEAD_PMU
Is there a reason that all the alternatives use ifdef rather than if(IS_ENABLED())?
> + if (arch_id != 0 || impid != 0) > + return false; > + > + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT) > + return false; > + > + return true; > +#else > + return false; > +#endif > +} > + > static u32 thead_errata_probe(unsigned int stage, > unsigned long archid, unsigned long impid) > { > @@ -55,6 +71,9 @@ static u32 thead_errata_probe(unsigned int stage, > if (errata_probe_cmo(stage, archid, impid)) > cpu_req_errata |= (1U << ERRATA_THEAD_CMO); > > + if (errata_probe_pmu(stage, archid, impid)) > + cpu_req_errata |= (1U << ERRATA_THEAD_PMU);
BIT(ERRATA_THEAD_PMU), no? Ditto for the CMO I guess..
> + > return cpu_req_errata; > } >
Thanks, Conor.
| |