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 <ler...@redhat.com> > Fixes: https://bugs.launchpad.net/qemu/+bug/1830864 > Signed-off-by: Peter Maydell <peter.mayd...@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 <ler...@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); > } > } >