linux-kernel - RE: [RESEND PATCH v3 1/2] perf: coresight_pmu: Add support for ARM CoreSight PMU driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 Jul 2022 04:47:26 +0000
From:   Besar Wicaksono <bwicaksono@...dia.com>
To:     Robin Murphy <robin.murphy@....com>,
        Mathieu Poirier <mathieu.poirier@...aro.org>
CC:     Suzuki K Poulose <suzuki.poulose@....com>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "will@...nel.org" <will@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "sudeep.holla@....com" <sudeep.holla@....com>,
        "thanu.rangarajan@....com" <thanu.rangarajan@....com>,
        "Michael.Williams@....com" <Michael.Williams@....com>,
        Thierry Reding <treding@...dia.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Vikram Sethi <vsethi@...dia.com>,
        "mike.leach@...aro.org" <mike.leach@...aro.org>,
        "leo.yan@...aro.org" <leo.yan@...aro.org>
Subject: RE: [RESEND PATCH v3 1/2] perf: coresight_pmu: Add support for ARM
 CoreSight PMU driver



> -----Original Message-----
> From: Robin Murphy <robin.murphy@....com>
> Sent: Wednesday, July 13, 2022 3:13 AM
> To: Mathieu Poirier <mathieu.poirier@...aro.org>; Besar Wicaksono
> <bwicaksono@...dia.com>
> Cc: Suzuki K Poulose <suzuki.poulose@....com>; catalin.marinas@....com;
> will@...nel.org; mark.rutland@....com; linux-arm-
> kernel@...ts.infradead.org; linux-kernel@...r.kernel.org; linux-
> tegra@...r.kernel.org; sudeep.holla@....com;
> thanu.rangarajan@....com; Michael.Williams@....com; Thierry Reding
> <treding@...dia.com>; Jonathan Hunter <jonathanh@...dia.com>; Vikram
> Sethi <vsethi@...dia.com>; mike.leach@...aro.org; leo.yan@...aro.org
> Subject: Re: [RESEND PATCH v3 1/2] perf: coresight_pmu: Add support for
> ARM CoreSight PMU driver
> 
> External email: Use caution opening links or attachments
> 
> 
> On 2022-07-12 17:36, Mathieu Poirier wrote:
> [...]
> >>> If we have decied to call this arm_system_pmu, (which I am perfectly
> >>> happy with), could we please stick to that name for functions that we
> >>> export ?
> >>>
> >>> e.g,
> >>>
> s/coresight_pmu_sysfs_event_show/arm_system_pmu_event_show()/
> >>>
> >>
> >> Just want to confirm, is it just the public functions or do we need to
> replace
> >> all that has "coresight" naming ? Including the static functions, structs,
> filename.
> >
> > I think all references to "coresight" should be changed to
> "arm_system_pmu",
> > including filenames.  That way there is no doubt this IP block is not
> > related, and does not interoperate, with the any of the "coresight" IP
> blocks
> > already supported[1] in the kernel.
> >
> > I have looked at the documentation[2] in the cover letter and I agree
> > with an earlier comment from Sudeep that this IP has very little to do with
> any
> > of the other CoreSight IP blocks found in the CoreSight framework[1].
> Using the
> > "coresight" naming convention in this driver would be _extremely_
> confusing,
> > especially when it comes to exported functions.
> 
> But conversely, how is it not confusing to make up completely different
> names for things than what they're actually called? The CoreSight
> Performance Monitoring Unit is a part of the Arm CoreSight architecture,
> it says it right there on page 1. What if I instinctively associate the
> name Mathieu with someone more familiar to me, so to avoid confusion I'd
> prefer to call you Steve? Is that OK?
> 

What is the naming convention for modules under drivers/perf ?
In my observation, the names there correspond to the part monitored by
the PMU. The confusion on using "coresight_pmu" naming could be that
people may think the PMU monitors coresight system, i.e the trace system under hwtracing.
However, the driver in this patch is for a new PMU standard that monitors uncore
parts. Uncore was considered as terminology from Intel, so "system" was picked instead.
Please see this thread for reference:
  https://lore.kernel.org/linux-arm-kernel/20220510111318.GD27557@willie-the-truck/

> As it happens, Steve, I do actually agree with you that "coresight_" is
> a bad prefix here, but only for the reason that it's too general. TBH I
> think that's true of the existing Linux subsystem too, but that damage
> is already done, and I'd concur that there's little value in trying to
> unpick that now, despite the clear existence of products like CoreSight
> DAP and CoreSight ELA which don't have all that much to do with program
> trace either.
> 
> However, hindsight and inertia are hardly good reasons to double down on
> poor decisions, so if I was going to vote for anything here it would be
> "cspmu_", which is about as
> obviously-related-to-the-thing-it-actually-is as we can get while also
> being pleasantly concise.
> 
> [ And no, this isn't bikeshedding. Naming things right is *important* ]
> 

I agree having the correct name is important, especially at this early stage.
A direction of what the naming should describe would be very helpful here.

> Cheers,
> Robin.
> 
> >
> > Thanks,
> > Steve
> >
> > [1]. drivers/hwtracing/coresight/
> > [2]. https://developer.arm.com/documentation/ihi0091/latest