diff mbox

[V2] xen/arm: arm64: Update the Image header

Message ID 1472693901-2459-1-git-send-email-peng.fan@nxp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peng Fan Sept. 1, 2016, 1:38 a.m. UTC
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.

Introduce image.h and macros.h in this patch, just as Linux kernel.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---

V2: Addressing Julien's comments to follow linux kernel patch:
    a2c1d73b94ed49 "arm64: Update the Image header"

 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

Comments

Julien Grall Sept. 9, 2016, 1:19 p.m. UTC | #1
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.

>
> Introduce image.h and macros.h in this patch, just as Linux kernel.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> ---
>
> 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.

>
>  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.

>  #undef ENTRY
>  #undef ALIGN
>
> @@ -196,6 +197,10 @@ SECTIONS
>    .dtb : { *(.dtb) } :text
>  #endif
>
> +#if defined(__aarch64__)
> +  HEAD_SYMBOLS
> +#endif
> +
>    /* Sections to be discarded */
>    /DISCARD/ : {
>         *(.exit.text)
> diff --git a/xen/include/asm-arm/arm64/image.h b/xen/include/asm-arm/arm64/image.h
> new file mode 100644
> index 0000000..54751ca
> --- /dev/null
> +++ b/xen/include/asm-arm/arm64/image.h
> @@ -0,0 +1,62 @@
> +/*
> + * Copied and modified from Linux
> + * "commit 29b4817d4018df78086157ea3a55c1d9424a7cfc"
> + *
> + * Linker script macros to generate Image header fields.
> + *
> + * Copyright (C) 2014 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef __ASM_IMAGE_H
> +#define __ASM_IMAGE_H
> +
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +#define DATA_LE32(data)				\
> +	((((data) & 0x000000ff) << 24) |	\
> +	 (((data) & 0x0000ff00) << 8)  |	\
> +	 (((data) & 0x00ff0000) >> 8)  |	\
> +	 (((data) & 0xff000000) >> 24))
> +#else
> +#define DATA_LE32(data) ((data) & 0xffffffff)
> +#endif
> +
> +#define DEFINE_IMAGE_LE64(sym, data)				\
> +	sym##_lo32 = DATA_LE32((data) & 0xffffffff);		\
> +	sym##_hi32 = DATA_LE32((data) >> 32)
> +
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +#define __HEAD_FLAG_BE		1
> +#else
> +#define __HEAD_FLAG_BE		0
> +#endif
> +
> +#define __HEAD_FLAG_PAGE_SIZE	((PAGE_SHIFT - 10) / 2)
> +
> +#define __HEAD_FLAG_PHYS_BASE	1
> +
> +#define __HEAD_FLAGS		((__HEAD_FLAG_BE << 0) |	\
> +				 (__HEAD_FLAG_PAGE_SIZE << 1) |	\
> +				 (__HEAD_FLAG_PHYS_BASE << 3))
> +
> +/*
> + * These will output as part of the Image header, which should be little-endian
> + * regardless of the endianness of the kernel. While constant values could be
> + * endian swapped in head.S, all are done here for consistency.
> + */
> +#define HEAD_SYMBOLS						\
> +	DEFINE_IMAGE_LE64(_xen_size_le, _end - _start);		\
> +	DEFINE_IMAGE_LE64(_xen_offset_le, XEN_VIRT_START);	\
> +	DEFINE_IMAGE_LE64(_xen_flags_le, __HEAD_FLAGS);

Regards,
Peng Fan Sept. 12, 2016, 2:54 a.m. UTC | #2
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@nxp.com>
>>Cc: Stefano Stabellini <sstabellini@kernel.org>
>>Cc: Julien Grall <julien.grall@arm.com>
>>---
>>
>>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.
diff mbox

Patch

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>
 #undef ENTRY
 #undef ALIGN
 
@@ -196,6 +197,10 @@  SECTIONS
   .dtb : { *(.dtb) } :text
 #endif
 
+#if defined(__aarch64__)
+  HEAD_SYMBOLS
+#endif
+
   /* Sections to be discarded */
   /DISCARD/ : {
        *(.exit.text)
diff --git a/xen/include/asm-arm/arm64/image.h b/xen/include/asm-arm/arm64/image.h
new file mode 100644
index 0000000..54751ca
--- /dev/null
+++ b/xen/include/asm-arm/arm64/image.h
@@ -0,0 +1,62 @@ 
+/*
+ * Copied and modified from Linux
+ * "commit 29b4817d4018df78086157ea3a55c1d9424a7cfc"
+ *
+ * Linker script macros to generate Image header fields.
+ *
+ * Copyright (C) 2014 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __ASM_IMAGE_H
+#define __ASM_IMAGE_H
+
+#ifdef CONFIG_CPU_BIG_ENDIAN
+#define DATA_LE32(data)				\
+	((((data) & 0x000000ff) << 24) |	\
+	 (((data) & 0x0000ff00) << 8)  |	\
+	 (((data) & 0x00ff0000) >> 8)  |	\
+	 (((data) & 0xff000000) >> 24))
+#else
+#define DATA_LE32(data) ((data) & 0xffffffff)
+#endif
+
+#define DEFINE_IMAGE_LE64(sym, data)				\
+	sym##_lo32 = DATA_LE32((data) & 0xffffffff);		\
+	sym##_hi32 = DATA_LE32((data) >> 32)
+
+#ifdef CONFIG_CPU_BIG_ENDIAN
+#define __HEAD_FLAG_BE		1
+#else
+#define __HEAD_FLAG_BE		0
+#endif
+
+#define __HEAD_FLAG_PAGE_SIZE	((PAGE_SHIFT - 10) / 2)
+
+#define __HEAD_FLAG_PHYS_BASE	1
+
+#define __HEAD_FLAGS		((__HEAD_FLAG_BE << 0) |	\
+				 (__HEAD_FLAG_PAGE_SIZE << 1) |	\
+				 (__HEAD_FLAG_PHYS_BASE << 3))
+
+/*
+ * These will output as part of the Image header, which should be little-endian
+ * regardless of the endianness of the kernel. While constant values could be
+ * endian swapped in head.S, all are done here for consistency.
+ */
+#define HEAD_SYMBOLS						\
+	DEFINE_IMAGE_LE64(_xen_size_le, _end - _start);		\
+	DEFINE_IMAGE_LE64(_xen_offset_le, XEN_VIRT_START);	\
+	DEFINE_IMAGE_LE64(_xen_flags_le, __HEAD_FLAGS);
+
+#endif /* __ASM_IMAGE_H */
diff --git a/xen/include/asm-arm/arm64/macros.h b/xen/include/asm-arm/arm64/macros.h
new file mode 100644
index 0000000..25e57af
--- /dev/null
+++ b/xen/include/asm-arm/arm64/macros.h
@@ -0,0 +1,15 @@ 
+#ifndef __ASM_ARM_ARM64_MACROS_H
+#define __ASM_ARM_ARM64_MACROS_H
+
+/*
+ * Emit a 64-bit absolute little endian symbol reference in a way that
+ * ensures that it will be resolved at build time, even when building a
+ * PIE binary. This requires cooperation from the linker script, which
+ * must emit the lo32/hi32 halves individually.
+ */
+    .macro	le64sym, sym
+    .long	\sym\()_lo32
+    .long	\sym\()_hi32
+    .endm
+
+#endif /* __ASM_ARM_ARM32_MACROS_H */
diff --git a/xen/include/asm-arm/macros.h b/xen/include/asm-arm/macros.h
index 5d837cb..1d4bb41 100644
--- a/xen/include/asm-arm/macros.h
+++ b/xen/include/asm-arm/macros.h
@@ -8,7 +8,7 @@ 
 #if defined (CONFIG_ARM_32)
 # include <asm/arm32/macros.h>
 #elif defined(CONFIG_ARM_64)
-/* No specific ARM64 macros for now */
+# include <asm/arm64/macros.h>
 #else
 # error "unknown ARM variant"
 #endif