[PATCH] xen/arm: build as zImage

[PATCH] 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.

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..de9cdb2 100644
--- a/xen/arch/arm/head.S
+++ b/xen/arch/arm/head.S
@@ -22,6 +22,9 @@
 #include <asm/processor-ca15.h>
 #include <asm/asm_defns.h>
 
+#define ZIMAGE_MAGIC_NUMBER 0x016f2818
+#define XEN_PHYS_ADDRESS    0x80000000
+
 #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 +55,18 @@
 	 * or the initial pagetable code below will need adjustment. */
 	.global start
 start:
+
+	.rept	7
+	mov	r0, r0
+	.endr
+	mov	r0, r0
+	b	1f
+
+	.word	ZIMAGE_MAGIC_NUMBER				@ Magic numbers to help the loader
+	.word	(_start + XEN_PHYS_ADDRESS)		@ absolute load/run zImage address
+	.word	(_end + XEN_PHYS_ADDRESS)		@ zImage end address
+
+1:
 	cpsid aif                    /* Disable all interrupts */
 
 	/* Save the bootloader arguments in less-clobberable registers */

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

[Ian Campbell]

On Fri, 2012-11-23 at 15:45 +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.
> 
> 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..de9cdb2 100644
> --- a/xen/arch/arm/head.S
> +++ b/xen/arch/arm/head.S
> @@ -22,6 +22,9 @@
>  #include <asm/processor-ca15.h>
>  #include <asm/asm_defns.h>
>  
> +#define ZIMAGE_MAGIC_NUMBER 0x016f2818
> +#define XEN_PHYS_ADDRESS    0x80000000

This is platform specific. AIUI you can put 0 in the field and the boot
loader is expected to do something sensible, although that might require
a new enough bootloader (FSVO new enough).

If that doesn't work then this constant is actually in
xen/arch/arm/Makefile too and for the same reason:
        # XXX: VE model loads by VMA so instead of
        # making a proper ELF we link with LMA == VMA and adjust crudely
        $(OBJCOPY) --change-addresses +0x80000000 $< $@
        $(STRIP) $@

How about making this CONFIG_PHYSICAL_LOCAL_ADDR and plumbing through
like we do with CONFIG_DTB_FILE?

> +
>  #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 +55,18 @@
>  	 * or the initial pagetable code below will need adjustment. */
>  	.global start
>  start:
> +
> +	.rept	7
> +	mov	r0, r0
> +	.endr
> +	mov	r0, r0
> +	b	1f
> +
> +	.word	ZIMAGE_MAGIC_NUMBER				@ Magic numbers to help the loader
> +	.word	(_start + XEN_PHYS_ADDRESS)		@ absolute load/run zImage address

Wacky spaces?

> +	.word	(_end + XEN_PHYS_ADDRESS)		@ zImage end address
> +
> +1:
>  	cpsid aif                    /* Disable all interrupts */
>  
>  	/* Save the bootloader arguments in less-clobberable registers */

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

[Stefano Stabellini]

On Fri, 23 Nov 2012, Ian Campbell wrote:
> On Fri, 2012-11-23 at 15:45 +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.
> > 
> > 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..de9cdb2 100644
> > --- a/xen/arch/arm/head.S
> > +++ b/xen/arch/arm/head.S
> > @@ -22,6 +22,9 @@
> >  #include <asm/processor-ca15.h>
> >  #include <asm/asm_defns.h>
> >  
> > +#define ZIMAGE_MAGIC_NUMBER 0x016f2818
> > +#define XEN_PHYS_ADDRESS    0x80000000
> 
> This is platform specific. AIUI you can put 0 in the field and the boot
> loader is expected to do something sensible, although that might require
> a new enough bootloader (FSVO new enough).

It is expected to work with bootloaders other than U-Boot?  My guess is
that it could work with U-Boot, because it uses its own format on top of
zImage, but that it won't probably work with anything else, for example
UEFI/arm, that at the moment seems to support only zImage.


> If that doesn't work then this constant is actually in
> xen/arch/arm/Makefile too and for the same reason:
>         # XXX: VE model loads by VMA so instead of
>         # making a proper ELF we link with LMA == VMA and adjust crudely
>         $(OBJCOPY) --change-addresses +0x80000000 $< $@
>         $(STRIP) $@
> 
> How about making this CONFIG_PHYSICAL_LOCAL_ADDR and plumbing through
> like we do with CONFIG_DTB_FILE?

Yeah, I was thinking about that.
However in this case we would want to set the default somewhere, rather
than forcing the user to specify it.
Also we would need to export CONFIG_PHYSICAL_LOCAL_ADDR to a generated
header file somewhere. Do you have any suggestions on where?

> > +
> >  #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 +55,18 @@
> >  	 * or the initial pagetable code below will need adjustment. */
> >  	.global start
> >  start:
> > +
> > +	.rept	7
> > +	mov	r0, r0
> > +	.endr
> > +	mov	r0, r0
> > +	b	1f
> > +
> > +	.word	ZIMAGE_MAGIC_NUMBER				@ Magic numbers to help the loader
> > +	.word	(_start + XEN_PHYS_ADDRESS)		@ absolute load/run zImage address
> 
> Wacky spaces?

No, they are all tabs, that's why they look out of place here

> 
> > +	.word	(_end + XEN_PHYS_ADDRESS)		@ zImage end address
> > +
> > +1:
> >  	cpsid aif                    /* Disable all interrupts */
> >  
> >  	/* Save the bootloader arguments in less-clobberable registers */
> 
> 
> 

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

[Tim Deegan]

Hi,

At 15:45 +0000 on 23 Nov (1353685528), Stefano Stabellini wrote:
> diff --git a/xen/arch/arm/head.S b/xen/arch/arm/head.S
> index 25c4cfe..de9cdb2 100644
> --- a/xen/arch/arm/head.S
> +++ b/xen/arch/arm/head.S
> @@ -22,6 +22,9 @@
>  #include <asm/processor-ca15.h>
>  #include <asm/asm_defns.h>
>  
> +#define ZIMAGE_MAGIC_NUMBER 0x016f2818
> +#define XEN_PHYS_ADDRESS    0x80000000

That's a platform-specific constant.  I guess if bootloaders require it
to be baked into the image, we have no choice, but I think it should live
in config.h alongside the equivalent magic for other hardware addresses.

Also, it should probably be called XEN_LOAD_ADDRESS or ZIMAGE_LOAD_ADDRESS
to stop people thinking they can use it for v<->p translations.

> +
>  #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 +55,18 @@
>  	 * or the initial pagetable code below will need adjustment. */
>  	.global start
>  start:
> +
> +	.rept	7
> +	mov	r0, r0
> +	.endr
> +	mov	r0, r0
> +	b	1f

Please comment this, and align it with the rest of the file (operands
aligned using spaces, 6 chars to the right of the instruction).

Does zImage require these to be nops or could we just start with 
'b real_start ; .skip 32'?

> +	.word	ZIMAGE_MAGIC_NUMBER				@ Magic numbers to help the loader

Please use C-style comments and stick to <80 characters.

> +	.word	(_start + XEN_PHYS_ADDRESS)		@ absolute load/run zImage address

Surely just (XEN_PHYS_ADDRESS).  Or is XEN_PHYS_ADDRESS actually an offset?

> +	.word	(_end + XEN_PHYS_ADDRESS)		@ zImage end address
> +
> +1:

I think this deserves a real label. :)

Cheers,

Tim.

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

[Ian Campbell]

On Fri, 2012-11-23 at 16:14 +0000, Stefano Stabellini wrote:
> On Fri, 23 Nov 2012, Ian Campbell wrote:
> > On Fri, 2012-11-23 at 15:45 +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.
> > > 
> > > 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..de9cdb2 100644
> > > --- a/xen/arch/arm/head.S
> > > +++ b/xen/arch/arm/head.S
> > > @@ -22,6 +22,9 @@
> > >  #include <asm/processor-ca15.h>
> > >  #include <asm/asm_defns.h>
> > >  
> > > +#define ZIMAGE_MAGIC_NUMBER 0x016f2818
> > > +#define XEN_PHYS_ADDRESS    0x80000000
> > 
> > This is platform specific. AIUI you can put 0 in the field and the boot
> > loader is expected to do something sensible, although that might require
> > a new enough bootloader (FSVO new enough).
> 
> It is expected to work with bootloaders other than U-Boot?  My guess is
> that it could work with U-Boot, because it uses its own format on top of
> zImage, but that it won't probably work with anything else, for example
> UEFI/arm, that at the moment seems to support only zImage.

I thought this was a feature of zImage not u-boot, u-boot has its own
header and ignores this field I think.

