diff mbox series

[v2] perf/cxlpmu: Support missing events in 3.1 spec

Message ID 20240930203445.149954-1-dave@stgolabs.net
State Superseded
Headers show
Series [v2] perf/cxlpmu: Support missing events in 3.1 spec | expand

Commit Message

Davidlohr Bueso Sept. 30, 2024, 8:34 p.m. UTC 
Update the CXL PMU driver to support the new events introduced
in the latest revision. These are:

- read/write accesses with TEE constraints.
- S2M indicating Modified state.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---

Changes from v1:
 - only update spec references of groups that are affected (Alison, Dave).

 drivers/perf/cxl_pmu.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Jonathan Cameron Oct. 4, 2024, 11:44 a.m. UTC | #1 
On Mon, 30 Sep 2024 13:34:45 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> Update the CXL PMU driver to support the new events introduced
> in the latest revision. These are:
> 
> - read/write accesses with TEE constraints.
> - S2M indicating Modified state.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Driver is in perf though so you need to +CC perf maintainers as
they will probably pick this up directly.

Jonathan

> ---
> 
> Changes from v1:
>  - only update spec references of groups that are affected (Alison, Dave).
> 
>  drivers/perf/cxl_pmu.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 43d68b69e630..644caf039718 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -377,12 +377,14 @@ static struct attribute *cxl_pmu_event_attrs[] = {
>  	/* CXL rev 3.0 Table 13-5 directly lists these */
>  	CXL_PMU_EVENT_CXL_ATTR(cachedata_d2h_data,		CXL_PMU_GID_CACHE_DATA, BIT(0)),
>  	CXL_PMU_EVENT_CXL_ATTR(cachedata_h2d_data,		CXL_PMU_GID_CACHE_DATA, BIT(1)),
> -	/* CXL rev 3.0 Table 3-29 M2S Req Memory Opcodes */
> +	/* CXL rev 3.1 Table 3-35 M2S Req Memory Opcodes */
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_meminv,			CXL_PMU_GID_M2S_REQ, BIT(0)),
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrd,			CXL_PMU_GID_M2S_REQ, BIT(1)),
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrddata,		CXL_PMU_GID_M2S_REQ, BIT(2)),
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrdfwd,		CXL_PMU_GID_M2S_REQ, BIT(3)),
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memwrfwd,		CXL_PMU_GID_M2S_REQ, BIT(4)),
> +	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrdtee,		CXL_PMU_GID_M2S_REQ, BIT(5)),
> +	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrddatatee,		CXL_PMU_GID_M2S_REQ, BIT(6)),
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memspecrd,		CXL_PMU_GID_M2S_REQ, BIT(8)),
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_meminvnt,		CXL_PMU_GID_M2S_REQ, BIT(9)),
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memcleanevict,		CXL_PMU_GID_M2S_REQ, BIT(10)),
> @@ -404,10 +406,11 @@ static struct attribute *cxl_pmu_event_attrs[] = {
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_curblk,		CXL_PMU_GID_S2M_BISNP, BIT(4)),
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_datblk,		CXL_PMU_GID_S2M_BISNP, BIT(5)),
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_invblk,		CXL_PMU_GID_S2M_BISNP, BIT(6)),
> -	/* CXL rev 3.0 Table 3-43 S2M NDR Opcopdes */
> +	/* CXL rev 3.1 Table 3-50 S2M NDR Opcopdes */
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmp,			CXL_PMU_GID_S2M_NDR, BIT(0)),
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmps,			CXL_PMU_GID_S2M_NDR, BIT(1)),
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmpe,			CXL_PMU_GID_S2M_NDR, BIT(2)),
> +	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmpm,			CXL_PMU_GID_S2M_NDR, BIT(3)),
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_biconflictack,		CXL_PMU_GID_S2M_NDR, BIT(4)),
>  	/* CXL rev 3.0 Table 3-46 S2M DRS opcodes */
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_drs_memdata,			CXL_PMU_GID_S2M_DRS, BIT(0)),
Alison Schofield Oct. 4, 2024, 4:43 p.m. UTC | #2 
On Mon, Sep 30, 2024 at 01:34:45PM -0700, Davidlohr Bueso wrote:
> Update the CXL PMU driver to support the new events introduced
> in the latest revision. These are:
> 
> - read/write accesses with TEE constraints.
> - S2M indicating Modified state.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
> 
> Changes from v1:
>  - only update spec references of groups that are affected (Alison, Dave).
> 

Typo in a comment s/Opcopdes/Opcodes may be fixed-up upon applying.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

