Hi Luca, On 09/04/2024 13:45, Luca Fancellu wrote: > > > From: Penny Zheng <penny.zh...@arm.com> > > Static shared memory acts as reserved memory in guest, so it shall be > excluded from extended regions. > > Extended regions are taken care of under three different scenarios: > normal DomU, direct-map domain with iommu on, and direct-map domain > with iommu off. > > For normal DomU, we create a new function "remove_shm_holes_for_domU", > to firstly transfer original outputs into the format of > "struct rangeset", then use "remove_shm_from_rangeset" to remove static > shm from them. > > For direct-map domain with iommu on, after we get guest shm info from "kinfo", > we use "remove_shm_from_rangeset" to remove static shm. > > For direct-map domain with iommu off, as static shm has already been taken > care of through reserved memory banks, we do nothing. Stale info given that shmem is no longer part of reserved memory banks. It's been taken care of by removing shmem regions in find_unallocated_memory()
> > Signed-off-by: Penny Zheng <penny.zh...@arm.com> > Signed-off-by: Luca Fancellu <luca.fance...@arm.com> > --- > v2: > - Fixed commit title, fixed comment, moved call of remove_shm_from_rangeset > after populating rangeset in find_memory_holes, print error code when > possible > - used PFN_DOWN where needed > v1: > - Rework of > https://patchwork.kernel.org/project/xen-devel/patch/20231206090623.1932275-8-penny.zh...@arm.com/ > --- > --- > xen/arch/arm/domain_build.c | 16 ++++- > xen/arch/arm/include/asm/domain_build.h | 2 + > xen/arch/arm/include/asm/static-shmem.h | 18 +++++ > xen/arch/arm/static-shmem.c | 88 +++++++++++++++++++++++++ > 4 files changed, 121 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 01d66fbde92b..9b36d6bb70c9 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -817,8 +817,8 @@ int __init make_memory_node(const struct domain *d, > return res; > } > > -static int __init add_ext_regions(unsigned long s_gfn, unsigned long e_gfn, > - void *data) > +int __init add_ext_regions(unsigned long s_gfn, unsigned long e_gfn, > + void *data) > { > struct membanks *ext_regions = data; > paddr_t start, size; > @@ -978,6 +978,8 @@ static int __init handle_pci_range(const struct > dt_device_node *dev, > * - MMIO > * - Host RAM > * - PCI aperture > + * - Static shared memory regions, which are described by special property > + * "xen,shared-mem" > */ > static int __init find_memory_holes(const struct kernel_info *kinfo, > struct membanks *ext_regions) > @@ -1005,6 +1007,11 @@ static int __init find_memory_holes(const struct > kernel_info *kinfo, > goto out; > } > > + /* Remove static shared memory regions */ > + res = remove_shm_from_rangeset(kinfo, mem_holes); > + if ( res ) > + goto out; > + > /* > * Remove regions described by "reg" and "ranges" properties where > * the memory is addressable (MMIO, RAM, PCI BAR, etc). > @@ -1097,7 +1104,10 @@ static int __init find_domU_holes(const struct > kernel_info *kinfo, > res = 0; > } > > - return res; > + if ( res ) > + return res; > + > + return remove_shm_holes_for_domU(kinfo, ext_regions); > } > > int __init make_hypervisor_node(struct domain *d, > diff --git a/xen/arch/arm/include/asm/domain_build.h > b/xen/arch/arm/include/asm/domain_build.h > index a6f276cc4263..026d975da28e 100644 > --- a/xen/arch/arm/include/asm/domain_build.h > +++ b/xen/arch/arm/include/asm/domain_build.h > @@ -51,6 +51,8 @@ static inline int prepare_acpi(struct domain *d, struct > kernel_info *kinfo) > int prepare_acpi(struct domain *d, struct kernel_info *kinfo); > #endif > > +int add_ext_regions(unsigned long s_gfn, unsigned long e_gfn, void *data); > + > #endif > > /* > diff --git a/xen/arch/arm/include/asm/static-shmem.h > b/xen/arch/arm/include/asm/static-shmem.h > index 90aafc81e740..2e8b138eb989 100644 > --- a/xen/arch/arm/include/asm/static-shmem.h > +++ b/xen/arch/arm/include/asm/static-shmem.h > @@ -28,6 +28,12 @@ void early_print_info_shmem(void); > > void init_sharedmem_pages(void); > > +int remove_shm_from_rangeset(const struct kernel_info *kinfo, > + struct rangeset *rangeset); > + > +int remove_shm_holes_for_domU(const struct kernel_info *kinfo, > + struct membanks *ext_regions); > + > #else /* !CONFIG_STATIC_SHM */ > > static inline int make_resv_memory_node(const struct kernel_info *kinfo, > @@ -59,6 +65,18 @@ static inline void early_print_info_shmem(void) {}; > > static inline void init_sharedmem_pages(void) {}; > > +static inline int remove_shm_from_rangeset(const struct kernel_info *kinfo, > + struct rangeset *rangeset) > +{ > + return 0; > +} > + > +static inline int remove_shm_holes_for_domU(const struct kernel_info *kinfo, > + struct membanks *ext_regions) > +{ > + return 0; > +} > + > #endif /* CONFIG_STATIC_SHM */ > > #endif /* __ASM_STATIC_SHMEM_H_ */ > diff --git a/xen/arch/arm/static-shmem.c b/xen/arch/arm/static-shmem.c > index fcf8227bf404..a08413ff44e2 100644 > --- a/xen/arch/arm/static-shmem.c > +++ b/xen/arch/arm/static-shmem.c > @@ -1,6 +1,7 @@ > /* SPDX-License-Identifier: GPL-2.0-only */ > > #include <xen/libfdt/libfdt.h> > +#include <xen/rangeset.h> > #include <xen/sched.h> > > #include <asm/domain_build.h> > @@ -574,6 +575,93 @@ void __init init_sharedmem_pages(void) > init_staticmem_bank(&shmem->bank[bank]); > } > > +int __init remove_shm_from_rangeset(const struct kernel_info *kinfo, > + struct rangeset *rangeset) > +{ > + const struct membanks *shm_mem = &kinfo->shm_mem.common; > + unsigned int i; > + > + /* Remove static shared memory regions */ > + for ( i = 0; i < shm_mem->nr_banks; i++ ) > + { > + paddr_t start, end; > + int res; > + > + start = shm_mem->bank[i].start; > + end = shm_mem->bank[i].start + shm_mem->bank[i].size - 1; If you look at other rangeset_remove_range use cases and error messages, 1 is subtracted in PFN_DOWN() so that the error message contains end unchanged. Please adhere to that so that printed messages are consistent. > + res = rangeset_remove_range(rangeset, PFN_DOWN(start), > PFN_DOWN(end)); > + if ( res ) > + { > + printk(XENLOG_ERR > + "Failed to remove: %#"PRIpaddr"->%#"PRIpaddr", error: > %d\n", > + start, end, res); > + return -EINVAL; > + } > + } > + > + return 0; > +} > + > +int __init remove_shm_holes_for_domU(const struct kernel_info *kinfo, > + struct membanks *ext_regions) > +{ > + const struct membanks *shm_mem = &kinfo->shm_mem.common; > + struct rangeset *guest_holes; > + unsigned int i; > + paddr_t start; > + paddr_t end; > + int res; > + > + /* No static shared memory region. */ > + if ( shm_mem->nr_banks == 0 ) > + return 0; > + > + dt_dprintk("Remove static shared memory holes from extended regions of > DomU\n"); > + > + guest_holes = rangeset_new(NULL, NULL, 0); > + if ( !guest_holes ) > + return -ENOMEM; > + > + /* Copy extended regions sets into the rangeset */ > + for ( i = 0; i < ext_regions->nr_banks; i++ ) > + { > + start = ext_regions->bank[i].start; > + end = start + ext_regions->bank[i].size - 1; > + > + res = rangeset_add_range(guest_holes, PFN_DOWN(start), > PFN_DOWN(end)); same remark about the end > + if ( res ) > + { > + printk(XENLOG_ERR > + "Failed to add: %#"PRIpaddr"->%#"PRIpaddr", error: %d\n", > + start, end, res); > + goto out; > + } > + } > + > + /* Remove static shared memory regions */ > + res = remove_shm_from_rangeset(kinfo, guest_holes); > + if ( res ) > + goto out; > + Could you please add a comment explaining here what's done below? > + i = ext_regions->nr_banks - 1; > + start = ext_regions->bank[0].start; > + end = ext_regions->bank[i].start + ext_regions->bank[i].size - 1; > + > + /* Reset original extended regions to hold new value */ > + ext_regions->nr_banks = 0; > + res = rangeset_report_ranges(guest_holes, PFN_DOWN(start), PFN_DOWN(end), > + add_ext_regions, ext_regions); > + if ( res ) > + ext_regions->nr_banks = 0; > + else if ( !ext_regions->nr_banks ) > + res = -ENOENT; > + > + out: > + rangeset_destroy(guest_holes); > + > + return res; > +} > + > /* > * Local variables: > * mode: C > -- > 2.34.1 > ~Michal
