All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-06 14:01 ` Vasily Khoruzhick
  0 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-08-06 14:01 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Mark Rutland,
	linux-arm-kernel, devicetree
  Cc: Jared D . McNeill, Harald Geyer

Looks like PMU in A64 is broken, it generates no interrupts at all and
as result 'perf top' shows no events.

Tested on Pine64-LTS.

Fixes: 34a97fcc71c2 ("arm64: dts: allwinner: a64: Add PMU node")
Cc: Harald Geyer <harald@ccbib.org>
Cc: Jared D. McNeill <jmcneill@NetBSD.org>
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 9cc9bdde81ac..cd92f546c483 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -142,15 +142,6 @@
 		clock-output-names = "ext-osc32k";
 	};
 
-	pmu {
-		compatible = "arm,cortex-a53-pmu";
-		interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
-			     <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
-			     <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
-			     <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
-	};
-
 	psci {
 		compatible = "arm,psci-0.2";
 		method = "smc";
-- 
2.22.0

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-06 14:01 ` Vasily Khoruzhick
  0 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-08-06 14:01 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Mark Rutland,
	linux-arm-kernel, devicetree
  Cc: Jared D . McNeill, Harald Geyer

Looks like PMU in A64 is broken, it generates no interrupts at all and
as result 'perf top' shows no events.

Tested on Pine64-LTS.

Fixes: 34a97fcc71c2 ("arm64: dts: allwinner: a64: Add PMU node")
Cc: Harald Geyer <harald@ccbib.org>
Cc: Jared D. McNeill <jmcneill@NetBSD.org>
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 9cc9bdde81ac..cd92f546c483 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -142,15 +142,6 @@
 		clock-output-names = "ext-osc32k";
 	};
 
