[PATCH v3] xen:arm: Populate arm64 image header

[PATCH v3] xen:arm: Populate arm64 image header

[Amit Singh Tomar]

This patch adds image size and flags to XEN image header. It uses
those fields according to the updated Linux kernel image definition.

With this patch bootloader can now place XEN image anywhere in system
RAM at 2MB aligned address without to worry about relocation.
For instance, it fixes the XEN boot on Amlogic SoC where bootloader(U-BOOT)
always relocates the XEN image to an address range reserved for firmware data.

Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Changes since v2:
        * Undo changes in assembler.h(as suggested by Andre)
Changes since v1:
        * Updated commit message
        * Removed endianess code
---
 xen/arch/arm/arm64/head.S | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index d63734f..ef87b5c 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -32,6 +32,13 @@
 #define PT_DEV    0xe71 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=0 P=1 */
 #define PT_DEV_L3 0xe73 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=1 P=1 */
 
+#define __HEAD_FLAG_PAGE_SIZE   ((PAGE_SHIFT - 10) / 2)
+
+#define __HEAD_FLAG_PHYS_BASE   1
+
+#define __HEAD_FLAGS            ((__HEAD_FLAG_PAGE_SIZE << 1) | \
+                                 (__HEAD_FLAG_PHYS_BASE << 3))
+
 #if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
 #include EARLY_PRINTK_INC
 #endif
@@ -120,8 +127,8 @@ 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 */
+        .quad   _end - start         /* Effective size of kernel image, little-endian */
+        .quad   __HEAD_FLAGS         /* Informative flags, little-endian */
         .quad   0                    /* reserved */
         .quad   0                    /* reserved */
         .quad   0                    /* reserved */
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3] xen:arm: Populate arm64 image header

[Andre Przywara]

Hi,

On 11/09/2018 17:48, Amit Singh Tomar wrote:
> This patch adds image size and flags to XEN image header. It uses
> those fields according to the updated Linux kernel image definition.
> 
> With this patch bootloader can now place XEN image anywhere in system
> RAM at 2MB aligned address without to worry about relocation.
> For instance, it fixes the XEN boot on Amlogic SoC where bootloader(U-BOOT)
> always relocates the XEN image to an address range reserved for firmware data.
> 
> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>

Thanks, looks good to me now:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre.

> ---
> Changes since v2:
>         * Undo changes in assembler.h(as suggested by Andre)
> Changes since v1:
>         * Updated commit message
>         * Removed endianess code
> ---
>  xen/arch/arm/arm64/head.S | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index d63734f..ef87b5c 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -32,6 +32,13 @@
>  #define PT_DEV    0xe71 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=0 P=1 */
>  #define PT_DEV_L3 0xe73 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=1 P=1 */
>  
> +#define __HEAD_FLAG_PAGE_SIZE   ((PAGE_SHIFT - 10) / 2)
> +
> +#define __HEAD_FLAG_PHYS_BASE   1
> +
> +#define __HEAD_FLAGS            ((__HEAD_FLAG_PAGE_SIZE << 1) | \
> +                                 (__HEAD_FLAG_PHYS_BASE << 3))
> +
>  #if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
>  #include EARLY_PRINTK_INC
>  #endif
> @@ -120,8 +127,8 @@ 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 */
> +        .quad   _end - start         /* Effective size of kernel image, little-endian */
> +        .quad   __HEAD_FLAGS         /* Informative flags, little-endian */
>          .quad   0                    /* reserved */
>          .quad   0                    /* reserved */
>          .quad   0                    /* reserved */
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3] xen:arm: Populate arm64 image header

[Julien Grall]

On 09/12/2018 10:03 AM, Andre Przywara wrote:
> Hi,
> 
> On 11/09/2018 17:48, Amit Singh Tomar wrote:
>> This patch adds image size and flags to XEN image header. It uses
>> those fields according to the updated Linux kernel image definition.
>>
>> With this patch bootloader can now place XEN image anywhere in system
>> RAM at 2MB aligned address without to worry about relocation.
>> For instance, it fixes the XEN boot on Amlogic SoC where bootloader(U-BOOT)
>> always relocates the XEN image to an address range reserved for firmware data.
>>
>> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> 
> Thanks, looks good to me now:
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Acked-by: Julien Grall <julien.grall@arm.com>

And commited.

Cheers,

> 
> Cheers,
> Andre.
> 
>> ---
>> Changes since v2:
>>          * Undo changes in assembler.h(as suggested by Andre)
>> Changes since v1:
>>          * Updated commit message
>>          * Removed endianess code
>> ---
>>   xen/arch/arm/arm64/head.S | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
>> index d63734f..ef87b5c 100644
>> --- a/xen/arch/arm/arm64/head.S
>> +++ b/xen/arch/arm/arm64/head.S
>> @@ -32,6 +32,13 @@
>>   #define PT_DEV    0xe71 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=0 P=1 */
>>   #define PT_DEV_L3 0xe73 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=1 P=1 */
>>   
>> +#define __HEAD_FLAG_PAGE_SIZE   ((PAGE_SHIFT - 10) / 2)
>> +
>> +#define __HEAD_FLAG_PHYS_BASE   1
>> +
>> +#define __HEAD_FLAGS            ((__HEAD_FLAG_PAGE_SIZE << 1) | \
>> +                                 (__HEAD_FLAG_PHYS_BASE << 3))
>> +
>>   #if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
>>   #include EARLY_PRINTK_INC
>>   #endif
>> @@ -120,8 +127,8 @@ 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 */
>> +        .quad   _end - start         /* Effective size of kernel image, little-endian */
>> +        .quad   __HEAD_FLAGS         /* Informative flags, little-endian */
>>           .quad   0                    /* reserved */
>>           .quad   0                    /* reserved */
>>           .quad   0                    /* reserved */
>>

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3] xen:arm: Populate arm64 image header

[Stewart Hildebrand]

On 11/09/2018 17:48, Amit Singh Tomar wrote:
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index d63734f..ef87b5c 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -120,8 +127,8 @@ 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 */
> +        .quad   _end - start         /* Effective size of kernel image, little-endian */
> +        .quad   __HEAD_FLAGS         /* Informative flags, little-endian */
>           .quad   0                    /* reserved */
>           .quad   0                    /* reserved */
>           .quad   0                    /* reserved */

Since 17bd254a xen:arm: Populate arm64 image header, qemu-system-aarch64 has not been too happy about booting Xen.

Trying to launch qemu-system-aarch64 gives the following error:
rom: requested regions overlap (rom bootloader. free=0x00000000400d0150, addr=0x0000000040000000)
qemu-system-aarch64: rom check and register reset failed

Reverting 17bd254a allowed it to boot again. Alternatively, setting the image offset to some value allowed it to boot again.
 
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index ef87b5c..8879c77 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -126,7 +126,7 @@ efi_head:
          */
         add     x13, x18, #0x16
         b       real_start           /* branch to kernel start */
-        .quad   0                    /* Image load offset from start of RAM */
+        .quad   0x00080000           /* Image load offset from start of RAM */
         .quad   _end - start         /* Effective size of kernel image, little-endian */
         .quad   __HEAD_FLAGS         /* Informative flags, little-endian */
         .quad   0                    /* reserved */

I'm not sure if this is a fault of qemu, or if Xen should put some value in the image load offset field?

For reference, I'm using the following script to build and launch qemu+Xen https://gist.github.com/stewdk/110f43e0cc1d905fc6ed4c7e10d8d35e
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Xen boot failure on QEMU (WAS: Re: [PATCH v3] xen:arm: Populate arm64 image header)

[Julien Grall]

(+ Peter Maydell and Stefano)

Hi Steward,

Thank you for the bug report.

On 05/10/2018 23:17, Stewart Hildebrand wrote:
> On 11/09/2018 17:48, Amit Singh Tomar wrote:
>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
>> index d63734f..ef87b5c 100644
>> --- a/xen/arch/arm/arm64/head.S
>> +++ b/xen/arch/arm/arm64/head.S
>> @@ -120,8 +127,8 @@ 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 */
>> +        .quad   _end - start         /* Effective size of kernel image, little-endian */
>> +        .quad   __HEAD_FLAGS         /* Informative flags, little-endian */
>>            .quad   0                    /* reserved */
>>            .quad   0                    /* reserved */
>>            .quad   0                    /* reserved */
> 
> Since 17bd254a xen:arm: Populate arm64 image header, qemu-system-aarch64 has not been too happy about booting Xen.
> 
> Trying to launch qemu-system-aarch64 gives the following error:
> rom: requested regions overlap (rom bootloader. free=0x00000000400d0150, addr=0x0000000040000000)
> qemu-system-aarch64: rom check and register reset failed
> 
> Reverting 17bd254a allowed it to boot again. Alternatively, setting the image offset to some value allowed it to boot again.
>   
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index ef87b5c..8879c77 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -126,7 +126,7 @@ efi_head:
>            */
>           add     x13, x18, #0x16
>           b       real_start           /* branch to kernel start */
> -        .quad   0                    /* Image load offset from start of RAM */
> +        .quad   0x00080000           /* Image load offset from start of RAM */
>           .quad   _end - start         /* Effective size of kernel image, little-endian */
>           .quad   __HEAD_FLAGS         /* Informative flags, little-endian */
>           .quad   0                    /* reserved */
> 
> I'm not sure if this is a fault of qemu, or if Xen should put some value in the image load offset field?

Per the Linux arm64 booting protocol [1], the load offset can definitely 
be 0. The bootloader (here QEMU) should not assume a specific text 
offset, Linux actually provides an option to randomize the text offset 
in order to test that assumption (see ARM64_RANDOMIZE_TEXT_OFFSET).

I have CCed Stefano and Peter who could give more details on how QEMU is 
handling the Image protocol.

> 
> For reference, I'm using the following script to build and launch qemu+Xen https://gist.github.com/stewdk/110f43e0cc1d905fc6ed4c7e10d8d35e
> 

Cheers,

[1] https://www.kernel.org/doc/Documentation/arm64/booting.txt

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: Xen boot failure on QEMU (WAS: Re: [PATCH v3] xen:arm: Populate arm64 image header)

[Peter Maydell]

On 8 October 2018 at 19:00, Julien Grall <julien.grall@arm.com> wrote:
> Per the Linux arm64 booting protocol [1], the load offset can definitely be
> 0. The bootloader (here QEMU) should not assume a specific text offset,
> Linux actually provides an option to randomize the text offset in order to
> test that assumption (see ARM64_RANDOMIZE_TEXT_OFFSET).
>
> I have CCed Stefano and Peter who could give more details on how QEMU is
> handling the Image protocol.

QEMU's code to handle this is in hw/arm/boot.c/load_aarch64_image().
We do have code to handle reading the text offset from the Image
header. The error message
rom: requested regions overlap (rom bootloader.
free=0x00000000400d0150, addr=0x0000000040000000)

suggests that we have correctly pulled the text offset from the
Image file, but it overlaps with something else. Specifically,
for the virt board we write our startup "bootloader" (about
a dozen insns of assembly that do the job of jumping to the kernel)
at the very bottom of RAM, so you can't use that bit of RAM for
the image. In theory boot.c could notice that the image is where
it would by default put the bootloader code and move that somewhere
else (it is IIRC position-independent), but that would be quite
complicated code for a corner case, and it doesn't help you with
existing deployed versions of QEMU anyway.

thanks
-- PMM

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: Xen boot failure on QEMU (WAS: Re: [PATCH v3] xen:arm: Populate arm64 image header)

[Andre Przywara]

On Tue, 9 Oct 2018 09:58:14 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

Hi,

> On 8 October 2018 at 19:00, Julien Grall <julien.grall@arm.com> wrote:
> > Per the Linux arm64 booting protocol [1], the load offset can
> > definitely be 0. The bootloader (here QEMU) should not assume a
> > specific text offset, Linux actually provides an option to
> > randomize the text offset in order to test that assumption (see
> > ARM64_RANDOMIZE_TEXT_OFFSET).
> >
> > I have CCed Stefano and Peter who could give more details on how
> > QEMU is handling the Image protocol.  
> 
> QEMU's code to handle this is in hw/arm/boot.c/load_aarch64_image().
> We do have code to handle reading the text offset from the Image
> header. The error message
> rom: requested regions overlap (rom bootloader.
> free=0x00000000400d0150, addr=0x0000000040000000)
> 
> suggests that we have correctly pulled the text offset from the
> Image file, but it overlaps with something else. Specifically,
> for the virt board we write our startup "bootloader" (about
> a dozen insns of assembly that do the job of jumping to the kernel)
> at the very bottom of RAM, so you can't use that bit of RAM for
> the image. In theory boot.c could notice that the image is where
> it would by default put the bootloader code and move that somewhere
> else (it is IIRC position-independent), but that would be quite
> complicated code for a corner case, and it doesn't help you with
> existing deployed versions of QEMU anyway.

While the latter is true, QEMU still violates the kernel arm64
booting protocol:
"The Image must be placed text_offset bytes from a 2MB aligned base
address anywhere in usable system RAM and called there."

I tried some values yesterday, just from those experiments it seems
like any 4K aligned values would work for Xen. So what about the
following:
- We use a 4K text_offset for Xen, to work with existing QEMUs.
- QEMU switches the kernel image load address to
  DRAM + 2MB + text_offset, when it sees that text_offset is smaller
  than the "startup bootloader" size.
That way people can upgrade either of Xen or QEMU to fix the problem,
and QEMU becomes more kernel boot protocol compliant.

Happy to provide the trivial Xen patch ;-)

Cheers,
Andre.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel