perf: arm_cspmu: Reject events meant for other PMUs

[PATCH] perf: arm_cspmu: Reject events meant for other PMUs

Posted by Ilkka Koskinen 2 years, 1 month ago

Coresight PMU driver didn't reject events meant for other PMUs.
This caused some of the Core PMU events disappearing from
the output of "perf list". In addition, trying to run e.g.

     $ perf stat -e r2 sleep 1

made Coresight PMU driver to handle the event instead of letting
Core PMU driver to deal with it.

Cc: stable@vger.kernel.org
Fixes: e37dfd65731d ("perf: arm_cspmu: Add support for ARM CoreSight PMU driver")
Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 42b72042f6b3..2cc35dded007 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -676,6 +676,9 @@ static int arm_cspmu_event_init(struct perf_event *event)
 
 	cspmu = to_arm_cspmu(event->pmu);
 
+	if (event->attr.type != event->pmu->type)
+		return -ENOENT;
+
 	/*
 	 * Following other "uncore" PMUs, we do not support sampling mode or
 	 * attach to a task (per-process mode).
-- 
2.40.1

Re: [PATCH] perf: arm_cspmu: Reject events meant for other PMUs

Posted by Catalin Marinas 2 years, 1 month ago

On Thu, 02 Nov 2023 17:16:54 -0700, Ilkka Koskinen wrote:
> Coresight PMU driver didn't reject events meant for other PMUs.
> This caused some of the Core PMU events disappearing from
> the output of "perf list". In addition, trying to run e.g.
> 
>      $ perf stat -e r2 sleep 1
> 
> made Coresight PMU driver to handle the event instead of letting
> Core PMU driver to deal with it.
> 
> [...]

Applied to arm64 (for-next/core), thanks!

[1/1] perf: arm_cspmu: Reject events meant for other PMUs
      https://git.kernel.org/arm64/c/15c7ef7341a2

-- 
Catalin

Re: [PATCH] perf: arm_cspmu: Reject events meant for other PMUs

Posted by Ilkka Koskinen 2 years, 1 month ago

Hi Catalin,

On Tue, 7 Nov 2023, Catalin Marinas wrote:
> On Thu, 02 Nov 2023 17:16:54 -0700, Ilkka Koskinen wrote:
>> Coresight PMU driver didn't reject events meant for other PMUs.
>> This caused some of the Core PMU events disappearing from
>> the output of "perf list". In addition, trying to run e.g.
>>
>>      $ perf stat -e r2 sleep 1
>>
>> made Coresight PMU driver to handle the event instead of letting
>> Core PMU driver to deal with it.
>>
>> [...]
>
> Applied to arm64 (for-next/core), thanks!

Just wanted to ask if you are planning to submit this patch for 6.7-rcX as 
a fix? We have found out that one can even crash AmpereOne by running
"perf stat -C 1 sleep 1" without the patch.

Cheers, Ilkka

>
> [1/1] perf: arm_cspmu: Reject events meant for other PMUs
>      https://git.kernel.org/arm64/c/15c7ef7341a2
>
> -- 
> Catalin
>
>

Re: [PATCH] perf: arm_cspmu: Reject events meant for other PMUs

Posted by Catalin Marinas 2 years, 1 month ago

On Tue, Nov 07, 2023 at 02:12:58PM -0800, Ilkka Koskinen wrote:
> On Tue, 7 Nov 2023, Catalin Marinas wrote:
> > On Thu, 02 Nov 2023 17:16:54 -0700, Ilkka Koskinen wrote:
> > > Coresight PMU driver didn't reject events meant for other PMUs.
> > > This caused some of the Core PMU events disappearing from
> > > the output of "perf list". In addition, trying to run e.g.
> > > 
> > >      $ perf stat -e r2 sleep 1
> > > 
> > > made Coresight PMU driver to handle the event instead of letting
> > > Core PMU driver to deal with it.
> > > 
> > > [...]
> > 
> > Applied to arm64 (for-next/core), thanks!
> 
> Just wanted to ask if you are planning to submit this patch for 6.7-rcX as a
> fix? We have found out that one can even crash AmpereOne by running
> "perf stat -C 1 sleep 1" without the patch.

Yes, I'll send it for 6.7-rc1 (probably tomorrow).

-- 
Catalin

Re: [PATCH] perf: arm_cspmu: Reject events meant for other PMUs

Posted by Ilkka Koskinen 2 years, 1 month ago

On Wed, 8 Nov 2023, Catalin Marinas wrote:
> On Tue, Nov 07, 2023 at 02:12:58PM -0800, Ilkka Koskinen wrote:
>> On Tue, 7 Nov 2023, Catalin Marinas wrote:
>>> On Thu, 02 Nov 2023 17:16:54 -0700, Ilkka Koskinen wrote:
>>>> Coresight PMU driver didn't reject events meant for other PMUs.
>>>> This caused some of the Core PMU events disappearing from
>>>> the output of "perf list". In addition, trying to run e.g.
>>>>
>>>>      $ perf stat -e r2 sleep 1
>>>>
>>>> made Coresight PMU driver to handle the event instead of letting
>>>> Core PMU driver to deal with it.
>>>>
>>>> [...]
>>>
>>> Applied to arm64 (for-next/core), thanks!
>>
>> Just wanted to ask if you are planning to submit this patch for 6.7-rcX as a
>> fix? We have found out that one can even crash AmpereOne by running
>> "perf stat -C 1 sleep 1" without the patch.
>
> Yes, I'll send it for 6.7-rc1 (probably tomorrow).

Sounds perfert. Thanks a lot!

--Ilkka

>
> -- 
> Catalin
>

Re: [PATCH] perf: arm_cspmu: Reject events meant for other PMUs

Posted by Will Deacon 2 years, 1 month ago

[+Catalin]

On Thu, Nov 02, 2023 at 05:16:54PM -0700, Ilkka Koskinen wrote:
> Coresight PMU driver didn't reject events meant for other PMUs.
> This caused some of the Core PMU events disappearing from
> the output of "perf list". In addition, trying to run e.g.
> 
>      $ perf stat -e r2 sleep 1
> 
> made Coresight PMU driver to handle the event instead of letting
> Core PMU driver to deal with it.
> 
> Cc: stable@vger.kernel.org
> Fixes: e37dfd65731d ("perf: arm_cspmu: Add support for ARM CoreSight PMU driver")
> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> ---
>  drivers/perf/arm_cspmu/arm_cspmu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index 42b72042f6b3..2cc35dded007 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -676,6 +676,9 @@ static int arm_cspmu_event_init(struct perf_event *event)
>  
>  	cspmu = to_arm_cspmu(event->pmu);
>  
> +	if (event->attr.type != event->pmu->type)
> +		return -ENOENT;
> +

Acked-by: Will Deacon <will@kernel.org>

Catalin -- please can you pick this up as a fix in the arm64 tree?

Cheers,

Will

Re: [PATCH] perf: arm_cspmu: Reject events meant for other PMUs

Posted by Mark Rutland 2 years, 1 month ago

On Thu, Nov 02, 2023 at 05:16:54PM -0700, Ilkka Koskinen wrote:
> Coresight PMU driver didn't reject events meant for other PMUs.
> This caused some of the Core PMU events disappearing from
> the output of "perf list". In addition, trying to run e.g.
> 
>      $ perf stat -e r2 sleep 1
> 
> made Coresight PMU driver to handle the event instead of letting
> Core PMU driver to deal with it.
> 
> Cc: stable@vger.kernel.org
> Fixes: e37dfd65731d ("perf: arm_cspmu: Add support for ARM CoreSight PMU driver")
> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>

Acked-by: Mark Rutland <mark.rutland@arm.com>

Will, are you happy to pick this up?

Mark.

> ---
>  drivers/perf/arm_cspmu/arm_cspmu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index 42b72042f6b3..2cc35dded007 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -676,6 +676,9 @@ static int arm_cspmu_event_init(struct perf_event *event)
>  
>  	cspmu = to_arm_cspmu(event->pmu);
>  
> +	if (event->attr.type != event->pmu->type)
> +		return -ENOENT;
> +
>  	/*
>  	 * Following other "uncore" PMUs, we do not support sampling mode or
>  	 * attach to a task (per-process mode).
> -- 
> 2.40.1
>