| Message ID | 20211109231817.587168-1-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] xen/arm: don't assign domU static-mem to dom0 as reserved-memory | expand |
Hi Stefano, On 09/11/2021 23: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> > > --- > 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..c81a2bdfef 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)); The goal of this BUG_ON() was to catch potential overrun of the buffer before it is actually used. Now... > 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 = ®[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; > dt_child_set_range(&cells, addrcells, sizecells, start, size); > } > > + dt_dprintk("(reg size %d, nr cells %d)\n", reg_size, nr_cells); > + BUG_ON(nr_cells >= ARRAY_SIZE(reg)); ... this is moved after we write to the array. Which means the badness already happened and the BUG_ON() becomes pointless. I would still like to have the BUG_ON() in place. So I would move it in the loop just after nr_cells is updated. > + > res = fdt_property(fdt, "reg", reg, nr_cells * sizeof(*reg)); > if ( res ) > return res; >
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 9e92b640cd..c81a2bdfef 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 = ®[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; dt_child_set_range(&cells, addrcells, sizecells, start, size); } + dt_dprintk("(reg size %d, nr cells %d)\n", reg_size, nr_cells); + BUG_ON(nr_cells >= ARRAY_SIZE(reg)); + res = fdt_property(fdt, "reg", reg, nr_cells * sizeof(*reg)); if ( res ) return res;