>  drivers/perf/cxl_pmu.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 43d68b69e630..644caf039718 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -377,12 +377,14 @@ static struct attribute *cxl_pmu_event_attrs[] = {
>  	/* CXL rev 3.0 Table 13-5 directly lists these */
>  	CXL_PMU_EVENT_CXL_ATTR(cachedata_d2h_data,		CXL_PMU_GID_CACHE_DATA, BIT(0)),
>  	CXL_PMU_EVENT_CXL_ATTR(cachedata_h2d_data,		CXL_PMU_GID_CACHE_DATA, BIT(1)),
> -	/* CXL rev 3.0 Table 3-29 M2S Req Memory Opcodes */
> +	/* CXL rev 3.1 Table 3-35 M2S Req Memory Opcodes */
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_meminv,			CXL_PMU_GID_M2S_REQ, BIT(0)),
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrd,			CXL_PMU_GID_M2S_REQ, BIT(1)),
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrddata,		CXL_PMU_GID_M2S_REQ, BIT(2)),
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrdfwd,		CXL_PMU_GID_M2S_REQ, BIT(3)),
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memwrfwd,		CXL_PMU_GID_M2S_REQ, BIT(4)),
> +	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrdtee,		CXL_PMU_GID_M2S_REQ, BIT(5)),
> +	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrddatatee,		CXL_PMU_GID_M2S_REQ, BIT(6)),
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memspecrd,		CXL_PMU_GID_M2S_REQ, BIT(8)),
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_meminvnt,		CXL_PMU_GID_M2S_REQ, BIT(9)),
>  	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memcleanevict,		CXL_PMU_GID_M2S_REQ, BIT(10)),
> @@ -404,10 +406,11 @@ static struct attribute *cxl_pmu_event_attrs[] = {
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_curblk,		CXL_PMU_GID_S2M_BISNP, BIT(4)),
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_datblk,		CXL_PMU_GID_S2M_BISNP, BIT(5)),
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_invblk,		CXL_PMU_GID_S2M_BISNP, BIT(6)),
> -	/* CXL rev 3.0 Table 3-43 S2M NDR Opcopdes */
> +	/* CXL rev 3.1 Table 3-50 S2M NDR Opcopdes */
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmp,			CXL_PMU_GID_S2M_NDR, BIT(0)),
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmps,			CXL_PMU_GID_S2M_NDR, BIT(1)),
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmpe,			CXL_PMU_GID_S2M_NDR, BIT(2)),
> +	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmpm,			CXL_PMU_GID_S2M_NDR, BIT(3)),
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_biconflictack,		CXL_PMU_GID_S2M_NDR, BIT(4)),
>  	/* CXL rev 3.0 Table 3-46 S2M DRS opcodes */
>  	CXL_PMU_EVENT_CXL_ATTR(s2m_drs_memdata,			CXL_PMU_GID_S2M_DRS, BIT(0)),
> -- 
> 2.46.1
>
Davidlohr Bueso Oct. 10, 2024, 2:45 a.m. UTC | #3 
On Fri, 04 Oct 2024, Jonathan Cameron wrote:\n

>Driver is in perf though so you need to +CC perf maintainers as
>they will probably pick this up directly.

Doing the v3 to add the typo Alison pointed out, I remembered
why I didn't Cc a perf list. Which is it? it is not clear to me
exactly who is responsible to pick this up outside of the cxl
world:

$ ./scripts/get_maintainer.pl 0001-perf-cxlpmu-Support-missing-events-in-3.1-spec.patch
Jonathan Cameron <jonathan.cameron@huawei.com> (maintainer:COMPUTE EXPRESS LINK PMU (CPMU))
Will Deacon <will@kernel.org> (maintainer:ARM PMU PROFILING AND DEBUGGING)
Mark Rutland <mark.rutland@arm.com> (maintainer:ARM PMU PROFILING AND DEBUGGING)
linux-cxl@vger.kernel.org (open list:COMPUTE EXPRESS LINK PMU (CPMU))
linux-arm-kernel@lists.infradead.org (moderated list:ARM PMU PROFILING AND DEBUGGING)
linux-kernel@vger.kernel.org (open list)

... Yeah I guess it's Will and Mark, but arm seems really
misleading and out of place for this. Imo both the cxl driver
and pmu code should have the same maintainer scope.

But even if this does not change, I think the maintainer entries
should at least be labeled something more generic.

