[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: "Orzel, Michal" <michal.orzel@xxxxxxx>
  • From: Luca Fancellu <Luca.Fancellu@xxxxxxx>
  • Date: Wed, 26 Feb 2025 10:59:20 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=arm.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=arm.com])
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=2; 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=4QrsW2AoSNUhRF54mu4rMgR2e66z7oSRy6T7iZPb0eg=; b=DMX3Y/+AxSFYYWQkQm4/ip7qTVCVv4khKAsWRbeiz5LsXfJ/mx+QjxFx76+Mgm5EJ+F+T15Wv82Ta3TXhBbaRoi+I9uDYbUTklhI6TsTP8vFd1P6MpeFjl+OHJ8uy/dErDDOs/OAhI7vuW36AfcDOQ4SiWsbkHhwpwiUOO26Do0WLhlZaBanq3e967hJ4NhOgwAyBTx6ERq7hmDSELHHWGb36ubIjTALBp2KK/rflZra0lc4czsbl1DXSqffdedcDwVazNTbE+NACkGlr+OQ+pdspQl8PuWM8Gtow9uT7dyFZVguH7uSo2nySQh/JzCEQUhnLxtdTcErRj6op+0wtw==
  • 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=4QrsW2AoSNUhRF54mu4rMgR2e66z7oSRy6T7iZPb0eg=; b=hVC2GOwFORlsO0Nq1I8UZNFjmHm7a4s2Lgmu4g97PXL4tdv6NApp8kD4SNdWh4u9nBsI3uv1BarijOj8ESZ7psyJxIdxAwU/1VZ85hLL6qpTMueUUV/V/7qR2R9DnpVr8fb6YQN5S57IEjSpG+C2oz6pqucSs0Kx/tDB2Cj/VR081izDZTJRiufrzip3xjlKzECZmr2dta9gUiwulNozp2XgJaHRxhEYNZHT5gyIVVQPct8HBZNRQ2olxpXxQASW1/LZj0JWfCBA5ZlnFax5ZTvdnrARZzabMaWmrwQ4uJ8MSy0QjCWZBNg2D+JbiiX+o0wLtlfeYM48x0suXDv8Sw==
  • Arc-seal: i=2; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=pass; b=LSQ6v8FPgScbmimUirnSsYpCbcAXLuHnQblSj1wLG62zGAP1AsEpMMui9yFY8sVSGxX/BwtfA1UzOJrUQ8kHCC2MKkFbhU6k6NhLuAiklW9JPqQbd108ZdG7o+5qKgO2n16LdnXI/DNPAr/7ZQ5Op309EQy8QgTI4SFrzaHqmCBwJo25ERXLNsIlM9J/qlbSgMYKYBv9U6s8DXwyN8k+MSx1dl16brwnqIeO4DOyUyRQ71h7KHipYcnz2LP+PB+6G3rCv5gdLElS5eM/mu+1j1MV2jDb0vuUOEUyJU9twNabvIzDhbakygR93/sF9yzRdHElwQHmHAcyj+gf/APcTA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=OYJ9lmjXNL6bB91TsQGcQmdx0Z7lqD3nLSg7REyqIXdZiVRQY5KejLn2gXDCEWxbec5iXqxDOIoDwVuVj3qA/B6d29NjGwfgx6iwVLhdj02phOTcAz5VWXE+39c8o4+iV276POPCGfRqF6u4NcS0Cw5Yw2HjHeV4LGXHQNzixyzuGC3YwvEQVqaBBZwZTQHi+Zl7xlvxMniEjoz8/VcCC1Cegn5lJzppF91/Ieng+Np+LTOrkUiPocsjG0FJ6Lgc1k1vxrbhQ/poQDgObMaSoLG7KuJUtTGvdVSs/zxrpdJRWOmlLlUxKAaqX2K+WuIwST3lru96d/y6Pq5o76zbYg==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, 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:59:49 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHbiCmjnkyna+WuREGzHZ/umIzQ77NZZMaAgAABvwCAAALigIAAARmA
  • Thread-topic: [PATCH] xen/arm: Don't use copy_from_paddr for DTB relocation

>> 
>>> 
>>>> 
>>>>    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?
>> 
>> you mean something like:
>> 
>> static void * __init relocate_fdt(size_t dtb_size)
>> {
>> void *fdt = xmalloc_bytes(dtb_size);
>> 
>> if ( !fdt )
>> panic("Unable to allocate memory for relocating the Device-Tree.\n");
>> 
>> memcpy(fdt, device_tree_flattened, dtb_size);
> You already make assumption about device_tree_flattened being global, so why
> not assigning device_tree_flattened = fdt in the function as well?

just because it’s more easy to follow the global variable changes when reading 
the start_xen(…)
code as the function is the only one modifying it.

If you strongly oppose to that I’ll change, but imo it’s easier to follow the 
code in this way

Cheers,
Luca


 


Rackspace

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