Messages in this thread | | | Date | Fri, 20 Oct 2023 10:05:20 +0100 | From | Conor Dooley <> | Subject | Re: [RFC PATCH v2 07/10] perf: RISC-V: Move T-Head PMU to CPU feature alternative framework |
| |
On Fri, Oct 20, 2023 at 04:54:58PM +0800, Yu-Chien Peter Lin wrote: > On Thu, Oct 19, 2023 at 05:13:00PM +0100, Conor Dooley wrote: > > On Thu, Oct 19, 2023 at 10:01:19PM +0800, Yu Chien Peter Lin wrote: > > > > $subject: perf: RISC-V: Move T-Head PMU to CPU feature alternative framework > > > > IMO, this should be "RISC-V, perf:" or just "RISC-V" as the changes > > being made to the arch code are far more meaningful than those > > elsewhere. > > OK will update the subject to "RISC-V:" > > > > The custom PMU extension was developed to support perf event sampling > > > prior to the ratification of Sscofpmf. Instead of utilizing the standard > > > bits and CSR of Sscofpmf, a set of custom CSRs is added. So we may > > > consider it as a CPU feature rather than an erratum. > > > > > > T-Head cores need to append "xtheadpmu" to the riscv,isa-extensions > > > for each cpu node in device tree, and enable CONFIG_THEAD_CUSTOM_PMU > > > for proper functioning as of this commit. > > > > And in doing so, you regress break perf for existing DTs :( > > You didn't add the property to existing DTS in-kernel either, so if this > > series was applied, perf would just entirely stop working, no? > > Only `perf record/top` stop working I think. > > There are too many users out there, and don't have the boards to > test, so leave those DTS unchanged, it would be great if T-Head > community could help to check/update their DTS.
So, there are too many users to add xtheadpmu to the devicetrees, but not too many users to make changes that will cause a regression? I'm not following the logic here, sorry.
> > > Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com> > > > --- > > > Hi All, > > > > > > This is in preparation for introducing other PMU alternative. > > > We follow Conor's suggestion [1] to use cpu feature alternative > > > framework rather than errta, if you want to stick with errata > > > alternative or have other issues, please let me know. Thanks. > > > > Personally, I like this conversion, but it is going to regress support > > for perf on any T-Head cores which may be a bitter pill to get people to > > actually accept... > > Perhaps we could add this "improved" detection in parallel, and > > eventually remove the m*id based stuff in the future. > > > > > [1] https://patchwork.kernel.org/project/linux-riscv/patch/20230907021635.1002738-4-peterlin@andestech.com/#25503860 > > > > > > Changes v1 -> v2: > > > - New patch > > > ---
> > > @@ -805,7 +816,8 @@ static int pmu_sbi_setup_irqs(struct riscv_pmu *pmu, struct platform_device *pde > > > if (riscv_isa_extension_available(NULL, SSCOFPMF)) { > > > riscv_pmu_irq_num = RV_IRQ_PMU; > > > riscv_pmu_use_irq = true; > > > - } else if (IS_ENABLED(CONFIG_ERRATA_THEAD_PMU) && > > > + } else if (riscv_isa_extension_available(NULL, XTHEADPMU) && > > > + IS_ENABLED(CONFIG_THEAD_CUSTOM_PMU) && > > > riscv_cached_mvendorid(0) == THEAD_VENDOR_ID && > > > riscv_cached_marchid(0) == 0 && > > > riscv_cached_mimpid(0) == 0) { > > > > Can all of the m*id checks be removed, since the firmware is now > > explicitly telling us that the T-Head PMU is supported? > > I can only comfirm that boards with "allwinner,sun20i-d1" compatible > string uses the T-Head PMU device callbacks.
I'm not sure how that is an answer to my question.
Thanks, Conor. [unhandled content-type:application/pgp-signature] | |