http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html
says:
        The start address is usually 0 as the zImage code is position
        independent.
        
Of course this has to be true in the modern multiplatform zImage world.

Looking at a random zImage:
        000000 e1a00000 e1a00000 e1a00000 e1a00000
        *
        000020 ea000002 016f2818 00000000 00220548
        
Where 016f2818 is the magic, 0 is the start and 220548 is the
end/length.

I knew I didn't imagine it ;-)

> > If that doesn't work then this constant is actually in
> > xen/arch/arm/Makefile too and for the same reason:
> >         # XXX: VE model loads by VMA so instead of
> >         # making a proper ELF we link with LMA == VMA and adjust crudely
> >         $(OBJCOPY) --change-addresses +0x80000000 $< $@
> >         $(STRIP) $@
> > 
> > How about making this CONFIG_PHYSICAL_LOCAL_ADDR and plumbing through
> > like we do with CONFIG_DTB_FILE?
> 
> Yeah, I was thinking about that.
> However in this case we would want to set the default somewhere, rather
> than forcing the user to specify it.
> Also we would need to export CONFIG_PHYSICAL_LOCAL_ADDR to a generated
> header file somewhere. Do you have any suggestions on where?

For the DTB we just do:
ifdef CONFIG_DTB_FILE
AFLAGS += -DCONFIG_DTB_FILE=\"$(CONFIG_DTB_FILE)\"
else
..

If indeed we need this then we should do it the same way, and probably
set the default == 0.

Ian

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

[Tim Deegan]

At 16:14 +0000 on 23 Nov (1353687242), Stefano Stabellini wrote:
> On Fri, 23 Nov 2012, Ian Campbell wrote:
> > If that doesn't work then this constant is actually in
> > xen/arch/arm/Makefile too and for the same reason:
> >         # XXX: VE model loads by VMA so instead of
> >         # making a proper ELF we link with LMA == VMA and adjust crudely
> >         $(OBJCOPY) --change-addresses +0x80000000 $< $@
> >         $(STRIP) $@
> > 
> > How about making this CONFIG_PHYSICAL_LOCAL_ADDR and plumbing through
> > like we do with CONFIG_DTB_FILE?

Good idea.  In fact it already exists, as CONFIG_LOAD_ADDRESS in
config/arm.mk, and should probably be used here, with a bit of $(())
magic to get the right offset.

> Yeah, I was thinking about that.
> However in this case we would want to set the default somewhere, rather
> than forcing the user to specify it.
> Also we would need to export CONFIG_PHYSICAL_LOCAL_ADDR to a generated
> header file somewhere. Do you have any suggestions on where?

We could pass it with -D when we're processing head.S, as we already do
for xen.lds.S.  (Though the XEN_PHYS_START name (un)used in xen.lds.S
should probably also be XEN_LOAD_ADDRESS).

Tim.

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

[Tim Deegan]

At 16:26 +0000 on 23 Nov (1353687991), Ian Campbell wrote:
> http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html
> says:
>         The start address is usually 0 as the zImage code is position
>         independent.
>         
> Of course this has to be true in the modern multiplatform zImage world.
> 
> Looking at a random zImage:
>         000000 e1a00000 e1a00000 e1a00000 e1a00000
>         *
>         000020 ea000002 016f2818 00000000 00220548
>         
> Where 016f2818 is the magic, 0 is the start and 220548 is the
> end/length.
> 
> I knew I didn't imagine it ;-)

Yay!  We should definitely do that then, at least until we get a report
of it not working. :)

Tim.

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

[Stefano Stabellini]

On Fri, 23 Nov 2012, Tim Deegan wrote:
> At 16:26 +0000 on 23 Nov (1353687991), Ian Campbell wrote:
> > http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html
> > says:
> >         The start address is usually 0 as the zImage code is position
> >         independent.
> >         
> > Of course this has to be true in the modern multiplatform zImage world.
> > 
> > Looking at a random zImage:
> >         000000 e1a00000 e1a00000 e1a00000 e1a00000
> >         *
> >         000020 ea000002 016f2818 00000000 00220548
> >         
> > Where 016f2818 is the magic, 0 is the start and 220548 is the
> > end/length.
> > 
> > I knew I didn't imagine it ;-)
> 
> Yay!  We should definitely do that then, at least until we get a report
> of it not working. :)

Ok, 0 it is then