Re: [PATCH] xen/arm: skip first 32 bytes of zimage32

[Julien Grall <julien@xxxxxxx>]

Hi Stefano,

On 20/03/2022 01:05, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
>
> The first 32 bytes of zImage32 are NOPs, not useful just there for
> compatibility. The reason is that some bootloaders skip the first 32
> bytes when starting the kernel. See the comment in Linux
> arch/arm/boot/compressed/head.S.
Please mention the Linux verson.

> Since the introduction of CONFIG_EFI in Linux arm32, those NOPs
> operations have changed implementation from:
>
>      mov r0, r0
>
> to:
>      .inst   MZ_MAGIC | (0x1310 << 16)   @ tstne r0, #0x4d000
I have duplicated the comment and the instructions below:

                @ This is a two-instruction NOP, which happens to bear the
                @ PE/COFF signature "MZ" in the first two bytes, so the 
kernel
                @ is accepted as an EFI binary. Booting via the UEFI stub
                @ will not execute those instructions, but the ARM/Linux
                @ boot protocol does, so we need some NOPs here.
                .inst   MZ_MAGIC | (0xe225 << 16)       @ eor r5, r5, 
0x4d000
                eor     r5, r5, 0x4d000                 @ undo previous 
insn

I read this as they are NOPs and this change should not break the 
ARM/Linux boot protocol (we are using it in Xen).
BTW, the instruction decoding is different compare to me. Which version 
of Linux are you using?
> See arch/arm/boot/compressed/efi-header.S.
>
> The new implementation doesn't work on Xen (at least on all versions of
> QEMU I tried).
As I wrote above, they are NOPs. So why is this breaking?

> Since the first 32 bytes are made to be skipped anyway, skip them. This
> enables Xen to load and start successfully (on QEMU) aarch32 kernels
> with CONFIG_EFI.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> ---
>   xen/arch/arm/kernel.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 8f43caa186..105a010bf4 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -411,10 +411,10 @@ static int __init kernel_zimage32_probe(struct 
> kernel_info *info,
>           }
>       }
>   
> -    info->zimage.kernel_addr = addr;
> +    info->zimage.kernel_addr = addr + 32;
This will need some explanation in the code. The code in the tools will 
also need to be updated.
>   
>       info->zimage.start = start;
> -    info->zimage.len = end - start;
> +    info->zimage.len = end - start - 32;
>   
>       info->load = kernel_zimage_load;
>   
Cheers,

--
Julien Grall