[PATCH 0/7] boot: cleanup and prep for more platforms

[PATCH 0/7] boot: cleanup and prep for more platforms

[Milton Miller]

What follows are the first 7 of my 13 patch series working on a zImage
that kexec will boot to, at least on 64 bit boxes.

The first one makes the Makefile maintainable, the second and third
make the Makefile work correctly.  The forth is a cleanup for a 
common idiom, the fifth adds some missing relocations, and the
sixth and seventh allow other code to reuse the existing functions
after necessary setup.

All have been compile and link tested, and were created against
a 2.6.20 tree + the 20 patches I found related to the boot directory
in powerpc.git + patchwork numbers 10010 and 10012 (which seemed
to be moving that direction).  I have included additional notes
in some of the patches with the testing.  I think all could be
applied to for-2.6.22.

The last 4, that I am not posting at this time, build upon these
to create a zImage for use by 64-bit kexec, add a sreset based
derivative target, and allow for disjoint vmlinuz payloads.  I hope
to be able to find time in the near future to debug and post
them.

milton

boot-zImage-rule
boot-add-wrapper-to-wrapperbits
boot-add-FORCE
boot-add-fatal
boot-crt0-missing-reloc
boot-crt0-export
boot-export-flush-cache

[PATCH 3/7] boot: use FORCE

[Milton Miller]

Kbuild if_changed and if_changed_dep require the use of the dummy
FORCE to get the dependencies right.   Also add to targets to get
correct behavior.

Signed-off-by: Milton Miller <miltonm@bga.com>
--- 
Testing: Several repeat runs with V=2 and going up and down this series.
Without this, there were several missed rebuilds.
 Makefile |   24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

Index: kernel/arch/powerpc/boot/Makefile
===================================================================
--- kernel.orig/arch/powerpc/boot/Makefile	2007-03-19 09:15:06.000000000 -0500
+++ kernel/arch/powerpc/boot/Makefile	2007-03-19 09:15:19.000000000 -0500
@@ -85,24 +85,25 @@ quiet_cmd_bootas = BOOTAS  $@
       cmd_bootas = $(CROSS32CC) -Wp,-MD,$(depfile) $(BOOTAFLAGS) -c -o $@ $<
 
 quiet_cmd_bootar = BOOTAR  $@
-      cmd_bootar = $(CROSS32AR) -cr $@.$$$$ $^; mv $@.$$$$ $@
+      cmd_bootar = $(CROSS32AR) -cr $@.$$$$ $(filter-out FORCE,$^); mv $@.$$$$ $@
 
-$(patsubst %.c,%.o, $(filter %.c, $(src-boot))): %.o: %.c
+$(patsubst %.c,%.o, $(filter %.c, $(src-boot))): %.o: %.c FORCE
 	$(call if_changed_dep,bootcc)
-$(patsubst %.S,%.o, $(filter %.S, $(src-boot))): %.o: %.S
+$(patsubst %.S,%.o, $(filter %.S, $(src-boot))): %.o: %.S FORCE
 	$(call if_changed_dep,bootas)
 
-$(obj)/wrapper.a: $(obj-wlib)
-	$(call cmd,bootar)
+$(obj)/wrapper.a: $(obj-wlib) FORCE
+	$(call if_changed,bootar)
 
 hostprogs-y	:= addnote addRamDisk hack-coff mktree
 
+targets		+= $(patsubst $(obj)/%,%,$(obj-boot) wrapper.a)
 extra-y		:= $(obj)/wrapper.a $(obj-plat) $(obj)/empty.o \
 		   $(obj)/zImage.lds $(obj)/zImage.coff.lds
 
 wrapper		:=$(srctree)/$(src)/wrapper
 wrapperbits	:= $(extra-y) $(addprefix $(obj)/,addnote hack-coff mktree) \
-			$(wrapper)
+			$(wrapper) FORCE
 
 #############
 # Bits for building various flavours of zImage
@@ -127,7 +128,7 @@ $(obj)/zImage.initrd.ps3: vmlinux
 	@echo "  WARNING zImage.initrd.ps3 not supported (yet)"
 
 $(obj)/uImage: vmlinux $(wrapperbits)
-	$(call cmd,wrap,uboot)
+	$(call if_changed,wrap,uboot)
 
 image-$(CONFIG_PPC_PSERIES)		+= zImage.pseries
 image-$(CONFIG_PPC_MAPLE)		+= zImage.pseries
@@ -146,19 +147,20 @@ image-$(CONFIG_PPC_PMAC)	+= zImage.coff 
 endif
 
 initrd-y := $(patsubst zImage%, zImage.initrd%, $(image-y))
+targets	+= $(image-y) $(initrd-y)
 
 $(addprefix $(obj)/,$(filter-out uImage zImage.ps3,$(image-y))): \
 		$(obj)/zImage.%: vmlinux $(wrapperbits)
-	$(call cmd,wrap,$(patsubst $(obj)/zImage.%,%,$@))
+	$(call if_changed,wrap,$(patsubst $(obj)/zImage.%,%,$@))
 
 $(addprefix $(obj)/,$(filter-out uImage zImage.initrd.ps3,$(initrd-y))): \
 		$(obj)/zImage.initrd.%: vmlinux $(wrapperbits) \
 		$(obj)/ramdisk.image.gz
-	$(call cmd,wrap_initrd,$(patsubst $(obj)/zImage.initrd.%,%,$@),,,$(obj)/ramdisk.image.gz)
+	$(call if_changed,wrap,$(patsubst $(obj)/zImage.initrd.%,%,$@),,,$(obj)/ramdisk.image.gz)
 
-$(obj)/zImage:		$(addprefix $(obj)/, $(image-y))
+$(obj)/zImage:		$(addprefix $(obj)/, $(image-y)) FORCE
 	@rm -f $@; ln $< $@
-$(obj)/zImage.initrd:	$(addprefix $(obj)/, $(initrd-y))
+$(obj)/zImage.initrd:	$(addprefix $(obj)/, $(initrd-y)) FORCE
 	@rm -f $@; ln $< $@
 
 install: $(CONFIGURE) $(image-y)

[PATCH 4/7] bootwrapper: add a fatal error helper

[Milton Miller]

Add a macro fatal that calls printf then exit.  User must include stdio.h.

Typically replaces 3 lines with 1, although I added back some whitespace.

Signed-off-by: Milton Miller <miltonm@bga.com>
--- 
 gunzip_util.c |   36 ++++++++++++------------------------
 main.c        |   30 +++++++++++-------------------
 of.c          |    7 +++----
 ops.h         |    2 ++
 4 files changed, 28 insertions(+), 47 deletions(-)

