|
[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, Stefano Stabellini wrote:
> 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
I forgot to add that I fixed a couple of grammar errors in the commit
message as I 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 |