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

[Tim Deegan <tim@xen.org>]

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.