1. Patchew
  2. Xen
  3. View series

[PATCH v3] xen/arm: don't assign domU static-mem to dom0 as reserved-memory

Stefano Stabellini posted 1 patch 2 years, 5 months ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20211110201812.934306-1-sstabellini@kernel.org
xen/arch/arm/domain_build.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
[PATCH v3] xen/arm: don't assign domU static-mem to dom0 as reserved-memory
Posted by Stefano Stabellini 2 years, 5 months ago 
From: Stefano Stabellini <stefano.stabellini@xilinx.com>

DomUs static-mem ranges are added to the reserved_mem array for
accounting, but they shouldn't be assigned to dom0 as the other regular
reserved-memory ranges in device tree.

In make_memory_nodes, fix the error by skipping banks with xen_domain
set to true in the reserved-memory array. Also make sure to use the
first valid (!xen_domain) start address for the memory node name.

Fixes: 41c031ff437b ("xen/arm: introduce domain on Static Allocation")
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
---
Changes in v3:
- move BUG_ON inside the loop

Changes in v2:
- improve commit message
- improve in-code comment
- update nr_cells appropriately
---
 xen/arch/arm/domain_build.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9e92b640cd..dafbc13962 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -862,21 +862,25 @@ static int __init make_memory_node(const struct domain *d,
 {
     int res, i;
     int reg_size = addrcells + sizecells;
-    int nr_cells = reg_size * mem->nr_banks;
+    int nr_cells = 0;
     /* Placeholder for memory@ + a 64-bit number + \0 */
     char buf[24];
     __be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells */];
     __be32 *cells;
 
-    BUG_ON(nr_cells >= ARRAY_SIZE(reg));
     if ( mem->nr_banks == 0 )
         return -ENOENT;
 
-    dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",
-               reg_size, nr_cells);
+    /* find first memory range not bound to a Xen domain */
+    for ( i = 0; i < mem->nr_banks && mem->bank[i].xen_domain; i++ )
+        ;
+    if ( i == mem->nr_banks )
+        return 0;
+
+    dt_dprintk("Create memory node\n");
 
     /* ePAPR 3.4 */
-    snprintf(buf, sizeof(buf), "memory@%"PRIx64, mem->bank[0].start);
+    snprintf(buf, sizeof(buf), "memory@%"PRIx64, mem->bank[i].start);
     res = fdt_begin_node(fdt, buf);
     if ( res )
         return res;
@@ -886,17 +890,24 @@ static int __init make_memory_node(const struct domain *d,
         return res;
 
     cells = &reg[0];
-    for ( i = 0 ; i < mem->nr_banks; i++ )
+    for ( ; i < mem->nr_banks; i++ )
     {
         u64 start = mem->bank[i].start;
         u64 size = mem->bank[i].size;
 
+        if ( mem->bank[i].xen_domain )
+            continue;
+
         dt_dprintk("  Bank %d: %#"PRIx64"->%#"PRIx64"\n",
                    i, start, start + size);
 
+        nr_cells += reg_size;
+        BUG_ON(nr_cells >= ARRAY_SIZE(reg));
         dt_child_set_range(&cells, addrcells, sizecells, start, size);
     }
 
+    dt_dprintk("(reg size %d, nr cells %d)\n", reg_size, nr_cells);
+
     res = fdt_property(fdt, "reg", reg, nr_cells * sizeof(*reg));
     if ( res )
         return res;
-- 
2.25.1
Re: [PATCH v3] xen/arm: don't assign domU static-mem to dom0 as reserved-memory
Posted by Julien Grall 2 years, 5 months ago 
Hi Stefano,

On 10/11/2021 20:18, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> 
> DomUs static-mem ranges are added to the reserved_mem array for
> accounting, but they shouldn't be assigned to dom0 as the other regular
> reserved-memory ranges in device tree.
> 
> In make_memory_nodes, fix the error by skipping banks with xen_domain
> set to true in the reserved-memory array. Also make sure to use the
> first valid (!xen_domain) start address for the memory node name.
> 
> Fixes: 41c031ff437b ("xen/arm: introduce domain on Static Allocation")
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> Release-Acked-by: Ian Jackson <iwj@xenproject.org>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

> ---
> Changes in v3:
> - move BUG_ON inside the loop
> 
> Changes in v2:
> - improve commit message
> - improve in-code comment
> - update nr_cells appropriately
> ---
>   xen/arch/arm/domain_build.c | 23 +++++++++++++++++------
>   1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 9e92b640cd..dafbc13962 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -862,21 +862,25 @@ static int __init make_memory_node(const struct domain *d,
>   {
>       int res, i;
>       int reg_size = addrcells + sizecells;
> -    int nr_cells = reg_size * mem->nr_banks;
> +    int nr_cells = 0;
>       /* Placeholder for memory@ + a 64-bit number + \0 */
>       char buf[24];
>       __be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells */];
>       __be32 *cells;
>   
> -    BUG_ON(nr_cells >= ARRAY_SIZE(reg));
>       if ( mem->nr_banks == 0 )
>           return -ENOENT;
>   
> -    dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",
> -               reg_size, nr_cells);
> +    /* find first memory range not bound to a Xen domain */
> +    for ( i = 0; i < mem->nr_banks && mem->bank[i].xen_domain; i++ )
> +        ;
> +    if ( i == mem->nr_banks )
> +        return 0;
> +
> +    dt_dprintk("Create memory node\n");
>   
>       /* ePAPR 3.4 */
> -    snprintf(buf, sizeof(buf), "memory@%"PRIx64, mem->bank[0].start);
> +    snprintf(buf, sizeof(buf), "memory@%"PRIx64, mem->bank[i].start);
>       res = fdt_begin_node(fdt, buf);
>       if ( res )
>           return res;
> @@ -886,17 +890,24 @@ static int __init make_memory_node(const struct domain *d,
>           return res;
>   
>       cells = &reg[0];
> -    for ( i = 0 ; i < mem->nr_banks; i++ )
> +    for ( ; i < mem->nr_banks; i++ )
>       {
>           u64 start = mem->bank[i].start;
>           u64 size = mem->bank[i].size;
>   
> +        if ( mem->bank[i].xen_domain )
> +            continue;
> +
>           dt_dprintk("  Bank %d: %#"PRIx64"->%#"PRIx64"\n",
>                      i, start, start + size);
>   
> +        nr_cells += reg_size;
> +        BUG_ON(nr_cells >= ARRAY_SIZE(reg));
>           dt_child_set_range(&cells, addrcells, sizecells, start, size);
>       }
>   
> +    dt_dprintk("(reg size %d, nr cells %d)\n", reg_size, nr_cells);
> +
>       res = fdt_property(fdt, "reg", reg, nr_cells * sizeof(*reg));
>       if ( res )
>           return res;
> 

-- 
Julien Grall
Re: [PATCH v3] xen/arm: don't assign domU static-mem to dom0 as reserved-memory
Posted by Ian Jackson 2 years, 5 months ago 
Julien Grall writes ("Re: [PATCH v3] xen/arm: don't assign domU static-mem to dom0 as reserved-memory"):
> Hi Stefano,
> 
> On 10/11/2021 20:18, Stefano Stabellini wrote:
> > From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > 
> > DomUs static-mem ranges are added to the reserved_mem array for
> > accounting, but they shouldn't be assigned to dom0 as the other regular
> > reserved-memory ranges in device tree.
> > 
> > In make_memory_nodes, fix the error by skipping banks with xen_domain
> > set to true in the reserved-memory array. Also make sure to use the
> > first valid (!xen_domain) start address for the memory node name.
> > 
> > Fixes: 41c031ff437b ("xen/arm: introduce domain on Static Allocation")
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > Release-Acked-by: Ian Jackson <iwj@xenproject.org>
> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>

FTAOD: I don't see a for-4.16 tag here.  So I think this is targeted
for post-4.16 ?  But I wonder if it ought to be considered.  I don't
understand the impact of the bug that is being fixed here.

Thanks,
Ian.
Re: [PATCH v3] xen/arm: don't assign domU static-mem to dom0 as reserved-memory
Posted by Julien Grall 2 years, 5 months ago 
Hi Ian,

On 11/11/2021 13:33, Ian Jackson wrote:
> Julien Grall writes ("Re: [PATCH v3] xen/arm: don't assign domU static-mem to dom0 as reserved-memory"):
>> Hi Stefano,
>>
>> On 10/11/2021 20:18, Stefano Stabellini wrote:
>>> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
>>>
>>> DomUs static-mem ranges are added to the reserved_mem array for
>>> accounting, but they shouldn't be assigned to dom0 as the other regular
>>> reserved-memory ranges in device tree.
>>>
>>> In make_memory_nodes, fix the error by skipping banks with xen_domain
>>> set to true in the reserved-memory array. Also make sure to use the
>>> first valid (!xen_domain) start address for the memory node name.
>>>
>>> Fixes: 41c031ff437b ("xen/arm: introduce domain on Static Allocation")
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
>>> Release-Acked-by: Ian Jackson <iwj@xenproject.org>
>>
>> Reviewed-by: Julien Grall <jgrall@amazon.com>
> 
> FTAOD: I don't see a for-4.16 tag here.  So I think this is targeted
> for post-4.16 ?  But I wonder if it ought to be considered.  I don't
> understand the impact of the bug that is being fixed here.

The first version of the patch [1] was tagged with for-4.16 and contains 
the rationale.

I was thinking to commit it because it already contains your 
release-acked-by. Can you let me know if it still stands?

Cheers,

[1] 
https://lore.kernel.org/xen-devel/24970.20802.96908.223297@mariner.uk.xensource.com/T/#m058f2d243f6670ef48e77f40c25ac0115f0dae74

> 
> Thanks,
> Ian.
> 

-- 
Julien Grall
Re: [PATCH v3] xen/arm: don't assign domU static-mem to dom0 as reserved-memory
Posted by Ian Jackson 2 years, 5 months ago 
Julien Grall writes ("Re: [PATCH v3] xen/arm: don't assign domU static-mem to dom0 as reserved-memory"):
> The first version of the patch [1] was tagged with for-4.16 and contains 
> the rationale.
> 
> I was thinking to commit it because it already contains your 
> release-acked-by. Can you let me know if it still stands?

Oh.  Please go ahead.  I'm sorry to be confused; I rely on the
computer as my external storage :-).

Ian.
RE: [PATCH v3] xen/arm: don't assign domU static-mem to dom0 as reserved-memory
Posted by Penny Zheng 2 years, 5 months ago 
Hi Stefano

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@kernel.org>
> Sent: Thursday, November 11, 2021 4:18 AM
> To: julien@xen.org
> Cc: sstabellini@kernel.org; Penny Zheng <Penny.Zheng@arm.com>; Bertrand
> Marquis <Bertrand.Marquis@arm.com>; Wei Chen <Wei.Chen@arm.com>;
> iwj@xenproject.org; Volodymyr_Babchuk@epam.com; xen-
> devel@lists.xenproject.org; Stefano Stabellini <stefano.stabellini@xilinx.com>
> Subject: [PATCH v3] xen/arm: don't assign domU static-mem to dom0 as
> reserved-memory
> 
> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> 
> DomUs static-mem ranges are added to the reserved_mem array for
> accounting, but they shouldn't be assigned to dom0 as the other regular
> reserved-memory ranges in device tree.
> 
> In make_memory_nodes, fix the error by skipping banks with xen_domain set
> to true in the reserved-memory array. Also make sure to use the first valid
> (!xen_domain) start address for the memory node name.
> 
> Fixes: 41c031ff437b ("xen/arm: introduce domain on Static Allocation")
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> Release-Acked-by: Ian Jackson <iwj@xenproject.org>

Thx again for the fixing up.

Reviewed-by: Penny Zheng <penny.zheng@arm.com>
> ---
> Changes in v3:
> - move BUG_ON inside the loop
> 
> Changes in v2:
> - improve commit message
> - improve in-code comment
> - update nr_cells appropriately
> ---
>  xen/arch/arm/domain_build.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 9e92b640cd..dafbc13962 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -862,21 +862,25 @@ static int __init make_memory_node(const struct
> domain *d,  {
>      int res, i;
>      int reg_size = addrcells + sizecells;
> -    int nr_cells = reg_size * mem->nr_banks;
> +    int nr_cells = 0;
>      /* Placeholder for memory@ + a 64-bit number + \0 */
>      char buf[24];
>      __be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells */];
>      __be32 *cells;
> 
> -    BUG_ON(nr_cells >= ARRAY_SIZE(reg));
>      if ( mem->nr_banks == 0 )
>          return -ENOENT;
> 
> -    dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",
> -               reg_size, nr_cells);
> +    /* find first memory range not bound to a Xen domain */
> +    for ( i = 0; i < mem->nr_banks && mem->bank[i].xen_domain; i++ )
> +        ;
> +    if ( i == mem->nr_banks )
> +        return 0;
> +
> +    dt_dprintk("Create memory node\n");
> 
>      /* ePAPR 3.4 */
> -    snprintf(buf, sizeof(buf), "memory@%"PRIx64, mem->bank[0].start);
> +    snprintf(buf, sizeof(buf), "memory@%"PRIx64, mem->bank[i].start);
>      res = fdt_begin_node(fdt, buf);
>      if ( res )
>          return res;
> @@ -886,17 +890,24 @@ static int __init make_memory_node(const struct
> domain *d,
>          return res;
> 
>      cells = &reg[0];
> -    for ( i = 0 ; i < mem->nr_banks; i++ )
> +    for ( ; i < mem->nr_banks; i++ )
>      {
>          u64 start = mem->bank[i].start;
>          u64 size = mem->bank[i].size;
> 
> +        if ( mem->bank[i].xen_domain )
> +            continue;
> +
>          dt_dprintk("  Bank %d: %#"PRIx64"->%#"PRIx64"\n",
>                     i, start, start + size);
> 
> +        nr_cells += reg_size;
> +        BUG_ON(nr_cells >= ARRAY_SIZE(reg));
>          dt_child_set_range(&cells, addrcells, sizecells, start, size);
>      }
> 
> +    dt_dprintk("(reg size %d, nr cells %d)\n", reg_size, nr_cells);
> +
>      res = fdt_property(fdt, "reg", reg, nr_cells * sizeof(*reg));
>      if ( res )
>          return res;
> --
> 2.25.1

--
Cheers,
Penny Zheng

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.