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

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

Hi Ian,

On 14/07/14 17:39, Ian Campbell wrote:
> ---
>   xen/arch/arm/arm32/head.S |   54 +++++++++++++++++++++++++++++++++------------
>   xen/arch/arm/arm64/head.S |   50 +++++++++++++++++++++++++++++------------
>   xen/arch/arm/mm.c         |    8 +++++--
>   3 files changed, 82 insertions(+), 30 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 1319a13..3a72195 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -26,6 +26,7 @@
>
>   #define PT_PT     0xf7f /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=1 P=1 */
>   #define PT_MEM    0xf7d /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=0 P=1 */
> +#define PT_MEM_L3 0xf7f /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=1 P=1 */
>   #define PT_DEV    0xe71 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=0 P=1 */
>   #define PT_DEV_L3 0xe73 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=1 P=1 */
>
> @@ -279,25 +280,50 @@ cpu_init_done:
>           ldr   r4, =boot_second
>           add   r4, r4, r10            /* r4 := paddr (boot_second) */
>
> -        lsr   r2, r9, #SECOND_SHIFT  /* Base address for 2MB mapping */
> -        lsl   r2, r2, #SECOND_SHIFT
> +        ldr   r1, =boot_third
> +        add   r1, r1, r10            /* r1 := paddr (boot_third) */
> +        mov   r3, #0x0
> +
> +        /* ... map boot_third in boot_second[1] */
> +        orr   r2, r1, #PT_UPPER(PT)  /* r2:r3 := table map of boot_third */
> +        orr   r2, r2, #PT_LOWER(PT)  /* (+ rights for linear PT) */
> +        strd  r2, r3, [r4, #8]       /* Map it in slot 1 */
> +
> +        /* ... 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 */
> +
> +        /* 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?

Also what does prevent Xen to be shared between 2 third page table?

> +
> +2:
> +        lsl   r2, r1, #SECOND_SHIFT  /* Base address for 2MB mapping */
>           orr   r2, r2, #PT_UPPER(MEM) /* r2:r3 := section map */
>           orr   r2, r2, #PT_LOWER(MEM)
> +        lsl   r1, r1, #3             /* r1 := Slot offset */
> +        strd  r2, r3, [r4, r1]       /* Mapping of paddr(start) */
>
> -        /* ... map of vaddr(start) in boot_second */
> -        ldr   r1, =start
> -        lsr   r1, #(SECOND_SHIFT - 3)   /* Slot for vaddr(start) */
> -        strd  r2, r3, [r4, r1]       /* Map vaddr(start) */
> +        /* Setup boot_third: */
> +1:      ldr   r4, =boot_third
> +        add   r4, r4, r10            /* r4 := paddr (boot_third) */
>
> -        /* ... map of paddr(start) in boot_second */
> -        lsrs  r1, r9, #30            /* Base paddr */
> -        bne   1f                     /* If paddr(start) is not in slot 0
> -                                      * then the mapping was done in
> -                                      * boot_pgtable above */
> +        lsr   r2, r9, #THIRD_SHIFT  /* Base address for 4K mapping */
> +        lsl   r2, r2, #THIRD_SHIFT
> +        orr   r2, r2, #PT_UPPER(MEM_L3) /* r2:r3 := map */
> +        orr   r2, r2, #PT_LOWER(MEM_L3)
>
> -        mov   r1, r9, lsr #(SECOND_SHIFT - 3)   /* Slot for paddr(start) */
> -        strd  r2, r3, [r4, r1]       /* Map Xen there */
> -1:
> +        /* ... map of vaddr(start) in boot_third */
> +        mov   r1, #0
> +1:      strd  r2, r3, [r4, r1]       /* Map vaddr(start) */
> +        add   r2, r2, #4096          /* Next page */

I would use PAGE_SIZE or THIRD_SIZE here.

> +        add   r1, r1, #8             /* Next slot */
> +        cmp   r1, #(512*8)

Any reason to not use LPAE_ENTRIES here?


Regards,

-- 
Julien Grall