The /reserved-memory node is inherently not specific to static-shmem.
Move it to a more generic domain build context. While at it, add an
empty kernel_info_get_shm_mem_const() for the CONFIG_STATIC_SHM=n case,
as it can now be invoked in such case.
No functional change.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
xen/arch/arm/domain_build.c | 40 +++++++++++++++++++++++++++
xen/common/device-tree/static-shmem.c | 40 ---------------------------
xen/include/xen/fdt-domain-build.h | 2 ++
xen/include/xen/static-shmem.h | 15 ++++------
4 files changed, 48 insertions(+), 49 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 5fbc26f70988..e063d0d4076e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1564,6 +1564,46 @@ int __init make_chosen_node(const struct kernel_info *kinfo)
return res;
}
+int __init make_resv_memory_node(const struct kernel_info *kinfo, int addrcells,
+ int sizecells)
+{
+ const struct membanks *mem = kernel_info_get_shm_mem_const(kinfo);
+ void *fdt = kinfo->fdt;
+ int res = 0;
+ /* Placeholder for reserved-memory\0 */
+ const char resvbuf[16] = "reserved-memory";
+
+ if ( !mem || mem->nr_banks == 0 )
+ /* No shared memory provided. */
+ return 0;
+
+ dt_dprintk("Create reserved-memory node\n");
+
+ res = fdt_begin_node(fdt, resvbuf);
+ if ( res )
+ return res;
+
+ res = fdt_property(fdt, "ranges", NULL, 0);
+ if ( res )
+ return res;
+
+ res = fdt_property_cell(fdt, "#address-cells", addrcells);
+ if ( res )
+ return res;
+
+ res = fdt_property_cell(fdt, "#size-cells", sizecells);
+ if ( res )
+ return res;
+
+ res = make_shm_resv_memory_node(kinfo, addrcells, sizecells);
+ if ( res )
+ return res;
+
+ res = fdt_end_node(fdt);
+
+ return res;
+}
+
static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
struct dt_device_node *node,
p2m_type_t p2mt)
diff --git a/xen/common/device-tree/static-shmem.c b/xen/common/device-tree/static-shmem.c
index 8023c0a484c1..7eede97fa25d 100644
--- a/xen/common/device-tree/static-shmem.c
+++ b/xen/common/device-tree/static-shmem.c
@@ -730,46 +730,6 @@ int __init process_shm_node(const void *fdt, int node, uint32_t address_cells,
return 0;
}
-int __init make_resv_memory_node(const struct kernel_info *kinfo, int addrcells,
- int sizecells)
-{
- const struct membanks *mem = kernel_info_get_shm_mem_const(kinfo);
- void *fdt = kinfo->fdt;
- int res = 0;
- /* Placeholder for reserved-memory\0 */
- const char resvbuf[16] = "reserved-memory";
-
- if ( mem->nr_banks == 0 )
- /* No shared memory provided. */
- return 0;
-
- dt_dprintk("Create reserved-memory node\n");
-
- res = fdt_begin_node(fdt, resvbuf);
- if ( res )
- return res;
-
- res = fdt_property(fdt, "ranges", NULL, 0);
- if ( res )
- return res;
-
- res = fdt_property_cell(fdt, "#address-cells", addrcells);
- if ( res )
- return res;
-
- res = fdt_property_cell(fdt, "#size-cells", sizecells);
- if ( res )
- return res;
-
- res = make_shm_resv_memory_node(kinfo, addrcells, sizecells);
- if ( res )
- return res;
-
- res = fdt_end_node(fdt);
-
- return res;
-}
-
void __init early_print_info_shmem(void)
{
const struct membanks *shmem = bootinfo_get_shmem();
diff --git a/xen/include/xen/fdt-domain-build.h b/xen/include/xen/fdt-domain-build.h
index 45981dbec0b8..e9418857e386 100644
--- a/xen/include/xen/fdt-domain-build.h
+++ b/xen/include/xen/fdt-domain-build.h
@@ -25,6 +25,8 @@ int construct_domain(struct domain *d, struct kernel_info *kinfo);
int construct_hwdom(struct kernel_info *kinfo,
const struct dt_device_node *node);
int make_chosen_node(const struct kernel_info *kinfo);
+int make_resv_memory_node(const struct kernel_info *kinfo,
+ int addrcells, int sizecells);
int make_cpus_node(const struct domain *d, void *fdt);
int make_hypervisor_node(struct domain *d, const struct kernel_info *kinfo,
int addrcells, int sizecells);
diff --git a/xen/include/xen/static-shmem.h b/xen/include/xen/static-shmem.h
index 76a49869126c..9e7500ed2721 100644
--- a/xen/include/xen/static-shmem.h
+++ b/xen/include/xen/static-shmem.h
@@ -11,9 +11,6 @@
/* Worst case /memory node reg element: (addrcells + sizecells) */
#define DT_MEM_NODE_REG_RANGE_SIZE ((NR_MEM_BANKS + NR_SHMEM_BANKS) * 4)
-int make_resv_memory_node(const struct kernel_info *kinfo, int addrcells,
- int sizecells);
-
int process_shm(struct domain *d, struct kernel_info *kinfo,
const struct dt_device_node *node);
@@ -50,12 +47,6 @@ kernel_info_get_shm_mem_const(const struct kernel_info *kinfo)
/* Worst case /memory node reg element: (addrcells + sizecells) */
#define DT_MEM_NODE_REG_RANGE_SIZE (NR_MEM_BANKS * 4)
-static inline int make_resv_memory_node(const struct kernel_info *kinfo,
- int addrcells, int sizecells)
-{
- return 0;
-}
-
static inline int process_shm(struct domain *d, struct kernel_info *kinfo,
const struct dt_device_node *node)
{
@@ -80,6 +71,12 @@ static inline void shm_mem_node_fill_reg_range(const struct kernel_info *kinfo,
__be32 *reg, int *nr_cells,
int addrcells, int sizecells) {};
+static inline const struct membanks *
+kernel_info_get_shm_mem_const(const struct kernel_info *kinfo)
+{
+ return NULL;
+}
+
#endif /* CONFIG_STATIC_SHM */
#endif /* XEN_STATIC_SHMEM_H */
--
2.48.1On 7/5/25 10:27, Koichiro Den wrote:
> The /reserved-memory node is inherently not specific to static-shmem.
> Move it to a more generic domain build context. While at it, add an
> empty kernel_info_get_shm_mem_const() for the CONFIG_STATIC_SHM=n case,
> as it can now be invoked in such case.
>
> No functional change.
>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> xen/arch/arm/domain_build.c | 40 +++++++++++++++++++++++++++
> xen/common/device-tree/static-shmem.c | 40 ---------------------------
> xen/include/xen/fdt-domain-build.h | 2 ++
> xen/include/xen/static-shmem.h | 15 ++++------
> 4 files changed, 48 insertions(+), 49 deletions(-)
make_resv_memory_node() was recently moved from arch/arm to common in:
72c5fa220804 ("device-tree: Move Arm's static-shmem feature to common")
Is there any rationale for moving it back?
On Tue, Jul 08, 2025 at 04:12:50PM -0400, Stewart Hildebrand wrote:
> On 7/5/25 10:27, Koichiro Den wrote:
> > The /reserved-memory node is inherently not specific to static-shmem.
> > Move it to a more generic domain build context. While at it, add an
> > empty kernel_info_get_shm_mem_const() for the CONFIG_STATIC_SHM=n case,
> > as it can now be invoked in such case.
> >
> > No functional change.
> >
> > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > ---
> > xen/arch/arm/domain_build.c | 40 +++++++++++++++++++++++++++
> > xen/common/device-tree/static-shmem.c | 40 ---------------------------
> > xen/include/xen/fdt-domain-build.h | 2 ++
> > xen/include/xen/static-shmem.h | 15 ++++------
> > 4 files changed, 48 insertions(+), 49 deletions(-)
>
> make_resv_memory_node() was recently moved from arch/arm to common in:
>
> 72c5fa220804 ("device-tree: Move Arm's static-shmem feature to common")
>
> Is there any rationale for moving it back?
I overlooked that commit. So to preserve its intent, and make it usable
outside of the static-shmem purpose, which is my original intention, I
think I should place it in xen/common/device-tree/dom0less-build.c. What do
you think?
Thanks,
-Koichiro
On 7/9/25 03:58, Koichiro Den wrote:
> On Tue, Jul 08, 2025 at 04:12:50PM -0400, Stewart Hildebrand wrote:
>> On 7/5/25 10:27, Koichiro Den wrote:
>>> The /reserved-memory node is inherently not specific to static-shmem.
>>> Move it to a more generic domain build context. While at it, add an
>>> empty kernel_info_get_shm_mem_const() for the CONFIG_STATIC_SHM=n case,
>>> as it can now be invoked in such case.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Koichiro Den <den@valinux.co.jp>
>>> ---
>>> xen/arch/arm/domain_build.c | 40 +++++++++++++++++++++++++++
>>> xen/common/device-tree/static-shmem.c | 40 ---------------------------
>>> xen/include/xen/fdt-domain-build.h | 2 ++
>>> xen/include/xen/static-shmem.h | 15 ++++------
>>> 4 files changed, 48 insertions(+), 49 deletions(-)
>>
>> make_resv_memory_node() was recently moved from arch/arm to common in:
>>
>> 72c5fa220804 ("device-tree: Move Arm's static-shmem feature to common")
>>
>> Is there any rationale for moving it back?
>
> I overlooked that commit. So to preserve its intent, and make it usable
> outside of the static-shmem purpose, which is my original intention, I
> think I should place it in xen/common/device-tree/dom0less-build.c. What do
> you think?
Hm, if only considering its current callers, yes, because STATIC_SHM
depends on STATIC_MEMORY depends on DOM0LESS_BOOT. However, whether to
put it in the common domain-build.c or dom0less-build.c would really
depend on how you intend to use it, but I don't see any new calls to
make_resv_memory_node() on the hypervisor side in the remainder of the
series. Given that you do introduce an equivalent call on the toolstack
side, I wonder if that's an oversight on the hypervisor side?
Lastly, in the context of the series, your intent looks to be to add the
pv time region to /reserved-memory, but is that actually necessary?
On Fri, Jul 11, 2025 at 03:36:56PM -0400, Stewart Hildebrand wrote:
> On 7/9/25 03:58, Koichiro Den wrote:
> > On Tue, Jul 08, 2025 at 04:12:50PM -0400, Stewart Hildebrand wrote:
> >> On 7/5/25 10:27, Koichiro Den wrote:
> >>> The /reserved-memory node is inherently not specific to static-shmem.
> >>> Move it to a more generic domain build context. While at it, add an
> >>> empty kernel_info_get_shm_mem_const() for the CONFIG_STATIC_SHM=n case,
> >>> as it can now be invoked in such case.
> >>>
> >>> No functional change.
> >>>
> >>> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> >>> ---
> >>> xen/arch/arm/domain_build.c | 40 +++++++++++++++++++++++++++
> >>> xen/common/device-tree/static-shmem.c | 40 ---------------------------
> >>> xen/include/xen/fdt-domain-build.h | 2 ++
> >>> xen/include/xen/static-shmem.h | 15 ++++------
> >>> 4 files changed, 48 insertions(+), 49 deletions(-)
> >>
> >> make_resv_memory_node() was recently moved from arch/arm to common in:
> >>
> >> 72c5fa220804 ("device-tree: Move Arm's static-shmem feature to common")
> >>
> >> Is there any rationale for moving it back?
> >
> > I overlooked that commit. So to preserve its intent, and make it usable
> > outside of the static-shmem purpose, which is my original intention, I
> > think I should place it in xen/common/device-tree/dom0less-build.c. What do
> > you think?
>
> Hm, if only considering its current callers, yes, because STATIC_SHM
> depends on STATIC_MEMORY depends on DOM0LESS_BOOT. However, whether to
> put it in the common domain-build.c or dom0less-build.c would really
> depend on how you intend to use it, but I don't see any new calls to
> make_resv_memory_node() on the hypervisor side in the remainder of the
> series. Given that you do introduce an equivalent call on the toolstack
> side, I wonder if that's an oversight on the hypervisor side?
>
> Lastly, in the context of the series, your intent looks to be to add the
> pv time region to /reserved-memory, but is that actually necessary?
On second thought, it's not necessary as the region is always supposed to
be recognized by the call (follows SMCCC) only, and it resides in memory
holes that Xen will never use for any other purpose. I'll simplify the
whole patch series while also including the other feedback (ie. fixed
assignment instead of searching for unused range). That means I'll drop
this commit in v3.
Thanks for the review.
© 2016 - 2025 Red Hat, Inc.