The a53 and a7 counters seem to match up, so we advertise a7 so that
arm32 can probe.
Signed-off-by: Eric Anholt <[email protected]>
---
arch/arm/boot/dts/bcm2837.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/bcm2837.dtsi b/arch/arm/boot/dts/bcm2837.dtsi
index 7704bb029605..1f5e5c782835 100644
--- a/arch/arm/boot/dts/bcm2837.dtsi
+++ b/arch/arm/boot/dts/bcm2837.dtsi
@@ -17,6 +17,12 @@
};
};
+ arm-pmu {
+ compatible = "arm,cortex-a53-pmu", "arm,cortex-a7-pmu";
+ interrupt-parent = <&local_intc>;
+ interrupts = <9 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
timer {
compatible = "arm,armv7-timer";
interrupt-parent = <&local_intc>;
--
2.17.0
On Thu, 17 May 2018, Eric Anholt wrote:
> diff --git a/arch/arm/boot/dts/bcm2837.dtsi b/arch/arm/boot/dts/bcm2837.dtsi
> index 7704bb029605..1f5e5c782835 100644
> --- a/arch/arm/boot/dts/bcm2837.dtsi
> +++ b/arch/arm/boot/dts/bcm2837.dtsi
> @@ -17,6 +17,12 @@
> };
> };
>
> + arm-pmu {
> + compatible = "arm,cortex-a53-pmu", "arm,cortex-a7-pmu";
> + interrupt-parent = <&local_intc>;
> + interrupts = <9 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
why this and not
arm-pmu {
compatible = "arm,armv8-pmuv3";
interrupt-parent = <&local_intc>;
interrupts = <9>;
};
which works, though when I didn't get very far when I submitted the patch
to add this last August.
Vince
Vince Weaver <[email protected]> writes:
> On Thu, 17 May 2018, Eric Anholt wrote:
>
>> diff --git a/arch/arm/boot/dts/bcm2837.dtsi b/arch/arm/boot/dts/bcm2837.dtsi
>> index 7704bb029605..1f5e5c782835 100644
>> --- a/arch/arm/boot/dts/bcm2837.dtsi
>> +++ b/arch/arm/boot/dts/bcm2837.dtsi
>> @@ -17,6 +17,12 @@
>> };
>> };
>>
>> + arm-pmu {
>> + compatible = "arm,cortex-a53-pmu", "arm,cortex-a7-pmu";
>> + interrupt-parent = <&local_intc>;
>> + interrupts = <9 IRQ_TYPE_LEVEL_HIGH>;
>> + };
>> +
>
> why this and not
>
> arm-pmu {
> compatible = "arm,armv8-pmuv3";
> interrupt-parent = <&local_intc>;
> interrupts = <9>;
> };
>
> which works, though when I didn't get very far when I submitted the patch
> to add this last August.
Is that better than a53? I'm happy to switch to that. The important
part to me is the a7, since basically everyone with this hw is running
arm32.
> Eric Anholt <[email protected]> hat am 17. Mai 2018 um 15:17 geschrieben:
>
>
> The a53 and a7 counters seem to match up, so we advertise a7 so that
> arm32 can probe.
>
> Signed-off-by: Eric Anholt <[email protected]>
Acked-by: Stefan Wahren <[email protected]>
On Thu, 17 May 2018, Eric Anholt wrote:
>
> Is that better than a53? I'm happy to switch to that. The important
> part to me is the a7, since basically everyone with this hw is running
> arm32.
no, on further investigation it looks like a53 is more proper to use than
the generic armv8.
Is the armv8 pmu on the cortex-a53 backwards compatible with armv7? I'm
dreading having to pull up the various ARM ARMs to look for myself so if
it works for you I'm fine with that part too.
The biggest pushback I had with my original patch was no one believing irq
9 was the right one to use.
Vince
> Vince Weaver <[email protected]> hat am 17. Mai 2018 um 16:11 geschrieben:
>
>
> On Thu, 17 May 2018, Eric Anholt wrote:
>
> > diff --git a/arch/arm/boot/dts/bcm2837.dtsi b/arch/arm/boot/dts/bcm2837.dtsi
> > index 7704bb029605..1f5e5c782835 100644
> > --- a/arch/arm/boot/dts/bcm2837.dtsi
> > +++ b/arch/arm/boot/dts/bcm2837.dtsi
> > @@ -17,6 +17,12 @@
> > };
> > };
> >
> > + arm-pmu {
> > + compatible = "arm,cortex-a53-pmu", "arm,cortex-a7-pmu";
> > + interrupt-parent = <&local_intc>;
> > + interrupts = <9 IRQ_TYPE_LEVEL_HIGH>;
> > + };
> > +
>
> why this and not
>
> arm-pmu {
> compatible = "arm,armv8-pmuv3";
> interrupt-parent = <&local_intc>;
> interrupts = <9>;
> };
>
> which works, though when I didn't get very far when I submitted the patch
> to add this last August.
Eric's variant is more specific so we better chose this. The init code diverse between the general arm,armv8-pmuv3 and arm,cortex-a53-pmu.
>
> Vince
>
> _______________________________________________
> linux-rpi-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel
On Thu, 17 May 2018, Stefan Wahren wrote:
>
> > Eric Anholt <[email protected]> hat am 17. Mai 2018 um 15:17 geschrieben:
> >
> >
> > The a53 and a7 counters seem to match up, so we advertise a7 so that
> > arm32 can probe.
so how closely did you look at the a53/a7 differences? I see some major
differences, especially with the CPU_CYCLES event (0xff vs 0x11).
The proper fix here might be to add a cortex-a53 PMU entry to the armv7
code rather than trying to treat it as a cortex-a7.
Vince
On Thu, May 17, 2018 at 2:17 PM, Eric Anholt <[email protected]> wrote:
> The a53 and a7 counters seem to match up, so we advertise a7 so that
> arm32 can probe.
>
> Signed-off-by: Eric Anholt <[email protected]>
Tested-by: Peter Robinson <[email protected]>
We've carried the same/equiv patch in Fedora for a while with no issues.
Peter
> ---
> arch/arm/boot/dts/bcm2837.dtsi | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm2837.dtsi b/arch/arm/boot/dts/bcm2837.dtsi
> index 7704bb029605..1f5e5c782835 100644
> --- a/arch/arm/boot/dts/bcm2837.dtsi
> +++ b/arch/arm/boot/dts/bcm2837.dtsi
> @@ -17,6 +17,12 @@
> };
> };
>
> + arm-pmu {
> + compatible = "arm,cortex-a53-pmu", "arm,cortex-a7-pmu";
> + interrupt-parent = <&local_intc>;
> + interrupts = <9 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> timer {
> compatible = "arm,armv7-timer";
> interrupt-parent = <&local_intc>;
> --
> 2.17.0
>
>
> _______________________________________________
> linux-rpi-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel
Hi,
[added Peter, Ingo and Arnaldo]
> Vince Weaver <[email protected]> hat am 17. Mai 2018 um 18:34 geschrieben:
>
>
> On Thu, 17 May 2018, Stefan Wahren wrote:
>
> >
> > > Eric Anholt <[email protected]> hat am 17. Mai 2018 um 15:17 geschrieben:
> > >
> > >
> > > The a53 and a7 counters seem to match up, so we advertise a7 so that
> > > arm32 can probe.
>
> so how closely did you look at the a53/a7 differences? I see some major
> differences, especially with the CPU_CYCLES event (0xff vs 0x11).
>
> The proper fix here might be to add a cortex-a53 PMU entry to the armv7
> code rather than trying to treat it as a cortex-a7.
we like to use the PMU of BCM2837 SoC (4x A53 cores) under arm32 and arm64.
What is the right way (tm) to the define the DT compatibles?
Does the arm32 PMU driver need patching for proper A53 support?
Stefan
>
> Vince
Vince Weaver <[email protected]> writes:
> On Thu, 17 May 2018, Stefan Wahren wrote:
>
>>
>> > Eric Anholt <[email protected]> hat am 17. Mai 2018 um 15:17 geschrieben:
>> >
>> >
>> > The a53 and a7 counters seem to match up, so we advertise a7 so that
>> > arm32 can probe.
>
> so how closely did you look at the a53/a7 differences? I see some major
> differences, especially with the CPU_CYCLES event (0xff vs 0x11).
I'm a bit lost in the code, but it seemed like the 0xff was a
placeholder for a bit of special behavior, but that the cpu_cycles ->
ARMV7_PERFCTR_CLOCK_CYCLES mapping got you that same value in the end.
On Thu, May 17, 2018 at 06:55:26PM +0200, Stefan Wahren wrote:
> > Vince Weaver <[email protected]> hat am 17. Mai 2018 um 18:34 geschrieben:
> > On Thu, 17 May 2018, Stefan Wahren wrote:
> > > > Eric Anholt <[email protected]> hat am 17. Mai 2018 um 15:17 geschrieben:
> > > > The a53 and a7 counters seem to match up, so we advertise a7 so that
> > > > arm32 can probe.
> >
> > so how closely did you look at the a53/a7 differences? I see some major
> > differences, especially with the CPU_CYCLES event (0xff vs 0x11).
> >
> > The proper fix here might be to add a cortex-a53 PMU entry to the armv7
> > code rather than trying to treat it as a cortex-a7.
>
> we like to use the PMU of BCM2837 SoC (4x A53 cores) under arm32 and arm64.
>
> What is the right way (tm) to the define the DT compatibles?
> Does the arm32 PMU driver need patching for proper A53 support?
I'm completely clueless on all of this; Mark might have ideas.
On Thu, 17 May 2018, Peter Zijlstra wrote:
> On Thu, May 17, 2018 at 06:55:26PM +0200, Stefan Wahren wrote:
> > > Vince Weaver <[email protected]> hat am 17. Mai 2018 um 18:34 geschrieben:
> > > On Thu, 17 May 2018, Stefan Wahren wrote:
> > > > > Eric Anholt <[email protected]> hat am 17. Mai 2018 um 15:17 geschrieben:
>
> > > > > The a53 and a7 counters seem to match up, so we advertise a7 so that
> > > > > arm32 can probe.
> > >
> > > so how closely did you look at the a53/a7 differences? I see some major
> > > differences, especially with the CPU_CYCLES event (0xff vs 0x11).
> > >
> > > The proper fix here might be to add a cortex-a53 PMU entry to the armv7
> > > code rather than trying to treat it as a cortex-a7.
> >
> > we like to use the PMU of BCM2837 SoC (4x A53 cores) under arm32 and arm64.
> >
> > What is the right way (tm) to the define the DT compatibles?
> > Does the arm32 PMU driver need patching for proper A53 support?
>
> I'm completely clueless on all of this; Mark might have ideas.
Spending more time looking at it the only obvious differences are the
previously mentioned CYCLES difference, as well as the cortex-a7 has
18 events in the perf_cache_map but cortex-a53 only has 3. Plus probably
support for the various other features of the armv8v3 pmu that the a7
knows nothing about.
Is it hard to get lines in the DT changed once they are there? If we go
with cortex-a7 now, would it be possible to later drop that if proper
cortex-a53 support is added to the armv7 pmu driver? Or would that lead
to all kinds of back-compatability mess?
Vince
On Thu, 17 May 2018, Vince Weaver wrote:
> On Thu, 17 May 2018, Peter Zijlstra wrote:
> with cortex-a7 now, would it be possible to later drop that if proper
> cortex-a53 support is added to the armv7 pmu driver? Or would that lead
> to all kinds of back-compatability mess?
For what it's worth, the pi-foundation kernel bcm2710 device tree file
does:
arm-pmu {
#ifdef RPI364
compatible = "arm,armv8-pmuv3", "arm,cortex-a7-pmu";
#else
compatible = "arm,cortex-a7-pmu";
#endif
interrupt-parent = <&local_intc>;
interrupts = <9>;
};
Which is probably where I was getting the arm,armv8-pmuv3 from in my
original patch.
Vince
On 05/17/2018 12:31 PM, Vince Weaver wrote:
> On Thu, 17 May 2018, Vince Weaver wrote:
>
>> On Thu, 17 May 2018, Peter Zijlstra wrote:
>> with cortex-a7 now, would it be possible to later drop that if proper
>> cortex-a53 support is added to the armv7 pmu driver? Or would that lead
>> to all kinds of back-compatability mess?
>
> For what it's worth, the pi-foundation kernel bcm2710 device tree file
> does:
>
> arm-pmu {
> #ifdef RPI364
> compatible = "arm,armv8-pmuv3", "arm,cortex-a7-pmu";
> #else
> compatible = "arm,cortex-a7-pmu";
> #endif
> interrupt-parent = <&local_intc>;
> interrupts = <9>;
> };
>
>
> Which is probably where I was getting the arm,armv8-pmuv3 from in my
> original patch.
I thought somehow that Marc Z. had unified
arch/arm/kernel/perf_event_v7.c and arch/arm64/kernel/perf_event.c into
a common driver entry point under drivers/perf/arm_pmu.c but I don't see
it and after about 15 minutes looking at it, it does not look as trivial
as I though to separate out those files so the ARMv8 PMU description can
be moved into a generic location for instance.
FWIW, Broadcom STB chips, even when 64-bit capable or often used with an
32-bit ARM kernel, so having the ARMv8 PMUs work under a 32-bit ARM
kernel would be great. The downstream solution we have sued thus far is
to find the closest compatible string to represent those, which is not
great...
--
Florian
On 17/05/18 20:59, Florian Fainelli wrote:
> On 05/17/2018 12:31 PM, Vince Weaver wrote:
>> On Thu, 17 May 2018, Vince Weaver wrote:
>>
>>> On Thu, 17 May 2018, Peter Zijlstra wrote:
>>> with cortex-a7 now, would it be possible to later drop that if proper
>>> cortex-a53 support is added to the armv7 pmu driver? Or would that lead
>>> to all kinds of back-compatability mess?
>>
>> For what it's worth, the pi-foundation kernel bcm2710 device tree file
>> does:
>>
>> arm-pmu {
>> #ifdef RPI364
>> compatible = "arm,armv8-pmuv3", "arm,cortex-a7-pmu";
Hahaha. Funny that. Not. That's really silly. The DT *must* describe the
HW, and having contradictory information is not helping. This is going
to lead to all kind of miscounted events (to take the above example) A7
and A53 are significantly different, and thus will count events
differently....
>> #else
>> compatible = "arm,cortex-a7-pmu";
>> #endif
>> interrupt-parent = <&local_intc>;
>> interrupts = <9>;
>> };
>>
>>
>> Which is probably where I was getting the arm,armv8-pmuv3 from in my
>> original patch.
>
> I thought somehow that Marc Z. had unified
> arch/arm/kernel/perf_event_v7.c and arch/arm64/kernel/perf_event.c into
> a common driver entry point under drivers/perf/arm_pmu.c but I don't see
> it and after about 15 minutes looking at it, it does not look as trivial
> as I though to separate out those files so the ARMv8 PMU description can
> be moved into a generic location for instance.
I have a pretty simple series[1] which I used to profile 32bit guests on
an arm64 KVM host. Nobody really cared about it because running a 32bit
kernel on 64bit HW is a bit odd, to say the least, and I'm probably the
only one actually running 32bit VMs.
> FWIW, Broadcom STB chips, even when 64-bit capable or often used with an
> 32-bit ARM kernel, so having the ARMv8 PMUs work under a 32-bit ARM
> kernel would be great. The downstream solution we have sued thus far is
> to find the closest compatible string to represent those, which is not
> great...
Ah, so you're *really* doing that? I'm not going to ask why, I'm scared
of the answer... ;-)
Anyway, I can repost that series if that will prevent people from having
that kind of silly hacks.
Thanks,
M.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm/pmuv3-32bit
--
Jazz is not dead. It just smells funny...
Hi Marc,
> Marc Zyngier <[email protected]> hat am 18. Mai 2018 um 10:07 geschrieben:
>
> I have a pretty simple series[1] which I used to profile 32bit guests on
> an arm64 KVM host. Nobody really cared about it because running a 32bit
> kernel on 64bit HW is a bit odd, to say the least, and I'm probably the
> only one actually running 32bit VMs.
>
> > FWIW, Broadcom STB chips, even when 64-bit capable or often used with an
> > 32-bit ARM kernel, so having the ARMv8 PMUs work under a 32-bit ARM
> > kernel would be great. The downstream solution we have sued thus far is
> > to find the closest compatible string to represent those, which is not
> > great...
> Ah, so you're *really* doing that? I'm not going to ask why, I'm scared
> of the answer... ;-)
>
> Anyway, I can repost that series if that will prevent people from having
> that kind of silly hacks.
yes please. But there is a minor nit: some patches introduce new files without SPDX tags.
Thanks
Stefan
>
> Thanks,
>
> M.
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm/pmuv3-32bit
> --
> Jazz is not dead. It just smells funny...
On 18/05/18 10:37, Stefan Wahren wrote:
> Hi Marc,
>
>> Marc Zyngier <[email protected]> hat am 18. Mai 2018 um 10:07 geschrieben:
>>
>> I have a pretty simple series[1] which I used to profile 32bit guests on
>> an arm64 KVM host. Nobody really cared about it because running a 32bit
>> kernel on 64bit HW is a bit odd, to say the least, and I'm probably the
>> only one actually running 32bit VMs.
>>
>>> FWIW, Broadcom STB chips, even when 64-bit capable or often used with an
>>> 32-bit ARM kernel, so having the ARMv8 PMUs work under a 32-bit ARM
>>> kernel would be great. The downstream solution we have sued thus far is
>>> to find the closest compatible string to represent those, which is not
>>> great...
>> Ah, so you're *really* doing that? I'm not going to ask why, I'm scared
>> of the answer... ;-)
>>
>> Anyway, I can repost that series if that will prevent people from having
>> that kind of silly hacks.
>
> yes please. But there is a minor nit: some patches introduce new files without SPDX tags.
I'm shocked! ;-)
M.
--
Jazz is not dead. It just smells funny...