Thanks,
Davidlohr
Jonathan Cameron Oct. 16, 2024, 3:01 p.m. UTC | #4 
On Wed, 9 Oct 2024 19:45:05 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> On Fri, 04 Oct 2024, Jonathan Cameron wrote:\n
> 
> >Driver is in perf though so you need to +CC perf maintainers as
> >they will probably pick this up directly.  
> 
> Doing the v3 to add the typo Alison pointed out, I remembered
> why I didn't Cc a perf list. Which is it? it is not clear to me
> exactly who is responsible to pick this up outside of the cxl
> world:
> 
> $ ./scripts/get_maintainer.pl 0001-perf-cxlpmu-Support-missing-events-in-3.1-spec.patch
> Jonathan Cameron <jonathan.cameron@huawei.com> (maintainer:COMPUTE EXPRESS LINK PMU (CPMU))
> Will Deacon <will@kernel.org> (maintainer:ARM PMU PROFILING AND DEBUGGING)
> Mark Rutland <mark.rutland@arm.com> (maintainer:ARM PMU PROFILING AND DEBUGGING)
> linux-cxl@vger.kernel.org (open list:COMPUTE EXPRESS LINK PMU (CPMU))
> linux-arm-kernel@lists.infradead.org (moderated list:ARM PMU PROFILING AND DEBUGGING)
> linux-kernel@vger.kernel.org (open list)
> 
> ... Yeah I guess it's Will and Mark, but arm seems really
> misleading and out of place for this. Imo both the cxl driver
> and pmu code should have the same maintainer scope.

There was a discussion ages ago about changing that maintainer entry
to make it more generic (drivers/perf).  With a few exceptions the reality
is that Will and Mark end up reviewing everything in there :)

Generally for PMU drivers the perf specific aspects end up more complex
than the hardware side. That's why we put the driver in drivers/perf.

I don't think either Mark / Will / CXL maintainers really care a lot
about who does the patch wrangling on simple patches like this beyond
if it is from perf side no one wonders what the CXL maintainers are doing
touching code in drivers/perf.

Jonathan


> 
> But even if this does not change, I think the maintainer entries
> should at least be labeled something more generic.
> 
> Thanks,
> Davidlohr
diff mbox series

Patch

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 43d68b69e630..644caf039718 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -377,12 +377,14 @@  static struct attribute *cxl_pmu_event_attrs[] = {
 	/* CXL rev 3.0 Table 13-5 directly lists these */
 	CXL_PMU_EVENT_CXL_ATTR(cachedata_d2h_data,		CXL_PMU_GID_CACHE_DATA, BIT(0)),
 	CXL_PMU_EVENT_CXL_ATTR(cachedata_h2d_data,		CXL_PMU_GID_CACHE_DATA, BIT(1)),
-	/* CXL rev 3.0 Table 3-29 M2S Req Memory Opcodes */
+	/* CXL rev 3.1 Table 3-35 M2S Req Memory Opcodes */
 	CXL_PMU_EVENT_CXL_ATTR(m2s_req_meminv,			CXL_PMU_GID_M2S_REQ, BIT(0)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrd,			CXL_PMU_GID_M2S_REQ, BIT(1)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrddata,		CXL_PMU_GID_M2S_REQ, BIT(2)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrdfwd,		CXL_PMU_GID_M2S_REQ, BIT(3)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memwrfwd,		CXL_PMU_GID_M2S_REQ, BIT(4)),
+	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrdtee,		CXL_PMU_GID_M2S_REQ, BIT(5)),
+	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memrddatatee,		CXL_PMU_GID_M2S_REQ, BIT(6)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memspecrd,		CXL_PMU_GID_M2S_REQ, BIT(8)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_req_meminvnt,		CXL_PMU_GID_M2S_REQ, BIT(9)),
 	CXL_PMU_EVENT_CXL_ATTR(m2s_req_memcleanevict,		CXL_PMU_GID_M2S_REQ, BIT(10)),
@@ -404,10 +406,11 @@  static struct attribute *cxl_pmu_event_attrs[] = {
 	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_curblk,		CXL_PMU_GID_S2M_BISNP, BIT(4)),
 	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_datblk,		CXL_PMU_GID_S2M_BISNP, BIT(5)),
 	CXL_PMU_EVENT_CXL_ATTR(s2m_bisnp_invblk,		CXL_PMU_GID_S2M_BISNP, BIT(6)),
-	/* CXL rev 3.0 Table 3-43 S2M NDR Opcopdes */
+	/* CXL rev 3.1 Table 3-50 S2M NDR Opcopdes */
 	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmp,			CXL_PMU_GID_S2M_NDR, BIT(0)),
 	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmps,			CXL_PMU_GID_S2M_NDR, BIT(1)),
 	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmpe,			CXL_PMU_GID_S2M_NDR, BIT(2)),
+	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_cmpm,			CXL_PMU_GID_S2M_NDR, BIT(3)),
 	CXL_PMU_EVENT_CXL_ATTR(s2m_ndr_biconflictack,		CXL_PMU_GID_S2M_NDR, BIT(4)),
 	/* CXL rev 3.0 Table 3-46 S2M DRS opcodes */
 	CXL_PMU_EVENT_CXL_ATTR(s2m_drs_memdata,			CXL_PMU_GID_S2M_DRS, BIT(0)),