From: linux@roeck-us.net (Guenter Roeck)
To: linux-arm-kernel@lists.infradead.org
Subject: [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
Date: Thu, 7 Jan 2016 09:10:14 -0800 [thread overview]
Message-ID: <568E9BF6.6020809@roeck-us.net> (raw)
In-Reply-To: <20160107163722.GB4064@red-moon>
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
WARNING: multiple messages have this Message-ID (diff)
From: Guenter Roeck <linux@roeck-us.net>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Peter Maydell <peter.maydell@linaro.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Will Deacon <will.deacon@arm.com>,
QEMU Developers <qemu-devel@nongnu.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
Date: Thu, 7 Jan 2016 09:10:14 -0800 [thread overview]
Message-ID: <568E9BF6.6020809@roeck-us.net> (raw)
In-Reply-To: <20160107163722.GB4064@red-moon>
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
WARNING: multiple messages have this Message-ID (diff)
From: Guenter Roeck <linux@roeck-us.net>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Peter Maydell <peter.maydell@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
Will Deacon <will.deacon@arm.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore'
Date: Thu, 7 Jan 2016 09:10:14 -0800 [thread overview]
Message-ID: <568E9BF6.6020809@roeck-us.net> (raw)
In-Reply-To: <20160107163722.GB4064@red-moon>
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
next prev parent reply other threads:[~2016-01-07 17:10 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-24 0:52 arm64 qemu tests failing in linux-next since 'arm64: kernel: enforce pmuserenr_el0 initialization and restore' Guenter Roeck
2015-12-24 0:52 ` [Qemu-devel] " Guenter Roeck
2015-12-24 0:52 ` Guenter Roeck
2016-01-06 11:21 ` Lorenzo Pieralisi
2016-01-06 11:21 ` [Qemu-devel] " Lorenzo Pieralisi
2016-01-06 11:21 ` Lorenzo Pieralisi
2016-01-07 13:25 ` [Qemu-devel] " Peter Maydell
2016-01-07 13:25 ` Peter Maydell
2016-01-07 13:25 ` Peter Maydell
2016-01-07 15:53 ` Lorenzo Pieralisi
2016-01-07 15:53 ` Lorenzo Pieralisi
2016-01-07 15:53 ` Lorenzo Pieralisi
2016-01-07 15:58 ` Peter Maydell
2016-01-07 15:58 ` Peter Maydell
2016-01-07 15:58 ` Peter Maydell
2016-01-07 16:37 ` Lorenzo Pieralisi
2016-01-07 16:37 ` Lorenzo Pieralisi
2016-01-07 16:37 ` Lorenzo Pieralisi
2016-01-07 16:56 ` Peter Maydell
2016-01-07 16:56 ` Peter Maydell
2016-01-07 16:56 ` Peter Maydell
2016-01-07 17:13 ` Guenter Roeck
2016-01-07 17:13 ` Guenter Roeck
2016-01-07 17:13 ` Guenter Roeck
2016-01-07 17:10 ` Guenter Roeck [this message]
2016-01-07 17:10 ` Guenter Roeck
2016-01-07 17:10 ` Guenter Roeck
2016-01-07 17:19 ` Peter Maydell
2016-01-07 17:19 ` Peter Maydell
2016-01-07 17:19 ` Peter Maydell
2016-01-07 18:31 ` Lorenzo Pieralisi
2016-01-07 18:31 ` Lorenzo Pieralisi
2016-01-07 18:31 ` Lorenzo Pieralisi
2016-01-07 16:21 ` Guenter Roeck
2016-01-07 16:21 ` Guenter Roeck
2016-01-07 16:21 ` Guenter Roeck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=568E9BF6.6020809@roeck-us.net \
--to=linux@roeck-us.net \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.