Re: [XEN][PATCH v6 02/19] common/device_tree: handle memory allocation failure in __unflatten_device_tree()
Hi Vikram,
On 31/05/2023 22:32, Vikram Garhwal wrote:
> Hi Michal,
>
> On 5/5/23 2:38 AM, Michal Orzel wrote:
>> On 03/05/2023 01:36, Vikram Garhwal wrote:
>>>
>>> Change __unflatten_device_tree() return type to integer so it can propagate
>>> memory allocation failure. Add panic() in dt_unflatten_host_device_tree()
>>> for
>>> memory allocation failure during boot.
>>>
>>> Signed-off-by: Vikram Garhwal
>> I think we are missing a Fixes tag.
> Like the below line?
> Fixes: fb97eb6 ("xen/arm: Create a hierarchical device tree")
Please check docs/process/sending-patches.pandoc
The correct format for a Fixes tag should be:
Fixes: fb97eb614acf ("xen/arm: Create a hierarchical device tree")
~Michal
Re: [XEN][PATCH v6 02/19] common/device_tree: handle memory allocation failure in __unflatten_device_tree()
Hi Michal,
On 5/5/23 2:38 AM, Michal Orzel wrote:
On 03/05/2023 01:36, Vikram Garhwal wrote:
Change __unflatten_device_tree() return type to integer so it can propagate
memory allocation failure. Add panic() in dt_unflatten_host_device_tree() for
memory allocation failure during boot.
Signed-off-by: Vikram Garhwal
I think we are missing a Fixes tag.
Like the below line?
Fixes: fb97eb6 ("xen/arm: Create a hierarchical device tree")
Original patch for your reference:
https://github.com/xen-project/xen/commit/fb97eb614acfbcc812098bbbe5dde99271fe0a0d
Regards,
Vikram
---
xen/common/device_tree.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 5f7ae45304..fc38a0b3dd 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -2056,8 +2056,8 @@ static unsigned long unflatten_dt_node(const void *fdt,
* @fdt: The fdt to expand
* @mynodes: The device_node tree created by the call
*/
-static void __init __unflatten_device_tree(const void *fdt,
- struct dt_device_node **mynodes)
+static int __init __unflatten_device_tree(const void *fdt,
+ struct dt_device_node **mynodes)
{
unsigned long start, mem, size;
struct dt_device_node **allnextp = mynodes;
@@ -2078,6 +2078,8 @@ static void __init __unflatten_device_tree(const void
*fdt,
/* Allocate memory for the expanded device tree */
mem = (unsigned long)_xmalloc (size + 4, __alignof__(struct
dt_device_node));
+if ( !mem )
+return -ENOMEM;
((__be32 *)mem)[size / 4] = cpu_to_be32(0xdeadbeef);
@@ -2095,6 +2097,8 @@ static void __init __unflatten_device_tree(const void
*fdt,
*allnextp = NULL;
dt_dprintk(" <- unflatten_device_tree()\n");
+
+return 0;
}
static void dt_alias_add(struct dt_alias_prop *ap,
@@ -2179,7 +2183,10 @@ dt_find_interrupt_controller(const struct
dt_device_match *matches)
void __init dt_unflatten_host_device_tree(void)
{
-__unflatten_device_tree(device_tree_flattened, &dt_host);
+int error = __unflatten_device_tree(device_tree_flattened, &dt_host);
NIT: there should be a blank line between definitions and rest of the code
+if ( error )
+panic("__unflatten_device_tree failed with error %d\n", error);
+
dt_alias_scan();
}
--
2.17.1
FWICS, patches 2 and 4 are not strictly related to DTBO and are fixing issues
and propagating errors which is always good. Therefore by moving them to the
start
of the series, they could be merged right away reducing the number of patches
to review.
At the moment, they can't be because patch 3 placed in-between is strictly
related to the series.
@julien?
~Michal
Re: [XEN][PATCH v6 02/19] common/device_tree: handle memory allocation failure in __unflatten_device_tree()
On 03/05/2023 01:36, Vikram Garhwal wrote:
>
>
> Change __unflatten_device_tree() return type to integer so it can propagate
> memory allocation failure. Add panic() in dt_unflatten_host_device_tree() for
> memory allocation failure during boot.
>
> Signed-off-by: Vikram Garhwal
I think we are missing a Fixes tag.
> ---
> xen/common/device_tree.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 5f7ae45304..fc38a0b3dd 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -2056,8 +2056,8 @@ static unsigned long unflatten_dt_node(const void *fdt,
> * @fdt: The fdt to expand
> * @mynodes: The device_node tree created by the call
> */
> -static void __init __unflatten_device_tree(const void *fdt,
> - struct dt_device_node **mynodes)
> +static int __init __unflatten_device_tree(const void *fdt,
> + struct dt_device_node **mynodes)
> {
> unsigned long start, mem, size;
> struct dt_device_node **allnextp = mynodes;
> @@ -2078,6 +2078,8 @@ static void __init __unflatten_device_tree(const void
> *fdt,
>
> /* Allocate memory for the expanded device tree */
> mem = (unsigned long)_xmalloc (size + 4, __alignof__(struct
> dt_device_node));
> +if ( !mem )
> +return -ENOMEM;
>
> ((__be32 *)mem)[size / 4] = cpu_to_be32(0xdeadbeef);
>
> @@ -2095,6 +2097,8 @@ static void __init __unflatten_device_tree(const void
> *fdt,
> *allnextp = NULL;
>
> dt_dprintk(" <- unflatten_device_tree()\n");
> +
> +return 0;
> }
>
> static void dt_alias_add(struct dt_alias_prop *ap,
> @@ -2179,7 +2183,10 @@ dt_find_interrupt_controller(const struct
> dt_device_match *matches)
>
> void __init dt_unflatten_host_device_tree(void)
> {
> -__unflatten_device_tree(device_tree_flattened, &dt_host);
> +int error = __unflatten_device_tree(device_tree_flattened, &dt_host);
NIT: there should be a blank line between definitions and rest of the code
> +if ( error )
> +panic("__unflatten_device_tree failed with error %d\n", error);
> +
> dt_alias_scan();
> }
>
> --
> 2.17.1
>
>
FWICS, patches 2 and 4 are not strictly related to DTBO and are fixing issues
and propagating errors which is always good. Therefore by moving them to the
start
of the series, they could be merged right away reducing the number of patches
to review.
At the moment, they can't be because patch 3 placed in-between is strictly
related to the series.
@julien?
~Michal
RE: [XEN][PATCH v6 02/19] common/device_tree: handle memory allocation failure in __unflatten_device_tree()
Hi Vikram, > -Original Message- > Subject: [XEN][PATCH v6 02/19] common/device_tree: handle memory > allocation failure in __unflatten_device_tree() > > Change __unflatten_device_tree() return type to integer so it can propagate > memory allocation failure. Add panic() in dt_unflatten_host_device_tree() for > memory allocation failure during boot. > > Signed-off-by: Vikram Garhwal Reviewed-by: Henry Wang Kind regards, Henry
[XEN][PATCH v6 02/19] common/device_tree: handle memory allocation failure in __unflatten_device_tree()
Change __unflatten_device_tree() return type to integer so it can propagate
memory allocation failure. Add panic() in dt_unflatten_host_device_tree() for
memory allocation failure during boot.
Signed-off-by: Vikram Garhwal
---
xen/common/device_tree.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 5f7ae45304..fc38a0b3dd 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -2056,8 +2056,8 @@ static unsigned long unflatten_dt_node(const void *fdt,
* @fdt: The fdt to expand
* @mynodes: The device_node tree created by the call
*/
-static void __init __unflatten_device_tree(const void *fdt,
- struct dt_device_node **mynodes)
+static int __init __unflatten_device_tree(const void *fdt,
+ struct dt_device_node **mynodes)
{
unsigned long start, mem, size;
struct dt_device_node **allnextp = mynodes;
@@ -2078,6 +2078,8 @@ static void __init __unflatten_device_tree(const void
*fdt,
/* Allocate memory for the expanded device tree */
mem = (unsigned long)_xmalloc (size + 4, __alignof__(struct
dt_device_node));
+if ( !mem )
+return -ENOMEM;
((__be32 *)mem)[size / 4] = cpu_to_be32(0xdeadbeef);
@@ -2095,6 +2097,8 @@ static void __init __unflatten_device_tree(const void
*fdt,
*allnextp = NULL;
dt_dprintk(" <- unflatten_device_tree()\n");
+
+return 0;
}
static void dt_alias_add(struct dt_alias_prop *ap,
@@ -2179,7 +2183,10 @@ dt_find_interrupt_controller(const struct
dt_device_match *matches)
void __init dt_unflatten_host_device_tree(void)
{
-__unflatten_device_tree(device_tree_flattened, &dt_host);
+int error = __unflatten_device_tree(device_tree_flattened, &dt_host);
+if ( error )
+panic("__unflatten_device_tree failed with error %d\n", error);
+
dt_alias_scan();
}
--
2.17.1
