[PATCH 0/3] perf: arm_cspmu: nvidia: update event list and filter

[PATCH 0/3] perf: arm_cspmu: nvidia: update event list and filter

[Besar Wicaksono]

This series update the list of supported events and filters in NVIDIA Grace
CoreSight PMUs.

Besar Wicaksono (3):
  perf: arm_cspmu: nvidia: remove unsupported SCF events
  perf: arm_cspmu: nvidia: update CNVLINK PMU events
  perf: arm_cspmu: nvidia: enable NVLINK-C2C port filtering

 Documentation/admin-guide/perf/nvidia-pmu.rst | 32 +++++++
 drivers/perf/arm_cspmu/nvidia_cspmu.c         | 92 +++++--------------
 2 files changed, 56 insertions(+), 68 deletions(-)


base-commit: ad618736883b8970f66af799e34007475fe33a68
-- 
2.34.1

[PATCH 1/3] perf: arm_cspmu: nvidia: remove unsupported SCF events

[Besar Wicaksono]

Remove unsupported events under SCF PMU.

Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
---
 drivers/perf/arm_cspmu/nvidia_cspmu.c | 64 ---------------------------
 1 file changed, 64 deletions(-)

diff --git a/drivers/perf/arm_cspmu/nvidia_cspmu.c b/drivers/perf/arm_cspmu/nvidia_cspmu.c
index d0ef611240aa..ea2d44adfa7c 100644
--- a/drivers/perf/arm_cspmu/nvidia_cspmu.c
+++ b/drivers/perf/arm_cspmu/nvidia_cspmu.c
@@ -54,65 +54,24 @@ static struct attribute *scf_pmu_event_attrs[] = {
 	ARM_CSPMU_EVENT_ATTR(scf_cache_wb,			0xF3),
 
 	NV_CSPMU_EVENT_ATTR_4(socket, rd_data,			0x101),
-	NV_CSPMU_EVENT_ATTR_4(socket, dl_rsp,			0x105),
 	NV_CSPMU_EVENT_ATTR_4(socket, wb_data,			0x109),
-	NV_CSPMU_EVENT_ATTR_4(socket, ev_rsp,			0x10d),
-	NV_CSPMU_EVENT_ATTR_4(socket, prb_data,			0x111),
 
 	NV_CSPMU_EVENT_ATTR_4(socket, rd_outstanding,		0x115),
-	NV_CSPMU_EVENT_ATTR_4(socket, dl_outstanding,		0x119),
-	NV_CSPMU_EVENT_ATTR_4(socket, wb_outstanding,		0x11d),
-	NV_CSPMU_EVENT_ATTR_4(socket, wr_outstanding,		0x121),
-	NV_CSPMU_EVENT_ATTR_4(socket, ev_outstanding,		0x125),
-	NV_CSPMU_EVENT_ATTR_4(socket, prb_outstanding,		0x129),
 
 	NV_CSPMU_EVENT_ATTR_4(socket, rd_access,		0x12d),
-	NV_CSPMU_EVENT_ATTR_4(socket, dl_access,		0x131),
 	NV_CSPMU_EVENT_ATTR_4(socket, wb_access,		0x135),
 	NV_CSPMU_EVENT_ATTR_4(socket, wr_access,		0x139),
-	NV_CSPMU_EVENT_ATTR_4(socket, ev_access,		0x13d),
-	NV_CSPMU_EVENT_ATTR_4(socket, prb_access,		0x141),
-
-	NV_CSPMU_EVENT_ATTR_4(ocu, gmem_rd_data,		0x145),
-	NV_CSPMU_EVENT_ATTR_4(ocu, gmem_rd_access,		0x149),
-	NV_CSPMU_EVENT_ATTR_4(ocu, gmem_wb_access,		0x14d),
-	NV_CSPMU_EVENT_ATTR_4(ocu, gmem_rd_outstanding,		0x151),
-	NV_CSPMU_EVENT_ATTR_4(ocu, gmem_wr_outstanding,		0x155),
-
-	NV_CSPMU_EVENT_ATTR_4(ocu, rem_rd_data,			0x159),
-	NV_CSPMU_EVENT_ATTR_4(ocu, rem_rd_access,		0x15d),
-	NV_CSPMU_EVENT_ATTR_4(ocu, rem_wb_access,		0x161),
-	NV_CSPMU_EVENT_ATTR_4(ocu, rem_rd_outstanding,		0x165),
-	NV_CSPMU_EVENT_ATTR_4(ocu, rem_wr_outstanding,		0x169),
 
 	ARM_CSPMU_EVENT_ATTR(gmem_rd_data,			0x16d),
 	ARM_CSPMU_EVENT_ATTR(gmem_rd_access,			0x16e),
 	ARM_CSPMU_EVENT_ATTR(gmem_rd_outstanding,		0x16f),
-	ARM_CSPMU_EVENT_ATTR(gmem_dl_rsp,			0x170),
-	ARM_CSPMU_EVENT_ATTR(gmem_dl_access,			0x171),
-	ARM_CSPMU_EVENT_ATTR(gmem_dl_outstanding,		0x172),
 	ARM_CSPMU_EVENT_ATTR(gmem_wb_data,			0x173),
 	ARM_CSPMU_EVENT_ATTR(gmem_wb_access,			0x174),
-	ARM_CSPMU_EVENT_ATTR(gmem_wb_outstanding,		0x175),
-	ARM_CSPMU_EVENT_ATTR(gmem_ev_rsp,			0x176),
-	ARM_CSPMU_EVENT_ATTR(gmem_ev_access,			0x177),
-	ARM_CSPMU_EVENT_ATTR(gmem_ev_outstanding,		0x178),
 	ARM_CSPMU_EVENT_ATTR(gmem_wr_data,			0x179),
-	ARM_CSPMU_EVENT_ATTR(gmem_wr_outstanding,		0x17a),
 	ARM_CSPMU_EVENT_ATTR(gmem_wr_access,			0x17b),
 
 	NV_CSPMU_EVENT_ATTR_4(socket, wr_data,			0x17c),
 
-	NV_CSPMU_EVENT_ATTR_4(ocu, gmem_wr_data,		0x180),
-	NV_CSPMU_EVENT_ATTR_4(ocu, gmem_wb_data,		0x184),
-	NV_CSPMU_EVENT_ATTR_4(ocu, gmem_wr_access,		0x188),
-	NV_CSPMU_EVENT_ATTR_4(ocu, gmem_wb_outstanding,		0x18c),
-
-	NV_CSPMU_EVENT_ATTR_4(ocu, rem_wr_data,			0x190),
-	NV_CSPMU_EVENT_ATTR_4(ocu, rem_wb_data,			0x194),
-	NV_CSPMU_EVENT_ATTR_4(ocu, rem_wr_access,		0x198),
-	NV_CSPMU_EVENT_ATTR_4(ocu, rem_wb_outstanding,		0x19c),
-
 	ARM_CSPMU_EVENT_ATTR(gmem_wr_total_bytes,		0x1a0),
 	ARM_CSPMU_EVENT_ATTR(remote_socket_wr_total_bytes,	0x1a1),
 	ARM_CSPMU_EVENT_ATTR(remote_socket_rd_data,		0x1a2),
@@ -122,35 +81,12 @@ static struct attribute *scf_pmu_event_attrs[] = {
 	ARM_CSPMU_EVENT_ATTR(cmem_rd_data,			0x1a5),
 	ARM_CSPMU_EVENT_ATTR(cmem_rd_access,			0x1a6),
 	ARM_CSPMU_EVENT_ATTR(cmem_rd_outstanding,		0x1a7),
-	ARM_CSPMU_EVENT_ATTR(cmem_dl_rsp,			0x1a8),
-	ARM_CSPMU_EVENT_ATTR(cmem_dl_access,			0x1a9),
-	ARM_CSPMU_EVENT_ATTR(cmem_dl_outstanding,		0x1aa),
 	ARM_CSPMU_EVENT_ATTR(cmem_wb_data,			0x1ab),
 	ARM_CSPMU_EVENT_ATTR(cmem_wb_access,			0x1ac),
-	ARM_CSPMU_EVENT_ATTR(cmem_wb_outstanding,		0x1ad),
-	ARM_CSPMU_EVENT_ATTR(cmem_ev_rsp,			0x1ae),
-	ARM_CSPMU_EVENT_ATTR(cmem_ev_access,			0x1af),
-	ARM_CSPMU_EVENT_ATTR(cmem_ev_outstanding,		0x1b0),
 	ARM_CSPMU_EVENT_ATTR(cmem_wr_data,			0x1b1),
-	ARM_CSPMU_EVENT_ATTR(cmem_wr_outstanding,		0x1b2),
-
-	NV_CSPMU_EVENT_ATTR_4(ocu, cmem_rd_data,		0x1b3),
-	NV_CSPMU_EVENT_ATTR_4(ocu, cmem_rd_access,		0x1b7),
-	NV_CSPMU_EVENT_ATTR_4(ocu, cmem_wb_access,		0x1bb),
-	NV_CSPMU_EVENT_ATTR_4(ocu, cmem_rd_outstanding,		0x1bf),
-	NV_CSPMU_EVENT_ATTR_4(ocu, cmem_wr_outstanding,		0x1c3),
-
-	ARM_CSPMU_EVENT_ATTR(ocu_prb_access,			0x1c7),
-	ARM_CSPMU_EVENT_ATTR(ocu_prb_data,			0x1c8),
-	ARM_CSPMU_EVENT_ATTR(ocu_prb_outstanding,		0x1c9),
 
 	ARM_CSPMU_EVENT_ATTR(cmem_wr_access,			0x1ca),
 
-	NV_CSPMU_EVENT_ATTR_4(ocu, cmem_wr_access,		0x1cb),
-	NV_CSPMU_EVENT_ATTR_4(ocu, cmem_wb_data,		0x1cf),
-	NV_CSPMU_EVENT_ATTR_4(ocu, cmem_wr_data,		0x1d3),
-	NV_CSPMU_EVENT_ATTR_4(ocu, cmem_wb_outstanding,		0x1d7),
-
 	ARM_CSPMU_EVENT_ATTR(cmem_wr_total_bytes,		0x1db),
 
 	ARM_CSPMU_EVENT_ATTR(cycles, ARM_CSPMU_EVT_CYCLES_DEFAULT),
-- 
2.34.1

[PATCH 2/3] perf: arm_cspmu: nvidia: update CNVLINK PMU events

[Besar Wicaksono]

Rename loc* and rem* events in CNVLINK PMU to cmem* and gmem* events.

Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
---
 drivers/perf/arm_cspmu/nvidia_cspmu.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

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
 	},
 	{
-- 
2.34.1

Re: [PATCH 2/3] perf: arm_cspmu: nvidia: update CNVLINK PMU events

[Will Deacon]

On Wed, Sep 18, 2024 at 09:58:45PM +0000, Besar Wicaksono wrote:
> Rename loc* and rem* events in CNVLINK PMU to cmem* and gmem* events.
> 
> Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
> ---
>  drivers/perf/arm_cspmu/nvidia_cspmu.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> 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?

Will

RE: [PATCH 2/3] perf: arm_cspmu: nvidia: update CNVLINK PMU events

[Besar Wicaksono]

> -----Original Message-----
> From: Will Deacon <will@kernel.org>
> Sent: Monday, October 14, 2024 8:19 AM
> To: Besar Wicaksono <bwicaksono@nvidia.com>
> Cc: suzuki.poulose@arm.com; robin.murphy@arm.com;
> catalin.marinas@arm.com; mark.rutland@arm.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> tegra@vger.kernel.org; Thierry Reding <treding@nvidia.com>; Jon Hunter
> <jonathanh@nvidia.com>; Vikram Sethi <vsethi@nvidia.com>; Rich Wiley
> <rwiley@nvidia.com>; Bob Knight <rknight@nvidia.com>
> Subject: Re: [PATCH 2/3] perf: arm_cspmu: nvidia: update CNVLINK PMU
> events
> 
> External email: Use caution opening links or attachments
> 
> 
> On Wed, Sep 18, 2024 at 09:58:45PM +0000, Besar Wicaksono wrote:
> > Rename loc* and rem* events in CNVLINK PMU to cmem* and gmem*
> events.
> >
> > Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
> > ---
> >  drivers/perf/arm_cspmu/nvidia_cspmu.c | 21 ++++++++++++++++++++-
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > 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?

Regards,
Besar

Re: [PATCH 2/3] perf: arm_cspmu: nvidia: update CNVLINK PMU events

[Will Deacon]

On Tue, Oct 15, 2024 at 05:21:06PM +0000, Besar Wicaksono wrote:
> 
> 
> > -----Original Message-----
> > From: Will Deacon <will@kernel.org>
> > Sent: Monday, October 14, 2024 8:19 AM
> > To: Besar Wicaksono <bwicaksono@nvidia.com>
> > Cc: suzuki.poulose@arm.com; robin.murphy@arm.com;
> > catalin.marinas@arm.com; mark.rutland@arm.com; linux-arm-
> > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> > tegra@vger.kernel.org; Thierry Reding <treding@nvidia.com>; Jon Hunter
> > <jonathanh@nvidia.com>; Vikram Sethi <vsethi@nvidia.com>; Rich Wiley
> > <rwiley@nvidia.com>; Bob Knight <rknight@nvidia.com>
> > Subject: Re: [PATCH 2/3] perf: arm_cspmu: nvidia: update CNVLINK PMU
> > events
> > 
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Wed, Sep 18, 2024 at 09:58:45PM +0000, Besar Wicaksono wrote:
> > > Rename loc* and rem* events in CNVLINK PMU to cmem* and gmem*
> > events.
> > >
> > > Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
> > > ---
> > >  drivers/perf/arm_cspmu/nvidia_cspmu.c | 21 ++++++++++++++++++++-
> > >  1 file changed, 20 insertions(+), 1 deletion(-)
> > >
> > > 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.

Will

RE: [PATCH 2/3] perf: arm_cspmu: nvidia: update CNVLINK PMU events

[Besar Wicaksono]

> -----Original Message-----
> From: Will Deacon <will@kernel.org>
> Sent: Wednesday, October 23, 2024 11:27 AM
> To: Besar Wicaksono <bwicaksono@nvidia.com>
> Cc: suzuki.poulose@arm.com; robin.murphy@arm.com;
> catalin.marinas@arm.com; mark.rutland@arm.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> tegra@vger.kernel.org; Thierry Reding <treding@nvidia.com>; Jon Hunter
> <jonathanh@nvidia.com>; Vikram Sethi <vsethi@nvidia.com>; Rich Wiley
> <rwiley@nvidia.com>; Bob Knight <rknight@nvidia.com>
> Subject: Re: [PATCH 2/3] perf: arm_cspmu: nvidia: update CNVLINK PMU
> events
> 
> External email: Use caution opening links or attachments
> 
> 
> On Tue, Oct 15, 2024 at 05:21:06PM +0000, Besar Wicaksono wrote:
> >
> >
> > > -----Original Message-----
> > > From: Will Deacon <will@kernel.org>
> > > Sent: Monday, October 14, 2024 8:19 AM
> > > To: Besar Wicaksono <bwicaksono@nvidia.com>
> > > Cc: suzuki.poulose@arm.com; robin.murphy@arm.com;
> > > catalin.marinas@arm.com; mark.rutland@arm.com; linux-arm-
> > > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> > > tegra@vger.kernel.org; Thierry Reding <treding@nvidia.com>; Jon Hunter
> > > <jonathanh@nvidia.com>; Vikram Sethi <vsethi@nvidia.com>; Rich Wiley
> > > <rwiley@nvidia.com>; Bob Knight <rknight@nvidia.com>
> > > Subject: Re: [PATCH 2/3] perf: arm_cspmu: nvidia: update CNVLINK PMU
> > > events
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > On Wed, Sep 18, 2024 at 09:58:45PM +0000, Besar Wicaksono wrote:
> > > > Rename loc* and rem* events in CNVLINK PMU to cmem* and gmem*
> > > events.
> > > >
> > > > Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
> > > > ---
> > > >  drivers/perf/arm_cspmu/nvidia_cspmu.c | 21
> ++++++++++++++++++++-
> > > >  1 file changed, 20 insertions(+), 1 deletion(-)
> > > >
> > > > 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.

Do you have suggestion of the proper approach?

Thanks,
Besar

Re: [PATCH 2/3] perf: arm_cspmu: nvidia: update CNVLINK PMU events

[Will Deacon]

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

RE: [PATCH 2/3] perf: arm_cspmu: nvidia: update CNVLINK PMU events

[Besar Wicaksono]

> -----Original Message-----
> From: Will Deacon <will@kernel.org>
> Sent: Monday, October 28, 2024 10:17 AM
> To: Besar Wicaksono <bwicaksono@nvidia.com>
> Cc: suzuki.poulose@arm.com; robin.murphy@arm.com;
> catalin.marinas@arm.com; mark.rutland@arm.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> tegra@vger.kernel.org; Thierry Reding <treding@nvidia.com>; Jon Hunter
> <jonathanh@nvidia.com>; Vikram Sethi <vsethi@nvidia.com>; Rich Wiley
> <rwiley@nvidia.com>; Bob Knight <rknight@nvidia.com>
> Subject: Re: [PATCH 2/3] perf: arm_cspmu: nvidia: update CNVLINK PMU
> events
> 
> External email: Use caution opening links or attachments
> 
> 
> 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.

Hi Will,

Could you please review the V2 patches in https://lore.kernel.org/linux-arm-kernel/20241031142118.1865965-1-bwicaksono@nvidia.com/T/#t?

Thanks,
Besar

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

[Besar Wicaksono]

Enable NVLINK-C2C port filtering to distinguish traffic from
different GPUs connected to NVLINK-C2C.

Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.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;
 
 	return event->attr.config1 & ctx->filter_mask;
@@ -229,7 +230,7 @@ static const struct nv_cspmu_match nv_cspmu_match[] = {
 	{
 	  .prodid = 0x104,
 	  .prodid_mask = NV_PRODID_MASK,
-	  .filter_mask = 0x0,
+	  .filter_mask = NV_NVL_C2C_FILTER_ID_MASK,
 	  .filter_default_val = NV_NVL_C2C_FILTER_ID_MASK,
 	  .name_pattern = "nvidia_nvlink_c2c1_pmu_%u",
 	  .name_fmt = NAME_FMT_SOCKET,
@@ -239,7 +240,7 @@ static const struct nv_cspmu_match nv_cspmu_match[] = {
 	{
 	  .prodid = 0x105,
 	  .prodid_mask = NV_PRODID_MASK,
-	  .filter_mask = 0x0,
+	  .filter_mask = NV_NVL_C2C_FILTER_ID_MASK,
 	  .filter_default_val = NV_NVL_C2C_FILTER_ID_MASK,
 	  .name_pattern = "nvidia_nvlink_c2c0_pmu_%u",
 	  .name_fmt = NAME_FMT_SOCKET,
-- 
2.34.1

Re: [PATCH 3/3] perf: arm_cspmu: nvidia: enable NVLINK-C2C port filtering

[Will Deacon]

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@nvidia.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.

Will

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

[Besar Wicaksono]

> -----Original Message-----
> From: Will Deacon <will@kernel.org>
> Sent: Monday, October 14, 2024 8:29 AM
> To: Besar Wicaksono <bwicaksono@nvidia.com>
> Cc: suzuki.poulose@arm.com; robin.murphy@arm.com;
> catalin.marinas@arm.com; mark.rutland@arm.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> tegra@vger.kernel.org; Thierry Reding <treding@nvidia.com>; Jon Hunter
> <jonathanh@nvidia.com>; Vikram Sethi <vsethi@nvidia.com>; Rich Wiley
> <rwiley@nvidia.com>; Bob Knight <rknight@nvidia.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@nvidia.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

Re: [PATCH 3/3] perf: arm_cspmu: nvidia: enable NVLINK-C2C port filtering

[Will Deacon]

On Tue, Oct 15, 2024 at 05:29:28PM +0000, Besar Wicaksono wrote:
> > > 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?

Yes. If you want to change the behaviour for other PMUs, then please be
explicit about that. In any case, I don't think you should check the whole
of config1.

Will