On Fri, Mar 04, 2016 at 03:15:23PM +0100, Patrick Wildt wrote:
> Hi,
>
> this diff makes armv7 map the FDT, if available, and uses it to read
> information about the machine's available memory and bootargs.
>
> I'd like to get some opinions about the way I have implemented some
> stuff. For instance, I need the size of the FDT so I can properly
> copy it. Does it make sense to implement fdt_get_size()? Is there
> another, preferred way?
>
> Additionally, reading memory information, like where a node is mapped,
> is a bit more complicated than just reading two values. Especially if
> there are translation ranges on a node's parent. I also have working
> code or that, but I skipped "ranges" to not bloat the diff any further.
If memory serves me correctly we need ranges right out of the gate for
the exynos boards I have, but those aren't important to anyone except
for me so I'm ok with waiting a short while.
>
> I would like to extend the fdt_* API and add helpers to the code, so
> that it's easier to use the device tree. Is this the correct way?
> Should I rather implement some OF_* API?
My initial thought is that it makes sense to extend fdt for this.
I have not tested this diff, but I like the direction that this is
going.
Cheers,
bmercer
>
> Patrick
>
> diff --git sys/arch/armv7/armv7/armv7_machdep.c
> sys/arch/armv7/armv7/armv7_machdep.c
> index 11bea90..0a067c2 100644
> --- sys/arch/armv7/armv7/armv7_machdep.c
> +++ sys/arch/armv7/armv7/armv7_machdep.c
> @@ -130,6 +130,7 @@
> #include <armv7/armv7/armv7_machdep.h>
>
> #include <dev/cons.h>
> +#include <dev/ofw/fdt.h>
>
> #include <net/if.h>
>
> @@ -387,8 +388,10 @@ initarm(void *arg0, void *arg1, void *arg2)
> int loop, loop1, i, physsegs;
> u_int l1pagetable;
> pv_addr_t kernel_l1pt;
> + pv_addr_t fdt;
> paddr_t memstart;
> psize_t memsize;
> + void *config;
> extern uint32_t esym; /* &_end if no symbols are loaded */
>
> /* early bus_space_map support */
> @@ -423,6 +426,49 @@ initarm(void *arg0, void *arg1, void *arg2)
> armv7_a4x_bs_tag.bs_map = bootstrap_bs_map;
> tmp_bs_tag.bs_map = bootstrap_bs_map;
>
> + /*
> + * Now, map the bootconfig/FDT area.
> + *
> + * As we don't know the size of a possible FDT, map the size of a
> + * typical bootstrap bs map. The FDT might not be aligned, so this
> + * might take up to two L1_S_SIZEd mappings. In the unlikely case
> + * that the FDT is bigger than L1_S_SIZE (0x00100000), we need to
> + * remap it.
> + *
> + * XXX: There's (currently) no way to unmap a bootstrap mapping, so
> + * we might lose a bit of the bootstrap address space.
> + */
> + bootstrap_bs_map(NULL, (bus_addr_t)arg2, L1_S_SIZE, 0,
> + (bus_space_handle_t *)&config);
> + if (fdt_init(config) && fdt_get_size(config) != 0) {
> + uint32_t size = fdt_get_size(config);
> + if (size > L1_S_SIZE)
> + bootstrap_bs_map(NULL, (bus_addr_t)arg2, size, 0,
> + (bus_space_handle_t *)&config);
> + }
> +
> + if (fdt_init(config) && fdt_get_size(config) != 0) {
> + struct fdt_memory mem;
> + void *node;
> +
> + node = fdt_find_node("/memory");
> + if (node == NULL || fdt_get_memory_address(node, 0, &mem))
> + panic("initarm: no memory specificed");
> +
> + memstart = mem.addr;
> + memsize = mem.size;
> + physical_start = mem.addr;
> + physical_end = MIN(mem.addr + mem.size, (paddr_t)-PAGE_SIZE);
> +
> + node = fdt_find_node("/chosen");
> + if (node != NULL) {
> + char *bootargs;
> + if (fdt_node_property(node, "bootargs", &bootargs))
> + process_kernel_args(bootargs);
> + }
> + }
> +
> + /* XXX: Use FDT information. */
> platform_init();
> platform_disable_l2_if_needed();
>
> @@ -433,41 +479,36 @@ initarm(void *arg0, void *arg1, void *arg2)
> printf("\n%s booting ...\n", platform_boot_name());
>
> printf("arg0 %p arg1 %p arg2 %p\n", arg0, arg1, arg2);
> - parse_uboot_tags(arg2);
>
> - /*
> - * Examine the boot args string for options we need to know about
> - * now.
> - */
> - process_kernel_args(bootconfig.bootstring);
> + if (fdt_get_size(config) == 0) {
> + parse_uboot_tags(config);
> +
> + /*
> + * Examine the boot args string for options we need to know
> about
> + * now.
> + */
> + process_kernel_args(bootconfig.bootstring);
> +
> + /* normally u-boot will set up bootconfig.dramblocks */
> + bootconfig_dram(&bootconfig, &memstart, &memsize);
> +
> + /*
> + * Set up the variables that define the availablilty of
> + * physical memory.
> + *
> + * XXX pmap_bootstrap() needs an enema.
> + */
> + physical_start = bootconfig.dram[0].address;
> + physical_end = MIN((uint64_t)physical_start +
> + (bootconfig.dram[0].pages * PAGE_SIZE),
> (paddr_t)-PAGE_SIZE);
> + }
> +
> #ifdef RAMDISK_HOOKS
> - boothowto |= RB_DFLTROOT;
> + boothowto |= RB_DFLTROOT;
> #endif /* RAMDISK_HOOKS */
>
> - /* normally u-boot will set up bootconfig.dramblocks */
> - bootconfig_dram(&bootconfig, &memstart, &memsize);
> -
> - /*
> - * Set up the variables that define the availablilty of
> - * physical memory. For now, we're going to set
> - * physical_freestart to 0xa0200000 (where the kernel
> - * was loaded), and allocate the memory we need downwards.
> - * If we get too close to the page tables that RedBoot
> - * set up, we will panic. We will update physical_freestart
> - * and physical_freeend later to reflect what pmap_bootstrap()
> - * wants to see.
> - *
> - * XXX pmap_bootstrap() needs an enema.
> - */
> - physical_start = bootconfig.dram[0].address;
> - physical_end = MIN((uint64_t)physical_start +
> - (bootconfig.dram[0].pages * PAGE_SIZE), (paddr_t)-PAGE_SIZE);
> -
> - {
> - physical_freestart = (((unsigned long)esym - KERNEL_TEXT_BASE
> +0xfff) & ~0xfff) + memstart;
> - physical_freeend = MIN((uint64_t)memstart+memsize,
> - (paddr_t)-PAGE_SIZE);
> - }
> + physical_freestart = (((unsigned long)esym - KERNEL_TEXT_BASE +0xfff) &
> ~0xfff) + memstart;
> + physical_freeend = MIN((uint64_t)memstart+memsize, (paddr_t)-PAGE_SIZE);
>
> physmem = (physical_end - physical_start) / PAGE_SIZE;
>
> @@ -566,6 +607,15 @@ initarm(void *arg0, void *arg1, void *arg2)
> #endif
>
> /*
> + * Allocate pages for an FDT copy.
> + */
> + if (fdt_get_size(config) != 0) {
> + uint32_t size = fdt_get_size(config);
> + valloc_pages(fdt, round_page(size) / PAGE_SIZE);
> + memcpy((void *)fdt.pv_pa, config, size);
> + }
> +
> + /*
> * XXX Defer this to later so that we can reclaim the memory
> * XXX used by the RedBoot page tables.
> */
> @@ -656,6 +706,12 @@ initarm(void *arg0, void *arg1, void *arg2)
> pmap_map_entry(l1pagetable, vector_page, systempage.pv_pa,
> PROT_READ | PROT_WRITE | PROT_EXEC, PTE_CACHE);
>
> + /* Map the FDT. */
> + if (fdt.pv_va && fdt.pv_pa)
> + pmap_map_chunk(l1pagetable, fdt.pv_va, fdt.pv_pa,
> + round_page(fdt_get_size((void *)fdt.pv_pa)),
> + PROT_READ | PROT_WRITE, PTE_CACHE);
> +
> /*
> * map integrated peripherals at same address in l1pagetable
> * so that we can continue to use console.
> @@ -716,6 +772,10 @@ initarm(void *arg0, void *arg1, void *arg2)
> prefetch_abort_handler_address = (u_int)prefetch_abort_handler;
> undefined_handler_address = (u_int)undefinedinstruction_bounce;
>
> + /* Now we can reinit the FDT, using the virtual address. */
> + if (fdt.pv_va && fdt.pv_pa)
> + fdt_init((void *)fdt.pv_va);
> +
> /* Initialise the undefined instruction handlers */
> #ifdef VERBOSE_INIT_ARM
> printf("undefined ");
> diff --git sys/arch/armv7/conf/files.armv7 sys/arch/armv7/conf/files.armv7
> index 84c6064..3136ad0 100644
> --- sys/arch/armv7/conf/files.armv7
> +++ sys/arch/armv7/conf/files.armv7
> @@ -10,6 +10,7 @@ major {rd = 18}
>
> define fdt {}
> file arch/armv7/fdt/fdt_machdep.c fdt needs-flag
> +file dev/ofw/fdt.c
>
> file arch/arm/arm/conf.c
>
> diff --git sys/dev/ofw/fdt.c sys/dev/ofw/fdt.c
> index 6111faa..18f6077 100644
> --- sys/dev/ofw/fdt.c
> +++ sys/dev/ofw/fdt.c
> @@ -31,6 +31,8 @@ void *skip_property(u_int32_t *);
> void *skip_props(u_int32_t *);
> void *skip_node_name(u_int32_t *);
> void *fdt_parent_node_recurse(void *, void *);
> +int fdt_node_property_int(void *, char *, int *);
> +int fdt_node_property_ints(void *, char *, int *, int);
> #ifdef DEBUG
> void fdt_print_node_recurse(void *, int);
> #endif
> @@ -95,6 +97,21 @@ fdt_init(void *fdt)
> return version;
> }
>
> + /*
> + * Return the size of the FDT.
> + */
> +size_t
> +fdt_get_size(void *fdt)
> +{
> + if (!fdt)
> + return 0;
> +
> + if (!fdt_check_head(fdt))
> + return 0;
> +
> + return betoh32(((struct fdt_head *)fdt)->fh_size);
> +}
> +
> /*
> * Retrieve string pointer from strings table.
> */
> @@ -172,6 +189,35 @@ fdt_node_property(void *node, char *name, char **out)
> }
>
> /*
> + * Retrieves node property as integers and puts them in the given
> + * integer array.
> + */
> +int
> +fdt_node_property_ints(void *node, char *name, int *out, int outlen)
> +{
> + int *data;
> + int i, inlen;
> +
> + inlen = fdt_node_property(node, name, (char **)&data) / sizeof(int);
> + if (inlen <= 0)
> + return -1;
> +
> + for (i = 0; i < inlen && i < outlen; i++)
> + out[i] = betoh32(data[i]);
> +
> + return i;
> +}
> +
> +/*
> + * Retrieves node property as an integer.
> + */
> +int
> +fdt_node_property_int(void *node, char *name, int *out)
> +{
> + return fdt_node_property_ints(node, name, out, 1);
> +}
> +
> +/*
> * Retrieves next node, skipping all the children nodes of the pointed node
> * if passed 0 wil return first node of the tree (root)
> */
> @@ -318,6 +364,52 @@ fdt_parent_node(void *node)
> return fdt_parent_node_recurse(pnode, node);
> }
>
> +/*
> + * Parse the memory address and size of a node.
> + */
> +int
> +fdt_get_memory_address(void *node, int idx, struct fdt_memory *mem)
> +{
> + void *parent;
> + int ac, sc, off, ret, *in, inlen;
> +
> + if (node == NULL)
> + return 1;
> +
> + parent = fdt_parent_node(node);
> + if (parent == NULL)
> + return 1;
> +
> + /* We only support 32-bit (1), and 64-bit (2) wide addresses here. */
> + ret = fdt_node_property_int(parent, "#address-cells", &ac);
> + if (ret != 1 || ac <= 0 || ac > 2)
> + return 1;
> +
> + /* We only support 32-bit (1), and 64-bit (2) wide sizes here. */
> + ret = fdt_node_property_int(parent, "#size-cells", &sc);
> + if (ret != 1 || sc <= 0 || sc > 2)
> + return 1;
> +
> + inlen = fdt_node_property(node, "reg", (char **)&in) / sizeof(int);
> + if (inlen < ((idx + 1) * (ac + sc)))
> + return 1;
> +
> + off = idx * (ac + sc);
> +
> + mem->addr = betoh32(in[off]);
> + if (ac == 2)
> + mem->addr = (mem->addr << 32) + betoh32(in[off + 1]);
> +
> + mem->size = betoh32(in[off + ac]);
> + if (sc == 2)
> + mem->size = (mem->size << 32) + betoh32(in[off + ac + 1]);
> +
> + /* TODO: translate memory address in ranges */
> + //return fdt_translate_memory_address(parent, mem);
> +
> + return 0;
> +}
> +
> #ifdef DEBUG
> /*
> * Debug methods for printing whole tree, particular odes and properies
> diff --git sys/dev/ofw/fdt.h sys/dev/ofw/fdt.h
> index 6a21239..ecb8bed 100644
> --- sys/dev/ofw/fdt.h
> +++ sys/dev/ofw/fdt.h
> @@ -38,6 +38,11 @@ struct fdt {
> int strings_size;
> };
>
> +struct fdt_memory {
> + uint64_t addr;
> + uint64_t size;
> +};
> +
> #define FDT_MAGIC 0xd00dfeed
> #define FDT_NODE_BEGIN 0x01
> #define FDT_NODE_END 0x02
> @@ -48,12 +53,14 @@ struct fdt {
> #define FDT_CODE_VERSION 0x11
>
> int fdt_init(void *);
> +size_t fdt_get_size(void *);
> void *fdt_next_node(void *);
> void *fdt_child_node(void *);
> char *fdt_node_name(void *);
> void *fdt_find_node(char *);
> int fdt_node_property(void *, char *, char **);
> void *fdt_parent_node(void *);
> +int fdt_get_memory_address(void *, int, struct fdt_memory *);
> #ifdef DEBUG
> void *fdt_print_property(void *, int);
> void fdt_print_node(void *, int);
>