qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
@ 2015-12-24  0:52 Guenter Roeck
  2016-01-06 11:21 ` Lorenzo Pieralisi
  2016-01-07 13:25 ` Peter Maydell
  0 siblings, 2 replies; 12+ messages in thread
From: Guenter Roeck @ 2015-12-24  0:52 UTC (permalink / raw)
  To: linux-kernel@vger.kernel.org
  Cc: Mark Rutland, Lorenzo Pieralisi, Will Deacon, QEMU Developers,
	linux-arm-kernel@lists.infradead.org

Hi all,

since commit 60792ad349f3 ("arm64: kernel: enforce pmuserenr_el0 initialization
and restore"), my arm64 qemu tests of linux-next are failing. After this commit,
qemu does not display any output.

Qemu version is 2.5.0. Linux kernel configuration is arm64:defconfig.

qemu command line is as follows:

	qemu-system-aarch64 -machine virt -cpu cortex-a57 -machine type=virt -nographic -smp 1 \
		-m 512 -kernel arch/arm64/boot/Image -initrd rootfs.arm64.cpio -no-reboot \
		-append "console=ttyAMA0"

Any idea what might cause this problem and how to fix it (presumably in qemu) ?

Bisect log is attached below. Reverting commit 60792ad349f3 on top of linux-next
fixes the problem.

Thanks,
Guenter

---
# bad: [80c75a0f1d81922bf322c0634d1e1a15825a89e6] Add linux-next specific files for 20151223
# good: [4ef7675344d687a0ef5b0d7c0cee12da005870c0] Linux 4.4-rc6
git bisect start 'HEAD' 'v4.4-rc6'
# bad: [52c8be920db8e42d195ca7fe93fe31aa9958100e] Merge remote-tracking branch 'drm/drm-next'
git bisect bad 52c8be920db8e42d195ca7fe93fe31aa9958100e
# bad: [ef1ebde6af4d839a07d787440e35f0ae3b02e567] Merge remote-tracking branch 'v4l-dvb/master'
git bisect bad ef1ebde6af4d839a07d787440e35f0ae3b02e567
# bad: [1e4012d4f91fd118167f14c4172bf779a7884d26] Merge remote-tracking branch 'tegra/for-next'
git bisect bad 1e4012d4f91fd118167f14c4172bf779a7884d26
# good: [392fd3291c93094ca65853cca5e168016c4e08b1] Merge branch 'next/dt64' into for-next
git bisect good 392fd3291c93094ca65853cca5e168016c4e08b1
# bad: [49fc6d2449b0cebd9738694a9c9ee794c3686797] Merge remote-tracking branch 'omap/for-next'
git bisect bad 49fc6d2449b0cebd9738694a9c9ee794c3686797
# bad: [f5a47ef34509cbce244c18bef02b175d0e48dc4f] Merge remote-tracking branch 'at91/at91-next'
git bisect bad f5a47ef34509cbce244c18bef02b175d0e48dc4f
# good: [59f8d523983105e8490603ae1c0798207e9781e6] Merge remote-tracking branch 'arc/for-next'
git bisect good 59f8d523983105e8490603ae1c0798207e9781e6
# good: [40499303a6c59c96da587a91fca617017106e908] Merge branch 'next/defconfig' into for-next
git bisect good 40499303a6c59c96da587a91fca617017106e908
# good: [ea07b401d16052b43782c6389c9c2115aa3077ff] Merge branches 'component' and 'misc' into for-next
git bisect good ea07b401d16052b43782c6389c9c2115aa3077ff
# bad: [5d7ee87708d4d86fcc32afc9552d05f7625d303d] arm64: perf: add support for Cortex-A72
git bisect bad 5d7ee87708d4d86fcc32afc9552d05f7625d303d
# good: [9e9caa6a496174e53d7753baa4779717771da4a7] arm64: perf: Add event descriptions
git bisect good 9e9caa6a496174e53d7753baa4779717771da4a7
# bad: [60792ad349f3c6dc5735aafefe5dc9121c79e320] arm64: kernel: enforce pmuserenr_el0 initialization and restore
git bisect bad 60792ad349f3c6dc5735aafefe5dc9121c79e320
# good: [aae881ad73460e1b2aea01f079a0541bd5a9136c] arm64: perf: Correct Cortex-A53/A57 compatible values
git bisect good aae881ad73460e1b2aea01f079a0541bd5a9136c
# first bad commit: [60792ad349f3c6dc5735aafefe5dc9121c79e320] arm64: kernel: enforce pmuserenr_el0 initialization and restore

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

* Re: [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
  2015-12-24  0:52 [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore' Guenter Roeck
@ 2016-01-06 11:21 ` Lorenzo Pieralisi
  2016-01-07 13:25 ` Peter Maydell
  1 sibling, 0 replies; 12+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-06 11:21 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Mark Rutland, Will Deacon, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, QEMU Developers

Hi Guenter,

On Wed, Dec 23, 2015 at 04:52:51PM -0800, Guenter Roeck wrote:
> Hi all,
> 
> since commit 60792ad349f3 ("arm64: kernel: enforce pmuserenr_el0 initialization
> and restore"), my arm64 qemu tests of linux-next are failing. After this commit,
> qemu does not display any output.
> 
> Qemu version is 2.5.0. Linux kernel configuration is arm64:defconfig.
> 
> qemu command line is as follows:
> 
> 	qemu-system-aarch64 -machine virt -cpu cortex-a57 -machine type=virt -nographic -smp 1 \
> 		-m 512 -kernel arch/arm64/boot/Image -initrd rootfs.arm64.cpio -no-reboot \
> 		-append "console=ttyAMA0"
> 
> Any idea what might cause this problem and how to fix it (presumably in qemu) ?

We took notice of this and we are going to look into it shortly, thanks for
the heads-up.

Lorenzo

> 
> Bisect log is attached below. Reverting commit 60792ad349f3 on top of linux-next
> fixes the problem.
> 
> Thanks,
> Guenter
> 
> ---
> # bad: [80c75a0f1d81922bf322c0634d1e1a15825a89e6] Add linux-next specific files for 20151223
> # good: [4ef7675344d687a0ef5b0d7c0cee12da005870c0] Linux 4.4-rc6
> git bisect start 'HEAD' 'v4.4-rc6'
> # bad: [52c8be920db8e42d195ca7fe93fe31aa9958100e] Merge remote-tracking branch 'drm/drm-next'
> git bisect bad 52c8be920db8e42d195ca7fe93fe31aa9958100e
> # bad: [ef1ebde6af4d839a07d787440e35f0ae3b02e567] Merge remote-tracking branch 'v4l-dvb/master'
> git bisect bad ef1ebde6af4d839a07d787440e35f0ae3b02e567
> # bad: [1e4012d4f91fd118167f14c4172bf779a7884d26] Merge remote-tracking branch 'tegra/for-next'
> git bisect bad 1e4012d4f91fd118167f14c4172bf779a7884d26
> # good: [392fd3291c93094ca65853cca5e168016c4e08b1] Merge branch 'next/dt64' into for-next
> git bisect good 392fd3291c93094ca65853cca5e168016c4e08b1
> # bad: [49fc6d2449b0cebd9738694a9c9ee794c3686797] Merge remote-tracking branch 'omap/for-next'
> git bisect bad 49fc6d2449b0cebd9738694a9c9ee794c3686797
> # bad: [f5a47ef34509cbce244c18bef02b175d0e48dc4f] Merge remote-tracking branch 'at91/at91-next'
> git bisect bad f5a47ef34509cbce244c18bef02b175d0e48dc4f
> # good: [59f8d523983105e8490603ae1c0798207e9781e6] Merge remote-tracking branch 'arc/for-next'
> git bisect good 59f8d523983105e8490603ae1c0798207e9781e6
> # good: [40499303a6c59c96da587a91fca617017106e908] Merge branch 'next/defconfig' into for-next
> git bisect good 40499303a6c59c96da587a91fca617017106e908
> # good: [ea07b401d16052b43782c6389c9c2115aa3077ff] Merge branches 'component' and 'misc' into for-next
> git bisect good ea07b401d16052b43782c6389c9c2115aa3077ff
> # bad: [5d7ee87708d4d86fcc32afc9552d05f7625d303d] arm64: perf: add support for Cortex-A72
> git bisect bad 5d7ee87708d4d86fcc32afc9552d05f7625d303d
> # good: [9e9caa6a496174e53d7753baa4779717771da4a7] arm64: perf: Add event descriptions
> git bisect good 9e9caa6a496174e53d7753baa4779717771da4a7
> # bad: [60792ad349f3c6dc5735aafefe5dc9121c79e320] arm64: kernel: enforce pmuserenr_el0 initialization and restore
> git bisect bad 60792ad349f3c6dc5735aafefe5dc9121c79e320
> # good: [aae881ad73460e1b2aea01f079a0541bd5a9136c] arm64: perf: Correct Cortex-A53/A57 compatible values
> git bisect good aae881ad73460e1b2aea01f079a0541bd5a9136c
> # first bad commit: [60792ad349f3c6dc5735aafefe5dc9121c79e320] arm64: kernel: enforce pmuserenr_el0 initialization and restore
> 

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

* Re: [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
  2015-12-24  0:52 [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore' Guenter Roeck
  2016-01-06 11:21 ` Lorenzo Pieralisi
@ 2016-01-07 13:25 ` Peter Maydell
  2016-01-07 15:53   ` Lorenzo Pieralisi
  1 sibling, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2016-01-07 13:25 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Mark Rutland, Lorenzo Pieralisi, Will Deacon,
	linux-kernel@vger.kernel.org, QEMU Developers,
	linux-arm-kernel@lists.infradead.org

On 24 December 2015 at 00:52, Guenter Roeck <linux@roeck-us.net> wrote:
> Hi all,
>
> since commit 60792ad349f3 ("arm64: kernel: enforce pmuserenr_el0
> initialization
> and restore"), my arm64 qemu tests of linux-next are failing. After this
> commit,
> qemu does not display any output.
>
> Qemu version is 2.5.0. Linux kernel configuration is arm64:defconfig.
>
> qemu command line is as follows:
>
>         qemu-system-aarch64 -machine virt -cpu cortex-a57 -machine type=virt
> -nographic -smp 1 \
>                 -m 512 -kernel arch/arm64/boot/Image -initrd
> rootfs.arm64.cpio -no-reboot \
>                 -append "console=ttyAMA0"
>
> Any idea what might cause this problem and how to fix it (presumably in
> qemu) ?

This turns out to be because QEMU doesn't currently implement
PMUSERENR_EL0 for AArch64 (we do have an AArch32 implementation),
so you get an immediate UNDEF when the kernel touches it, followed
by an infinite loop of UNDEF exceptions because the instruction
at the UNDEF vector entrypoint is unallocated at this point in
execution.

We had previously been relying on the kernel not attempting to
touch the PMU if the ID_AA64DFR0_EL1 PMUVer bits read 0000
("Performance Monitors extension System registers not implemented").

Since the v8 ARM ARM states that the Performance Monitors Extension is
an optional feature of an implementation, this seems like a kernel
bug to me. (QEMU should probably get round to implementing the PMU
at some point for feature parity with v7, but this has not been
a priority for us since they're not actually very useful in a
fully emulated setup.)

thanks
-- PMM

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

* Re: [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
  2016-01-07 13:25 ` Peter Maydell
@ 2016-01-07 15:53   ` Lorenzo Pieralisi
  2016-01-07 15:58     ` Peter Maydell
  2016-01-07 16:21     ` Guenter Roeck
  0 siblings, 2 replies; 12+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-07 15:53 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Mark Rutland, Will Deacon, linux-kernel@vger.kernel.org,
	QEMU Developers, Guenter Roeck,
	linux-arm-kernel@lists.infradead.org

On Thu, Jan 07, 2016 at 01:25:35PM +0000, Peter Maydell wrote:
> On 24 December 2015 at 00:52, Guenter Roeck <linux@roeck-us.net> wrote:
> > Hi all,
> >
> > since commit 60792ad349f3 ("arm64: kernel: enforce pmuserenr_el0
> > initialization
> > and restore"), my arm64 qemu tests of linux-next are failing. After this
> > commit,
> > qemu does not display any output.
> >
> > Qemu version is 2.5.0. Linux kernel configuration is arm64:defconfig.
> >
> > qemu command line is as follows:
> >
> >         qemu-system-aarch64 -machine virt -cpu cortex-a57 -machine type=virt
> > -nographic -smp 1 \
> >                 -m 512 -kernel arch/arm64/boot/Image -initrd
> > rootfs.arm64.cpio -no-reboot \
> >                 -append "console=ttyAMA0"
> >
> > Any idea what might cause this problem and how to fix it (presumably in
> > qemu) ?
> 
> This turns out to be because QEMU doesn't currently implement
> PMUSERENR_EL0 for AArch64 (we do have an AArch32 implementation),
> so you get an immediate UNDEF when the kernel touches it, followed
> by an infinite loop of UNDEF exceptions because the instruction
> at the UNDEF vector entrypoint is unallocated at this point in
> execution.
> 
> We had previously been relying on the kernel not attempting to
> touch the PMU if the ID_AA64DFR0_EL1 PMUVer bits read 0000
> ("Performance Monitors extension System registers not implemented").

Ok, thanks for looking into this. I wonder why reading pmcr_el0 does
not suffer from the same problem though.

> Since the v8 ARM ARM states that the Performance Monitors Extension is
> an optional feature of an implementation, this seems like a kernel
> bug to me. (QEMU should probably get round to implementing the PMU
> at some point for feature parity with v7, but this has not been
> a priority for us since they're not actually very useful in a
> fully emulated setup.)

Fixup patch coming, thanks.

Lorenzo

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

* Re: [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
  2016-01-07 15:53   ` Lorenzo Pieralisi
@ 2016-01-07 15:58     ` Peter Maydell
  2016-01-07 16:37       ` Lorenzo Pieralisi
  2016-01-07 16:21     ` Guenter Roeck
  1 sibling, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2016-01-07 15:58 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Mark Rutland, Will Deacon, linux-kernel@vger.kernel.org,
	QEMU Developers, Guenter Roeck,
	linux-arm-kernel@lists.infradead.org

On 7 January 2016 at 15:53, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> On Thu, Jan 07, 2016 at 01:25:35PM +0000, Peter Maydell wrote:
>> We had previously been relying on the kernel not attempting to
>> touch the PMU if the ID_AA64DFR0_EL1 PMUVer bits read 0000
>> ("Performance Monitors extension System registers not implemented").
>
> Ok, thanks for looking into this. I wonder why reading pmcr_el0 does
> not suffer from the same problem though.

Just a pragmatic thing on QEMU's end, I expect -- the kernel already
touched PMCR_EL0 and we wanted to be able to boot it, so we have an
implementation of it.

thanks
-- PMM

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

* Re: [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
  2016-01-07 15:53   ` Lorenzo Pieralisi
  2016-01-07 15:58     ` Peter Maydell
@ 2016-01-07 16:21     ` Guenter Roeck
  1 sibling, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2016-01-07 16:21 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Peter Maydell
  Cc: Mark Rutland, Will Deacon, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, QEMU Developers

On 01/07/2016 07:53 AM, Lorenzo Pieralisi wrote:
> On Thu, Jan 07, 2016 at 01:25:35PM +0000, Peter Maydell wrote:
>> On 24 December 2015 at 00:52, Guenter Roeck <linux@roeck-us.net> wrote:
>>> Hi all,
>>>
>>> since commit 60792ad349f3 ("arm64: kernel: enforce pmuserenr_el0
>>> initialization
>>> and restore"), my arm64 qemu tests of linux-next are failing. After this
>>> commit,
>>> qemu does not display any output.
>>>
>>> Qemu version is 2.5.0. Linux kernel configuration is arm64:defconfig.
>>>
>>> qemu command line is as follows:
>>>
>>>          qemu-system-aarch64 -machine virt -cpu cortex-a57 -machine type=virt
>>> -nographic -smp 1 \
>>>                  -m 512 -kernel arch/arm64/boot/Image -initrd
>>> rootfs.arm64.cpio -no-reboot \
>>>                  -append "console=ttyAMA0"
>>>
>>> Any idea what might cause this problem and how to fix it (presumably in
>>> qemu) ?
>>
>> This turns out to be because QEMU doesn't currently implement
>> PMUSERENR_EL0 for AArch64 (we do have an AArch32 implementation),
>> so you get an immediate UNDEF when the kernel touches it, followed
>> by an infinite loop of UNDEF exceptions because the instruction
>> at the UNDEF vector entrypoint is unallocated at this point in
>> execution.
>>
>> We had previously been relying on the kernel not attempting to
>> touch the PMU if the ID_AA64DFR0_EL1 PMUVer bits read 0000
>> ("Performance Monitors extension System registers not implemented").
>
> Ok, thanks for looking into this. I wonder why reading pmcr_el0 does
> not suffer from the same problem though.
>
>> Since the v8 ARM ARM states that the Performance Monitors Extension is
>> an optional feature of an implementation, this seems like a kernel
>> bug to me. (QEMU should probably get round to implementing the PMU
>> at some point for feature parity with v7, but this has not been
>> a priority for us since they're not actually very useful in a
>> fully emulated setup.)
>
> Fixup patch coming, thanks.
>

The following code around the register accesses fixes the problem for me.
+       mrs     x0, ID_AA64DFR0_EL1
+       tst     x0, #0xf00
+       b.eq    1f
         msr     pmuserenr_el0, xzr              // Disable PMU access from EL0
+1:

I don't have a real system, so I can not verify if the register is correctly
set there. Plus, of course, I don't really know aarch64 assembler, so the above
code may be plain wrong ;-).

Guenter

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

* Re: [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
  2016-01-07 15:58     ` Peter Maydell
@ 2016-01-07 16:37       ` Lorenzo Pieralisi
  2016-01-07 16:56         ` Peter Maydell
  2016-01-07 17:10         ` Guenter Roeck
  0 siblings, 2 replies; 12+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-07 16:37 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Mark Rutland, Will Deacon, linux-kernel@vger.kernel.org,
	QEMU Developers, Guenter Roeck,
	linux-arm-kernel@lists.infradead.org

On Thu, Jan 07, 2016 at 03:58:15PM +0000, Peter Maydell wrote:
> On 7 January 2016 at 15:53, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> > On Thu, Jan 07, 2016 at 01:25:35PM +0000, Peter Maydell wrote:
> >> We had previously been relying on the kernel not attempting to
> >> touch the PMU if the ID_AA64DFR0_EL1 PMUVer bits read 0000
> >> ("Performance Monitors extension System registers not implemented").
> >
> > Ok, thanks for looking into this. I wonder why reading pmcr_el0 does
> > not suffer from the same problem though.
> 
> Just a pragmatic thing on QEMU's end, I expect -- the kernel already
> touched PMCR_EL0 and we wanted to be able to boot it, so we have an
> implementation of it.

If that's the case, that was the wrong approach IMHO. QEMU has to comply
with the Aarch64 architecture which means that either the CPU it models
has a Performance Monitors extension or it does not. If reading pmcr_el0
does not fault I could tell you this is a QEMU regression because currently
it _does_ model pmcr_el0 while (hopefully) ID_AA64DFR0_EL1 PMUVer reports
it should not.

I will add code that guards both register accesses to fix both bugs at
once.

Thanks,
Lorenzo

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

* Re: [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
  2016-01-07 16:37       ` Lorenzo Pieralisi
@ 2016-01-07 16:56         ` Peter Maydell
  2016-01-07 17:13           ` Guenter Roeck
  2016-01-07 17:10         ` Guenter Roeck
  1 sibling, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2016-01-07 16:56 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Mark Rutland, Will Deacon, linux-kernel@vger.kernel.org,
	QEMU Developers, Guenter Roeck,
	linux-arm-kernel@lists.infradead.org

On 7 January 2016 at 16:37, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> On Thu, Jan 07, 2016 at 03:58:15PM +0000, Peter Maydell wrote:
>> On 7 January 2016 at 15:53, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
>> > Ok, thanks for looking into this. I wonder why reading pmcr_el0 does
>> > not suffer from the same problem though.
>>
>> Just a pragmatic thing on QEMU's end, I expect -- the kernel already
>> touched PMCR_EL0 and we wanted to be able to boot it, so we have an
>> implementation of it.
>
> If that's the case, that was the wrong approach IMHO. QEMU has to comply
> with the Aarch64 architecture which means that either the CPU it models
> has a Performance Monitors extension or it does not. If reading pmcr_el0
> does not fault I could tell you this is a QEMU regression because currently
> it _does_ model pmcr_el0 while (hopefully) ID_AA64DFR0_EL1 PMUVer reports
> it should not.

I agree it's a bug, but QEMU simply doesn't have enough
developers to become fully compliant with the architecture (ie to
implement every part of it completely). So we concentrate on the
parts that people are actually using, and fill in the rest and
fix the bugs as time permits or as real guest software starts to
use it.

If you want a guaranteed matches-the-architecture software model
of an ARM CPU then other models are available :-)

thanks
-- PMM

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

* Re: [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
  2016-01-07 16:37       ` Lorenzo Pieralisi
  2016-01-07 16:56         ` Peter Maydell
@ 2016-01-07 17:10         ` Guenter Roeck
  2016-01-07 17:19           ` Peter Maydell
  2016-01-07 18:31           ` Lorenzo Pieralisi
  1 sibling, 2 replies; 12+ messages in thread
From: Guenter Roeck @ 2016-01-07 17:10 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Peter Maydell
  Cc: Mark Rutland, Will Deacon, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, QEMU Developers

On 01/07/2016 08:37 AM, Lorenzo Pieralisi wrote:
> On Thu, Jan 07, 2016 at 03:58:15PM +0000, Peter Maydell wrote:
>> On 7 January 2016 at 15:53, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
>>> On Thu, Jan 07, 2016 at 01:25:35PM +0000, Peter Maydell wrote:
>>>> We had previously been relying on the kernel not attempting to
>>>> touch the PMU if the ID_AA64DFR0_EL1 PMUVer bits read 0000
>>>> ("Performance Monitors extension System registers not implemented").
>>>
>>> Ok, thanks for looking into this. I wonder why reading pmcr_el0 does
>>> not suffer from the same problem though.
>>
>> Just a pragmatic thing on QEMU's end, I expect -- the kernel already
>> touched PMCR_EL0 and we wanted to be able to boot it, so we have an
>> implementation of it.
>
> If that's the case, that was the wrong approach IMHO. QEMU has to comply
> with the Aarch64 architecture which means that either the CPU it models
> has a Performance Monitors extension or it does not. If reading pmcr_el0
> does not fault I could tell you this is a QEMU regression because currently
> it _does_ model pmcr_el0 while (hopefully) ID_AA64DFR0_EL1 PMUVer reports
> it should not.
>

Strictly speaking you may be right (regression is a bit strong, though),
but for my part I tend to be pragmatic.

A warning message such as "Access to unimplemented register X" may be useful,
but effectively disabling all (older) aarch64 Linux kernels in qemu could be
seen as a bit dogmatic and would not be very helpful.

> I will add code that guards both register accesses to fix both bugs at
> once.
>

I assume you'll fix the the unconditional access(es) to pmcr_el0.

Question here is the scope of registers associated with PMUVer. Are there
any other registers which would need to be guarded ?

Thanks,
Guenter

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

* Re: [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
  2016-01-07 16:56         ` Peter Maydell
@ 2016-01-07 17:13           ` Guenter Roeck
  0 siblings, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2016-01-07 17:13 UTC (permalink / raw)
  To: Peter Maydell, Lorenzo Pieralisi
  Cc: Mark Rutland, Will Deacon, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, QEMU Developers

On 01/07/2016 08:56 AM, Peter Maydell wrote:
> On 7 January 2016 at 16:37, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
>> On Thu, Jan 07, 2016 at 03:58:15PM +0000, Peter Maydell wrote:
>>> On 7 January 2016 at 15:53, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
>>>> Ok, thanks for looking into this. I wonder why reading pmcr_el0 does
>>>> not suffer from the same problem though.
>>>
>>> Just a pragmatic thing on QEMU's end, I expect -- the kernel already
>>> touched PMCR_EL0 and we wanted to be able to boot it, so we have an
>>> implementation of it.
>>
>> If that's the case, that was the wrong approach IMHO. QEMU has to comply
>> with the Aarch64 architecture which means that either the CPU it models
>> has a Performance Monitors extension or it does not. If reading pmcr_el0
>> does not fault I could tell you this is a QEMU regression because currently
>> it _does_ model pmcr_el0 while (hopefully) ID_AA64DFR0_EL1 PMUVer reports
>> it should not.
>
> I agree it's a bug, but QEMU simply doesn't have enough
> developers to become fully compliant with the architecture (ie to
> implement every part of it completely). So we concentrate on the
> parts that people are actually using, and fill in the rest and
> fix the bugs as time permits or as real guest software starts to
> use it.
>
> If you want a guaranteed matches-the-architecture software model
> of an ARM CPU then other models are available :-)
>
I think it would be better to convince ARM to put some manpower into
enhancing qemu, instead of telling them to use some other model ;-).

Guenter

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

* Re: [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
  2016-01-07 17:10         ` Guenter Roeck
@ 2016-01-07 17:19           ` Peter Maydell
  2016-01-07 18:31           ` Lorenzo Pieralisi
  1 sibling, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2016-01-07 17:19 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Mark Rutland, Lorenzo Pieralisi, Will Deacon,
	linux-kernel@vger.kernel.org, QEMU Developers,
	linux-arm-kernel@lists.infradead.org

On 7 January 2016 at 17:10, Guenter Roeck <linux@roeck-us.net> wrote:
> Strictly speaking you may be right (regression is a bit strong, though),
> but for my part I tend to be pragmatic.
>
> A warning message such as "Access to unimplemented register X" may be
> useful

You can get these from QEMU if you pass it "-d unimp", which logs
various kinds of things-not-yet-implemented, with a couple of caveats:
 * the warning is when we translate the code, not when we execute it
 * it won't warn for registers which we implement but not completely
   (eg only partial functionality or dummy reads-as-written)

In this case it printed
"write access to unsupported AArch64 system register op0:3 op1:3 crn:9
crm:14 op2:0"

The 'guest_errors' suboption to -d warns about things which appear
to be errors in the guest OS, for instance some kinds of UNPREDICTABLE,
and may also be of interest.

Neither guest_errors nor unimp are comprehensive (there are many
more situations where we don't warn than where we do) but they can
be helpful sometimes.

thanks
-- PMM

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

* Re: [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
  2016-01-07 17:10         ` Guenter Roeck
  2016-01-07 17:19           ` Peter Maydell
@ 2016-01-07 18:31           ` Lorenzo Pieralisi
  1 sibling, 0 replies; 12+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-07 18:31 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Mark Rutland, Peter Maydell, Will Deacon,
	linux-kernel@vger.kernel.org, QEMU Developers,
	linux-arm-kernel@lists.infradead.org

On Thu, Jan 07, 2016 at 09:10:14AM -0800, Guenter Roeck wrote:

[...]

> >If that's the case, that was the wrong approach IMHO. QEMU has to comply
> >with the Aarch64 architecture which means that either the CPU it models
> >has a Performance Monitors extension or it does not. If reading pmcr_el0
> >does not fault I could tell you this is a QEMU regression because currently
> >it _does_ model pmcr_el0 while (hopefully) ID_AA64DFR0_EL1 PMUVer reports
> >it should not.
> >
> 
> Strictly speaking you may be right (regression is a bit strong, though),
> but for my part I tend to be pragmatic.

It is a kernel bug and I will fix it. Regardless, I still think that
modelling pmcr_el0 to make sure the kernel boot even with ID_AA64DFR0_EL1
PMUVer reporting that the CPU is not implementing a Performance Monitors
extension was wrong.

> A warning message such as "Access to unimplemented register X" may be useful,
> but effectively disabling all (older) aarch64 Linux kernels in qemu could be
> seen as a bit dogmatic and would not be very helpful.
> 
> >I will add code that guards both register accesses to fix both bugs at
> >once.
> >
> 
> I assume you'll fix the the unconditional access(es) to pmcr_el0.

Yes.

> Question here is the scope of registers associated with PMUVer. Are there
> any other registers which would need to be guarded ?

None that I am aware of, other PMU registers are accessed only if PMUs
are probed (since they are present in DT or ACPI), which means that at
that point QEMU will have to model the Performance Monitors extension
entirely since it advertises them in the respective FW.

I could add a warning in the v8 PMU probing path to check PMUVer if we
think that's helpful.

Lorenzo

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

end of thread, other threads:[~2016-01-07 18:30 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-24  0:52 [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore' Guenter Roeck
2016-01-06 11:21 ` Lorenzo Pieralisi
2016-01-07 13:25 ` Peter Maydell
2016-01-07 15:53   ` Lorenzo Pieralisi
2016-01-07 15:58     ` Peter Maydell
2016-01-07 16:37       ` Lorenzo Pieralisi
2016-01-07 16:56         ` Peter Maydell
2016-01-07 17:13           ` Guenter Roeck
2016-01-07 17:10         ` Guenter Roeck
2016-01-07 17:19           ` Peter Maydell
2016-01-07 18:31           ` Lorenzo Pieralisi
2016-01-07 16:21     ` Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).