Index: kernel/arch/powerpc/boot/main.c
===================================================================
--- kernel.orig/arch/powerpc/boot/main.c	2007-03-19 08:32:50.000000000 -0500
+++ kernel/arch/powerpc/boot/main.c	2007-03-19 08:33:02.000000000 -0500
@@ -120,10 +120,9 @@ static struct addr_range prep_kernel(voi
 	gunzip_start(&gzstate, vmlinuz_addr, vmlinuz_size);
 	gunzip_exactly(&gzstate, elfheader, sizeof(elfheader));
 
-	if (!parse_elf64(elfheader, &ei) && !parse_elf32(elfheader, &ei)) {
-		printf("Error: not a valid PPC32 or PPC64 ELF file!\n\r");
-		exit();
-	}
+	if (!parse_elf64(elfheader, &ei) && !parse_elf32(elfheader, &ei))
+		fatal("Error: not a valid PPC32 or PPC64 ELF file!\n\r");
+
 	if (platform_ops.image_hdr)
 		platform_ops.image_hdr(elfheader);
 
@@ -137,11 +136,9 @@ static struct addr_range prep_kernel(voi
 	if (platform_ops.vmlinux_alloc) {
 		addr = platform_ops.vmlinux_alloc(ei.memsize);
 	} else {
-		if ((unsigned long)_start < ei.memsize) {
-			printf("Insufficient memory for kernel at address 0!"
+		if ((unsigned long)_start < ei.memsize)
+			fatal("Insufficient memory for kernel at address 0!"
 			       " (_start=%lx)\n\r", _start);
-			exit();
-		}
 	}
 
 	/* Finally, gunzip the kernel */
@@ -191,11 +188,9 @@ static struct addr_range prep_initrd(str
 		printf("Allocating 0x%lx bytes for initrd ...\n\r",
 		       initrd_size);
 		initrd_addr = (unsigned long)malloc(initrd_size);
-		if (! initrd_addr) {
-			printf("Can't allocate memory for initial "
+		if (! initrd_addr)
+			fatal("Can't allocate memory for initial "
 			       "ramdisk !\n\r");
-			exit();
-		}
 		printf("Relocating initrd 0x%p <- 0x%p (0x%lx bytes)\n\r",
 		       initrd_addr, old_addr, initrd_size);
 		memmove((void *)initrd_addr, old_addr, initrd_size);
@@ -205,10 +200,8 @@ static struct addr_range prep_initrd(str
 
 	/* Tell the kernel initrd address via device tree */
 	devp = finddevice("/chosen");
-	if (! devp) {
-		printf("Device tree has no chosen node!\n\r");
-		exit();
-	}
+	if (! devp)
+		fatal("Device tree has no chosen node!\n\r");
 
 	initrd_start = (u32)initrd_addr;
 	initrd_end = (u32)initrd_addr + initrd_size;
@@ -305,7 +298,6 @@ void start(void *sp)
 		kentry((unsigned long)initrd.addr, initrd.size,
 		       loader_info.promptr);
 
-	/* console closed so printf below may not work */
-	printf("Error: Linux kernel returned to zImage boot wrapper!\n\r");
-	exit();
+	/* console closed so printf in fatal below may not work */
+	fatal("Error: Linux kernel returned to zImage boot wrapper!\n\r");
 }
Index: kernel/arch/powerpc/boot/of.c
===================================================================
--- kernel.orig/arch/powerpc/boot/of.c	2007-03-19 08:32:50.000000000 -0500
+++ kernel/arch/powerpc/boot/of.c	2007-03-19 08:33:02.000000000 -0500
@@ -212,10 +212,9 @@ static void *of_vmlinux_alloc(unsigned l
 {
 	void *p = malloc(size);
 
-	if (!p) {
-		printf("Can't allocate memory for kernel image!\n\r");
-		exit();
-	}
+	if (!p)
+		fatal("Can't allocate memory for kernel image!\n\r");
+
 	return p;
 }
 
Index: kernel/arch/powerpc/boot/ops.h
===================================================================
--- kernel.orig/arch/powerpc/boot/ops.h	2007-03-19 08:32:50.000000000 -0500
+++ kernel/arch/powerpc/boot/ops.h	2007-03-19 08:33:02.000000000 -0500
@@ -157,6 +157,8 @@ static inline void exit(void)
 		platform_ops.exit();
 	for(;;);
 }
+#define fatal(args...) { printf(args); exit(); }
+
 
 #define BSS_STACK(size) \
 	static char _bss_stack[size]; \
Index: kernel/arch/powerpc/boot/gunzip_util.c
===================================================================
--- kernel.orig/arch/powerpc/boot/gunzip_util.c	2007-03-19 08:32:50.000000000 -0500
+++ kernel/arch/powerpc/boot/gunzip_util.c	2007-03-19 08:33:02.000000000 -0500
@@ -52,18 +52,14 @@ void gunzip_start(struct gunzip_state *s
 		int r, flags;
 
 		state->s.workspace = state->scratch;
-		if (zlib_inflate_workspacesize() > sizeof(state->scratch)) {
-			printf("insufficient scratch space for gunzip\n\r");
-			exit();
-		}
+		if (zlib_inflate_workspacesize() > sizeof(state->scratch))
+			fatal("insufficient scratch space for gunzip\n\r");
 
 		/* skip header */
 		hdrlen = 10;
 		flags = hdr[3];
-		if (hdr[2] != Z_DEFLATED || (flags & RESERVED) != 0) {
-			printf("bad gzipped data\n\r");
-			exit();
-		}
+		if (hdr[2] != Z_DEFLATED || (flags & RESERVED) != 0)
+			fatal("bad gzipped data\n\r");
 		if ((flags & EXTRA_FIELD) != 0)
 			hdrlen = 12 + hdr[10] + (hdr[11] << 8);
 		if ((flags & ORIG_NAME) != 0)
@@ -74,16 +70,12 @@ void gunzip_start(struct gunzip_state *s
 				;
 		if ((flags & HEAD_CRC) != 0)
 			hdrlen += 2;
-		if (hdrlen >= srclen) {
-			printf("gunzip_start: ran out of data in header\n\r");
-			exit();
-		}
+		if (hdrlen >= srclen)
+			fatal("gunzip_start: ran out of data in header\n\r");
 
 		r = zlib_inflateInit2(&state->s, -MAX_WBITS);
-		if (r != Z_OK) {
-			printf("inflateInit2 returned %d\n\r", r);
-			exit();
-		}
+		if (r != Z_OK)
+			fatal("inflateInit2 returned %d\n\r", r);
 	}
 
 	state->s.next_in = src + hdrlen;
@@ -117,10 +109,8 @@ int gunzip_partial(struct gunzip_state *
 		state->s.next_out = dst;
 		state->s.avail_out = dstlen;
 		r = zlib_inflate(&state->s, Z_FULL_FLUSH);
-		if (r != Z_OK && r != Z_STREAM_END) {
-			printf("inflate returned %d msg: %s\n\r", r, state->s.msg);
-			exit();
-		}
+		if (r != Z_OK && r != Z_STREAM_END)
+			fatal("inflate returned %d msg: %s\n\r", r, state->s.msg);
 		len = state->s.next_out - (unsigned char *)dst;
 	} else {
 		/* uncompressed image */
@@ -151,10 +141,8 @@ void gunzip_exactly(struct gunzip_state 
 	int len;
 
 	len  = gunzip_partial(state, dst, dstlen);
-	if (len < dstlen) {
-		printf("gunzip_block: ran out of data\n\r");
-		exit();
-	}
+	if (len < dstlen)
+		fatal("gunzip_block: ran out of data\n\r");
 }
 
 /**

[PATCH 2/7] boot: rebuild when wrapper changes

[Milton Miller]

Since there is magic defined per platform in the wrapper script, the
zImage targets should depend on it.

Signed-off-by: Milton Miller <miltonm@bga.com>
--- 
 Makefile |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: kernel/arch/powerpc/boot/Makefile
===================================================================
--- kernel.orig/arch/powerpc/boot/Makefile	2007-03-19 08:11:18.000000000 -0500
+++ kernel/arch/powerpc/boot/Makefile	2007-03-19 08:12:14.000000000 -0500
@@ -101,7 +101,8 @@ extra-y		:= $(obj)/wrapper.a $(obj-plat)
 		   $(obj)/zImage.lds $(obj)/zImage.coff.lds
 
 wrapper		:=$(srctree)/$(src)/wrapper
-wrapperbits	:= $(extra-y) $(addprefix $(obj)/,addnote hack-coff mktree)
+wrapperbits	:= $(extra-y) $(addprefix $(obj)/,addnote hack-coff mktree) \
+			$(wrapper)
 
 #############
 # Bits for building various flavours of zImage

[PATCH 6/7] bootwrapper: allow platforms to call library zImage_start

[Milton Miller]

Some platforms might need to run some code before the zImage start, but
could otherwise use the bss clear and relocation code.   Export the
start address strongly as zImage_start_lib.

--- 
Compiles and links with test user, stack set properly.
 crt0.S |    2 ++
 1 file changed, 2 insertions(+)

Index: kernel/arch/powerpc/boot/crt0.S
===================================================================
--- kernel.orig/arch/powerpc/boot/crt0.S	2007-03-19 04:15:03.000000000 -0500
+++ kernel/arch/powerpc/boot/crt0.S	2007-03-19 04:15:03.000000000 -0500
@@ -19,6 +19,8 @@ _zimage_start_opd:
 	.weak	_zimage_start
 	.globl	_zimage_start
 _zimage_start:
+	.globl	_zimage_start_lib
+_zimage_start_lib:
 	/* Work out the offset between the address we were linked at
 	   and the address where we're running. */
 	bl	1f

[PATCH 5/7] bootwrapper: missing relocation in crt0.S

[Milton Miller]

crt0.S had provisions to provide run address relocaton to got2 and
cache flush, but not on the bss clear or stack pointer load.  Apply
the same fixup for them.

--- 
Compiles and links.  Stack is created within the image when linked at
4M and run at 0.  Did not verify bss relocation.
 crt0.S |    9 +++++++++
 1 file changed, 9 insertions(+)

Index: kernel/arch/powerpc/boot/crt0.S
===================================================================
--- kernel.orig/arch/powerpc/boot/crt0.S	2007-03-16 23:22:00.000000000 -0500
+++ kernel/arch/powerpc/boot/crt0.S	2007-03-16 23:42:42.000000000 -0500
@@ -63,21 +63,30 @@ _zimage_start:
 	/* Clear the BSS */
 	lis	r9,__bss_start@ha
 	addi	r9,r9,__bss_start@l
+	add	r9,r0,r9
 	lis	r8,_end@ha
 	addi	r8,r8,_end@l
+	add	r8,r0,r8
 	li	r0,0
 5:	stw	r0,0(r9)
 	addi	r9,r9,4
 	cmplw	cr0,r9,r8
 	blt	5b
 
+	/* recreate relocation offset */
+	lis	r9,_end@ha
+	addi	r9,r9,_end@l
+	subf	r0,r9,r8
+
 	/* Possibly set up a custom stack */
 .weak	_platform_stack_top
 	lis	r8,_platform_stack_top@ha
 	addi	r8,r8,_platform_stack_top@l
 	cmpwi	r8,0
 	beq	6f
+	add	r8,r0,r8
 	lwz	r1,0(r8)
+	add	r1,r0,r1
 	li	r0,0
 	stwu	r0,-16(r1)	/* establish a stack frame */
 6:

[PATCH 7/7] boot: export flush_cache

[Milton Miller]

Move the declaration of flush_cache to ops.h for use by platform code.

Signed-off-by: Milton Miller <miltonm@bga.com>
--- 
Compiles and links with user, and that is the purpose.
 main.c |    2 --
 ops.h  |    2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

Index: kernel/arch/powerpc/boot/main.c
===================================================================
--- kernel.orig/arch/powerpc/boot/main.c	2007-03-19 04:16:06.000000000 -0500
+++ kernel/arch/powerpc/boot/main.c	2007-03-19 04:16:56.000000000 -0500
@@ -18,8 +18,6 @@
 #include "gunzip_util.h"
 #include "flatdevtree.h"
 
-extern void flush_cache(void *, unsigned long);
-
 extern char _start[];
 extern char __bss_start[];
 extern char _end[];
Index: kernel/arch/powerpc/boot/ops.h
===================================================================
--- kernel.orig/arch/powerpc/boot/ops.h	2007-03-19 04:16:06.000000000 -0500
+++ kernel/arch/powerpc/boot/ops.h	2007-03-19 04:30:38.000000000 -0500
@@ -79,7 +79,7 @@ int serial_console_init(void);
 int ns16550_console_init(void *devp, struct serial_console_data *scdp);
 void *simple_alloc_init(char *base, u32 heap_size, u32 granularity,
 		u32 max_allocs);
-
+extern void flush_cache(void *, unsigned long);
 
 static inline void *finddevice(const char *name)
 {

[PATCH 1/7] boot: use a common zImage rule

[Milton Miller]

Before the plethora of platforms gets any worse, establish a common
rule to invoke the wrapper for any platform.  Add arguments to
the rule for initrd, dts, dtb, etc.   Show example usage with initrd.

For now, uImage and zImage*ps3 are special.  Hopefully ps3 will join
the party.

Signed-off-by: Milton Miller <miltonm@bga.com>
--- 
One reviewer pointed out the explicit exception rule are now seperated
from the generic rule.  While the rule needs to go after the image-
assignment, that code could be moved up.
 Makefile |   46 ++++++++++++----------------------------------
 1 file changed, 12 insertions(+), 34 deletions(-)

Index: kernel/arch/powerpc/boot/Makefile
===================================================================
--- kernel.orig/arch/powerpc/boot/Makefile	2007-03-19 08:23:03.000000000 -0500
+++ kernel/arch/powerpc/boot/Makefile	2007-03-19 09:14:59.000000000 -0500
@@ -114,41 +114,10 @@ CROSSWRAP := -C "$(CROSS_COMPILE)"
 endif
 endif
 
+# args (to if_changed): 1 = (this rule), 2 = platform, 3 = dts 4=dtb 5=initrd
 quiet_cmd_wrap	= WRAP    $@
-      cmd_wrap	=$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) vmlinux
-quiet_cmd_wrap_initrd = WRAP    $@
-      cmd_wrap_initrd =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) \
-				-i $(obj)/ramdisk.image.gz vmlinux
-
-$(obj)/zImage.chrp: vmlinux $(wrapperbits)
-	$(call cmd,wrap,chrp)
-
-$(obj)/zImage.initrd.chrp: vmlinux $(wrapperbits)
-	$(call cmd,wrap_initrd,chrp)
-
-$(obj)/zImage.pseries:	vmlinux $(wrapperbits)
-	$(call cmd,wrap,pseries)
-
-$(obj)/zImage.initrd.pseries: vmlinux $(wrapperbits)
-	$(call cmd,wrap_initrd,pseries)
-
-$(obj)/zImage.pmac: vmlinux $(wrapperbits)
-	$(call cmd,wrap,pmac)
-
-$(obj)/zImage.initrd.pmac: vmlinux $(wrapperbits)
-	$(call cmd,wrap_initrd,pmac)
-
-$(obj)/zImage.coff: vmlinux $(wrapperbits)
-	$(call cmd,wrap,pmaccoff)
-
-$(obj)/zImage.initrd.coff: vmlinux $(wrapperbits)
-	$(call cmd,wrap_initrd,pmaccoff)
-
-$(obj)/zImage.miboot: vmlinux $(wrapperbits)
-	$(call cmd,wrap,miboot)
-
-$(obj)/zImage.initrd.miboot: vmlinux $(wrapperbits)
-	$(call cmd,wrap_initrd,miboot)
+      cmd_wrap	=$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) \
+      		$(if $3, -s $3)$(if $4, -d $4)$(if $5, -i $5) vmlinux
 
 $(obj)/zImage.ps3: vmlinux
 	$(STRIP) -s -R .comment $< -o $@
@@ -177,6 +146,15 @@ endif
 
 initrd-y := $(patsubst zImage%, zImage.initrd%, $(image-y))
 
+$(addprefix $(obj)/,$(filter-out uImage zImage.ps3,$(image-y))): \
+		$(obj)/zImage.%: vmlinux $(wrapperbits)
+	$(call cmd,wrap,$(patsubst $(obj)/zImage.%,%,$@))
+
+$(addprefix $(obj)/,$(filter-out uImage zImage.initrd.ps3,$(initrd-y))): \
+		$(obj)/zImage.initrd.%: vmlinux $(wrapperbits) \
+		$(obj)/ramdisk.image.gz
+	$(call cmd,wrap_initrd,$(patsubst $(obj)/zImage.initrd.%,%,$@),,,$(obj)/ramdisk.image.gz)
+
 $(obj)/zImage:		$(addprefix $(obj)/, $(image-y))
 	@rm -f $@; ln $< $@
 $(obj)/zImage.initrd:	$(addprefix $(obj)/, $(initrd-y))

Re: [PATCH 2/7] boot: rebuild when wrapper changes

[David Gibson]

On Mon, Mar 19, 2007 at 02:58:04PM -0600, Milton Miller wrote:
> 
> Since there is magic defined per platform in the wrapper script, the
> zImage targets should depend on it.
> 
> Signed-off-by: Milton Miller <miltonm@bga.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 4/7] bootwrapper: add a fatal error helper

[David Gibson]

On Mon, Mar 19, 2007 at 02:58:03PM -0600, Milton Miller wrote:
> Add a macro fatal that calls printf then exit.  User must include stdio.h.
> 
> Typically replaces 3 lines with 1, although I added back some whitespace.
> 
> Signed-off-by: Milton Miller <miltonm@bga.com>

One tiny nit...

[snip]
> Index: kernel/arch/powerpc/boot/ops.h
> ===================================================================
> --- kernel.orig/arch/powerpc/boot/ops.h	2007-03-19 08:32:50.000000000 -0500
> +++ kernel/arch/powerpc/boot/ops.h	2007-03-19 08:33:02.000000000 -0500
> @@ -157,6 +157,8 @@ static inline void exit(void)
>  		platform_ops.exit();
>  	for(;;);
>  }
> +#define fatal(args...) { printf(args); exit(); }
> +

This is the old gcc way of doing a varargs macro, not the new C99
way.  The new way would be:

#define fata(...)	{ printf(__VA_ARGS__); exit(); }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 5/7] bootwrapper: missing relocation in crt0.S

[David Gibson]

On Mon, Mar 19, 2007 at 02:58:05PM -0600, Milton Miller wrote:
> crt0.S had provisions to provide run address relocaton to got2 and
> cache flush, but not on the bss clear or stack pointer load.  Apply
> the same fixup for them.

Um.. missing Signed-off

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 1/7] boot: use a common zImage rule

[David Gibson]

On Mon, Mar 19, 2007 at 02:58:07PM -0600, Milton Miller wrote:
> Before the plethora of platforms gets any worse, establish a common
> rule to invoke the wrapper for any platform.  Add arguments to
> the rule for initrd, dts, dtb, etc.   Show example usage with initrd.
> 
> For now, uImage and zImage*ps3 are special.  Hopefully ps3 will join
> the party.
> 
> Signed-off-by: Milton Miller <miltonm@bga.com>
> --- 
> One reviewer pointed out the explicit exception rule are now seperated
> from the generic rule.  While the rule needs to go after the image-
> assignment, that code could be moved up.

The problem with this patch is that while the wrap command can take a
dts or dtb, there's no way to specify which should be used with each
target.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 6/7] bootwrapper: allow platforms to call library zImage_start

[David Gibson]

On Mon, Mar 19, 2007 at 02:58:05PM -0600, Milton Miller wrote:
> Some platforms might need to run some code before the zImage start, but
> could otherwise use the bss clear and relocation code.   Export the
> start address strongly as zImage_start_lib.

You're missing a signed off, but apart from that.

Acked-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 7/7] boot: export flush_cache

