Skip to contentSkip to conversation

boot: cleanup and prep for more platforms

PATCH In review Milton Miller 6 participants 43 messages Mar 19, 2007 – Mar 29, 2007 PowerPC

lore
Share & tools
lore
#0 lore raw

cover boot: cleanup and prep for more platforms

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

#1 lore raw

PATCH 1/7 boot: use a common zImage rule

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.

---
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(-)
arch/powerpc/boot/Makefile +12 -34
Index: kernel/arch/powerpc/boot/Makefile
===================================================================
@@ -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))
#2 lore raw

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

Quoted 4 lines · 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.

Quoted 8 lines

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

---
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.

Signature · 4 lines
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
#3 lore raw

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

Quoted 8 lines · David Gibson
Quoted 3 lines · 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.

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

#4 lore raw

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

Quoted 22 lines · Milton Miller
Quoted 8 lines · David Gibson
Quoted 3 lines · 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.

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.

Quoted 6 lines

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
---

arch/powerpc/boot/Makefile +4 -0
Index: 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
#5 lore raw

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

In reply to Milton Miller
Quoted 8 lines · Milton Miller
Quoted 5 lines · David Gibson
Quoted 3 lines · 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.

Quoted 7 lines
Quoted 3 lines

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.

Quoted 5 lines

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).

Quoted 6 lines

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.

Signature · 4 lines
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
#6 lore raw

PATCH 2/7 boot: rebuild when wrapper changes

In reply to Milton Miller

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

---

 Makefile |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/powerpc/boot/Makefile +2 -1
Index: kernel/arch/powerpc/boot/Makefile
===================================================================
@@ -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
Acked-by on v1 2/7
#7 lore raw

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

Quoted 5 lines · Milton Miller

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

Signature · 4 lines
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
#9 lore raw

PATCH 3/7 boot: use FORCE

In reply to 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.

---
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(-)
arch/powerpc/boot/Makefile +13 -11
Index: kernel/arch/powerpc/boot/Makefile
===================================================================
@@ -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)
#10 lore raw

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

Quoted 4 lines

quiet_cmd_bootar = BOOTAR $@

(quoted diff) +1 -1
-      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

#11 lore raw

PATCH 4/7 bootwrapper: add a fatal error helper

In reply to 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.

---

 gunzip_util.c |   36 ++++++++++++------------------------
 main.c        |   30 +++++++++++-------------------
 of.c          |    7 +++----
 ops.h         |    2 ++
 4 files changed, 28 insertions(+), 47 deletions(-)
arch/powerpc/boot/main.c +11 -19
Index: kernel/arch/powerpc/boot/main.c
===================================================================
@@ -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");
 }
arch/powerpc/boot/of.c +3 -4
Index: kernel/arch/powerpc/boot/of.c
===================================================================
@@ -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;
 }
 
