|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/2] xen/arm: Stop relocating Xen
On Thu, 29 Nov 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.
>
> I don't believe the low memory is an issue because Xen is quite tiny
> (< 2MB). So the best solution is 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>
> ---
> xen/arch/arm/arm32/head.S | 54 +++------------------------------------
> xen/arch/arm/arm64/head.S | 50 +++++-------------------------------
> xen/arch/arm/mm.c | 20 ++++-----------
> xen/arch/arm/setup.c | 65
> +++--------------------------------------------
> xen/include/asm-arm/mm.h | 2 +-
> 5 files changed, 18 insertions(+), 173 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 9428c3f5a2..4589a37874 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -605,52 +605,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 2556e57a99..f6931e007f 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;
> @@ -652,7 +650,7 @@ void __init setup_pagetables(unsigned long
> boot_phys_offset, paddr_t xen_paddr)
> /* Break up the Xen mapping into 4k pages and protect them separately. */
> for ( i = 0; i < LPAE_ENTRIES; i++ )
> {
> - mfn_t mfn = mfn_add(maddr_to_mfn(xen_paddr), i);
> + mfn_t mfn = mfn_add(maddr_to_mfn((vaddr_t)_start + phys_offset), i);
Why (vaddr_t) instead of (paddr_t) or (unsigned long)? I understand that
vaddr_t is sufficient but paddr_t or unsigned long would be more
consistent and appropriate?
> unsigned long va = XEN_VIRT_START + (i << PAGE_SHIFT);
>
> if ( !is_kernel(va) )
> @@ -687,21 +685,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;
let's take this opportunity to get rid of BOOT_RELOC_VIRT_START
completely
These are two minor issues -- I tested this patch on the ZynqMP and it
works fine.
> - 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 aec53f30d3..e84172fbc3 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 |