From: Julien Grall <julien.grall@linaro.org>
To: Ian Campbell <ian.campbell@citrix.com>, xen-devel@lists.xen.org
Cc: tim@xen.org, stefano.stabellini@eu.citrix.com
Subject: Re: [PATCH v3 2/6] xen: arm: Handle 4K aligned hypervisor load address.
Date: Mon, 21 Jul 2014 15:16:44 +0100 [thread overview]
Message-ID: <53CD20CC.80601@linaro.org> (raw)
In-Reply-To: <1405947600-26047-2-git-send-email-ian.campbell@citrix.com>
Hi Ian,
On 07/21/2014 01:59 PM, Ian Campbell wrote:
> Currently the boot page tables map Xen at XEN_VIRT_START using a 2MB section
> mapping. This means that the bootloader must load Xen at a 2MB aligned address.
> Unfortunately this is not the case with UEFI on the Juno platform where Xen
> fails to boot. Furthermore the Linux boot protocol (which Xen claims to adhere
> to) does not have this restriction, therefore this is our bug and not the
> bootloader's.
>
> Fix this by adding third level pagetables to the boot time pagetables, allowing
> us to map a Xen which is aligned only to a 4K boundary. This only affects the
> boot time page tables since Xen will later relocate itself to a 2MB aligned
> address. Strictly speaking the non-boot processors could make use of this and
> use a section mapping, but it is simpler if all processors follow the same boot
> path.
>
> Strictly speaking the Linux boot protocol doesn't even require 4K alignment
> (and apparently Linux can cope with this), but so far all bootloaders appear to
> provide it, so support for this is left for another day.
>
> In order to use LPAE_ENTRIES in head.S we need to define it in an asm friendly
> way.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
Regards,
> ---
> v3: Use LPAE_ENTRY_MASK.
> Use "ldr rX, =XXX" to avoid opencoding the construction of the
> const.
> Update comment at start to reflect this change
> v2: Use LPAE_ENTRIES and PAGE_SIZE
> Minor updates to the asm
>
> update comment
> ---
> xen/arch/arm/arm32/head.S | 62 ++++++++++++++++++++++++++++++++------------
> xen/arch/arm/arm64/head.S | 62 +++++++++++++++++++++++++++++++-------------
> xen/arch/arm/mm.c | 8 ++++--
> xen/include/asm-arm/page.h | 2 +-
> 4 files changed, 97 insertions(+), 37 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 51501dc..9bc893f 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 */
>
> @@ -73,7 +74,7 @@
>
> /* This must be the very first address in the loaded image.
> * It should be linked at XEN_VIRT_START, and loaded at any
> - * 2MB-aligned address. All of text+data+bss must fit in 2MB,
> + * 4K-aligned address. All of text+data+bss must fit in 2MB,
> * or the initial pagetable code below will need adjustment. */
> .global start
> start:
> @@ -258,11 +259,11 @@ cpu_init_done:
> /* Setup boot_pgtable: */
> ldr r1, =boot_second
> add r1, r1, r10 /* r1 := paddr (boot_second) */
> - mov r3, #0x0
>
> /* ... map boot_second in boot_pgtable[0] */
> orr r2, r1, #PT_UPPER(PT) /* r2:r3 := table map of boot_second */
> orr r2, r2, #PT_LOWER(PT) /* (+ rights for linear PT) */
> + mov r3, #0x0
> strd r2, r3, [r4, #0] /* Map it in slot 0 */
>
> /* ... map of paddr(start) in boot_pgtable */
> @@ -279,31 +280,60 @@ 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) */
> +
> + /* ... 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) */
> + mov r3, #0x0
> + strd r2, r3, [r4, #8] /* Map it in slot 1 */
> +
> + /* ... map of paddr(start) in boot_second */
> + lsr r2, r9, #SECOND_SHIFT /* Offset of base paddr in boot_second */
> + ldr r3, =LPAE_ENTRY_MASK
> + and r1, r2, r3
> + cmp r1, #1
> + beq virtphys_clash /* It's in slot 1, which we cannot handle */
> +
> + lsl r2, r2, #SECOND_SHIFT /* Base address for 2MB mapping */
> orr r2, r2, #PT_UPPER(MEM) /* r2:r3 := section map */
> orr r2, r2, #PT_LOWER(MEM)
> + mov r3, #0x0
> + 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 r3, #0x0
>
> - 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, #PAGE_SIZE /* Next page */
> + add r1, r1, #8 /* Next slot */
> + cmp r1, #(LPAE_ENTRIES<<3) /* 512*8-byte entries per page */
> + blo 1b
>
> /* Defer fixmap and dtb mapping until after paging enabled, to
> * avoid them clashing with the 1:1 mapping. */
>
> /* boot pagetable setup complete */
>
> + b 1f
> +
> +virtphys_clash:
> + /* 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
> +
> +1:
> PRINT("- Turning on paging -\r\n")
>
> ldr r1, =paging /* Explicit vaddr, not RIP-relative */
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index d46481b..7c04e5b 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -27,6 +27,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 */
>
> @@ -95,7 +96,7 @@
> *
> * This must be the very first address in the loaded image.
> * It should be linked at XEN_VIRT_START, and loaded at any
> - * 2MB-aligned address. All of text+data+bss must fit in 2MB,
> + * 4K-aligned address. All of text+data+bss must fit in 2MB,
> * or the initial pagetable code below will need adjustment.
> */
>
> @@ -274,8 +275,9 @@ skip_bss:
> lsl x2, x1, #ZEROETH_SHIFT /* Base address for 512GB mapping */
> mov x3, #PT_MEM /* x2 := Section mapping */
> orr x2, x2, x3
> - lsl x1, x1, #3 /* x1 := Slot offset */
> - str x2, [x4, x1] /* Mapping of paddr(start)*/
> + and x1, x1, #LPAE_ENTRY_MASK /* x1 := Slot offset */
> + lsl x1, x1, #3
> + str x2, [x4, x1] /* Mapping of paddr(start) */
>
> 1: /* Setup boot_first: */
> ldr x4, =boot_first /* Next level into boot_first */
> @@ -290,7 +292,7 @@ skip_bss:
>
> /* ... map of paddr(start) in boot_first */
> lsr x2, x19, #FIRST_SHIFT /* x2 := Offset of base paddr in boot_first */
> - and x1, x2, 0x1ff /* x1 := Slot to use */
> + and x1, x2, #LPAE_ENTRY_MASK /* x1 := Slot to use */
> cbz x1, 1f /* It's in slot 0, map in boot_second */
>
> lsl x2, x2, #FIRST_SHIFT /* Base address for 1GB mapping */
> @@ -303,31 +305,55 @@ skip_bss:
> ldr x4, =boot_second /* Next level into boot_second */
> add x4, x4, x20 /* x4 := paddr(boot_second) */
>
> - lsr x2, x19, #SECOND_SHIFT /* Base address for 2MB mapping */
> - lsl x2, x2, #SECOND_SHIFT
> + /* ... map boot_third in boot_second[1] */
> + ldr x1, =boot_third
> + add x1, x1, x20 /* x1 := paddr(boot_third) */
> + mov x3, #PT_PT /* x2 := table map of boot_third */
> + orr x2, x1, x3 /* + rights for linear PT */
> + str x2, [x4, #8] /* Map it in slot 1 */
> +
> + /* ... map of paddr(start) in boot_second */
> + lsr x2, x19, #SECOND_SHIFT /* x2 := Offset of base paddr in boot_second */
> + and x1, x2, #LPAE_ENTRY_MASK /* x1 := Slot to use */
> + cmp x1, #1
> + b.eq virtphys_clash /* It's in slot 1, which we cannot handle */
> +
> + lsl x2, x2, #SECOND_SHIFT /* Base address for 2MB mapping */
> mov x3, #PT_MEM /* x2 := Section map */
> orr x2, x2, x3
> + lsl x1, x1, #3 /* x1 := Slot offset */
> + str x2, [x4, x1] /* Create mapping of paddr(start)*/
>
> - /* ... map of vaddr(start) in boot_second */
> - ldr x1, =start
> - lsr x1, x1, #(SECOND_SHIFT - 3) /* Slot for vaddr(start) */
> - str x2, [x4, x1] /* Map vaddr(start) */
> +1: /* Setup boot_third: */
> + ldr x4, =boot_third
> + add x4, x4, x20 /* x4 := paddr (boot_third) */
>
> - /* ... map of paddr(start) in boot_second */
> - lsr x1, x19, #FIRST_SHIFT /* Base paddr */
> - cbnz x1, 1f /* If paddr(start) is not in slot 0
> - * then the mapping was done in
> - * boot_pgtable or boot_first above */
> + lsr x2, x19, #THIRD_SHIFT /* Base address for 4K mapping */
> + lsl x2, x2, #THIRD_SHIFT
> + mov x3, #PT_MEM_L3 /* x2 := Section map */
> + orr x2, x2, x3
>
> - lsr x1, x19, #(SECOND_SHIFT - 3) /* Slot for paddr(start) */
> - str x2, [x4, x1] /* Map Xen there */
> -1:
> + /* ... map of vaddr(start) in boot_third */
> + mov x1, xzr
> +1: str x2, [x4, x1] /* Map vaddr(start) */
> + add x2, x2, #PAGE_SIZE /* Next page */
> + add x1, x1, #8 /* Next slot */
> + cmp x1, #(LPAE_ENTRIES<<3) /* 512 entries per page */
> + b.lt 1b
>
> /* Defer fixmap and dtb mapping until after paging enabled, to
> * avoid them clashing with the 1:1 mapping. */
>
> /* boot pagetable setup complete */
>
> + b 1f
> +
> +virtphys_clash:
> + /* 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
> +
> +1:
> PRINT("- Turning on paging -\r\n")
>
> ldr x1, =paging /* Explicit vaddr, not RIP-relative */
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 03a0533..fdc7c98 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -47,8 +47,9 @@ struct domain *dom_xen, *dom_io, *dom_cow;
> * to the CPUs own pagetables.
> *
> * These pagetables have a very simple structure. They include:
> - * - a 2MB mapping of xen at XEN_VIRT_START, boot_first and
> - * boot_second are used to populate the trie down to that mapping.
> + * - 2MB worth of 4K mappings of xen at XEN_VIRT_START, boot_first and
> + * boot_second are used to populate the tables down to boot_third
> + * which contains the actual mapping.
> * - a 1:1 mapping of xen at its current physical address. This uses a
> * section mapping at whichever of boot_{pgtable,first,second}
> * covers that physical address.
> @@ -69,6 +70,7 @@ lpae_t boot_pgtable[LPAE_ENTRIES] __attribute__((__aligned__(4096)));
> lpae_t boot_first[LPAE_ENTRIES] __attribute__((__aligned__(4096)));
> #endif
> lpae_t boot_second[LPAE_ENTRIES] __attribute__((__aligned__(4096)));
> +lpae_t boot_third[LPAE_ENTRIES] __attribute__((__aligned__(4096)));
>
> /* Main runtime page tables */
>
> @@ -492,6 +494,8 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
> #endif
> memset(boot_second, 0x0, PAGE_SIZE);
> clean_and_invalidate_xen_dcache(boot_second);
> + memset(boot_third, 0x0, PAGE_SIZE);
> + clean_and_invalidate_xen_dcache(boot_third);
>
> /* Break up the Xen mapping into 4k pages and protect them separately. */
> for ( i = 0; i < LPAE_ENTRIES; i++ )
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index 113be5a..739038a 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -396,7 +396,7 @@ static inline int gva_to_ipa(vaddr_t va, paddr_t *paddr)
> */
>
> #define LPAE_SHIFT 9
> -#define LPAE_ENTRIES (1u << LPAE_SHIFT)
> +#define LPAE_ENTRIES (_AC(1,U) << LPAE_SHIFT)
> #define LPAE_ENTRY_MASK (LPAE_ENTRIES - 1)
>
> #define THIRD_SHIFT (PAGE_SHIFT)
>
--
Julien Grall
next prev parent reply other threads:[~2014-07-21 14:16 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-21 12:58 [PATCH v3 0/6] xen: arm: various improvements to boot time page table handling Ian Campbell
2014-07-21 12:59 ` [PATCH v3 1/6] xen: arm: correct whitespace/comments and use #defines in head.S Ian Campbell
2014-07-21 12:59 ` [PATCH v3 2/6] xen: arm: Handle 4K aligned hypervisor load address Ian Campbell
2014-07-21 13:20 ` Tim Deegan
2014-07-21 13:29 ` Ian Campbell
2014-07-21 13:38 ` Tim Deegan
2014-07-21 13:46 ` Ian Campbell
2014-07-21 14:16 ` Julien Grall [this message]
2014-07-21 12:59 ` [PATCH v3 3/6] xen: arm: Do not use level 0 section mappings in boot page tables Ian Campbell
2014-07-21 12:59 ` [PATCH v3 4/6] xen: arm: avoid unnecessary additional " Ian Campbell
2014-07-21 12:59 ` [PATCH v3 5/6] xen: arm: ensure that the boot code is <4K in size Ian Campbell
2014-07-21 13:00 ` [PATCH v3 6/6] xen: arm: Correctly use GLOBAL/ENTRY in head.S, avoid .global Ian Campbell
2014-07-21 14:18 ` Julien Grall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53CD20CC.80601@linaro.org \
--to=julien.grall@linaro.org \
--cc=ian.campbell@citrix.com \
--cc=stefano.stabellini@eu.citrix.com \
--cc=tim@xen.org \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).