bootwrapper: Add a firmware-independent "raw" target.

PATCH In review Stephen Neuendorffer 4 participants 5 messages Dec 13, 2007 – Dec 16, 2007 PowerPC

lore

Share & tools

lore

Stephen Neuendorffer

author

2007-12-13 23:43 UTC #0 lore raw

PATCH 1/7 bootwrapper: Add a firmware-independent "raw" target.

From: Grant Likely <[email protected]>

This target produces a flat binary rather than an ELF file, fixes the entry point at the beginning of the image, and takes a complete device tree with no fixups needed.

The device tree must have labels on /#address-cells, the timebase frequency, and the memory size.

Signed-off-by Grant Likely <[email protected]>

---
 arch/powerpc/Kconfig             |   12 +++++++++++
 arch/powerpc/boot/Makefile       |    6 ++++-
 arch/powerpc/boot/io.h           |    7 ++++++
 arch/powerpc/boot/raw-platform.c |   41 ++++++++++++++++++++++++++++++++++++++
 arch/powerpc/boot/wrapper        |   15 +++++++++----
 5 files changed, 75 insertions(+), 6 deletions(-)
 create mode 100644 arch/powerpc/boot/raw-platform.c

arch/powerpc/Kconfig +12 -0

index 232c298..4a60ec4 100644
@@ -394,6 +394,18 @@ config WANT_DEVICE_TREE
 	bool
 	default n
 
+config BUILD_RAW_IMAGE
+	bool "Build firmware-independent image"
+	select WANT_DEVICE_TREE
+	help
+	  If this is enabled, a firmware independent "raw" image will be
+	  built, as zImage.raw.  This requires a completely filled-in
+	  device tree, with the following labels:
+
+	  mem_size_cells: on /#address-cells
+	  memsize: on the size portion of /memory/reg
+	  timebase: on the boot CPU's timebase property
+
 config DEVICE_TREE
 	string "Static device tree source file"
 	depends on WANT_DEVICE_TREE

arch/powerpc/boot/Makefile +5 -1

index 18e3271..975e1f5 100644
@@ -56,7 +56,7 @@ src-plat := of.c cuboot-52xx.c cuboot-83xx.c cuboot-85xx.c holly.c \
 		cuboot-ebony.c treeboot-ebony.c prpmc2800.c \
 		ps3-head.S ps3-hvcall.S ps3.c treeboot-bamboo.c cuboot-8xx.c \
 		cuboot-pq2.c cuboot-sequoia.c treeboot-walnut.c cuboot-bamboo.c \
-		fixed-head.S ep88xc.c cuboot-hpc2.c
+		fixed-head.S ep88xc.c cuboot-hpc2.c raw-platform.c
 src-boot := $(src-wlib) $(src-plat) empty.c
 
 src-boot := $(addprefix $(obj)/, $(src-boot))

@@ -159,6 +159,7 @@ image-$(CONFIG_EBONY)			+= treeImage.ebony cuImage.ebony
 image-$(CONFIG_BAMBOO)			+= treeImage.bamboo cuImage.bamboo
 image-$(CONFIG_SEQUOIA)			+= cuImage.sequoia
 image-$(CONFIG_WALNUT)			+= treeImage.walnut
+image-$(CONFIG_BUILD_RAW_IMAGE)		+= zImage.raw
 endif
 
 # For 32-bit powermacs, build the COFF and miboot images

@@ -221,6 +222,9 @@ $(obj)/treeImage.initrd.%: vmlinux $(dts) $(wrapperbits)
 $(obj)/treeImage.%: vmlinux $(dts) $(wrapperbits)
 	$(call if_changed,wrap,treeboot-$*,$(dts))
 
+$(obj)/zImage.raw: vmlinux $(dts) $(wrapperbits)
+	$(call if_changed,wrap,raw,$(dts))
+
 # If there isn't a platform selected then just strip the vmlinux.
 ifeq (,$(image-y))
 image-y := vmlinux.strip

arch/powerpc/boot/io.h +7 -0

index ccaedae..ec57ec9 100644
@@ -99,4 +99,11 @@ static inline void barrier(void)
 	asm volatile("" : : : "memory");
 }
 
+static inline void disable_irq(void)
+{
+	int dummy;
+	asm volatile("mfmsr %0; rlwinm %0, %0, 0, ~(1<<15); mtmsr %0" :
+	             "=r" (dummy) : : "memory");
+}
+
 #endif /* _IO_H */

arch/powerpc/boot/raw-platform.c new file +41 -0

41 lines not on this page · raw message

arch/powerpc/boot/wrapper +10 -5

index 31147a0..716cd44 100755
@@ -116,10 +116,11 @@ while [ "$#" -gt 0 ]; do
 done
 
 if [ -n "$dts" ]; then
