Skip to contentSkip to conversation

Further zImage work

PATCH In review David Gibson 4 participants 18 messages Mar 21, 2007 – Mar 22, 2007 PowerPC

lore
Share & tools
lore
#0 lore raw

cover Further zImage work

The following patch series is an attempt to consolidate some of the more settled ideas in the many zImage patches flying around at the moment.

This patch series needs to be applied on top of patches 4-7 from Milton Miller's recent series, and patch number 2 from Scott Wood's most recent cuboot series (patch 1 from that series having already been applied to powerpc.git).

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
#1 lore raw

PATCH 1/4 powerpc: Add gcc format warnings to zImage printf()

This patch adds the correct attributes to the zImage's versions of printf to make gcc generate format string mismatch warnings. It also corrects several minor problems with format strings in the zImage thus discovered.

---

 arch/powerpc/boot/main.c  |    6 +++---
 arch/powerpc/boot/stdio.h |    4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)
arch/powerpc/boot/stdio.h +2 -2
Index: working-2.6/arch/powerpc/boot/stdio.h
===================================================================
@@ -7,11 +7,11 @@
 #define	EINVAL		22	/* Invalid argument */
 #define ENOSPC		28	/* No space left on device */
 
-extern int printf(const char *fmt, ...);
+extern int printf(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
 
 #define fprintf(fmt, args...)	printf(args)
 
-extern int sprintf(char *buf, const char *fmt, ...);
+extern int sprintf(char *buf, const char *fmt, ...) __attribute__((format(printf, 2, 3)));
 
 extern int vsprintf(char *buf, const char *fmt, va_list args);
 
arch/powerpc/boot/main.c +3 -3
Index: working-2.6/arch/powerpc/boot/main.c
===================================================================
@@ -136,7 +136,7 @@ static struct addr_range prep_kernel(voi
 	} else {
 		if ((unsigned long)_start < ei.memsize)
 			fatal("Insufficient memory for kernel at address 0!"
-			       " (_start=%lx)\n\r", _start);
+			       " (_start=%p)\n\r", _start);
 	}
 
 	/* Finally, gunzip the kernel */
