From: Laszlo Ersek <lersek@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>,
qemu-arm@nongnu.org, qemu-devel@nongnu.org
Cc: "Alex Bennée" <alex.bennee@linaro.org>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>,
"Richard Henderson" <richard.henderson@linaro.org>
Subject: Re: [Qemu-devel] [PATCH for-4.1] target/arm: Limit ID register assertions to TCG
Date: Fri, 19 Jul 2019 00:45:42 +0200 [thread overview]
Message-ID: <a47660f2-4b72-3ae7-7bf2-90db54d553ae@redhat.com> (raw)
In-Reply-To: <20190718125928.20147-1-peter.maydell@linaro.org>
On 07/18/19 14:59, Peter Maydell wrote:
> In arm_cpu_realizefn() we make several assertions about the values of
> guest ID registers:
> * if the CPU provides AArch32 v7VE or better it must advertise the
> ARM_DIV feature
> * if the CPU provides AArch32 A-profile v6 or better it must
> advertise the Jazelle feature
>
> These are essentially consistency checks that our ID register
> specifications in cpu.c didn't accidentally miss out a feature,
> because increasingly the TCG emulation gates features on the values
> in ID registers rather than using old-style checks of ARM_FEATURE_FOO
> bits.
>
> Unfortunately, these asserts can cause problems if we're running KVM,
> because in that case we don't control the values of the ID registers
> -- we read them from the host kernel. In particular, if the host
> kernel is older than 4.15 then it doesn't expose the ID registers via
> the KVM_GET_ONE_REG ioctl, and we set up dummy values for some
> registers and leave the rest at zero. (See the comment in
> target/arm/kvm64.c kvm_arm_get_host_cpu_features().) This set of
> dummy values is not sufficient to pass our assertions, and so on
> those kernels running an AArch32 guest on AArch64 will assert.
>
> We could provide a more sophisticated set of dummy ID registers in
> this case, but that still leaves the possibility of a host CPU which
> reports bogus ID register values that would cause us to assert. It's
> more robust to only do these ID register checks if we're using TCG,
> as that is the only case where this is truly a QEMU code bug.
>
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Fixes: https://bugs.launchpad.net/qemu/+bug/1830864
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Laszlo, would you mind testing this on your setup? I don't have
> a system with an old enough kernel to trigger the assert. (The
> change is pretty much a "has to work" one though :-))
32-bit guest runs fine, with this patch applied to v4.1.0-rc1 :)
Tested-by: Laszlo Ersek <lersek@redhat.com>
Thank you!
Laszlo
>
> target/arm/cpu.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 1959467fdc8..9eb40ff755f 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1369,6 +1369,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> * There exist AArch64 cpus without AArch32 support. When KVM
> * queries ID_ISAR0_EL1 on such a host, the value is UNKNOWN.
> * Similarly, we cannot check ID_AA64PFR0 without AArch64 support.
> + * As a general principle, we also do not make ID register
> + * consistency checks anywhere unless using TCG, because only
> + * for TCG would a consistency-check failure be a QEMU bug.
> */
> if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
> no_aa32 = !cpu_isar_feature(aa64_aa32, cpu);
> @@ -1383,7 +1386,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> * Presence of EL2 itself is ARM_FEATURE_EL2, and of the
> * Security Extensions is ARM_FEATURE_EL3.
> */
> - assert(no_aa32 || cpu_isar_feature(arm_div, cpu));
> + assert(!tcg_enabled() || no_aa32 || cpu_isar_feature(arm_div, cpu));
> set_feature(env, ARM_FEATURE_LPAE);
> set_feature(env, ARM_FEATURE_V7);
> }
> @@ -1409,7 +1412,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> if (arm_feature(env, ARM_FEATURE_V6)) {
> set_feature(env, ARM_FEATURE_V5);
> if (!arm_feature(env, ARM_FEATURE_M)) {
> - assert(no_aa32 || cpu_isar_feature(jazelle, cpu));
> + assert(!tcg_enabled() || no_aa32 || cpu_isar_feature(jazelle, cpu));
> set_feature(env, ARM_FEATURE_AUXCR);
> }
> }
>
next prev parent reply other threads:[~2019-07-18 22:46 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-18 12:59 [Qemu-devel] [PATCH for-4.1] target/arm: Limit ID register assertions to TCG Peter Maydell
2019-07-18 14:58 ` Richard Henderson
2019-07-18 22:45 ` Laszlo Ersek [this message]
2019-07-19 17:21 ` Philippe Mathieu-Daudé
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=a47660f2-4b72-3ae7-7bf2-90db54d553ae@redhat.com \
--to=lersek@redhat.com \
--cc=alex.bennee@linaro.org \
--cc=peter.maydell@linaro.org \
--cc=philmd@redhat.com \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.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 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).