On Fri, Jul 22, 2016 at 12:05:57AM +0200, Mark Kettenis wrote:
> Currently armv7 kernels expect to be loaded at the bottom of physical
> ram. The diff below removes this restriction.
>
> This is important for two reasons:
>
> 1. On some SoCs physical memory starts at address 0. However, the
> u-boot EFI interface has a nasty bug where allocating memory at
> address 0 always succeeds. Even if there is no memory at that
> address! Therefore BOOTARM.EFI starts looking for available memory
> to load the kernel at address 0x10000000.
>
> 2. There is not guarantee that we can always load the kernel at the
> bottom of physical memory as that memory might be in use by the
> firmware.
>
> The diff passes the physical address at which the kernel has been
> loaded in arg0, which is currently unused. The assembly code that
> forms the entry point of the kernel already calculated this address so
> passing it is trivial. If we weren't loaded at the bottom of physical
> memory, we will make the memory pages before the kernel available to
> uvm such that we don't lose any memory.
>
> ok (after unlock)?
>
>
> Index: arch/armv7/armv7/armv7_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/armv7/armv7/armv7_machdep.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 armv7_machdep.c
> --- arch/armv7/armv7/armv7_machdep.c 14 Jun 2016 10:03:51 -0000 1.31
> +++ arch/armv7/armv7/armv7_machdep.c 21 Jul 2016 20:58:08 -0000
> @@ -389,6 +389,7 @@ initarm(void *arg0, void *arg1, void *ar
> u_int l1pagetable;
> pv_addr_t kernel_l1pt;
> pv_addr_t fdt;
> + paddr_t loadaddr;
> paddr_t memstart;
> psize_t memsize;
> void *config;
> @@ -399,6 +400,7 @@ initarm(void *arg0, void *arg1, void *ar
> int (*map_func_save)(void *, bus_addr_t, bus_size_t, int,
> bus_space_handle_t *);
>
> + loadaddr = (paddr_t)arg0;
> board_id = (uint32_t)arg1;
> /*
> * u-boot has decided the top four bits are
> @@ -514,7 +516,7 @@ initarm(void *arg0, void *arg1, void *ar
> boothowto |= RB_DFLTROOT;
> #endif /* RAMDISK_HOOKS */
>
> - physical_freestart = (((unsigned long)esym - KERNEL_TEXT_BASE +0xfff) &
> ~0xfff) + memstart;
> + physical_freestart = (((unsigned long)esym - KERNEL_TEXT_BASE +0xfff) &
> ~0xfff) + loadaddr;
> physical_freeend = MIN((uint64_t)memstart+memsize, (paddr_t)-PAGE_SIZE);
>
> physmem = (physical_end - physical_start) / PAGE_SIZE;
> @@ -557,7 +559,7 @@ initarm(void *arg0, void *arg1, void *ar
> /* Define a macro to simplify memory allocation */
> #define valloc_pages(var, np) \
> alloc_pages((var).pv_pa, (np)); \
> - (var).pv_va = KERNEL_BASE + (var).pv_pa - physical_start;
> + (var).pv_va = KERNEL_BASE + (var).pv_pa - loadaddr;
>
> #define alloc_pages(var, np) \
> (var) = physical_freestart; \
> @@ -677,10 +679,10 @@ initarm(void *arg0, void *arg1, void *ar
> logical = 0x00000000; /* offset of kernel in RAM */
>
> logical += pmap_map_chunk(l1pagetable, KERNEL_BASE + logical,
> - physical_start + logical, textsize,
> + loadaddr + logical, textsize,
> PROT_READ | PROT_WRITE | PROT_EXEC, PTE_CACHE);
> logical += pmap_map_chunk(l1pagetable, KERNEL_BASE + logical,
> - physical_start + logical, totalsize - textsize,
> + loadaddr + logical, totalsize - textsize,
> PROT_READ | PROT_WRITE, PTE_CACHE);
> }
>
> @@ -798,6 +800,12 @@ initarm(void *arg0, void *arg1, void *ar
> atop(physical_freestart), atop(physical_freeend), 0);
>
> physsegs = MIN(bootconfig.dramblocks, VM_PHYSSEG_MAX);
> +
> + if (physical_start < loadaddr) {
> + uvm_page_physload(atop(physical_start), atop(loadaddr),
> + atop(physical_start), atop(loadaddr), 0);
> + physsegs--;
I don't completely grasp what "physsegs--" does in the case of not
having a device tree. Did you test this on non-fdt machines?
On FDT machines bootconfig should not be initialized, dramblocks thus
zero and physsegs, too? Wouldn't this then underflow? As it's a signed
int, I guess it shouldn't do any harm.
If it were for me we could get rid of the bootconfig stuff, assuming
we want to be device-tree-only.
Otherwise I like this diff. I use it in my tree to boot the Marvell
(RAM starts at 0x0). Thanks, Mark.
Patrick
> + }
>
> for (i = 1; i < physsegs; i++) {
> paddr_t dramstart = bootconfig.dram[i].address;
> Index: arch/armv7/armv7/armv7_start.S
> ===================================================================
> RCS file: /cvs/src/sys/arch/armv7/armv7/armv7_start.S,v
> retrieving revision 1.6
> diff -u -p -r1.6 armv7_start.S
> --- arch/armv7/armv7/armv7_start.S 25 Apr 2016 04:46:57 -0000 1.6
> +++ arch/armv7/armv7/armv7_start.S 21 Jul 2016 20:58:08 -0000
> @@ -161,7 +161,7 @@ _C_LABEL(bootstrap_start):
> CPWAIT(r0)
>
> /* Restore U-Boot arguments */
> - mov r0, r6
> + mov r0, r9
> mov r1, r7
> mov r2, r8
>
>