diff mbox series

[v5,11/11] xen/arm: create another /memory node for static shm

Message ID 20231206090623.1932275-12-Penny.Zheng@arm.com (mailing list archive)
State New, archived
Headers show
Series Follow-up static shared memory PART I | expand

Commit Message

Penny Zheng Dec. 6, 2023, 9:06 a.m. UTC
Static shared memory region shall be described both under /memory and
/reserved-memory.

We introduce export_shm_memory_node() to create another /memory node to
contain the static shared memory ranges.

Signed-off-by: Penny Zheng <penny.zheng@arm.com>

---
v3 -> v4:
new commit
---
v4 -> v5:
rebase and no changes
---
 xen/arch/arm/dom0less-build.c           |  8 ++++++++
 xen/arch/arm/domain_build.c             |  8 ++++++++
 xen/arch/arm/include/asm/static-shmem.h | 10 ++++++++++
 xen/arch/arm/static-shmem.c             | 19 +++++++++++++++++++
 4 files changed, 45 insertions(+)

Comments

Michal Orzel Dec. 7, 2023, 3:28 p.m. UTC | #1
Hi Penny,

On 06/12/2023 10:06, Penny Zheng wrote:
> 
> 
> Static shared memory region shall be described both under /memory and
> /reserved-memory.
> 
> We introduce export_shm_memory_node() to create another /memory node to
> contain the static shared memory ranges.
> 
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> 
> ---
> v3 -> v4:
> new commit
> ---
> v4 -> v5:
> rebase and no changes
> ---
>  xen/arch/arm/dom0less-build.c           |  8 ++++++++
>  xen/arch/arm/domain_build.c             |  8 ++++++++
>  xen/arch/arm/include/asm/static-shmem.h | 10 ++++++++++
>  xen/arch/arm/static-shmem.c             | 19 +++++++++++++++++++
>  4 files changed, 45 insertions(+)
> 
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index ac096fa3fa..870b8a553f 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -645,6 +645,14 @@ static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo)
>      if ( ret )
>          goto err;
> 
> +    /* Create a memory node to store the static shared memory regions */
> +    if ( kinfo->shminfo.nr_banks != 0 )
There is no need for this check to be repeated every time export_shm_memory_node is used.
When the feature is disabled, export_shm_memory_node will return 0 anyway.
Furthermore, there is no need for kinfo->shminfo exposure. Please move the check to the function itself.

Also, I think both this and previous patch could be moved towards the beginning of the series.
They are not related to other things you do in the series.


