[PATCH] Provide mechanism for editing builtin command-line in zImage binary.

[PATCH] Provide mechanism for editing builtin command-line in zImage binary.

[mostrows]

zImage will store the builtin command-line in a dedicated section, allowing
it to be easily identified and edited with user-space tools.

zImage will set /chosen/bootargs to the stored builtin command-line setting,
if /chosen/bootargs is empty (emulating the behavior in prom_init.c).

Use of this mechanism avoids the need to modify firmware or rely on a
bootloader to customize kernel arguments (and overall system
behavior).  The command line can be edited as needed when a zImage is
copied to a TFTP staging area for download by firmware.

--
Signed-off-by: Michal Ostrowski <mostrows@watson.ibm.com>

---

 arch/powerpc/boot/Makefile |    3 ++-
 arch/powerpc/boot/main.c   |   28 ++++++++++++++++++++++++++++
 arch/powerpc/boot/prom.h   |    6 ++++++
 3 files changed, 36 insertions(+), 1 deletions(-)

38d6b8581e01886e352b699c978a3a5ac6672622
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 840ae59..d7ba474 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -41,7 +41,8 @@ src-boot += $(zlib)
 src-boot := $(addprefix $(obj)/, $(src-boot))
 obj-boot := $(addsuffix .o, $(basename $(src-boot)))
 
-BOOTCFLAGS	+= -I$(obj) -I$(srctree)/$(obj)
+BOOTCFLAGS	+= -I$(obj) -I$(srctree)/$(obj) -I$(O)/include -I$(O)/include2\
+			-D__KERNEL__
 
 quiet_cmd_copy_zlib = COPY    $@
       cmd_copy_zlib = sed "s@__attribute_used__@@;s@<linux/\([^>]\+\).*@\"\1\"@" $< > $@
diff --git a/arch/powerpc/boot/main.c b/arch/powerpc/boot/main.c
index 816446f..eef25de 100644
--- a/arch/powerpc/boot/main.c
+++ b/arch/powerpc/boot/main.c
@@ -10,6 +10,9 @@
  */
 #include <stdarg.h>
 #include <stddef.h>
+#include <linux/autoconf.h>
+#include <asm/setup.h>
+
 #include "elf.h"
 #include "page.h"
 #include "string.h"
@@ -33,6 +36,16 @@ extern char _vmlinux_end[];
 extern char _initrd_start[];
 extern char _initrd_end[];
 
+#ifdef CONFIG_CMDLINE
+/* Putting this in a seperate section allows for simple tools to 
+ * edit this setting after a build (i.e. as the kernel image is being deployed
+ * for booting. 
+ */
+static char builtin_cmdline[COMMAND_LINE_SIZE]
+		__attribute__((section("__builtin_cmdline"))) = CONFIG_CMDLINE;
+#endif
+
+
 struct addr_range {
 	unsigned long addr;
 	unsigned long size;
@@ -204,6 +217,19 @@ static int is_elf32(void *hdr)
 	return 1;
 }
 
+void export_cmdline(void* chosen_handle)
+{
+#ifdef CONFIG_CMDLINE
+        int len;
+        char cmdline[COMMAND_LINE_SIZE];
+        len = getprop(chosen_handle, "bootargs", cmdline, sizeof(cmdline));
+        if ((len <= 0 || cmdline[0] == 0) && builtin_cmdline[0]) {
+                setprop(chosen_handle, "bootargs", builtin_cmdline,
+                        strlen(builtin_cmdline)+1);
+        }
+#endif /* CONFIG_CMDLINE */
+}
+
 void start(unsigned long a1, unsigned long a2, void *promptr, void *sp)
 {
 	int len;
@@ -289,6 +315,8 @@ void start(unsigned long a1, unsigned lo
 		memmove((void *)vmlinux.addr,(void *)vmlinuz.addr,vmlinuz.size);
 	}
 
+	export_cmdline(chosen_handle);
+
 	/* Skip over the ELF header */
 #ifdef DEBUG
 	printf("... skipping 0x%lx bytes of ELF header\n\r",
diff --git a/arch/powerpc/boot/prom.h b/arch/powerpc/boot/prom.h
index 3e2ddd4..f613092 100644
--- a/arch/powerpc/boot/prom.h
+++ b/arch/powerpc/boot/prom.h
@@ -31,4 +31,10 @@ static inline int getprop(void *phandle,
 	return call_prom("getprop", 4, 1, phandle, name, buf, buflen);
 }
 
+static inline int setprop(void *phandle, const char *name,
+                          void *buf, int buflen)
+{
+        return call_prom("setprop", 4, 1, phandle, name, buf, buflen);
+}
+
 #endif				/* _PPC_BOOT_PROM_H_ */
-- 
1.1.4.g0b63-dirty

Re: [PATCH] Provide mechanism for editing builtin command-line in zImage binary.

[Michal Ostrowski]

For reference, below is a shell script I use to edit the command-line
embedded in a zImage.


-- 
Michal Ostrowski <mostrows@watson.ibm.com>



#!/bin/bash
#
# Copyright (C) 2006 Michal Ostrowski <mostrows@watson.ibm.com>, IBM Corp.
#
# 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.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA

usage(){
    echo 'set_builtin_cmdline [--cmd <file>] <input_obj> [<output_obj>]'
    echo '    Sets the builtin command line embedded in an object file.'
    echo
    echo '    If <output_obj> is not specified, <input_obj> is modified'
    echo '    in place.'
    echo '    If --cmd <file> argument is present contents of embedded'
    echo '    command line will be copied from <file>, otherwise from stdin.'
    exit 1
}

cmd_file=

while case "$#" in 0) break ;; esac
do
  
        case "$1" in
        --cmd)
		case "$#" in
		    1)
			usage ;;
		    *)
			cmd_file="$2";
			shift;;
		esac;;
        *)
                set x "$@"
                shift
                break ;;
        esac
        shift
done

if [ "$#" -lt 1 ]; then
    echo "No input object specified." ; 
    usage;
fi

if [ "$#" -gt 2 ]; then
    echo "Unrecognized arguments: $@";
    usage;
fi


infile=$1
shift;

if [ ! -r "$infile" ] ; then 
    echo "Can't read '$infile'."; 
    usage;
fi

if [ "$#" -eq 0 ] ; then
    outfile=$infile;
else
    outfile=$1;
    if ! cp $infile $outfile ;  then
	echo "Can't create output: '$outfile' $?" 
	usage;
    fi
    shift;
fi

offset=$(objdump -h  $infile | \
         gawk -- '{if($2=="__builtin_cmdline") {print strtonum("0x" $6);}}')
size=$(objdump -h  $infile | \
       gawk -- '{if($2=="__builtin_cmdline") {print strtonum("0x" $3);}}')

if [ "$cmd_file" ] ; then
    [ -r "$cmd_file" ] || (echo "Can't read '$cmd_file'." ; usage);
    cmdline=$(cat $cmd_file);
else
    cmdline=$(cat);
fi

if [ "x$offset" != "x" ] ; then 
    if [ "$offset" -ne 0  ] ; then
	set -e
	# Zero the destination buffer first
	exec 2>/dev/null
	dd if=/dev/zero of=$outfile bs=1 seek=$offset conv=notrunc count=$size
	echo -n "$cmdline" | \
	    dd of=$outfile bs=1 seek=$offset conv=notrunc count=$size
	exit $?
    fi
fi

Re: [PATCH] Provide mechanism for editing builtin command-line in zImage binary.

[Mark A. Greer]

On Mon, May 29, 2006 at 10:01:03PM -0400, mostrows@watson.ibm.com wrote:
> zImage will store the builtin command-line in a dedicated section, allowing
> it to be easily identified and edited with user-space tools.
>
> zImage will set /chosen/bootargs to the stored builtin command-line setting,
> if /chosen/bootargs is empty (emulating the behavior in prom_init.c).
> 
> Use of this mechanism avoids the need to modify firmware or rely on a
> bootloader to customize kernel arguments (and overall system
> behavior).  The command line can be edited as needed when a zImage is
> copied to a TFTP staging area for download by firmware.

Why do this?  Why not get rid of storing the cmdline in the zImage altogether?  

We already have equivalent functionality by storing it in the dt's
/chosen/bootargs so why this unnecessary complexity?

Add some code to edit the /chosen/bootargs at zImage runtime (just like
arch/ppc used to) and we're covered.  AFAICT, we're already adding a tool
to tack on flat dt's to an already existing zImage so you're not doing
anything we can't--or won't--do already.

Agree?

Mark

Re: [PATCH] Provide mechanism for editing builtin command-line in zImage binary.

[Michal Ostrowski]

On Tue, 2006-05-30 at 13:41 -0700, Mark A. Greer wrote:
> On Mon, May 29, 2006 at 10:01:03PM -0400, mostrows@watson.ibm.com wrote:
> > zImage will store the builtin command-line in a dedicated section, allowing
> > it to be easily identified and edited with user-space tools.
> >
> > zImage will set /chosen/bootargs to the stored builtin command-line setting,
> > if /chosen/bootargs is empty (emulating the behavior in prom_init.c).
> > 
> > Use of this mechanism avoids the need to modify firmware or rely on a
> > bootloader to customize kernel arguments (and overall system
> > behavior).  The command line can be edited as needed when a zImage is
> > copied to a TFTP staging area for download by firmware.




> Why do this?  Why not get rid of storing the cmdline in the zImage altogether?  
> 

The CONFIG_CMDLINE that we have now is very useful if I want the
behavior of the system to be dependent on the kernel/command line I
specify that it used (particularly in BOOTP/TFTP scenarios).  For such
systems I configure their firmware to not provide any arguments and to
always download via TFTP a designated kernel image.

What this scenario does, is it allows me to specify system behavior by
putting the right kernel with the right command line in the magic
location from where it will be downloaded.

The problem is, that if I have some large number of machines and I want
to use the same kernel for all of them (and simply change the command
line between them) then I have to effectively build a custom kernel for
each.  This patch allows me to build one kernel and then simply edit the
command line embedded within it.

The key point is that the command line changes and I don't want to have
to require a firmware interaction every time I change it.



> We already have equivalent functionality by storing it in the dt's
> /chosen/bootargs so why this unnecessary complexity?
> 


> Add some code to edit the /chosen/bootargs at zImage runtime (just like
> arch/ppc used to) and we're covered.

That is what this patch is doing.

>   AFAICT, we're already adding a tool
> to tack on flat dt's to an already existing zImage so you're not doing
> anything we can't--or won't--do already.
> 

Can you please point me at this code so that I can evaluate it in
detail?

I'm curious how the tacked-on dt is expected to interact with the real
FW dt, and in particular how you would expect the interrogation
of /chosen/bootargs in prom_init.c (using prom_getprop()) to pick up the
command line value I specify in the tacked-on dt.


-- 
Michal Ostrowski <mostrows@watson.ibm.com>

Re: [PATCH] Provide mechanism for editing builtin command-line in zImage binary.

[Mark A. Greer]

On Tue, May 30, 2006 at 05:12:37PM -0400, Michal Ostrowski wrote:
> On Tue, 2006-05-30 at 13:41 -0700, Mark A. Greer wrote:
> > On Mon, May 29, 2006 at 10:01:03PM -0400, mostrows@watson.ibm.com wrote:
> > > zImage will store the builtin command-line in a dedicated section, allowing
> > > it to be easily identified and edited with user-space tools.
> > >
> > > zImage will set /chosen/bootargs to the stored builtin command-line setting,
> > > if /chosen/bootargs is empty (emulating the behavior in prom_init.c).
> > > 
> > > Use of this mechanism avoids the need to modify firmware or rely on a
> > > bootloader to customize kernel arguments (and overall system
> > > behavior).  The command line can be edited as needed when a zImage is
> > > copied to a TFTP staging area for download by firmware.
> 
> 
> 
> 
> > Why do this?  Why not get rid of storing the cmdline in the zImage altogether?  
> > 
> 
> The CONFIG_CMDLINE that we have now is very useful if I want the
> behavior of the system to be dependent on the kernel/command line I
> specify that it used (particularly in BOOTP/TFTP scenarios).  For such
> systems I configure their firmware to not provide any arguments and to
> always download via TFTP a designated kernel image.
> 
> What this scenario does, is it allows me to specify system behavior by
> putting the right kernel with the right command line in the magic
> location from where it will be downloaded.
> 
> The problem is, that if I have some large number of machines and I want
> to use the same kernel for all of them (and simply change the command
> line between them) then I have to effectively build a custom kernel for
> each.  This patch allows me to build one kernel and then simply edit the
> command line embedded within it.
> 
> The key point is that the command line changes and I don't want to have
> to require a firmware interaction every time I change it.

Okay.  I hadn't thought of that scenario.  You're happy with the dt that
the fw gives you except that you want to change the bootargs.

I guess we can keep CONFIG_CMDLINE around then.

> > We already have equivalent functionality by storing it in the dt's
> > /chosen/bootargs so why this unnecessary complexity?
> > 
> 
> 
> > Add some code to edit the /chosen/bootargs at zImage runtime (just like
> > arch/ppc used to) and we're covered.
> 
> That is what this patch is doing.
>
> >   AFAICT, we're already adding a tool
> > to tack on flat dt's to an already existing zImage so you're not doing
> > anything we can't--or won't--do already.
> > 
> 
> Can you please point me at this code so that I can evaluate it in
> detail?

It doesn't exist yet and no one has jumped up to make that tool that I
have seen.  I've been messing with bootwrapper code and part of that
adds cmdline editing from a running bootwrapper.  We still need someone
to write this tool (assuming that's the way we're going):

http://ozlabs.org/pipermail/linuxppc-dev/2006-April/022435.html

Care to volunteer?  ;)

> I'm curious how the tacked-on dt is expected to interact with the real
> FW dt,

That's a good question.  I was thinking that a tacked on dt would be a
complete replacement for whatever dt came from the fw (but extracting
some info like /memory props).  That probably works okay for non-OF
systems but may not work for OF systems b/c there is so much more info
in the OF dt.  Someone who knows about OF will have to speak up here.

> and in particular how you would expect the interrogation
> of /chosen/bootargs in prom_init.c (using prom_getprop()) to pick up the
> command line value I specify in the tacked-on dt.

If a flattened dt is used instead of an OF dt the prom_init code isn't used.

Mark

Re: [PATCH] Provide mechanism for editing builtin command-line in zImage binary.

[Michal Ostrowski]

On Wed, 2006-05-31 at 13:04 -0700, Mark A. Greer wrote:
> On Tue, May 30, 2006 at 05:12:37PM -0400, Michal Ostrowski wrote:
> > On Tue, 2006-05-30 at 13:41 -0700, Mark A. Greer wrote:
> > > On Mon, May 29, 2006 at 10:01:03PM -0400, mostrows@watson.ibm.com wrote:


> > I'm curious how the tacked-on dt is expected to interact with the real
> > FW dt,
> 
> That's a good question.  I was thinking that a tacked on dt would be a
> complete replacement for whatever dt came from the fw (but extracting
> some info like /memory props).  That probably works okay for non-OF
> systems but may not work for OF systems b/c there is so much more info
> in the OF dt.  Someone who knows about OF will have to speak up here.
> 

I've had some experience with trying to edit existing OF trees (i.e.
take a G5 OF tree and alter it to reflect the fact that the OS has a
hypervisor between it and the HW).  It's not a pleasant experience.

Thus for OF based systems I'd be very wary of trying to edit the OF tree
in arbitrary ways prior to Linux seeing it.

> > and in particular how you would expect the interrogation
> > of /chosen/bootargs in prom_init.c (using prom_getprop()) to pick up the
> > command line value I specify in the tacked-on dt.
> 
> If a flattened dt is used instead of an OF dt the prom_init code isn't used.

If we always skip prom_init.c code then we may skip stuff like TCE
allocation which is only ever done on the first boot (and never on
kexec).  


-- 
Michal Ostrowski <mostrows@watson.ibm.com>

Re: [PATCH] Provide mechanism for editing builtin command-line in zImage binary.

[Matthew McClintock]

On Wed, 2006-05-31 at 16:26 -0400, Michal Ostrowski wrote:
> I've had some experience with trying to edit existing OF trees (i.e.
> take a G5 OF tree and alter it to reflect the fact that the OS has a
> hypervisor between it and the HW).  It's not a pleasant experience.
> 
> Thus for OF based systems I'd be very wary of trying to edit the OF
> tree
> in arbitrary ways prior to Linux seeing it. 

Out of curiosity what was hard about it? Also it is worth mentioning
some systems don't have to privilege of having a OF tree ready to go by
the time Linux starts.

-Matthew

Re: [PATCH] Provide mechanism for editing builtin command-line in zImage binary.

[Michal Ostrowski]

On Wed, 2006-05-31 at 15:35 -0500, Matthew McClintock wrote:
> On Wed, 2006-05-31 at 16:26 -0400, Michal Ostrowski wrote:
> > I've had some experience with trying to edit existing OF trees (i.e.
> > take a G5 OF tree and alter it to reflect the fact that the OS has a
> > hypervisor between it and the HW).  It's not a pleasant experience.
> > 
> > Thus for OF based systems I'd be very wary of trying to edit the OF
> > tree
> > in arbitrary ways prior to Linux seeing it. 
> 
> Out of curiosity what was hard about it? 

Well, suppose that you want to remove a particular device from an OF
tree.  At what point are you certain that you've completely removed all
references to it?

I've always been concerned that there are some properties remaining in
the tree that may refer to the node I am removing (resulting in an
inconsistent tree).  If you're working with one particular FW provider
then you may come up with code that does it right, but such code may not
necessarily catch all the extensions provided by another FW provider.  

I've found Apple and IBM FW like to do things in different ways.  In
particular IBM FW likes to add "ibm,*" properties and you'd have to know
the meaning of all such properties to ensure you've caught all
references to the device you're pruning.

Like with most things, getting a solution to solve your immediate
problem is easy; a perfect, general solution is much, much more
difficult.


(Granted, some things, such as adding a new "memory" node are pretty
easy to do.)

