1. Patchew
  2. Xen
  3. View series

[PATCH] xen/arm: Don't call process_shm_chosen() during ACPI boot

Michal Orzel posted 1 patch 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20250402084233.114604-1-michal.orzel@amd.com
xen/arch/arm/domain_build.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
[PATCH] xen/arm: Don't call process_shm_chosen() during ACPI boot
Posted by Michal Orzel 7 months ago 
Static shared memory requires device-tree boot. At the moment, booting
with ACPI enabled and CONFIG_STATIC_SHM=y results in a data abort when
dereferencing node in process_shm() because dt_host is always NULL.

Fixes: 09c0a8976acf ("xen/arm: enable statically shared memory on Dom0")
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
 xen/arch/arm/domain_build.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2b5b4331834f..85f423214a44 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2325,9 +2325,12 @@ int __init construct_hwdom(struct kernel_info *kinfo)
     else
         allocate_memory(d, kinfo);
 
-    rc = process_shm_chosen(d, kinfo);
-    if ( rc < 0 )
-        return rc;
+    if ( acpi_disabled )
+    {
+        rc = process_shm_chosen(d, kinfo);
+        if ( rc < 0 )
+            return rc;
+    }
 
     /* Map extra GIC MMIO, irqs and other hw stuffs to dom0. */
     rc = gic_map_hwdom_extra_mappings(d);
-- 
2.25.1
Re: [PATCH] xen/arm: Don't call process_shm_chosen() during ACPI boot
Posted by Bertrand Marquis 7 months ago 
Hi Michal,

> On 2 Apr 2025, at 10:42, Michal Orzel <michal.orzel@amd.com> wrote:
> 
> Static shared memory requires device-tree boot. At the moment, booting
> with ACPI enabled and CONFIG_STATIC_SHM=y results in a data abort when
> dereferencing node in process_shm() because dt_host is always NULL.
> 
> Fixes: 09c0a8976acf ("xen/arm: enable statically shared memory on Dom0")
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> ---
> xen/arch/arm/domain_build.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 2b5b4331834f..85f423214a44 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2325,9 +2325,12 @@ int __init construct_hwdom(struct kernel_info *kinfo)
>     else
>         allocate_memory(d, kinfo);
> 
> -    rc = process_shm_chosen(d, kinfo);
> -    if ( rc < 0 )
> -        return rc;
> +    if ( acpi_disabled )
> +    {
> +        rc = process_shm_chosen(d, kinfo);
> +        if ( rc < 0 )
> +            return rc;
> +    }
> 
>     /* Map extra GIC MMIO, irqs and other hw stuffs to dom0. */
>     rc = gic_map_hwdom_extra_mappings(d);
> -- 
> 2.25.1
>
[PATCH v2] xen/arm: Drop process_shm_chosen()
Posted by Michal Orzel 7 months ago 
There's no benefit in having process_shm_chosen() next to process_shm().
The former is just a helper to pass "/chosen" node to the latter for
hwdom case. Drop process_shm_chosen() and instead use process_shm()
passing NULL as node parameter, which will result in searching for and
using /chosen to find shm node (the DT full path search is done in
process_shm() to avoid expensive lookup if !CONFIG_STATIC_SHM). This
will simplify future handling of hw/control domain separation.

Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
Changes in v2:
 - rebase on ACPI fix
 - add BUG_ON for !/chosen
---
 xen/arch/arm/domain_build.c             |  2 +-
 xen/arch/arm/include/asm/static-shmem.h | 14 --------------
 xen/arch/arm/static-shmem.c             |  7 +++++++
 3 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 85f423214a44..634333cddef3 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2327,7 +2327,7 @@ int __init construct_hwdom(struct kernel_info *kinfo)
 
     if ( acpi_disabled )
     {
-        rc = process_shm_chosen(d, kinfo);
+        rc = process_shm(d, kinfo, NULL);
         if ( rc < 0 )
             return rc;
     }
