Re: [Qemu-devel] [PULL 08/10] target/arm: Conditionalize some asserts on aarch32 support

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 7/16/19 8:42 PM, Laszlo Ersek wrote:
> On 07/16/19 18:59, Peter Maydell wrote:
>> On Tue, 16 Jul 2019 at 17:51, Laszlo Ersek <lersek@redhat.com> wrote:
>>> The issue still reproduces, so it makes sense for me to look at the host
>>> kernel version... Well, I'm afraid it won't help much, for an upstream
>>> investigation:
>>>
>>>   4.14.0-115.8.2.el7a.aarch64
>>>
>>> This is the latest released kernel from "Red Hat Enterprise Linux for
>>> ARM 64 7".
>>
>> OK. (I'm using 4.15.0-51-generic from ubuntu).
>>
>> Could you run with QEMU under gdb, and when it hits the
>> assertion go back up a stack frame to the arm_cpu_realizefn()
>> frame, and then "print /x cpu->isar" ? That should show us
>> what we think we've got as ID registers from the kernel.
>> (You might need to build QEMU with --enable-debug to get
>> useful enough debug info to do that, not sure.)
> 
> (My qemu build script always builds QEMU in two configs, the difference
> being --prefix and --enable-debug.)
> 
> This is what I got:
> 
> (gdb) frame 4
> #4  0x00000000006a063c in arm_cpu_realizefn (dev=0x1761140,
>     errp=0xffffffffe540)
>     at .../qemu/target/arm/cpu.c:1159
> 1159            assert(no_aa32 || cpu_isar_feature(arm_div, cpu));
> (gdb) print /x cpu->isar
> $1 = {id_isar0 = 0x0, id_isar1 = 0x0, id_isar2 = 0x0, id_isar3 = 0x0,
>   id_isar4 = 0x0, id_isar5 = 0x0, id_isar6 = 0x0, mvfr0 = 0x0,
>   mvfr1 = 0x0, mvfr2 = 0x0, id_aa64isar0 = 0x0, id_aa64isar1 = 0x0,
>   id_aa64pfr0 = 0x11, id_aa64pfr1 = 0x0, id_aa64mmfr0 = 0x0,
>   id_aa64mmfr1 = 0x0}

For ISAR0, DIVIDE=0

so cpu_isar_feature(arm_div, cpu)=false

For AA64PFR0, EL0=1, EL1=1.

EL0 = 1: EL0 can be executed in AArch64 state only.
EL1 = 1: EL1 can be executed in AArch64 state only.

so cpu_isar_feature(aa64_aa32, cpu)=false
then no_aa32=true

The commit description is "on a host that doesn't support aarch32 mode
at all, neither arm_div nor jazelle will be supported either."

Shouldn't we use a slighly different logic? Such:

-    assert(no_aa32 || cpu_isar_feature(arm_div, cpu));
+    assert(no_aa32 && !cpu_isar_feature(arm_div, cpu));