linux-kernel - Re: [PATCH 2/3] soc: sifive: Add SiFive private L2 cache 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]

Message-ID: <20230621-stupor-violin-930ccc7d7593@spud>
Date:   Wed, 21 Jun 2023 16:17:24 +0100
From:   Conor Dooley <conor@...nel.org>
To:     Eric Lin <eric.lin@...ive.com>
Cc:     Conor Dooley <conor.dooley@...rochip.com>, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, palmer@...belt.com,
        paul.walmsley@...ive.com, aou@...s.berkeley.edu, maz@...nel.org,
        chenhuacai@...nel.org, baolu.lu@...ux.intel.com, will@...nel.org,
        kan.liang@...ux.intel.com, nnac123@...ux.ibm.com,
        pierre.gondois@....com, jgross@...e.com, chao.gao@...el.com,
        maobibo@...ngson.cn, linux-riscv@...ts.infradead.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        dslin1010@...il.com, Greentime Hu <greentime.hu@...ive.com>,
        Zong Li <zong.li@...ive.com>, Nick Hu <nick.hu@...ive.com>,
        mark.rutland@....com, arnd@...db.de
Subject: Re: [PATCH 2/3] soc: sifive: Add SiFive private L2 cache PMU driver

Arnd, Perf people,

On Tue, Jun 20, 2023 at 11:14:32AM +0800, Eric Lin wrote:
> On Fri, Jun 16, 2023 at 6:13 PM Conor Dooley <conor.dooley@...rochip.com> wrote:
> >
> > On Fri, Jun 16, 2023 at 02:32:09PM +0800, Eric Lin wrote:
> > > From: Greentime Hu <greentime.hu@...ive.com>
> > >
> > > This adds SiFive private L2 cache PMU driver. User
> > > can use perf tool to profile by event name and event id.
> > >
> > > Example:
> > > $ perf stat -C 0 -e /sifive_pl2_pmu/inner_acquire_block_btot/
> > >                 -e /sifive_pl2_pmu/inner_acquire_block_ntob/
> > >                 -e /sifive_pl2_pmu/inner_acquire_block_ntot/ ls
> > >
> > >  Performance counter stats for 'CPU(s) 0':
> > >
> > >                300      sifive_pl2_pmu/inner_acquire_block_btot/
> > >              17801      sifive_pl2_pmu/inner_acquire_block_ntob/
> > >               5253      sifive_pl2_pmu/inner_acquire_block_ntot/
> > >
> > >        0.088917326 seconds time elapsed
> > >
> > > $ perf stat -C 0 -e /sifive_pl2_pmu/event=0x10001/
> > >                 -e /sifive_pl2_pmu/event=0x4001/
> > >                 -e /sifive_pl2_pmu/event=0x8001/ ls
> > >
> > >  Performance counter stats for 'CPU(s) 0':
> > >
> > >                251      sifive_pl2_pmu/event=0x10001/
> > >               2620      sifive_pl2_pmu/event=0x4001/
> > >                644      sifive_pl2_pmu/event=0x8001/
> > >
> > >        0.092827110 seconds time elapsed
> > >
> > > Signed-off-by: Greentime Hu <greentime.hu@...ive.com>
> > > Signed-off-by: Eric Lin <eric.lin@...ive.com>
> > > Reviewed-by: Zong Li <zong.li@...ive.com>
> > > Reviewed-by: Nick Hu <nick.hu@...ive.com>
> > > ---
> > >  drivers/soc/sifive/Kconfig            |   9 +
> > >  drivers/soc/sifive/Makefile           |   1 +
> > >  drivers/soc/sifive/sifive_pl2.h       |  20 +
> > >  drivers/soc/sifive/sifive_pl2_cache.c |  16 +
> > >  drivers/soc/sifive/sifive_pl2_pmu.c   | 669 ++++++++++++++++++++++++++
> >
> > Perf drivers should be in drivers/perf, no?
> >
> 
> Hi Conor,
> 
> Yes, I see most of the drivers are in the drivers/perf.
> 
> But I grep perf_pmu_register(), it seems not all the pmu drivers are
> in drivers/perf as below:
> 
> arch/arm/mach-imx/mmdc.c:517:   ret =
> perf_pmu_register(&(pmu_mmdc->pmu), name, -1);
> arch/arm/mm/cache-l2x0-pmu.c:552:       ret =
> perf_pmu_register(l2x0_pmu, l2x0_name, -1);
> ...
> drivers/dma/idxd/perfmon.c:627: rc = perf_pmu_register(&idxd_pmu->pmu,
> idxd_pmu->name, -1);
> drivers/fpga/dfl-fme-perf.c:904:static int
> fme_perf_pmu_register(struct platform_device *pdev,
> drivers/fpga/dfl-fme-perf.c:929:        ret = perf_pmu_register(pmu, name, -1);
> ...
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:549:    ret =
> perf_pmu_register(&pmu_entry->pmu, pmu_name, -1);
> drivers/gpu/drm/i915/i915_pmu.c:1190:   ret =
> perf_pmu_register(&pmu->base, pmu->name, -1);
> drivers/hwtracing/coresight/coresight-etm-perf.c:907:   ret =
> perf_pmu_register(&etm_pmu, CORESIGHT_ETM_PMU_NAME, -1);
> drivers/hwtracing/ptt/hisi_ptt.c:895:   ret =
> perf_pmu_register(&hisi_ptt->hisi_ptt_pmu, pmu_name, -1);
> drivers/iommu/intel/perfmon.c:570:      return
> perf_pmu_register(&iommu_pmu->pmu, iommu_pmu->pmu.name, -1);
> drivers/nvdimm/nd_perf.c:309:   rc = perf_pmu_register(&nd_pmu->pmu,
> nd_pmu->pmu.name, -1);
> ...
> 
> I just wondering what kind of pmu drivers should be in drivers/perf
> and what kind of pmu drivers should not be in drivers/perf.
> Thanks.

To be quite honest, I have no idea.
I'm just a wee bit wary of taking anything that appears to have another
home via drivers/soc. I'd rather break drivers out, using the aux bus or
similar if need be, so that people who are knowledgeable in an area are
CCed on patches.
Hopefully Arnd or the Perf people can offer some guidance here. If it
does go into drivers/soc, it'll need a review from someone knowledgeable
of perf anyway.

Cheers,
Conor.

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)