Date: Sat, 13 Jun 2015 14:53:35 -0700 From: Maxim Sobolev <sobomax@sippysoft.com> 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: <CAH7qZfvUwztY=2dNDYNgLvJE2_qUVYsCw-0Nk=SfJFQn=NxOmw@mail.gmail.com> In-Reply-To: <CAH7qZfts5k_4kOvkdsACNQdbqGd1Gu=m7%2BF1MO9FLgeQUOWEsg@mail.gmail.com> References: <CAH7qZfts5k_4kOvkdsACNQdbqGd1Gu=m7%2BF1MO9FLgeQUOWEsg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Just in case, my debug code and proposed fix can be found here: https://github.com/sobomax/FreeBSD.arm/commit/48509790f82352cb455d0f5029291b917afb985b Unless I hear from you I am going to push it into the repository in the next few days (modulo printf() debug stuff). Thanks! On Thu, Jun 11, 2015 at 6:21 AM, Maxim Sobolev <sobomax@freebsd.org> wrote: > 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); >> > -- Maksym Sobolyev Sippy Software, Inc. Internet Telephony (VoIP) Experts Tel (Canada): +1-778-783-0474 Tel (Toll-Free): +1-855-747-7779 Fax: +1-866-857-6942 Web: http://www.sippysoft.com MSN: sales@sippysoft.com Skype: SippySoft
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAH7qZfvUwztY=2dNDYNgLvJE2_qUVYsCw-0Nk=SfJFQn=NxOmw>