Messages in this thread | | | Date | Wed, 22 Nov 2023 19:43:09 -0600 | Subject | Re: [RFC PATCH v2 07/10] perf: RISC-V: Move T-Head PMU to CPU feature alternative framework | From | Samuel Holland <> |
| |
On 10/23/23 03:26, Conor Dooley wrote: > On Sun, Oct 22, 2023 at 05:09:09PM +0800, Yu-Chien Peter Lin wrote: >> On Fri, Oct 20, 2023 at 10:05:20AM +0100, Conor Dooley wrote: >>> 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. >> >> humm, I'll try. I assume that the sun20i-d1s.dtsi is all I need >> to update for T-Head PMU. > > I think you can actually add it to all users of T-Head CPUs currently in > mainline since all those cpus report the 0 mimpid and 0 marchid that is > being used as the detection method in the current code. > > That said, changing the in-kernel devicetrees doesn't solve the > regression problem. Not every dts lives in the linux codebase, for > example, and just because they don't, doesn't mean we can just not > care about them! > > As a result, I don't think that we can just do a conversion here from > one method to another like this, since it's likely to break things for > people. Certainly interested in hearing from those that support the > T-Head IP based SoCs about whether they'd be okay with something like > this.
PMU support is not required to boot, and it didn't really work correctly anyway until OpenSBI commit c9a296d0edc9 ("platform: generic: allwinner: fix OF process for T-HEAD c9xx pmu"), which is still not in any released OpenSBI version.
So I am fine with requiring a devicetree update for continued PMU support.
Regards, Samuel
| |