arch/powerpc/boot/ops.h +2 -0
Index: kernel/arch/powerpc/boot/ops.h
===================================================================
@@ -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]; \
arch/powerpc/boot/gunzip_util.c +12 -24
Index: kernel/arch/powerpc/boot/gunzip_util.c
===================================================================
@@ -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");
 }
 
 /**
#12 lore raw

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

Quoted 5 lines · 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.

One tiny nit...

[snip]

arch/powerpc/boot/ops.h static inline void exit(void) · line 161

Quoted 10 lines

Index: kernel/arch/powerpc/boot/ops.h
===================================================================

arch/powerpc/boot/ops.h +2 -0
@@ -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(); }

Signature · 4 lines
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
#13 lore raw

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

Quoted 25 lines · David Gibson
Quoted 5 lines · 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.

One tiny nit...

[snip]

arch/powerpc/boot/ops.h static inline void exit(void) · line 161

Quoted 11 lines

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

arch/powerpc/boot/ops.h +2 -0
@@ -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:

Quoted 2 lines

#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

#14 lore raw

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

Quoted 30 lines · Milton Miller
Quoted 27 lines · David Gibson
Quoted 7 lines · 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.

One tiny nit...

[snip]

arch/powerpc/boot/ops.h static inline void exit(void) · line 161

Quoted 11 lines

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

arch/powerpc/boot/ops.h +2 -0
@@ -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:

Quoted 5 lines
Quoted 2 lines

#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__.

Signature · 4 lines
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
#15 lore raw

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

In reply to 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(+)
arch/powerpc/boot/crt0.S +9 -0
Index: kernel/arch/powerpc/boot/crt0.S
===================================================================
@@ -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:
#16 lore raw

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

Quoted 3 lines · 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.

Um.. missing Signed-off

Signature · 4 lines
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
#17 lore raw

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

In reply to Milton Miller

arch/powerpc/boot/crt0.S _zimage_start: · line 70

Quoted 13 lines · Milton Miller

Index: kernel/arch/powerpc/boot/crt0.S
===================================================================

arch/powerpc/boot/crt0.S +2 -0
@@ -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?

arch/powerpc/boot/crt0.S _zimage_start: · line 80

Quoted 10 lines
arch/powerpc/boot/crt0.S +5 -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
+

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

#18 lore raw

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

In reply to 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(+)
arch/powerpc/boot/crt0.S +2 -0
Index: kernel/arch/powerpc/boot/crt0.S
===================================================================
@@ -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
Acked-by on v1 6/7
#19 lore raw

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

Quoted 3 lines · 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.

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

Signature · 4 lines
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
#20 lore raw

PATCH 7/7 boot: export flush_cache

In reply to Milton Miller

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

---
Compiles and links with user, and that is the purpose.

 main.c |    2 --
 ops.h  |    2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)
arch/powerpc/boot/main.c +0 -2
Index: kernel/arch/powerpc/boot/main.c
===================================================================
@@ -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[];
arch/powerpc/boot/ops.h +1 -1
Index: kernel/arch/powerpc/boot/ops.h
===================================================================
@@ -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)
 {
Acked-by on v1 7/7
#21 lore raw

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

Quoted 3 lines · Milton Miller

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

Signature · 4 lines
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
#22 lore raw

cover boot: cleanup and prep for more platforms

In reply to 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

#23 lore raw

PATCH 1/8 bootwrapper: add a fatal error helper

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.

arch/powerpc/boot/main.c +11 -19
Index: kernel/arch/powerpc/boot/main.c
===================================================================
@@ -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");
 }
arch/powerpc/boot/of.c +3 -4
Index: kernel/arch/powerpc/boot/of.c
===================================================================
@@ -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;
 }
 
arch/powerpc/boot/ops.h +2 -0
Index: kernel/arch/powerpc/boot/ops.h
===================================================================
@@ -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]; \
arch/powerpc/boot/gunzip_util.c +12 -24
Index: kernel/arch/powerpc/boot/gunzip_util.c
===================================================================
@@ -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");
 }
 
 /**
#24 lore raw

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

In reply to 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.

arch/powerpc/boot/crt0.S +9 -0
Index: kernel/arch/powerpc/boot/crt0.S
===================================================================
@@ -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:
#25 lore raw

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

Quoted 14 lines · 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.

Index: kernel/arch/powerpc/boot/crt0.S
===================================================================

arch/powerpc/boot/crt0.S _zimage_start: · line 71

Quoted 9 lines
arch/powerpc/boot/crt0.S +2 -0
@@ -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.

#26 lore raw

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

In reply to 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 usage, stack set properly.

arch/powerpc/boot/crt0.S +2 -0
Index: kernel/arch/powerpc/boot/crt0.S
===================================================================
@@ -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
#27 lore raw

PATCH 4/8 boot: export flush_cache

In reply to Milton Miller

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

--
Compiles and links with user, and that is the purpose.

arch/powerpc/boot/main.c +0 -2
Index: kernel/arch/powerpc/boot/main.c
===================================================================
@@ -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[];
arch/powerpc/boot/ops.h +1 -1
Index: kernel/arch/powerpc/boot/ops.h
===================================================================
@@ -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)
 {
#28 lore raw

PATCH 5/8 boot: rebuild when wrapper changes

In reply to Milton Miller

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

arch/powerpc/boot/Makefile +2 -1
Index: kernel/arch/powerpc/boot/Makefile
===================================================================
@@ -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
#29 lore raw

PATCH 6/8 boot: use FORCE

In reply to 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.

---
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.

arch/powerpc/boot/Makefile +19 -17
Index: kernel/arch/powerpc/boot/Makefile
===================================================================
@@ -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 $< $@
#30 lore raw

PATCH 7/8 boot: clean rule fixes

In reply to 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-.

--
Testing: did a few builds and cleans

arch/powerpc/boot/Makefile +4 -4
Index: kernel/arch/powerpc/boot/Makefile
===================================================================
@@ -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
#31 lore raw

PATCH 8/8 boot: use a common zImage rule

In reply to 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.

---
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.

arch/powerpc/boot/Makefile +24 -43
Index: kernel/arch/powerpc/boot/Makefile
===================================================================
@@ -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))
#32 lore raw

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

Quoted 10 lines · 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.

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

Just one nit..

Quoted 5 lines

Index: kernel/arch/powerpc/boot/Makefile
===================================================================

arch/powerpc/boot/Makefile CROSSWRAP := -C "$(CROSS_COMPILE)" · line 123

Quoted 9 lines
arch/powerpc/boot/Makefile +1 -4
@@ -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...

Signature · 4 lines
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
#33 lore raw

cover boot: cleanup and prep for more platforms

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

#34 lore raw

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

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.

---
This one uses r10 to zero, so that the offset in r0 is preserved.

arch/powerpc/boot/crt0.S +6 -2
Index: kernel/arch/powerpc/boot/crt0.S
===================================================================
@@ -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:
#35 lore raw

RFC 1/1 bootwrapper: allow vmlinuz to be an external payload

In reply to Milton Miller

From: Milton Miller <[email protected]>

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.

---
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.

arch/powerpc/boot/main.c +8 -3
Index: kernel/arch/powerpc/boot/main.c
===================================================================
@@ -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");
 
arch/powerpc/boot/ops.h +4 -0
Index: kernel/arch/powerpc/boot/ops.h
===================================================================
@@ -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;
 
#36 lore raw

PATCH 2/4 bootwrapper: remove unused variable

In reply to 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.

arch/powerpc/boot/gunzip_util.c +0 -2
Index: kernel/arch/powerpc/boot/gunzip_util.c
===================================================================
@@ -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
#37 lore raw

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

Quoted 3 lines · 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.

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.

Signature · 4 lines
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
#38 lore raw

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

Quoted 7 lines · David Gibson
Quoted 3 lines · 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.

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

#39 lore raw

PATCH 4/4 bootwrapper: decompress less, check more

In reply to 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.

arch/powerpc/boot/main.c +6 -3
Index: kernel/arch/powerpc/boot/main.c
===================================================================
@@ -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);
#40 lore raw

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

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.

---
Changed the gunzipping message back to the original to stay within 80 characters per printed line.

arch/powerpc/boot/main.c +4 -1
Index: kernel/arch/powerpc/boot/main.c
===================================================================
@@ -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);
#41 lore raw

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

In reply to 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.

arch/powerpc/boot/wrapper +1 -1
Index: kernel/arch/powerpc/boot/wrapper
===================================================================
@@ -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
arch/powerpc/boot/Makefile +7 -1
Index: kernel/arch/powerpc/boot/Makefile
===================================================================
@@ -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))