[David Gibson]

On Mon, Mar 19, 2007 at 02:58:06PM -0600, Milton Miller wrote:
> Move the declaration of flush_cache to ops.h for use by platform code.
> 
> Signed-off-by: Milton Miller <miltonm@bga.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 4/7] bootwrapper: add a fatal error helper

[Milton Miller]

On Mar 19, 2007, at 7:38 PM, David Gibson wrote:

> On Mon, Mar 19, 2007 at 02:58:03PM -0600, Milton Miller wrote:
>> Add a macro fatal that calls printf then exit.  User must include 
>> stdio.h.
>>
>> Typically replaces 3 lines with 1, although I added back some 
>> whitespace.
>>
>> Signed-off-by: Milton Miller <miltonm@bga.com>
>
> One tiny nit...
>
> [snip]
>> Index: kernel/arch/powerpc/boot/ops.h
>> ===================================================================
>> --- kernel.orig/arch/powerpc/boot/ops.h	2007-03-19 08:32:50.000000000 
>> -0500
>> +++ kernel/arch/powerpc/boot/ops.h	2007-03-19 08:33:02.000000000 -0500
>> @@ -157,6 +157,8 @@ static inline void exit(void)
>>  		platform_ops.exit();
>>  	for(;;);
>>  }
>> +#define fatal(args...) { printf(args); exit(); }
>> +
>
> This is the old gcc way of doing a varargs macro, not the new C99
> way.  The new way would be:
>
> #define fata(...)	{ printf(__VA_ARGS__); exit(); }
>

I was following the example of fprintf in stdio.h.   Is the new C99
way used elsewhere in the kernel?

milton

Re: [PATCH 1/7] boot: use a common zImage rule

[Milton Miller]

On Mar 19, 2007, at 10:30 PM, David Gibson wrote:

> On Mon, Mar 19, 2007 at 02:58:07PM -0600, Milton Miller wrote:
>> Before the plethora of platforms gets any worse, establish a common
>> rule to invoke the wrapper for any platform.  Add arguments to
>> the rule for initrd, dts, dtb, etc.   Show example usage with initrd.
>
> The problem with this patch is that while the wrap command can take a
> dts or dtb, there's no way to specify which should be used with each
> target.

I thought a bit about that before I posted, but didn't write anything.
We could add standard varable names dts-$(platform) dtb-$(platform),
since make would default such names as unset.

But this does raise the point, instead of listing the image names, 
should
we be seelcting the platforms and generating the image names with the
expansion.   That would save us stripping of zImage to get the platform
name in the rule.

I'll try to test this later.

Should I respin patch 3, add FORCE, without this so we can merge that
first?   I put this one first so there would be less adding rules that
would be removed later, but the other is really a bugfix and this is
a cleanup.

milton

Re: [PATCH 2/7] boot: rebuild when wrapper changes

[Segher Boessenkool]

> Since there is magic defined per platform in the wrapper script, the
> zImage targets should depend on it.

Thank you, I've been bitten by this before but didn't see
an easy fix.  It's so obvious though :-)

> Signed-off-by: Milton Miller <miltonm@bga.com>

Acked-by: Segher Boessenkool <segher@kernel.crashing.org>


Segher

Re: [PATCH 3/7] boot: use FORCE

[Segher Boessenkool]

>  quiet_cmd_bootar = BOOTAR  $@
> -      cmd_bootar = $(CROSS32AR) -cr $@.$$$$ $^; mv $@.$$$$ $@
> +      cmd_bootar = $(CROSS32AR) -cr $@.$$$$ $(filter-out FORCE,$^); 
> mv $@.$$$$ $@