-    if [ -z "$dtb" ]; then
-	dtb="$platform.dtb"
-    fi
-    dtc -O dtb -o "$dtb" -b 0 -V 16 "$dts"
+    dtasm="$object/$platform.dtb.S"
+    dto="$object/$platform.dtb.o"
+    echo '.section .kernel:dtb,"a"' > "$dtasm"
+    dtc -O asm -b 0 -V 16 "$dts" >> "$dtasm"
+    ${CROSS}gcc "$dtasm" -c -o "$dto"
 fi
 
 if [ -z "$kernel" ]; then

@@ -167,6 +168,10 @@ ep88xc)
     platformo="$object/fixed-head.o $object/$platform.o"
     binary=y
     ;;
+raw)
+    platformo="$object/fixed-head.o $object/raw-platform.o"
+    binary=y
+    ;;
 esac
 
 vmz="$tmpdir/`basename \"$kernel\"`.$ext"

@@ -230,7 +235,7 @@ fi
 
 if [ "$platform" != "miboot" ]; then
     ${CROSS}ld -m elf32ppc -T $lds -o "$ofile" \
-	$platformo $tmp $object/wrapper.a
+	$platformo $tmp $dto $object/wrapper.a
     rm $tmp
 fi

Signature · 1 line

1.5.3.4

Milton Miller

2007-12-14 08:46 UTC #1 lore raw

Quoted 11 lines · Stephen Neuendorffer

From: Grant Likely <grant.likely at secretlab.ca>
This target produces a flat binary rather than an ELF file, fixes the entry point at the beginning of the image, and takes a complete device tree with no fixups needed.
The device tree must have labels on /#address-cells, the timebase frequency, and the memory size.
Signed-off-by Grant Likely <grant.likely at secretlab.ca>
---

You indicated in the intro in 0/ that this was not ready, and you didn't include your own s-o-b, but you did not put any statements to that effect in the header. The intro is not copied into patchwork, which maintainers often use when deciding what to push.

Now on to why this should not be merged:

In addition to the above, it changes the build rules. It tries to change wrapper to assemble the .dtb into a .o from a .S file, but doesn't set any flags to force the assembler into the right mode. In contrast the linker is controlled by the .lds linker script.

In addition, the requirement for assembly labels can easily be eliminated. As mentioned above, they are used for 3 properties. With the existing library (in 2.6.24 and earlier), call simple_malloc_init with a small bss array (like BSS_STACK does to allocate stack), and then read the properties out of the device tree. At that point, call simple_malloc_init a second time using the found memory size. As I said the last time this was posted, my patches to boot from kexec implemented this strategy.

However, with the new libfdt, which is already in for-2.6.25, we should no longer need malloc() to simple read the tree. At least that is what was advertised.

Quoted 3 lines

+$(obj)/zImage.raw: vmlinux $(dts) $(wrapperbits)
+ $(call if_changed,wrap,raw,$(dts))

This should be handled by the standard zImage% rule.

Quoted 4 lines

+void platform_init(unsigned long r3, unsigned long r4, unsigned long r5,
+ unsigned long r6, unsigned long r7)

The calling convention of platform_init is controlled by head, and doesn't have a global prototype. Since you use none of these arguments, this implementation should take void.

In general, serial_console_init is not safe to call, as the wrapper has no access mode change for its io access macros and instead requires firmware to setup a mapping. That and the fact the wrapper uses the same properties to choose the console as the kernel means even kexec-tools can't add or delete a property to kill that call. (This statement is made from the viewpoint that this raw image should be suitable for kexec in addition to being loaded by a dumb firmware).

I'm not sure exactly what platform you are using this on. Apparently it is a legacy firmware that loads the image and jumps to it leaving interrupts on and not invalidating the cache.

I was going to suggest this platform_init be turned into a helper like simple_platform_init or so. However, that brings up a philosophy question on what the directory should look like (I was going for a modular library with lots of small routines loaded on demand, but others are going for fewer files that tend to drag sometimes used routines. They later resist features with "thats a lot of code"). And with the read_only libfdt pending, the remaining code maybe considered too trivial not to replicate in the platforms that need it.

milton

Grant Likely

2007-12-14 15:23 UTC #2 lore raw

Quoted 4 lines · Milton Miller

Quoted 2 lines · Stephen Neuendorffer
From: Grant Likely <grant.likely at secretlab.ca>

Quoted 8 lines

Quoted 8 lines
This target produces a flat binary rather than an ELF file, fixes the entry point at the beginning of the image, and takes a complete device tree with no fixups needed.
The device tree must have labels on /#address-cells, the timebase frequency, and the memory size.
Signed-off-by Grant Likely <grant.likely at secretlab.ca>

For the record; this patch was lifted from my dev tree and in turn I stole it from Scott Wood.

Quoted 6 lines

