linux-kernel - RE: [PATCH 3/3] perf: arm_cspmu: nvidia: enable NVLINK-C2C port filtering

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:
 <SJ0PR12MB5676D1817E2C2F3693B82D4EA0452@SJ0PR12MB5676.namprd12.prod.outlook.com>
Date: Tue, 15 Oct 2024 17:29:28 +0000
From: Besar Wicaksono <bwicaksono@...dia.com>
To: Will Deacon <will@...nel.org>
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 3/3] perf: arm_cspmu: nvidia: enable NVLINK-C2C port
 filtering



> -----Original Message-----
> From: Will Deacon <will@...nel.org>
> Sent: Monday, October 14, 2024 8:29 AM
> To: Besar Wicaksono <bwicaksono@...dia.com>
> Cc: suzuki.poulose@....com; robin.murphy@....com;
> catalin.marinas@....com; mark.rutland@....com; linux-arm-
> kernel@...ts.infradead.org; linux-kernel@...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 3/3] perf: arm_cspmu: nvidia: enable NVLINK-C2C port
> filtering
> 
> External email: Use caution opening links or attachments
> 
> 
> On Wed, Sep 18, 2024 at 09:58:46PM +0000, Besar Wicaksono wrote:
> > Enable NVLINK-C2C port filtering to distinguish traffic from
> > different GPUs connected to NVLINK-C2C.
> >
> > Signed-off-by: Besar Wicaksono <bwicaksono@...dia.com>
> > ---
> >  Documentation/admin-guide/perf/nvidia-pmu.rst | 32
> +++++++++++++++++++
> >  drivers/perf/arm_cspmu/nvidia_cspmu.c         |  7 ++--
> >  2 files changed, 36 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/perf/nvidia-pmu.rst
> b/Documentation/admin-guide/perf/nvidia-pmu.rst
> > index 2e0d47cfe7ea..6d1d3206b4ad 100644
> > --- a/Documentation/admin-guide/perf/nvidia-pmu.rst
> > +++ b/Documentation/admin-guide/perf/nvidia-pmu.rst
> > @@ -86,6 +86,22 @@ Example usage:
> >
> >     perf stat -a -e nvidia_nvlink_c2c0_pmu_3/event=0x0/
> >
> > +The NVLink-C2C has two ports that can be connected to one GPU
> (occupying both
> > +ports) or to two GPUs (one GPU per port). The user can use "port" bitmap
> > +parameter to select the port(s) to monitor. Each bit represents the port
> number,
> > +e.g. "port=0x1" corresponds to port 0 and "port=0x3" is for port 0 and 1.
> The
> > +PMU will monitor both ports by default if not specified.
> > +
> > +Example for port filtering:
> > +
> > +* Count event id 0x0 from the GPU connected with socket 0 on port 0::
> > +
> > +   perf stat -a -e nvidia_nvlink_c2c0_pmu_0/event=0x0,port=0x1/
> > +
> > +* Count event id 0x0 from the GPUs connected with socket 0 on port 0 and
> port 1::
> > +
> > +   perf stat -a -e nvidia_nvlink_c2c0_pmu_0/event=0x0,port=0x3/
> > +
> >  NVLink-C2C1 PMU
> >  -------------------
> >
> > @@ -116,6 +132,22 @@ Example usage:
> >
> >     perf stat -a -e nvidia_nvlink_c2c1_pmu_3/event=0x0/
> >
> > +The NVLink-C2C has two ports that can be connected to one GPU
> (occupying both
> > +ports) or to two GPUs (one GPU per port). The user can use "port" bitmap
> > +parameter to select the port(s) to monitor. Each bit represents the port
> number,
> > +e.g. "port=0x1" corresponds to port 0 and "port=0x3" is for port 0 and 1.
> The
> > +PMU will monitor both ports by default if not specified.
> > +
> > +Example for port filtering:
> > +
> > +* Count event id 0x0 from the GPU connected with socket 0 on port 0::
> > +
> > +   perf stat -a -e nvidia_nvlink_c2c1_pmu_0/event=0x0,port=0x1/
> > +
> > +* Count event id 0x0 from the GPUs connected with socket 0 on port 0 and
> port 1::
> > +
> > +   perf stat -a -e nvidia_nvlink_c2c1_pmu_0/event=0x0,port=0x3/
> > +
> >  CNVLink PMU
> >  ---------------
> >
> > diff --git a/drivers/perf/arm_cspmu/nvidia_cspmu.c
> b/drivers/perf/arm_cspmu/nvidia_cspmu.c
> > index d1cd9975e71a..cd51177347e5 100644
> > --- a/drivers/perf/arm_cspmu/nvidia_cspmu.c
> > +++ b/drivers/perf/arm_cspmu/nvidia_cspmu.c
> > @@ -149,6 +149,7 @@ static struct attribute *pcie_pmu_format_attrs[] = {
> >
> >  static struct attribute *nvlink_c2c_pmu_format_attrs[] = {
> >       ARM_CSPMU_FORMAT_EVENT_ATTR,
> > +     ARM_CSPMU_FORMAT_ATTR(port, "config1:0-1"),
> >       NULL,
> >  };
> >
> > @@ -193,7 +194,7 @@ static u32 nv_cspmu_event_filter(const struct
> perf_event *event)
> >       const struct nv_cspmu_ctx *ctx =
> >               to_nv_cspmu_ctx(to_arm_cspmu(event->pmu));
> >
> > -     if (ctx->filter_mask == 0)
> > +     if (ctx->filter_mask == 0 || event->attr.config1 == 0)
> >               return ctx->filter_default_val;
> 
> Isn't this a bit too broad? It looks like this filter function is used
> beyond the C2C PMU (i.e. the PCIe PMU) and you're also checking the whole
> of config1 rather than just the port field.
> 

I think the other PMUs (PCIE and CNVLINK) that have similar filters will also benefit
from this change, since a filter value of 0 on these PMUs are meaningless. Should I
make the intention clearer by moving this particular change into a separate patch?

Thanks,
Besar