diff mbox series

perf: arm_cspmu: Reject events meant for other PMUs

Message ID 20231103001654.35565-1-ilkka@os.amperecomputing.com
State New
Headers
Series perf: arm_cspmu: Reject events meant for other PMUs |

Commit Message

Ilkka Koskinen Nov. 3, 2023, 12:16 a.m. UTC 
  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(+)

Comments

Besar Wicaksono Nov. 3, 2023, 2:14 a.m. UTC | #1 
Hi Ilkka,

Thanks for fixing it.

Reviewed-by: Besar Wicaksono <bwicaksono@nvidia.com>

Regards,
Besar

> -----Original Message-----
> From: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> Sent: Thursday, November 2, 2023 7:17 PM
> To: Besar Wicaksono <bwicaksono@nvidia.com>; Suzuki K Poulose
> <suzuki.poulose@arm.com>; Robin Murphy <robin.murphy@arm.com>; Will
> Deacon <will@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Raag
> Jadav <raag.jadav@intel.com>; Ilkka Koskinen
> <ilkka@os.amperecomputing.com>
> Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [PATCH] perf: arm_cspmu: Reject events meant for other PMUs
> 
> External email: Use caution opening links or attachments
> 
> 
> 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
Anshuman Khandual Nov. 3, 2023, 6:55 a.m. UTC | #2 
On 11/3/23 05:46, 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;
> +
>  	/*
>  	 * Following other "uncore" PMUs, we do not support sampling mode or
>  	 * attach to a task (per-process mode).

LGTM

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Mark Rutland Nov. 3, 2023, 11:37 a.m. UTC | #3 
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
>
Will Deacon Nov. 7, 2023, 10:50 a.m. UTC | #4 
[+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
Catalin Marinas Nov. 7, 2023, 2:17 p.m. UTC | #5 
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
Ilkka Koskinen Nov. 7, 2023, 10:12 p.m. UTC | #6 
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
>
>
Catalin Marinas Nov. 8, 2023, 10:05 a.m. UTC | #7 
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).
Ilkka Koskinen Nov. 8, 2023, 7 p.m. UTC | #8 
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
>
diff mbox series

Patch

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).