Re: [PATCH 2/5] xen: arm: Handle 4K aligned hypervisor load address.

[Julien Grall <julien.grall@linaro.org>]

On 16/07/14 17:53, Ian Campbell wrote:
> On Wed, 2014-07-16 at 16:41 +0100, Julien Grall wrote:
>>> +        /* Identity map clashes with boot_third, which we cannot handle yet */
>>> +        PRINT("Unable to build boot page tables - virt and phys addresses clash.\r\n")
>>> +        b     fail
>>
>> AFAIU, this can happen if the kernel is loaded around 2MB in the memory,
>> right?
>
> Yes from 2MB up to (but not including) 4MB.
>
> It is an error (I think) that this patch bugs if Xen is loaded at
> exactly 2MB, since then the virtual and identity-physical mappings are
> the same.
>
>> Also what does prevent Xen to be shared between 2 third page table?
>
> This is the virtual mapping, which always starts at exactly 2MB, so that
> can only happen if Xen is larger than 2MB, which we assume is not the
> case both here and in various bits of the C code start of day
> relocating/setup etc.

Sorry I was thinking that boot_third is used for the 1:1 mapping.

It looks like you are using a 2MB mapping for the identity mapping:

+        /* ... map of paddr(start) in boot_second */
+        lsrs  r1, r9, #SECOND_SHIFT  /* Offset of base paddr in 
boot_second */
+        mov   r2, #0x0ff             /* r2 := LPAE entries mask */
+        orr   r2, r2, #0x100
+        and   r1, r1, r2
+        cmp   r1, #1
+        bne   2f                     /* It's not in slot 1, map it */

r9 contains the physical address of start, but the binary could cross 
the 2MB boundary (because, for instance, the start address is at 
0xXX2FXXXXX). So the assembly code to enable the pagination may not be 
on the same slot.

I think this very unlikely, but if it happens it will be hard to debug.
Maybe you can add a sanity check or add a label before the pagination is 
enabled and use it in the slot.

BTW I think you can use lsr instead of lsrs to get the offset.

Regards,

-- 
Julien Grall