> Also it is worth mentioning
> some systems don't have to privilege of having a OF tree ready to go by
> the time Linux starts.

For such systems I think the right approach is to provide a
DTC-generated OF tree (provided that one ensures that we don't skip
important parts of prom_init.c).



-- 
Michal Ostrowski <mostrows@watson.ibm.com>

Re: [PATCH] Provide mechanism for editing builtin command-line in zImage binary.

[Paul Mackerras]

Michal Ostrowski writes:

> -BOOTCFLAGS	+= -I$(obj) -I$(srctree)/$(obj)
> +BOOTCFLAGS	+= -I$(obj) -I$(srctree)/$(obj) -I$(O)/include -I$(O)/include2\
> +			-D__KERNEL__

I don't like this; we are trying to keep the boot wrapper separate,
not make it more dependent on the rest of the kernel.

> +#include <linux/autoconf.h>
> +#include <asm/setup.h>

We don't really need COMMAND_LINE_SIZE here - nothing says the command
line from the firmware or the boot wrapper *has* to be the same size
as the kernel's command-line buffer.

> +static char builtin_cmdline[COMMAND_LINE_SIZE]
> +		__attribute__((section("__builtin_cmdline"))) = CONFIG_CMDLINE;
> +#endif

If CONFIG_CMDLINE is set, then the kernel image *already* has the
default in it, and we don't need another copy here.  Just make your
buffer here some fixed, known size and uninitialized (i.e. all
zeroes).  Do the setprop if the /chosen/bootargs is empty and your
buffer has been edited to be non-null.

Paul.

[PATCH] Editable kernel command-line in zImage binary.

[mostrows]

zImage will set /chosen/bootargs (if it is otherwise empty) with the
contents of a buffer in the section "__builtin_cmdline".  This permits
tools to edit zImage binaries to set the command-line eventually
processed by vmlinux.

--
Signed-off-by: Michal Ostrowski <mostrows@watson.ibm.com>

---

 arch/powerpc/boot/main.c |   27 +++++++++++++++++++++++++++
 arch/powerpc/boot/prom.h |    7 +++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

40bea6ace95796a1575437d6dfdbef671012ffc1
diff --git a/arch/powerpc/boot/main.c b/arch/powerpc/boot/main.c
index 816446f..c65a7ba 100644
--- a/arch/powerpc/boot/main.c
+++ b/arch/powerpc/boot/main.c
@@ -33,6 +33,14 @@ extern char _vmlinux_end[];
 extern char _initrd_start[];
 extern char _initrd_end[];
 
+/* A buffer that may be edited by tools operating on a zImage binary so as to
+ * 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[512]
+	__attribute__((section("__builtin_cmdline")));
+
+
 struct addr_range {
 	unsigned long addr;
 	unsigned long size;
@@ -204,6 +212,23 @@ static int is_elf32(void *hdr)
 	return 1;
 }
 
+void export_cmdline(void* chosen_handle)
+{
+        int len;
+        char cmdline[2] = { 0, 0 };
+
+	if (builtin_cmdline[0] == 0) 
+		return;
+
+        len = getprop(chosen_handle, "bootargs", cmdline, sizeof(cmdline));
+        if (len > 0 && cmdline[0] != 0) 
+		return;
+
+	setprop(chosen_handle, "bootargs", builtin_cmdline,
+		strlen(builtin_cmdline) + 1);
+}
+
+
 void start(unsigned long a1, unsigned long a2, void *promptr, void *sp)
 {
 	int len;
@@ -289,6 +314,8 @@ void start(unsigned long a1, unsigned lo
 		memmove((void *)vmlinux.addr,(void *)vmlinuz.addr,vmlinuz.size);
 	}
 
+	export_cmdline(chosen_handle);
+
 	/* Skip over the ELF header */
 #ifdef DEBUG
 	printf("... skipping 0x%lx bytes of ELF header\n\r",
diff --git a/arch/powerpc/boot/prom.h b/arch/powerpc/boot/prom.h
index 3e2ddd4..a57b184 100644
--- a/arch/powerpc/boot/prom.h
+++ b/arch/powerpc/boot/prom.h
@@ -31,4 +31,11 @@ static inline int getprop(void *phandle,
 	return call_prom("getprop", 4, 1, phandle, name, buf, buflen);
 }
 
+
+static inline int setprop(void *phandle, const char *name,
+			  void *buf, int buflen)
+{
+	return call_prom("setprop", 4, 1, phandle, name, buf, buflen);
+}
+
 #endif				/* _PPC_BOOT_PROM_H_ */
-- 
1.1.4.g0b63-dirty