|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH for-4.12 v3] xen/arm: Stop relocating Xen
On Tue, 18 Dec 2018, Julien Grall wrote:
> At the moment, Xen is relocated towards the end of the memory. While
> this has the advantage to free space in low memory, the code is not
> compliant with the break-before-make because it requires to switch
> between two sets of page-table. This is not entirely trivial to fix as
> it would require us to go through an identity mapping and disabling MMU.
>
> Furthermore, it looks like that some platform (such as the Hikey960)
> may not be able to bring-up secondary CPUs if the entry is too high.
>
> While Xen should be quite tiny (< 2MB), the current algorigthm to
> allocate Dom0 memory will allocate memory chunk of at least 128MB.
> Those memory chunks will always be 128MB. This means that depending on
> where the modules are loaded, an extra 128MB may disappear.
>
> As there are up to 4 modules (initramfs, XSM, kernel, DTB) loaded in
> low memory. The problem is not entirely new as you could already waste
> 512MB of low-memory. The right solution would be to fix the allocation
> algorightm. But this is independent from this patch.
>
> For user in control of the memory (such as in U-boot), all modules
> should be loaded as much as possible together or outside low-memory (i.e
> above 4GB). For other users (i.e Grub/UEFI), I believe the bootloader is
> already keeping everything together.
>
> Based on the above, it would be fine to stop relocating Xen. This has
> the advantage to simplify the code and should speed-up the boot as
> relocation is not necessary anymore.
>
> Note that the break-before-make issue is not fixed by this patch.
>
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> Reported-by: Matthew Daley <mattd@xxxxxxxxxxx>
> Tested-by: Matthew Daley <mattd@xxxxxxxxxxx>
Reviewed and committed
> ---
> Changes in v3:
> - Update the commit message
>
> Changes in v2:
> - Add Matthew's tested-by
> ---
> xen/arch/arm/arm32/head.S | 54 +++------------------------------------
> xen/arch/arm/arm64/head.S | 50 +++++-------------------------------
> xen/arch/arm/mm.c | 18 +++----------
> xen/arch/arm/setup.c | 65
> +++--------------------------------------------
> xen/include/asm-arm/mm.h | 2 +-
> 5 files changed, 17 insertions(+), 172 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 93b51e9ef2..390a505e05 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -469,58 +469,12 @@ fail: PRINT("- Boot failed -\r\n")
> GLOBAL(_end_boot)
>
> /*
> - * Copy Xen to new location and switch TTBR
> + * Switch TTBR
> * r1:r0 ttbr
> - * r2 source address
> - * r3 destination address
> - * [sp]=>r4 length
> *
> - * Source and destination must be word aligned, length is rounded up
> - * to a 16 byte boundary.
> - *
> - * MUST BE VERY CAREFUL when saving things to RAM over the copy
> + * TODO: This code does not comply with break-before-make.
> */
> -ENTRY(relocate_xen)
> - push {r4,r5,r6,r7,r8,r9,r10,r11}
> -
> - ldr r4, [sp, #8*4] /* Get 4th argument from stack */
> -
> - /* Copy 16 bytes at a time using:
> - * r5: counter
> - * r6: data
> - * r7: data
> - * r8: data
> - * r9: data
> - * r10: source
> - * r11: destination
> - */
> - mov r5, r4
> - mov r10, r2
> - mov r11, r3
> -1: ldmia r10!, {r6, r7, r8, r9}
> - stmia r11!, {r6, r7, r8, r9}
> -
> - subs r5, r5, #16
> - bgt 1b
> -
> - /* Flush destination from dcache using:
> - * r5: counter
> - * r6: step
> - * r7: vaddr
> - */
> - dsb /* So the CPU issues all writes to the range */
> -
> - mov r5, r4
> - ldr r6, =dcache_line_bytes /* r6 := step */
> - ldr r6, [r6]
> - mov r7, r3
> -
> -1: mcr CP32(r7, DCCMVAC)
> -
> - add r7, r7, r6
> - subs r5, r5, r6
> - bgt 1b
> -
> +ENTRY(switch_ttbr)
> dsb /* Ensure the flushes happen before
> * continuing */
> isb /* Ensure synchronization with
> previous
> @@ -543,8 +497,6 @@ ENTRY(relocate_xen)
> dsb /* Ensure completion of TLB+BP flush
> */
> isb
>
> - pop {r4, r5,r6,r7,r8,r9,r10,r11}
> -
> mov pc, lr
>
> #ifdef CONFIG_EARLY_PRINTK
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index ef87b5c254..0b7f6e7f92 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -609,52 +609,14 @@ fail: PRINT("- Boot failed -\r\n")
>
> GLOBAL(_end_boot)
>
> -/* Copy Xen to new location and switch TTBR
> - * x0 ttbr
> - * x1 source address
> - * x2 destination address
> - * x3 length
> +/*
> + * Switch TTBR
> *
> - * Source and destination must be word aligned, length is rounded up
> - * to a 16 byte boundary.
> + * x0 ttbr
> *
> - * MUST BE VERY CAREFUL when saving things to RAM over the copy */
> -ENTRY(relocate_xen)
> - /* Copy 16 bytes at a time using:
> - * x9: counter
> - * x10: data
> - * x11: data
> - * x12: source
> - * x13: destination
> - */
> - mov x9, x3
> - mov x12, x1
> - mov x13, x2
> -
> -1: ldp x10, x11, [x12], #16
> - stp x10, x11, [x13], #16
> -
> - subs x9, x9, #16
> - bgt 1b
> -
> - /* Flush destination from dcache using:
> - * x9: counter
> - * x10: step
> - * x11: vaddr
> - */
> - dsb sy /* So the CPU issues all writes to the range */
> -
> - mov x9, x3
> - ldr x10, =dcache_line_bytes /* x10 := step */
> - ldr x10, [x10]
> - mov x11, x2
> -
> -1: dc cvac, x11
> -
> - add x11, x11, x10
> - subs x9, x9, x10
> - bgt 1b
> -
> + * TODO: This code does not comply with break-before-make.
> + */
> +ENTRY(switch_ttbr)
> dsb sy /* Ensure the flushes happen before
> * continuing */
> isb /* Ensure synchronization with previous
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 91f3aef93c..d96a6655ee 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -601,7 +601,7 @@ void __init remove_early_mappings(void)
> flush_xen_data_tlb_range_va(BOOT_FDT_VIRT_START, BOOT_FDT_SLOT_SIZE);
> }
>
> -extern void relocate_xen(uint64_t ttbr, void *src, void *dst, size_t len);
> +extern void switch_ttbr(uint64_t ttbr);
>
> /* Clear a translation table and clean & invalidate the cache */
> static void clear_table(void *table)
> @@ -612,15 +612,13 @@ static void clear_table(void *table)
>
> /* Boot-time pagetable setup.
> * Changes here may need matching changes in head.S */
> -void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t
> xen_paddr)
> +void __init setup_pagetables(unsigned long boot_phys_offset)
> {
> uint64_t ttbr;
> - unsigned long dest_va;
> lpae_t pte, *p;
> int i;
>
> - /* Calculate virt-to-phys offset for the new location */
> - phys_offset = xen_paddr - (unsigned long) _start;
> + phys_offset = boot_phys_offset;
>
> #ifdef CONFIG_ARM_64
> p = (void *) xen_pgtable;
> @@ -686,21 +684,13 @@ void __init setup_pagetables(unsigned long
> boot_phys_offset, paddr_t xen_paddr)
> pte = boot_second[second_table_offset(BOOT_FDT_VIRT_START + SZ_2M)];
> xen_second[second_table_offset(BOOT_FDT_VIRT_START + SZ_2M)] = pte;
>
> - /* ... Boot Misc area for xen relocation */
> - dest_va = BOOT_RELOC_VIRT_START;
> - pte = mfn_to_xen_entry(maddr_to_mfn(xen_paddr), MT_NORMAL);
> - /* Map the destination in xen_second. */
> - xen_second[second_table_offset(dest_va)] = pte;
> - /* Map the destination in boot_second. */
> - write_pte(boot_second + second_table_offset(dest_va), pte);
> - flush_xen_data_tlb_range_va_local(dest_va, SECOND_SIZE);
> #ifdef CONFIG_ARM_64
> ttbr = (uintptr_t) xen_pgtable + phys_offset;
> #else
> ttbr = (uintptr_t) cpu0_pgtable + phys_offset;
> #endif
>
> - relocate_xen(ttbr, _start, (void*)dest_va, _end - _start);
> + switch_ttbr(ttbr);
>
> /* Clear the copy of the boot pagetables. Each secondary CPU
> * rebuilds these itself (see head.S) */
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index e83221ab79..fb923cdf67 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -374,6 +374,7 @@ void __init discard_initial_modules(void)
> remove_early_mappings();
> }
>
> +#ifdef CONFIG_ARM_32
> /*
> * Returns the end address of the highest region in the range s..e
> * with required size and alignment that does not conflict with the
> @@ -440,6 +441,7 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t
> e,
> }
> return e;
> }
> +#endif
>
> /*
> * Return the end of the non-module region starting at s. In other
> @@ -475,59 +477,6 @@ static paddr_t __init next_module(paddr_t s, paddr_t
> *end)
> return lowest;
> }
>
> -
> -/**
> - * get_xen_paddr - get physical address to relocate Xen to
> - *
> - * Xen is relocated to as near to the top of RAM as possible and
> - * aligned to a XEN_PADDR_ALIGN boundary.
> - */
> -static paddr_t __init get_xen_paddr(void)
> -{
> - struct meminfo *mi = &bootinfo.mem;
> - paddr_t min_size;
> - paddr_t paddr = 0;
> - int i;
> -
> - min_size = (_end - _start + (XEN_PADDR_ALIGN-1)) & ~(XEN_PADDR_ALIGN-1);
> -
> - /* Find the highest bank with enough space. */
> - for ( i = 0; i < mi->nr_banks; i++ )
> - {
> - const struct membank *bank = &mi->bank[i];
> - paddr_t s, e;
> -
> - if ( bank->size >= min_size )
> - {
> - e = consider_modules(bank->start, bank->start + bank->size,
> - min_size, XEN_PADDR_ALIGN, 0);
> - if ( !e )
> - continue;
> -
> -#ifdef CONFIG_ARM_32
> - /* Xen must be under 4GB */
> - if ( e > 0x100000000ULL )
> - e = 0x100000000ULL;
> - if ( e < bank->start )
> - continue;
> -#endif
> -
> - s = e - min_size;
> -
> - if ( s > paddr )
> - paddr = s;
> - }
> - }
> -
> - if ( !paddr )
> - panic("Not enough memory to relocate Xen\n");
> -
> - printk("Placing Xen at 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
> - paddr, paddr + min_size);
> -
> - return paddr;
> -}
> -
> static void __init init_pdx(void)
> {
> paddr_t bank_start, bank_size, bank_end;
> @@ -783,7 +732,6 @@ void __init start_xen(unsigned long boot_phys_offset,
> {
> size_t fdt_size;
> int cpus, i;
> - paddr_t xen_paddr;
> const char *cmdline;
> struct bootmodule *xen_bootmodule;
> struct domain *dom0;
> @@ -827,14 +775,7 @@ void __init start_xen(unsigned long boot_phys_offset,
> (paddr_t)(uintptr_t)(_end - _start + 1), false);
> BUG_ON(!xen_bootmodule);
>
> - xen_paddr = get_xen_paddr();
> - setup_pagetables(boot_phys_offset, xen_paddr);
> -
> - /* Update Xen's address now that we have relocated. */
> - printk("Update BOOTMOD_XEN from %"PRIpaddr"-%"PRIpaddr" =>
> %"PRIpaddr"-%"PRIpaddr"\n",
> - xen_bootmodule->start, xen_bootmodule->start +
> xen_bootmodule->size,
> - xen_paddr, xen_paddr + xen_bootmodule->size);
> - xen_bootmodule->start = xen_paddr;
> + setup_pagetables(boot_phys_offset);
>
> setup_mm(fdt_paddr, fdt_size);
>
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index b2f6104a7f..eafa26f56e 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -169,7 +169,7 @@ extern unsigned long total_pages;
> #define PDX_GROUP_SHIFT SECOND_SHIFT
>
> /* Boot-time pagetable setup */
> -extern void setup_pagetables(unsigned long boot_phys_offset, paddr_t
> xen_paddr);
> +extern void setup_pagetables(unsigned long boot_phys_offset);
> /* Map FDT in boot pagetable */
> extern void *early_fdt_map(paddr_t fdt_paddr);
> /* Remove early mappings */
> --
> 2.11.0
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |