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

Re: [XEN v1] xen/Arm: Probe the entry point address of an uImage correctly


  • To: Julien Grall <julien@xxxxxxx>, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Ayan Kumar Halder <ayankuma@xxxxxxx>
  • Date: Thu, 8 Dec 2022 15:00:58 +0000
  • 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=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=trOoBB4zJ/Q2br93f+i3mHaJUxaW568hjl3j95QlG28=; b=ecz9EHd+Xyb+JapgX7dAvz/4TKm7mYv+eufjdst30NQ5czsZhn89Zqoz75kzvOyZABkBxksp73SD1qe4DvKDnyBzpUPvc91XzqFxtZG6iJ3akW5f/CaTv/vCyTD40McjedJXsNB0WsTiWqNr6E66SRJeUUDkYGLSpLftXCWSgi7aqBhXMgnbFJRR4y/GHOmWtea9bm02tamhU0QYl9AZKRCjtM+jj7p87RPfhEi0kNkwWa84Pulj+Q5VVHdtFDHsPZMF/bhOMKjNx9y1HVcXWf6gynAF3jI47qNTL4fOYRsJLXQN4jlRR2I+zx3zHw5RnpdrrNSuyMIu6KutI4v37A==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MY0bXqRJK/vkBbGo73yDj33BP7v1pkF3kfiAN86rR0eatfGG62fa3KsXpsuckpMkj3sJ1qbf3aNXZHEnGPxhTLKnX5xlKB22Bz3YEVLPaFggc6vr6h6PlB6Qhow0365/VLK0rGe0n6fa55ZPFd+90nil36jmm+8Ee8vKMDuxmf9WIHn4nrgwRXb6L6wUA9soP9Bn8m258sw3U2UfSNnBrF7n1II8Zm1QZ69k3seuyDbaW3yjCSfShkBczJPO7DR5oaew6j7xqmkfjUbeZuS/iGnHYPU+M8XqiXDMtMstS9d+a+Ip2d8uxZbBJl6Yc5/ZbIrZxU5u4rwp2KOoMMk69Q==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com;
  • Cc: sstabellini@xxxxxxxxxx, stefanos@xxxxxxxxxx, Volodymyr_Babchuk@xxxxxxxx, bertrand.marquis@xxxxxxx, michal.orzel@xxxxxxx, Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
  • Delivery-date: Thu, 08 Dec 2022 15:01:30 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>


On 08/12/2022 13:51, Julien Grall wrote:
Hi,
Hi Julien,

Title extra NIT: I have seen it multiple time and so far refrain to say it. Please use 'arm' rather than 'Arm'. This is for consistency in the way we name the subsystem in the title.
I will take care of it henceforth.

On 08/12/2022 12:49, Ayan Kumar Halder wrote:
Currently, kernel_uimage_probe() does not set info->zimage.start. As a
result, it contains the default value (ie 0). This causes,
kernel_zimage_place() to treat the binary (contained within uImage) as
position independent executable. Thus, it loads it at an incorrect address.

The correct approach would be to read "uimage.ep" and set
info->zimage.start. This will ensure that the binary is loaded at the
correct address.

In non-statically allocated setup, a user doesn't know where the memory for dom0/domU will be allocated.

So I think this was correct to ignore the address. In fact, I am worry that...


Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
---

I uncovered this issue while loading Zephyr as a dom0less domU with Xen on R52 FVP. Zephyr builds with static device tree. Thus, the load address is
always fixed.

  xen/arch/arm/kernel.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 2556a45c38..e4e8c67669 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -222,6 +222,8 @@ static int __init kernel_uimage_probe(struct kernel_info *info,
      if ( len > size - sizeof(uimage) )
          return -EINVAL;
  +    info->zimage.start = be32_to_cpu(uimage.ep);
... this will now ended up to break anyone that may have set an address but didn't care where it should be loaded.

Can we use a config option (STATIC_MEMORY or any option you may suggest) to distinguish between the two ?

Or using some magic number (eg 0xdeadbeef) for uimage.ep which will denote position independent ? This needs to be documented in docs/misc/arm/booting.txt.

Or ...


I also understand your use case but now, we have contradictory approaches. I am not entirely sure how we can solve it. We may have to break those users (Cc some folks that may use it). But we should figure out what is the alternative for them.
I am open to any alternative suggestion.

If we decide to break those users, then this should be documented in the commit message and in docs/misc/arm/booting.txt (which interestingly didn't mention uImage).

I agree.

- Ayan


Cheers,




 


Rackspace

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