Quoted 4 lines
+$(obj)/zImage.raw: vmlinux $(dts) $(wrapperbits)
+ $(call if_changed,wrap,raw,$(dts))
This should be handled by the standard zImage% rule.

It's weird. On my system the standard zImage rule wouldn't do this target. I had to add this rule just to get it to compile. I've not yet been motivated to revisit and fix it.

Quoted 3 lines

I'm not sure exactly what platform you are using this on. Apparently it is a legacy firmware that loads the image and jumps to it leaving interrupts on and not invalidating the cache.

Heh; actually it's a platform with *no* firmware. :-)

It's on a Xilinx FPGA. SDRAM is initialized by the FPGA design and the kernel image is loaded either with a debugger, or via the SystemACE device. In either case, the kernel is the first thing to run after reset.... oh, and reset is not as complete as one would normally expect.

Cheers,
g.

Signature · 4 lines

Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[email protected]
(403) 399-0195

Stephen Neuendorffer

author

2007-12-14 17:31 UTC #3 lore raw

In reply to Milton Miller

=20

Quoted 19 lines

-----Original Message-----
From: Milton Miller [mailto:[email protected]]=20
Sent: Friday, December 14, 2007 12:06 AM
To: Stephen Neuendorffer
Cc: ppcdev; Grant Likely
Subject: Re: [PATCH 1/7] bootwrapper: Add a=20 firmware-independent "raw" target.
=20
On Fri Dec 14 10:43:27 EST 2007, Stephen Neuendorffer wrote:
=20
Quoted 9 lines
From: Grant Likely <grant.likely at secretlab.ca>
This target produces a flat binary rather than an ELF file, fixes the entry point at the beginning of the image, and takes a complete device tree with no fixups needed.
The device tree must have labels on /#address-cells, the timebase frequency, and the memory size.

Quoted 8 lines

Quoted 2 lines
Signed-off-by Grant Likely <grant.likely at secretlab.ca>
---
=20
=20
You indicated in the intro in 0/ that this was not ready, and you=20 didn't include your own s-o-b, but you did not put any statements to=20 that effect in the header. The intro is not copied into patchwork,=20 which maintainers often use when deciding what to push.

Sorry... Still trying to figure out the process.

Quoted 13 lines

Now on to why this should not be merged:
=20
In addition to the above, it changes the build rules. It tries to=20 change wrapper to assemble the .dtb into a .o from a .S file, but=20 doesn't set any flags to force the assembler into the right mode. In=20 contrast the linker is controlled by the .lds linker script. =20
In addition, the requirement for assembly labels can easily be=20 eliminated. As mentioned above, they are used for 3=20 properties. With=20
the existing library (in 2.6.24 and earlier), call simple_malloc_init=20 with a small bss array (like BSS_STACK does to allocate stack), and=20 then read the properties out of the device tree. At that point, call=20

Quoted 8 lines

simple_malloc_init a second time using the found memory size. As I=20
said the last time this was posted, my patches to boot from kexec=20 implemented this strategy.
=20
However, with the new libfdt, which is already in for-2.6.25,=20 we should=20
no longer need malloc() to simple read the tree. At least that is=20
what was advertised.

Yes, I agree, I just haven't had a chance to go back and write that code yet...

Thanks for the comments,
Steve

David Gibson

2007-12-16 11:35 UTC #4 lore raw

In reply to Milton Miller

Quoted 36 lines · Milton Miller

Quoted 11 lines · Stephen Neuendorffer
From: Grant Likely <grant.likely at secretlab.ca>
This target produces a flat binary rather than an ELF file, fixes the entry point at the beginning of the image, and takes a complete device tree with no fixups needed.
The device tree must have labels on /#address-cells, the timebase frequency, and the memory size.
Signed-off-by Grant Likely <grant.likely at secretlab.ca>
---
You indicated in the intro in 0/ that this was not ready, and you didn't include your own s-o-b, but you did not put any statements to that effect in the header. The intro is not copied into patchwork, which maintainers often use when deciding what to push.
Now on to why this should not be merged:
In addition to the above, it changes the build rules. It tries to change wrapper to assemble the .dtb into a .o from a .S file, but doesn't set any flags to force the assembler into the right mode. In contrast the linker is controlled by the .lds linker script.
In addition, the requirement for assembly labels can easily be eliminated. As mentioned above, they are used for 3 properties. With the existing library (in 2.6.24 and earlier), call simple_malloc_init with a small bss array (like BSS_STACK does to allocate stack), and then read the properties out of the device tree. At that point, call
simple_malloc_init a second time using the found memory size. As I
said the last time this was posted, my patches to boot from kexec implemented this strategy.

Quoted 3 lines

However, with the new libfdt, which is already in for-2.6.25, we should
no longer need malloc() to simple read the tree. At least that is
what was advertised.

That's correct. No malloc() is necessary for read-only access.

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