Change the signature of process_memory_node to match
device_tree_node_func. Thanks to this change, the next patch will be
able to use device_tree_for_each_node to call process_memory_node on all
the children of a provided node.
Return error if there is no reg property or if nr_banks is reached. Let
the caller deal with the error.
Add a printk when device tree parsing fails.
Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
---
Changes in v6:
- fix out of space check
- bring back printk when address_cells or size_cells are not properly set
- return -EINVAL in that case (different from reg missing)
- add printk when parsing fails
- return -ENOENT when memory size is 0
Changes in v5:
- return -ENOENT if address_cells or size_cells are not properly set
Changes in v4:
- return error if there is no reg propery, remove printk
- return error if nr_banks is reached
Changes in v3:
- improve commit message
- check return value of process_memory_node
Changes in v2:
- new
---
xen/arch/arm/bootfdt.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index f1614ef7fc..9dc2c1352d 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -130,9 +130,10 @@ int __init device_tree_for_each_node(const void *fdt, int node,
return 0;
}
-static void __init process_memory_node(const void *fdt, int node,
- const char *name,
- u32 address_cells, u32 size_cells)
+static int __init process_memory_node(const void *fdt, int node,
+ const char *name, int depth,
+ u32 address_cells, u32 size_cells,
+ void *data)
{
const struct fdt_property *prop;
int i;
@@ -145,15 +146,12 @@ static void __init process_memory_node(const void *fdt, int node,
{
printk("fdt: node `%s': invalid #address-cells or #size-cells",
name);
- return;
+ return -EINVAL;
}
prop = fdt_get_property(fdt, node, "reg", NULL);
if ( !prop )
- {
- printk("fdt: node `%s': missing `reg' property\n", name);
- return;
- }
+ return -ENOENT;
cell = (const __be32 *)prop->data;
banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
@@ -162,11 +160,15 @@ static void __init process_memory_node(const void *fdt, int node,
{
device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
if ( !size )
- continue;
+ return -ENOENT;
bootinfo.mem.bank[bootinfo.mem.nr_banks].start = start;
bootinfo.mem.bank[bootinfo.mem.nr_banks].size = size;
bootinfo.mem.nr_banks++;
}
+
+ if ( i < banks )
+ return -ENOSPC;
+ return 0;
}
static void __init process_multiboot_node(const void *fdt, int node,
@@ -298,15 +300,20 @@ static int __init early_scan_node(const void *fdt,
u32 address_cells, u32 size_cells,
void *data)
{
+ int rc = 0;
+
if ( device_tree_node_matches(fdt, node, "memory") )
- process_memory_node(fdt, node, name, address_cells, size_cells);
+ rc = process_memory_node(fdt, node, name, depth,
+ address_cells, size_cells, NULL);
else if ( depth <= 3 && (device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) ||
device_tree_node_compatible(fdt, node, "multiboot,module" )))
process_multiboot_node(fdt, node, name, address_cells, size_cells);
else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen") )
process_chosen_node(fdt, node, name, address_cells, size_cells);
- return 0;
+ if ( rc < 0 )
+ printk("fdt: node `%s': parsing failed\n", name);
+ return rc;
}
static void __init early_print_info(void)
--
2.17.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-develHi,
On 16/08/2019 00:36, Stefano Stabellini wrote:
> Change the signature of process_memory_node to match
> device_tree_node_func. Thanks to this change, the next patch will be
> able to use device_tree_for_each_node to call process_memory_node on all
> the children of a provided node.
>
> Return error if there is no reg property or if nr_banks is reached. Let
> the caller deal with the error.
This sentence does not match the change below. Only 2 of the new error paths are
described here.
>
> Add a printk when device tree parsing fails.
>
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> ---
> Changes in v6:
> - fix out of space check
> - bring back printk when address_cells or size_cells are not properly set
> - return -EINVAL in that case (different from reg missing)
> - add printk when parsing fails
> - return -ENOENT when memory size is 0
>
> Changes in v5:
> - return -ENOENT if address_cells or size_cells are not properly set
>
> Changes in v4:
> - return error if there is no reg propery, remove printk
> - return error if nr_banks is reached
>
> Changes in v3:
> - improve commit message
> - check return value of process_memory_node
>
> Changes in v2:
> - new
> ---
> xen/arch/arm/bootfdt.c | 29 ++++++++++++++++++-----------
> 1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> index f1614ef7fc..9dc2c1352d 100644
> --- a/xen/arch/arm/bootfdt.c
> +++ b/xen/arch/arm/bootfdt.c
> @@ -130,9 +130,10 @@ int __init device_tree_for_each_node(const void *fdt, int node,
> return 0;
> }
>
> -static void __init process_memory_node(const void *fdt, int node,
> - const char *name,
> - u32 address_cells, u32 size_cells)
> +static int __init process_memory_node(const void *fdt, int node,
> + const char *name, int depth,
> + u32 address_cells, u32 size_cells,
> + void *data)
> {
> const struct fdt_property *prop;
> int i;
> @@ -145,15 +146,12 @@ static void __init process_memory_node(const void *fdt, int node,
> {
> printk("fdt: node `%s': invalid #address-cells or #size-cells",
> name);
> - return;
> + return -EINVAL;
> }
>
> prop = fdt_get_property(fdt, node, "reg", NULL);
> if ( !prop )
> - {
> - printk("fdt: node `%s': missing `reg' property\n", name);
> - return;
> - }
> + return -ENOENT;
>
> cell = (const __be32 *)prop->data;
> banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
> @@ -162,11 +160,15 @@ static void __init process_memory_node(const void *fdt, int node,
> {
> device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
> if ( !size )
> - continue;
> + return -ENOENT;
I don't think we can treat the same way the lack of "regs" properties and a size
of 0.
The former is expected as binding allow you to do it for reserved-memory. The
latter is the user not writing the property correctly. So ignoring the latter
will result to Xen potentially missing some reserved-regions (not great!).
So, similar to #address-cells/#size-cells discussion, we should return an error
we are able to distinguish. Probably -EINVAL.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On Fri, 16 Aug 2019, Julien Grall wrote:
> Hi,
>
> On 16/08/2019 00:36, Stefano Stabellini wrote:
> > Change the signature of process_memory_node to match
> > device_tree_node_func. Thanks to this change, the next patch will be
> > able to use device_tree_for_each_node to call process_memory_node on all
> > the children of a provided node.
> >
> > Return error if there is no reg property or if nr_banks is reached. Let
> > the caller deal with the error.
>
> This sentence does not match the change below. Only 2 of the new error paths
> are described here.
>
> >
> > Add a printk when device tree parsing fails.
> >
> > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> > ---
> > Changes in v6:
> > - fix out of space check
> > - bring back printk when address_cells or size_cells are not properly set
> > - return -EINVAL in that case (different from reg missing)
> > - add printk when parsing fails
> > - return -ENOENT when memory size is 0
> >
> > Changes in v5:
> > - return -ENOENT if address_cells or size_cells are not properly set
> >
> > Changes in v4:
> > - return error if there is no reg propery, remove printk
> > - return error if nr_banks is reached
> >
> > Changes in v3:
> > - improve commit message
> > - check return value of process_memory_node
> >
> > Changes in v2:
> > - new
> > ---
> > xen/arch/arm/bootfdt.c | 29 ++++++++++++++++++-----------
> > 1 file changed, 18 insertions(+), 11 deletions(-)
> >
> > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> > index f1614ef7fc..9dc2c1352d 100644
> > --- a/xen/arch/arm/bootfdt.c
> > +++ b/xen/arch/arm/bootfdt.c
> > @@ -130,9 +130,10 @@ int __init device_tree_for_each_node(const void *fdt,
> > int node,
> > return 0;
> > }
> > -static void __init process_memory_node(const void *fdt, int node,
> > - const char *name,
> > - u32 address_cells, u32 size_cells)
> > +static int __init process_memory_node(const void *fdt, int node,
> > + const char *name, int depth,
> > + u32 address_cells, u32 size_cells,
> > + void *data)
> > {
> > const struct fdt_property *prop;
> > int i;
> > @@ -145,15 +146,12 @@ static void __init process_memory_node(const void
> > *fdt, int node,
> > {
> > printk("fdt: node `%s': invalid #address-cells or #size-cells",
> > name);
> > - return;
> > + return -EINVAL;
> > }
> > prop = fdt_get_property(fdt, node, "reg", NULL);
> > if ( !prop )
> > - {
> > - printk("fdt: node `%s': missing `reg' property\n", name);
> > - return;
> > - }
> > + return -ENOENT;
> > cell = (const __be32 *)prop->data;
> > banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
> > @@ -162,11 +160,15 @@ static void __init process_memory_node(const void
> > *fdt, int node,
> > {
> > device_tree_get_reg(&cell, address_cells, size_cells, &start,
> > &size);
> > if ( !size )
> > - continue;
> > + return -ENOENT;
>
> I don't think we can treat the same way the lack of "regs" properties and a
> size of 0.
>
> The former is expected as binding allow you to do it for reserved-memory. The
> latter is the user not writing the property correctly. So ignoring the latter
> will result to Xen potentially missing some reserved-regions (not great!).
>
> So, similar to #address-cells/#size-cells discussion, we should return an
> error we are able to distinguish. Probably -EINVAL.
I think you are right, and honestly I was thinking about it while I
updated this patch. If I use -EINVAL, it would be the same return error
as the "invalid #address-cells or #size-cells". I just wanted to
double-check that it is OK for you.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 8/17/19 1:48 AM, Stefano Stabellini wrote:
> On Fri, 16 Aug 2019, Julien Grall wrote:
> I think you are right, and honestly I was thinking about it while I
> updated this patch. If I use -EINVAL, it would be the same return error
> as the "invalid #address-cells or #size-cells". I just wanted to
> double-check that it is OK for you.
We don't need to differentiate "invalid #{address, size}-cells" from
"zero size", so using the same errno is fine.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2026 Red Hat, Inc.