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] [day] [month] [year] [list]
Message-ID: <20241028151725.GA2484@willie-the-truck>
Date: Mon, 28 Oct 2024 15:17:26 +0000
From: Will Deacon <will@...nel.org>
To: Besar Wicaksono <bwicaksono@...dia.com>
Cc: "suzuki.poulose@....com" <suzuki.poulose@....com>,
	"robin.murphy@....com" <robin.murphy@....com>,
	"catalin.marinas@....com" <catalin.marinas@....com>,
	"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>,
	Thierry Reding <treding@...dia.com>,
	Jon Hunter <jonathanh@...dia.com>, Vikram Sethi <vsethi@...dia.com>,
	Rich Wiley <rwiley@...dia.com>, Bob Knight <rknight@...dia.com>
Subject: Re: [PATCH 2/3] perf: arm_cspmu: nvidia: update CNVLINK PMU events

On Thu, Oct 24, 2024 at 02:10:55PM +0000, Besar Wicaksono wrote:
> > > > > diff --git a/drivers/perf/arm_cspmu/nvidia_cspmu.c
> > > > b/drivers/perf/arm_cspmu/nvidia_cspmu.c
> > > > > index ea2d44adfa7c..d1cd9975e71a 100644
> > > > > --- a/drivers/perf/arm_cspmu/nvidia_cspmu.c
> > > > > +++ b/drivers/perf/arm_cspmu/nvidia_cspmu.c
> > > > > @@ -112,6 +112,25 @@ static struct attribute *mcf_pmu_event_attrs[]
> > = {
> > > > >       NULL,
> > > > >  };
> > > > >
> > > > > +static struct attribute *mcf_cnvlink_pmu_event_attrs[] = {
> > > > > +     ARM_CSPMU_EVENT_ATTR(rd_bytes_cmem,                     0x0),
> > > > > +     ARM_CSPMU_EVENT_ATTR(rd_bytes_gmem,                     0x1),
> > > > > +     ARM_CSPMU_EVENT_ATTR(wr_bytes_cmem,                     0x2),
> > > > > +     ARM_CSPMU_EVENT_ATTR(wr_bytes_gmem,                     0x3),
> > > > > +     ARM_CSPMU_EVENT_ATTR(total_bytes_cmem,                  0x4),
> > > > > +     ARM_CSPMU_EVENT_ATTR(total_bytes_gmem,                  0x5),
> > > > > +     ARM_CSPMU_EVENT_ATTR(rd_req_cmem,                       0x6),
> > > > > +     ARM_CSPMU_EVENT_ATTR(rd_req_gmem,                       0x7),
> > > > > +     ARM_CSPMU_EVENT_ATTR(wr_req_cmem,                       0x8),
> > > > > +     ARM_CSPMU_EVENT_ATTR(wr_req_gmem,                       0x9),
> > > > > +     ARM_CSPMU_EVENT_ATTR(total_req_cmem,                    0xa),
> > > > > +     ARM_CSPMU_EVENT_ATTR(total_req_gmem,                    0xb),
> > > > > +     ARM_CSPMU_EVENT_ATTR(rd_cum_outs_cmem,                  0xc),
> > > > > +     ARM_CSPMU_EVENT_ATTR(rd_cum_outs_gmem,                  0xd),
> > > > > +     ARM_CSPMU_EVENT_ATTR(cycles,
> > > > ARM_CSPMU_EVT_CYCLES_DEFAULT),
> > > > > +     NULL,
> > > > > +};
> > > > > +
> > > > >  static struct attribute *generic_pmu_event_attrs[] = {
> > > > >       ARM_CSPMU_EVENT_ATTR(cycles,
> > > > ARM_CSPMU_EVT_CYCLES_DEFAULT),
> > > > >       NULL,
> > > > > @@ -234,7 +253,7 @@ static const struct nv_cspmu_match
> > > > nv_cspmu_match[] = {
> > > > >         .filter_default_val = NV_CNVL_FILTER_ID_MASK,
> > > > >         .name_pattern = "nvidia_cnvlink_pmu_%u",
> > > > >         .name_fmt = NAME_FMT_SOCKET,
> > > > > -       .event_attr = mcf_pmu_event_attrs,
> > > > > +       .event_attr = mcf_cnvlink_pmu_event_attrs,
> > > > >         .format_attr = cnvlink_pmu_format_attrs
> > > > >       },
> > > >
> > > > Hmm. Isn't this a user-visible change? For example, will scripts driving
> > > > 'perf' with the old event names continue to work after this patch?
> > > >
> > >
> > > Yes this is user visible. I am expecting user script to be updated accordingly.
> > > Would this be reasonable?
> > 
> > I don't think so, no. We don't tend to require userspace changes as a
> > result of upgrading the kernel.
> 
> Are you referring to userspace change just on the perf tool side?
> Cause this PMU doesn't have JSON scripts for alias/metric in the perf tool yet.

I'm not sure that matters, does it? If the mappings are exposed in sysfs,
then the tool will pick them up.

> Do you have suggestion of the proper approach?

I'd say leave the event names like they are and if you want to add aliases,
do that in userspace.

Will

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