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
>  
> 

Reply via email to