@@ -145,7 +145,7 @@ static struct addr_range prep_kernel(voi
 	/* discard up to the actual load data */
 	gunzip_discard(&gzstate, ei.elfoffset - sizeof(elfheader));
 	len = gunzip_finish(&gzstate, addr, ei.memsize);
-	printf("done 0x%lx bytes\n\r", len);
+	printf("done 0x%x bytes\n\r", len);
 
 	flush_cache(addr, ei.loadsize);
 
@@ -189,7 +189,7 @@ static struct addr_range prep_initrd(str
 		if (! initrd_addr)
 			fatal("Can't allocate memory for initial "
 			       "ramdisk !\n\r");
-		printf("Relocating initrd 0x%p <- 0x%p (0x%lx bytes)\n\r",
+		printf("Relocating initrd 0x%lx <- 0x%p (0x%lx bytes)\n\r",
 		       initrd_addr, old_addr, initrd_size);
 		memmove((void *)initrd_addr, old_addr, initrd_size);
 	}
#2 lore raw

Re: [PATCH 1/4] powerpc: Add gcc format warnings to zImage printf()

+extern int sprintf(char *buf, const char *fmt, ...) __attribute__((format(printf, 2, 3)));

you exceeded 80 columns by 10.

milton

#3 lore raw

Re: [PATCH 1/4] powerpc: Add gcc format warnings to zImage printf()

Quoted 4 lines · Milton Miller

+extern int sprintf(char *buf, const char *fmt, ...) __attribute__((format(printf, 2, 3)));

you exceeded 80 columns by 10.

Revised in second spin.

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
#4 lore raw

PATCH 2/4 powerpc: Add device tree utility functions to zImage

In reply to David Gibson

This patch adds a library of useful device tree manipulation functions to the zImage library, for use by platform code. These functions are based on the hooks already in dt_ops, so they're not dependent on a particular device tree implementation. This patch also slightly streamlines the code in main.c using these new functions.

This is a consolidation of my work in this area with Scott Wood's patches to a very similar end.

---

 arch/powerpc/boot/Makefile  |    2 
 arch/powerpc/boot/devtree.c |  126 ++++++++++++++++++++++++++++++++++++++++++++
 arch/powerpc/boot/main.c    |   21 ++-----
 arch/powerpc/boot/ops.h     |   27 ++++++++-
 4 files changed, 160 insertions(+), 16 deletions(-)
arch/powerpc/boot/ops.h +25 -2
Index: working-2.6/arch/powerpc/boot/ops.h
===================================================================
@@ -91,11 +91,16 @@ static inline int getprop(void *devp, co
 	return (dt_ops.getprop) ? dt_ops.getprop(devp, name, buf, buflen) : -1;
 }
 
-static inline int setprop(void *devp, const char *name, void *buf, int buflen)
+static inline int setprop(void *devp, const char *name,
+			  const void *buf, int buflen)
 {
 	return (dt_ops.setprop) ? dt_ops.setprop(devp, name, buf, buflen) : -1;
 }
-
+#define setprop_val(devp, name, val) \
+	do { \
+		typeof(val) x = (val); \
+		setprop((devp), (name), &x, sizeof(x)); \
+	} while(0)
 static inline int setprop_str(void *devp, const char *name, const char *buf)
 {
 	if (dt_ops.setprop)
@@ -140,6 +145,24 @@ static inline void *find_node_by_devtype
 	return find_node_by_prop_value_str(prev, "device_type", type);
 }
 
+int dt_path_getprop(const char *path, const char *name, void *buf, int bufsize);
+void dt_path_setprop(const char *path, const char *name, void *val, int size);
+#define dt_path_setprop_val(path, name, val)	\
+	do { \
+		typeof(val) x = (val); \
+		dt_path_setprop((path),(name),&x,sizeof(x)); \
+	} while (0)
+#define dt_path_setprop_str(path, name, str) \
+	dt_path_setprop((path), (name), (str), strlen(str)+1)
+
+void dt_fixup_memory(u64 start, u64 size);
+void dt_fixup_cpu_clocks(u32 cpufreq, u32 tbfreq, u32 busfreq);
+void dt_fixup_clock(const char *path, u32 freq);
+void __dt_fixup_mac_addresses(u32 startindex, ...);
+#define dt_fixup_mac_addresses(...) \
+	__dt_fixup_mac_addresses(0, __VA_ARGS__, NULL)
+
+
 static inline void *malloc(u32 size)
 {
 	return (platform_ops.malloc) ? platform_ops.malloc(size) : NULL;
arch/powerpc/boot/Makefile +1 -1
Index: working-2.6/arch/powerpc/boot/Makefile
===================================================================
@@ -42,7 +42,7 @@ $(addprefix $(obj)/,$(zlib) main.o): $(a
 
 src-wlib := string.S crt0.S stdio.c main.c flatdevtree.c flatdevtree_misc.c \
 		ns16550.c serial.c simple_alloc.c div64.S util.S \
-		gunzip_util.c $(zlib)
+		gunzip_util.c $(zlib) devtree.c
 src-plat := of.c
 src-boot := $(src-wlib) $(src-plat) empty.c
 
arch/powerpc/boot/main.c +8 -13
Index: working-2.6/arch/powerpc/boot/main.c
===================================================================
@@ -156,9 +156,6 @@ static struct addr_range prep_initrd(str
 				     unsigned long initrd_addr,
 				     unsigned long initrd_size)
 {
-	void *devp;
-	u32 initrd_start, initrd_end;
-
 	/* If we have an image attached to us, it overrides anything
 	 * supplied by the loader. */
 	if (_initrd_end > _initrd_start) {
@@ -197,16 +194,10 @@ static struct addr_range prep_initrd(str
 	printf("initrd head: 0x%lx\n\r", *((unsigned long *)initrd_addr));
 
 	/* Tell the kernel initrd address via device tree */
-	devp = finddevice("/chosen");
-	if (! devp)
-		fatal("Device tree has no chosen node!\n\r");
-
-	initrd_start = (u32)initrd_addr;
-	initrd_end = (u32)initrd_addr + initrd_size;
-
-	setprop(devp, "linux,initrd-start", &initrd_start,
-		sizeof(initrd_start));
-	setprop(devp, "linux,initrd-end", &initrd_end, sizeof(initrd_end));
+	dt_path_setprop_val("/chosen", "linux,initrd-start",
+			    (u32)(initrd_addr));
+	dt_path_setprop_val("/chosen", "linux,initrd-end",
+			    (u32)(initrd_addr + initrd_size));
 
 	return (struct addr_range){(void *)initrd_addr, initrd_size};
 }
@@ -262,6 +253,10 @@ void start(void *sp)
 	printf("\n\rzImage starting: loaded at 0x%p (sp: 0x%p)\n\r",
 	       _start, sp);
 
+	/* Ensure that the device tree has a /chosen node */
+	if (! finddevice("/chosen"))
+		create_node(NULL, "chosen");
+
 	vmlinux = prep_kernel();
 	initrd = prep_initrd(vmlinux, loader_info.initrd_addr,
 			     loader_info.initrd_size);
arch/powerpc/boot/devtree.c new file +126 -0

126 lines not on this page · raw message

#5 lore raw

Re: [PATCH 2/4] powerpc: Add device tree utility functions to zImage

Following your reply to Scott's comments about space in the xlate_reg thread, how about splitting the ones used in main.c which will always be included from the others that are more for device tree pokers.

For that matter, your streamlining adds runtime overhead. Why not cache the checked or created phandle?

Also, in dt_path_getprop, did you want to call finddevice instead of testing the function is not weak and undefined?

milton

#6 lore raw

Re: [PATCH 2/4] powerpc: Add device tree utility functions to zImage

Quoted 4 lines · Milton Miller

Following your reply to Scott's comments about space in the xlate_reg thread, how about splitting the ones used in main.c which will always be included from the others that are more for device tree pokers.

Yeah, I guess.

Quoted 2 lines

For that matter, your streamlining adds runtime overhead. Why not cache the checked or created phandle?

Well, I made a judgeement that source simplicity was more valuable than runtime speed. However, in the second spin, I've reconsidered and gotten rid of the dt_path_{get,set}prop functions entirely which addresses both concerns above.

Quoted 2 lines

Also, in dt_path_getprop, did you want to call finddevice instead of testing the function is not weak and undefined?

Yes. Fixed now.

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
#7 lore raw

Re: [PATCH 2/4] powerpc: Add device tree utility functions to zImage

In reply to David Gibson

arch/powerpc/boot/main.c void start(void *sp) · line 262

Quoted 11 lines
arch/powerpc/boot/main.c +4 -0
@@ -262,6 +253,10 @@ void start(void *sp)
 	printf("\n\rzImage starting: loaded at 0x%p (sp: 0x%p)\n\r",
 	       _start, sp);
 
+	/* Ensure that the device tree has a /chosen node */
+	if (! finddevice("/chosen"))
+		create_node(NULL, "chosen");
+
 	vmlinux = prep_kernel();
 	initrd = prep_initrd(vmlinux, loader_info.initrd_addr,
 			     loader_info.initrd_size);

So this is done early, and unconditional now? Just double checking, not enough context (for me, anyway) in the patch itself.

Segher

#8 lore raw

Re: [PATCH 2/4] powerpc: Add device tree utility functions to zImage

Quoted 4 lines · Segher Boessenkool
Quoted 4 lines
arch/powerpc/boot/main.c +0 -0
@@ -262,6 +253,10 @@ void start(void *sp)
 	printf("\n\rzImage starting: loaded at 0x%p (sp: 0x%p)\n\r",
 	       _start, sp);
 
Quoted 11 lines

arch/powerpc/boot/main.c void start(void *sp) · line 262

Quoted 7 lines
arch/powerpc/boot/main.c +4 -0
+	/* Ensure that the device tree has a /chosen node */
+	if (! finddevice("/chosen"))
+		create_node(NULL, "chosen");
+
 	vmlinux = prep_kernel();
 	initrd = prep_initrd(vmlinux, loader_info.initrd_addr,
 			     loader_info.initrd_size);

So this is done early, and unconditional now? Just double checking, not enough context (for me, anyway) in the patch itself.

Yes, that's right.

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

Re: [PATCH 2/4] powerpc: Add device tree utility functions to zImage

In reply to David Gibson

arch/powerpc/boot/devtree.c line 28

Quoted 8 lines · David Gibson
arch/powerpc/boot/devtree.c new file +8 -0
+int dt_path_getprop(const char *path, const char *name, void *buf, int bufsize)
+{
+	void *devp = finddevice;
+
+	if (devp)
+		return getprop(devp, name, buf, bufsize);
+	return -1;
+}

Should be "void *devp = finddevice(path);".

-Scott

#10 lore raw

Re: [PATCH 2/4] powerpc: Add device tree utility functions to zImage

Quoted 11 lines · Scott Wood

arch/powerpc/boot/devtree.c line 28

Quoted 8 lines · David Gibson
arch/powerpc/boot/devtree.c new file +8 -0
+int dt_path_getprop(const char *path, const char *name, void *buf, int bufsize)
+{
+	void *devp = finddevice;
+
+	if (devp)
+		return getprop(devp, name, buf, bufsize);
+	return -1;
+}

Should be "void *devp = finddevice(path);".

Eep. Yes.

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
#11 lore raw

PATCH 3/4 powerpc: Clean up zImage handling of the command line

In reply to David Gibson

This patch cleans up how the zImage code manipulates the kernel command line. Notable improvements from the old handling:
- Command line manipulation is consolidated into a new
prep_cmdline() function, rather than being scattered across start() and some helper functions
- Less stack space use: we use just a single global command
line buffer, which can be initialized by an external tool as before, we no longer need another command line sized buffer on the stack.
- Easier to support platforms whose firmware passes a
commandline, but not a device tree. Platform code can now point new loader_info fields to the firmware's command line, rather than having to do early manipulation of the /chosen bootargs property which may then be rewritten again by the core.

---

 arch/powerpc/boot/main.c |   55 +++++++++++++++++------------------------------
 arch/powerpc/boot/ops.h  |    2 +
 2 files changed, 23 insertions(+), 34 deletions(-)
arch/powerpc/boot/main.c +21 -34
Index: working-2.6/arch/powerpc/boot/main.c
===================================================================
@@ -206,31 +206,23 @@ static struct addr_range prep_initrd(str
  * edit the command line passed to vmlinux (by setting /chosen/bootargs).
  * The buffer is put in it's own section so that tools may locate it easier.
  */
-static char builtin_cmdline[COMMAND_LINE_SIZE]
+static char cmdline[COMMAND_LINE_SIZE]
 	__attribute__((__section__("__builtin_cmdline")));
 
-static void get_cmdline(char *buf, int size)
+static void prep_cmdline(void)
 {
-	void *devp;
-	int len = strlen(builtin_cmdline);
+	if (cmdline[0] == '\0')
+		dt_path_getprop("/chosen", "bootargs",
+				cmdline, COMMAND_LINE_SIZE-1);
+
+	printf("\n\rLinux/PowerPC load: %s", cmdline);
+	/* If possible, edit the command line */
+	if (console_ops.edit_cmdline)
+		console_ops.edit_cmdline(cmdline, COMMAND_LINE_SIZE);
+	printf("\n\r");
 
-	buf[0] = '\0';
-
-	if (len > 0) { /* builtin_cmdline overrides dt's /chosen/bootargs */
-		len = min(len, size-1);
-		strncpy(buf, builtin_cmdline, len);
-		buf[len] = '\0';
-	}
-	else if ((devp = finddevice("/chosen")))
-		getprop(devp, "bootargs", buf, size);
-}
-
-static void set_cmdline(char *buf)
-{
-	void *devp;
-
-	if ((devp = finddevice("/chosen")))
-		setprop(devp, "bootargs", buf, strlen(buf) + 1);
+	/* Put the command line back into the devtree for the kernel */
+	dt_path_setprop_str("/chosen", "bootargs", cmdline);
 }
 
 struct platform_ops platform_ops;
@@ -242,9 +234,15 @@ void start(void *sp)
 {
 	struct addr_range vmlinux, initrd;
 	kernel_entry_t kentry;
-	char cmdline[COMMAND_LINE_SIZE];
 	unsigned long ft_addr = 0;
 
+	/* Do this first, because malloc() could clobber the loader's
+	 * command line.  Only use the loader command line if a
+	 * built-in command line wasn't set by an external tool */
+	if ((loader_info.cmdline_len > 0) && (cmdline[0] == '\0'))
+		memmove(cmdline, loader_info.cmdline,
+			min(loader_info.cmdline_len, COMMAND_LINE_SIZE-1));
+
 	if (console_ops.open && (console_ops.open() < 0))
 		exit();
 	if (platform_ops.fixups)
@@ -260,18 +258,7 @@ void start(void *sp)
 	vmlinux = prep_kernel();
 	initrd = prep_initrd(vmlinux, loader_info.initrd_addr,
 			     loader_info.initrd_size);
-
-	/* If cmdline came from zimage wrapper or if we can edit the one
-	 * in the dt, print it out and edit it, if possible.
-	 */
-	if ((strlen(builtin_cmdline) > 0) || console_ops.edit_cmdline) {
-		get_cmdline(cmdline, COMMAND_LINE_SIZE);
-		printf("\n\rLinux/PowerPC load: %s", cmdline);
-		if (console_ops.edit_cmdline)
-			console_ops.edit_cmdline(cmdline, COMMAND_LINE_SIZE);
-		printf("\n\r");
-		set_cmdline(cmdline);
-	}
+	prep_cmdline();
 
 	printf("Finalizing device tree...");
 	if (dt_ops.finalize)
arch/powerpc/boot/ops.h +2 -0
Index: working-2.6/arch/powerpc/boot/ops.h
===================================================================
@@ -70,6 +70,8 @@ struct serial_console_data {
 struct loader_info {
 	void *promptr;
 	unsigned long initrd_addr, initrd_size;
+	char *cmdline;
+	int cmdline_len;
 };
 extern struct loader_info loader_info;
 
#12 lore raw

PATCH 4/4 powerpc: New reg.h for the zImage

In reply to David Gibson

This patch adds a reg.h to the zImage code, with common definitions for accessing system reigsters. For now, this includes functions for retreiving the PVR and the stack pointer. This patch then uses the new reg.h to let start() display the running stack address without having to explicitly pass the stack as a parameter from the asm code.

---

 arch/powerpc/boot/crt0.S |    1 -
 arch/powerpc/boot/main.c |    5 +++--
 arch/powerpc/boot/ops.h  |    2 +-
 arch/powerpc/boot/reg.h  |   22 ++++++++++++++++++++++
 4 files changed, 26 insertions(+), 4 deletions(-)
arch/powerpc/boot/reg.h new file +22 -0
Index: working-2.6/arch/powerpc/boot/reg.h
===================================================================
@@ -0,0 +1,22 @@
+#ifndef _PPC_BOOT_REG_H
+#define _PPC_BOOT_REG_H
+/*
+ * Copyright 2007 Davud Gibson, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+static inline u32 mfpvr(void)
+{
+	u32 pvr;
+	asm volatile ("mfpvr	%0" : "=r"(pvr));
+	return pvr;
+}
+
+register void *__stack_pointer asm("r1");
+#define get_sp()	(__stack_pointer)
+
+#endif	/* _PPC_BOOT_REG_H */
arch/powerpc/boot/crt0.S +0 -1
Index: working-2.6/arch/powerpc/boot/crt0.S
===================================================================
@@ -97,5 +97,4 @@ _zimage_start_lib:
 	bl	platform_init
 
 	/* Call start */
-	mr	r3,r1
 	b	start
arch/powerpc/boot/main.c +3 -2
Index: working-2.6/arch/powerpc/boot/main.c
===================================================================
@@ -17,6 +17,7 @@
 #include "ops.h"
 #include "gunzip_util.h"
 #include "flatdevtree.h"
+#include "reg.h"
 
 extern char _start[];
 extern char __bss_start[];
@@ -230,7 +231,7 @@ struct dt_ops dt_ops;
 struct console_ops console_ops;
 struct loader_info loader_info;
 
-void start(void *sp)
+void start(void)
 {
 	struct addr_range vmlinux, initrd;
 	kernel_entry_t kentry;
@@ -249,7 +250,7 @@ void start(void *sp)
 		platform_ops.fixups();
 
 	printf("\n\rzImage starting: loaded at 0x%p (sp: 0x%p)\n\r",
-	       _start, sp);
+	       _start, get_sp());
 
 	/* Ensure that the device tree has a /chosen node */
 	if (! finddevice("/chosen"))
arch/powerpc/boot/ops.h +1 -1
Index: working-2.6/arch/powerpc/boot/ops.h
===================================================================
@@ -75,7 +75,7 @@ struct loader_info {
 };
 extern struct loader_info loader_info;
 
-void start(void *sp);
+void start(void);
 int ft_init(void *dt_blob, unsigned int max_size, unsigned int max_find_device);
 int serial_console_init(void);
 int ns16550_console_init(void *devp, struct serial_console_data *scdp);
#13 lore raw

cover Further zImage work (spin 2)

In reply to David Gibson

Ok, here's a revised version of that last series of zImage work. This now applies on top of the for-2.6.22 branch of powerpc.git, plus Milton's latest zImage patch series.

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
#14 lore raw

PATCH 1/4 powerpc: Add gcc format warnings to zImage printf()

This patch adds the correct attributes to the zImage's versions of printf to make gcc generate format string mismatch warnings. It also corrects several minor problems with format strings in the zImage thus discovered.

---

 arch/powerpc/boot/main.c  |    6 +++---
 arch/powerpc/boot/stdio.h |    5 +++--
 2 files changed, 6 insertions(+), 5 deletions(-)
arch/powerpc/boot/stdio.h +3 -2
Index: working-2.6/arch/powerpc/boot/stdio.h
===================================================================
@@ -7,11 +7,12 @@
 #define	EINVAL		22	/* Invalid argument */
 #define ENOSPC		28	/* No space left on device */
 
-extern int printf(const char *fmt, ...);
+extern int printf(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
 
 #define fprintf(fmt, args...)	printf(args)
 
-extern int sprintf(char *buf, const char *fmt, ...);
+extern int sprintf(char *buf, const char *fmt, ...)
+	__attribute__((format(printf, 2, 3)));
 
 extern int vsprintf(char *buf, const char *fmt, va_list args);
 
arch/powerpc/boot/main.c +3 -3
Index: working-2.6/arch/powerpc/boot/main.c
===================================================================
@@ -136,7 +136,7 @@ static struct addr_range prep_kernel(voi
 	} else {
 		if ((unsigned long)_start < ei.memsize)
 			fatal("Insufficient memory for kernel at address 0!"
-			       " (_start=%lx)\n\r", _start);
+			       " (_start=%p)\n\r", _start);
 	}
 
 	/* Finally, gunzip the kernel */
@@ -145,7 +145,7 @@ static struct addr_range prep_kernel(voi
 	/* discard up to the actual load data */
 	gunzip_discard(&gzstate, ei.elfoffset - sizeof(elfheader));
 	len = gunzip_finish(&gzstate, addr, ei.memsize);
-	printf("done 0x%lx bytes\n\r", len);
+	printf("done 0x%x bytes\n\r", len);
 
 	flush_cache(addr, ei.loadsize);
 
@@ -189,7 +189,7 @@ static struct addr_range prep_initrd(str
 		if (! initrd_addr)
 			fatal("Can't allocate memory for initial "
 			       "ramdisk !\n\r");
-		printf("Relocating initrd 0x%p <- 0x%p (0x%lx bytes)\n\r",
+		printf("Relocating initrd 0x%lx <- 0x%p (0x%lx bytes)\n\r",
 		       initrd_addr, old_addr, initrd_size);
 		memmove((void *)initrd_addr, old_addr, initrd_size);
 	}
#15 lore raw

PATCH 2/4 powerpc: Add device tree utility functions to zImage

In reply to David Gibson

This patch adds a library of useful device tree manipulation functions to the zImage library, for use by platform code. These functions are based on the hooks already in dt_ops, so they're not dependent on a particular device tree implementation. This patch also slightly streamlines the code in main.c using these new functions.

This is a consolidation of my work in this area with Scott Wood's patches to a very similar end.

---

 arch/powerpc/boot/Makefile  |    2 
 arch/powerpc/boot/devtree.c |  111 ++++++++++++++++++++++++++++++++++++++++++++
 arch/powerpc/boot/main.c    |   27 ++++------
 arch/powerpc/boot/ops.h     |   13 +++++
 4 files changed, 136 insertions(+), 17 deletions(-)
arch/powerpc/boot/ops.h +13 -0
Index: working-2.6/arch/powerpc/boot/ops.h
===================================================================
@@ -96,6 +96,11 @@ static inline int setprop(void *devp, co
 {
 	return (dt_ops.setprop) ? dt_ops.setprop(devp, name, buf, buflen) : -1;
 }
+#define setprop_val(devp, name, val) \
+	do { \
+		typeof(val) x = (val); \
+		setprop((devp), (name), &x, sizeof(x)); \
+	} while (0)
 
 static inline int setprop_str(void *devp, const char *name, const char *buf)
 {
@@ -141,6 +146,14 @@ static inline void *find_node_by_devtype
 	return find_node_by_prop_value_str(prev, "device_type", type);
 }
 
+void dt_fixup_memory(u64 start, u64 size);
+void dt_fixup_cpu_clocks(u32 cpufreq, u32 tbfreq, u32 busfreq);
+void dt_fixup_clock(const char *path, u32 freq);
+void __dt_fixup_mac_addresses(u32 startindex, ...);
+#define dt_fixup_mac_addresses(...) \
+	__dt_fixup_mac_addresses(0, __VA_ARGS__, NULL)
+
+
 static inline void *malloc(u32 size)
 {
 	return (platform_ops.malloc) ? platform_ops.malloc(size) : NULL;
arch/powerpc/boot/Makefile +1 -1
Index: working-2.6/arch/powerpc/boot/Makefile
===================================================================
@@ -42,7 +42,7 @@ $(addprefix $(obj)/,$(zlib) main.o): $(a
 
 src-wlib := string.S crt0.S stdio.c main.c flatdevtree.c flatdevtree_misc.c \
 		ns16550.c serial.c simple_alloc.c div64.S util.S \
-		gunzip_util.c $(zlib)
+		gunzip_util.c $(zlib) devtree.c
 src-plat := of.c
 src-boot := $(src-wlib) $(src-plat) empty.c
 
arch/powerpc/boot/main.c +11 -16
Index: working-2.6/arch/powerpc/boot/main.c
===================================================================
@@ -152,13 +152,10 @@ static struct addr_range prep_kernel(voi
 	return (struct addr_range){addr, ei.memsize};
 }
 
-static struct addr_range prep_initrd(struct addr_range vmlinux,
+static struct addr_range prep_initrd(struct addr_range vmlinux, void *chosen,
 				     unsigned long initrd_addr,
 				     unsigned long initrd_size)
 {
-	void *devp;
-	u32 initrd_start, initrd_end;
-
 	/* If we have an image attached to us, it overrides anything
 	 * supplied by the loader. */
 	if (_initrd_end > _initrd_start) {
@@ -197,16 +194,8 @@ static struct addr_range prep_initrd(str
 	printf("initrd head: 0x%lx\n\r", *((unsigned long *)initrd_addr));
 
 	/* Tell the kernel initrd address via device tree */
-	devp = finddevice("/chosen");
-	if (! devp)
-		fatal("Device tree has no chosen node!\n\r");
-
-	initrd_start = (u32)initrd_addr;
-	initrd_end = (u32)initrd_addr + initrd_size;
-
-	setprop(devp, "linux,initrd-start", &initrd_start,
-		sizeof(initrd_start));
-	setprop(devp, "linux,initrd-end", &initrd_end, sizeof(initrd_end));
+	setprop_val(chosen, "linux,initrd-start", (u32)(initrd_addr));
+	setprop_val(chosen, "linux,initrd-end", (u32)(initrd_addr+initrd_size));
 
 	return (struct addr_range){(void *)initrd_addr, initrd_size};
 }
@@ -253,6 +242,7 @@ void start(void *sp)
 	kernel_entry_t kentry;
 	char cmdline[COMMAND_LINE_SIZE];
 	unsigned long ft_addr = 0;
+	void *chosen;
 
 	if (console_ops.open && (console_ops.open() < 0))
 		exit();
@@ -262,9 +252,14 @@ void start(void *sp)
 	printf("\n\rzImage starting: loaded at 0x%p (sp: 0x%p)\n\r",
 	       _start, sp);
 
+	/* Ensure that the device tree has a /chosen node */
+	chosen = finddevice("/chosen");
+	if (! chosen)
+		chosen = create_node(NULL, "chosen");
+
 	vmlinux = prep_kernel();
-	initrd = prep_initrd(vmlinux, loader_info.initrd_addr,
-			     loader_info.initrd_size);
+	initrd = prep_initrd(vmlinux, chosen,
+			     loader_info.initrd_addr, loader_info.initrd_size);
 
 	/* If cmdline came from zimage wrapper or if we can edit the one
 	 * in the dt, print it out and edit it, if possible.
arch/powerpc/boot/devtree.c new file +111 -0

111 lines not on this page · raw message

#16 lore raw

PATCH 3/4 powerpc: Clean up zImage handling of the command line

In reply to David Gibson

This patch cleans up how the zImage code manipulates the kernel command line. Notable improvements from the old handling:
- Command line manipulation is consolidated into a new
prep_cmdline() function, rather than being scattered across start() and some helper functions
- Less stack space use: we use just a single global command
line buffer, which can be initialized by an external tool as before, we no longer need another command line sized buffer on the stack.
- Easier to support platforms whose firmware passes a
commandline, but not a device tree. Platform code can now point new loader_info fields to the firmware's command line, rather than having to do early manipulation of the /chosen bootargs property which may then be rewritten again by the core.

---

 arch/powerpc/boot/main.c |   52 +++++++++++++++++------------------------------
 arch/powerpc/boot/ops.h  |    2 +
 2 files changed, 21 insertions(+), 33 deletions(-)
arch/powerpc/boot/main.c +19 -33
Index: working-2.6/arch/powerpc/boot/main.c
===================================================================
@@ -204,31 +204,22 @@ static struct addr_range prep_initrd(str
  * edit the command line passed to vmlinux (by setting /chosen/bootargs).
  * The buffer is put in it's own section so that tools may locate it easier.
  */
-static char builtin_cmdline[COMMAND_LINE_SIZE]
+static char cmdline[COMMAND_LINE_SIZE]
 	__attribute__((__section__("__builtin_cmdline")));
 
-static void get_cmdline(char *buf, int size)
+static void prep_cmdline(void *chosen)
 {
-	void *devp;
-	int len = strlen(builtin_cmdline);
+	if (cmdline[0] == '\0')
+		getprop(chosen, "bootargs", cmdline, COMMAND_LINE_SIZE-1);
 
-	buf[0] = '\0';
+	printf("\n\rLinux/PowerPC load: %s", cmdline);
+	/* If possible, edit the command line */
+	if (console_ops.edit_cmdline)
+		console_ops.edit_cmdline(cmdline, COMMAND_LINE_SIZE);
+	printf("\n\r");
 
-	if (len > 0) { /* builtin_cmdline overrides dt's /chosen/bootargs */
-		len = min(len, size-1);
-		strncpy(buf, builtin_cmdline, len);
-		buf[len] = '\0';
-	}
-	else if ((devp = finddevice("/chosen")))
-		getprop(devp, "bootargs", buf, size);
-}
-
-static void set_cmdline(char *buf)
-{
-	void *devp;
-
-	if ((devp = finddevice("/chosen")))
-		setprop(devp, "bootargs", buf, strlen(buf) + 1);
+	/* Put the command line back into the devtree for the kernel */
+	setprop_str(chosen, "bootargs", cmdline);
 }
 
 struct platform_ops platform_ops;
@@ -240,10 +231,16 @@ void start(void *sp)
 {
 	struct addr_range vmlinux, initrd;
 	kernel_entry_t kentry;
-	char cmdline[COMMAND_LINE_SIZE];
 	unsigned long ft_addr = 0;
 	void *chosen;
 
+	/* Do this first, because malloc() could clobber the loader's
+	 * command line.  Only use the loader command line if a
+	 * built-in command line wasn't set by an external tool */
+	if ((loader_info.cmdline_len > 0) && (cmdline[0] == '\0'))
+		memmove(cmdline, loader_info.cmdline,
+			min(loader_info.cmdline_len, COMMAND_LINE_SIZE-1));
+
 	if (console_ops.open && (console_ops.open() < 0))
 		exit();
 	if (platform_ops.fixups)
@@ -260,18 +257,7 @@ void start(void *sp)
 	vmlinux = prep_kernel();
 	initrd = prep_initrd(vmlinux, chosen,
 			     loader_info.initrd_addr, loader_info.initrd_size);
-
-	/* If cmdline came from zimage wrapper or if we can edit the one
-	 * in the dt, print it out and edit it, if possible.
-	 */
-	if ((strlen(builtin_cmdline) > 0) || console_ops.edit_cmdline) {
-		get_cmdline(cmdline, COMMAND_LINE_SIZE);
-		printf("\n\rLinux/PowerPC load: %s", cmdline);
-		if (console_ops.edit_cmdline)
-			console_ops.edit_cmdline(cmdline, COMMAND_LINE_SIZE);
-		printf("\n\r");
-		set_cmdline(cmdline);
-	}
+	prep_cmdline(chosen);
 
 	printf("Finalizing device tree...");
 	if (dt_ops.finalize)
arch/powerpc/boot/ops.h +2 -0
Index: working-2.6/arch/powerpc/boot/ops.h
===================================================================
@@ -70,6 +70,8 @@ struct serial_console_data {
 struct loader_info {
 	void *promptr;
 	unsigned long initrd_addr, initrd_size;
+	char *cmdline;
+	int cmdline_len;
 };
 extern struct loader_info loader_info;
 
#17 lore raw

PATCH 4/4 powerpc: New reg.h for the zImage

In reply to David Gibson

This patch adds a reg.h to the zImage code, with common definitions for accessing system reigsters. For now, this includes functions for retreiving the PVR and the stack pointer. This patch then uses the new reg.h to let start() display the running stack address without having to explicitly pass the stack as a parameter from the asm code.

---

 arch/powerpc/boot/crt0.S |    1 -
 arch/powerpc/boot/main.c |    5 +++--
 arch/powerpc/boot/ops.h  |    2 +-
 arch/powerpc/boot/reg.h  |   22 ++++++++++++++++++++++
 4 files changed, 26 insertions(+), 4 deletions(-)
arch/powerpc/boot/reg.h new file +22 -0
Index: working-2.6/arch/powerpc/boot/reg.h
===================================================================
@@ -0,0 +1,22 @@
+#ifndef _PPC_BOOT_REG_H
+#define _PPC_BOOT_REG_H
+/*
+ * Copyright 2007 Davud Gibson, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+static inline u32 mfpvr(void)
+{
+	u32 pvr;
+	asm volatile ("mfpvr	%0" : "=r"(pvr));
+	return pvr;
+}
+
+register void *__stack_pointer asm("r1");
+#define get_sp()	(__stack_pointer)
+
+#endif	/* _PPC_BOOT_REG_H */
arch/powerpc/boot/crt0.S +0 -1
Index: working-2.6/arch/powerpc/boot/crt0.S
===================================================================
@@ -97,5 +97,4 @@ _zimage_start_lib:
 	bl	platform_init
 
 	/* Call start */
-	mr	r3,r1
 	b	start
arch/powerpc/boot/main.c +3 -2
Index: working-2.6/arch/powerpc/boot/main.c
===================================================================
@@ -17,6 +17,7 @@
 #include "ops.h"
 #include "gunzip_util.h"
 #include "flatdevtree.h"
+#include "reg.h"
 
 extern char _start[];
 extern char __bss_start[];
@@ -230,7 +231,7 @@ struct dt_ops dt_ops;
 struct console_ops console_ops;
 struct loader_info loader_info;
 
-void start(void *sp)
+void start(void)
 {
 	struct addr_range vmlinux, initrd;
 	kernel_entry_t kentry;
@@ -249,7 +250,7 @@ void start(void *sp)
 		platform_ops.fixups();
 
 	printf("\n\rzImage starting: loaded at 0x%p (sp: 0x%p)\n\r",
-	       _start, sp);
+	       _start, get_sp());
 
 	/* Ensure that the device tree has a /chosen node */
 	if (! finddevice("/chosen"))
arch/powerpc/boot/ops.h +1 -1
Index: working-2.6/arch/powerpc/boot/ops.h
===================================================================
@@ -75,7 +75,7 @@ struct loader_info {
 };
 extern struct loader_info loader_info;
 
-void start(void *sp);
+void start(void);
 int ft_init(void *dt_blob, unsigned int max_size, unsigned int max_find_device);
 int serial_console_init(void);
 int ns16550_console_init(void *devp, struct serial_console_data *scdp);