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

Re: [PATCH] xen/arm: Don't use copy_from_paddr for DTB relocation


  • To: Luca Fancellu <luca.fancellu@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: "Orzel, Michal" <michal.orzel@xxxxxxx>
  • Date: Wed, 26 Feb 2025 11:38:19 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=688jrxlfloaqRoVFmmGu6uTzdhTh0I0rlw2bPJafoW4=; b=tyd3LUvD4dEu2rR84Q0PKZmlqEGc/7bYQ0E1xgPGuyXGU9/c0LapbviL3At66AG0Zmk1v9AuCTUdW5agl4G8gcxbszJqQJ26WBJmTyKZeuogRM0090hpruAQNXxymJ15mZLRrGNTGnf2KcLFfXe5PY0niKD5dN30roZ8EnV6kZBADVV05uPj75vu5PWuhmGzhWz7Vp9mFMGPsw+At42qjnlYKaQl/QMrrruZXItSUcLK+iEywITRFUbaSiPe9J+Eenp2+rhmDWA1JcaBtwy1PBP4cEAsToQWhaD2d/HG+lAXiYO2GKlNavgDHuN597oB52z782bCqkjqD793wHrwig==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=c6pw6brzBlQl/ntbwyXAhXMKMGot3vPfPKx7lFBXCETHaleSCF+Anh3eLAgHEjdIryUXoHGKIBkiVM/O4wFQDKxxUlzrjGT2QQY7FYSS8FwsNduy1iyAPlbUZbLPQ+7bE59a7VCOFaslf8Yz8tXwfy7CvJ3t3KtGz8jpxZJN3r/4LMn0oEP+9ei0P/2htchLYhVWtORyPgOZKpHplmyBTlmR+zqhVXeuc8tK1e7GyUiOdijXrKLibin/vXiLr3wghyZddzuA0vefJ9eUSb3FypEcQelpbv5wbBGtBcOonfSRfWxJT59mpwboXkPztwTTB8ZfEK63CWIpGREjxa+ZRg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com;
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Wed, 26 Feb 2025 10:38:35 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>


On 26/02/2025 09:36, Luca Fancellu wrote:
> 
> 
> Currently the early stage of the Arm boot maps the DTB using
> early_fdt_map() using PAGE_HYPERVISOR_RO which is cacheable
> read-only memory, later during DTB relocation the function
> copy_from_paddr() is used to map pages in the same range on
> the fixmap but using PAGE_HYPERVISOR_WC which is non-cacheable
> read-write memory.
> 
> The Arm specifications, ARM DDI0487L.a, section B2.11 "Mismatched
> memory attributes" discourage using mismatched attributes for
> aliases of the same location.
> 
> Given that there is nothing preventing the relocation since the region
> is already mapped, fix that by open-coding copy_from_paddr inside
> relocate_fdt, without mapping on the fixmap.
> 
> Fixes: 1bdc81dac816 ("arm: setup MM using information from the device tree")
Why Fixes tag? I don't see it as a bug and something we need to backport...

> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> ---
>  xen/arch/arm/setup.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index c1f2d1b89d43..b1fd4b44a2e1 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -237,14 +237,15 @@ void __init discard_initial_modules(void)
>  }
> 
>  /* Relocate the FDT in Xen heap */
> -static void * __init relocate_fdt(paddr_t dtb_paddr, size_t dtb_size)
> +static void * __init relocate_fdt(const void *dtb_vaddr, size_t dtb_size)
>  {
>      void *fdt = xmalloc_bytes(dtb_size);
> 
>      if ( !fdt )
>          panic("Unable to allocate memory for relocating the Device-Tree.\n");
> 
> -    copy_from_paddr(fdt, dtb_paddr, dtb_size);
> +    memcpy(fdt, dtb_vaddr, dtb_size);
> +    clean_dcache_va_range(dtb_vaddr, dtb_size);
The purpose of cleaning dcache after memcpy is to clean the new area i.e.
destination == fdt, not source region.

> 
>      return fdt;
>  }
> @@ -362,7 +363,7 @@ void asmlinkage __init start_xen(unsigned long fdt_paddr)
>      if ( acpi_disabled )
>      {
>          printk("Booting using Device Tree\n");
> -        device_tree_flattened = relocate_fdt(fdt_paddr, fdt_size);
> +        device_tree_flattened = relocate_fdt(device_tree_flattened, 
> fdt_size);
NIT: It can be just my PoV but it reads odd. Why can't relocate_fdt modify
device_tree_flattened pointer directly in the function?

>          dt_unflatten_host_device_tree();
>      }
>      else
> --
> 2.34.1
> 

~Michal




 


Rackspace

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