[PATCH v2] xen/arm: build as zImage

[PATCH v2] xen/arm: build as zImage

[Stefano Stabellini]

The zImage format is extremely simple: it only consists of a magic
number and 2 addresses in a specific position (see
http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html#d0e309).

Some bootloaders expect a zImage; considering that it doesn't cost us
much to build Xen compatible with the format, make it so.

Changes in v2:
- code style;
- pass 0 a start address.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/xen/arch/arm/head.S b/xen/arch/arm/head.S
index 25c4cfe..347ddcf 100644
--- a/xen/arch/arm/head.S
+++ b/xen/arch/arm/head.S
@@ -22,6 +22,8 @@
 #include <asm/processor-ca15.h>
 #include <asm/asm_defns.h>
 
+#define ZIMAGE_MAGIC_NUMBER 0x016f2818
+
 #define PT_PT  0xe7f /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=111, T=1, P=1 */
 #define PT_MEM 0xe7d /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=111, T=0, P=1 */
 #define PT_DEV 0xe71 /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=100, T=0, P=1 */
@@ -52,6 +54,18 @@
 	 * or the initial pagetable code below will need adjustment. */
 	.global start
 start:
+
+	/* zImage magic header, see:
+	 * http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html#d0e309
+	 */
+	.skip 0x20
+	b     past_zImage
+	.word ZIMAGE_MAGIC_NUMBER    /* Magic numbers to help the loader */
+	.word 0x00000000             /* absolute load/run zImage address or
+	                              * 0 for PiC */
+	.word _end                   /* zImage end address */
+
+past_zImage:
 	cpsid aif                    /* Disable all interrupts */
 
 	/* Save the bootloader arguments in less-clobberable registers */

Re: [PATCH v2] xen/arm: build as zImage

[Ian Campbell]

On Fri, 2012-11-23 at 18:04 +0000, Stefano Stabellini wrote:
> The zImage format is extremely simple: it only consists of a magic
> number and 2 addresses in a specific position (see
> http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html#d0e309).
> 
> Some bootloaders expect a zImage; considering that it doesn't cost us
> much to build Xen compatible with the format, make it so.
> 
> Changes in v2:
> - code style;
> - pass 0 a start address.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> diff --git a/xen/arch/arm/head.S b/xen/arch/arm/head.S
> index 25c4cfe..347ddcf 100644
> --- a/xen/arch/arm/head.S
> +++ b/xen/arch/arm/head.S
> @@ -22,6 +22,8 @@
>  #include <asm/processor-ca15.h>
>  #include <asm/asm_defns.h>
>  
> +#define ZIMAGE_MAGIC_NUMBER 0x016f2818
> +
>  #define PT_PT  0xe7f /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=111, T=1, P=1 */
>  #define PT_MEM 0xe7d /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=111, T=0, P=1 */
>  #define PT_DEV 0xe71 /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=100, T=0, P=1 */
> @@ -52,6 +54,18 @@
>  	 * or the initial pagetable code below will need adjustment. */
>  	.global start
>  start:
> +
> +	/* zImage magic header, see:
> +	 * http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html#d0e309
> +	 */
> +	.skip 0x20

This was better as the explicit repeated nops, as it stands it's not at
all clear that you can start executing at start. It might even rely on
the specific assemblers behaviour?

Also this wasn't what Tim suggested, which was branch then skip, not
skip then branch. I think the nops are better for consistency with Linux
though.

> +	b     past_zImage
> +	.word ZIMAGE_MAGIC_NUMBER    /* Magic numbers to help the loader */
> +	.word 0x00000000             /* absolute load/run zImage address or
> +	                              * 0 for PiC */
> +	.word _end                   /* zImage end address */

This needs to be "_end - _start".

Ian.

Re: [PATCH v2] xen/arm: build as zImage

[Tim Deegan]

At 18:04 +0000 on 23 Nov (1353693893), Stefano Stabellini wrote:
> The zImage format is extremely simple: it only consists of a magic
> number and 2 addresses in a specific position (see
> http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html#d0e309).
> 
> Some bootloaders expect a zImage; considering that it doesn't cost us
> much to build Xen compatible with the format, make it so.
> 
> Changes in v2:
> - code style;
> - pass 0 a start address.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> diff --git a/xen/arch/arm/head.S b/xen/arch/arm/head.S
> index 25c4cfe..347ddcf 100644
> --- a/xen/arch/arm/head.S
> +++ b/xen/arch/arm/head.S
> @@ -22,6 +22,8 @@
>  #include <asm/processor-ca15.h>
>  #include <asm/asm_defns.h>
>  
> +#define ZIMAGE_MAGIC_NUMBER 0x016f2818
> +
>  #define PT_PT  0xe7f /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=111, T=1, P=1 */
>  #define PT_MEM 0xe7d /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=111, T=0, P=1 */
>  #define PT_DEV 0xe71 /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=100, T=0, P=1 */
> @@ -52,6 +54,18 @@
>  	 * or the initial pagetable code below will need adjustment. */
>  	.global start
>  start:
> +
> +	/* zImage magic header, see:
> +	 * http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html#d0e309
> +	 */
> +	.skip 0x20
> +	b     past_zImage

Surely branch _before_ the padding?  

> +	.word ZIMAGE_MAGIC_NUMBER    /* Magic numbers to help the loader */
> +	.word 0x00000000             /* absolute load/run zImage address or
> +	                              * 0 for PiC */
> +	.word _end                   /* zImage end address */

Not (_end - _start)?

Tim.

Re: [PATCH v2] xen/arm: build as zImage

[Stefano Stabellini]

On Fri, 23 Nov 2012, Ian Campbell wrote:
> On Fri, 2012-11-23 at 18:04 +0000, Stefano Stabellini wrote:
> > The zImage format is extremely simple: it only consists of a magic
> > number and 2 addresses in a specific position (see
> > http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html#d0e309).
> > 
> > Some bootloaders expect a zImage; considering that it doesn't cost us
> > much to build Xen compatible with the format, make it so.
> > 
> > Changes in v2:
> > - code style;
> > - pass 0 a start address.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > diff --git a/xen/arch/arm/head.S b/xen/arch/arm/head.S
> > index 25c4cfe..347ddcf 100644
> > --- a/xen/arch/arm/head.S
> > +++ b/xen/arch/arm/head.S
> > @@ -22,6 +22,8 @@
> >  #include <asm/processor-ca15.h>
> >  #include <asm/asm_defns.h>
> >  
> > +#define ZIMAGE_MAGIC_NUMBER 0x016f2818
> > +
> >  #define PT_PT  0xe7f /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=111, T=1, P=1 */
> >  #define PT_MEM 0xe7d /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=111, T=0, P=1 */
> >  #define PT_DEV 0xe71 /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=100, T=0, P=1 */
> > @@ -52,6 +54,18 @@
> >  	 * or the initial pagetable code below will need adjustment. */
> >  	.global start
> >  start:
> > +
> > +	/* zImage magic header, see:
> > +	 * http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html#d0e309
> > +	 */
> > +	.skip 0x20
> 
> This was better as the explicit repeated nops, as it stands it's not at
> all clear that you can start executing at start. It might even rely on
> the specific assemblers behaviour?
>
> Also this wasn't what Tim suggested, which was branch then skip, not
> skip then branch. I think the nops are better for consistency with Linux
> though.

You are right. Thinking twice about it, I would rather keep the code as
it was originally, for consistency with Linux.


> 
> > +	b     past_zImage
> > +	.word ZIMAGE_MAGIC_NUMBER    /* Magic numbers to help the loader */
> > +	.word 0x00000000             /* absolute load/run zImage address or
> > +	                              * 0 for PiC */
> > +	.word _end                   /* zImage end address */
> 
> This needs to be "_end - _start".

Yep.

Re: [PATCH v2] xen/arm: build as zImage

[Tim Deegan]

At 12:11 +0000 on 27 Nov (1354018273), Stefano Stabellini wrote:
> On Fri, 23 Nov 2012, Ian Campbell wrote:
> > On Fri, 2012-11-23 at 18:04 +0000, Stefano Stabellini wrote:
> > > The zImage format is extremely simple: it only consists of a magic
> > > number and 2 addresses in a specific position (see
> > > http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html#d0e309).
> > > 
> > > Some bootloaders expect a zImage; considering that it doesn't cost us
> > > much to build Xen compatible with the format, make it so.
> > > 
> > > Changes in v2:
> > > - code style;
> > > - pass 0 a start address.
> > > 
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > 
> > > diff --git a/xen/arch/arm/head.S b/xen/arch/arm/head.S
> > > index 25c4cfe..347ddcf 100644
> > > --- a/xen/arch/arm/head.S
> > > +++ b/xen/arch/arm/head.S
> > > @@ -22,6 +22,8 @@
> > >  #include <asm/processor-ca15.h>
> > >  #include <asm/asm_defns.h>
> > >  
> > > +#define ZIMAGE_MAGIC_NUMBER 0x016f2818
> > > +
> > >  #define PT_PT  0xe7f /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=111, T=1, P=1 */
> > >  #define PT_MEM 0xe7d /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=111, T=0, P=1 */
> > >  #define PT_DEV 0xe71 /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=100, T=0, P=1 */
> > > @@ -52,6 +54,18 @@
> > >  	 * or the initial pagetable code below will need adjustment. */
> > >  	.global start
> > >  start:
> > > +
> > > +	/* zImage magic header, see:
> > > +	 * http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html#d0e309
> > > +	 */
> > > +	.skip 0x20
> > 
> > This was better as the explicit repeated nops, as it stands it's not at
> > all clear that you can start executing at start. It might even rely on
> > the specific assemblers behaviour?
> >
> > Also this wasn't what Tim suggested, which was branch then skip, not
> > skip then branch. I think the nops are better for consistency with Linux
> > though.
> 
> You are right. Thinking twice about it, I would rather keep the code as
> it was originally, for consistency with Linux.

Does consistency with linux buy us anything (beyond untidy code)?
I very much prefer the other way.  And if we do need to fill with nops,
why 7 repeated and one standalone?

Tim.

Re: [PATCH v2] xen/arm: build as zImage

[Ian Campbell]

On Tue, 2012-11-27 at 13:58 +0000, Tim Deegan wrote:
> At 12:11 +0000 on 27 Nov (1354018273), Stefano Stabellini wrote:
> > On Fri, 23 Nov 2012, Ian Campbell wrote:
> > > On Fri, 2012-11-23 at 18:04 +0000, Stefano Stabellini wrote:
> > > > The zImage format is extremely simple: it only consists of a magic
> > > > number and 2 addresses in a specific position (see
> > > > http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html#d0e309).
> > > > 
> > > > Some bootloaders expect a zImage; considering that it doesn't cost us
> > > > much to build Xen compatible with the format, make it so.
> > > > 
> > > > Changes in v2:
> > > > - code style;
> > > > - pass 0 a start address.
> > > > 
> > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > > 
> > > > diff --git a/xen/arch/arm/head.S b/xen/arch/arm/head.S
> > > > index 25c4cfe..347ddcf 100644
> > > > --- a/xen/arch/arm/head.S
> > > > +++ b/xen/arch/arm/head.S
> > > > @@ -22,6 +22,8 @@
> > > >  #include <asm/processor-ca15.h>
> > > >  #include <asm/asm_defns.h>
> > > >  
> > > > +#define ZIMAGE_MAGIC_NUMBER 0x016f2818
> > > > +
> > > >  #define PT_PT  0xe7f /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=111, T=1, P=1 */
> > > >  #define PT_MEM 0xe7d /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=111, T=0, P=1 */
> > > >  #define PT_DEV 0xe71 /* nG=1, AF=1, SH=10, AP=01, NS=1, ATTR=100, T=0, P=1 */
> > > > @@ -52,6 +54,18 @@
> > > >  	 * or the initial pagetable code below will need adjustment. */
> > > >  	.global start
> > > >  start:
> > > > +
> > > > +	/* zImage magic header, see:
> > > > +	 * http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html#d0e309
> > > > +	 */
> > > > +	.skip 0x20
> > > 
> > > This was better as the explicit repeated nops, as it stands it's not at
> > > all clear that you can start executing at start. It might even rely on
> > > the specific assemblers behaviour?
> > >
> > > Also this wasn't what Tim suggested, which was branch then skip, not
> > > skip then branch. I think the nops are better for consistency with Linux
> > > though.
> > 
> > You are right. Thinking twice about it, I would rather keep the code as
> > it was originally, for consistency with Linux.
> 
> Does consistency with linux buy us anything (beyond untidy code)?
> I very much prefer the other way.

I suspect the reason for this run of nops is some buggy bootloader which
jumped to byte 4*N instead of the start, for whatever reason. They may
well be historical but I don't see any reason to risk it.

Given that we are mimicing Linux's protocol I think remaining blindly
compatible is worth it.

>   And if we do need to fill with nops,
> why 7 repeated and one standalone?

In the original Linux it is actually:
                mov     r0, r0
                .endr
   ARM(         mov     r0, r0          )
   ARM(         b       1f              )
 THUMB(         adr     r12, BSYM(1f)   )
 THUMB(         bx      r12             )

i.e. once you strip out the thumb stuff (because we don't do thumb mode
hypervisor) you end up with what Stefano had. So indeed we could just do
8 in our case.

Ian.