* [PATCH for-4.16] xen/arm: don't assign domU static-mem to dom0 as reserved-memory
@ 2021-11-09 0:48 Stefano Stabellini
2021-11-09 9:41 ` Julien Grall
2021-11-09 10:45 ` Ian Jackson
0 siblings, 2 replies; 4+ messages in thread
From: Stefano Stabellini @ 2021-11-09 0:48 UTC (permalink / raw)
To: julien
Cc: Penny.Zheng, Bertrand.Marquis, Wei.Chen, sstabellini, iwj,
Volodymyr_Babchuk, xen-devel, Stefano Stabellini
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.
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
This patch should be considered for 4.16 as it fixes an incorrect
behavior.
The risk is low for two reasons:
- the change is simple
- make_memory_node is easily tested because it gets called at every
boot, e.g. gitlab-ci and OSSTest exercise this path
I tested this patch successfully with and without xen,static-mem.
---
xen/arch/arm/domain_build.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 1fbafeaea8..56d3ff9d08 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -874,11 +874,17 @@ static int __init make_memory_node(const struct domain *d,
if ( mem->nr_banks == 0 )
return -ENOENT;
+ for ( i = 0; i < mem->nr_banks && mem->bank[i].xen_domain; i++ )
+ ;
+ /* No reserved-memory ranges to expose to Dom0 */
+ if ( i == mem->nr_banks )
+ return 0;
+
dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",
reg_size, nr_cells);
/* 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;
@@ -888,11 +894,14 @@ static int __init make_memory_node(const struct domain *d,
return res;
cells = ®[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);
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.16] xen/arm: don't assign domU static-mem to dom0 as reserved-memory
2021-11-09 0:48 [PATCH for-4.16] xen/arm: don't assign domU static-mem to dom0 as reserved-memory Stefano Stabellini
@ 2021-11-09 9:41 ` Julien Grall
2021-11-09 23:19 ` Stefano Stabellini
2021-11-09 10:45 ` Ian Jackson
1 sibling, 1 reply; 4+ messages in thread
From: Julien Grall @ 2021-11-09 9:41 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Penny.Zheng, Bertrand.Marquis, Wei.Chen, iwj, Volodymyr_Babchuk,
xen-devel, Stefano Stabellini
Hi Stefano,
On 09/11/2021 00:48, 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.
>
This is a bug fix. So please add a Fixes tag. In this case, I think it
should be:
Fixes: 41c031ff437b ("xen/arm: introduce domain on Static Allocation")
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> ---
>
> This patch should be considered for 4.16 as it fixes an incorrect
> behavior.
>
> The risk is low for two reasons:
> - the change is simple
> - make_memory_node is easily tested because it gets called at every
> boot, e.g. gitlab-ci and OSSTest exercise this path
>
> I tested this patch successfully with and without xen,static-mem.
>
> ---
> xen/arch/arm/domain_build.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 1fbafeaea8..56d3ff9d08 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -874,11 +874,17 @@ static int __init make_memory_node(const struct domain *d,
> if ( mem->nr_banks == 0 )
> return -ENOENT;
>
> + for ( i = 0; i < mem->nr_banks && mem->bank[i].xen_domain; i++ )
> + ;
> + /* No reserved-memory ranges to expose to Dom0 */
I find this comment a bit misleading because make_memory_node() will
also be called to create normal memory region. I would drop
"reserved-memory" and add a comment on top of the loop explaining what
the loop does.
> + if ( i == mem->nr_banks )
> + return 0;
> +
> dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",
> reg_size, nr_cells);
I think you need to adjust nr_cells otherwise we would write garbagge in
the DT if we need to exclude some regions.
>
> /* 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;
> @@ -888,11 +894,14 @@ static int __init make_memory_node(const struct domain *d,
> return res;
>
> cells = ®[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);
>
>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.16] xen/arm: don't assign domU static-mem to dom0 as reserved-memory
2021-11-09 9:41 ` Julien Grall
@ 2021-11-09 23:19 ` Stefano Stabellini
0 siblings, 0 replies; 4+ messages in thread
From: Stefano Stabellini @ 2021-11-09 23:19 UTC (permalink / raw)
To: Julien Grall
Cc: Stefano Stabellini, Penny.Zheng, Bertrand.Marquis, Wei.Chen, iwj,
Volodymyr_Babchuk, xen-devel, Stefano Stabellini
On Tue, 9 Nov 2021, Julien Grall wrote:
> On 09/11/2021 00:48, 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.
> >
>
> This is a bug fix. So please add a Fixes tag. In this case, I think it should
> be:
>
> Fixes: 41c031ff437b ("xen/arm: introduce domain on Static Allocation")
Thanks, will add
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > ---
> >
> > This patch should be considered for 4.16 as it fixes an incorrect
> > behavior.
> >
> > The risk is low for two reasons:
> > - the change is simple
> > - make_memory_node is easily tested because it gets called at every
> > boot, e.g. gitlab-ci and OSSTest exercise this path
> >
> > I tested this patch successfully with and without xen,static-mem.
> >
> > ---
> > xen/arch/arm/domain_build.c | 13 +++++++++++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 1fbafeaea8..56d3ff9d08 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -874,11 +874,17 @@ static int __init make_memory_node(const struct domain
> > *d,
> > if ( mem->nr_banks == 0 )
> > return -ENOENT;
> > + for ( i = 0; i < mem->nr_banks && mem->bank[i].xen_domain; i++ )
> > + ;
> > + /* No reserved-memory ranges to expose to Dom0 */
> I find this comment a bit misleading because make_memory_node() will also be
> called to create normal memory region. I would drop "reserved-memory" and add
> a comment on top of the loop explaining what the loop does.
Yeah, I agree, I moved it and changed it
> > + if ( i == mem->nr_banks )
> > + return 0;
> > +
> > dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",
> > reg_size, nr_cells);
>
> I think you need to adjust nr_cells otherwise we would write garbagge in the
> DT if we need to exclude some regions.
Good point! Fixed in the next version
> > /* 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;
> > @@ -888,11 +894,14 @@ static int __init make_memory_node(const struct domain
> > *d,
> > return res;
> > cells = ®[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);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.16] xen/arm: don't assign domU static-mem to dom0 as reserved-memory
2021-11-09 0:48 [PATCH for-4.16] xen/arm: don't assign domU static-mem to dom0 as reserved-memory Stefano Stabellini
2021-11-09 9:41 ` Julien Grall
@ 2021-11-09 10:45 ` Ian Jackson
1 sibling, 0 replies; 4+ messages in thread
From: Ian Jackson @ 2021-11-09 10:45 UTC (permalink / raw)
To: Stefano Stabellini
Cc: julien, Penny.Zheng, Bertrand.Marquis, Wei.Chen, iwj,
Volodymyr_Babchuk, xen-devel, Stefano Stabellini
Stefano Stabellini writes ("[PATCH for-4.16] 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.
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-11-09 23:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-09 0:48 [PATCH for-4.16] xen/arm: don't assign domU static-mem to dom0 as reserved-memory Stefano Stabellini
2021-11-09 9:41 ` Julien Grall
2021-11-09 23:19 ` Stefano Stabellini
2021-11-09 10:45 ` Ian Jackson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).