-	pmu {
-		compatible = "arm,cortex-a53-pmu";
-		interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
-			     <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
-			     <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
-			     <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
-	};
-
 	psci {
 		compatible = "arm,psci-0.2";
 		method = "smc";
-- 
2.22.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-06 14:01 ` Vasily Khoruzhick
@ 2019-08-06 14:35   ` Robin Murphy
  -1 siblings, 0 replies; 48+ messages in thread
From: Robin Murphy @ 2019-08-06 14:35 UTC (permalink / raw)
  To: Vasily Khoruzhick, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Mark Rutland, linux-arm-kernel, devicetree
  Cc: Jared D . McNeill, Harald Geyer

On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> Looks like PMU in A64 is broken, it generates no interrupts at all and
> as result 'perf top' shows no events.

Does something like 'perf stat sleep 1' at least count cycles correctly? 
It could well just be that the interrupt numbers are wrong...

> Tested on Pine64-LTS.
> 
> Fixes: 34a97fcc71c2 ("arm64: dts: allwinner: a64: Add PMU node")
> Cc: Harald Geyer <harald@ccbib.org>
> Cc: Jared D. McNeill <jmcneill@NetBSD.org>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>   arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 9 ---------
>   1 file changed, 9 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 9cc9bdde81ac..cd92f546c483 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -142,15 +142,6 @@
>   		clock-output-names = "ext-osc32k";
>   	};
>   
> -	pmu {
> -		compatible = "arm,cortex-a53-pmu";
> -		interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;

Cross-referencing between some random DTs in the H6 BSP I happen to have 
to hand and the A64 User Manual, it looks a lot like someone just forgot 
to subtract 32 from these to satisfy the awkward GIC binding - that 
wants the SPI index rather than the actual interrupt source number, 
which implies these should probably be 120-123 rather than 152-155.

Robin.

> -		interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
> -	};
> -
>   	psci {
>   		compatible = "arm,psci-0.2";
>   		method = "smc";
> 

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-06 14:35   ` Robin Murphy
  0 siblings, 0 replies; 48+ messages in thread
From: Robin Murphy @ 2019-08-06 14:35 UTC (permalink / raw)
  To: Vasily Khoruzhick, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Mark Rutland, linux-arm-kernel, devicetree
  Cc: Jared D . McNeill, Harald Geyer

On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> Looks like PMU in A64 is broken, it generates no interrupts at all and
> as result 'perf top' shows no events.

Does something like 'perf stat sleep 1' at least count cycles correctly? 
It could well just be that the interrupt numbers are wrong...

> Tested on Pine64-LTS.
> 
> Fixes: 34a97fcc71c2 ("arm64: dts: allwinner: a64: Add PMU node")
> Cc: Harald Geyer <harald@ccbib.org>
> Cc: Jared D. McNeill <jmcneill@NetBSD.org>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>   arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 9 ---------
>   1 file changed, 9 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 9cc9bdde81ac..cd92f546c483 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -142,15 +142,6 @@
>   		clock-output-names = "ext-osc32k";
>   	};
>   
> -	pmu {
> -		compatible = "arm,cortex-a53-pmu";
> -		interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;

Cross-referencing between some random DTs in the H6 BSP I happen to have 
to hand and the A64 User Manual, it looks a lot like someone just forgot 
to subtract 32 from these to satisfy the awkward GIC binding - that 
wants the SPI index rather than the actual interrupt source number, 
which implies these should probably be 120-123 rather than 152-155.

Robin.

> -		interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
> -	};
> -
>   	psci {
>   		compatible = "arm,psci-0.2";
>   		method = "smc";
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-06 14:35   ` Robin Murphy
@ 2019-08-06 14:45     ` Vasily Khoruzhick
  -1 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-08-06 14:45 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, arm-linux

On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > Looks like PMU in A64 is broken, it generates no interrupts at all and
> > as result 'perf top' shows no events.
>
> Does something like 'perf stat sleep 1' at least count cycles correctly?
> It could well just be that the interrupt numbers are wrong...

Looks like it does, at least result looks plausible:

$ perf stat sleep 1

Performance counter stats for 'sleep 1':

             4.08 msec task-clock:u              #    0.004 CPUs
utilized
                0      context-switches:u        #    0.000 K/sec
                0      cpu-migrations:u          #    0.000 K/sec
               55      page-faults:u             #    0.013 M/sec
          527,711      cycles:u                  #    0.129 GHz
          197,262      instructions:u            #    0.37  insn per
cycle
           24,242      branches:u                #    5.947 M/sec
            5,083      branch-misses:u           #   20.97% of all
branches

      1.011928625 seconds time elapsed

      0.000000000 seconds user
      0.007196000 seconds sys

> > Tested on Pine64-LTS.
> >
> > Fixes: 34a97fcc71c2 ("arm64: dts: allwinner: a64: Add PMU node")
> > Cc: Harald Geyer <harald@ccbib.org>
> > Cc: Jared D. McNeill <jmcneill@NetBSD.org>
> > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > ---
> >   arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 9 ---------
> >   1 file changed, 9 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > index 9cc9bdde81ac..cd92f546c483 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > @@ -142,15 +142,6 @@
> >               clock-output-names = "ext-osc32k";
> >       };
> >
> > -     pmu {
> > -             compatible = "arm,cortex-a53-pmu";
> > -             interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> > -                          <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
> > -                          <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
> > -                          <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
>
> Cross-referencing between some random DTs in the H6 BSP I happen to have
> to hand and the A64 User Manual, it looks a lot like someone just forgot
> to subtract 32 from these to satisfy the awkward GIC binding - that
> wants the SPI index rather than the actual interrupt source number,
> which implies these should probably be 120-123 rather than 152-155.

Tried that, didn't work. 'grep pmu /proc/interrupts' shows zeroes, and
'perf top' is completely silent.

> Robin.
>
> > -             interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
> > -     };
> > -
> >       psci {
> >               compatible = "arm,psci-0.2";
> >               method = "smc";
> >

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-06 14:45     ` Vasily Khoruzhick
  0 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-08-06 14:45 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, arm-linux

On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > Looks like PMU in A64 is broken, it generates no interrupts at all and
> > as result 'perf top' shows no events.
>
> Does something like 'perf stat sleep 1' at least count cycles correctly?
> It could well just be that the interrupt numbers are wrong...

Looks like it does, at least result looks plausible:

$ perf stat sleep 1

Performance counter stats for 'sleep 1':

             4.08 msec task-clock:u              #    0.004 CPUs
utilized
                0      context-switches:u        #    0.000 K/sec
                0      cpu-migrations:u          #    0.000 K/sec
               55      page-faults:u             #    0.013 M/sec
          527,711      cycles:u                  #    0.129 GHz
          197,262      instructions:u            #    0.37  insn per
cycle
           24,242      branches:u                #    5.947 M/sec
            5,083      branch-misses:u           #   20.97% of all
branches

      1.011928625 seconds time elapsed

      0.000000000 seconds user
      0.007196000 seconds sys

> > Tested on Pine64-LTS.
> >
> > Fixes: 34a97fcc71c2 ("arm64: dts: allwinner: a64: Add PMU node")
> > Cc: Harald Geyer <harald@ccbib.org>
> > Cc: Jared D. McNeill <jmcneill@NetBSD.org>
> > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > ---
> >   arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 9 ---------
> >   1 file changed, 9 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > index 9cc9bdde81ac..cd92f546c483 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > @@ -142,15 +142,6 @@
> >               clock-output-names = "ext-osc32k";
> >       };
> >
> > -     pmu {
> > -             compatible = "arm,cortex-a53-pmu";
> > -             interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> > -                          <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
> > -                          <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
> > -                          <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
>
> Cross-referencing between some random DTs in the H6 BSP I happen to have
> to hand and the A64 User Manual, it looks a lot like someone just forgot
> to subtract 32 from these to satisfy the awkward GIC binding - that
> wants the SPI index rather than the actual interrupt source number,
> which implies these should probably be 120-123 rather than 152-155.

Tried that, didn't work. 'grep pmu /proc/interrupts' shows zeroes, and
'perf top' is completely silent.

> Robin.
>
> > -             interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
> > -     };
> > -
> >       psci {
> >               compatible = "arm,psci-0.2";
> >               method = "smc";
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-06 14:01 ` Vasily Khoruzhick
@ 2019-08-06 19:10   ` Emmanuel Vadot
  -1 siblings, 0 replies; 48+ messages in thread
From: Emmanuel Vadot @ 2019-08-06 19:10 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, linux-arm-kernel

On Tue,  6 Aug 2019 07:01:35 -0700
Vasily Khoruzhick <anarsoul@gmail.com> wrote:

> Looks like PMU in A64 is broken, it generates no interrupts at all and
> as result 'perf top' shows no events.
> 
> Tested on Pine64-LTS.
> 
> Fixes: 34a97fcc71c2 ("arm64: dts: allwinner: a64: Add PMU node")
> Cc: Harald Geyer <harald@ccbib.org>
> Cc: Jared D. McNeill <jmcneill@NetBSD.org>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 9cc9bdde81ac..cd92f546c483 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -142,15 +142,6 @@
>  		clock-output-names = "ext-osc32k";
>  	};
>  
> -	pmu {
> -		compatible = "arm,cortex-a53-pmu";
> -		interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> -		interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
> -	};
> -
>  	psci {
>  		compatible = "arm,psci-0.2";
>  		method = "smc";
> -- 
> 2.22.0

 It doesn't work for me too on FreeBSD, and yes the interrupts are
wrong but this is not the problem. Maybe there is a reset line but it's
not documented in the documentation.

 Reviewed-by: Emmanuel Vadot <manu@FreeBSD.org>

-- 
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-06 19:10   ` Emmanuel Vadot
  0 siblings, 0 replies; 48+ messages in thread
From: Emmanuel Vadot @ 2019-08-06 19:10 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, linux-arm-kernel

On Tue,  6 Aug 2019 07:01:35 -0700
Vasily Khoruzhick <anarsoul@gmail.com> wrote:

> Looks like PMU in A64 is broken, it generates no interrupts at all and
> as result 'perf top' shows no events.
> 
> Tested on Pine64-LTS.
> 
> Fixes: 34a97fcc71c2 ("arm64: dts: allwinner: a64: Add PMU node")
> Cc: Harald Geyer <harald@ccbib.org>
> Cc: Jared D. McNeill <jmcneill@NetBSD.org>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 9cc9bdde81ac..cd92f546c483 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -142,15 +142,6 @@
>  		clock-output-names = "ext-osc32k";
>  	};
>  
> -	pmu {
> -		compatible = "arm,cortex-a53-pmu";
> -		interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> -		interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
> -	};
> -
>  	psci {
>  		compatible = "arm,psci-0.2";
>  		method = "smc";
> -- 
> 2.22.0

 It doesn't work for me too on FreeBSD, and yes the interrupts are
wrong but this is not the problem. Maybe there is a reset line but it's
not documented in the documentation.

 Reviewed-by: Emmanuel Vadot <manu@FreeBSD.org>

-- 
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-06 14:45     ` Vasily Khoruzhick
@ 2019-08-06 20:19       ` Harald Geyer
  -1 siblings, 0 replies; 48+ messages in thread
From: Harald Geyer @ 2019-08-06 20:19 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Robin Murphy, arm-linux

Vasily Khoruzhick writes:
> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> >
> > On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > as result 'perf top' shows no events.
> >
> > Does something like 'perf stat sleep 1' at least count cycles correctly?
> > It could well just be that the interrupt numbers are wrong...
> 
> Looks like it does, at least result looks plausible:

I'm using perf stat regularly (cache benchmarks) and it works fine.

Unfortunately I wasn't aware that perf stat is a poor test for
the interrupts part of the node, when I added it. So I'm not too
surprised I got it wrong.

However, it would be unfortunate if the node got removed completely,
because perf stat would not work anymore. Maybe we can only remove
the interrupts or just fix them even if the HW doesn't work?

Harald

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-06 20:19       ` Harald Geyer
  0 siblings, 0 replies; 48+ messages in thread
From: Harald Geyer @ 2019-08-06 20:19 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Robin Murphy, arm-linux

Vasily Khoruzhick writes:
> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> >
> > On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > as result 'perf top' shows no events.
> >
> > Does something like 'perf stat sleep 1' at least count cycles correctly?
> > It could well just be that the interrupt numbers are wrong...
> 
> Looks like it does, at least result looks plausible:

I'm using perf stat regularly (cache benchmarks) and it works fine.

Unfortunately I wasn't aware that perf stat is a poor test for
the interrupts part of the node, when I added it. So I'm not too
surprised I got it wrong.

However, it would be unfortunate if the node got removed completely,
because perf stat would not work anymore. Maybe we can only remove
the interrupts or just fix them even if the HW doesn't work?

Harald

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-06 20:19       ` Harald Geyer
@ 2019-08-06 20:52         ` Vasily Khoruzhick
  -1 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-08-06 20:52 UTC (permalink / raw)
  To: Harald Geyer
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Robin Murphy, arm-linux

On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
>
> Vasily Khoruzhick writes:
> > On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > >
> > > On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > as result 'perf top' shows no events.
> > >
> > > Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > It could well just be that the interrupt numbers are wrong...
> >
> > Looks like it does, at least result looks plausible:
>
> I'm using perf stat regularly (cache benchmarks) and it works fine.
>
> Unfortunately I wasn't aware that perf stat is a poor test for
> the interrupts part of the node, when I added it. So I'm not too
> surprised I got it wrong.
>
> However, it would be unfortunate if the node got removed completely,
> because perf stat would not work anymore. Maybe we can only remove
> the interrupts or just fix them even if the HW doesn't work?

I'm not familiar with PMU driver. Is it possible to get it working
without interrupts?

>
> Harald

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-06 20:52         ` Vasily Khoruzhick
  0 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-08-06 20:52 UTC (permalink / raw)
  To: Harald Geyer
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Robin Murphy, arm-linux

On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
>
> Vasily Khoruzhick writes:
> > On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > >
> > > On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > as result 'perf top' shows no events.
> > >
> > > Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > It could well just be that the interrupt numbers are wrong...
> >
> > Looks like it does, at least result looks plausible:
>
> I'm using perf stat regularly (cache benchmarks) and it works fine.
>
> Unfortunately I wasn't aware that perf stat is a poor test for
> the interrupts part of the node, when I added it. So I'm not too
> surprised I got it wrong.
>
> However, it would be unfortunate if the node got removed completely,
> because perf stat would not work anymore. Maybe we can only remove
> the interrupts or just fix them even if the HW doesn't work?

I'm not familiar with PMU driver. Is it possible to get it working
without interrupts?

>
> Harald

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-06 20:52         ` Vasily Khoruzhick
@ 2019-08-06 21:14           ` Robin Murphy
  -1 siblings, 0 replies; 48+ messages in thread
From: Robin Murphy @ 2019-08-06 21:14 UTC (permalink / raw)
  To: Vasily Khoruzhick, Harald Geyer
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, arm-linux

On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
>>
>> Vasily Khoruzhick writes:
>>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
>>>>
>>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
>>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
>>>>> as result 'perf top' shows no events.
>>>>
>>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
>>>> It could well just be that the interrupt numbers are wrong...
>>>
>>> Looks like it does, at least result looks plausible:
>>
>> I'm using perf stat regularly (cache benchmarks) and it works fine.
>>
>> Unfortunately I wasn't aware that perf stat is a poor test for
>> the interrupts part of the node, when I added it. So I'm not too
>> surprised I got it wrong.
>>
>> However, it would be unfortunate if the node got removed completely,
>> because perf stat would not work anymore. Maybe we can only remove
>> the interrupts or just fix them even if the HW doesn't work?
> 
> I'm not familiar with PMU driver. Is it possible to get it working
> without interrupts?

Yup - you get a grumpy message from the driver, it will refuse sampling 
events (the ones which weren't working anyway), and if you measure 
anything for long enough that a counter overflows you'll get wonky 
results. But for counting hardware events over relatively short periods 
it'll still do the job.

Robin.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-06 21:14           ` Robin Murphy
  0 siblings, 0 replies; 48+ messages in thread
From: Robin Murphy @ 2019-08-06 21:14 UTC (permalink / raw)
  To: Vasily Khoruzhick, Harald Geyer
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, arm-linux

On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
>>
>> Vasily Khoruzhick writes:
>>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
>>>>
>>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
>>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
>>>>> as result 'perf top' shows no events.
>>>>
>>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
>>>> It could well just be that the interrupt numbers are wrong...
>>>
>>> Looks like it does, at least result looks plausible:
>>
>> I'm using perf stat regularly (cache benchmarks) and it works fine.
>>
>> Unfortunately I wasn't aware that perf stat is a poor test for
>> the interrupts part of the node, when I added it. So I'm not too
>> surprised I got it wrong.
>>
>> However, it would be unfortunate if the node got removed completely,
>> because perf stat would not work anymore. Maybe we can only remove
>> the interrupts or just fix them even if the HW doesn't work?
> 
> I'm not familiar with PMU driver. Is it possible to get it working
> without interrupts?

Yup - you get a grumpy message from the driver, it will refuse sampling 
events (the ones which weren't working anyway), and if you measure 
anything for long enough that a counter overflows you'll get wonky 
results. But for counting hardware events over relatively short periods 
it'll still do the job.

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-06 21:14           ` Robin Murphy
@ 2019-08-07  2:39             ` Vasily Khoruzhick
  -1 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-08-07  2:39 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, arm-linux

On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> >>
> >> Vasily Khoruzhick writes:
> >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> >>>>
> >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> >>>>> as result 'perf top' shows no events.
> >>>>
> >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> >>>> It could well just be that the interrupt numbers are wrong...
> >>>
> >>> Looks like it does, at least result looks plausible:
> >>
> >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> >>
> >> Unfortunately I wasn't aware that perf stat is a poor test for
> >> the interrupts part of the node, when I added it. So I'm not too
> >> surprised I got it wrong.
> >>
> >> However, it would be unfortunate if the node got removed completely,
> >> because perf stat would not work anymore. Maybe we can only remove
> >> the interrupts or just fix them even if the HW doesn't work?
> >
> > I'm not familiar with PMU driver. Is it possible to get it working
> > without interrupts?
>
> Yup - you get a grumpy message from the driver, it will refuse sampling
> events (the ones which weren't working anyway), and if you measure
> anything for long enough that a counter overflows you'll get wonky
> results. But for counting hardware events over relatively short periods
> it'll still do the job.

I tried to drop interrupts completely from the node but 'perf top' is
still broken. Though now in different way: it complains "cycles: PMU
Hardware doesn't support sampling/overflow-interrupts. Try 'perf
stat'"

Is there any way to make it working?

>
> Robin.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-07  2:39             ` Vasily Khoruzhick
  0 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-08-07  2:39 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, arm-linux

On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> >>
> >> Vasily Khoruzhick writes:
> >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> >>>>
> >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> >>>>> as result 'perf top' shows no events.
> >>>>
> >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> >>>> It could well just be that the interrupt numbers are wrong...
> >>>
> >>> Looks like it does, at least result looks plausible:
> >>
> >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> >>
> >> Unfortunately I wasn't aware that perf stat is a poor test for
> >> the interrupts part of the node, when I added it. So I'm not too
> >> surprised I got it wrong.
> >>
> >> However, it would be unfortunate if the node got removed completely,
> >> because perf stat would not work anymore. Maybe we can only remove
> >> the interrupts or just fix them even if the HW doesn't work?
> >
> > I'm not familiar with PMU driver. Is it possible to get it working
> > without interrupts?
>
> Yup - you get a grumpy message from the driver, it will refuse sampling
> events (the ones which weren't working anyway), and if you measure
> anything for long enough that a counter overflows you'll get wonky
> results. But for counting hardware events over relatively short periods
> it'll still do the job.

I tried to drop interrupts completely from the node but 'perf top' is
still broken. Though now in different way: it complains "cycles: PMU
Hardware doesn't support sampling/overflow-interrupts. Try 'perf
stat'"

Is there any way to make it working?

>
> Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-06 21:14           ` Robin Murphy
@ 2019-08-07 11:12             ` Mark Rutland
  -1 siblings, 0 replies; 48+ messages in thread
From: Mark Rutland @ 2019-08-07 11:12 UTC (permalink / raw)
  To: Robin Murphy
  Cc: devicetree, Jared D . McNeill, Maxime Ripard, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, arm-linux

On Tue, Aug 06, 2019 at 10:14:39PM +0100, Robin Murphy wrote:
> On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > 
> > > Vasily Khoruzhick writes:
> > > > On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > 
> > > > > On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > as result 'perf top' shows no events.
> > > > > 
> > > > > Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > It could well just be that the interrupt numbers are wrong...
> > > > 
> > > > Looks like it does, at least result looks plausible:
> > > 
> > > I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > 
> > > Unfortunately I wasn't aware that perf stat is a poor test for
> > > the interrupts part of the node, when I added it. So I'm not too
> > > surprised I got it wrong.
> > > 
> > > However, it would be unfortunate if the node got removed completely,
> > > because perf stat would not work anymore. Maybe we can only remove
> > > the interrupts or just fix them even if the HW doesn't work?
> > 
> > I'm not familiar with PMU driver. Is it possible to get it working
> > without interrupts?
> 
> Yup - you get a grumpy message from the driver, it will refuse sampling
> events (the ones which weren't working anyway), and if you measure anything
> for long enough that a counter overflows you'll get wonky results. But for
> counting hardware events over relatively short periods it'll still do the
> job.

Even that's stupidly dodgy; a CPU_CYCLES event could easily overflow
several times between the kernel sampling it, so you can lose billions
of events without any idea that happened.

For other PMUs we can fix that with a hrtimer, but I think for a CPU PMU
it has to be at such a high frequency that it imposes a ridiculous
overhead, even assuming we can choose a sufficient frequency. :/

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-07 11:12             ` Mark Rutland
  0 siblings, 0 replies; 48+ messages in thread
From: Mark Rutland @ 2019-08-07 11:12 UTC (permalink / raw)
  To: Robin Murphy
  Cc: devicetree, Jared D . McNeill, Maxime Ripard, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, arm-linux

On Tue, Aug 06, 2019 at 10:14:39PM +0100, Robin Murphy wrote:
> On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > 
> > > Vasily Khoruzhick writes:
> > > > On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > 
> > > > > On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > as result 'perf top' shows no events.
> > > > > 
> > > > > Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > It could well just be that the interrupt numbers are wrong...
> > > > 
> > > > Looks like it does, at least result looks plausible:
> > > 
> > > I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > 
> > > Unfortunately I wasn't aware that perf stat is a poor test for
> > > the interrupts part of the node, when I added it. So I'm not too
> > > surprised I got it wrong.
> > > 
> > > However, it would be unfortunate if the node got removed completely,
> > > because perf stat would not work anymore. Maybe we can only remove
> > > the interrupts or just fix them even if the HW doesn't work?
> > 
> > I'm not familiar with PMU driver. Is it possible to get it working
> > without interrupts?
> 
> Yup - you get a grumpy message from the driver, it will refuse sampling
> events (the ones which weren't working anyway), and if you measure anything
> for long enough that a counter overflows you'll get wonky results. But for
> counting hardware events over relatively short periods it'll still do the
> job.

Even that's stupidly dodgy; a CPU_CYCLES event could easily overflow
several times between the kernel sampling it, so you can lose billions
of events without any idea that happened.

For other PMUs we can fix that with a hrtimer, but I think for a CPU PMU
it has to be at such a high frequency that it imposes a ridiculous
overhead, even assuming we can choose a sufficient frequency. :/

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-07  2:39             ` Vasily Khoruzhick
@ 2019-08-07 11:56               ` Maxime Ripard
  -1 siblings, 0 replies; 48+ messages in thread
From: Maxime Ripard @ 2019-08-07 11:56 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> >
> > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > >>
> > >> Vasily Khoruzhick writes:
> > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > >>>>
> > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > >>>>> as result 'perf top' shows no events.
> > >>>>
> > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > >>>> It could well just be that the interrupt numbers are wrong...
> > >>>
> > >>> Looks like it does, at least result looks plausible:
> > >>
> > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > >>
> > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > >> the interrupts part of the node, when I added it. So I'm not too
> > >> surprised I got it wrong.
> > >>
> > >> However, it would be unfortunate if the node got removed completely,
> > >> because perf stat would not work anymore. Maybe we can only remove
> > >> the interrupts or just fix them even if the HW doesn't work?
> > >
> > > I'm not familiar with PMU driver. Is it possible to get it working
> > > without interrupts?
> >
> > Yup - you get a grumpy message from the driver, it will refuse sampling
> > events (the ones which weren't working anyway), and if you measure
> > anything for long enough that a counter overflows you'll get wonky
> > results. But for counting hardware events over relatively short periods
> > it'll still do the job.
>
> I tried to drop interrupts completely from the node but 'perf top' is
> still broken. Though now in different way: it complains "cycles: PMU
> Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> stat'"

I have no idea if that's the culprit, but what is the state of the
0x09010000 register?

(in particular, are the bits 16-19 and 24 set or not?

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-07 11:56               ` Maxime Ripard
  0 siblings, 0 replies; 48+ messages in thread
From: Maxime Ripard @ 2019-08-07 11:56 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> >
> > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > >>
> > >> Vasily Khoruzhick writes:
> > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > >>>>
> > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > >>>>> as result 'perf top' shows no events.
> > >>>>
> > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > >>>> It could well just be that the interrupt numbers are wrong...
> > >>>
> > >>> Looks like it does, at least result looks plausible:
> > >>
> > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > >>
> > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > >> the interrupts part of the node, when I added it. So I'm not too
> > >> surprised I got it wrong.
> > >>
> > >> However, it would be unfortunate if the node got removed completely,
> > >> because perf stat would not work anymore. Maybe we can only remove
> > >> the interrupts or just fix them even if the HW doesn't work?
> > >
> > > I'm not familiar with PMU driver. Is it possible to get it working
> > > without interrupts?
> >
> > Yup - you get a grumpy message from the driver, it will refuse sampling
> > events (the ones which weren't working anyway), and if you measure
> > anything for long enough that a counter overflows you'll get wonky
> > results. But for counting hardware events over relatively short periods
> > it'll still do the job.
>
> I tried to drop interrupts completely from the node but 'perf top' is
> still broken. Though now in different way: it complains "cycles: PMU
> Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> stat'"

I have no idea if that's the culprit, but what is the state of the
0x09010000 register?

(in particular, are the bits 16-19 and 24 set or not?

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-07  2:39             ` Vasily Khoruzhick
@ 2019-08-07 11:59               ` Robin Murphy
  -1 siblings, 0 replies; 48+ messages in thread
From: Robin Murphy @ 2019-08-07 11:59 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, arm-linux

On 07/08/2019 03:39, Vasily Khoruzhick wrote:
> On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
>>> On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
>>>>
>>>> Vasily Khoruzhick writes:
>>>>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
>>>>>>
>>>>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
>>>>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
>>>>>>> as result 'perf top' shows no events.
>>>>>>
>>>>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
>>>>>> It could well just be that the interrupt numbers are wrong...
>>>>>
>>>>> Looks like it does, at least result looks plausible:
>>>>
>>>> I'm using perf stat regularly (cache benchmarks) and it works fine.
>>>>
>>>> Unfortunately I wasn't aware that perf stat is a poor test for
>>>> the interrupts part of the node, when I added it. So I'm not too
>>>> surprised I got it wrong.
>>>>
>>>> However, it would be unfortunate if the node got removed completely,
>>>> because perf stat would not work anymore. Maybe we can only remove
>>>> the interrupts or just fix them even if the HW doesn't work?
>>>
>>> I'm not familiar with PMU driver. Is it possible to get it working
>>> without interrupts?
>>
>> Yup - you get a grumpy message from the driver, it will refuse sampling
>> events (the ones which weren't working anyway), and if you measure
>> anything for long enough that a counter overflows you'll get wonky
>> results. But for counting hardware events over relatively short periods
>> it'll still do the job.
> 
> I tried to drop interrupts completely from the node but 'perf top' is
> still broken. Though now in different way: it complains "cycles: PMU
> Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> stat'"
> 
> Is there any way to make it working?

As the message implies, 'perf top' can't work because it uses sampling 
events, which are based on periodic interrupts. If the IRQs aren't 
there, then too bad, as there's no alternative.

One other possibility is that the IRQs really are wired up, but the 
firmware is somehow leaving them configured as Secure group 0, such that 
Linux has no visibility of them.

Robin.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-07 11:59               ` Robin Murphy
  0 siblings, 0 replies; 48+ messages in thread
From: Robin Murphy @ 2019-08-07 11:59 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, arm-linux

On 07/08/2019 03:39, Vasily Khoruzhick wrote:
> On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
>>> On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
>>>>
>>>> Vasily Khoruzhick writes:
>>>>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
>>>>>>
>>>>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
>>>>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
>>>>>>> as result 'perf top' shows no events.
>>>>>>
>>>>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
>>>>>> It could well just be that the interrupt numbers are wrong...
>>>>>
>>>>> Looks like it does, at least result looks plausible:
>>>>
>>>> I'm using perf stat regularly (cache benchmarks) and it works fine.
>>>>
>>>> Unfortunately I wasn't aware that perf stat is a poor test for
>>>> the interrupts part of the node, when I added it. So I'm not too
>>>> surprised I got it wrong.
>>>>
>>>> However, it would be unfortunate if the node got removed completely,
>>>> because perf stat would not work anymore. Maybe we can only remove
>>>> the interrupts or just fix them even if the HW doesn't work?
>>>
>>> I'm not familiar with PMU driver. Is it possible to get it working
>>> without interrupts?
>>
>> Yup - you get a grumpy message from the driver, it will refuse sampling
>> events (the ones which weren't working anyway), and if you measure
>> anything for long enough that a counter overflows you'll get wonky
>> results. But for counting hardware events over relatively short periods
>> it'll still do the job.
> 
> I tried to drop interrupts completely from the node but 'perf top' is
> still broken. Though now in different way: it complains "cycles: PMU
> Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> stat'"
> 
> Is there any way to make it working?

As the message implies, 'perf top' can't work because it uses sampling 
events, which are based on periodic interrupts. If the IRQs aren't 
there, then too bad, as there's no alternative.

One other possibility is that the IRQs really are wired up, but the 
firmware is somehow leaving them configured as Secure group 0, such that 
Linux has no visibility of them.

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-07 11:56               ` Maxime Ripard
@ 2019-08-07 17:36                 ` Vasily Khoruzhick
  -1 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-08-07 17:36 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > >
> > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > >>
> > > >> Vasily Khoruzhick writes:
> > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > >>>>
> > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > >>>>> as result 'perf top' shows no events.
> > > >>>>
> > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > >>>> It could well just be that the interrupt numbers are wrong...
> > > >>>
> > > >>> Looks like it does, at least result looks plausible:
> > > >>
> > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > >>
> > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > >> the interrupts part of the node, when I added it. So I'm not too
> > > >> surprised I got it wrong.
> > > >>
> > > >> However, it would be unfortunate if the node got removed completely,
> > > >> because perf stat would not work anymore. Maybe we can only remove
> > > >> the interrupts or just fix them even if the HW doesn't work?
> > > >
> > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > without interrupts?
> > >
> > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > events (the ones which weren't working anyway), and if you measure
> > > anything for long enough that a counter overflows you'll get wonky
> > > results. But for counting hardware events over relatively short periods
> > > it'll still do the job.
> >
> > I tried to drop interrupts completely from the node but 'perf top' is
> > still broken. Though now in different way: it complains "cycles: PMU
> > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > stat'"
>
> I have no idea if that's the culprit, but what is the state of the
> 0x09010000 register?

What register is that and how do I check it?

> (in particular, are the bits 16-19 and 24 set or not?
>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-07 17:36                 ` Vasily Khoruzhick
  0 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-08-07 17:36 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > >
> > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > >>
> > > >> Vasily Khoruzhick writes:
> > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > >>>>
> > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > >>>>> as result 'perf top' shows no events.
> > > >>>>
> > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > >>>> It could well just be that the interrupt numbers are wrong...
> > > >>>
> > > >>> Looks like it does, at least result looks plausible:
> > > >>
> > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > >>
> > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > >> the interrupts part of the node, when I added it. So I'm not too
> > > >> surprised I got it wrong.
> > > >>
> > > >> However, it would be unfortunate if the node got removed completely,
> > > >> because perf stat would not work anymore. Maybe we can only remove
> > > >> the interrupts or just fix them even if the HW doesn't work?
> > > >
> > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > without interrupts?
> > >
> > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > events (the ones which weren't working anyway), and if you measure
> > > anything for long enough that a counter overflows you'll get wonky
> > > results. But for counting hardware events over relatively short periods
> > > it'll still do the job.
> >
> > I tried to drop interrupts completely from the node but 'perf top' is
> > still broken. Though now in different way: it complains "cycles: PMU
> > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > stat'"
>
> I have no idea if that's the culprit, but what is the state of the
> 0x09010000 register?

What register is that and how do I check it?

> (in particular, are the bits 16-19 and 24 set or not?
>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-07 17:36                 ` Vasily Khoruzhick
  (?)
@ 2019-08-08 16:26                 ` Maxime Ripard
  2019-08-08 19:59                     ` Vasily Khoruzhick
  -1 siblings, 1 reply; 48+ messages in thread
From: Maxime Ripard @ 2019-08-08 16:26 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux


[-- Attachment #1.1: Type: text/plain, Size: 2652 bytes --]

On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > >
> > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > >>
> > > > >> Vasily Khoruzhick writes:
> > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > >>>>
> > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > >>>>> as result 'perf top' shows no events.
> > > > >>>>
> > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > >>>
> > > > >>> Looks like it does, at least result looks plausible:
> > > > >>
> > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > >>
> > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > >> surprised I got it wrong.
> > > > >>
> > > > >> However, it would be unfortunate if the node got removed completely,
> > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > >
> > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > without interrupts?
> > > >
> > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > events (the ones which weren't working anyway), and if you measure
> > > > anything for long enough that a counter overflows you'll get wonky
> > > > results. But for counting hardware events over relatively short periods
> > > > it'll still do the job.
> > >
> > > I tried to drop interrupts completely from the node but 'perf top' is
> > > still broken. Though now in different way: it complains "cycles: PMU
> > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > stat'"
> >
> > I have no idea if that's the culprit, but what is the state of the
> > 0x09010000 register?
>
> What register is that and how do I check it?

It's in the CPUX Configuration block, and the bits are labelled as CPU
Debug Reset.

And if you have busybox, you can use devmem.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-08 16:26                 ` Maxime Ripard
@ 2019-08-08 19:59                     ` Vasily Khoruzhick
  0 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-08-08 19:59 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > >
> > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > >>
> > > > > >> Vasily Khoruzhick writes:
> > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > >>>>
> > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > >>>>> as result 'perf top' shows no events.
> > > > > >>>>
> > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > >>>
> > > > > >>> Looks like it does, at least result looks plausible:
> > > > > >>
> > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > >>
> > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > >> surprised I got it wrong.
> > > > > >>
> > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > >
> > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > without interrupts?
> > > > >
> > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > events (the ones which weren't working anyway), and if you measure
> > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > results. But for counting hardware events over relatively short periods
> > > > > it'll still do the job.
> > > >
> > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > stat'"
> > >
> > > I have no idea if that's the culprit, but what is the state of the
> > > 0x09010000 register?
> >
> > What register is that and how do I check it?
>
> It's in the CPUX Configuration block, and the bits are labelled as CPU
> Debug Reset.
>
> And if you have busybox, you can use devmem.

CPUX configuration block is at 0x01700000 according to A64 user
manual, and particular register you're interested in is at 0x01700080,
its value is 0x1110110F.

Bits 16-19 are not defined in user manual and are not set.

> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-08 19:59                     ` Vasily Khoruzhick
  0 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-08-08 19:59 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > >
> > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > >>
> > > > > >> Vasily Khoruzhick writes:
> > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > >>>>
> > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > >>>>> as result 'perf top' shows no events.
> > > > > >>>>
> > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > >>>
> > > > > >>> Looks like it does, at least result looks plausible:
> > > > > >>
> > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > >>
> > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > >> surprised I got it wrong.
> > > > > >>
> > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > >
> > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > without interrupts?
> > > > >
> > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > events (the ones which weren't working anyway), and if you measure
> > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > results. But for counting hardware events over relatively short periods
> > > > > it'll still do the job.
> > > >
> > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > stat'"
> > >
> > > I have no idea if that's the culprit, but what is the state of the
> > > 0x09010000 register?
> >
> > What register is that and how do I check it?
>
> It's in the CPUX Configuration block, and the bits are labelled as CPU
> Debug Reset.
>
> And if you have busybox, you can use devmem.

CPUX configuration block is at 0x01700000 according to A64 user
manual, and particular register you're interested in is at 0x01700080,
its value is 0x1110110F.

Bits 16-19 are not defined in user manual and are not set.

> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-08 19:59                     ` Vasily Khoruzhick
  (?)
@ 2019-08-12  8:04                     ` Maxime Ripard
  2019-08-12 18:01                         ` Vasily Khoruzhick
  -1 siblings, 1 reply; 48+ messages in thread
From: Maxime Ripard @ 2019-08-12  8:04 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux


[-- Attachment #1.1: Type: text/plain, Size: 3330 bytes --]

On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > >
> > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > >>
> > > > > > >> Vasily Khoruzhick writes:
> > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > >>>>
> > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > >>>>
> > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > >>>
> > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > >>
> > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > >>
> > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > >> surprised I got it wrong.
> > > > > > >>
> > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > >
> > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > without interrupts?
> > > > > >
> > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > results. But for counting hardware events over relatively short periods
> > > > > > it'll still do the job.
> > > > >
> > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > stat'"
> > > >
> > > > I have no idea if that's the culprit, but what is the state of the
> > > > 0x09010000 register?
> > >
> > > What register is that and how do I check it?
> >
> > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > Debug Reset.
> >
> > And if you have busybox, you can use devmem.
>
> CPUX configuration block is at 0x01700000 according to A64 user
> manual, and particular register you're interested in is at 0x01700080,
> its value is 0x1110110F.
>
> Bits 16-19 are not defined in user manual and are not set.

Sorry, I somehow thought this was for the H6...

I don't have any idea then :/

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-12  8:04                     ` Maxime Ripard
@ 2019-08-12 18:01                         ` Vasily Khoruzhick
  0 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-08-12 18:01 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > >
> > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > >
> > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > >>
> > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > >>>>
> > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > >>>>
> > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > >>>
> > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > >>
> > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > >>
> > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > >> surprised I got it wrong.
> > > > > > > >>
> > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > >
> > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > without interrupts?
> > > > > > >
> > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > it'll still do the job.
> > > > > >
> > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > stat'"
> > > > >
> > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > 0x09010000 register?
> > > >
> > > > What register is that and how do I check it?
> > >
> > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > Debug Reset.
> > >
> > > And if you have busybox, you can use devmem.
> >
> > CPUX configuration block is at 0x01700000 according to A64 user
> > manual, and particular register you're interested in is at 0x01700080,
> > its value is 0x1110110F.
> >
> > Bits 16-19 are not defined in user manual and are not set.
>
> Sorry, I somehow thought this was for the H6...
>
> I don't have any idea then :/

OK, so what should we do? 'perf top'/'perf record' work fine if PMU
node is dropped, but they don't work if PMU node is present (even with
interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
working instead of 'perf stat'

> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-12 18:01                         ` Vasily Khoruzhick
  0 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-08-12 18:01 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > >
> > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > >
> > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > >>
> > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > >>>>
> > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > >>>>
> > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > >>>
> > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > >>
> > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > >>
> > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > >> surprised I got it wrong.
> > > > > > > >>
> > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > >
> > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > without interrupts?
> > > > > > >
> > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > it'll still do the job.
> > > > > >
> > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > stat'"
> > > > >
> > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > 0x09010000 register?
> > > >
> > > > What register is that and how do I check it?
> > >
> > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > Debug Reset.
> > >
> > > And if you have busybox, you can use devmem.
> >
> > CPUX configuration block is at 0x01700000 according to A64 user
> > manual, and particular register you're interested in is at 0x01700080,
> > its value is 0x1110110F.
> >
> > Bits 16-19 are not defined in user manual and are not set.
>
> Sorry, I somehow thought this was for the H6...
>
> I don't have any idea then :/

OK, so what should we do? 'perf top'/'perf record' work fine if PMU
node is dropped, but they don't work if PMU node is present (even with
interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
working instead of 'perf stat'

> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-12 18:01                         ` Vasily Khoruzhick
  (?)
@ 2019-08-12 18:22                         ` Harald Geyer
  -1 siblings, 0 replies; 48+ messages in thread
From: Harald Geyer @ 2019-08-12 18:22 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Robin Murphy, arm-linux

Vasily Khoruzhick writes:
> OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> node is dropped, but they don't work if PMU node is present (even with
> interrupts dropped).

Really? Even if you tell it to only listen to software events? (Which
is the only thing you get without a PMU anyway, I believe.)

> I'd prefer to have 'perf top' and 'perf record'
> working instead of 'perf stat'

I think, if a broken PMU confuses 'perf top' beyond usability, it
should be fixed.

Harald

-- 
Nationalratswahl: Ich trete als unabhängiger Experte für Klimaschutz
und freie Software an! Ich will mit Vorzugsstimmen ins Parlament kommen,
weil wenn ich es nicht mache, kümmert sich womöglich niemand darum.
https://haraldgeyer.at

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-12 18:01                         ` Vasily Khoruzhick
@ 2019-08-13  5:39                           ` Maxime Ripard
  -1 siblings, 0 replies; 48+ messages in thread
From: Maxime Ripard @ 2019-08-13  5:39 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > >
> > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > >
> > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > >>
> > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > >>>>
> > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > >>>>
> > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > >>>
> > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > >>
> > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > >>
> > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > >> surprised I got it wrong.
> > > > > > > > >>
> > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > >
> > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > without interrupts?
> > > > > > > >
> > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > it'll still do the job.
> > > > > > >
> > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > stat'"
> > > > > >
> > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > 0x09010000 register?
> > > > >
> > > > > What register is that and how do I check it?
> > > >
> > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > Debug Reset.
> > > >
> > > > And if you have busybox, you can use devmem.
> > >
> > > CPUX configuration block is at 0x01700000 according to A64 user
> > > manual, and particular register you're interested in is at 0x01700080,
> > > its value is 0x1110110F.
> > >
> > > Bits 16-19 are not defined in user manual and are not set.
> >
> > Sorry, I somehow thought this was for the H6...
> >
> > I don't have any idea then :/
>
> OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> node is dropped, but they don't work if PMU node is present (even with
> interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> working instead of 'perf stat'

Well, it doesn't work so we should just remove the node, and if
someone wants it back, they should figure it out.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-08-13  5:39                           ` Maxime Ripard
  0 siblings, 0 replies; 48+ messages in thread
From: Maxime Ripard @ 2019-08-13  5:39 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > >
> > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > >
> > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > >>
> > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > >>>>
> > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > >>>>
> > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > >>>
> > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > >>
> > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > >>
> > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > >> surprised I got it wrong.
> > > > > > > > >>
> > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > >
> > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > without interrupts?
> > > > > > > >
> > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > it'll still do the job.
> > > > > > >
> > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > stat'"
> > > > > >
> > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > 0x09010000 register?
> > > > >
> > > > > What register is that and how do I check it?
> > > >
> > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > Debug Reset.
> > > >
> > > > And if you have busybox, you can use devmem.
> > >
> > > CPUX configuration block is at 0x01700000 according to A64 user
> > > manual, and particular register you're interested in is at 0x01700080,
> > > its value is 0x1110110F.
> > >
> > > Bits 16-19 are not defined in user manual and are not set.
> >
> > Sorry, I somehow thought this was for the H6...
> >
> > I don't have any idea then :/
>
> OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> node is dropped, but they don't work if PMU node is present (even with
> interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> working instead of 'perf stat'

Well, it doesn't work so we should just remove the node, and if
someone wants it back, they should figure it out.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-08-13  5:39                           ` Maxime Ripard
@ 2019-09-23 23:51                             ` Vasily Khoruzhick
  -1 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-09-23 23:51 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
>
> On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> > On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > >
> > > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > >
> > > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > >
> > > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > > >>
> > > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > >>>>
> > > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > > >>>>
> > > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > > >>>
> > > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > > >>
> > > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > > >>
> > > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > > >> surprised I got it wrong.
> > > > > > > > > >>
> > > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > > >
> > > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > > without interrupts?
> > > > > > > > >
> > > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > > it'll still do the job.
> > > > > > > >
> > > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > > stat'"
> > > > > > >
> > > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > > 0x09010000 register?
> > > > > >
> > > > > > What register is that and how do I check it?
> > > > >
> > > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > > Debug Reset.
> > > > >
> > > > > And if you have busybox, you can use devmem.
> > > >
> > > > CPUX configuration block is at 0x01700000 according to A64 user
> > > > manual, and particular register you're interested in is at 0x01700080,
> > > > its value is 0x1110110F.
> > > >
> > > > Bits 16-19 are not defined in user manual and are not set.
> > >
> > > Sorry, I somehow thought this was for the H6...
> > >
> > > I don't have any idea then :/
> >
> > OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> > node is dropped, but they don't work if PMU node is present (even with
> > interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> > working instead of 'perf stat'
>
> Well, it doesn't work so we should just remove the node, and if
> someone wants it back, they should figure it out.

Hey Maxime,

So can you merge this patch?

> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-09-23 23:51                             ` Vasily Khoruzhick
  0 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-09-23 23:51 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
>
> On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> > On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > >
> > > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > >
> > > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > >
> > > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > > >>
> > > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > >>>>
> > > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > > >>>>
> > > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > > >>>
> > > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > > >>
> > > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > > >>
> > > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > > >> surprised I got it wrong.
> > > > > > > > > >>
> > > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > > >
> > > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > > without interrupts?
> > > > > > > > >
> > > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > > it'll still do the job.
> > > > > > > >
> > > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > > stat'"
> > > > > > >
> > > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > > 0x09010000 register?
> > > > > >
> > > > > > What register is that and how do I check it?
> > > > >
> > > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > > Debug Reset.
> > > > >
> > > > > And if you have busybox, you can use devmem.
> > > >
> > > > CPUX configuration block is at 0x01700000 according to A64 user
> > > > manual, and particular register you're interested in is at 0x01700080,
> > > > its value is 0x1110110F.
> > > >
> > > > Bits 16-19 are not defined in user manual and are not set.
> > >
> > > Sorry, I somehow thought this was for the H6...
> > >
> > > I don't have any idea then :/
> >
> > OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> > node is dropped, but they don't work if PMU node is present (even with
> > interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> > working instead of 'perf stat'
>
> Well, it doesn't work so we should just remove the node, and if
> someone wants it back, they should figure it out.

Hey Maxime,

So can you merge this patch?

> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-09-23 23:51                             ` Vasily Khoruzhick
@ 2019-09-23 23:55                               ` Vasily Khoruzhick
  -1 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-09-23 23:55 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Mon, Sep 23, 2019 at 4:51 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>
> On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
> >
> > On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> > > On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > >
> > > > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > >
> > > > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > > > >>
> > > > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > >>>>
> > > > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > > > >>>>
> > > > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > > > >>>
> > > > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > > > >>
> > > > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > > > >>
> > > > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > > > >> surprised I got it wrong.
> > > > > > > > > > >>
> > > > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > > > >
> > > > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > > > without interrupts?
> > > > > > > > > >
> > > > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > > > it'll still do the job.
> > > > > > > > >
> > > > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > > > stat'"
> > > > > > > >
> > > > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > > > 0x09010000 register?
> > > > > > >
> > > > > > > What register is that and how do I check it?
> > > > > >
> > > > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > > > Debug Reset.
> > > > > >
> > > > > > And if you have busybox, you can use devmem.
> > > > >
> > > > > CPUX configuration block is at 0x01700000 according to A64 user
> > > > > manual, and particular register you're interested in is at 0x01700080,
> > > > > its value is 0x1110110F.
> > > > >
> > > > > Bits 16-19 are not defined in user manual and are not set.
> > > >
> > > > Sorry, I somehow thought this was for the H6...
> > > >
> > > > I don't have any idea then :/
> > >
> > > OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> > > node is dropped, but they don't work if PMU node is present (even with
> > > interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> > > working instead of 'perf stat'
> >
> > Well, it doesn't work so we should just remove the node, and if
> > someone wants it back, they should figure it out.
>
> Hey Maxime,
>
> So can you merge this patch?

Added new Maxime's email to CC

> > Maxime
> >
> > --
> > Maxime Ripard, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-09-23 23:55                               ` Vasily Khoruzhick
  0 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-09-23 23:55 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Mon, Sep 23, 2019 at 4:51 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>
> On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
> >
> > On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> > > On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > >
> > > > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > >
> > > > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > > > >>
> > > > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > >>>>
> > > > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > > > >>>>
> > > > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > > > >>>
> > > > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > > > >>
> > > > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > > > >>
> > > > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > > > >> surprised I got it wrong.
> > > > > > > > > > >>
> > > > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > > > >
> > > > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > > > without interrupts?
> > > > > > > > > >
> > > > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > > > it'll still do the job.
> > > > > > > > >
> > > > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > > > stat'"
> > > > > > > >
> > > > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > > > 0x09010000 register?
> > > > > > >
> > > > > > > What register is that and how do I check it?
> > > > > >
> > > > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > > > Debug Reset.
> > > > > >
> > > > > > And if you have busybox, you can use devmem.
> > > > >
> > > > > CPUX configuration block is at 0x01700000 according to A64 user
> > > > > manual, and particular register you're interested in is at 0x01700080,
> > > > > its value is 0x1110110F.
> > > > >
> > > > > Bits 16-19 are not defined in user manual and are not set.
> > > >
> > > > Sorry, I somehow thought this was for the H6...
> > > >
> > > > I don't have any idea then :/
> > >
> > > OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> > > node is dropped, but they don't work if PMU node is present (even with
> > > interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> > > working instead of 'perf stat'
> >
> > Well, it doesn't work so we should just remove the node, and if
> > someone wants it back, they should figure it out.
>
> Hey Maxime,
>
> So can you merge this patch?

Added new Maxime's email to CC

> > Maxime
> >
> > --
> > Maxime Ripard, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-09-23 23:55                               ` Vasily Khoruzhick
  (?)
@ 2019-09-25 11:08                               ` Maxime Ripard
  2019-10-31 19:10                                   ` Clément Péron
  -1 siblings, 1 reply; 48+ messages in thread
From: Maxime Ripard @ 2019-09-25 11:08 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux


[-- Attachment #1.1: Type: text/plain, Size: 4868 bytes --]

On Mon, Sep 23, 2019 at 04:55:59PM -0700, Vasily Khoruzhick wrote:
> On Mon, Sep 23, 2019 at 4:51 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> >
> > On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
> > <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> > > > On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > >
> > > > > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > > > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > >
> > > > > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > > > > >>
> > > > > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > > >>>>
> > > > > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > > > > >>>>
> > > > > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > > > > >>>
> > > > > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > > > > >>
> > > > > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > > > > >>
> > > > > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > > > > >> surprised I got it wrong.
> > > > > > > > > > > >>
> > > > > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > > > > >
> > > > > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > > > > without interrupts?
> > > > > > > > > > >
> > > > > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > > > > it'll still do the job.
> > > > > > > > > >
> > > > > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > > > > stat'"
> > > > > > > > >
> > > > > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > > > > 0x09010000 register?
> > > > > > > >
> > > > > > > > What register is that and how do I check it?
> > > > > > >
> > > > > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > > > > Debug Reset.
> > > > > > >
> > > > > > > And if you have busybox, you can use devmem.
> > > > > >
> > > > > > CPUX configuration block is at 0x01700000 according to A64 user
> > > > > > manual, and particular register you're interested in is at 0x01700080,
> > > > > > its value is 0x1110110F.
> > > > > >
> > > > > > Bits 16-19 are not defined in user manual and are not set.
> > > > >
> > > > > Sorry, I somehow thought this was for the H6...
> > > > >
> > > > > I don't have any idea then :/
> > > >
> > > > OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> > > > node is dropped, but they don't work if PMU node is present (even with
> > > > interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> > > > working instead of 'perf stat'
> > >
> > > Well, it doesn't work so we should just remove the node, and if
> > > someone wants it back, they should figure it out.
> >
> > Hey Maxime,
> >
> > So can you merge this patch?
>
> Added new Maxime's email to CC

Queued as a fix for 5.4, thanks!
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-09-25 11:08                               ` Maxime Ripard
@ 2019-10-31 19:10                                   ` Clément Péron
  0 siblings, 0 replies; 48+ messages in thread
From: Clément Péron @ 2019-10-31 19:10 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Vasily Khoruzhick, Mark Rutland, devicetree, Jared D . McNeill,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, Robin Murphy, arm-linux

Hi,

Just a remark here but the interrupt are from 152 to 155 SPI.
But there is an offset of 32 no (remove SGI/PPI)?
This should be from 120 to 123

Regards,
Clément

On Wed, 25 Sep 2019 at 13:09, Maxime Ripard <mripard@kernel.org> wrote:
>
> On Mon, Sep 23, 2019 at 04:55:59PM -0700, Vasily Khoruzhick wrote:
> > On Mon, Sep 23, 2019 at 4:51 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> > >
> > > On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
> > > <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> > > > > On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > >
> > > > > > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > > > > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > >
> > > > > > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > > > >>>>
> > > > > > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > > > > > >>>>
> > > > > > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > > > > > >>>
> > > > > > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > > > > > >> surprised I got it wrong.
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > > > > > >
> > > > > > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > > > > > without interrupts?
> > > > > > > > > > > >
> > > > > > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > > > > > it'll still do the job.
> > > > > > > > > > >
> > > > > > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > > > > > stat'"
> > > > > > > > > >
> > > > > > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > > > > > 0x09010000 register?
> > > > > > > > >
> > > > > > > > > What register is that and how do I check it?
> > > > > > > >
> > > > > > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > > > > > Debug Reset.
> > > > > > > >
> > > > > > > > And if you have busybox, you can use devmem.
> > > > > > >
> > > > > > > CPUX configuration block is at 0x01700000 according to A64 user
> > > > > > > manual, and particular register you're interested in is at 0x01700080,
> > > > > > > its value is 0x1110110F.
> > > > > > >
> > > > > > > Bits 16-19 are not defined in user manual and are not set.
> > > > > >
> > > > > > Sorry, I somehow thought this was for the H6...
> > > > > >
> > > > > > I don't have any idea then :/
> > > > >
> > > > > OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> > > > > node is dropped, but they don't work if PMU node is present (even with
> > > > > interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> > > > > working instead of 'perf stat'
> > > >
> > > > Well, it doesn't work so we should just remove the node, and if
> > > > someone wants it back, they should figure it out.
> > >
> > > Hey Maxime,
> > >
> > > So can you merge this patch?
> >
> > Added new Maxime's email to CC
>
> Queued as a fix for 5.4, thanks!
> Maxime
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-10-31 19:10                                   ` Clément Péron
  0 siblings, 0 replies; 48+ messages in thread
From: Clément Péron @ 2019-10-31 19:10 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, Jared D . McNeill, devicetree, Chen-Yu Tsai,
	Rob Herring, Harald Geyer, Robin Murphy, arm-linux

Hi,

Just a remark here but the interrupt are from 152 to 155 SPI.
But there is an offset of 32 no (remove SGI/PPI)?
This should be from 120 to 123

Regards,
Clément

On Wed, 25 Sep 2019 at 13:09, Maxime Ripard <mripard@kernel.org> wrote:
>
> On Mon, Sep 23, 2019 at 04:55:59PM -0700, Vasily Khoruzhick wrote:
> > On Mon, Sep 23, 2019 at 4:51 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> > >
> > > On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
> > > <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> > > > > On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > >
> > > > > > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > > > > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > >
> > > > > > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > > > >>>>
> > > > > > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > > > > > >>>>
> > > > > > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > > > > > >>>
> > > > > > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > > > > > >> surprised I got it wrong.
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > > > > > >
> > > > > > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > > > > > without interrupts?
> > > > > > > > > > > >
> > > > > > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > > > > > it'll still do the job.
> > > > > > > > > > >
> > > > > > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > > > > > stat'"
> > > > > > > > > >
> > > > > > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > > > > > 0x09010000 register?
> > > > > > > > >
> > > > > > > > > What register is that and how do I check it?
> > > > > > > >
> > > > > > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > > > > > Debug Reset.
> > > > > > > >
> > > > > > > > And if you have busybox, you can use devmem.
> > > > > > >
> > > > > > > CPUX configuration block is at 0x01700000 according to A64 user
> > > > > > > manual, and particular register you're interested in is at 0x01700080,
> > > > > > > its value is 0x1110110F.
> > > > > > >
> > > > > > > Bits 16-19 are not defined in user manual and are not set.
> > > > > >
> > > > > > Sorry, I somehow thought this was for the H6...
> > > > > >
> > > > > > I don't have any idea then :/
> > > > >
> > > > > OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> > > > > node is dropped, but they don't work if PMU node is present (even with
> > > > > interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> > > > > working instead of 'perf stat'
> > > >
> > > > Well, it doesn't work so we should just remove the node, and if
> > > > someone wants it back, they should figure it out.
> > >
> > > Hey Maxime,
> > >
> > > So can you merge this patch?
> >
> > Added new Maxime's email to CC
>
> Queued as a fix for 5.4, thanks!
> Maxime
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-10-31 19:10                                   ` Clément Péron
@ 2019-10-31 20:35                                     ` Vasily Khoruzhick
  -1 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-10-31 20:35 UTC (permalink / raw)
  To: Clément Péron
  Cc: Maxime Ripard, Mark Rutland, devicetree, Jared D . McNeill,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Thu, Oct 31, 2019 at 12:10 PM Clément Péron <peron.clem@gmail.com> wrote:
>
> Hi,
>
> Just a remark here but the interrupt are from 152 to 155 SPI.
> But there is an offset of 32 no (remove SGI/PPI)?
> This should be from 120 to 123

I already tried it (and I believe someone already suggested it above),
it doesn't fix PMU interrupts though.

> Regards,
> Clément
>
> On Wed, 25 Sep 2019 at 13:09, Maxime Ripard <mripard@kernel.org> wrote:
> >
> > On Mon, Sep 23, 2019 at 04:55:59PM -0700, Vasily Khoruzhick wrote:
> > > On Mon, Sep 23, 2019 at 4:51 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> > > >
> > > > On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
> > > > <maxime.ripard@bootlin.com> wrote:
> > > > >
> > > > > On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> > > > > > On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > >
> > > > > > > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > > >
> > > > > > > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > > > > > > >>
> > > > > > > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > > > > > > >>>
> > > > > > > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > > > > > > >>
> > > > > > > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > > > > > > >>
> > > > > > > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > > > > > > >> surprised I got it wrong.
> > > > > > > > > > > > > >>
> > > > > > > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > > > > > > without interrupts?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > > > > > > it'll still do the job.
> > > > > > > > > > > >
> > > > > > > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > > > > > > stat'"
> > > > > > > > > > >
> > > > > > > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > > > > > > 0x09010000 register?
> > > > > > > > > >
> > > > > > > > > > What register is that and how do I check it?
> > > > > > > > >
> > > > > > > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > > > > > > Debug Reset.
> > > > > > > > >
> > > > > > > > > And if you have busybox, you can use devmem.
> > > > > > > >
> > > > > > > > CPUX configuration block is at 0x01700000 according to A64 user
> > > > > > > > manual, and particular register you're interested in is at 0x01700080,
> > > > > > > > its value is 0x1110110F.
> > > > > > > >
> > > > > > > > Bits 16-19 are not defined in user manual and are not set.
> > > > > > >
> > > > > > > Sorry, I somehow thought this was for the H6...
> > > > > > >
> > > > > > > I don't have any idea then :/
> > > > > >
> > > > > > OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> > > > > > node is dropped, but they don't work if PMU node is present (even with
> > > > > > interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> > > > > > working instead of 'perf stat'
> > > > >
> > > > > Well, it doesn't work so we should just remove the node, and if
> > > > > someone wants it back, they should figure it out.
> > > >
> > > > Hey Maxime,
> > > >
> > > > So can you merge this patch?
> > >
> > > Added new Maxime's email to CC
> >
> > Queued as a fix for 5.4, thanks!
> > Maxime
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-10-31 20:35                                     ` Vasily Khoruzhick
  0 siblings, 0 replies; 48+ messages in thread
From: Vasily Khoruzhick @ 2019-10-31 20:35 UTC (permalink / raw)
  To: Clément Péron
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On Thu, Oct 31, 2019 at 12:10 PM Clément Péron <peron.clem@gmail.com> wrote:
>
> Hi,
>
> Just a remark here but the interrupt are from 152 to 155 SPI.
> But there is an offset of 32 no (remove SGI/PPI)?
> This should be from 120 to 123

I already tried it (and I believe someone already suggested it above),
it doesn't fix PMU interrupts though.

> Regards,
> Clément
>
> On Wed, 25 Sep 2019 at 13:09, Maxime Ripard <mripard@kernel.org> wrote:
> >
> > On Mon, Sep 23, 2019 at 04:55:59PM -0700, Vasily Khoruzhick wrote:
> > > On Mon, Sep 23, 2019 at 4:51 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> > > >
> > > > On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
> > > > <maxime.ripard@bootlin.com> wrote:
> > > > >
> > > > > On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> > > > > > On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > >
> > > > > > > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > > >
> > > > > > > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > > > > > > >>
> > > > > > > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > > > > > > >>>
> > > > > > > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > > > > > > >>
> > > > > > > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > > > > > > >>
> > > > > > > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > > > > > > >> surprised I got it wrong.
> > > > > > > > > > > > > >>
> > > > > > > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > > > > > > without interrupts?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > > > > > > it'll still do the job.
> > > > > > > > > > > >
> > > > > > > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > > > > > > stat'"
> > > > > > > > > > >
> > > > > > > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > > > > > > 0x09010000 register?
> > > > > > > > > >
> > > > > > > > > > What register is that and how do I check it?
> > > > > > > > >
> > > > > > > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > > > > > > Debug Reset.
> > > > > > > > >
> > > > > > > > > And if you have busybox, you can use devmem.
> > > > > > > >
> > > > > > > > CPUX configuration block is at 0x01700000 according to A64 user
> > > > > > > > manual, and particular register you're interested in is at 0x01700080,
> > > > > > > > its value is 0x1110110F.
> > > > > > > >
> > > > > > > > Bits 16-19 are not defined in user manual and are not set.
> > > > > > >
> > > > > > > Sorry, I somehow thought this was for the H6...
> > > > > > >
> > > > > > > I don't have any idea then :/
> > > > > >
> > > > > > OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> > > > > > node is dropped, but they don't work if PMU node is present (even with
> > > > > > interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> > > > > > working instead of 'perf stat'
> > > > >
> > > > > Well, it doesn't work so we should just remove the node, and if
> > > > > someone wants it back, they should figure it out.
> > > >
> > > > Hey Maxime,
> > > >
> > > > So can you merge this patch?
> > >
> > > Added new Maxime's email to CC
> >
> > Queued as a fix for 5.4, thanks!
> > Maxime
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-10-31 20:35                                     ` Vasily Khoruzhick
@ 2019-11-01 11:30                                       ` Clément Péron
  -1 siblings, 0 replies; 48+ messages in thread
From: Clément Péron @ 2019-11-01 11:30 UTC (permalink / raw)
  To: Vasily Khoruzhick, Andre Przywara
  Cc: Maxime Ripard, Mark Rutland, devicetree, Jared D . McNeill,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, Robin Murphy, arm-linux

Hi

On Thu, 31 Oct 2019 at 21:35, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>
> On Thu, Oct 31, 2019 at 12:10 PM Clément Péron <peron.clem@gmail.com> wrote:
> >
> > Hi,
> >
> > Just a remark here but the interrupt are from 152 to 155 SPI.
> > But there is an offset of 32 no (remove SGI/PPI)?
> > This should be from 120 to 123
>
> I already tried it (and I believe someone already suggested it above),
> it doesn't fix PMU interrupts though.

Ok thanks for the confirmation.

Made a research about the PMU for A64 and found that Andre Przywara
made a patch to enable it:
https://gist.github.com/apritzel/d025abaa1425fcaf5991b5ffcf18a0a3

Maybe he can confirm or not the issue on A64 ?

Regards,
Clément

>
> > Regards,
> > Clément
> >
> > On Wed, 25 Sep 2019 at 13:09, Maxime Ripard <mripard@kernel.org> wrote:
> > >
> > > On Mon, Sep 23, 2019 at 04:55:59PM -0700, Vasily Khoruzhick wrote:
> > > > On Mon, Sep 23, 2019 at 4:51 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> > > > >
> > > > > On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
> > > > > <maxime.ripard@bootlin.com> wrote:
> > > > > >
> > > > > > On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> > > > > > > On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > >
> > > > > > > > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > > > > > > > >>>
> > > > > > > > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > > > > > > > >> surprised I got it wrong.
> > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > > > > > > > without interrupts?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > > > > > > > it'll still do the job.
> > > > > > > > > > > > >
> > > > > > > > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > > > > > > > stat'"
> > > > > > > > > > > >
> > > > > > > > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > > > > > > > 0x09010000 register?
> > > > > > > > > > >
> > > > > > > > > > > What register is that and how do I check it?
> > > > > > > > > >
> > > > > > > > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > > > > > > > Debug Reset.
> > > > > > > > > >
> > > > > > > > > > And if you have busybox, you can use devmem.
> > > > > > > > >
> > > > > > > > > CPUX configuration block is at 0x01700000 according to A64 user
> > > > > > > > > manual, and particular register you're interested in is at 0x01700080,
> > > > > > > > > its value is 0x1110110F.
> > > > > > > > >
> > > > > > > > > Bits 16-19 are not defined in user manual and are not set.
> > > > > > > >
> > > > > > > > Sorry, I somehow thought this was for the H6...
> > > > > > > >
> > > > > > > > I don't have any idea then :/
> > > > > > >
> > > > > > > OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> > > > > > > node is dropped, but they don't work if PMU node is present (even with
> > > > > > > interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> > > > > > > working instead of 'perf stat'
> > > > > >
> > > > > > Well, it doesn't work so we should just remove the node, and if
> > > > > > someone wants it back, they should figure it out.
> > > > >
> > > > > Hey Maxime,
> > > > >
> > > > > So can you merge this patch?
> > > >
> > > > Added new Maxime's email to CC
> > >
> > > Queued as a fix for 5.4, thanks!
> > > Maxime
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-11-01 11:30                                       ` Clément Péron
  0 siblings, 0 replies; 48+ messages in thread
From: Clément Péron @ 2019-11-01 11:30 UTC (permalink / raw)
  To: Vasily Khoruzhick, Andre Przywara
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, Robin Murphy, arm-linux

Hi

On Thu, 31 Oct 2019 at 21:35, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>
> On Thu, Oct 31, 2019 at 12:10 PM Clément Péron <peron.clem@gmail.com> wrote:
> >
> > Hi,
> >
> > Just a remark here but the interrupt are from 152 to 155 SPI.
> > But there is an offset of 32 no (remove SGI/PPI)?
> > This should be from 120 to 123
>
> I already tried it (and I believe someone already suggested it above),
> it doesn't fix PMU interrupts though.

Ok thanks for the confirmation.

Made a research about the PMU for A64 and found that Andre Przywara
made a patch to enable it:
https://gist.github.com/apritzel/d025abaa1425fcaf5991b5ffcf18a0a3

Maybe he can confirm or not the issue on A64 ?

Regards,
Clément

>
> > Regards,
> > Clément
> >
> > On Wed, 25 Sep 2019 at 13:09, Maxime Ripard <mripard@kernel.org> wrote:
> > >
> > > On Mon, Sep 23, 2019 at 04:55:59PM -0700, Vasily Khoruzhick wrote:
> > > > On Mon, Sep 23, 2019 at 4:51 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> > > > >
> > > > > On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
> > > > > <maxime.ripard@bootlin.com> wrote:
> > > > > >
> > > > > > On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
> > > > > > > On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > >
> > > > > > > > On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > > > On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
> > > > > > > > > > > > > On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
> > > > > > > > > > > > > > > On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
> > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > >> Vasily Khoruzhick writes:
> > > > > > > > > > > > > > >>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > > >>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
> > > > > > > > > > > > > > >>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
> > > > > > > > > > > > > > >>>>> as result 'perf top' shows no events.
> > > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > > >>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
> > > > > > > > > > > > > > >>>> It could well just be that the interrupt numbers are wrong...
> > > > > > > > > > > > > > >>>
> > > > > > > > > > > > > > >>> Looks like it does, at least result looks plausible:
> > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > >> I'm using perf stat regularly (cache benchmarks) and it works fine.
> > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > >> Unfortunately I wasn't aware that perf stat is a poor test for
> > > > > > > > > > > > > > >> the interrupts part of the node, when I added it. So I'm not too
> > > > > > > > > > > > > > >> surprised I got it wrong.
> > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > >> However, it would be unfortunate if the node got removed completely,
> > > > > > > > > > > > > > >> because perf stat would not work anymore. Maybe we can only remove
> > > > > > > > > > > > > > >> the interrupts or just fix them even if the HW doesn't work?
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > I'm not familiar with PMU driver. Is it possible to get it working
> > > > > > > > > > > > > > > without interrupts?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Yup - you get a grumpy message from the driver, it will refuse sampling
> > > > > > > > > > > > > > events (the ones which weren't working anyway), and if you measure
> > > > > > > > > > > > > > anything for long enough that a counter overflows you'll get wonky
> > > > > > > > > > > > > > results. But for counting hardware events over relatively short periods
> > > > > > > > > > > > > > it'll still do the job.
> > > > > > > > > > > > >
> > > > > > > > > > > > > I tried to drop interrupts completely from the node but 'perf top' is
> > > > > > > > > > > > > still broken. Though now in different way: it complains "cycles: PMU
> > > > > > > > > > > > > Hardware doesn't support sampling/overflow-interrupts. Try 'perf
> > > > > > > > > > > > > stat'"
> > > > > > > > > > > >
> > > > > > > > > > > > I have no idea if that's the culprit, but what is the state of the
> > > > > > > > > > > > 0x09010000 register?
> > > > > > > > > > >
> > > > > > > > > > > What register is that and how do I check it?
> > > > > > > > > >
> > > > > > > > > > It's in the CPUX Configuration block, and the bits are labelled as CPU
> > > > > > > > > > Debug Reset.
> > > > > > > > > >
> > > > > > > > > > And if you have busybox, you can use devmem.
> > > > > > > > >
> > > > > > > > > CPUX configuration block is at 0x01700000 according to A64 user
> > > > > > > > > manual, and particular register you're interested in is at 0x01700080,
> > > > > > > > > its value is 0x1110110F.
> > > > > > > > >
> > > > > > > > > Bits 16-19 are not defined in user manual and are not set.
> > > > > > > >
> > > > > > > > Sorry, I somehow thought this was for the H6...
> > > > > > > >
> > > > > > > > I don't have any idea then :/
> > > > > > >
> > > > > > > OK, so what should we do? 'perf top'/'perf record' work fine if PMU
> > > > > > > node is dropped, but they don't work if PMU node is present (even with
> > > > > > > interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
> > > > > > > working instead of 'perf stat'
> > > > > >
> > > > > > Well, it doesn't work so we should just remove the node, and if
> > > > > > someone wants it back, they should figure it out.
> > > > >
> > > > > Hey Maxime,
> > > > >
> > > > > So can you merge this patch?
> > > >
> > > > Added new Maxime's email to CC
> > >
> > > Queued as a fix for 5.4, thanks!
> > > Maxime
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-11-01 11:30                                       ` Clément Péron
@ 2019-11-01 15:47                                         ` Andre Przywara
  -1 siblings, 0 replies; 48+ messages in thread
From: Andre Przywara @ 2019-11-01 15:47 UTC (permalink / raw)
  To: Clément Péron, Vasily Khoruzhick
  Cc: Maxime Ripard, Mark Rutland, devicetree, Jared D . McNeill,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On 11/1/19 11:30 AM, Clément Péron wrote:

Hi,

> On Thu, 31 Oct 2019 at 21:35, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>>
>> On Thu, Oct 31, 2019 at 12:10 PM Clément Péron <peron.clem@gmail.com> wrote:
>>>
>>> Hi,
>>>
>>> Just a remark here but the interrupt are from 152 to 155 SPI.
>>> But there is an offset of 32 no (remove SGI/PPI)?
>>> This should be from 120 to 123
>>
>> I already tried it (and I believe someone already suggested it above),
>> it doesn't fix PMU interrupts though.
> 
> Ok thanks for the confirmation.
> 
> Made a research about the PMU for A64 and found that Andre Przywara
> made a patch to enable it:
> https://gist.github.com/apritzel/d025abaa1425fcaf5991b5ffcf18a0a3
> 
> Maybe he can confirm or not the issue on A64 ?

Well, I tried it back then, but couldn't make the interrupts work (and 
yes, I tried +/- 32). That's the reason I didn't send it back then.

I can't say whether the IRQ lines are not wired or the manual just gives 
the wrong numbers. I don't have access to a board before Sunday, but if 
someone wants to beat me to it:
- Hack U-Boot to add a command to program one PMU counter to expire 
quickly, and enable overflow interrupts.
- Enable *all* SPIs on the GIC distributor level, and enable the 
distributor. Keep the GIC CPU interface disabled.
- Trigger the U-Boot command, and inspect the GICD_ISPENDR registers to 
see if any SPI fired.
- Also double check the PMU overflow status register to verify that the 
event triggered.

Cheers,
Andre.

> 
> Regards,
> Clément
> 
>>
>>> Regards,
>>> Clément
>>>
>>> On Wed, 25 Sep 2019 at 13:09, Maxime Ripard <mripard@kernel.org> wrote:
>>>>
>>>> On Mon, Sep 23, 2019 at 04:55:59PM -0700, Vasily Khoruzhick wrote:
>>>>> On Mon, Sep 23, 2019 at 4:51 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>>>>>>
>>>>>> On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
>>>>>> <maxime.ripard@bootlin.com> wrote:
>>>>>>>
>>>>>>> On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
>>>>>>>> On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>>>>>>>>>
>>>>>>>>> On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
>>>>>>>>>> On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
>>>>>>>>>>>> On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
>>>>>>>>>>>>>> On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
>>>>>>>>>>>>>>>> On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Vasily Khoruzhick writes:
>>>>>>>>>>>>>>>>>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
>>>>>>>>>>>>>>>>>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
>>>>>>>>>>>>>>>>>>>> as result 'perf top' shows no events.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
>>>>>>>>>>>>>>>>>>> It could well just be that the interrupt numbers are wrong...
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Looks like it does, at least result looks plausible:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I'm using perf stat regularly (cache benchmarks) and it works fine.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Unfortunately I wasn't aware that perf stat is a poor test for
>>>>>>>>>>>>>>>>> the interrupts part of the node, when I added it. So I'm not too
>>>>>>>>>>>>>>>>> surprised I got it wrong.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> However, it would be unfortunate if the node got removed completely,
>>>>>>>>>>>>>>>>> because perf stat would not work anymore. Maybe we can only remove
>>>>>>>>>>>>>>>>> the interrupts or just fix them even if the HW doesn't work?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I'm not familiar with PMU driver. Is it possible to get it working
>>>>>>>>>>>>>>>> without interrupts?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Yup - you get a grumpy message from the driver, it will refuse sampling
>>>>>>>>>>>>>>> events (the ones which weren't working anyway), and if you measure
>>>>>>>>>>>>>>> anything for long enough that a counter overflows you'll get wonky
>>>>>>>>>>>>>>> results. But for counting hardware events over relatively short periods
>>>>>>>>>>>>>>> it'll still do the job.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I tried to drop interrupts completely from the node but 'perf top' is
>>>>>>>>>>>>>> still broken. Though now in different way: it complains "cycles: PMU
>>>>>>>>>>>>>> Hardware doesn't support sampling/overflow-interrupts. Try 'perf
>>>>>>>>>>>>>> stat'"
>>>>>>>>>>>>>
>>>>>>>>>>>>> I have no idea if that's the culprit, but what is the state of the
>>>>>>>>>>>>> 0x09010000 register?
>>>>>>>>>>>>
>>>>>>>>>>>> What register is that and how do I check it?
>>>>>>>>>>>
>>>>>>>>>>> It's in the CPUX Configuration block, and the bits are labelled as CPU
>>>>>>>>>>> Debug Reset.
>>>>>>>>>>>
>>>>>>>>>>> And if you have busybox, you can use devmem.
>>>>>>>>>>
>>>>>>>>>> CPUX configuration block is at 0x01700000 according to A64 user
>>>>>>>>>> manual, and particular register you're interested in is at 0x01700080,
>>>>>>>>>> its value is 0x1110110F.
>>>>>>>>>>
>>>>>>>>>> Bits 16-19 are not defined in user manual and are not set.
>>>>>>>>>
>>>>>>>>> Sorry, I somehow thought this was for the H6...
>>>>>>>>>
>>>>>>>>> I don't have any idea then :/
>>>>>>>>
>>>>>>>> OK, so what should we do? 'perf top'/'perf record' work fine if PMU
>>>>>>>> node is dropped, but they don't work if PMU node is present (even with
>>>>>>>> interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
>>>>>>>> working instead of 'perf stat'
>>>>>>>
>>>>>>> Well, it doesn't work so we should just remove the node, and if
>>>>>>> someone wants it back, they should figure it out.
>>>>>>
>>>>>> Hey Maxime,
>>>>>>
>>>>>> So can you merge this patch?
>>>>>
>>>>> Added new Maxime's email to CC
>>>>
>>>> Queued as a fix for 5.4, thanks!
>>>> Maxime
>>>> _______________________________________________
>>>> linux-arm-kernel mailing list
>>>> linux-arm-kernel@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-11-01 15:47                                         ` Andre Przywara
  0 siblings, 0 replies; 48+ messages in thread
From: Andre Przywara @ 2019-11-01 15:47 UTC (permalink / raw)
  To: Clément Péron, Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, Robin Murphy, arm-linux

On 11/1/19 11:30 AM, Clément Péron wrote:

Hi,

> On Thu, 31 Oct 2019 at 21:35, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>>
>> On Thu, Oct 31, 2019 at 12:10 PM Clément Péron <peron.clem@gmail.com> wrote:
>>>
>>> Hi,
>>>
>>> Just a remark here but the interrupt are from 152 to 155 SPI.
>>> But there is an offset of 32 no (remove SGI/PPI)?
>>> This should be from 120 to 123
>>
>> I already tried it (and I believe someone already suggested it above),
>> it doesn't fix PMU interrupts though.
> 
> Ok thanks for the confirmation.
> 
> Made a research about the PMU for A64 and found that Andre Przywara
> made a patch to enable it:
> https://gist.github.com/apritzel/d025abaa1425fcaf5991b5ffcf18a0a3
> 
> Maybe he can confirm or not the issue on A64 ?

Well, I tried it back then, but couldn't make the interrupts work (and 
yes, I tried +/- 32). That's the reason I didn't send it back then.

I can't say whether the IRQ lines are not wired or the manual just gives 
the wrong numbers. I don't have access to a board before Sunday, but if 
someone wants to beat me to it:
- Hack U-Boot to add a command to program one PMU counter to expire 
quickly, and enable overflow interrupts.
- Enable *all* SPIs on the GIC distributor level, and enable the 
distributor. Keep the GIC CPU interface disabled.
- Trigger the U-Boot command, and inspect the GICD_ISPENDR registers to 
see if any SPI fired.
- Also double check the PMU overflow status register to verify that the 
event triggered.

Cheers,
Andre.

> 
> Regards,
> Clément
> 
>>
>>> Regards,
>>> Clément
>>>
>>> On Wed, 25 Sep 2019 at 13:09, Maxime Ripard <mripard@kernel.org> wrote:
>>>>
>>>> On Mon, Sep 23, 2019 at 04:55:59PM -0700, Vasily Khoruzhick wrote:
>>>>> On Mon, Sep 23, 2019 at 4:51 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>>>>>>
>>>>>> On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
>>>>>> <maxime.ripard@bootlin.com> wrote:
>>>>>>>
>>>>>>> On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
>>>>>>>> On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>>>>>>>>>
>>>>>>>>> On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick wrote:
>>>>>>>>>> On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick wrote:
>>>>>>>>>>>> On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily Khoruzhick wrote:
>>>>>>>>>>>>>> On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
>>>>>>>>>>>>>>>> On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer <harald@ccbib.org> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Vasily Khoruzhick writes:
>>>>>>>>>>>>>>>>>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote:
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
>>>>>>>>>>>>>>>>>>>> Looks like PMU in A64 is broken, it generates no interrupts at all and
>>>>>>>>>>>>>>>>>>>> as result 'perf top' shows no events.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Does something like 'perf stat sleep 1' at least count cycles correctly?
>>>>>>>>>>>>>>>>>>> It could well just be that the interrupt numbers are wrong...
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Looks like it does, at least result looks plausible:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I'm using perf stat regularly (cache benchmarks) and it works fine.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Unfortunately I wasn't aware that perf stat is a poor test for
>>>>>>>>>>>>>>>>> the interrupts part of the node, when I added it. So I'm not too
>>>>>>>>>>>>>>>>> surprised I got it wrong.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> However, it would be unfortunate if the node got removed completely,
>>>>>>>>>>>>>>>>> because perf stat would not work anymore. Maybe we can only remove
>>>>>>>>>>>>>>>>> the interrupts or just fix them even if the HW doesn't work?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I'm not familiar with PMU driver. Is it possible to get it working
>>>>>>>>>>>>>>>> without interrupts?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Yup - you get a grumpy message from the driver, it will refuse sampling
>>>>>>>>>>>>>>> events (the ones which weren't working anyway), and if you measure
>>>>>>>>>>>>>>> anything for long enough that a counter overflows you'll get wonky
>>>>>>>>>>>>>>> results. But for counting hardware events over relatively short periods
>>>>>>>>>>>>>>> it'll still do the job.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I tried to drop interrupts completely from the node but 'perf top' is
>>>>>>>>>>>>>> still broken. Though now in different way: it complains "cycles: PMU
>>>>>>>>>>>>>> Hardware doesn't support sampling/overflow-interrupts. Try 'perf
>>>>>>>>>>>>>> stat'"
>>>>>>>>>>>>>
>>>>>>>>>>>>> I have no idea if that's the culprit, but what is the state of the
>>>>>>>>>>>>> 0x09010000 register?
>>>>>>>>>>>>
>>>>>>>>>>>> What register is that and how do I check it?
>>>>>>>>>>>
>>>>>>>>>>> It's in the CPUX Configuration block, and the bits are labelled as CPU
>>>>>>>>>>> Debug Reset.
>>>>>>>>>>>
>>>>>>>>>>> And if you have busybox, you can use devmem.
>>>>>>>>>>
>>>>>>>>>> CPUX configuration block is at 0x01700000 according to A64 user
>>>>>>>>>> manual, and particular register you're interested in is at 0x01700080,
>>>>>>>>>> its value is 0x1110110F.
>>>>>>>>>>
>>>>>>>>>> Bits 16-19 are not defined in user manual and are not set.
>>>>>>>>>
>>>>>>>>> Sorry, I somehow thought this was for the H6...
>>>>>>>>>
>>>>>>>>> I don't have any idea then :/
>>>>>>>>
>>>>>>>> OK, so what should we do? 'perf top'/'perf record' work fine if PMU
>>>>>>>> node is dropped, but they don't work if PMU node is present (even with
>>>>>>>> interrupts dropped). I'd prefer to have 'perf top' and 'perf record'
>>>>>>>> working instead of 'perf stat'
>>>>>>>
>>>>>>> Well, it doesn't work so we should just remove the node, and if
>>>>>>> someone wants it back, they should figure it out.
>>>>>>
>>>>>> Hey Maxime,
>>>>>>
>>>>>> So can you merge this patch?
>>>>>
>>>>> Added new Maxime's email to CC
>>>>
>>>> Queued as a fix for 5.4, thanks!
>>>> Maxime
>>>> _______________________________________________
>>>> linux-arm-kernel mailing list
>>>> linux-arm-kernel@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
  2019-11-01 15:47                                         ` Andre Przywara
@ 2019-11-11  1:43                                           ` André Przywara
  -1 siblings, 0 replies; 48+ messages in thread
From: André Przywara @ 2019-11-11  1:43 UTC (permalink / raw)
  To: Clément Péron, Vasily Khoruzhick
  Cc: Maxime Ripard, Mark Rutland, devicetree, Jared D . McNeill,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, Robin Murphy, arm-linux

[-- Attachment #1: Type: text/plain, Size: 9521 bytes --]

On 01/11/2019 15:47, Andre Przywara wrote:
> On 11/1/19 11:30 AM, Clément Péron wrote:

Hi,

for the sake of finishing this thread, in case search engines point to it:

>> On Thu, 31 Oct 2019 at 21:35, Vasily Khoruzhick <anarsoul@gmail.com>
>> wrote:
>>>
>>> On Thu, Oct 31, 2019 at 12:10 PM Clément Péron <peron.clem@gmail.com>
>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> Just a remark here but the interrupt are from 152 to 155 SPI.
>>>> But there is an offset of 32 no (remove SGI/PPI)?
>>>> This should be from 120 to 123
>>>
>>> I already tried it (and I believe someone already suggested it above),
>>> it doesn't fix PMU interrupts though.
>>
>> Ok thanks for the confirmation.
>>
>> Made a research about the PMU for A64 and found that Andre Przywara
>> made a patch to enable it:
>> https://gist.github.com/apritzel/d025abaa1425fcaf5991b5ffcf18a0a3
>>
>> Maybe he can confirm or not the issue on A64 ?
> 
> Well, I tried it back then, but couldn't make the interrupts work (and
> yes, I tried +/- 32). That's the reason I didn't send it back then.

I did the experiment drafted below, and found that the interrupts are
not 152-155, as the manual describes, but 148-151 instead.
And yes, those are the GIC interrupt IDs used, but the GIC binding
requires us to give the SPI numbers, so we need to subtract 32, which
was not the case for the original patch.
Thanks to Maxime for merging the fix patch:
https://archive.armlinux.org.uk/lurker/message/20191105.110651.914455de.en.html

In case someone wants to confirm this or in general needs to
find/confirm IRQ numbers for some peripherals:

> I can't say whether the IRQ lines are not wired or the manual just gives
> the wrong numbers. I don't have access to a board before Sunday, but if
> someone wants to beat me to it:
> - Hack U-Boot to add a command to program one PMU counter to expire
> quickly, and enable overflow interrupts.

Attaching a U-Boot patch to trigger a PMU cycle counter overflow IRQ.

> - Enable *all* SPIs on the GIC distributor level, and enable the
> distributor. Keep the GIC CPU interface disabled.

The GIC distributor on most Allwinner SoCs is located at 0x01c81000.
This is the first address shown in the GIC's DT node.
To enable all IRQs in U-Boot, on the U-Boot command prompt:
=> mw.l 0x01c81100 0xffffffff 0x20
This will set all possible 1024 bits in the GICD_ISENABLERn registers
(offset 0x100) to 1. Reading this back will reveal which IRQs the GIC
actually is configured for, typically we have much fewer SPIs supported.
To enable the distributor (really: group 1 interrupts):
=> mw.l 0x01c81000 1
As suggested, we don't touch the GIC CPU interface, so no interrupts
will actually reach the core.

> - Trigger the U-Boot command, and inspect the GICD_ISPENDR registers to
> see if any SPI fired.

Trigger the IRQ, for instance using the command provided by the patch
above. Keep the PMU enabled, because it will deassert the IRQ lines
otherwise:

=> pmuc start
=> pmuc
(check that the counter overflows)
=> md.l 0x01c81200 0x20

This will print the pending bits (GICD_ISPENDRn at offset 0x200) for
each IRQ. In U-Boot we expect no other IRQs to ever fire, so any bit set
in here would be due to our experiment.
In this case the output looked like:
01c81200: 00000000 00000000 00000000 00000000
01c81210: 00100000 00000000 00000000 00000000
If you do the counting, this is bit 20 in word 4, so 4 * 32 + 20 = 148.
Subtract 32 to get the number that goes into the DT node.
U-Boot is UP, so this is probably the IRQ on core 0. A reasonable
assumption is that the other cores just follow behind this, which you
can confirm in Linux, by running some perf command on only a single core
and watching the interrupt count increasing:
$ grep pmu /proc/interrupts
$ taskset -cp 1 $$
$ perf record sleep 3
$ grep pmu /proc/interrupts

> - Also double check the PMU overflow status register to verify that the
> event triggered.

Just calling "pmuc" will print the PMOVSCLR register.


Hope that helps!

Cheers,
Andre

>>>> On Wed, 25 Sep 2019 at 13:09, Maxime Ripard <mripard@kernel.org> wrote:
>>>>>
>>>>> On Mon, Sep 23, 2019 at 04:55:59PM -0700, Vasily Khoruzhick wrote:
>>>>>> On Mon, Sep 23, 2019 at 4:51 PM Vasily Khoruzhick
>>>>>> <anarsoul@gmail.com> wrote:
>>>>>>>
>>>>>>> On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
>>>>>>> <maxime.ripard@bootlin.com> wrote:
>>>>>>>>
>>>>>>>> On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
>>>>>>>>> On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard
>>>>>>>>> <maxime.ripard@bootlin.com> wrote:
>>>>>>>>>>
>>>>>>>>>> On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick
>>>>>>>>>> wrote:
>>>>>>>>>>> On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard
>>>>>>>>>>> <maxime.ripard@bootlin.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>> On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard
>>>>>>>>>>>>> <maxime.ripard@bootlin.com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily
>>>>>>>>>>>>>> Khoruzhick wrote:
>>>>>>>>>>>>>>> On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy
>>>>>>>>>>>>>>> <robin.murphy@arm.com> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
>>>>>>>>>>>>>>>>> On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer
>>>>>>>>>>>>>>>>> <harald@ccbib.org> wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Vasily Khoruzhick writes:
>>>>>>>>>>>>>>>>>>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy
>>>>>>>>>>>>>>>>>>> <robin.murphy@arm.com> wrote:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
>>>>>>>>>>>>>>>>>>>>> Looks like PMU in A64 is broken, it generates no
>>>>>>>>>>>>>>>>>>>>> interrupts at all and
>>>>>>>>>>>>>>>>>>>>> as result 'perf top' shows no events.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Does something like 'perf stat sleep 1' at least
>>>>>>>>>>>>>>>>>>>> count cycles correctly?
>>>>>>>>>>>>>>>>>>>> It could well just be that the interrupt numbers are
>>>>>>>>>>>>>>>>>>>> wrong...
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Looks like it does, at least result looks plausible:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I'm using perf stat regularly (cache benchmarks) and
>>>>>>>>>>>>>>>>>> it works fine.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Unfortunately I wasn't aware that perf stat is a poor
>>>>>>>>>>>>>>>>>> test for
>>>>>>>>>>>>>>>>>> the interrupts part of the node, when I added it. So
>>>>>>>>>>>>>>>>>> I'm not too
>>>>>>>>>>>>>>>>>> surprised I got it wrong.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> However, it would be unfortunate if the node got
>>>>>>>>>>>>>>>>>> removed completely,
>>>>>>>>>>>>>>>>>> because perf stat would not work anymore. Maybe we can
>>>>>>>>>>>>>>>>>> only remove
>>>>>>>>>>>>>>>>>> the interrupts or just fix them even if the HW doesn't
>>>>>>>>>>>>>>>>>> work?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I'm not familiar with PMU driver. Is it possible to get
>>>>>>>>>>>>>>>>> it working
>>>>>>>>>>>>>>>>> without interrupts?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Yup - you get a grumpy message from the driver, it will
>>>>>>>>>>>>>>>> refuse sampling
>>>>>>>>>>>>>>>> events (the ones which weren't working anyway), and if
>>>>>>>>>>>>>>>> you measure
>>>>>>>>>>>>>>>> anything for long enough that a counter overflows you'll
>>>>>>>>>>>>>>>> get wonky
>>>>>>>>>>>>>>>> results. But for counting hardware events over
>>>>>>>>>>>>>>>> relatively short periods
>>>>>>>>>>>>>>>> it'll still do the job.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I tried to drop interrupts completely from the node but
>>>>>>>>>>>>>>> 'perf top' is
>>>>>>>>>>>>>>> still broken. Though now in different way: it complains
>>>>>>>>>>>>>>> "cycles: PMU
>>>>>>>>>>>>>>> Hardware doesn't support sampling/overflow-interrupts.
>>>>>>>>>>>>>>> Try 'perf
>>>>>>>>>>>>>>> stat'"
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I have no idea if that's the culprit, but what is the
>>>>>>>>>>>>>> state of the
>>>>>>>>>>>>>> 0x09010000 register?
>>>>>>>>>>>>>
>>>>>>>>>>>>> What register is that and how do I check it?
>>>>>>>>>>>>
>>>>>>>>>>>> It's in the CPUX Configuration block, and the bits are
>>>>>>>>>>>> labelled as CPU
>>>>>>>>>>>> Debug Reset.
>>>>>>>>>>>>
>>>>>>>>>>>> And if you have busybox, you can use devmem.
>>>>>>>>>>>
>>>>>>>>>>> CPUX configuration block is at 0x01700000 according to A64 user
>>>>>>>>>>> manual, and particular register you're interested in is at
>>>>>>>>>>> 0x01700080,
>>>>>>>>>>> its value is 0x1110110F.
>>>>>>>>>>>
>>>>>>>>>>> Bits 16-19 are not defined in user manual and are not set.
>>>>>>>>>>
>>>>>>>>>> Sorry, I somehow thought this was for the H6...
>>>>>>>>>>
>>>>>>>>>> I don't have any idea then :/
>>>>>>>>>
>>>>>>>>> OK, so what should we do? 'perf top'/'perf record' work fine if
>>>>>>>>> PMU
>>>>>>>>> node is dropped, but they don't work if PMU node is present
>>>>>>>>> (even with
>>>>>>>>> interrupts dropped). I'd prefer to have 'perf top' and 'perf
>>>>>>>>> record'
>>>>>>>>> working instead of 'perf stat'
>>>>>>>>
>>>>>>>> Well, it doesn't work so we should just remove the node, and if
>>>>>>>> someone wants it back, they should figure it out.
>>>>>>>
>>>>>>> Hey Maxime,
>>>>>>>
>>>>>>> So can you merge this patch?
>>>>>>
>>>>>> Added new Maxime's email to CC
>>>>>
>>>>> Queued as a fix for 5.4, thanks!
>>>>> Maxime
>>>>> _______________________________________________
>>>>> linux-arm-kernel mailing list
>>>>> linux-arm-kernel@lists.infradead.org
>>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 


[-- Attachment #2: 0001-arm64-Add-performance-monitoring-unit-test-command.patch --]
[-- Type: text/x-patch, Size: 3459 bytes --]

From 169f01c8545a68b5f36515c49d171b9e0555ec1b Mon Sep 17 00:00:00 2001
From: Andre Przywara <andre.przywara@arm.com>
Date: Mon, 4 Nov 2019 21:54:51 +0000
Subject: [PATCH] arm64: Add performance monitoring unit test command

Mostly useful for identifying the interrupts used.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 cmd/Makefile  |  1 +
 cmd/arm_pmu.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)
 create mode 100644 cmd/arm_pmu.c

diff --git a/cmd/Makefile b/cmd/Makefile
index ac843b4b16..bced333222 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -182,6 +182,7 @@ endif # !CONFIG_SPL_BUILD
 
 # core command
 obj-y += nvedit.o
+obj-$(CONFIG_ARM64) += arm_pmu.o
 
 obj-$(CONFIG_TI_COMMON_CMD_OPTIONS) += ti/
 
diff --git a/cmd/arm_pmu.c b/cmd/arm_pmu.c
new file mode 100644
index 0000000000..0bc072b2f5
--- /dev/null
+++ b/cmd/arm_pmu.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2019 Arm Ltd.
+ */
+
+#include <common.h>
+#include <command.h>
+#include <linux/bitops.h>
+
+
+int do_pmuc(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	u64 reg;
+
+	if (argc < 2) {
+		/* PMU control register */
+		__asm__ volatile("mrs %0, PMCR_EL0\n" : "=r"(reg));
+		printf("PMCR: 0x%llx\n", reg);
+		/* overflow status register */
+		__asm__ volatile("mrs %0, PMOVSCLR_EL0\n" : "=r"(reg));
+		printf("PMOVSCLR: 0x%llx\n", reg);
+		/* counter register */
+		__asm__ volatile("mrs %0, PMCCNTR_EL0\n" : "=r"(reg));
+		printf("counter: 0x%llx\n", reg);
+		return CMD_RET_SUCCESS;
+	}
+
+	if (!strcmp(argv[1], "start")) {
+		/* enable cycle counting */
+		__asm__ volatile("msr PMCNTENSET_EL0, %0\n" :: "r"(BIT(31)));
+		/* enable overflow interrupt */
+		__asm__ volatile("msr PMINTENSET_EL1, %0\n" :: "r"(BIT(31)));
+		/* allow counting in EL2 */
+		__asm__ volatile("msr PMCCFILTR_EL0, %0\n" :: "r"(BIT(27)));
+		__asm__ volatile("mrs %0, PMCR_EL0\n" : "=r"(reg));
+		/* enable PMU */
+		reg |= 0x1;
+		__asm__ volatile("msr PMCR_EL0, %0\n" :: "r"(reg));
+		return CMD_RET_SUCCESS;
+	}
+	if (!strcmp(argv[1], "reset")) {
+		/* disable all event and cycle counting */
+		__asm__ volatile("msr PMCNTENCLR_EL0, %0\n" :: "r"(0xffffffff));
+		/* disable all overflow interrupts */
+		__asm__ volatile("msr PMINTENCLR_EL1, %0\n" :: "r"(0xffffffff));
+		/* clear all overflow interrupts */
+		__asm__ volatile("msr PMOVSCLR_EL0, %0\n" :: "r"(0xffffffff));
+		/* clear filter (blocks EL2 counting) */
+		__asm__ volatile("msr PMCCFILTR_EL0, %0\n" :: "r"(0));
+		__asm__ volatile("mrs %0, PMCR_EL0\n" : "=r"(reg));
+		/* disable PMU */
+		reg &= ~0x1;
+		__asm__ volatile("msr PMCR_EL0, %0\n" :: "r"(reg));
+		return CMD_RET_SUCCESS;
+	}
+	if (!strcmp(argv[1], "stop")) {
+		__asm__ volatile("mrs %0, PMCR_EL0\n" : "=r"(reg));
+		/* disable PMU */
+		reg &= ~0x1;
+		__asm__ volatile("msr PMCR_EL0, %0\n" :: "r"(reg));
+		return CMD_RET_SUCCESS;
+	}
+
+	reg = simple_strtoul(argv[1], NULL, 16);
+	__asm__ volatile("msr PMCCNTR_EL0, %0\n" :: "r" (reg));
+
+	return CMD_RET_SUCCESS;
+}
+
+U_BOOT_CMD(
+	pmuc,	2,	1,	do_pmuc,
+	"test the ARM performance monitoring unit",
+	"(no argument): print the cycle counter, PMCR and overflow status\n"
+	"        start: enable the cycle counter and interrupts\n"
+	"         stop: disable the cycle counter and interrupts\n"
+	"        reset: reset the PMU\n"
+	"      <value>: set the cycle counter\n"
+);
-- 
2.14.5


^ permalink raw reply related	[flat|nested] 48+ messages in thread

* Re: [PATCH] arm64: dts: allwinner: a64: Drop PMU node
@ 2019-11-11  1:43                                           ` André Przywara
  0 siblings, 0 replies; 48+ messages in thread
From: André Przywara @ 2019-11-11  1:43 UTC (permalink / raw)
  To: Clément Péron, Vasily Khoruzhick
  Cc: Mark Rutland, devicetree, Jared D . McNeill, Maxime Ripard,
	Chen-Yu Tsai, Rob Herring, Harald Geyer, Robin Murphy, arm-linux

[-- Attachment #1: Type: text/plain, Size: 9521 bytes --]

On 01/11/2019 15:47, Andre Przywara wrote:
> On 11/1/19 11:30 AM, Clément Péron wrote:

Hi,

for the sake of finishing this thread, in case search engines point to it:

>> On Thu, 31 Oct 2019 at 21:35, Vasily Khoruzhick <anarsoul@gmail.com>
>> wrote:
>>>
>>> On Thu, Oct 31, 2019 at 12:10 PM Clément Péron <peron.clem@gmail.com>
>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> Just a remark here but the interrupt are from 152 to 155 SPI.
>>>> But there is an offset of 32 no (remove SGI/PPI)?
>>>> This should be from 120 to 123
>>>
>>> I already tried it (and I believe someone already suggested it above),
>>> it doesn't fix PMU interrupts though.
>>
>> Ok thanks for the confirmation.
>>
>> Made a research about the PMU for A64 and found that Andre Przywara
>> made a patch to enable it:
>> https://gist.github.com/apritzel/d025abaa1425fcaf5991b5ffcf18a0a3
>>
>> Maybe he can confirm or not the issue on A64 ?
> 
> Well, I tried it back then, but couldn't make the interrupts work (and
> yes, I tried +/- 32). That's the reason I didn't send it back then.

I did the experiment drafted below, and found that the interrupts are
not 152-155, as the manual describes, but 148-151 instead.
And yes, those are the GIC interrupt IDs used, but the GIC binding
requires us to give the SPI numbers, so we need to subtract 32, which
was not the case for the original patch.
Thanks to Maxime for merging the fix patch:
https://archive.armlinux.org.uk/lurker/message/20191105.110651.914455de.en.html

In case someone wants to confirm this or in general needs to
find/confirm IRQ numbers for some peripherals:

> I can't say whether the IRQ lines are not wired or the manual just gives
> the wrong numbers. I don't have access to a board before Sunday, but if
> someone wants to beat me to it:
> - Hack U-Boot to add a command to program one PMU counter to expire
> quickly, and enable overflow interrupts.

Attaching a U-Boot patch to trigger a PMU cycle counter overflow IRQ.

> - Enable *all* SPIs on the GIC distributor level, and enable the
> distributor. Keep the GIC CPU interface disabled.

The GIC distributor on most Allwinner SoCs is located at 0x01c81000.
This is the first address shown in the GIC's DT node.
To enable all IRQs in U-Boot, on the U-Boot command prompt:
=> mw.l 0x01c81100 0xffffffff 0x20
This will set all possible 1024 bits in the GICD_ISENABLERn registers
(offset 0x100) to 1. Reading this back will reveal which IRQs the GIC
actually is configured for, typically we have much fewer SPIs supported.
To enable the distributor (really: group 1 interrupts):
=> mw.l 0x01c81000 1
As suggested, we don't touch the GIC CPU interface, so no interrupts
will actually reach the core.

> - Trigger the U-Boot command, and inspect the GICD_ISPENDR registers to
> see if any SPI fired.

Trigger the IRQ, for instance using the command provided by the patch
above. Keep the PMU enabled, because it will deassert the IRQ lines
otherwise:

=> pmuc start
=> pmuc
(check that the counter overflows)
=> md.l 0x01c81200 0x20

This will print the pending bits (GICD_ISPENDRn at offset 0x200) for
each IRQ. In U-Boot we expect no other IRQs to ever fire, so any bit set
in here would be due to our experiment.
In this case the output looked like:
01c81200: 00000000 00000000 00000000 00000000
01c81210: 00100000 00000000 00000000 00000000
If you do the counting, this is bit 20 in word 4, so 4 * 32 + 20 = 148.
Subtract 32 to get the number that goes into the DT node.
U-Boot is UP, so this is probably the IRQ on core 0. A reasonable
assumption is that the other cores just follow behind this, which you
can confirm in Linux, by running some perf command on only a single core
and watching the interrupt count increasing:
$ grep pmu /proc/interrupts
$ taskset -cp 1 $$
$ perf record sleep 3
$ grep pmu /proc/interrupts

> - Also double check the PMU overflow status register to verify that the
> event triggered.

Just calling "pmuc" will print the PMOVSCLR register.


Hope that helps!

Cheers,
Andre

>>>> On Wed, 25 Sep 2019 at 13:09, Maxime Ripard <mripard@kernel.org> wrote:
>>>>>
>>>>> On Mon, Sep 23, 2019 at 04:55:59PM -0700, Vasily Khoruzhick wrote:
>>>>>> On Mon, Sep 23, 2019 at 4:51 PM Vasily Khoruzhick
>>>>>> <anarsoul@gmail.com> wrote:
>>>>>>>
>>>>>>> On Mon, Aug 12, 2019 at 10:39 PM Maxime Ripard
>>>>>>> <maxime.ripard@bootlin.com> wrote:
>>>>>>>>
>>>>>>>> On Mon, Aug 12, 2019 at 11:01:51AM -0700, Vasily Khoruzhick wrote:
>>>>>>>>> On Mon, Aug 12, 2019 at 1:04 AM Maxime Ripard
>>>>>>>>> <maxime.ripard@bootlin.com> wrote:
>>>>>>>>>>
>>>>>>>>>> On Thu, Aug 08, 2019 at 12:59:07PM -0700, Vasily Khoruzhick
>>>>>>>>>> wrote:
>>>>>>>>>>> On Thu, Aug 8, 2019 at 9:26 AM Maxime Ripard
>>>>>>>>>>> <maxime.ripard@bootlin.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> On Wed, Aug 07, 2019 at 10:36:08AM -0700, Vasily Khoruzhick
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>> On Wed, Aug 7, 2019 at 4:56 AM Maxime Ripard
>>>>>>>>>>>>> <maxime.ripard@bootlin.com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Tue, Aug 06, 2019 at 07:39:26PM -0700, Vasily
>>>>>>>>>>>>>> Khoruzhick wrote:
>>>>>>>>>>>>>>> On Tue, Aug 6, 2019 at 2:14 PM Robin Murphy
>>>>>>>>>>>>>>> <robin.murphy@arm.com> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 2019-08-06 9:52 pm, Vasily Khoruzhick wrote:
>>>>>>>>>>>>>>>>> On Tue, Aug 6, 2019 at 1:19 PM Harald Geyer
>>>>>>>>>>>>>>>>> <harald@ccbib.org> wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Vasily Khoruzhick writes:
>>>>>>>>>>>>>>>>>>> On Tue, Aug 6, 2019 at 7:35 AM Robin Murphy
>>>>>>>>>>>>>>>>>>> <robin.murphy@arm.com> wrote:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> On 06/08/2019 15:01, Vasily Khoruzhick wrote:
>>>>>>>>>>>>>>>>>>>>> Looks like PMU in A64 is broken, it generates no
>>>>>>>>>>>>>>>>>>>>> interrupts at all and
>>>>>>>>>>>>>>>>>>>>> as result 'perf top' shows no events.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Does something like 'perf stat sleep 1' at least
>>>>>>>>>>>>>>>>>>>> count cycles correctly?
>>>>>>>>>>>>>>>>>>>> It could well just be that the interrupt numbers are
>>>>>>>>>>>>>>>>>>>> wrong...
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Looks like it does, at least result looks plausible:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I'm using perf stat regularly (cache benchmarks) and
>>>>>>>>>>>>>>>>>> it works fine.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Unfortunately I wasn't aware that perf stat is a poor
>>>>>>>>>>>>>>>>>> test for
>>>>>>>>>>>>>>>>>> the interrupts part of the node, when I added it. So
>>>>>>>>>>>>>>>>>> I'm not too
>>>>>>>>>>>>>>>>>> surprised I got it wrong.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> However, it would be unfortunate if the node got
>>>>>>>>>>>>>>>>>> removed completely,
>>>>>>>>>>>>>>>>>> because perf stat would not work anymore. Maybe we can
>>>>>>>>>>>>>>>>>> only remove
>>>>>>>>>>>>>>>>>> the interrupts or just fix them even if the HW doesn't
>>>>>>>>>>>>>>>>>> work?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I'm not familiar with PMU driver. Is it possible to get
>>>>>>>>>>>>>>>>> it working
>>>>>>>>>>>>>>>>> without interrupts?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Yup - you get a grumpy message from the driver, it will
>>>>>>>>>>>>>>>> refuse sampling
>>>>>>>>>>>>>>>> events (the ones which weren't working anyway), and if
>>>>>>>>>>>>>>>> you measure
>>>>>>>>>>>>>>>> anything for long enough that a counter overflows you'll
>>>>>>>>>>>>>>>> get wonky
>>>>>>>>>>>>>>>> results. But for counting hardware events over
>>>>>>>>>>>>>>>> relatively short periods
>>>>>>>>>>>>>>>> it'll still do the job.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I tried to drop interrupts completely from the node but
>>>>>>>>>>>>>>> 'perf top' is
>>>>>>>>>>>>>>> still broken. Though now in different way: it complains
>>>>>>>>>>>>>>> "cycles: PMU
>>>>>>>>>>>>>>> Hardware doesn't support sampling/overflow-interrupts.
>>>>>>>>>>>>>>> Try 'perf
>>>>>>>>>>>>>>> stat'"
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I have no idea if that's the culprit, but what is the
>>>>>>>>>>>>>> state of the
>>>>>>>>>>>>>> 0x09010000 register?
>>>>>>>>>>>>>
>>>>>>>>>>>>> What register is that and how do I check it?
>>>>>>>>>>>>
>>>>>>>>>>>> It's in the CPUX Configuration block, and the bits are
>>>>>>>>>>>> labelled as CPU
>>>>>>>>>>>> Debug Reset.
>>>>>>>>>>>>
>>>>>>>>>>>> And if you have busybox, you can use devmem.
>>>>>>>>>>>
>>>>>>>>>>> CPUX configuration block is at 0x01700000 according to A64 user
>>>>>>>>>>> manual, and particular register you're interested in is at
>>>>>>>>>>> 0x01700080,
>>>>>>>>>>> its value is 0x1110110F.
>>>>>>>>>>>
>>>>>>>>>>> Bits 16-19 are not defined in user manual and are not set.
>>>>>>>>>>
>>>>>>>>>> Sorry, I somehow thought this was for the H6...
>>>>>>>>>>
>>>>>>>>>> I don't have any idea then :/
>>>>>>>>>
>>>>>>>>> OK, so what should we do? 'perf top'/'perf record' work fine if
>>>>>>>>> PMU
>>>>>>>>> node is dropped, but they don't work if PMU node is present
>>>>>>>>> (even with
>>>>>>>>> interrupts dropped). I'd prefer to have 'perf top' and 'perf
>>>>>>>>> record'
>>>>>>>>> working instead of 'perf stat'
>>>>>>>>
>>>>>>>> Well, it doesn't work so we should just remove the node, and if
>>>>>>>> someone wants it back, they should figure it out.
>>>>>>>
>>>>>>> Hey Maxime,
>>>>>>>
>>>>>>> So can you merge this patch?
>>>>>>
>>>>>> Added new Maxime's email to CC
>>>>>
>>>>> Queued as a fix for 5.4, thanks!
>>>>> Maxime
>>>>> _______________________________________________
>>>>> linux-arm-kernel mailing list
>>>>> linux-arm-kernel@lists.infradead.org
>>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 


[-- Attachment #2: 0001-arm64-Add-performance-monitoring-unit-test-command.patch --]
[-- Type: text/x-patch, Size: 3459 bytes --]

From 169f01c8545a68b5f36515c49d171b9e0555ec1b Mon Sep 17 00:00:00 2001
From: Andre Przywara <andre.przywara@arm.com>
Date: Mon, 4 Nov 2019 21:54:51 +0000
Subject: [PATCH] arm64: Add performance monitoring unit test command

Mostly useful for identifying the interrupts used.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 cmd/Makefile  |  1 +
 cmd/arm_pmu.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)
 create mode 100644 cmd/arm_pmu.c

diff --git a/cmd/Makefile b/cmd/Makefile
index ac843b4b16..bced333222 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -182,6 +182,7 @@ endif # !CONFIG_SPL_BUILD
 
 # core command
 obj-y += nvedit.o
+obj-$(CONFIG_ARM64) += arm_pmu.o
 
 obj-$(CONFIG_TI_COMMON_CMD_OPTIONS) += ti/
 
diff --git a/cmd/arm_pmu.c b/cmd/arm_pmu.c
new file mode 100644
index 0000000000..0bc072b2f5
--- /dev/null
+++ b/cmd/arm_pmu.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2019 Arm Ltd.
+ */
+
+#include <common.h>
+#include <command.h>
+#include <linux/bitops.h>
+
+
+int do_pmuc(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	u64 reg;
+
+	if (argc < 2) {
+		/* PMU control register */
+		__asm__ volatile("mrs %0, PMCR_EL0\n" : "=r"(reg));
+		printf("PMCR: 0x%llx\n", reg);
+		/* overflow status register */
+		__asm__ volatile("mrs %0, PMOVSCLR_EL0\n" : "=r"(reg));
+		printf("PMOVSCLR: 0x%llx\n", reg);
+		/* counter register */
+		__asm__ volatile("mrs %0, PMCCNTR_EL0\n" : "=r"(reg));
+		printf("counter: 0x%llx\n", reg);
+		return CMD_RET_SUCCESS;
+	}
+
+	if (!strcmp(argv[1], "start")) {
+		/* enable cycle counting */
+		__asm__ volatile("msr PMCNTENSET_EL0, %0\n" :: "r"(BIT(31)));
+		/* enable overflow interrupt */
+		__asm__ volatile("msr PMINTENSET_EL1, %0\n" :: "r"(BIT(31)));
+		/* allow counting in EL2 */
+		__asm__ volatile("msr PMCCFILTR_EL0, %0\n" :: "r"(BIT(27)));
+		__asm__ volatile("mrs %0, PMCR_EL0\n" : "=r"(reg));
+		/* enable PMU */
+		reg |= 0x1;
+		__asm__ volatile("msr PMCR_EL0, %0\n" :: "r"(reg));
+		return CMD_RET_SUCCESS;
+	}
+	if (!strcmp(argv[1], "reset")) {
+		/* disable all event and cycle counting */
+		__asm__ volatile("msr PMCNTENCLR_EL0, %0\n" :: "r"(0xffffffff));
+		/* disable all overflow interrupts */
+		__asm__ volatile("msr PMINTENCLR_EL1, %0\n" :: "r"(0xffffffff));
+		/* clear all overflow interrupts */
+		__asm__ volatile("msr PMOVSCLR_EL0, %0\n" :: "r"(0xffffffff));
+		/* clear filter (blocks EL2 counting) */
+		__asm__ volatile("msr PMCCFILTR_EL0, %0\n" :: "r"(0));
+		__asm__ volatile("mrs %0, PMCR_EL0\n" : "=r"(reg));
+		/* disable PMU */
+		reg &= ~0x1;
+		__asm__ volatile("msr PMCR_EL0, %0\n" :: "r"(reg));
+		return CMD_RET_SUCCESS;
+	}
+	if (!strcmp(argv[1], "stop")) {
+		__asm__ volatile("mrs %0, PMCR_EL0\n" : "=r"(reg));
+		/* disable PMU */
+		reg &= ~0x1;
+		__asm__ volatile("msr PMCR_EL0, %0\n" :: "r"(reg));
+		return CMD_RET_SUCCESS;
+	}
+
+	reg = simple_strtoul(argv[1], NULL, 16);
+	__asm__ volatile("msr PMCCNTR_EL0, %0\n" :: "r" (reg));
+
+	return CMD_RET_SUCCESS;
+}
+
+U_BOOT_CMD(
+	pmuc,	2,	1,	do_pmuc,
+	"test the ARM performance monitoring unit",
+	"(no argument): print the cycle counter, PMCR and overflow status\n"
+	"        start: enable the cycle counter and interrupts\n"
+	"         stop: disable the cycle counter and interrupts\n"
+	"        reset: reset the PMU\n"
+	"      <value>: set the cycle counter\n"
+);
-- 
2.14.5


[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 48+ messages in thread

end of thread, other threads:[~2019-11-11  1:40 UTC | newest]

Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-06 14:01 [PATCH] arm64: dts: allwinner: a64: Drop PMU node Vasily Khoruzhick
2019-08-06 14:01 ` Vasily Khoruzhick
2019-08-06 14:35 ` Robin Murphy
2019-08-06 14:35   ` Robin Murphy
2019-08-06 14:45   ` Vasily Khoruzhick
2019-08-06 14:45     ` Vasily Khoruzhick
2019-08-06 20:19     ` Harald Geyer
2019-08-06 20:19       ` Harald Geyer
2019-08-06 20:52       ` Vasily Khoruzhick
2019-08-06 20:52         ` Vasily Khoruzhick
2019-08-06 21:14         ` Robin Murphy
2019-08-06 21:14           ` Robin Murphy
2019-08-07  2:39           ` Vasily Khoruzhick
2019-08-07  2:39             ` Vasily Khoruzhick
2019-08-07 11:56             ` Maxime Ripard
2019-08-07 11:56               ` Maxime Ripard
2019-08-07 17:36               ` Vasily Khoruzhick
2019-08-07 17:36                 ` Vasily Khoruzhick
2019-08-08 16:26                 ` Maxime Ripard
2019-08-08 19:59                   ` Vasily Khoruzhick
2019-08-08 19:59                     ` Vasily Khoruzhick
2019-08-12  8:04                     ` Maxime Ripard
2019-08-12 18:01                       ` Vasily Khoruzhick
2019-08-12 18:01                         ` Vasily Khoruzhick
2019-08-12 18:22                         ` Harald Geyer
2019-08-13  5:39                         ` Maxime Ripard
2019-08-13  5:39                           ` Maxime Ripard
2019-09-23 23:51                           ` Vasily Khoruzhick
2019-09-23 23:51                             ` Vasily Khoruzhick
2019-09-23 23:55                             ` Vasily Khoruzhick
2019-09-23 23:55                               ` Vasily Khoruzhick
2019-09-25 11:08                               ` Maxime Ripard
2019-10-31 19:10                                 ` Clément Péron
2019-10-31 19:10                                   ` Clément Péron
2019-10-31 20:35                                   ` Vasily Khoruzhick
2019-10-31 20:35                                     ` Vasily Khoruzhick
2019-11-01 11:30                                     ` Clément Péron
2019-11-01 11:30                                       ` Clément Péron
2019-11-01 15:47                                       ` Andre Przywara
2019-11-01 15:47                                         ` Andre Przywara
2019-11-11  1:43                                         ` André Przywara
2019-11-11  1:43                                           ` André Przywara
2019-08-07 11:59             ` Robin Murphy
2019-08-07 11:59               ` Robin Murphy
2019-08-07 11:12           ` Mark Rutland
2019-08-07 11:12             ` Mark Rutland
2019-08-06 19:10 ` Emmanuel Vadot
2019-08-06 19:10   ` Emmanuel Vadot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.