Is there no way to do this without using filter-out?  It's
ugly and a bit fragile.  Oh and it shouldn't be ; but &&
(not your fault, but while you're at it...)


Segher

Re: [PATCH 1/7] boot: use a common zImage rule

[Mark A. Greer]

On Tue, Mar 20, 2007 at 08:47:20AM -0500, Milton Miller wrote:
> 
> On Mar 19, 2007, at 10:30 PM, David Gibson wrote:
> 
> > On Mon, Mar 19, 2007 at 02:58:07PM -0600, Milton Miller wrote:
> >> Before the plethora of platforms gets any worse, establish a common
> >> rule to invoke the wrapper for any platform.  Add arguments to
> >> the rule for initrd, dts, dtb, etc.   Show example usage with initrd.
> >
> > The problem with this patch is that while the wrap command can take a
> > dts or dtb, there's no way to specify which should be used with each
> > target.
> 
> I thought a bit about that before I posted, but didn't write anything.
> We could add standard varable names dts-$(platform) dtb-$(platform),
> since make would default such names as unset.
> 
> But this does raise the point, instead of listing the image names, 
> should
> we be seelcting the platforms and generating the image names with the
> expansion.   That would save us stripping of zImage to get the platform
> name in the rule.
> 
> I'll try to test this later.
> 
> Should I respin patch 3, add FORCE, without this so we can merge that
> first?   I put this one first so there would be less adding rules that
> would be removed later, but the other is really a bugfix and this is
> a cleanup.

FYI, I made the patch below so I could build a zImage with a dts.  I also
added the rule:

	image-$(CONFIG_PPC_PRPMC2800)		+= zImage.dts.prpmc2800

to build a zImage.dts.prpmc2800 that contained a dts.

Mark
---

Index: powerpc/arch/powerpc/boot/Makefile
===================================================================
--- powerpc.orig/arch/powerpc/boot/Makefile
+++ powerpc/arch/powerpc/boot/Makefile
@@ -153,6 +153,10 @@ $(addprefix $(obj)/,$(filter-out uImage 
                $(obj)/zImage.%: vmlinux $(wrapperbits)
        $(call if_changed,wrap,$(patsubst $(obj)/zImage.%,%,$@))
 
+$(addprefix $(obj)/,$(filter-out uImage zImage.ps3,$(image-y))): \
+               $(obj)/zImage.dts.%: vmlinux $(wrapperbits)
+       $(call if_changed,wrap,$(patsubst $(obj)/zImage.dts.%,%,$@),$(src)/dts/$(CONFIG_DEVICE_TREE))
+
 $(addprefix $(obj)/,$(filter-out uImage
zImage.initrd.ps3,$(initrd-y))): \
                $(obj)/zImage.initrd.%: vmlinux $(wrapperbits) \
                $(obj)/ramdisk.image.gz

Re: [PATCH 5/7] bootwrapper: missing relocation in crt0.S

[Mark A. Greer]

On Mon, Mar 19, 2007 at 02:58:05PM -0600, Milton Miller wrote:

> Index: kernel/arch/powerpc/boot/crt0.S
> ===================================================================
> --- kernel.orig/arch/powerpc/boot/crt0.S	2007-03-16 23:22:00.000000000 -0500
> +++ kernel/arch/powerpc/boot/crt0.S	2007-03-16 23:42:42.000000000 -0500
> @@ -63,21 +63,30 @@ _zimage_start:
>  	/* Clear the BSS */
>  	lis	r9,__bss_start@ha
>  	addi	r9,r9,__bss_start@l
> +	add	r9,r0,r9
>  	lis	r8,_end@ha
>  	addi	r8,r8,_end@l
> +	add	r8,r0,r8
>  	li	r0,0
                ^^
		Use some reg other than r0?

>  5:	stw	r0,0(r9)
>  	addi	r9,r9,4
>  	cmplw	cr0,r9,r8
>  	blt	5b
>  
> +	/* recreate relocation offset */
> +	lis	r9,_end@ha
> +	addi	r9,r9,_end@l
> +	subf	r0,r9,r8
> +

Wouldn't it make sense to just use another reg to zero out the bss so
you don't have to recreate the relocation offset?

Mark

Re: [PATCH 4/7] bootwrapper: add a fatal error helper

[David Gibson]

On Tue, Mar 20, 2007 at 08:38:45AM -0500, Milton Miller wrote:
> 
> On Mar 19, 2007, at 7:38 PM, David Gibson wrote:
> 
> > On Mon, Mar 19, 2007 at 02:58:03PM -0600, Milton Miller wrote:
> >> Add a macro fatal that calls printf then exit.  User must include 
> >> stdio.h.
> >>
> >> Typically replaces 3 lines with 1, although I added back some 
> >> whitespace.
> >>
> >> Signed-off-by: Milton Miller <miltonm@bga.com>
> >
> > One tiny nit...
> >
> > [snip]
> >> Index: kernel/arch/powerpc/boot/ops.h
> >> ===================================================================
> >> --- kernel.orig/arch/powerpc/boot/ops.h	2007-03-19 08:32:50.000000000 
> >> -0500
> >> +++ kernel/arch/powerpc/boot/ops.h	2007-03-19 08:33:02.000000000 -0500
> >> @@ -157,6 +157,8 @@ static inline void exit(void)
> >>  		platform_ops.exit();
> >>  	for(;;);
> >>  }
> >> +#define fatal(args...) { printf(args); exit(); }
> >> +
> >
> > This is the old gcc way of doing a varargs macro, not the new C99
> > way.  The new way would be:
> >
> > #define fata(...)	{ printf(__VA_ARGS__); exit(); }
> >
> 
> I was following the example of fprintf in stdio.h.   Is the new C99
> way used elsewhere in the kernel?

Yes, in a variety of places.  Just grep for __VA_ARGS__.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 1/7] boot: use a common zImage rule

[David Gibson]

On Tue, Mar 20, 2007 at 08:47:20AM -0500, Milton Miller wrote:
> 
> On Mar 19, 2007, at 10:30 PM, David Gibson wrote:
> 
> > On Mon, Mar 19, 2007 at 02:58:07PM -0600, Milton Miller wrote:
> >> Before the plethora of platforms gets any worse, establish a common
> >> rule to invoke the wrapper for any platform.  Add arguments to
> >> the rule for initrd, dts, dtb, etc.   Show example usage with initrd.
> >
> > The problem with this patch is that while the wrap command can take a
> > dts or dtb, there's no way to specify which should be used with each
> > target.
> 
> I thought a bit about that before I posted, but didn't write anything.
> We could add standard varable names dts-$(platform) dtb-$(platform),
> since make would default such names as unset.

Well, that works until we have a platform that can operate with
several different dtbs.  I believe Scott's cuboot stuff will have this
property.

> But this does raise the point, instead of listing the image names, 
> should
> we be seelcting the platforms and generating the image names with the
> expansion.   That would save us stripping of zImage to get the platform
> name in the rule.

That sounds like a good idea.  $(platform-y) instead of $(image-y).

> I'll try to test this later.
> 
> Should I respin patch 3, add FORCE, without this so we can merge that
> first?   I put this one first so there would be less adding rules that
> would be removed later, but the other is really a bugfix and this is
> a cleanup.

Yes, I think that's a good idea.  Apart from the textual conflicts 2-7
look sensible.  I like the idea of patch 1, but it heads into some
complications that need more thought.

So, some more thought...

I don't like the idea of a per-platform dts variable (as above),
because there will be platforms which can support more than one device
tree.  I also don't like the idea of a single variable giving the dts,
since that takes us back to one-board-per-config, even when the actual
zImage code is common across multiple boards.

There are actually three parameters (apart from the vmlinux itself)
which determine the final bootable binary:
	1) what code to include in the zImage
	2) the included device tree (if any)
	3) exectable packaging (ELF, uImage, etc.)

These are certainly not orthogonal; most combinations of the 3 won't
make sense.  But, the parameters are independent in the sense that for
any 2 of them, there are situations in which there are multiple values
that could make sense for the other one.  For example:
	- if (1) = Ebony wrapper and (2) = Ebony device tree, the
binary can be pacakged either for cuboot or for treeboot
	- if (1) = Open Firmware wrapper and (2) = no device tree,
then (3) can be ELF or COFF, depending on the format supported by the
OF version in question.
	- if (1) = wrapper for cuboot on mpc83xx and (3) = uImage,
then there are several device trees which could be used, since there
are several mpc83xx boards
	- it's not hard to image a board that has two firmware
versions with incompatible properties such that if (2) = SomeBoard's
device tree and (3) = whatever then (1) could sensibly be either be
"SomeBoard old firmware wrapper" or "SomeBoard new firmware wrapper".

Then there's the possibility of a similar firmware running on several
different boards, and having a zImage which incorporates several
device trees, picking the right one based on a firmware-supplied board
ID of some sort.

Right now the Makefile targets can select both "platform", which picks
both (1) and (3) and, seperately, the device tree, (2).

So, thinking about it in this framework.  It's the job of the wrapper
script to take (in whatever form) the values of (1), (2) and (3) and
produce an image.  It's the job of the Kconfig and Makefile between
them to generate, based on the kernel config, a list of (1)-(2)-(3)
triples and invoke the wrapper for all of them.

Quite how best to achieve this is not yet obvious to me.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[PATCH 0/8] boot: cleanup and prep for more platforms

[Milton Miller]

After comments from David, I am resending.  Per his request, I have
moved 4-7 to 0-3, and moved the common rule invocation last.  I
also added the missing sign offs.

I substantially reworked the zImage-rule one, including it be just
a default rule that any platform target can override.  Hopefully
this will make it mergable now, even as we defer how to select
the dtb and dts.  I also inserted a patch to fix the clean target
in the boot directory.

milton

boot-add-fatal
boot-crt0-missing-reloc
boot-crt0-export
boot-export-flush-cache
boot-add-wrapper-to-wrapperbits
boot-add-FORCE
boot-fix-clean
boot-zImage-rule

[PATCH 6/8] boot: use FORCE

[Milton Miller]

Kbuild if_changed and if_changed_dep require the use of the dummy
FORCE to get the dependencies right.   Also add to targets to get
correct behavior.

Signed-off-by: Milton Miller <miltonm@bga.com>
--- 
Previous Testing: Several repeat runs with V=2 and going up and down a series.
Without this, there were several missed rebuilds, including missing archive
rebulid when adding and dropping a patch adding platform support.

This version: 
- Rebased to before common zImage rule.

- zImage or zImage.initrd not included.  They did not have cmd_ defined.
This means that if you change your config or the Makefile to change the
first listed image, you won't get the new zImage link.

Index: kernel/arch/powerpc/boot/Makefile
===================================================================
--- kernel.orig/arch/powerpc/boot/Makefile	2007-03-21 03:14:04.000000000 -0500
+++ kernel/arch/powerpc/boot/Makefile	2007-03-21 03:14:17.000000000 -0500
@@ -85,24 +85,25 @@ quiet_cmd_bootas = BOOTAS  $@
       cmd_bootas = $(CROSS32CC) -Wp,-MD,$(depfile) $(BOOTAFLAGS) -c -o $@ $<
 
 quiet_cmd_bootar = BOOTAR  $@
-      cmd_bootar = $(CROSS32AR) -cr $@.$$$$ $^; mv $@.$$$$ $@
+      cmd_bootar = $(CROSS32AR) -cr $@.$$$$ $(filter-out FORCE,$^); mv $@.$$$$ $@
 
-$(patsubst %.c,%.o, $(filter %.c, $(src-boot))): %.o: %.c
+$(patsubst %.c,%.o, $(filter %.c, $(src-boot))): %.o: %.c FORCE
 	$(call if_changed_dep,bootcc)
-$(patsubst %.S,%.o, $(filter %.S, $(src-boot))): %.o: %.S
+$(patsubst %.S,%.o, $(filter %.S, $(src-boot))): %.o: %.S FORCE
 	$(call if_changed_dep,bootas)
 
-$(obj)/wrapper.a: $(obj-wlib)
-	$(call cmd,bootar)
+$(obj)/wrapper.a: $(obj-wlib) FORCE
+	$(call if_changed,bootar)
 
 hostprogs-y	:= addnote addRamDisk hack-coff mktree
 
+targets		+= $(patsubst $(obj)/%,%,$(obj-boot) wrapper.a)
 extra-y		:= $(obj)/wrapper.a $(obj-plat) $(obj)/empty.o \
 		   $(obj)/zImage.lds $(obj)/zImage.coff.lds
 
 wrapper		:=$(srctree)/$(src)/wrapper
 wrapperbits	:= $(extra-y) $(addprefix $(obj)/,addnote hack-coff mktree) \
-			$(wrapper)
+			$(wrapper) FORCE
 
 #############
 # Bits for building various flavours of zImage
@@ -122,34 +123,34 @@ quiet_cmd_wrap_initrd = WRAP    $@
 				-i $(obj)/ramdisk.image.gz vmlinux
 
 $(obj)/zImage.chrp: vmlinux $(wrapperbits)
-	$(call cmd,wrap,chrp)
+	$(call if_changed,wrap,chrp)
 
 $(obj)/zImage.initrd.chrp: vmlinux $(wrapperbits)
-	$(call cmd,wrap_initrd,chrp)
+	$(call if_changed,wrap_initrd,chrp)
 
 $(obj)/zImage.pseries:	vmlinux $(wrapperbits)
-	$(call cmd,wrap,pseries)
+	$(call if_changed,wrap,pseries)
 
 $(obj)/zImage.initrd.pseries: vmlinux $(wrapperbits)
-	$(call cmd,wrap_initrd,pseries)
+	$(call if_changed,wrap_initrd,pseries)
 
 $(obj)/zImage.pmac: vmlinux $(wrapperbits)
-	$(call cmd,wrap,pmac)
+	$(call if_changed,wrap,pmac)
 
 $(obj)/zImage.initrd.pmac: vmlinux $(wrapperbits)
-	$(call cmd,wrap_initrd,pmac)
+	$(call if_changed,wrap_initrd,pmac)
 
 $(obj)/zImage.coff: vmlinux $(wrapperbits)
-	$(call cmd,wrap,pmaccoff)
+	$(call if_changed,wrap,pmaccoff)
 
 $(obj)/zImage.initrd.coff: vmlinux $(wrapperbits)
-	$(call cmd,wrap_initrd,pmaccoff)
+	$(call if_changed,wrap_initrd,pmaccoff)
 
 $(obj)/zImage.miboot: vmlinux $(wrapperbits)
-	$(call cmd,wrap,miboot)
+	$(call if_changed,wrap,miboot)
 
 $(obj)/zImage.initrd.miboot: vmlinux $(wrapperbits)
-	$(call cmd,wrap_initrd,miboot)
+	$(call if_changed,wrap_initrd,miboot)
 
 $(obj)/zImage.ps3: vmlinux
 	$(STRIP) -s -R .comment $< -o $@
@@ -158,7 +159,7 @@ $(obj)/zImage.initrd.ps3: vmlinux
 	@echo "  WARNING zImage.initrd.ps3 not supported (yet)"
 
 $(obj)/uImage: vmlinux $(wrapperbits)
-	$(call cmd,wrap,uboot)
+	$(call if_changed,wrap,uboot)
 
 image-$(CONFIG_PPC_PSERIES)		+= zImage.pseries
 image-$(CONFIG_PPC_MAPLE)		+= zImage.pseries
@@ -177,6 +178,7 @@ image-$(CONFIG_PPC_PMAC)	+= zImage.coff 
 endif
 
 initrd-y := $(patsubst zImage%, zImage.initrd%, $(image-y))
+targets	+= $(image-y) $(initrd-y)
 
 $(obj)/zImage:		$(addprefix $(obj)/, $(image-y))
 	@rm -f $@; ln $< $@

[PATCH 1/8] bootwrapper: add a fatal error helper

[Milton Miller]

Add a macro fatal that calls printf then exit.  User must include stdio.h.

Typically replaces 3 lines with 1, although I added back some whitespace.

Signed-off-by: Milton Miller <miltonm@bga.com>

