On Fri, Mar 04, 2016 at 09:38:18AM -0500, Brandon Mercer wrote:
> 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. 

Yes, I stumbled upon that on the raspberry pi.  The default device tree
delivered by the raspberry pi foundation makes use of that.

Excerpt:

/ {
        #address-cells = <0x1>;
        #size-cells = <0x1>;
        interrupt-parent = <0x1>;
        compatible = "brcm,bcm2710", "brcm,bcm2709";
        model = "Raspberry Pi 3 Model B";
[...]
        soc {
                compatible = "simple-bus";
                #address-cells = <0x1>;
                #size-cells = <0x1>;
                ranges = <0x7e000000 0x3f000000 0x1000000>;
[...]
                interrupt-controller@7e00b200 {
                        compatible = "brcm,bcm2708-armctrl-ic";
                        reg = <0x7e00b200 0x200>;
                        interrupt-controller;
                        #interrupt-cells = <0x2>;
                        linux,phandle = <0x1>;
                        phandle = <0x1>;
                };

So basically the interrupt-controller node says it's at 0x7e00b200,
but you need to translate that address using the parent's ranges
array to find out that the actual address is 0x3f00b200.  If that
had another parent with a range, it would have to be translated
another time.

diff --git sys/dev/ofw/fdt.c sys/dev/ofw/fdt.c
index 18f6077..0993c92 100644
--- sys/dev/ofw/fdt.c
+++ sys/dev/ofw/fdt.c
@@ -33,6 +33,7 @@ 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);
+int     fdt_translate_memory_address(void *, struct fdt_memory *);
 #ifdef DEBUG
 void    fdt_print_node_recurse(void *, int);
 #endif
@@ -361,10 +362,94 @@ fdt_parent_node(void *node)
        if (!tree_inited)
                return NULL;
 
+       if (node == pnode)
+               return NULL;
+
        return fdt_parent_node_recurse(pnode, node);
 }
 
 /*
+ * Translate memory address depending on parent's range.
+ */
+int
+fdt_translate_memory_address(void *node, struct fdt_memory *mem)
+{
+       void *parent;
+       int pac, psc, ac, sc, ret, rlen, rone, *range;
+       uint64_t from, to, size;
+
+       if (node == NULL || mem == NULL)
+               return 1;
+
+       parent = fdt_parent_node(node);
+       if (parent == NULL)
+               return 0;
+
+       /* Empty ranges, 1:1 mapping. No ranges, translation barrier. */
+       rlen = fdt_node_property(node, "ranges", (char **)&range) / sizeof(int);
+       if (range == NULL)
+               return 0;
+       if (rlen <= 0)
+               return fdt_translate_memory_address(parent, mem);
+
+       /* We only support 32-bit (1), and 64-bit (2) wide addresses here. */
+       ret = fdt_node_property_int(parent, "#address-cells", &pac);
+       if (ret != 1 || pac <= 0 || pac > 2)
+               return 1;
+
+       /* We only support 32-bit (1), and 64-bit (2) wide sizes here. */
+       ret = fdt_node_property_int(parent, "#size-cells", &psc);
+       if (ret != 1 || psc <= 0 || psc > 2)
+               return 1;
+
+       /* We only support 32-bit (1), and 64-bit (2) wide addresses here. */
+       ret = fdt_node_property_int(node, "#address-cells", &ac);
+       if (ret <= 0)
+               ac = pac;
+       else 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(node, "#size-cells", &sc);
+       if (ret <= 0)
+               sc = psc;
+       else if (ret > 1 || sc <= 0 || sc > 2)
+               return 1;
+
+       /* Must have at least one range. */
+       rone = pac + ac + sc;
+       if (rlen < rone)
+               return 1;
+
+       /* For each range. */
+       for (; rlen >= rone; rlen -= rone, range += rone) {
+               /* Extract from and size, so we can see if we fit. */
+               from = betoh32(range[0]);
+               if (ac == 2)
+                       from = (from << 32) + betoh32(range[1]);
+               size = betoh32(range[ac + pac]);
+               if (sc == 2)
+                       size = (size << 32) + betoh32(range[ac + pac + 1]);
+
+               /* Try next, if we're not in the range. */
+               if (mem->addr < from || (mem->addr + mem->size) > (from + size))
+                       continue;
+
+               /* All good, extract to address and translate. */
+               to = betoh32(range[ac]);
+               if (pac == 2)
+                       to = (to << 32) + betoh32(range[ac + 1]);
+
+               mem->addr -= from;
+               mem->addr += to;
+               return fdt_translate_memory_address(parent, mem);
+       }
+
+       /* Range must have been in there. */
+       return 1;
+}
+
+/*
  * Parse the memory address and size of a node.
  */
 int
@@ -404,10 +489,7 @@ fdt_get_memory_address(void *node, int idx, struct 
fdt_memory *mem)
        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;
+       return fdt_translate_memory_address(parent, mem);
 }
 
 #ifdef DEBUG


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