diff --git a/xen/arch/arm/include/asm/static-shmem.h b/xen/arch/arm/include/asm/static-shmem.h
index fd0867c4f26b..94eaa9d500f9 100644
--- a/xen/arch/arm/include/asm/static-shmem.h
+++ b/xen/arch/arm/include/asm/static-shmem.h
@@ -18,14 +18,6 @@ int make_resv_memory_node(const struct kernel_info *kinfo, int addrcells,
 int process_shm(struct domain *d, struct kernel_info *kinfo,
                 const struct dt_device_node *node);
 
-static inline int process_shm_chosen(struct domain *d,
-                                     struct kernel_info *kinfo)
-{
-    const struct dt_device_node *node = dt_find_node_by_path("/chosen");
-
-    return process_shm(d, kinfo, node);
-}
-
 int process_shm_node(const void *fdt, int node, uint32_t address_cells,
                      uint32_t size_cells);
 
@@ -74,12 +66,6 @@ static inline int process_shm(struct domain *d, struct kernel_info *kinfo,
     return 0;
 }
 
-static inline int process_shm_chosen(struct domain *d,
-                                     struct kernel_info *kinfo)
-{
-    return 0;
-}
-
 static inline void init_sharedmem_pages(void) {};
 
 static inline int remove_shm_from_rangeset(const struct kernel_info *kinfo,
diff --git a/xen/arch/arm/static-shmem.c b/xen/arch/arm/static-shmem.c
index c74fa13d4847..e8d4ca3ba3ff 100644
--- a/xen/arch/arm/static-shmem.c
+++ b/xen/arch/arm/static-shmem.c
@@ -297,6 +297,13 @@ int __init process_shm(struct domain *d, struct kernel_info *kinfo,
 {
     struct dt_device_node *shm_node;
 
+    /* Hwdom case - shm node under /chosen */
+    if ( !node )
+    {
+        node = dt_find_node_by_path("/chosen");
+        BUG_ON(!node);
+    }
+
     dt_for_each_child_node(node, shm_node)
     {
         const struct membank *boot_shm_bank;
-- 
2.25.1
Re: [PATCH v2] xen/arm: Drop process_shm_chosen()
Posted by Bertrand Marquis 7 months ago 
Hi Michal,

> On 2 Apr 2025, at 10:42, Michal Orzel <michal.orzel@amd.com> wrote:
> 
> There's no benefit in having process_shm_chosen() next to process_shm().
> The former is just a helper to pass "/chosen" node to the latter for
> hwdom case. Drop process_shm_chosen() and instead use process_shm()
> passing NULL as node parameter, which will result in searching for and
> using /chosen to find shm node (the DT full path search is done in
> process_shm() to avoid expensive lookup if !CONFIG_STATIC_SHM). This
> will simplify future handling of hw/control domain separation.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Thanks for the BUG_ON :-)

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> ---
> Changes in v2:
> - rebase on ACPI fix
> - add BUG_ON for !/chosen
> ---
> xen/arch/arm/domain_build.c             |  2 +-
> xen/arch/arm/include/asm/static-shmem.h | 14 --------------
> xen/arch/arm/static-shmem.c             |  7 +++++++
> 3 files changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 85f423214a44..634333cddef3 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2327,7 +2327,7 @@ int __init construct_hwdom(struct kernel_info *kinfo)
> 
>     if ( acpi_disabled )
>     {
> -        rc = process_shm_chosen(d, kinfo);
> +        rc = process_shm(d, kinfo, NULL);
>         if ( rc < 0 )
>             return rc;
>     }
> diff --git a/xen/arch/arm/include/asm/static-shmem.h b/xen/arch/arm/include/asm/static-shmem.h
> index fd0867c4f26b..94eaa9d500f9 100644
> --- a/xen/arch/arm/include/asm/static-shmem.h
> +++ b/xen/arch/arm/include/asm/static-shmem.h
> @@ -18,14 +18,6 @@ int make_resv_memory_node(const struct kernel_info *kinfo, int addrcells,
> int process_shm(struct domain *d, struct kernel_info *kinfo,
>                 const struct dt_device_node *node);
> 
> -static inline int process_shm_chosen(struct domain *d,
> -                                     struct kernel_info *kinfo)
> -{
> -    const struct dt_device_node *node = dt_find_node_by_path("/chosen");
> -
> -    return process_shm(d, kinfo, node);
> -}
> -
> int process_shm_node(const void *fdt, int node, uint32_t address_cells,
>                      uint32_t size_cells);
> 
> @@ -74,12 +66,6 @@ static inline int process_shm(struct domain *d, struct kernel_info *kinfo,
>     return 0;
> }
> 
> -static inline int process_shm_chosen(struct domain *d,
> -                                     struct kernel_info *kinfo)
> -{
> -    return 0;
> -}
> -
> static inline void init_sharedmem_pages(void) {};
> 
> static inline int remove_shm_from_rangeset(const struct kernel_info *kinfo,
> diff --git a/xen/arch/arm/static-shmem.c b/xen/arch/arm/static-shmem.c
> index c74fa13d4847..e8d4ca3ba3ff 100644
> --- a/xen/arch/arm/static-shmem.c
> +++ b/xen/arch/arm/static-shmem.c
> @@ -297,6 +297,13 @@ int __init process_shm(struct domain *d, struct kernel_info *kinfo,
> {
>     struct dt_device_node *shm_node;
> 
> +    /* Hwdom case - shm node under /chosen */
> +    if ( !node )
> +    {
> +        node = dt_find_node_by_path("/chosen");
> +        BUG_ON(!node);
> +    }
> +
>     dt_for_each_child_node(node, shm_node)
>     {
>         const struct membank *boot_shm_bank;
> -- 
> 2.25.1
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.