> +    {
> +        ret = export_shm_memory_node(d, kinfo, addrcells, sizecells);
> +        if ( ret )
> +            goto err;
> +    }
> +
>      ret = make_resv_memory_node(d, kinfo->fdt, addrcells, sizecells, kinfo);
>      if ( ret )
>          goto err;
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index f098678ea3..4e450cb4c7 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1873,6 +1873,14 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
>                  return res;
>          }
> 
> +        /* Create a memory node to store the static shared memory regions */
> +        if ( kinfo->shminfo.nr_banks != 0 )
> +        {
> +            res = export_shm_memory_node(d, kinfo, addrcells, sizecells);
> +            if ( res )
> +                return res;
> +        }
> +
>          /* Avoid duplicate /reserved-memory nodes in Device Tree */
>          if ( !kinfo->resv_mem )
>          {
> diff --git a/xen/arch/arm/include/asm/static-shmem.h b/xen/arch/arm/include/asm/static-shmem.h
> index 6cb4ef9646..385fd24c17 100644
> --- a/xen/arch/arm/include/asm/static-shmem.h
> +++ b/xen/arch/arm/include/asm/static-shmem.h
> @@ -38,6 +38,10 @@ int make_shm_memory_node(const struct domain *d,
>                           void *fdt,
>                           int addrcells, int sizecells,
>                           const struct kernel_info *kinfo);
> +
> +int export_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, void *fdt,
> @@ -86,6 +90,12 @@ static inline int make_shm_memory_node(const struct domain *d,
>      return 0;
>  }
> 
> +static inline int export_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 bfce5bbad0..e583aae685 100644
> --- a/xen/arch/arm/static-shmem.c
> +++ b/xen/arch/arm/static-shmem.c
> @@ -505,6 +505,25 @@ int __init process_shm(struct domain *d, struct kernel_info *kinfo,
>      return 0;
>  }
> 
> +int __init export_shm_memory_node(const struct domain *d,
> +                                  const struct kernel_info *kinfo,
> +                                  int addrcells, int sizecells)
> +{
> +    unsigned int i = 0;
> +    struct meminfo shm_meminfo;
> +
> +    /* Extract meminfo from kinfo.shminfo */
> +    for ( ; i < kinfo->shminfo.nr_banks; i++ )
> +    {
> +        shm_meminfo.bank[i].start = kinfo->shminfo.bank[i].membank.start;
> +        shm_meminfo.bank[i].size = kinfo->shminfo.bank[i].membank.size;
> +        shm_meminfo.bank[i].type = MEMBANK_DEFAULT;
Is all of this really needed? This series introduces so many structures to avoid using
meminfo but at the end we still need to use it. The amount of meminfo like structures copying
done in this series worries me a bit.

~Michal
diff mbox series

Patch

diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index ac096fa3fa..870b8a553f 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -645,6 +645,14 @@  static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo)
     if ( ret )
         goto err;
 
+    /* Create a memory node to store the static shared memory regions */
+    if ( kinfo->shminfo.nr_banks != 0 )
+    {
+        ret = export_shm_memory_node(d, kinfo, addrcells, sizecells);
+        if ( ret )
+            goto err;
+    }
+
     ret = make_resv_memory_node(d, kinfo->fdt, addrcells, sizecells, kinfo);
     if ( ret )
         goto err;
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index f098678ea3..4e450cb4c7 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1873,6 +1873,14 @@  static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
                 return res;
         }
 
+        /* Create a memory node to store the static shared memory regions */
+        if ( kinfo->shminfo.nr_banks != 0 )
+        {
+            res = export_shm_memory_node(d, kinfo, addrcells, sizecells);
+            if ( res )
+                return res;
+        }
+
         /* Avoid duplicate /reserved-memory nodes in Device Tree */
         if ( !kinfo->resv_mem )
         {
diff --git a/xen/arch/arm/include/asm/static-shmem.h b/xen/arch/arm/include/asm/static-shmem.h
index 6cb4ef9646..385fd24c17 100644
--- a/xen/arch/arm/include/asm/static-shmem.h
+++ b/xen/arch/arm/include/asm/static-shmem.h
@@ -38,6 +38,10 @@  int make_shm_memory_node(const struct domain *d,
                          void *fdt,
                          int addrcells, int sizecells,
                          const struct kernel_info *kinfo);
+
+int export_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, void *fdt,
@@ -86,6 +90,12 @@  static inline int make_shm_memory_node(const struct domain *d,
     return 0;
 }
 
+static inline int export_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 bfce5bbad0..e583aae685 100644
--- a/xen/arch/arm/static-shmem.c
+++ b/xen/arch/arm/static-shmem.c
@@ -505,6 +505,25 @@  int __init process_shm(struct domain *d, struct kernel_info *kinfo,
     return 0;
 }
 
+int __init export_shm_memory_node(const struct domain *d,
+                                  const struct kernel_info *kinfo,
+                                  int addrcells, int sizecells)
+{
+    unsigned int i = 0;
+    struct meminfo shm_meminfo;
+
+    /* Extract meminfo from kinfo.shminfo */
+    for ( ; i < kinfo->shminfo.nr_banks; i++ )
+    {
+        shm_meminfo.bank[i].start = kinfo->shminfo.bank[i].membank.start;
+        shm_meminfo.bank[i].size = kinfo->shminfo.bank[i].membank.size;
+        shm_meminfo.bank[i].type = MEMBANK_DEFAULT;
+    }
+    shm_meminfo.nr_banks = kinfo->shminfo.nr_banks;
+
+    return make_memory_node(d, kinfo->fdt, addrcells, sizecells, &shm_meminfo);
+}
+
 int __init make_shm_memory_node(const struct domain *d, void *fdt,
                                 int addrcells, int sizecells,
                                 const struct kernel_info *kinfo)