Re: [Xen-devel] [PATCH V1 11/29] xen/dts: Check "reg" property length in process_multiboot_node

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 09/06/2013 05:40 PM, Ian Campbell wrote:
> On Wed, 2013-08-28 at 15:47 +0100, Julien Grall wrote:
> > When the device tree compiler (dtc) can't find right #address-cells
> > and #size-cells, it will assume the encoding is 1 for each.
> Without inserting them into the DTB I suppose?
Yes. I have noticed that the encoding is not 1 for each but
"If missing, a client program should assume a default value of 2 for 
#address-cells, and a value of 1 for #size-cells." (ePAR 2.3.5)
dtc will warns with the following messages:
Warning (reg_format): "reg" property in /chosen/modules/module@0 has 
invalid length (8 bytes) (#address-cells == 2, #size-cells == 1)
Warning (avoid_default_addr_size): Relying on default #address-cells 
value for /chosen/modules/module@0
Warning (avoid_default_addr_size): Relying on default #size-cells value 
for /chosen/modules/module@0
> > As multiboot node are inside the /chosen, dtc will asume the previous values
> "assume"
>
> > if the both property are not correct set.
> "the both property" => "both properties" and "correctly"
>
> By "previous values" do you mean the values set in the (grand)parent
> node?
Yes. I will update the comment.

> > During boot, Xen will browse the fdt and store the nearest #address-cells
> > and #size-cells value. If the size of "reg" is smaller, Xen can retrieve
> > wrong range.
> How come Xen can find a suitable #address-cells but dtc cannot? Are we
> not handling them correctly perhaps?
So we have two issues:
  1) Xen doesn't check the length of the reg property (this patch)
  2) Default value for #address-cells and #size-cells are wrong

> > Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> > ---
> >   xen/common/device_tree.c |    6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> > index 9568250..7295f34 100644
> > --- a/xen/common/device_tree.c
> > +++ b/xen/common/device_tree.c
> > @@ -467,10 +467,14 @@ static void __init process_multiboot_node(const void 
> > *fdt, int node,
> >
> >       mod = &early_info.modules.module[nr];
> >
> > -    prop = fdt_get_property(fdt, node, "reg", NULL);
> > +    prop = fdt_get_property(fdt, node, "reg", &len);
> >       if ( !prop )
> >           early_panic("node %s missing `reg' property\n", name);
> >
> > +    if ( len < dt_cells_to_size(address_cells + size_cells) )
> > +        early_panic("fdt: node `%s': `reg` property length is too short\n",
> > +                    name);
> > +
> >       cell = (const u32 *)prop->data;
> >       device_tree_get_reg(&cell, address_cells, size_cells,
> >                           &mod->start, &mod->size);

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel