| Message ID | 1377701263-3319-12-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
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? > 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? > 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? > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > --- > 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);
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@linaro.org> >> --- >> 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); > >
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);
When the device tree compiler (dtc) can't find right #address-cells and #size-cells, it will assume the encoding is 1 for each. As multiboot node are inside the /chosen, dtc will asume the previous values if the both property are not correct set. 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. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/common/device_tree.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)