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

Reply via email to