[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 10/11] xen/arm: fix duplicate /reserved-memory node in Dom0


  • To: Luca Fancellu <luca.fancellu@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Michal Orzel <michal.orzel@xxxxxxx>
  • Date: Mon, 25 Mar 2024 09:09:26 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=arm.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=9qqCeHiGxcnKtEmwLOpaSsREI/g5oQTNBqhU0BNwswI=; b=DIIaC8F27Vfet681OAWpNQQidzI16LKbCEM7muWJ6TN2iSSS+q0zX2BNv59E3mnmkLe9Ws+qoM9Ifn7kyTAAq8O59+8ddcb9545YPWd25S9SfUO34Zpd3VasXYsBXF1wp3OysXBoauD6/wdnZzgOt68PNYKh9veief6UW7JekqE1mpn5tv431PQwaR94BHlPkZ8cPK9f1aZOzbdkoOXj4TRtrxz5jCo+/Br8kr4FMX/Jcsd/U8fE6AoGy0WYfg1bf6V/1f+QpgFFOWyq5pWuZHvZQzm6RpAmBSut6vaB+Z9pMaI0SGti2lxTsQji7Fds5pkkAI+YjV7Se08gr4c2mg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=jN1pE6z/OX8Kc5m1SX3UsIIbW9pMZ3oi76ysGWe2+1yOUwDo3n6hY6JD0HLwmTpYcFOq746Qn3iW5txEObiwtedmltX6nN3Dcx0Dh+9mt+4cPv7JekV21k1v5Gdtpjtloc44Hn5guB9Qr/Xin/q6HNaK6q3LzO1RSyEcK3ndyrmqzZd2IeqGS9JLFFOzD5FLUKBUPf06vCY6MdmOqgV8a15XOeuO1r8sVmxoZZwmSXpyOZLLKQmxIp4UEyKrbye1628wi5XimhYoU08/c+CCzQEyaP4ZdQRXIaC+9Aa+qMy51VjDz00sKUAE6ABHlKn4dqPSZfir/shWehtzGXm//Q==
  • Cc: Penny Zheng <Penny.Zheng@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Mon, 25 Mar 2024 08:09:38 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Luca,

On 12/03/2024 14:03, Luca Fancellu wrote:
> 
> 
> From: Penny Zheng <Penny.Zheng@xxxxxxx>
> 
> 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@xxxxxxx>
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> ---
> v1:
>  - Rework of 
> https://patchwork.kernel.org/project/xen-devel/patch/20231206090623.1932275-11-Penny.Zheng@xxxxxxx/
> ---
>  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@xxxxxxx>

~Michal




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.