Index: kernel/arch/powerpc/boot/main.c
===================================================================
--- kernel.orig/arch/powerpc/boot/main.c	2007-03-20 22:08:00.000000000 -0500
+++ kernel/arch/powerpc/boot/main.c	2007-03-21 02:13:49.000000000 -0500
@@ -120,10 +120,9 @@ static struct addr_range prep_kernel(voi
 	gunzip_start(&gzstate, vmlinuz_addr, vmlinuz_size);
 	gunzip_exactly(&gzstate, elfheader, sizeof(elfheader));
 
-	if (!parse_elf64(elfheader, &ei) && !parse_elf32(elfheader, &ei)) {
-		printf("Error: not a valid PPC32 or PPC64 ELF file!\n\r");
-		exit();
-	}
+	if (!parse_elf64(elfheader, &ei) && !parse_elf32(elfheader, &ei))
+		fatal("Error: not a valid PPC32 or PPC64 ELF file!\n\r");
+
 	if (platform_ops.image_hdr)
 		platform_ops.image_hdr(elfheader);
 
@@ -137,11 +136,9 @@ static struct addr_range prep_kernel(voi
 	if (platform_ops.vmlinux_alloc) {
 		addr = platform_ops.vmlinux_alloc(ei.memsize);
 	} else {
-		if ((unsigned long)_start < ei.memsize) {
-			printf("Insufficient memory for kernel at address 0!"
+		if ((unsigned long)_start < ei.memsize)
+			fatal("Insufficient memory for kernel at address 0!"
 			       " (_start=%lx)\n\r", _start);
-			exit();
-		}
 	}
 
 	/* Finally, gunzip the kernel */
@@ -191,11 +188,9 @@ static struct addr_range prep_initrd(str
 		printf("Allocating 0x%lx bytes for initrd ...\n\r",
 		       initrd_size);
 		initrd_addr = (unsigned long)malloc(initrd_size);
-		if (! initrd_addr) {
-			printf("Can't allocate memory for initial "
+		if (! initrd_addr)
+			fatal("Can't allocate memory for initial "
 			       "ramdisk !\n\r");
-			exit();
-		}
 		printf("Relocating initrd 0x%p <- 0x%p (0x%lx bytes)\n\r",
 		       initrd_addr, old_addr, initrd_size);
 		memmove((void *)initrd_addr, old_addr, initrd_size);
@@ -205,10 +200,8 @@ static struct addr_range prep_initrd(str
 
 	/* Tell the kernel initrd address via device tree */
 	devp = finddevice("/chosen");
-	if (! devp) {
-		printf("Device tree has no chosen node!\n\r");
-		exit();
-	}
+	if (! devp)
+		fatal("Device tree has no chosen node!\n\r");
 
 	initrd_start = (u32)initrd_addr;
 	initrd_end = (u32)initrd_addr + initrd_size;
@@ -305,7 +298,6 @@ void start(void *sp)
 		kentry((unsigned long)initrd.addr, initrd.size,
 		       loader_info.promptr);
 
-	/* console closed so printf below may not work */
-	printf("Error: Linux kernel returned to zImage boot wrapper!\n\r");
-	exit();
+	/* console closed so printf in fatal below may not work */
+	fatal("Error: Linux kernel returned to zImage boot wrapper!\n\r");
 }
Index: kernel/arch/powerpc/boot/of.c
===================================================================
--- kernel.orig/arch/powerpc/boot/of.c	2007-03-20 22:08:00.000000000 -0500
+++ kernel/arch/powerpc/boot/of.c	2007-03-21 02:13:49.000000000 -0500
@@ -212,10 +212,9 @@ static void *of_vmlinux_alloc(unsigned l
 {
 	void *p = malloc(size);
 
-	if (!p) {
-		printf("Can't allocate memory for kernel image!\n\r");
-		exit();
-	}
+	if (!p)
+		fatal("Can't allocate memory for kernel image!\n\r");
+
 	return p;
 }
 
Index: kernel/arch/powerpc/boot/ops.h
===================================================================
--- kernel.orig/arch/powerpc/boot/ops.h	2007-03-21 02:12:52.000000000 -0500
+++ kernel/arch/powerpc/boot/ops.h	2007-03-21 02:13:49.000000000 -0500
@@ -106,6 +106,8 @@ static inline void exit(void)
 		platform_ops.exit();
 	for(;;);
 }
+#define fatal(args...) { printf(args); exit(); }
+
 
 #define BSS_STACK(size) \
 	static char _bss_stack[size]; \
Index: kernel/arch/powerpc/boot/gunzip_util.c
===================================================================
--- kernel.orig/arch/powerpc/boot/gunzip_util.c	2007-03-20 22:08:00.000000000 -0500
+++ kernel/arch/powerpc/boot/gunzip_util.c	2007-03-21 02:13:49.000000000 -0500
@@ -52,18 +52,14 @@ void gunzip_start(struct gunzip_state *s
 		int r, flags;
 
 		state->s.workspace = state->scratch;
-		if (zlib_inflate_workspacesize() > sizeof(state->scratch)) {
-			printf("insufficient scratch space for gunzip\n\r");
-			exit();
-		}
+		if (zlib_inflate_workspacesize() > sizeof(state->scratch))
+			fatal("insufficient scratch space for gunzip\n\r");
 
 		/* skip header */
 		hdrlen = 10;
 		flags = hdr[3];
-		if (hdr[2] != Z_DEFLATED || (flags & RESERVED) != 0) {
-			printf("bad gzipped data\n\r");
-			exit();
-		}
+		if (hdr[2] != Z_DEFLATED || (flags & RESERVED) != 0)
+			fatal("bad gzipped data\n\r");
 		if ((flags & EXTRA_FIELD) != 0)
 			hdrlen = 12 + hdr[10] + (hdr[11] << 8);
 		if ((flags & ORIG_NAME) != 0)
@@ -74,16 +70,12 @@ void gunzip_start(struct gunzip_state *s
 				;
 		if ((flags & HEAD_CRC) != 0)
 			hdrlen += 2;
-		if (hdrlen >= srclen) {
-			printf("gunzip_start: ran out of data in header\n\r");
-			exit();
-		}
+		if (hdrlen >= srclen)
+			fatal("gunzip_start: ran out of data in header\n\r");
 
 		r = zlib_inflateInit2(&state->s, -MAX_WBITS);
-		if (r != Z_OK) {
-			printf("inflateInit2 returned %d\n\r", r);
-			exit();
-		}
+		if (r != Z_OK)
+			fatal("inflateInit2 returned %d\n\r", r);
 	}
 
 	state->s.next_in = src + hdrlen;
@@ -117,10 +109,8 @@ int gunzip_partial(struct gunzip_state *
 		state->s.next_out = dst;
 		state->s.avail_out = dstlen;
 		r = zlib_inflate(&state->s, Z_FULL_FLUSH);
-		if (r != Z_OK && r != Z_STREAM_END) {
-			printf("inflate returned %d msg: %s\n\r", r, state->s.msg);
-			exit();
-		}
+		if (r != Z_OK && r != Z_STREAM_END)
+			fatal("inflate returned %d msg: %s\n\r", r, state->s.msg);
 		len = state->s.next_out - (unsigned char *)dst;
 	} else {
 		/* uncompressed image */
@@ -151,10 +141,8 @@ void gunzip_exactly(struct gunzip_state 
 	int len;
 
 	len  = gunzip_partial(state, dst, dstlen);
-	if (len < dstlen) {
-		printf("gunzip_block: ran out of data\n\r");
-		exit();
-	}
+	if (len < dstlen)
+		fatal("gunzip_block: ran out of data\n\r");
 }
 
 /**

[PATCH 5/8] boot: rebuild when wrapper changes

[Milton Miller]

Since there is magic defined per platform in the wrapper script, the
zImage targets should depend on it.

Signed-off-by: Milton Miller <miltonm@bga.com>

Index: kernel/arch/powerpc/boot/Makefile
===================================================================
--- kernel.orig/arch/powerpc/boot/Makefile	2007-03-21 01:40:16.000000000 -0500
+++ kernel/arch/powerpc/boot/Makefile	2007-03-21 02:15:16.000000000 -0500
@@ -101,7 +101,8 @@ extra-y		:= $(obj)/wrapper.a $(obj-plat)
 		   $(obj)/zImage.lds $(obj)/zImage.coff.lds
 
 wrapper		:=$(srctree)/$(src)/wrapper
-wrapperbits	:= $(extra-y) $(addprefix $(obj)/,addnote hack-coff mktree)
+wrapperbits	:= $(extra-y) $(addprefix $(obj)/,addnote hack-coff mktree) \
+			$(wrapper)
 
 #############
 # Bits for building various flavours of zImage

[PATCH 3/8] bootwrapper: allow platforms to call library zImage_start

[Milton Miller]

Some platforms might need to run some code before the zImage start, but
could otherwise use the bss clear and relocation code.   Export the
start address strongly as zImage_start_lib.

Signed-off-by: Milton Miller <miltonm@bga.com>
---
Compiles and links with usage, stack set properly.

Index: kernel/arch/powerpc/boot/crt0.S
===================================================================
--- kernel.orig/arch/powerpc/boot/crt0.S	2007-03-19 04:15:03.000000000 -0500
+++ kernel/arch/powerpc/boot/crt0.S	2007-03-19 04:15:03.000000000 -0500
@@ -19,6 +19,8 @@ _zimage_start_opd:
 	.weak	_zimage_start
 	.globl	_zimage_start
 _zimage_start:
+	.globl	_zimage_start_lib
+_zimage_start_lib:
 	/* Work out the offset between the address we were linked at
 	   and the address where we're running. */
 	bl	1f

[PATCH 2/8] bootwrapper: missing relocation in crt0.S

[Milton Miller]

crt0.S had provisions to provide run address relocaton to got2 and
cache flush, but not on the bss clear or stack pointer load.  Apply
the same fixup for them.

Signed-off-by: Milton Miller <miltonm@bga.com>
--- 
Compiles and links.  Stack is created within the image when linked at
4M and run at 0.  Did not verify bss relocation.

Index: kernel/arch/powerpc/boot/crt0.S
===================================================================
--- kernel.orig/arch/powerpc/boot/crt0.S	2007-03-16 23:22:00.000000000 -0500
+++ kernel/arch/powerpc/boot/crt0.S	2007-03-16 23:42:42.000000000 -0500
@@ -63,21 +63,30 @@ _zimage_start:
 	/* Clear the BSS */
 	lis	r9,__bss_start@ha
 	addi	r9,r9,__bss_start@l
+	add	r9,r0,r9
 	lis	r8,_end@ha
 	addi	r8,r8,_end@l
+	add	r8,r0,r8
 	li	r0,0
 5:	stw	r0,0(r9)
 	addi	r9,r9,4
 	cmplw	cr0,r9,r8
 	blt	5b
 
+	/* recreate relocation offset */
+	lis	r9,_end@ha
+	addi	r9,r9,_end@l
+	subf	r0,r9,r8
+
 	/* Possibly set up a custom stack */
 .weak	_platform_stack_top
 	lis	r8,_platform_stack_top@ha
 	addi	r8,r8,_platform_stack_top@l
 	cmpwi	r8,0
 	beq	6f
+	add	r8,r0,r8
 	lwz	r1,0(r8)
+	add	r1,r0,r1
 	li	r0,0
 	stwu	r0,-16(r1)	/* establish a stack frame */
 6:

[PATCH 4/8] boot: export flush_cache

[Milton Miller]

Move the declaration of flush_cache to ops.h for use by platform code.

Signed-off-by: Milton Miller <miltonm@bga.com>
--
Compiles and links with user, and that is the purpose.

Index: kernel/arch/powerpc/boot/main.c
===================================================================
--- kernel.orig/arch/powerpc/boot/main.c	2007-03-21 02:13:49.000000000 -0500
+++ kernel/arch/powerpc/boot/main.c	2007-03-21 02:15:06.000000000 -0500
@@ -18,8 +18,6 @@
 #include "gunzip_util.h"
 #include "flatdevtree.h"
 
-extern void flush_cache(void *, unsigned long);
-
 extern char _start[];
 extern char __bss_start[];
 extern char _end[];
Index: kernel/arch/powerpc/boot/ops.h
===================================================================
--- kernel.orig/arch/powerpc/boot/ops.h	2007-03-21 02:13:49.000000000 -0500
+++ kernel/arch/powerpc/boot/ops.h	2007-03-21 02:15:06.000000000 -0500
@@ -72,7 +72,7 @@ int serial_console_init(void);
 int ns16550_console_init(void *devp, struct serial_console_data *scdp);
 void *simple_alloc_init(char *base, u32 heap_size, u32 granularity,
 		u32 max_allocs);
-
+extern void flush_cache(void *, unsigned long);
 
 static inline void *finddevice(const char *name)
 {

[PATCH 7/8] boot: clean rule fixes

[Milton Miller]

Now that obj-boot is in targets, we can remove (twice) it from clean-files.
zImage.initrd was missing, move zImage nearer where its used, and place
zImage.initrd next to it.  Remove non-ported zImage.sandpoint, and add 
auto-generation of unconfigured zImage.initrd* like image-.

Signed-off-by: Milton Miller <miltonm@bga.com>
--
Testing: did a few builds and cleans

Index: kernel/arch/powerpc/boot/Makefile
===================================================================
--- kernel.orig/arch/powerpc/boot/Makefile	2007-03-21 03:14:53.000000000 -0500
+++ kernel/arch/powerpc/boot/Makefile	2007-03-21 03:27:37.000000000 -0500
@@ -76,7 +76,7 @@ $(obj)/zImage.lds $(obj)/zImage.coff.lds
 	@cp $< $@
 
 clean-files := $(zlib) $(zlibheader) $(zliblinuxheader) \
-		empty.c zImage zImage.coff.lds zImage.lds zImage.sandpoint
+		empty.c zImage.coff.lds zImage.lds
 
 quiet_cmd_bootcc = BOOTCC  $@
       cmd_bootcc = $(CROSS32CC) -Wp,-MD,$(depfile) $(BOOTCFLAGS) -c -o $@ $<
@@ -177,6 +177,7 @@ ifeq ($(CONFIG_PPC32),y)
 image-$(CONFIG_PPC_PMAC)	+= zImage.coff zImage.miboot
 endif
 
+initrd-  := $(patsubst zImage%, zImage.initrd%, $(image-n) $(image-))
 initrd-y := $(patsubst zImage%, zImage.initrd%, $(image-y))
 targets	+= $(image-y) $(initrd-y)
 
@@ -188,6 +189,5 @@ $(obj)/zImage.initrd:	$(addprefix $(obj)
 install: $(CONFIGURE) $(image-y)
 	sh -x $(srctree)/$(src)/install.sh "$(KERNELRELEASE)" vmlinux System.map "$(INSTALL_PATH)" $<
 
-clean-files += $(addprefix $(objtree)/, $(obj-boot) vmlinux.strip.gz)
-clean-files += $(addprefix $(objtree)/, $(obj-boot) vmlinux.bin.gz)
-clean-files += $(image-)
+clean-files += $(addprefix $(objtree)/, vmlinux.strip.gz vmlinux.bin.gz)
+clean-files += $(image-) $(initrd-) zImage zImage.initrd

[PATCH 8/8] boot: use a common zImage rule

[Milton Miller]

Before the plethora of platforms gets any worse, establish a common
rule to invoke the wrapper for any platform.  Add arguments to
the rule for initrd, dts, dtb, etc.   Show example usage with initrd.

Create default rules for zImage, and zImage.initrd.  initrd targets
depend on the ramdisk file.

Don't consider targets for zImage.initrd that are targets for zImage.
This means uImage is no longer considered a target for zImage.initrd.

Signed-off-by: Milton Miller <miltonm@bga.com>
--- 
Rebased after boot-add-FORCE.   Removed the explicit call of the rule,
and fixed the zImage.initrd vs zImage rule selection by making both
rules have the same requirements.

Index: kernel/arch/powerpc/boot/Makefile
===================================================================
--- kernel.orig/arch/powerpc/boot/Makefile	2007-03-21 03:27:37.000000000 -0500
+++ kernel/arch/powerpc/boot/Makefile	2007-03-21 03:37:32.000000000 -0500
@@ -116,50 +116,10 @@ CROSSWRAP := -C "$(CROSS_COMPILE)"
 endif
 endif
 
+# args (to if_changed): 1 = (this rule), 2 = platform, 3 = dts 4=dtb 5=initrd
 quiet_cmd_wrap	= WRAP    $@
-      cmd_wrap	=$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) vmlinux
-quiet_cmd_wrap_initrd = WRAP    $@
-      cmd_wrap_initrd =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) \
-				-i $(obj)/ramdisk.image.gz vmlinux
-
-$(obj)/zImage.chrp: vmlinux $(wrapperbits)
-	$(call if_changed,wrap,chrp)
-
-$(obj)/zImage.initrd.chrp: vmlinux $(wrapperbits)
-	$(call if_changed,wrap_initrd,chrp)
-
-$(obj)/zImage.pseries:	vmlinux $(wrapperbits)
-	$(call if_changed,wrap,pseries)
-
-$(obj)/zImage.initrd.pseries: vmlinux $(wrapperbits)
-	$(call if_changed,wrap_initrd,pseries)
-
-$(obj)/zImage.pmac: vmlinux $(wrapperbits)
-	$(call if_changed,wrap,pmac)
-
-$(obj)/zImage.initrd.pmac: vmlinux $(wrapperbits)
-	$(call if_changed,wrap_initrd,pmac)
-
-$(obj)/zImage.coff: vmlinux $(wrapperbits)
-	$(call if_changed,wrap,pmaccoff)
-
-$(obj)/zImage.initrd.coff: vmlinux $(wrapperbits)
-	$(call if_changed,wrap_initrd,pmaccoff)
-
-$(obj)/zImage.miboot: vmlinux $(wrapperbits)
-	$(call if_changed,wrap,miboot)
-
-$(obj)/zImage.initrd.miboot: vmlinux $(wrapperbits)
-	$(call if_changed,wrap_initrd,miboot)
-
-$(obj)/zImage.ps3: vmlinux
-	$(STRIP) -s -R .comment $< -o $@
-
-$(obj)/zImage.initrd.ps3: vmlinux
-	@echo "  WARNING zImage.initrd.ps3 not supported (yet)"
-
-$(obj)/uImage: vmlinux $(wrapperbits)
-	$(call if_changed,wrap,uboot)
+      cmd_wrap	=$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) \
+      		$(if $3, -s $3)$(if $4, -d $4)$(if $5, -i $5) vmlinux
 
 image-$(CONFIG_PPC_PSERIES)		+= zImage.pseries
 image-$(CONFIG_PPC_MAPLE)		+= zImage.pseries
@@ -179,8 +139,29 @@ endif
 
 initrd-  := $(patsubst zImage%, zImage.initrd%, $(image-n) $(image-))
 initrd-y := $(patsubst zImage%, zImage.initrd%, $(image-y))
+initrd-y := $(filter-out $(image-y), $(initrd-y))
 targets	+= $(image-y) $(initrd-y)
 
+$(addprefix $(obj)/, $(initrd-y)): $(obj)/ramdisk.image.gz
+
+# Don't put the ramdisk on the pattern rule; when its missing make will try
+# the pattern rule with less dependencies that also matches (even with the
+# hard dependency listed).
+$(obj)/zImage.initrd.%: vmlinux $(wrapperbits)
+	$(call if_changed,wrap,$*,,,$(obj)/ramdisk.image.gz)
+
+$(obj)/zImage.%: vmlinux $(wrapperbits)
+	$(call if_changed,wrap,$*)
+
+$(obj)/zImage.ps3: vmlinux
+	$(STRIP) -s -R .comment $< -o $@
+
+$(obj)/zImage.initrd.ps3: vmlinux
+	@echo "  WARNING zImage.initrd.ps3 not supported (yet)"
+
+$(obj)/uImage: vmlinux $(wrapperbits)
+	$(call if_changed,wrap,uboot)
+
 $(obj)/zImage:		$(addprefix $(obj)/, $(image-y))
 	@rm -f $@; ln $< $@
 $(obj)/zImage.initrd:	$(addprefix $(obj)/, $(initrd-y))

Re: [PATCH 8/8] boot: use a common zImage rule

[David Gibson]

On Wed, Mar 21, 2007 at 09:03:23AM -0600, Milton Miller wrote:
> Before the plethora of platforms gets any worse, establish a common
> rule to invoke the wrapper for any platform.  Add arguments to
> the rule for initrd, dts, dtb, etc.   Show example usage with initrd.
> 
> Create default rules for zImage, and zImage.initrd.  initrd targets
> depend on the ramdisk file.
> 
> Don't consider targets for zImage.initrd that are targets for zImage.
> This means uImage is no longer considered a target for
> zImage.initrd.

This seems happy enough on Ebony now (with Ebony overriding the
default targets).

Just one nit..

> Index: kernel/arch/powerpc/boot/Makefile
> ===================================================================
> --- kernel.orig/arch/powerpc/boot/Makefile	2007-03-21 03:27:37.000000000 -0500
> +++ kernel/arch/powerpc/boot/Makefile	2007-03-21 03:37:32.000000000 -0500
> @@ -116,50 +116,10 @@ CROSSWRAP := -C "$(CROSS_COMPILE)"
>  endif
>  endif
>  
> +# args (to if_changed): 1 = (this rule), 2 = platform, 3 = dts 4=dtb 5=initrd
>  quiet_cmd_wrap	= WRAP    $@
> -      cmd_wrap	=$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) vmlinux
> -quiet_cmd_wrap_initrd = WRAP    $@
> -      cmd_wrap_initrd =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) \
> -				-i $(obj)/ramdisk.image.gz vmlinux

The mixture of tabs and spaces in the line above causes emacs to
whinge about a suspicious line every time you try to save.  Needless
to say, that's kind of irritating...

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 2/8] bootwrapper: missing relocation in crt0.S

[Paul Mackerras]

Milton Miller writes:

> crt0.S had provisions to provide run address relocaton to got2 and
> cache flush, but not on the bss clear or stack pointer load.  Apply
> the same fixup for them.
> 
> Signed-off-by: Milton Miller <miltonm@bga.com>
> --- 
> Compiles and links.  Stack is created within the image when linked at
> 4M and run at 0.  Did not verify bss relocation.
> 
> Index: kernel/arch/powerpc/boot/crt0.S
> ===================================================================
> --- kernel.orig/arch/powerpc/boot/crt0.S	2007-03-16 23:22:00.000000000 -0500
> +++ kernel/arch/powerpc/boot/crt0.S	2007-03-16 23:42:42.000000000 -0500
> @@ -63,21 +63,30 @@ _zimage_start:
>  	/* Clear the BSS */
>  	lis	r9,__bss_start@ha
>  	addi	r9,r9,__bss_start@l
> +	add	r9,r0,r9
>  	lis	r8,_end@ha
>  	addi	r8,r8,_end@l
> +	add	r8,r0,r8
>  	li	r0,0
>  5:	stw	r0,0(r9)

How about changing the last two instructions here to use r10 instead
of r0 so we don't need to recreate the relocation offset?

Paul.

[PATCH 1/4] bootwrapper: missing relocation in crt0.S

[Milton Miller]

crt0.S had provisions to provide run address relocaton to got2 and
cache flush, but not on the bss clear or stack pointer load.  Apply
the same fixup for them.

Signed-off-by: Milton Miller <miltonm@bga.com>
--- 
This one uses r10 to zero, so that the offset in r0 is preserved.

Index: kernel/arch/powerpc/boot/crt0.S
===================================================================
--- kernel.orig/arch/powerpc/boot/crt0.S	2007-03-28 01:09:05.000000000 -0500
+++ kernel/arch/powerpc/boot/crt0.S	2007-03-28 01:09:21.000000000 -0500
@@ -65,10 +65,12 @@ _zimage_start_lib:
 	/* Clear the BSS */
 	lis	r9,__bss_start@ha
 	addi	r9,r9,__bss_start@l
+	add	r9,r0,r9
 	lis	r8,_end@ha
 	addi	r8,r8,_end@l
-	li	r0,0
-5:	stw	r0,0(r9)
+	add	r8,r0,r8
+	li	r10,0
+5:	stw	r10,0(r9)
 	addi	r9,r9,4
 	cmplw	cr0,r9,r8
 	blt	5b
@@ -79,7 +81,9 @@ _zimage_start_lib:
 	addi	r8,r8,_platform_stack_top@l
 	cmpwi	r8,0
 	beq	6f
+	add	r8,r0,r8
 	lwz	r1,0(r8)
+	add	r1,r0,r1
 	li	r0,0
 	stwu	r0,-16(r1)	/* establish a stack frame */
 6:

[PATCH 0/4] boot: cleanup and prep for more platforms

[Milton Miller]

Here is the lastest round of my patches to the bootwrapper.

It includes the updated missing relocations patchs, a few small
changes, and a RFC.

I now have my ppc64 kexec reboot code running, but the 1000 lines
need to be split apart for posting.

milton

[RFC] bootwrapper: allow vmlinuz to be an external payload

[Milton Miller]

From: Milton Miller <miltonm@bga.com>

Allow the bootwrapper to obtain an external platform supplied image.

My work in progress will find a specified file in a initramfs cpio
and advance the gunzip_util stream to the contents.  

Another use would be to uncompress directly from a memory mapped
region such as a flash partition.

Signed-off-by: Milton Miller <miltonm@bga.com>
--- 
Status: compiles and links, doesn't cause failures.

Passing the source and source len to the routine is just so we can
print it nice and pretty.

While we could take (archive-start, len, skip), we would end up 
doing the decompress processing for skip twice.

Index: kernel/arch/powerpc/boot/main.c
===================================================================
--- kernel.orig/arch/powerpc/boot/main.c	2007-03-28 02:32:24.000000000 -0500
+++ kernel/arch/powerpc/boot/main.c	2007-03-28 02:33:26.000000000 -0500
@@ -115,10 +115,15 @@ static struct addr_range prep_kernel(voi
 	struct elf_info ei;
 	int len;
 
-	/* gunzip the ELF header of the kernel */
-	gunzip_start(&gzstate, vmlinuz_addr, vmlinuz_size);
-	gunzip_exactly(&gzstate, elfheader, sizeof(elfheader));
+	/* Initialze zlib.  Any attached kernel overrides find_vmlinux */
+	if (vmlinuz_size)
+		gunzip_start(&gzstate, vmlinuz_addr, vmlinuz_size);
+	else
+		platform_ops.find_vmlinuz(&gzstate, &vmlinuz_addr,
+				&vmlinuz_size);
 
+	/* gunzip and parse the ELF header of the kernel */
+	gunzip_exactly(&gzstate, elfheader, sizeof(elfheader));
 	if (!parse_elf64(elfheader, &ei) && !parse_elf32(elfheader, &ei))
 		fatal("Error: not a valid PPC32 or PPC64 ELF file!\n\r");
 
Index: kernel/arch/powerpc/boot/ops.h
===================================================================
--- kernel.orig/arch/powerpc/boot/ops.h	2007-03-28 02:32:24.000000000 -0500
+++ kernel/arch/powerpc/boot/ops.h	2007-03-28 02:33:26.000000000 -0500
@@ -19,6 +19,8 @@
 #define	MAX_PATH_LEN		256
 #define	MAX_PROP_LEN		256 /* What should this be? */
 
+struct gunzip_state;
+
 /* Platform specific operations */
 struct platform_ops {
 	void	(*fixups)(void);
@@ -28,6 +30,8 @@ struct platform_ops {
 	void *	(*realloc)(void *ptr, unsigned long size);
 	void	(*exit)(void);
 	void *	(*vmlinux_alloc)(unsigned long size);
+ 	void	(*find_vmlinuz)(struct gunzip_state *, void **srcp,
+ 			unsigned long *lenp);
 };
 extern struct platform_ops platform_ops;
 

[PATCH 2/4] bootwrapper: remove unused variable

[Milton Miller]

gunzip_util.c has a static variable state, obsured in every function
by the first passed parameter.  Looks like this got missed in an API
change during development.

Signed-off-by: Milton Miller <miltonm@bga.com>

Index: kernel/arch/powerpc/boot/gunzip_util.c
===================================================================
--- kernel.orig/arch/powerpc/boot/gunzip_util.c	2007-03-27 16:02:08.000000000 -0500
+++ kernel/arch/powerpc/boot/gunzip_util.c	2007-03-27 16:02:11.000000000 -0500
@@ -14,8 +14,6 @@
 #include "ops.h"
 #include "gunzip_util.h"
 
-struct gunzip_state state;
-
 #define HEAD_CRC	2
 #define EXTRA_FIELD	4
 #define ORIG_NAME	8

Re: Patch: [PATCH 3/4] bootwrapper: no gzip fixes

[Milton Miller]

commit a9903811bf8d130a26004f9cb27b66513a267908 missed two uses of the
the .gz suffix in the wrapper script and didn't clean the additonal
possibly cached files.

Signed-off-by: Milton Miller <miltonm@bga.com>

Index: kernel/arch/powerpc/boot/wrapper
===================================================================
--- kernel.orig/arch/powerpc/boot/wrapper	2007-03-28 01:09:04.000000000 -0500
+++ kernel/arch/powerpc/boot/wrapper	2007-03-28 01:10:26.000000000 -0500
@@ -144,7 +144,7 @@ miboot|uboot)
 esac
 
 vmz="$tmpdir/`basename \"$kernel\"`.$ext"
-if [ -z "$cacheit" -o ! -f "$vmz.gz" -o "$vmz.gz" -ot "$kernel" ]; then
+if [ -z "$cacheit" -o ! -f "$vmz$gzip" -o "$vmz$gzip" -ot "$kernel" ]; then
     ${CROSS}objcopy $objflags "$kernel" "$vmz.$$"
 
     if [ -n "$gzip" ]; then
Index: kernel/arch/powerpc/boot/Makefile
===================================================================
--- kernel.orig/arch/powerpc/boot/Makefile	2007-03-28 01:09:05.000000000 -0500
+++ kernel/arch/powerpc/boot/Makefile	2007-03-28 01:23:20.000000000 -0500
@@ -170,5 +170,11 @@ $(obj)/zImage.initrd:	$(addprefix $(obj)
 install: $(CONFIGURE) $(image-y)
 	sh -x $(srctree)/$(src)/install.sh "$(KERNELRELEASE)" vmlinux System.map "$(INSTALL_PATH)" $<
 
-clean-files += $(addprefix $(objtree)/, vmlinux.strip.gz vmlinux.bin.gz)
+# anything not in $(targets)
 clean-files += $(image-) $(initrd-) zImage zImage.initrd
+
+# clean up files cached by wrapper
+clean-kernel := vmlinux.strip vmlinux.bin
+clean-kernel += $(addsuffix .gz,$(clean-kernel))
+# If not absolute clean-files are relative to $(obj).
+clean-files += $(addprefix $(objtree)/, $(clean-kernel))

[PATCH 4/4] bootwrapper: decompress less, check more

[Milton Miller]

Our kernels put everything in the first load segment, and we read that.
Instead of decompresing to the end of the gzip stream or supplied image
and hoping we get it all, decompress the expected size and complain if
it is not available.

Signed-off-by: Milton Miller <miltonm@bga.com>

Index: kernel/arch/powerpc/boot/main.c
===================================================================
--- kernel.orig/arch/powerpc/boot/main.c	2007-03-28 01:09:04.000000000 -0500
+++ kernel/arch/powerpc/boot/main.c	2007-03-28 01:42:48.000000000 -0500
@@ -141,11 +141,14 @@ static struct addr_range prep_kernel(voi
 	}
 
 	/* Finally, gunzip the kernel */
-	printf("gunzipping (0x%p <- 0x%p:0x%p)...", addr,
-	       vmlinuz_addr, vmlinuz_addr+vmlinuz_size);
+	printf("gunzipping (0x%p:0x%p <- 0x%p:0x%p)...", addr,
+	       addr + ei.loadsize, vmlinuz_addr, vmlinuz_addr+vmlinuz_size);
 	/* discard up to the actual load data */
 	gunzip_discard(&gzstate, ei.elfoffset - sizeof(elfheader));
-	len = gunzip_finish(&gzstate, addr, ei.memsize);
+	len = gunzip_finish(&gzstate, addr, ei.loadsize);
+	if (len != ei.loadsize)
+		fatal("ran out of data!  only got 0x%x of 0x%lx bytes.\n\r",
+				len, ei.loadsize);
 	printf("done 0x%x bytes\n\r", len);
 
 	flush_cache(addr, ei.loadsize);

Re: [PATCH 2/4] bootwrapper: remove unused variable

[David Gibson]

On Wed, Mar 28, 2007 at 02:21:04AM -0600, Milton Miller wrote:
> gunzip_util.c has a static variable state, obsured in every function
> by the first passed parameter.  Looks like this got missed in an API
> change during development.

I think you need to do a new pull.  This one got fixed in the same
patch that added extra doco about the gunzip functions.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 2/4] bootwrapper: remove unused variable

[Milton Miller]

On Mar 28, 2007, at 3:34 AM, David Gibson wrote:
> On Wed, Mar 28, 2007 at 02:21:04AM -0600, Milton Miller wrote:
>> gunzip_util.c has a static variable state, obsured in every function
>> by the first passed parameter.  Looks like this got missed in an API
>> change during development.
>
> I think you need to do a new pull.  This one got fixed in the same
> patch that added extra doco about the gunzip functions.

Sorry.  My pull was fine, my selection of relevant patches was
lacking.

I was extracting patches that hit files in boot but not boot/dts
to skip patches that were not boot wrapper related.   However, I
was using the wrong head to list what files are in the boot
directory.  Since it only touched new files, it didn't show up.
After changing to the for-2.6.22 head for the list of files, that
seems to be the only patch I missed.  I'm actually working of
2.6.20 plus the boot patches and the duplicate-reserve and
initrd-reserve patches.

milton

Re: Patch: [PATCH 3/4] bootwrapper: no gzip fixes

[Scott Wood]

Milton Miller wrote:
> commit a9903811bf8d130a26004f9cb27b66513a267908 missed two uses of the
> the .gz suffix in the wrapper script and didn't clean the additonal
> possibly cached files.
> 
> Signed-off-by: Milton Miller <miltonm@bga.com>

Acked-by: Scott Wood <scottwood@freescale.com>

-Scott

Re: [PATCH 4/4] bootwrapper: decompress less, check more

[Milton Miller]

Our kernels put everything in the first load segment, and we read that.
Instead of decompresing to the end of the gzip stream or supplied image
and hoping we get it all, decompress the expected size and complain if
it is not available.

Signed-off-by: Milton Miller <miltonm@bga.com>
--- 
Changed the gunzipping message back to the original to stay within 80
characters per printed line.

Index: kernel/arch/powerpc/boot/main.c
===================================================================
--- kernel.orig/arch/powerpc/boot/main.c	2007-03-29 06:12:16.000000000 -0500
+++ kernel/arch/powerpc/boot/main.c	2007-03-29 08:24:14.000000000 -0500
@@ -145,7 +145,10 @@ static struct addr_range prep_kernel(voi
 	       vmlinuz_addr, vmlinuz_addr+vmlinuz_size);
 	/* discard up to the actual load data */
 	gunzip_discard(&gzstate, ei.elfoffset - sizeof(elfheader));
-	len = gunzip_finish(&gzstate, addr, ei.memsize);
+	len = gunzip_finish(&gzstate, addr, ei.loadsize);
+	if (len != ei.loadsize)
+		fatal("ran out of data!  only got 0x%x of 0x%lx bytes.\n\r",
+				len, ei.loadsize);
 	printf("done 0x%x bytes\n\r", len);
 
 	flush_cache(addr, ei.loadsize);