Re: [Xen-devel] [PATCH V2] xen/arm: arm64: Update the Image header

[Peng Fan <van.freenix@xxxxxxxxx>]

Hi Julien,

On Fri, Sep 09, 2016 at 02:19:33PM +0100, Julien Grall wrote:
>Hello Peng,
>
>On 01/09/16 02:38, Peng Fan wrote:
>>This patch is mainly modified from Linux kernel:
>>[1] commit a2c1d73b94ed: arm64: Update the Image header
>>[2] commit 6ad1fe5d9077: arm64: avoid R_AARCH64_ABS64 relocations for Image 
>>header fields
>>
>>From [1]:
>>"
>>This patch adds an effective image size to the kernel header which
>>describes the amount of memory from the start of the kernel Image binary
>>which the kernel expects to use before detecting memory and handling any
>>memory reservations. This can be used by bootloaders to choose suitable
>>locations to load the kernel and/or other binaries such that the kernel
>>will not clobber any memory unexpectedly. As before, memory reservations
>>are required to prevent the kernel from clobbering these locations
>>later.
>>
>>Both the image load offset and the effective image size are forced to be
>>little-endian regardless of the native endianness of the kernel to
>>enable bootloaders to load a kernel of arbitrary endianness. Bootloaders
>>which wish to make use of the load offset can inspect the effective
>>image size field for a non-zero value to determine if the offset is of a
>>known endianness. To enable software to determine the endinanness of the
>>kernel as may be required for certain use-cases, a new flags field (also
>>little-endian) is added to the kernel header to export this information.
>>"
>>
>>In this patch, XEN_VIRT_START is used as the text offset. Then, bootloader,
>>such as U-Boot, will load the xen image to "dram_base + text_offset".
>>Not choose 0 as the text offset, because we may have spin table at dram_base.
>>Loading xen to dram_base will override the spin table.
>
>XEN_VIRT_START is *not* the text offset. It is the virtual address mark the
>start of Xen region when paging is enabled.
>
>Furthermore, your description here does not match the behavior of this patch.
>The kernel physical displacement is set to 1, this means the kernel will be
>loaded anywhere in the memory.
>
>It is the job of the bootloader to find an unused place to load Xen into the
>memory.

Agree. I'll keep the text offset 0 as before in V3.

>
>>
>>Introduce image.h and macros.h in this patch, just as Linux kernel.
>>
>>Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
>>Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>Cc: Julien Grall <julien.grall@xxxxxxx>
>>---
>>
>>V2: Addressing Julien's comments to follow linux kernel patch:
>>    a2c1d73b94ed49 "arm64: Update the Image header"
>
>Whilst I suggested to update all the header fields (image load offset,
>effective size, flags...), I don't think we should import a "verbatim" of the
>Linux header image.h. It is really complex for a questionable benefit.

Ok. I'll drop the image.h from Linux Kernel in V3

>
>>
>> xen/arch/arm/arm64/head.S          |  7 +++--
>> xen/arch/arm/xen.lds.S             |  5 +++
>> xen/include/asm-arm/arm64/image.h  | 62 
>> ++++++++++++++++++++++++++++++++++++++
>> xen/include/asm-arm/arm64/macros.h | 15 +++++++++
>> xen/include/asm-arm/macros.h       |  2 +-
>> 5 files changed, 87 insertions(+), 4 deletions(-)
>> create mode 100644 xen/include/asm-arm/arm64/image.h
>> create mode 100644 xen/include/asm-arm/arm64/macros.h
>>
>>diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
>>index 91e2817..0226463 100644
>>--- a/xen/arch/arm/arm64/head.S
>>+++ b/xen/arch/arm/arm64/head.S
>>@@ -21,6 +21,7 @@
>>  */
>>
>> #include <asm/config.h>
>>+#include <asm/macros.h>
>> #include <asm/page.h>
>> #include <asm/asm_defns.h>
>> #include <asm/early_printk.h>
>>@@ -114,9 +115,9 @@ efi_head:
>>          */
>>         add     x13, x18, #0x16
>>         b       real_start           /* branch to kernel start */
>>-        .quad   0                    /* Image load offset from start of RAM 
>>*/
>>-        .quad   0                    /* reserved */
>>-        .quad   0                    /* reserved */
>>+        le64sym _xen_offset_le       /* Image load offset from start of RAM, 
>>little-endian */
>>+        le64sym _xen_size_le         /* Effective size of kernel image, 
>>little-endian */
>>+        le64sym _xen_flags_le        /* Informative flags, little-endian */
>>         .quad   0                    /* reserved */
>>         .quad   0                    /* reserved */
>>         .quad   0                    /* reserved */
>>diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
>>index b24e93b..854c243 100644
>>--- a/xen/arch/arm/xen.lds.S
>>+++ b/xen/arch/arm/xen.lds.S
>>@@ -6,6 +6,7 @@
>> #include <xen/cache.h>
>> #include <asm/page.h>
>> #include <asm/percpu.h>
>>+#include <asm/arm64/image.h>
>
>Please don't include arm64 specific header in common code.

Fix in V3.

Thanks,
Peng.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel