Re: [v4 00/11] Improve RISC-V Perf support using SBI PMU and sscofpmf extension

Will Deacon
2021-12-14T18:09:26Z

On Mon, Dec 13, 2021 at 05:51:28PM -0800, Palmer Dabbelt wrote:
> On Mon, 25 Oct 2021 12:53:39 PDT (-0700), Atish Patra wrote:
> > This series adds improved perf support for RISC-V based system using
> > SBI PMU extension[1] and Sscofpmf extension[2]. The SBI PMU extension allows
> > the kernel to program the counters for different events and start/stop counters
> > while the sscofpmf extension allows the counter overflow interrupt and privilege
> > mode filtering. An hardware platform can leverage SBI PMU extension without
> > the sscofpmf extension if it supports mcountinhibit and mcounteren. However,
> > the reverse is not true. With both of these extension enabled, a platform can
> > take advantage of all both event counting and sampling using perf tool.
> >
> > This series introduces a platform perf driver instead of a existing arch
> > specific implementation. The new perf implementation has adopted a modular
> > approach where most of the generic event handling is done in the core library
> > while individual PMUs need to only implement necessary features specific to
> > the PMU. This is easily extensible and any future RISC-V PMU implementation
> > can leverage this. Currently, SBI PMU driver & legacy PMU driver are implemented
> > as a part of this series.
> >
> > The legacy driver tries to reimplement the existing minimal perf under a new
> > config to maintain backward compatibility. This implementation only allows
> > monitoring of always running cycle/instruction counters. Moreover, they can
> > not be started or stopped. In general, this is very limited and not very useful.
> > That's why, I am not very keen to carry the support into the new driver.
> > However, I don't want to break perf for any existing hardware platforms.
> > If everybody agrees that we don't need legacy perf implementation for older
> > implementation, I will be happy to drop PATCH 4.
>
> IMO we should keep it for a bit, so we have a transition period. These
> extensions are pretty new so we won't be able to count on everyone having
> them yet, this way we'll avoid breaking users.
>
> This generally looks good, but I don't see any Acks from the perf
> maintainers. I'm happy to take this through the RISC-V tree, but I'd
> generally like to have at least an ack as perf isn't really my subsystem.
> MAINTAINERS seems to indicate that's Will and Mark, they're not To'd so
> maybe they just missed this?
>
> I fixed a few trivial checkpatch warnings, updated Atish's email address,
> and put this on palmer/riscv-pmu. Happy to hear any comments, if nobody
> says anything then I'll just put that on riscv/for-next whenever I get back
> to my own email.

Fine by me! Most (all?) of the other drivers under drivers/perf/ are for
arm64, so I'm more than happy for you to handle the riscv one yourself.
If I end up with something that touches all of the drivers then we can
use a shared branch or something.

Will