[Qemu-devel] AArch64 QEMU System emulation: issue with TTBR0

[Qemu-devel] AArch64 QEMU System emulation: issue with TTBR0

[Claudio Fontana]

Hello Peter,

I am porting OSv to AArch64, and I have some working code running on
the Foundation Models,
where I run qemu natively with --enable-kvm,

which does not seem to work when run instead on top of the system emulation.

In particular I get a sync exception when I try to msr to TTBR0_EL1.

The ESR as read in env->cp15.esr_el[1] is 0x8400000e, which looking up
in the ESR table means

Instruction fault, with IFSC (instruction fault status code) = 0xe,
which should match

0b0011LL = permission fault (LL indicates level at which fault occurred).

with LL = 0b10 meaning EL2.

The code is in particular:

00000000401db2d0 <mmu::switch_to_runtime_page_tables()>:
    401db2d0:   d00037a0        adrp    x0, 408d1000 <unique_mtx+0x10>
    401db2d4:   9130e000        add     x0, x0, #0xc38
    401db2d8:   f9400000        ldr     x0, [x0]
    401db2dc:   92748c00        and     x0, x0, #0xfffffffff000
    401db2e0:   d5182000        msr     ttbr0_el1, x0
    401db2e4:   d5033fdf        isb
    401db2e8:   d00037a0        adrp    x0, 408d1000 <unique_mtx+0x10>
    401db2ec:   9130e000        add     x0, x0, #0xc38
    401db2f0:   f9400400        ldr     x0, [x0,#8]
    401db2f4:   92748c00        and     x0, x0, #0xfffffffff000
    401db2f8:   d5182020        msr     ttbr1_el1, x0
    401db2fc:   d5033fdf        isb
    401db300:   d5033f9f        dsb     sy
    401db304:   d508831f        tlbi    vmalle1is
    401db308:   d5033f9f        dsb     sy
    401db30c:   d5033fdf        isb
    401db310:   d65f03c0        ret

ELR_EL1 in env->elr_el[1] reads as 0x401da200, which is strangely
enough the address of the first instruction of the exception vector
entry for sync:

        ...
    401da200:   14000169        b       401da7a4 <entry_sync>
    401da204:   d503201f        nop
        ...
    401da280:   14000174        b       401da850 <entry_irq>
    401da284:   d503201f        nop

The source is available at:

https://github.com/cloudius-systems/osv/blob/master/arch/aarch64/mmu.cc

Thanks for any advice,

Claudio

Re: [Qemu-devel] AArch64 QEMU System emulation: issue with TTBR0

[Claudio Fontana]

On 2 June 2014 18:16, Claudio Fontana <hw.claudio@gmail.com> wrote:
> Hello Peter,
>
> I am porting OSv to AArch64, and I have some working code running on
> the Foundation Models,
> where I run qemu natively with --enable-kvm,
>
> which does not seem to work when run instead on top of the system emulation.
>
> In particular I get a sync exception when I try to msr to TTBR0_EL1.
>
> The ESR as read in env->cp15.esr_el[1] is 0x8400000e, which looking up
> in the ESR table means
>
> Instruction fault, with IFSC (instruction fault status code) = 0xe,
> which should match
>
> 0b0011LL = permission fault (LL indicates level at which fault occurred).
>
> with LL = 0b10 meaning EL2.
>
> The code is in particular:
>
> 00000000401db2d0 <mmu::switch_to_runtime_page_tables()>:
>     401db2d0:   d00037a0        adrp    x0, 408d1000 <unique_mtx+0x10>
>     401db2d4:   9130e000        add     x0, x0, #0xc38
>     401db2d8:   f9400000        ldr     x0, [x0]
>     401db2dc:   92748c00        and     x0, x0, #0xfffffffff000
>     401db2e0:   d5182000        msr     ttbr0_el1, x0
>     401db2e4:   d5033fdf        isb
>     401db2e8:   d00037a0        adrp    x0, 408d1000 <unique_mtx+0x10>
>     401db2ec:   9130e000        add     x0, x0, #0xc38
>     401db2f0:   f9400400        ldr     x0, [x0,#8]
>     401db2f4:   92748c00        and     x0, x0, #0xfffffffff000
>     401db2f8:   d5182020        msr     ttbr1_el1, x0
>     401db2fc:   d5033fdf        isb
>     401db300:   d5033f9f        dsb     sy
>     401db304:   d508831f        tlbi    vmalle1is
>     401db308:   d5033f9f        dsb     sy
>     401db30c:   d5033fdf        isb
>     401db310:   d65f03c0        ret
>
> ELR_EL1 in env->elr_el[1] reads as 0x401da200, which is strangely
> enough the address of the first instruction of the exception vector
> entry for sync:
>
>         ...
>     401da200:   14000169        b       401da7a4 <entry_sync>
>     401da204:   d503201f        nop
>         ...
>     401da280:   14000174        b       401da850 <entry_irq>
>     401da284:   d503201f        nop
>
> The source is available at:
>
> https://github.com/cloudius-systems/osv/blob/master/arch/aarch64/mmu.cc
>
> Thanks for any advice,
>
> Claudio

hmm one thing that came to mind of course after posting, is that QEMU
in system emulation mode probably tries to start at EL2 or EL3, while
on OSv I assume to be running as a guest at EL1.
However, I have a check to validate the exception level, after which I
should drop to EL1 instead of halting if I am at EL2/EL3.

The check however does not seem to capture this case, as it passes.
The check goes:

validate_el:
        mrs     x0, currentel
        ubfm    x0, x0, #2, #3 // current EL[3:2] -> X0
        cmp     x0, #1
        b.ne    halt
        ret

Re: [Qemu-devel] AArch64 QEMU System emulation: issue with TTBR0

[Peter Maydell]

On 2 June 2014 17:21, Claudio Fontana <hw.claudio@gmail.com> wrote:
> hmm one thing that came to mind of course after posting, is that QEMU
> in system emulation mode probably tries to start at EL2 or EL3, while
> on OSv I assume to be running as a guest at EL1.

No, at the moment both QEMU's system emulation and KVM
will start in EL1; EL2/EL3 emulation isn't implemented
yet, and KVM doesn't support guests running at EL2/3.

thanks
-- PMM

Re: [Qemu-devel] AArch64 QEMU System emulation: issue with TTBR0

[Peter Maydell]

On 2 June 2014 17:16, Claudio Fontana <hw.claudio@gmail.com> wrote:
> In particular I get a sync exception when I try to msr to TTBR0_EL1.
>
> The ESR as read in env->cp15.esr_el[1] is 0x8400000e, which looking up
> in the ESR table means
>
> Instruction fault, with IFSC (instruction fault status code) = 0xe,
> which should match
>
> 0b0011LL = permission fault (LL indicates level at which fault occurred).
>
> with LL = 0b10 meaning EL2.

No, "permission fault, second level" doesn't mean EL2, it means
"at the second level of the page table".

In other words, QEMU thinks the page tables you've just pointed
us at are wrong. I suggest sticking a breakpoint in get_phys_addr_lpae()
and walking through it to see what address it's trying to load
and why it thinks the page table walk indicates a permission
fault.

> The code is in particular:
>
> 00000000401db2d0 <mmu::switch_to_runtime_page_tables()>:
>     401db2d0:   d00037a0        adrp    x0, 408d1000 <unique_mtx+0x10>
>     401db2d4:   9130e000        add     x0, x0, #0xc38
>     401db2d8:   f9400000        ldr     x0, [x0]
>     401db2dc:   92748c00        and     x0, x0, #0xfffffffff000
>     401db2e0:   d5182000        msr     ttbr0_el1, x0
>     401db2e4:   d5033fdf        isb
>     401db2e8:   d00037a0        adrp    x0, 408d1000 <unique_mtx+0x10>
>     401db2ec:   9130e000        add     x0, x0, #0xc38
>     401db2f0:   f9400400        ldr     x0, [x0,#8]
>     401db2f4:   92748c00        and     x0, x0, #0xfffffffff000
>     401db2f8:   d5182020        msr     ttbr1_el1, x0
>     401db2fc:   d5033fdf        isb
>     401db300:   d5033f9f        dsb     sy
>     401db304:   d508831f        tlbi    vmalle1is
>     401db308:   d5033f9f        dsb     sy
>     401db30c:   d5033fdf        isb
>     401db310:   d65f03c0        ret
>
> ELR_EL1 in env->elr_el[1] reads as 0x401da200, which is strangely
> enough the address of the first instruction of the exception vector
> entry for sync:

This suggests that we faulted once, and then faulted again
because we couldn't read the insn at the vector entrypoint
either, and you didn't catch the first one in whatever debugger
you're using, only the second. (Were you single-stepping in
gdb? IIRC that has the effect of stopping after trying to
execute the first insn at the exception vector, rather than
before it.)

thanks
-- PMM

Re: [Qemu-devel] AArch64 QEMU System emulation: issue with TTBR0

[Claudio Fontana]

On 02.06.2014 18:37, Peter Maydell wrote:
> On 2 June 2014 17:16, Claudio Fontana <hw.claudio@gmail.com> wrote:
>> In particular I get a sync exception when I try to msr to TTBR0_EL1.
>>
>> The ESR as read in env->cp15.esr_el[1] is 0x8400000e, which looking up
>> in the ESR table means
>>
>> Instruction fault, with IFSC (instruction fault status code) = 0xe,
>> which should match
>>
>> 0b0011LL = permission fault (LL indicates level at which fault occurred).
>>
>> with LL = 0b10 meaning EL2.
> 
> No, "permission fault, second level" doesn't mean EL2, it means
> "at the second level of the page table".
> 
> In other words, QEMU thinks the page tables you've just pointed
> us at are wrong. I suggest sticking a breakpoint in get_phys_addr_lpae()
> and walking through it to see what address it's trying to load
> and why it thinks the page table walk indicates a permission
> fault.
> 
>> The code is in particular:
>>
>> 00000000401db2d0 <mmu::switch_to_runtime_page_tables()>:
>>     401db2d0:   d00037a0        adrp    x0, 408d1000 <unique_mtx+0x10>
>>     401db2d4:   9130e000        add     x0, x0, #0xc38
>>     401db2d8:   f9400000        ldr     x0, [x0]
>>     401db2dc:   92748c00        and     x0, x0, #0xfffffffff000
>>     401db2e0:   d5182000        msr     ttbr0_el1, x0
>>     401db2e4:   d5033fdf        isb
>>     401db2e8:   d00037a0        adrp    x0, 408d1000 <unique_mtx+0x10>
>>     401db2ec:   9130e000        add     x0, x0, #0xc38
>>     401db2f0:   f9400400        ldr     x0, [x0,#8]
>>     401db2f4:   92748c00        and     x0, x0, #0xfffffffff000
>>     401db2f8:   d5182020        msr     ttbr1_el1, x0
>>     401db2fc:   d5033fdf        isb
>>     401db300:   d5033f9f        dsb     sy
>>     401db304:   d508831f        tlbi    vmalle1is
>>     401db308:   d5033f9f        dsb     sy
>>     401db30c:   d5033fdf        isb
>>     401db310:   d65f03c0        ret
>>
>> ELR_EL1 in env->elr_el[1] reads as 0x401da200, which is strangely
>> enough the address of the first instruction of the exception vector
>> entry for sync:
> 
> This suggests that we faulted once, and then faulted again
> because we couldn't read the insn at the vector entrypoint
> either, and you didn't catch the first one in whatever debugger
> you're using, only the second. (Were you single-stepping in
> gdb? IIRC that has the effect of stopping after trying to
> execute the first insn at the exception vector, rather than
> before it.)
> 
> thanks
> -- PMM
> 

Thank you for the clarifications and advice, I think executable permissions might be involved, as removing the NX / PNX check in get_phys_addr_lpae() makes it proceed ahead ([OT: until it dies again trying to read the cpumask from the GIC]),
but I could not find anything really wrong with the PNX setting in OSv (not using NX), but probably more investigation would be needed.

Thanks,

Claudio

Re: [Qemu-devel] AArch64 QEMU System emulation: issue with TTBR0

[Ian Campbell]

On Tue, 2014-06-03 at 14:28 +0200, Claudio Fontana wrote:
> Thank you for the clarifications and advice, I think executable
> permissions might be involved, as removing the NX / PNX check in
> get_phys_addr_lpae() makes it proceed ahead

I'm seeing something very similar running modprobe, I get a kernel fault
(see below) which I also tracked down to the NX/PNX checks in
get_phys_addr_lpae().

At the moment I'm a bit suspicious of:
        /* Extract attributes from the descriptor and merge with table attrs */
        if (arm_feature(env, ARM_FEATURE_V8)) {
            attrs = extract64(descriptor, 2, 10)
                | (extract64(descriptor, 53, 11) << 10);
        } else {
            attrs = extract64(descriptor, 2, 10)
                | (extract64(descriptor, 52, 12) << 10);
        }

I'm not sure what the reason for the v8 difference is, it seems like it
is skipping extracting the CONTIG bit but I've not dug into the v8 ARM
ARM to figure out why that might be desirable...

Since in the v8 case extracts fewer bits from higher up but uses the
same << 10 shift, which seems like it ought to then confuse later checks
with use 1<<11 and 1<<12. Making that <<10 into <<11 doesn't help though
so I think I might be barking up the wrong tree...

Ian.

[    2.399053] Bad mode in Synchronous Abort handler detected, code 0x8400000f
[    2.399482] CPU: 0 PID: 45 Comm: systemd-udevd Not tainted 3.14-1-arm64 #1 Debian 3.14.5-2
[    2.399769] task: ffffffc006f96440 ti: ffffffc006fb0000 task.ti: ffffffc006fb0000
[    2.400603] PC is at virtio_init+0x0/0x2c [virtio]
[    2.401061] LR is at do_one_initcall+0xdc/0x124
[    2.401214] pc : [<ffffffbffc000658>] lr : [<ffffffc000080c30>] pstate: 60000145
[    2.401418] sp : ffffffc006fb3c50
[    2.401548] x29: ffffffc006fb3c50 x28: 0000000000000001 
[    2.401791] x27: 0000000000000000 x26: ffffffc0005f9000 
[    2.401973] x25: ffffffc006f1a380 x24: ffffffbffc000b08 
[    2.402152] x23: ffffffc006fb0000 x22: ffffffbffc000b58 
[    2.402331] x21: 0000000000000000 x20: ffffffbffc000658 
[    2.402505] x19: ffffffc006fb0000 x18: 0000007fd073cbb0 
[    2.402674] x17: 0000007fac1e6df0 x16: ffffffc000094274 
[    2.402826] x15: 0000007fac2690b8 x14: ffffffffffffffb8 
[    2.402978] x13: 00000000ffffffff x12: 0000000000000001 
[    2.403131] x11: ffffffc00063aea0 x10: 0000000000000000 
[    2.403318] x9 : 0000000000000001 x8 : 00000000000000dc 
[    2.403500] x7 : ffffffc007ffbf00 x6 : 2222222200112222 
[    2.403674] x5 : 0000000000000000 x4 : 0000000000000000 
[    2.403848] x3 : 0000000000000000 x2 : 0000000000000002 
[    2.404024] x1 : ffffffc006fb3c50 x0 : 0000000000000000 
[    2.404203] 
[    2.404473] Internal error: Oops - bad mode: 0 [#1] SMP
[    2.404743] Modules linked in: virtio(+)
[    2.405083] CPU: 0 PID: 45 Comm: systemd-udevd Not tainted 3.14-1-arm64 #1 Debian 3.14.5-2
[    2.405313] task: ffffffc006f96440 ti: ffffffc006fb0000 task.ti: ffffffc006fb0000
[    2.405546] PC is at virtio_init+0x0/0x2c [virtio]
[    2.405707] LR is at do_one_initcall+0xdc/0x124
[    2.405857] pc : [<ffffffbffc000658>] lr : [<ffffffc000080c30>] pstate: 60000145
[    2.406059] sp : ffffffc006fb3c50
[    2.406161] x29: ffffffc006fb3c50 x28: 0000000000000001 
[    2.406336] x27: 0000000000000000 x26: ffffffc0005f9000 
[    2.406510] x25: ffffffc006f1a380 x24: ffffffbffc000b08 
[    2.406685] x23: ffffffc006fb0000 x22: ffffffbffc000b58 
[    2.406861] x21: 0000000000000000 x20: ffffffbffc000658 
[    2.407039] x19: ffffffc006fb0000 x18: 0000007fd073cbb0 
[    2.407215] x17: 0000007fac1e6df0 x16: ffffffc000094274 
[    2.407392] x15: 0000007fac2690b8 x14: ffffffffffffffb8 
[    2.407567] x13: 00000000ffffffff x12: 0000000000000001 
[    2.407744] x11: ffffffc00063aea0 x10: 0000000000000000 
[    2.407915] x9 : 0000000000000001 x8 : 00000000000000dc 
[    2.408089] x7 : ffffffc007ffbf00 x6 : 2222222200112222 
[    2.408264] x5 : 0000000000000000 x4 : 0000000000000000 
[    2.408440] x3 : 0000000000000000 x2 : 0000000000000002 
[    2.408616] x1 : ffffffc006fb3c50 x0 : 0000000000000000 
[    2.408787] 
[    2.408872] Process systemd-udevd (pid: 45, stack limit = 0xffffffc006fb0058)
[    2.409115] Stack: (0xffffffc006fb3c50 to 0xffffffc006fb4000)
[    2.409389] 3c40:                                     06fb3cd0 ffffffc0 000f6c70 ffffffc0
[    2.409643] 3c60: 005f9b98 ffffffc0 005f9bb0 ffffffc0 fc000b20 ffffffbf fc000b58 ffffffbf
[    2.409892] 3c80: 06fb0000 ffffffc0 000b5170 ffffffc0 005f9b98 ffffffc0 005f9bb0 ffffffc0
[    2.410130] 3ca0: 00000000 00000000 ffffffff 00000000 fc000b08 ffffffbf 00000001 00000000
[    2.410364] 3cc0: 06fb3cd0 ffffffc0 000f6c64 ffffffc0 06fb3e40 ffffffc0 000f7374 ffffffc0
[    2.410598] 3ce0: 00000000 00000000 00000005 00000000 7fd1b730 0000007f 7fc926a4 0000007f
[    2.410832] 3d00: 80000000 00000000 00000015 00000000 00000114 00000000 00000111 00000000
[    2.411060] 3d20: 005f0000 ffffffc0 06fb0000 ffffffc0 06f6b0c0 ffffffc0 06f1a390 ffffffc0
[    2.411291] 3d40: 000f3bdc ffffffc0 00000072 00000000 0000006e 00000000 00533930 ffffffc0
[    2.411523] 3d60: 0000003f 00000000 0000feff 00000000 0000fff1 00000000 00000019 00000000
[    2.411755] 3d80: 06fb3de0 ffffffc0 fc003000 ffffffbf 06fb3ec4 ffffffc0 06fb3e80 ffffffc0
[    2.411987] 3da0: 00640360 ffffffc0 00000000 00000000 00000000 0000007f 00000000 00000000
[    2.412219] 3dc0: 80000000 00000000 00000015 00000000 06fb3de0 ffffffc0 00000784 00000000
[    2.412451] 3de0: 00000002 ffff81a4 00000001 00000000 00000000 00000000 00000000 00000000
[    2.412682] 3e00: fc0006c8 ffffffbf 00000005 00000000 00000000 00000000 00000000 00000000
[    2.412913] 3e20: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    2.413140] 3e40: d8f57fa0 0000007f 00082fec ffffffc0 2d241670 00000000 7fd1b730 0000007f
[    2.413368] 3e60: ffffffff ffffffff 2d237ed0 00000000 00031000 ffffff80 00003560 00000000
[    2.413603] 3e80: 000322e8 ffffff80 000321e8 ffffff80 000333a0 ffffff80 00000d68 00000000
[    2.413850] 3ea0: 00000f60 00000000 00000000 00000000 00000000 00000000 00000018 00000019
[    2.414082] 3ec0: 00000013 0000000f 0000000c 00000000 00000005 00000000 7fd1b730 0000007f
[    2.414310] 3ee0: 00000000 00000000 00000005 00000000 00000000 00000000 00000000 00000000
[    2.414538] 3f00: ffffffff ffffffff ffffffff ffffffff 00000111 00000000 feff00d0 fefefefe
[    2.414767] 3f20: ffffffff 00000000 00000028 00000000 00000030 00000000 00000008 00000000
[    2.414996] 3f40: 206f6b2e 6e72656b 7fcf90b8 0000007f 7fc92680 0000007f 7fd2d288 0000007f
[    2.415226] 3f60: d8f57d00 0000007f 2d241670 00000000 7fd1b730 0000007f 2d237ed0 00000000
[    2.415456] 3f80: 00000000 00000000 00020000 00000000 2d23ac20 00000000 00000000 00000000
[    2.415686] 3fa0: 00020000 00000000 00000000 00000000 00000000 00000000 d8f57fa0 0000007f
[    2.415921] 3fc0: 7fd152e8 0000007f d8f57fa0 0000007f 7fc926a4 0000007f 80000000 00000000
[    2.416152] 3fe0: 00000005 00000000 00000111 00000000 00000000 00000000 00000000 00000000
[    2.416412] Call trace:
[    2.416676] [<ffffffbffc000658>] virtio_init+0x0/0x2c [virtio]
[    2.416879] [<ffffffc0000f6c6c>] load_module+0x19b8/0x1f54
[    2.417109] [<ffffffc0000f7370>] SyS_finit_module+0x78/0x88
[    2.417370] Code: 95109058 90000000 9122e000 95108e70 (a9bf7bfd) 
[    2.418098] ---[ end trace 97ac04d8bcc4878d ]---

Re: [Qemu-devel] AArch64 QEMU System emulation: issue with TTBR0

[Peter Maydell]

On 8 June 2014 12:26, Ian Campbell <ijc@hellion.org.uk> wrote:
> On Tue, 2014-06-03 at 14:28 +0200, Claudio Fontana wrote:
>> Thank you for the clarifications and advice, I think executable
>> permissions might be involved, as removing the NX / PNX check in
>> get_phys_addr_lpae() makes it proceed ahead
>
> I'm seeing something very similar running modprobe, I get a kernel fault
> (see below) which I also tracked down to the NX/PNX checks in
> get_phys_addr_lpae().
>
> At the moment I'm a bit suspicious of:
>         /* Extract attributes from the descriptor and merge with table attrs */
>         if (arm_feature(env, ARM_FEATURE_V8)) {
>             attrs = extract64(descriptor, 2, 10)
>                 | (extract64(descriptor, 53, 11) << 10);
>         } else {
>             attrs = extract64(descriptor, 2, 10)
>                 | (extract64(descriptor, 52, 12) << 10);
>         }
>
> I'm not sure what the reason for the v8 difference is, it seems like it
> is skipping extracting the CONTIG bit but I've not dug into the v8 ARM
> ARM to figure out why that might be desirable...

The CONTIG bit is purely a hint to the implementation, so it's
valid to completely ignore it. (Given how QEMU's TLB works it
doesn't really make sense for us.) However it's present in v7's
long descriptor format as well, so I'm not sure why this change
is guarded by ARM_FEATURE_V8. Rob?

> Since in the v8 case extracts fewer bits from higher up but uses the
> same << 10 shift, which seems like it ought to then confuse later checks
> with use 1<<11 and 1<<12. Making that <<10 into <<11 doesn't help though
> so I think I might be barking up the wrong tree...

I think this should be unconditionally
            attrs = extract64(descriptor, 2, 10)
                 | (extract64(descriptor, 52, 12) << 10);
(as we currently have for v7 LPAE)
as we're just assembling the upper and lower attribute bits
into a contiguous set. We can then happily ignore anything
like CONTIG that we don't care about.

thanks
-- PMM

Re: [Qemu-devel] AArch64 QEMU System emulation: issue with TTBR0

[Ian Campbell]

On Sun, 2014-06-08 at 13:19 +0100, Peter Maydell wrote:
> On 8 June 2014 12:26, Ian Campbell <ijc@hellion.org.uk> wrote:
> > On Tue, 2014-06-03 at 14:28 +0200, Claudio Fontana wrote:
> >> Thank you for the clarifications and advice, I think executable
> >> permissions might be involved, as removing the NX / PNX check in
> >> get_phys_addr_lpae() makes it proceed ahead
> >
> > I'm seeing something very similar running modprobe, I get a kernel fault
> > (see below) which I also tracked down to the NX/PNX checks in
> > get_phys_addr_lpae().
> >
> > At the moment I'm a bit suspicious of:
> >         /* Extract attributes from the descriptor and merge with table attrs */
> >         if (arm_feature(env, ARM_FEATURE_V8)) {
> >             attrs = extract64(descriptor, 2, 10)
> >                 | (extract64(descriptor, 53, 11) << 10);
> >         } else {
> >             attrs = extract64(descriptor, 2, 10)
> >                 | (extract64(descriptor, 52, 12) << 10);
> >         }
> >
> > I'm not sure what the reason for the v8 difference is, it seems like it
> > is skipping extracting the CONTIG bit but I've not dug into the v8 ARM
> > ARM to figure out why that might be desirable...
> 
> The CONTIG bit is purely a hint to the implementation, so it's
> valid to completely ignore it. (Given how QEMU's TLB works it
> doesn't really make sense for us.) However it's present in v7's
> long descriptor format as well, so I'm not sure why this change
> is guarded by ARM_FEATURE_V8.

Exactly what I was wondering.

>  Rob?
> 
> > Since in the v8 case extracts fewer bits from higher up but uses the
> > same << 10 shift, which seems like it ought to then confuse later checks
> > with use 1<<11 and 1<<12. Making that <<10 into <<11 doesn't help though
> > so I think I might be barking up the wrong tree...
> 
> I think this should be unconditionally
>             attrs = extract64(descriptor, 2, 10)
>                  | (extract64(descriptor, 52, 12) << 10);
> (as we currently have for v7 LPAE)
> as we're just assembling the upper and lower attribute bits
> into a contiguous set. We can then happily ignore anything
> like CONTIG that we don't care about.

I agree. With the following debug the existing code generates:
        get_phys_addr_lpae: looking up ffffffbffc000658
        get_phys_addr_lpae: l1 descr 4007f003 (!XNTable,!PXNTable,!XN,!PXN)
        get_phys_addr_lpae: l2 descr 46e13003 (!XNTable,!PXNTable,!XN,!PXN)
        get_phys_addr_lpae: l3 descr 2c0000046e11713 (!XNTable,!PXNTable,XN,!PXN)
        attrs: lower 1c4 upper 16 (V8 style, 2..11, 53..63)
        attrs 59c4 | 0 => 59c4 (merging from tableattrs 0)
        get_phys_addr_lpae: fault with attrs 59c4, is_user 0, access_type 2, level 3
        get_phys_addr_lpae: tableattrs 0
        get_phys_addr_lpae: XN == 1000 yes, PXN == 800 yes

Notice the disconnect between PXN in the l3 descr and what we see when
we inject the fault (the final line). Making the change you suggest (by
uncommenting the "0 &&" in the debug) instead produces:
        get_phys_addr_lpae: looking up ffffffbffc000658
        get_phys_addr_lpae: l1 descr 4007f003 (!XNTable,!PXNTable,!XN,!PXN)
        get_phys_addr_lpae: l2 descr 46fee003 (!XNTable,!PXNTable,!XN,!PXN)
        get_phys_addr_lpae: l3 descr 2c0000046fec713 (!XNTable,!PXNTable,XN,!PXN)
        attrs: lower 1c4 upper 2c (V7 style, 2..11, 52..63)
        attrs b1c4 | 0 => b1c4 (merging from tableattrs 0)
        get_phys_addr_lpae: fault with attrs b1c4, is_user 0, access_type 2, level 3
        get_phys_addr_lpae: tableattrs 0
        get_phys_addr_lpae: XN == 1000 yes, PXN == 800 no
        
Which I think is correct.

Ian.


diff --git a/target-arm/helper.c b/target-arm/helper.c
index ec031f5..2748245 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3793,6 +3793,8 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
     int32_t va_size = 32;
     int32_t tbi = 0;
 
+    qemu_log("--------------------------------------\n");
+    qemu_log("%s: looking up %lx\n", __func__, address);
     if (arm_el_is_aa64(env, 1)) {
         va_size = 64;
         if (extract64(address, 55, 1))
@@ -3905,6 +3907,11 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
         descaddr |= (address >> (granule_sz * (4 - level))) & descmask;
         descaddr &= ~7ULL;
         descriptor = ldq_phys(cs->as, descaddr);
+        qemu_log("%s: l%d descr %lx (%s,%s,%s,%s)\n", __func__, level, descriptor,
+                 descriptor & (1UL<<60) ? "XNTable" : "!XNTable",
+                 descriptor & (1UL<<59) ? "PXNTable" : "!PXNTable",
+                 descriptor & (1UL<<54) ? "XN" : "!XN",
+                 descriptor & (1UL<<53) ? "PXN" : "!PXN");
         if (!(descriptor & 1) ||
             (!(descriptor & 2) && (level == 3))) {
             /* Invalid, or the Reserved level 3 encoding */
@@ -3929,13 +3936,23 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
         page_size = (1 << ((granule_sz * (4 - level)) + 3));
         descaddr |= (address & (page_size - 1));
         /* Extract attributes from the descriptor and merge with table attrs */
-        if (arm_feature(env, ARM_FEATURE_V8)) {
+        if (/*0 &&*/ arm_feature(env, ARM_FEATURE_V8)) {
             attrs = extract64(descriptor, 2, 10)
                 | (extract64(descriptor, 53, 11) << 10);
+            qemu_log("attrs: lower %lx upper %lx (V8 style, 2..11, 53..63)\n",
+                     extract64(descriptor, 2, 10),
+                     extract64(descriptor, 53, 11));
         } else {
             attrs = extract64(descriptor, 2, 10)
                 | (extract64(descriptor, 52, 12) << 10);
+            qemu_log("attrs: lower %lx upper %lx (V7 style, 2..11, 52..63)\n",
+                     extract64(descriptor, 2, 10),
+                     extract64(descriptor, 52, 12));
         }
+        qemu_log("attrs %x | %x => %x (merging from tableattrs %x)\n",
+                 attrs, extract32(tableattrs, 0, 2) << 11,
+                 attrs | extract32(tableattrs, 0, 2) << 11,
+                 tableattrs);
         attrs |= extract32(tableattrs, 0, 2) << 11; /* XN, PXN */
         attrs |= extract32(tableattrs, 3, 1) << 5; /* APTable[1] => AP[2] */
         /* The sense of AP[1] vs APTable[0] is reversed, as APTable[0] == 1
@@ -3961,13 +3978,22 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
         goto do_fault;
     }
     *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+#if 1
     if (attrs & (1 << 12) || (!is_user && (attrs & (1 << 11)))) {
         /* XN or PXN */
         if (access_type == 2) {
+            qemu_log("%s: fault with attrs %x, is_user %d, access_type %d, level %d\n", __func__,
+                attrs, is_user, access_type, level);
+            qemu_log("%s: tableattrs %x\n", __func__, tableattrs);
+            qemu_log("%s: XN == %x %s, PXN == %x %s\n", __func__,
+                     1<<12, (attrs & (1<<12)) ? "yes" : "no",
+                     1<<11, (attrs & (1<<11)) ? "yes" : "no");
+            abort();
             goto do_fault;
         }
         *prot &= ~PAGE_EXEC;
     }
+#endif
     if (attrs & (1 << 5)) {
         /* Write access forbidden */
         if (access_type == 1) {

Re: [Qemu-devel] AArch64 QEMU System emulation: issue with TTBR0

[Ian Campbell]

On Sun, 2014-06-08 at 14:27 +0100, Ian Campbell wrote:
>         get_phys_addr_lpae: XN == 1000 yes, PXN == 800 no
>         
> Which I think is correct.

Aha, for VMSA-v8 bit 54 is UXN unlike v7 where it is just XN. So that's
the underlying bug I'm seeing I think...

I'll confirm and post a patch.

Ian.

[Qemu-devel] [PATCH] target-arm: A64: Correct handling of UXN bit.

[Ian Campbell]

In v8 page tables bit 54 in the PTE is UXN in the EL0/EL1 translation regimes
and XN elsewhere. In v7 the bit is always XN. Since we only emulate EL0/EL1 we
can just treat this bit as UXN whenever we are in v8 mode.

Also correctly extract the upper attributes from the PTE entry, the v8 version
tried to avoid extracting the CONTIG bit and ended up with the upper bits being
off-by-one. Instead behave the same as v7 and extract (but ignore) the CONTIG
bit.

This fixes "Bad mode in Synchronous Abort handler detected, code 0x8400000f"
seen when modprobing modules under Linux.

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Claudio Fontana <claudio.fontana@huawei.com>
Cc: Rob Herring <robherring2@gmail.com>
---
 target-arm/helper.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index ec031f5..5872a00 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3929,13 +3929,8 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
         page_size = (1 << ((granule_sz * (4 - level)) + 3));
         descaddr |= (address & (page_size - 1));
         /* Extract attributes from the descriptor and merge with table attrs */
-        if (arm_feature(env, ARM_FEATURE_V8)) {
-            attrs = extract64(descriptor, 2, 10)
-                | (extract64(descriptor, 53, 11) << 10);
-        } else {
-            attrs = extract64(descriptor, 2, 10)
-                | (extract64(descriptor, 52, 12) << 10);
-        }
+        attrs = extract64(descriptor, 2, 10)
+            | (extract64(descriptor, 52, 12) << 10);
         attrs |= extract32(tableattrs, 0, 2) << 11; /* XN, PXN */
         attrs |= extract32(tableattrs, 3, 1) << 5; /* APTable[1] => AP[2] */
         /* The sense of AP[1] vs APTable[0] is reversed, as APTable[0] == 1
@@ -3961,8 +3956,12 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
         goto do_fault;
     }
     *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
-    if (attrs & (1 << 12) || (!is_user && (attrs & (1 << 11)))) {
-        /* XN or PXN */
+    if ((arm_feature(env, ARM_FEATURE_V8) && is_user && (attrs & (1 << 12))) ||
+        (!arm_feature(env, ARM_FEATURE_V8) && (attrs & (1 << 12))) ||
+        (!is_user && (attrs & (1 << 11)))) {
+        /* XN/UXN or PXN. Since we only implement EL0/EL1 we unconditionally
+         * treat XN/UXN as UXN for v8.
+         */
         if (access_type == 2) {
             goto do_fault;
         }
-- 
1.9.0

Re: [Qemu-devel] [PATCH] target-arm: A64: Correct handling of UXN bit.

[Peter Maydell]

On 8 June 2014 14:53, Ian Campbell <ijc@hellion.org.uk> wrote:
> In v8 page tables bit 54 in the PTE is UXN in the EL0/EL1 translation regimes
> and XN elsewhere. In v7 the bit is always XN. Since we only emulate EL0/EL1 we
> can just treat this bit as UXN whenever we are in v8 mode.
>
> Also correctly extract the upper attributes from the PTE entry, the v8 version
> tried to avoid extracting the CONTIG bit and ended up with the upper bits being
> off-by-one. Instead behave the same as v7 and extract (but ignore) the CONTIG
> bit.
>
> This fixes "Bad mode in Synchronous Abort handler detected, code 0x8400000f"
> seen when modprobing modules under Linux.
>
> Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Claudio Fontana <claudio.fontana@huawei.com>
> Cc: Rob Herring <robherring2@gmail.com>

Thanks, applied to target-arm.next.
To those interested in EL2/EL3 support: what's the
plan for telling the get_phys_addr() functions which
translation regime they should be operating in?

(since that is what indicates whether this bit is UXN or XN,
as well as having various other effects).

thanks
-- PMM

Re: [Qemu-devel] [PATCH] target-arm: A64: Correct handling of UXN bit.

[Edgar E. Iglesias]

On Mon, Jun 09, 2014 at 02:40:59PM +0100, Peter Maydell wrote:
> On 8 June 2014 14:53, Ian Campbell <ijc@hellion.org.uk> wrote:
> > In v8 page tables bit 54 in the PTE is UXN in the EL0/EL1 translation regimes
> > and XN elsewhere. In v7 the bit is always XN. Since we only emulate EL0/EL1 we
> > can just treat this bit as UXN whenever we are in v8 mode.
> >
> > Also correctly extract the upper attributes from the PTE entry, the v8 version
> > tried to avoid extracting the CONTIG bit and ended up with the upper bits being
> > off-by-one. Instead behave the same as v7 and extract (but ignore) the CONTIG
> > bit.
> >
> > This fixes "Bad mode in Synchronous Abort handler detected, code 0x8400000f"
> > seen when modprobing modules under Linux.
> >
> > Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
> > Cc: Peter Maydell <peter.maydell@linaro.org>
> > Cc: Claudio Fontana <claudio.fontana@huawei.com>
> > Cc: Rob Herring <robherring2@gmail.com>
> 
> Thanks, applied to target-arm.next.
> To those interested in EL2/EL3 support: what's the
> plan for telling the get_phys_addr() functions which
> translation regime they should be operating in?

My ARMv8 S2 MMU code is a bit of a mess at the moment but I'm thinking to add
translation_el and stage arguments to get_phys_addr(). We can work out the
details later but basically we need get_phys_addr() to be able to
do translation for the various ELs, Stage 1 and/or Stage 2 and to be able
to do the translations independent from the current CPU state. We need
the latter to support the various address translation regs (e.g ATS12E0R
from El3).

Cheers,
Edgar


> 
> (since that is what indicates whether this bit is UXN or XN,
> as well as having various other effects).
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH] target-arm: A64: Correct handling of UXN bit.

[Claudio Fontana]

This patch fixes my issue with page tables switching on OSv guest.

Thank you all!

Tested-by: Claudio Fontana <claudio.fontana@huawei.com>

On 08.06.2014 15:53, Ian Campbell wrote:
> In v8 page tables bit 54 in the PTE is UXN in the EL0/EL1 translation regimes
> and XN elsewhere. In v7 the bit is always XN. Since we only emulate EL0/EL1 we
> can just treat this bit as UXN whenever we are in v8 mode.
> 
> Also correctly extract the upper attributes from the PTE entry, the v8 version
> tried to avoid extracting the CONTIG bit and ended up with the upper bits being
> off-by-one. Instead behave the same as v7 and extract (but ignore) the CONTIG
> bit.
> 
> This fixes "Bad mode in Synchronous Abort handler detected, code 0x8400000f"
> seen when modprobing modules under Linux.
> 
> Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Claudio Fontana <claudio.fontana@huawei.com>
> Cc: Rob Herring <robherring2@gmail.com>
> ---
>  target-arm/helper.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index ec031f5..5872a00 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3929,13 +3929,8 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>          page_size = (1 << ((granule_sz * (4 - level)) + 3));
>          descaddr |= (address & (page_size - 1));
>          /* Extract attributes from the descriptor and merge with table attrs */
> -        if (arm_feature(env, ARM_FEATURE_V8)) {
> -            attrs = extract64(descriptor, 2, 10)
> -                | (extract64(descriptor, 53, 11) << 10);
> -        } else {
> -            attrs = extract64(descriptor, 2, 10)
> -                | (extract64(descriptor, 52, 12) << 10);
> -        }
> +        attrs = extract64(descriptor, 2, 10)
> +            | (extract64(descriptor, 52, 12) << 10);
>          attrs |= extract32(tableattrs, 0, 2) << 11; /* XN, PXN */
>          attrs |= extract32(tableattrs, 3, 1) << 5; /* APTable[1] => AP[2] */
>          /* The sense of AP[1] vs APTable[0] is reversed, as APTable[0] == 1
> @@ -3961,8 +3956,12 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>          goto do_fault;
>      }
>      *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> -    if (attrs & (1 << 12) || (!is_user && (attrs & (1 << 11)))) {
> -        /* XN or PXN */
> +    if ((arm_feature(env, ARM_FEATURE_V8) && is_user && (attrs & (1 << 12))) ||
> +        (!arm_feature(env, ARM_FEATURE_V8) && (attrs & (1 << 12))) ||
> +        (!is_user && (attrs & (1 << 11)))) {
> +        /* XN/UXN or PXN. Since we only implement EL0/EL1 we unconditionally
> +         * treat XN/UXN as UXN for v8.
> +         */
>          if (access_type == 2) {
>              goto do_fault;
>          }
> 


-- 
Claudio Fontana
Server Virtualization Architect
Huawei Technologies Duesseldorf GmbH
Riesstraße 25 - 80992 München

office: +49 89 158834 4135
mobile: +49 15253060158

Re: [Qemu-devel] AArch64 QEMU System emulation: issue with TTBR0

[Rob Herring]

On Mon, Jun 2, 2014 at 11:16 AM, Claudio Fontana <hw.claudio@gmail.com> wrote:
> Hello Peter,
>
> I am porting OSv to AArch64, and I have some working code running on
> the Foundation Models,
> where I run qemu natively with --enable-kvm,
>
> which does not seem to work when run instead on top of the system emulation.
>
> In particular I get a sync exception when I try to msr to TTBR0_EL1.

How are you configuring TCR register?

Rob

>
> The ESR as read in env->cp15.esr_el[1] is 0x8400000e, which looking up
> in the ESR table means
>
> Instruction fault, with IFSC (instruction fault status code) = 0xe,
> which should match
>
> 0b0011LL = permission fault (LL indicates level at which fault occurred).
>
> with LL = 0b10 meaning EL2.
>
> The code is in particular:
>
> 00000000401db2d0 <mmu::switch_to_runtime_page_tables()>:
>     401db2d0:   d00037a0        adrp    x0, 408d1000 <unique_mtx+0x10>
>     401db2d4:   9130e000        add     x0, x0, #0xc38
>     401db2d8:   f9400000        ldr     x0, [x0]
>     401db2dc:   92748c00        and     x0, x0, #0xfffffffff000
>     401db2e0:   d5182000        msr     ttbr0_el1, x0
>     401db2e4:   d5033fdf        isb
>     401db2e8:   d00037a0        adrp    x0, 408d1000 <unique_mtx+0x10>
>     401db2ec:   9130e000        add     x0, x0, #0xc38
>     401db2f0:   f9400400        ldr     x0, [x0,#8]
>     401db2f4:   92748c00        and     x0, x0, #0xfffffffff000
>     401db2f8:   d5182020        msr     ttbr1_el1, x0
>     401db2fc:   d5033fdf        isb
>     401db300:   d5033f9f        dsb     sy
>     401db304:   d508831f        tlbi    vmalle1is
>     401db308:   d5033f9f        dsb     sy
>     401db30c:   d5033fdf        isb
>     401db310:   d65f03c0        ret
>
> ELR_EL1 in env->elr_el[1] reads as 0x401da200, which is strangely
> enough the address of the first instruction of the exception vector
> entry for sync:
>
>         ...
>     401da200:   14000169        b       401da7a4 <entry_sync>
>     401da204:   d503201f        nop
>         ...
>     401da280:   14000174        b       401da850 <entry_irq>
>     401da284:   d503201f        nop
>
> The source is available at:
>
> https://github.com/cloudius-systems/osv/blob/master/arch/aarch64/mmu.cc
>
> Thanks for any advice,
>
> Claudio
>

Re: [Qemu-devel] AArch64 QEMU System emulation: issue with TTBR0

[Claudio Fontana]

On 03.06.2014 11:34, Rob Herring wrote:
> On Mon, Jun 2, 2014 at 11:16 AM, Claudio Fontana <hw.claudio@gmail.com> wrote:
>> Hello Peter,
>>
>> I am porting OSv to AArch64, and I have some working code running on
>> the Foundation Models,
>> where I run qemu natively with --enable-kvm,
>>
>> which does not seem to work when run instead on top of the system emulation.
>>
>> In particular I get a sync exception when I try to msr to TTBR0_EL1.
> 
> How are you configuring TCR register?

https://github.com/cloudius-systems/osv/blob/master/arch/aarch64/boot.S

contains more context, the register itself is initialized as such:
        ldr x0, =0x15b5103510
        msr tcr_el1, x0
        isb

all bits used are explained in a comment in the boot.S above.

Thanks,

Claudio
> 
> Rob
> 
>>
>> The ESR as read in env->cp15.esr_el[1] is 0x8400000e, which looking up
>> in the ESR table means
>>
>> Instruction fault, with IFSC (instruction fault status code) = 0xe,
>> which should match
>>
>> 0b0011LL = permission fault (LL indicates level at which fault occurred).
>>
>> with LL = 0b10 meaning EL2.
>>
>> The code is in particular:
>>
>> 00000000401db2d0 <mmu::switch_to_runtime_page_tables()>:
>>     401db2d0:   d00037a0        adrp    x0, 408d1000 <unique_mtx+0x10>
>>     401db2d4:   9130e000        add     x0, x0, #0xc38
>>     401db2d8:   f9400000        ldr     x0, [x0]
>>     401db2dc:   92748c00        and     x0, x0, #0xfffffffff000
>>     401db2e0:   d5182000        msr     ttbr0_el1, x0
>>     401db2e4:   d5033fdf        isb
>>     401db2e8:   d00037a0        adrp    x0, 408d1000 <unique_mtx+0x10>
>>     401db2ec:   9130e000        add     x0, x0, #0xc38
>>     401db2f0:   f9400400        ldr     x0, [x0,#8]
>>     401db2f4:   92748c00        and     x0, x0, #0xfffffffff000
>>     401db2f8:   d5182020        msr     ttbr1_el1, x0
>>     401db2fc:   d5033fdf        isb
>>     401db300:   d5033f9f        dsb     sy
>>     401db304:   d508831f        tlbi    vmalle1is
>>     401db308:   d5033f9f        dsb     sy
>>     401db30c:   d5033fdf        isb
>>     401db310:   d65f03c0        ret
>>
>> ELR_EL1 in env->elr_el[1] reads as 0x401da200, which is strangely
>> enough the address of the first instruction of the exception vector
>> entry for sync:
>>
>>         ...
>>     401da200:   14000169        b       401da7a4 <entry_sync>
>>     401da204:   d503201f        nop
>>         ...
>>     401da280:   14000174        b       401da850 <entry_irq>
>>     401da284:   d503201f        nop
>>
>> The source is available at:
>>
>> https://github.com/cloudius-systems/osv/blob/master/arch/aarch64/mmu.cc
>>
>> Thanks for any advice,
>>
>> Claudio
>>
>