Date: Thu, 11 Jun 2015 06:21:28 -0700 From: Maxim Sobolev <sobomax@FreeBSD.org> To: Ian Lepore <ian@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn: head/sys/boot: common uboot/common uboot/lib Message-ID: <CAH7qZfts5k_4kOvkdsACNQdbqGd1Gu=m7%2BF1MO9FLgeQUOWEsg@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
Hi Ian, there is some issues with that commit that I've run into when
trying to get FreeBSD booting on my Xilinx Zinq 7010-based board.
Basically, the instructions we have on Wiki suggests that the ubldr loading
address to be 0x100000. That makes ubldr panic with "not enough DRAM"
error. I've added some debug code into the for loop, you can find the
output below. As you can see the code is not handling the case when ubldr
is below 2MB and as such sblock == eubldr. On top of that, this_block and
this_size may be left uninitialized causing loading at some random address
and panicing then, instead of DRAM is too small panic.
kernel_addr=0x100000
ubldr_addr=0x100000
dtb_addr=0x1000
dtb_name=system.dtb
uenvcmd=echo Booting FreeBSD from SD...; mmcinfo && fatload mmc 0
${ubldr_addr} ubldr && fatload mmc 0 ${dtb_addr} ${dtb_name} && fdt addr
${dtb_addr} && bootelf ${ubldr_addr}
## Starting application at 0x00080094 ...
Consoles: U-Boot console
Compatible U-Boot API signature found @1f35d338
FreeBSD/armv6 U-Boot loader, Revision 1.2
(root@van01.sippysoft.com, Wed Jun 10 19:19:05 PDT 2015)
DRAM: 512MB
Number of U-Boot devices: 1
U-Boot env: loaderdev not set, will probe all devices.
Found U-Boot device: disk
Probing all disk devices...
Checking unit=0 slice=<auto> partition=<auto>... good.
/
subldr=0, eubldr=2097152
si->mr[0]: .flags=2, .start=0, .size=536870912
sblock=2097152, eblock=536870912
this_block=16, this_size=723100
si->mr[1]: .flags=0, .start=0, .size=0
si->mr[2]: .flags=0, .start=0, .size=0
si->mr[3]: .flags=0, .start=0, .size=0
si->mr[4]: .flags=0, .start=0, .size=0
si->mr[5]: .flags=0, .start=0, .size=0
si->mr[6]: .flags=0, .start=0, .size=0
si->mr[7]: .flags=0, .start=0, .size=0
si->mr[8]: .flags=0, .start=0, .size=0
si->mr[9]: .flags=0, .start=0, .size=0
si->mr[10]: .flags=0, .start=0, .size=0
si->mr[11]: .flags=0, .start=0, .size=0
si->mr[12]: .flags=0, .start=0, .size=0
si->mr[13]: .flags=0, .start=0, .size=0
si->mr[14]: .flags=0, .start=0, .size=0
si->mr[15]: .flags=0, .start=0, .size=0
/boot/kernel/kernel data=0x589e5c+0x3e1a4 data abort
pc : [<00080108>] lr : [<e3510000>]
sp : 1f35c488 ip : 1a000033 fp : 1f35c490
r10: 000cbfd0 r9 : 00000000 r8 : 00000000
r7 : 00030360 r6 : 00000010 r5 : 000af048 r4 : 0000002d
r3 : 00000000 r2 : 1f35c48f r1 : 00000000 r0 : 00000002
Flags: nzCv IRQs off FIQs off Mode SVC_32
Resetting CPU ...
On Sun, May 17, 2015 at 12:59 PM, Ian Lepore <ian@freebsd.org> wrote:
> Author: ian
> Date: Sun May 17 19:59:05 2015
> New Revision: 283035
> URL: https://svnweb.freebsd.org/changeset/base/283035
>
> Log:
> An ARM kernel can be loaded at any 2MB boundary, make ubldr aware of
> that.
>
> Previously, ubldr would use the virtual addresses in the elf headers by
> masking off the high bits and assuming the result was a physical address
> where the kernel should be loaded. That would sometimes discard
> significant bits of the physical address, but the effects of that were
> undone by archsw copy code that would find a large block of memory and
> apply an offset to the source/dest copy addresses. The result was that
> things were loaded at a different physical address than requested by the
> higher code layers, but that worked because other adjustments were
> applied
> later (such as when jumping to the entry point). Very confusing, and
> somewhat fragile.
>
> Now the archsw copy routines are just simple copies, and instead
> archsw.arch_loadaddr is implemented to choose a load address. The new
> routine uses some of the code from the old offset-translation routine to
> find the largest block of ram, but it excludes ubldr itself from that
> range, and also excludes If ubldr splits the largest block of ram in
> two, the kernel is loaded into the bottom of whichever resulting block is
> larger.
>
> As part of eliminating ubldr itself from the ram ranges, export the heap
> start/end addresses in a pair of new global variables.
>
> This change means that the virtual addresses in the arm kernel elf
> headers
> now have no meaning at all, except for the entry point address. There is
> an implicit assumption that the entry point is in the first text page,
> and
> that the address in the the header can be turned into an offset by
> masking
> it with PAGE_MASK. In the future we can link all arm kernels at a
> virtual
> address of 0xC0000000 with no need to use any low-order part of the
> address to influence where in ram the kernel gets loaded.
>
> Modified:
> head/sys/boot/common/load_elf.c
> head/sys/boot/uboot/common/main.c
> head/sys/boot/uboot/lib/copy.c
> head/sys/boot/uboot/lib/elf_freebsd.c
> head/sys/boot/uboot/lib/libuboot.h
>
> Modified: head/sys/boot/common/load_elf.c
>
> ==============================================================================
> --- head/sys/boot/common/load_elf.c Sun May 17 18:35:58 2015
> (r283034)
> +++ head/sys/boot/common/load_elf.c Sun May 17 19:59:05 2015
> (r283035)
> @@ -191,10 +191,17 @@ __elfN(loadfile_raw)(char *filename, u_i
> goto oerr;
> }
> /*
> - * Calculate destination address based on kernel entrypoint
> + * Calculate destination address based on kernel entrypoint.
> + *
> + * For ARM, the destination address is independent of any values
> in the
> + * elf header (an ARM kernel can be loaded at any 2MB boundary),
> so we
> + * leave dest set to the value calculated by
> archsw.arch_loadaddr() and
> + * passed in to this function.
> */
> +#ifndef __arm__
> if (ehdr->e_type == ET_EXEC)
> dest = (ehdr->e_entry & ~PAGE_MASK);
> +#endif
> if ((ehdr->e_entry & ~PAGE_MASK) == 0) {
> printf("elf" __XSTRING(__ELF_WORD_SIZE) "_loadfile: not a
> kernel (maybe static binary?)\n");
> err = EPERM;
> @@ -348,22 +355,18 @@ __elfN(loadimage)(struct preloaded_file
> off = 0;
> #elif defined(__arm__)
> /*
> - * The elf headers in some kernels specify virtual addresses in all
> - * header fields. More recently, the e_entry and p_paddr fields
> are the
> - * proper physical addresses. Even when the p_paddr fields are
> correct,
> - * the MI code below uses the p_vaddr fields with an offset added
> for
> - * loading (doing so is arguably wrong). To make loading work, we
> need
> - * an offset that represents the difference between physical and
> virtual
> - * addressing. ARM kernels are always linked at 0xCnnnnnnn.
> Depending
> - * on the headers, the offset value passed in may be physical or
> virtual
> - * (because it typically comes from e_entry), but we always replace
> - * whatever is passed in with the va<->pa offset. On the other
> hand, we
> - * always remove the high-order part of the entry address whether
> it's
> - * physical or virtual, because it will be adjusted later for the
> actual
> - * physical entry point based on where the image gets loaded.
> + * The elf headers in arm kernels specify virtual addresses in all
> + * header fields, even the ones that should be physical addresses.
> + * We assume the entry point is in the first page, and masking the
> page
> + * offset will leave us with the virtual address the kernel was
> linked
> + * at. We subtract that from the load offset, making 'off' into
> the
> + * value which, when added to a virtual address in an elf header,
> + * translates it to a physical address. We do the va->pa
> conversion on
> + * the entry point address in the header now, so that later we can
> + * launch the kernel by just jumping to that address.
> */
> - off = -0xc0000000;
> - ehdr->e_entry &= ~0xf0000000;
> + off -= ehdr->e_entry & ~PAGE_MASK;
> + ehdr->e_entry += off;
> #ifdef ELF_VERBOSE
> printf("ehdr->e_entry 0x%08x, va<->pa off %llx\n", ehdr->e_entry,
> off);
> #endif
>
> Modified: head/sys/boot/uboot/common/main.c
>
> ==============================================================================
> --- head/sys/boot/uboot/common/main.c Sun May 17 18:35:58 2015
> (r283034)
> +++ head/sys/boot/uboot/common/main.c Sun May 17 19:59:05 2015
> (r283035)
> @@ -28,6 +28,7 @@
>
> #include <sys/cdefs.h>
> __FBSDID("$FreeBSD$");
> +#include <sys/param.h>
>
> #include <stand.h>
>
> @@ -44,6 +45,9 @@ struct uboot_devdesc currdev;
> struct arch_switch archsw; /* MI/MD interface boundary */
> int devs_no;
>
> +uintptr_t uboot_heap_start;
> +uintptr_t uboot_heap_end;
> +
> struct device_type {
> const char *name;
> int type;
> @@ -414,7 +418,9 @@ main(void)
> * Initialise the heap as early as possible. Once this is done,
> * alloc() is usable. The stack is buried inside us, so this is
> safe.
> */
> - setheap((void *)end, (void *)(end + 512 * 1024));
> + uboot_heap_start = round_page((uintptr_t)end);
> + uboot_heap_end = uboot_heap_start + 512 * 1024;
> + setheap((void *)uboot_heap_start, (void *)uboot_heap_end);
>
> /*
> * Set up console.
> @@ -487,6 +493,7 @@ main(void)
> setenv("LINES", "24", 1); /* optional */
> setenv("prompt", "loader>", 1);
>
> + archsw.arch_loadaddr = uboot_loadaddr;
> archsw.arch_getdev = uboot_getdev;
> archsw.arch_copyin = uboot_copyin;
> archsw.arch_copyout = uboot_copyout;
>
> Modified: head/sys/boot/uboot/lib/copy.c
>
> ==============================================================================
> --- head/sys/boot/uboot/lib/copy.c Sun May 17 18:35:58 2015
> (r283034)
> +++ head/sys/boot/uboot/lib/copy.c Sun May 17 19:59:05 2015
> (r283035)
> @@ -27,66 +27,131 @@
>
> #include <sys/cdefs.h>
> __FBSDID("$FreeBSD$");
> +#include <sys/param.h>
>
> #include <stand.h>
> #include <stdint.h>
>
> #include "api_public.h"
> #include "glue.h"
> +#include "libuboot.h"
>
> /*
> * MD primitives supporting placement of module data
> */
>
> -void *
> -uboot_vm_translate(vm_offset_t o) {
> +#ifdef __arm__
> +#define KERN_ALIGN (2 * 1024 * 1024)
> +#else
> +#define KERN_ALIGN PAGE_SIZE
> +#endif
> +
> +/*
> + * Avoid low memory, u-boot puts things like args and dtb blobs there.
> + */
> +#define KERN_MINADDR max(KERN_ALIGN, (1024 * 1024))
> +
> +extern void _start(void); /* ubldr entry point address. */
> +
> +/*
> + * This is called for every object loaded (kernel, module, dtb file,
> etc). The
> + * expected return value is the next address at or after the given addr
> which is
> + * appropriate for loading the given object described by type and data.
> On each
> + * call the addr is the next address following the previously loaded
> object.
> + *
> + * The first call is for loading the kernel, and the addr argument will
> be zero,
> + * and we search for a big block of ram to load the kernel and modules.
> + *
> + * On subsequent calls the addr will be non-zero, and we just round it up
> so
> + * that each object begins on a page boundary.
> + */
> +uint64_t
> +uboot_loadaddr(u_int type, void *data, uint64_t addr)
> +{
> struct sys_info *si;
> - static uintptr_t start = 0;
> - static size_t size = 0;
> + uintptr_t sblock, eblock, subldr, eubldr;
> + uintptr_t biggest_block, this_block;
> + size_t biggest_size, this_size;
> int i;
> + char * envstr;
> +
> + if (addr == 0) {
> + /*
> + * If the loader_kernaddr environment variable is set,
> blindly
> + * honor it. It had better be right. We force
> interpretation
> + * of the value in base-16 regardless of any leading 0x
> prefix,
> + * because that's the U-Boot convention.
> + */
> + envstr = ub_env_get("loader_kernaddr");
> + if (envstr != NULL)
> + return (strtoul(envstr, NULL, 16));
>
> - if (size == 0) {
> + /*
> + * Find addr/size of largest DRAM block. Carve our own
> address
> + * range out of the block, because loading the kernel
> over the
> + * top ourself is a poor memory-conservation strategy.
> Avoid
> + * memory at beginning of the first block of physical ram,
> + * since u-boot likes to pass args and data there.
> Assume that
> + * u-boot has moved itself to the very top of ram and
> + * optimistically assume that we won't run into it up
> there.
> + */
> if ((si = ub_get_sys_info()) == NULL)
> panic("could not retrieve system info");
>
> - /* Find start/size of largest DRAM block. */
> + biggest_block = 0;
> + biggest_size = 0;
> + subldr = rounddown2((uintptr_t)_start, KERN_ALIGN);
> + eubldr = roundup2(uboot_heap_end, KERN_ALIGN);
> for (i = 0; i < si->mr_no; i++) {
> - if (si->mr[i].flags == MR_ATTR_DRAM
> - && si->mr[i].size > size) {
> - start = si->mr[i].start;
> - size = si->mr[i].size;
> + if (si->mr[i].flags != MR_ATTR_DRAM)
> + continue;
> + sblock = roundup2(si->mr[i].start, KERN_ALIGN);
> + eblock = rounddown2(si->mr[i].start +
> si->mr[i].size,
> + KERN_ALIGN);
> + if (biggest_size == 0)
> + sblock += KERN_MINADDR;
> + if (subldr >= sblock && subldr < eblock) {
> + if (subldr - sblock > eblock - eubldr) {
> + this_block = sblock;
> + this_size = subldr - sblock;
> + } else {
> + this_block = eubldr;
> + this_size = eblock - eubldr;
> + }
> + }
> + if (biggest_size < this_size) {
> + biggest_block = this_block;
> + biggest_size = this_size;
> }
> }
> -
> - if (size <= 0)
> - panic("No suitable DRAM?\n");
> - /*
> - printf("Loading into memory region 0x%08X-0x%08X (%d
> MiB)\n",
> - start, start + size, size / 1024 / 1024);
> - */
> + if (biggest_size == 0)
> + panic("Not enough DRAM to load kernel\n");
> +#if 0
> + printf("Loading kernel into region 0x%08x-0x%08x (%u
> MiB)\n",
> + biggest_block, biggest_block + biggest_size - 1,
> + biggest_size / 1024 / 1024);
> +#endif
> + return (biggest_block);
> }
> - if (o > size)
> - panic("Address offset 0x%08jX bigger than size 0x%08X\n",
> - (intmax_t)o, size);
> - return (void *)(start + o);
> + return roundup2(addr, PAGE_SIZE);
> }
>
> ssize_t
> uboot_copyin(const void *src, vm_offset_t dest, const size_t len)
> {
> - bcopy(src, uboot_vm_translate(dest), len);
> + bcopy(src, (void *)dest, len);
> return (len);
> }
>
> ssize_t
> uboot_copyout(const vm_offset_t src, void *dest, const size_t len)
> {
> - bcopy(uboot_vm_translate(src), dest, len);
> + bcopy((void *)src, dest, len);
> return (len);
> }
>
> ssize_t
> uboot_readin(const int fd, vm_offset_t dest, const size_t len)
> {
> - return (read(fd, uboot_vm_translate(dest), len));
> + return (read(fd, (void *)dest, len));
> }
>
> Modified: head/sys/boot/uboot/lib/elf_freebsd.c
>
> ==============================================================================
> --- head/sys/boot/uboot/lib/elf_freebsd.c Sun May 17 18:35:58 2015
> (r283034)
> +++ head/sys/boot/uboot/lib/elf_freebsd.c Sun May 17 19:59:05 2015
> (r283035)
> @@ -80,7 +80,7 @@ __elfN(uboot_exec)(struct preloaded_file
> if ((error = md_load(fp->f_args, &mdp)) != 0)
> return (error);
>
> - entry = uboot_vm_translate(e->e_entry);
> + entry = (void *)e->e_entry;
> printf("Kernel entry at 0x%x...\n", (unsigned)entry);
>
> dev_cleanup();
>
> Modified: head/sys/boot/uboot/lib/libuboot.h
>
> ==============================================================================
> --- head/sys/boot/uboot/lib/libuboot.h Sun May 17 18:35:58 2015
> (r283034)
> +++ head/sys/boot/uboot/lib/libuboot.h Sun May 17 19:59:05 2015
> (r283035)
> @@ -57,7 +57,10 @@ extern int devs_no;
> extern struct netif_driver uboot_net;
> extern struct devsw uboot_storage;
>
> -void *uboot_vm_translate(vm_offset_t);
> +extern uintptr_t uboot_heap_start;
> +extern uintptr_t uboot_heap_end;
> +
> +uint64_t uboot_loadaddr(u_int type, void *data, uint64_t addr);
> ssize_t uboot_copyin(const void *src, vm_offset_t dest, const
> size_t len);
> ssize_t uboot_copyout(const vm_offset_t src, void *dest, const
> size_t len);
> ssize_t uboot_readin(const int fd, vm_offset_t dest, const size_t
> len);
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAH7qZfts5k_4kOvkdsACNQdbqGd1Gu=m7%2BF1MO9FLgeQUOWEsg>