| Message ID | 1556658172-8824-9-git-send-email-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,01/10] xen: add a p2mt parameter to map_mmio_regions | expand |
Hi, On 4/30/19 10:02 PM, Stefano Stabellini wrote: > reserved-memory regions should be mapped as normal memory. At the > moment, they get remapped as device memory in dom0 because Xen doesn't > know any better. Add an explicit check for it. This part matches the title of the patch but... > > reserved-memory regions overlap with memory nodes. The overlapping > memory is reserved-memory and should be handled accordingly: > consider_modules and dt_unreserved_regions should skip these regions the > same way they are already skipping mem-reserve regions. ... this doesn't. They are actually two different things and should be handled in separate patches. > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > --- > Changes in v2: > - fix commit message: full overlap > - remove check_reserved_memory > - extend consider_modules and dt_unreserved_regions > --- > xen/arch/arm/domain_build.c | 7 +++++++ > xen/arch/arm/setup.c | 36 +++++++++++++++++++++++++++++++++--- > 2 files changed, 40 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 5e7f94c..e5d488d 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -1408,6 +1408,13 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo, > "WARNING: Path %s is reserved, skip the node as we may re-use the path.\n", > path); > > + /* > + * reserved-memory ranges should be mapped as normal memory in the > + * p2m. > + */ > + if ( !strcmp(dt_node_name(node), "reserved-memory") ) > + p2mt = p2m_mmio_direct_c; Do we really need this? The default type is already p2m_mmio_direct_c (see default_p2mt). > + > res = handle_device(d, node, p2mt); > if ( res) > return res; > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index ccb0f18..908b52c 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -204,6 +204,19 @@ void __init dt_unreserved_regions(paddr_t s, paddr_t e, > } > } > > + for ( ; i - nr < bootinfo.reserved_mem.nr_banks; i++ ) It took me a bit of time to understand why you do i - nr. I think we need some comments explaining the new logic. Longer term (i.e I will not push for it today :)), I think this code would benefits of using e820-like. It would make the code clearer and probably more efficient than what we currently have. > + { > + paddr_t r_s = bootinfo.reserved_mem.bank[i - nr].start; > + paddr_t r_e = r_s + bootinfo.reserved_mem.bank[i - nr].size; > + > + if ( s < r_e && r_s < e ) > + { > + dt_unreserved_regions(r_e, e, cb, i+1); > + dt_unreserved_regions(s, r_s, cb, i+1); > + return; > + } > + } > + > cb(s, e); > } > > @@ -390,7 +403,7 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e, > { > const struct bootmodules *mi = &bootinfo.modules; > int i; > - int nr_rsvd; > + int nr; > > s = (s+align-1) & ~(align-1); > e = e & ~(align-1); > @@ -416,9 +429,9 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e, > > /* Now check any fdt reserved areas. */ > > - nr_rsvd = fdt_num_mem_rsv(device_tree_flattened); > + nr = fdt_num_mem_rsv(device_tree_flattened); > > - for ( ; i < mi->nr_mods + nr_rsvd; i++ ) > + for ( ; i < mi->nr_mods + nr; i++ ) > { > paddr_t mod_s, mod_e; > > @@ -440,6 +453,23 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e, > return consider_modules(s, mod_s, size, align, i+1); > } > } > + > + /* Now check for reserved-memory regions */ > + nr += mi->nr_mods; Similar to the previous function, this needs to be documented. > + for ( ; i - nr < bootinfo.reserved_mem.nr_banks; i++ ) > + { > + paddr_t r_s = bootinfo.reserved_mem.bank[i - nr].start; > + paddr_t r_e = r_s + bootinfo.reserved_mem.bank[i - nr].size; > + > + if ( s < r_e && r_s < e ) > + { > + r_e = consider_modules(r_e, e, size, align, i+1); Coding style: space before and after the operator. Ideally, the rest of the function should be fixed. > + if ( r_e ) > + return r_e; > + > + return consider_modules(s, r_s, size, align, i+1); Same here. > + } > + } > return e; > } > #endif > Cheers,
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 5e7f94c..e5d488d 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1408,6 +1408,13 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo, "WARNING: Path %s is reserved, skip the node as we may re-use the path.\n", path); + /* + * reserved-memory ranges should be mapped as normal memory in the + * p2m. + */ + if ( !strcmp(dt_node_name(node), "reserved-memory") ) + p2mt = p2m_mmio_direct_c; + res = handle_device(d, node, p2mt); if ( res) return res; diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index ccb0f18..908b52c 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -204,6 +204,19 @@ void __init dt_unreserved_regions(paddr_t s, paddr_t e, } } + for ( ; i - nr < bootinfo.reserved_mem.nr_banks; i++ ) + { + paddr_t r_s = bootinfo.reserved_mem.bank[i - nr].start; + paddr_t r_e = r_s + bootinfo.reserved_mem.bank[i - nr].size; + + if ( s < r_e && r_s < e ) + { + dt_unreserved_regions(r_e, e, cb, i+1); + dt_unreserved_regions(s, r_s, cb, i+1); + return; + } + } + cb(s, e); } @@ -390,7 +403,7 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e, { const struct bootmodules *mi = &bootinfo.modules; int i; - int nr_rsvd; + int nr; s = (s+align-1) & ~(align-1); e = e & ~(align-1); @@ -416,9 +429,9 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e, /* Now check any fdt reserved areas. */ - nr_rsvd = fdt_num_mem_rsv(device_tree_flattened); + nr = fdt_num_mem_rsv(device_tree_flattened); - for ( ; i < mi->nr_mods + nr_rsvd; i++ ) + for ( ; i < mi->nr_mods + nr; i++ ) { paddr_t mod_s, mod_e; @@ -440,6 +453,23 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e, return consider_modules(s, mod_s, size, align, i+1); } } + + /* Now check for reserved-memory regions */ + nr += mi->nr_mods; + for ( ; i - nr < bootinfo.reserved_mem.nr_banks; i++ ) + { + paddr_t r_s = bootinfo.reserved_mem.bank[i - nr].start; + paddr_t r_e = r_s + bootinfo.reserved_mem.bank[i - nr].size; + + if ( s < r_e && r_s < e ) + { + r_e = consider_modules(r_e, e, size, align, i+1); + if ( r_e ) + return r_e; + + return consider_modules(s, r_s, size, align, i+1); + } + } return e; } #endif
reserved-memory regions should be mapped as normal memory. At the moment, they get remapped as device memory in dom0 because Xen doesn't know any better. Add an explicit check for it. reserved-memory regions overlap with memory nodes. The overlapping memory is reserved-memory and should be handled accordingly: consider_modules and dt_unreserved_regions should skip these regions the same way they are already skipping mem-reserve regions. Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> --- Changes in v2: - fix commit message: full overlap - remove check_reserved_memory - extend consider_modules and dt_unreserved_regions --- xen/arch/arm/domain_build.c | 7 +++++++ xen/arch/arm/setup.c | 36 +++++++++++++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 3 deletions(-)