| Message ID | 20240312130331.78418-11-luca.fancellu@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Static shared memory followup v2 - pt1 | expand |
Hi Luca, On 12/03/2024 14:03, Luca Fancellu wrote: > > > From: Penny Zheng <Penny.Zheng@arm.com> > > In case there is a /reserved-memory node already present in the host dtb, > current Xen codes would create yet another /reserved-memory node when > the static shared memory feature is enabled and static shared memory > region are present, this would result in an incorrect device tree s/region/regions, please add full stop after present. > generation and guest would not be able to detect the static shared memory s/guest/hwdom to make it clear when the issue can be observed > region. > > Avoid this issue checking the presence of the /reserved-memory node and by checking > appending the nodes instead of generating a duplicate /reserved-memory. > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> > --- > v1: > - Rework of https://patchwork.kernel.org/project/xen-devel/patch/20231206090623.1932275-11-Penny.Zheng@arm.com/ > --- > xen/arch/arm/domain_build.c | 23 ++++++++++++++++++++--- > xen/arch/arm/include/asm/static-shmem.h | 11 +++++++++++ > xen/arch/arm/static-shmem.c | 12 +++++++----- > 3 files changed, 38 insertions(+), 8 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 740c483ea2db..575e906d81a6 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -1620,6 +1620,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo, > DT_MATCH_PATH("/hypervisor"), > { /* sentinel */ }, > }; > + static __initdata bool res_mem_node_found = false; > struct dt_device_node *child; > int res, i, nirq, irq_id; > const char *name; > @@ -1714,6 +1715,19 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo, > if ( res ) > return res; > > + if ( dt_node_path_is_equal(node, "/reserved-memory") ) > + { > + res_mem_node_found = true; > + /* > + * Avoid duplicate /reserved-memory nodes in Device Tree, so list the s/list/add > + * static shared memory nodes there. > + */ > + res = make_shm_memory_node(d, kinfo, dt_n_addr_cells(node), > + dt_n_size_cells(node)); > + if ( res ) > + return res; > + } > + > for ( child = node->child; child != NULL; child = child->sibling ) > { > res = handle_node(d, kinfo, child, p2mt); > @@ -1766,9 +1780,12 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo, > return res; > } > > - res = make_resv_memory_node(d, kinfo, addrcells, sizecells); > - if ( res ) > - return res; > + if ( !res_mem_node_found ) > + { > + res = make_resv_memory_node(d, kinfo, addrcells, sizecells); > + if ( res ) > + return res; > + } > } > > res = fdt_end_node(kinfo->fdt); > diff --git a/xen/arch/arm/include/asm/static-shmem.h b/xen/arch/arm/include/asm/static-shmem.h > index 2f70aed53ac7..d28b9540d49b 100644 > --- a/xen/arch/arm/include/asm/static-shmem.h > +++ b/xen/arch/arm/include/asm/static-shmem.h > @@ -35,6 +35,10 @@ int remove_shm_from_rangeset(const struct kernel_info *kinfo, > int remove_shm_holes_for_domU(const struct kernel_info *kinfo, > struct membanks *ext_regions); > > +int make_shm_memory_node(const struct domain *d, > + const struct kernel_info *kinfo, int addrcells, > + int sizecells); > + > #else /* !CONFIG_STATIC_SHM */ > > static inline int make_resv_memory_node(const struct domain *d, > @@ -79,6 +83,13 @@ static inline int remove_shm_holes_for_domU(const struct kernel_info *kinfo, > return 0; > } > > +static inline int make_shm_memory_node(const struct domain *d, > + const struct kernel_info *kinfo, > + int addrcells, int sizecells) > +{ > + 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 b3e2105dd3f2..67d5fa3b5d25 100644 > --- a/xen/arch/arm/static-shmem.c > +++ b/xen/arch/arm/static-shmem.c > @@ -287,15 +287,17 @@ int __init process_shm(struct domain *d, struct kernel_info *kinfo, > return 0; > } > > -static int __init make_shm_memory_node(const struct domain *d, void *fdt, > - int addrcells, int sizecells, > - const struct membanks *mem) > +int __init make_shm_memory_node(const struct domain *d, > + const struct kernel_info *kinfo, int addrcells, > + int sizecells) Strictly speaking, changing the function signature is not mandatory for this change, so I would at least mention it in the commit msg. Other than that: Reviewed-by: Michal Orzel <michal.orzel@amd.com> ~Michal
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 740c483ea2db..575e906d81a6 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1620,6 +1620,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo, DT_MATCH_PATH("/hypervisor"), { /* sentinel */ }, }; + static __initdata bool res_mem_node_found = false; struct dt_device_node *child; int res, i, nirq, irq_id; const char *name; @@ -1714,6 +1715,19 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo, if ( res ) return res; + if ( dt_node_path_is_equal(node, "/reserved-memory") ) + { + res_mem_node_found = true; + /* + * Avoid duplicate /reserved-memory nodes in Device Tree, so list the + * static shared memory nodes there. + */ + res = make_shm_memory_node(d, kinfo, dt_n_addr_cells(node), + dt_n_size_cells(node)); + if ( res ) + return res; + } + for ( child = node->child; child != NULL; child = child->sibling ) { res = handle_node(d, kinfo, child, p2mt); @@ -1766,9 +1780,12 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo, return res; } - res = make_resv_memory_node(d, kinfo, addrcells, sizecells); - if ( res ) - return res; + if ( !res_mem_node_found ) + { + res = make_resv_memory_node(d, kinfo, addrcells, sizecells); + if ( res ) + return res; + } } res = fdt_end_node(kinfo->fdt); diff --git a/xen/arch/arm/include/asm/static-shmem.h b/xen/arch/arm/include/asm/static-shmem.h index 2f70aed53ac7..d28b9540d49b 100644 --- a/xen/arch/arm/include/asm/static-shmem.h +++ b/xen/arch/arm/include/asm/static-shmem.h @@ -35,6 +35,10 @@ int remove_shm_from_rangeset(const struct kernel_info *kinfo, int remove_shm_holes_for_domU(const struct kernel_info *kinfo, struct membanks *ext_regions); +int make_shm_memory_node(const struct domain *d, + const struct kernel_info *kinfo, int addrcells, + int sizecells); + #else /* !CONFIG_STATIC_SHM */ static inline int make_resv_memory_node(const struct domain *d, @@ -79,6 +83,13 @@ static inline int remove_shm_holes_for_domU(const struct kernel_info *kinfo, return 0; } +static inline int make_shm_memory_node(const struct domain *d, + const struct kernel_info *kinfo, + int addrcells, int sizecells) +{ + 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 b3e2105dd3f2..67d5fa3b5d25 100644 --- a/xen/arch/arm/static-shmem.c +++ b/xen/arch/arm/static-shmem.c @@ -287,15 +287,17 @@ int __init process_shm(struct domain *d, struct kernel_info *kinfo, return 0; } -static int __init make_shm_memory_node(const struct domain *d, void *fdt, - int addrcells, int sizecells, - const struct membanks *mem) +int __init make_shm_memory_node(const struct domain *d, + const struct kernel_info *kinfo, int addrcells, + int sizecells) { + const struct membanks *mem = &kinfo->shm_mem.common; + void *fdt = kinfo->fdt; unsigned int i = 0; int res = 0; if ( mem->nr_banks == 0 ) - return -ENOENT; + return 0; /* * For each shared memory region, a range is exposed under @@ -534,7 +536,7 @@ int __init make_resv_memory_node(const struct domain *d, if ( res ) return res; - res = make_shm_memory_node(d, fdt, addrcells, sizecells, mem); + res = make_shm_memory_node(d, kinfo, addrcells, sizecells); if ( res ) return res;