Re: [PATCH RFC 0/7] perf pmu-events: Support event aliasing for system PMUs

[Will Deacon]

On Tue, Feb 18, 2020 at 01:24:38PM +0000, John Garry wrote:
> On 18/02/2020 12:57, Will Deacon wrote:
> > On Fri, Jan 24, 2020 at 10:34:58PM +0800, John Garry wrote:
> > > Currently event aliasing for only CPU and uncore PMUs is supported. In
> > > fact, only uncore PMUs aliasing for when the uncore PMUs are fixed for a
> > > CPU is supported, which may not always be the case for certain
> > > architectures.
> > > 
> > > This series adds support for PMU event aliasing for system and other
> > > uncore PMUs which are not fixed to a specific CPU.
> > > 
> > > For this, we introduce support for another per-arch mapfile, which maps a
> > > particular system identifier to a set of system PMU events for that
> > > system. This is much the same as what we do for CPU event aliasing.
> > > 
> > > To support this, we need to change how we match a PMU to a mapfile,
> > > whether it should use a CPU or system mapfile. For this we do the
> > > following:
> > > 
> > > - For CPU PMU, we always match for the event mapfile based on the CPUID.
> > >    This has not changed.
> > > 
> > > - For an uncore or system PMU, we match first based on the SYSID (if set).
> > >    If this fails, then we match on the CPUID.
> > > 
> > >    This works for x86, as x86 would not have any system mapfiles for uncore
> > >    PMUs (and match on the CPUID).
> > > 
> > > Initial reference support is also added for ARM SMMUv3 PMCG (Performance
> > > Monitor Event Group) PMU for HiSilicon hip08 platform with only a single
> > > event so far - see driver in drivers/perf/arm_smmuv3_pmu.c for that driver.
> > 
> > Why don't we just expose SMMU_IIDR in the SMMUv3 PMU directory, so that
> > you can key off that?
> 
> That does not sound like a standard sysfs interface.

It's standard in the sense that PMUs already have their own directory under
sysfs where you can put things. For example, the "caps" directory is a
dumping ground for all sorts of PMU-specific information.

On the other hand, saying "please go figure out which SoC you're on"
certainly isn't standard and is likely to lead to unreliable, spaghetti
code.

> Anyway, I don't think that works for every case, quoting from
> https://lkml.org/lkml/2019/10/16/465:
> 
> "> Note: I do acknowledge that an overall issue is that we assume all PMCG
> IMP DEF events are same for a given SMMU model.
> 
> That assumption does technically fail already - I know MMU-600 has
> different IMP-DEF events for its TCU and TBUs, however as long as we can
> get as far as "this is some part of an MMU-600" the driver should be
> able to figure out the rest ..."

Perhaps I'm misreading this, but it sounds like if you knew it was an
MMU-600 then you'd be ok. I also don't understand how a SoC ID makes things
any easier in this regard.

> So even if it is solvable here, the kernel driver(s) will need to be
> reworked. And that is just solving one case in many.

PMU drivers will need to expose more information to userspace so that they
can be identified more precisely, yes. I wouldn't say they would need to be
"reworked".

> I'm nervous about coming up with a global "SYSID"
> > when we don't have the ability to standardise anything in that space.
> 
> I understand totally, especially if any sysid is based on DT bindings.

Well if this is going to be ACPI-only then it's a non-starter.

> But this is some sort of standardization:
> https://developer.arm.com/docs/den0028/c, see SMCCC_ARCH_SOC_ID

Yay, firmware :/

Even if this was widely implemented (it's not), I still think that it's
the wrong level of abstraction. Why not do away with ACPI/DT entirely
and predicate everything off the SoC ID?

Will