[PATCH] boot: find initrd location from device-tree

[PATCH] boot: find initrd location from device-tree

[Milton Miller]

Some platforms have a boot agent that can create or modifiy
properties in the device-tree and load images into memory.
Provide a helper to set the loader-info used by prep_initrd().

The code supports the 8-byte properties required by kernels
before 2.6.22 and 4-byte properties generated by prep_initrd()
and current 32 bit kernels (new kernels will read either size).

The types.h header now includes libgcc limits.h for UINT_MAX.

Signed-off-by: Milton Miller <miltonm@bga.com>
--- 
The file name dtscan reflects the possibility that simiar functions
like the rmo_top might be added to the file.

This is the first patch in a series of 18 that udpates and rediffs
of my kexec zImage support against 2.6.22-rc4.  Unfornately I left
the series file behind but I wanted to get this first patch sent
for Matt to use.

Index: kernel/arch/powerpc/boot/dtscan.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ kernel/arch/powerpc/boot/dtscan.c	2007-06-15 03:45:16.000000000 -0500
@@ -0,0 +1,86 @@
+/*
+ * 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, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ * Copyright (C) 2007 IBM Corporation.
+ *
+ * Authors: Milton Miller <miltonm@bga.com>
+ *
+ */
+
+#include "ops.h"
+#include "stdio.h"
+
+/**
+ * dt_find_initrd - set loader initrd location based on existing properties
+ *
+ * finds the linux,initrd-start and linux,initrd-end properties in
+ * the /chosen node and sets the loader initrd fields accordingly.
+ *
+ * Use this if your loader sets the properties to allow other code to
+ * relocate the tree and/or cause r3 and r4 to be set on true OF
+ * platforms.
+ */
+
+void dt_find_initrd(void)
+{
+	int rc;
+	unsigned long long initrd_start, initrd_end;
+	void *devp;
+	static const char start_prop[] = "linux,initrd-start";
+	static const char end_prop[] = "linux,initrd-end";
+
+	devp = finddevice("/chosen");
+	if (! devp) {
+		return;
+	}
+
+	/* The properties had to be 8 bytes until 2.6.22  */
+	rc = getprop(devp, start_prop, &initrd_start, sizeof(initrd_start));
+	if (rc < 0)
+		return;
+	if (rc == sizeof(unsigned long)) {
+		unsigned long tmp;
+		memcpy(&tmp, &initrd_start, rc);
+		initrd_start = tmp;
+	} else if (rc != sizeof(initrd_start)) {
+		printf("unexpected length of %s in /chosen!\n\r", start_prop);
+		return;
+	}
+
+	rc = getprop(devp, end_prop, &initrd_end, sizeof(initrd_end));
+	if (rc < 0) {
+		printf("chosen has %s but no %s!\n\r", start_prop, end_prop);
+		return;
+	}
+	if (rc == sizeof(unsigned long)) {
+		unsigned long tmp;
+		memcpy(&tmp, &initrd_end, rc);
+		initrd_end = tmp;
+	} else if (rc != sizeof(initrd_end)) {
+		printf("unexpected length of %s in /chosen!\n\r", end_prop);
+		return;
+	}
+
+	if (!initrd_start)
+		return;
+
+	/* if the initrd is above 4G, its untouchable in 32 bit mode */
+	if (initrd_end <= UINT_MAX && initrd_start < initrd_end) {
+		loader_info.initrd_addr = initrd_start;
+		loader_info.initrd_size  = initrd_end - initrd_start;
+	} else {
+		printf("ignoring loader supplied initrd parameters\n");
+	}
+}
Index: kernel/arch/powerpc/boot/ops.h
===================================================================
--- kernel.orig/arch/powerpc/boot/ops.h	2007-06-15 03:43:36.000000000 -0500
+++ kernel/arch/powerpc/boot/ops.h	2007-06-15 03:44:34.000000000 -0500
@@ -151,6 +151,7 @@ static inline void *find_node_by_devtype
 	return find_node_by_prop_value_str(prev, "device_type", type);
 }
 
+void dt_find_initrd(void);
 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);
Index: kernel/arch/powerpc/boot/Makefile
===================================================================
--- kernel.orig/arch/powerpc/boot/Makefile	2007-06-15 03:43:36.000000000 -0500
+++ kernel/arch/powerpc/boot/Makefile	2007-06-15 03:44:34.000000000 -0500
@@ -43,7 +43,7 @@ $(addprefix $(obj)/,$(zlib) gunzip_util.
 
 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 elf_util.c $(zlib) devtree.c \
+		gunzip_util.c elf_util.c $(zlib) devtree.c dtscan.c \
 		44x.c ebony.c mv64x60.c mpsc.c mv64x60_i2c.c
 src-plat := of.c cuboot-83xx.c cuboot-85xx.c holly.c \
 		cuboot-ebony.c treeboot-ebony.c prpmc2800.c
Index: kernel/arch/powerpc/boot/types.h
===================================================================
--- kernel.orig/arch/powerpc/boot/types.h	2007-06-15 03:36:21.000000000 -0500
+++ kernel/arch/powerpc/boot/types.h	2007-06-15 03:44:34.000000000 -0500
@@ -1,6 +1,9 @@
 #ifndef _TYPES_H_
 #define _TYPES_H_
 
+#define _LIBC_LIMITS_H_		/* don't recurse to system's headers */
+#include <limits.h>		/* MAX_UINT, etc */
+
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
 typedef unsigned char		u8;

[PATCH] boot: find initrd location from device-tree

[Milton Miller]

Some platforms have a boot agent that can create or modifiy
properties in the device-tree and load images into memory.
Provide a helper to set the loader-info used by prep_initrd().

The code supports the 8-byte properties required by kernels
before 2.6.22 and 4-byte properties generated by prep_initrd()
and current 32 bit kernels (new kernels will read either size).

The types.h header now includes libgcc limits.h for UINT_MAX.

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

Please reply to this message with the correct email account.


The file name dtscan reflects the possibility that simiar functions
like the rmo_top might be added to the file.

This is the first patch in a series of 18 that udpates and rediffs
of my kexec zImage support against 2.6.22-rc4.  Unfornately I left
the series file behind but I wanted to get this first patch sent
for Matt to use.

Index: kernel/arch/powerpc/boot/dtscan.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ kernel/arch/powerpc/boot/dtscan.c	2007-06-15 03:45:16.000000000 -0500
@@ -0,0 +1,86 @@
+/*
+ * 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, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ * Copyright (C) 2007 IBM Corporation.
+ *
+ * Authors: Milton Miller <miltonm@bga.com>
+ *
+ */
+
+#include "ops.h"
+#include "stdio.h"
+
+/**
+ * dt_find_initrd - set loader initrd location based on existing properties
+ *
+ * finds the linux,initrd-start and linux,initrd-end properties in
+ * the /chosen node and sets the loader initrd fields accordingly.
+ *
+ * Use this if your loader sets the properties to allow other code to
+ * relocate the tree and/or cause r3 and r4 to be set on true OF
+ * platforms.
+ */
+
+void dt_find_initrd(void)
+{
+	int rc;
+	unsigned long long initrd_start, initrd_end;
+	void *devp;
+	static const char start_prop[] = "linux,initrd-start";
+	static const char end_prop[] = "linux,initrd-end";
+
+	devp = finddevice("/chosen");
+	if (! devp) {
+		return;
+	}
+
+	/* The properties had to be 8 bytes until 2.6.22  */
+	rc = getprop(devp, start_prop, &initrd_start, sizeof(initrd_start));
+	if (rc < 0)
+		return;
+	if (rc == sizeof(unsigned long)) {
+		unsigned long tmp;
+		memcpy(&tmp, &initrd_start, rc);
+		initrd_start = tmp;
+	} else if (rc != sizeof(initrd_start)) {
+		printf("unexpected length of %s in /chosen!\n\r", start_prop);
+		return;
+	}
+
+	rc = getprop(devp, end_prop, &initrd_end, sizeof(initrd_end));
+	if (rc < 0) {
+		printf("chosen has %s but no %s!\n\r", start_prop, end_prop);
+		return;
+	}
+	if (rc == sizeof(unsigned long)) {
+		unsigned long tmp;
+		memcpy(&tmp, &initrd_end, rc);
+		initrd_end = tmp;
+	} else if (rc != sizeof(initrd_end)) {
+		printf("unexpected length of %s in /chosen!\n\r", end_prop);
+		return;
+	}
+
+	if (!initrd_start)
+		return;
+
+	/* if the initrd is above 4G, its untouchable in 32 bit mode */
+	if (initrd_end <= UINT_MAX && initrd_start < initrd_end) {
+		loader_info.initrd_addr = initrd_start;
+		loader_info.initrd_size  = initrd_end - initrd_start;
+	} else {
+		printf("ignoring loader supplied initrd parameters\n");
+	}
+}
Index: kernel/arch/powerpc/boot/ops.h
===================================================================
--- kernel.orig/arch/powerpc/boot/ops.h	2007-06-15 03:43:36.000000000 -0500
+++ kernel/arch/powerpc/boot/ops.h	2007-06-15 03:44:34.000000000 -0500
@@ -151,6 +151,7 @@ static inline void *find_node_by_devtype
 	return find_node_by_prop_value_str(prev, "device_type", type);
 }
 
+void dt_find_initrd(void);
 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);
Index: kernel/arch/powerpc/boot/Makefile
===================================================================
--- kernel.orig/arch/powerpc/boot/Makefile	2007-06-15 03:43:36.000000000 -0500
+++ kernel/arch/powerpc/boot/Makefile	2007-06-15 03:44:34.000000000 -0500
@@ -43,7 +43,7 @@ $(addprefix $(obj)/,$(zlib) gunzip_util.
 
 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 elf_util.c $(zlib) devtree.c \
+		gunzip_util.c elf_util.c $(zlib) devtree.c dtscan.c \
 		44x.c ebony.c mv64x60.c mpsc.c mv64x60_i2c.c
 src-plat := of.c cuboot-83xx.c cuboot-85xx.c holly.c \
 		cuboot-ebony.c treeboot-ebony.c prpmc2800.c
Index: kernel/arch/powerpc/boot/types.h
===================================================================
--- kernel.orig/arch/powerpc/boot/types.h	2007-06-15 03:36:21.000000000 -0500
+++ kernel/arch/powerpc/boot/types.h	2007-06-15 03:44:34.000000000 -0500
@@ -1,6 +1,9 @@
 #ifndef _TYPES_H_
 #define _TYPES_H_
 
+#define _LIBC_LIMITS_H_		/* don't recurse to system's headers */
+#include <limits.h>		/* MAX_UINT, etc */
+
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
 typedef unsigned char		u8;

Re: [PATCH] boot: find initrd location from device-tree

[Scott Wood]

Milton Miller wrote:
> The file name dtscan reflects the possibility that simiar functions
> like the rmo_top might be added to the file.

Aren't these kind of things what devtree.c is for?

-Scott

Re: [PATCH] boot: find initrd location from device-tree

[David Gibson]

On Fri, Jun 15, 2007 at 12:34:30PM -0500, Milton Miller wrote:
> Some platforms have a boot agent that can create or modifiy
> properties in the device-tree and load images into memory.
> Provide a helper to set the loader-info used by prep_initrd().
> 
> The code supports the 8-byte properties required by kernels
> before 2.6.22 and 4-byte properties generated by prep_initrd()
> and current 32 bit kernels (new kernels will read either size).
> 
> The types.h header now includes libgcc limits.h for UINT_MAX.
> 
> Signed-off-by: Milton Miller <miltonm@bga.com>
> --- 
> The file name dtscan reflects the possibility that simiar functions
> like the rmo_top might be added to the file.

Hrm, not sure if it's worth creating a new file for this, it could
probably go in devtree.c.  

> This is the first patch in a series of 18 that udpates and rediffs
> of my kexec zImage support against 2.6.22-rc4.  Unfornately I left
> the series file behind but I wanted to get this first patch sent
> for Matt to use.
> 
> Index: kernel/arch/powerpc/boot/dtscan.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ kernel/arch/powerpc/boot/dtscan.c	2007-06-15 03:45:16.000000000 -0500
> @@ -0,0 +1,86 @@
> +/*
> + * 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, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
> + *
> + * Copyright (C) 2007 IBM Corporation.
> + *
> + * Authors: Milton Miller <miltonm@bga.com>
> + *
> + */
> +
> +#include "ops.h"
> +#include "stdio.h"
> +
> +/**
> + * dt_find_initrd - set loader initrd location based on existing properties
> + *
> + * finds the linux,initrd-start and linux,initrd-end properties in
> + * the /chosen node and sets the loader initrd fields accordingly.
> + *
> + * Use this if your loader sets the properties to allow other code to
> + * relocate the tree and/or cause r3 and r4 to be set on true OF
> + * platforms.
> + */
> +
> +void dt_find_initrd(void)
> +{
> +	int rc;
> +	unsigned long long initrd_start, initrd_end;
> +	void *devp;
> +	static const char start_prop[] = "linux,initrd-start";
> +	static const char end_prop[] = "linux,initrd-end";

I see no point to using these constants, just use literals in the
getprop() calls.

> +	devp = finddevice("/chosen");
> +	if (! devp) {
> +		return;
> +	}
> +
> +	/* The properties had to be 8 bytes until 2.6.22  */
> +	rc = getprop(devp, start_prop, &initrd_start, sizeof(initrd_start));
> +	if (rc < 0)
> +		return;
> +	if (rc == sizeof(unsigned long)) {
> +		unsigned long tmp;
> +		memcpy(&tmp, &initrd_start, rc);
> +		initrd_start = tmp;
> +	} else if (rc != sizeof(initrd_start)) {
> +		printf("unexpected length of %s in /chosen!\n\r", start_prop);
> +		return;
> +	}
> +
> +	rc = getprop(devp, end_prop, &initrd_end, sizeof(initrd_end));
> +	if (rc < 0) {
> +		printf("chosen has %s but no %s!\n\r", start_prop, end_prop);
> +		return;
> +	}
> +	if (rc == sizeof(unsigned long)) {
> +		unsigned long tmp;
> +		memcpy(&tmp, &initrd_end, rc);
> +		initrd_end = tmp;
> +	} else if (rc != sizeof(initrd_end)) {
> +		printf("unexpected length of %s in /chosen!\n\r", end_prop);
> +		return;
> +	}
> +
> +	if (!initrd_start)
> +		return;
> +
> +	/* if the initrd is above 4G, its untouchable in 32 bit mode */
> +	if (initrd_end <= UINT_MAX && initrd_start < initrd_end) {
> +		loader_info.initrd_addr = initrd_start;
> +		loader_info.initrd_size  = initrd_end - initrd_start;
> +	} else {
> +		printf("ignoring loader supplied initrd parameters\n");

Saying why might be helpful.

> +	}
> +}
> Index: kernel/arch/powerpc/boot/ops.h
> ===================================================================
> --- kernel.orig/arch/powerpc/boot/ops.h	2007-06-15 03:43:36.000000000 -0500
> +++ kernel/arch/powerpc/boot/ops.h	2007-06-15 03:44:34.000000000 -0500
> @@ -151,6 +151,7 @@ static inline void *find_node_by_devtype
>  	return find_node_by_prop_value_str(prev, "device_type", type);
>  }
>  
> +void dt_find_initrd(void);
>  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);
> Index: kernel/arch/powerpc/boot/Makefile
> ===================================================================
> --- kernel.orig/arch/powerpc/boot/Makefile	2007-06-15 03:43:36.000000000 -0500
> +++ kernel/arch/powerpc/boot/Makefile	2007-06-15 03:44:34.000000000 -0500
> @@ -43,7 +43,7 @@ $(addprefix $(obj)/,$(zlib) gunzip_util.
>  
>  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 elf_util.c $(zlib) devtree.c \
> +		gunzip_util.c elf_util.c $(zlib) devtree.c dtscan.c \
>  		44x.c ebony.c mv64x60.c mpsc.c mv64x60_i2c.c
>  src-plat := of.c cuboot-83xx.c cuboot-85xx.c holly.c \
>  		cuboot-ebony.c treeboot-ebony.c prpmc2800.c
> Index: kernel/arch/powerpc/boot/types.h
> ===================================================================
> --- kernel.orig/arch/powerpc/boot/types.h	2007-06-15 03:36:21.000000000 -0500
> +++ kernel/arch/powerpc/boot/types.h	2007-06-15 03:44:34.000000000 -0500
> @@ -1,6 +1,9 @@
>  #ifndef _TYPES_H_
>  #define _TYPES_H_
>  
> +#define _LIBC_LIMITS_H_		/* don't recurse to system's headers */
> +#include <limits.h>		/* MAX_UINT, etc */
> +

I think it's really a bad idea to use any headers from outside the
boot context here.  We're dealing with explicit sized ints, so we can
safely define our own constants giving the limit values.

>  #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>  
>  typedef unsigned char		u8;
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
> 

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

Re: [PATCH] boot: find initrd location from device-tree

[Milton Miller]

[replying to David and Scott the second sub-thread]

On Jun 15, 2007, at 2:55 PM, Scott Wood wrote:
> Milton Miller wrote:
>> The file name dtscan reflects the possibility that simiar functions
>> like the rmo_top might be added to the file.
> Aren't these kind of things what devtree.c is for?

Hmm... i guess .. I didn't need the fixups in that file, but I guess 
the xlate routines couuld be useful for find top of memory.

That file is for "not specific to device tree library (firmware 
callback vs flat tree)" utilities?

On Mon Jun 18 13:18:22 EST 2007, David Gibson wrote:
> On Fri, Jun 15, 2007 at 12:34:30PM -0500, Milton Miller wrote:
> >
> > The types.h header now includes libgcc limits.h for UINT_MAX.
> >


> > ---
> > The file name dtscan reflects the possibility that simiar functions
> > like the rmo_top might be added to the file.
>
> Hrm, not sure if it's worth creating a new file for this, it could
> probably go in devtree.c.

Ok, two votes.  I'll move it next post.

> > +void dt_find_initrd(void)
> > +{
> > +     int rc;
> > +     unsigned long long initrd_start, initrd_end;
> > +     void *devp;
> > +     static const char start_prop[] = "linux,initrd-start";
> > +     static const char end_prop[] = "linux,initrd-end";
>
> I see no point to using these constants, just use literals in the
> getprop() calls.

When I had them inline, the get-property and the printf calls exceeded 
80 characters, making lots of multi-line function calls.  And 
seperating them out out made the error printfs the same, so they could 
be collapsed by gcc.  Either isn't a real strong argument, so if you 
insist I will write it out, but I thought it was nicer this way.

>
> > +     devp = finddevice("/chosen");
> > +     if (! devp) {
> > +             return;
> > +     }
> > +
> > +     /* The properties had to be 8 bytes until 2.6.22  */
> > +     rc = getprop(devp, start_prop, &initrd_start, 
> sizeof(initrd_start));
> > +     if (rc < 0)
> > +             return;
> > +     if (rc == sizeof(unsigned long)) {
> > +             unsigned long tmp;
> > +             memcpy(&tmp, &initrd_start, rc);
> > +             initrd_start = tmp;
> > +     } else if (rc != sizeof(initrd_start)) {
> > +             printf("unexpected length of %s in /chosen!\n\r", 
> start_prop);
> > +             return;
> > +     }
> > +
> > +     rc = getprop(devp, end_prop, &initrd_end, sizeof(initrd_end));
> > +     if (rc < 0) {
> > +             printf("chosen has %s but no %s!\n\r", start_prop, 
> end_prop);
> > +             return;
> > +     }
> > +     if (rc == sizeof(unsigned long)) {
> > +             unsigned long tmp;
> > +             memcpy(&tmp, &initrd_end, rc);
> > +             initrd_end = tmp;
> > +     } else if (rc != sizeof(initrd_end)) {
> > +             printf("unexpected length of %s in /chosen!\n\r", 
> end_prop);
> > +             return;
> > +     }
> > +
> > +     if (!initrd_start)
> > +             return;
> > +
> > +     /* if the initrd is above 4G, its untouchable in 32 bit mode */
> > +     if (initrd_end <= UINT_MAX && initrd_start < initrd_end) {
> > +             loader_info.initrd_addr = initrd_start;
> > +             loader_info.initrd_size  = initrd_end - initrd_start;
> > +     } else {
> > +             printf("ignoring loader supplied initrd parameters\n");
>
> Saying why might be helpful.

Hmm... well, that would mean seperating the && of the if; there are two 
possible reasons. (1) this 32-bit code can't handle such large 
addresses (in which case a 64-bit kernel may work), and (2) end is >= 
start (which means there really isn't any data.  I suppose I could 
reverse the sense of the if, use an else if, and make the final else 
the good path.



>
> > +     }
> > +}
...
> > Index: kernel/arch/powerpc/boot/types.h
> > ===================================================================
> > --- kernel.orig/arch/powerpc/boot/types.h     2007-06-15 
> 03:36:21.000000000 -0500
> > +++ kernel/arch/powerpc/boot/types.h  2007-06-15 03:44:34.000000000 
> -0500
> > @@ -1,6 +1,9 @@
> >  #ifndef _TYPES_H_
> >  #define _TYPES_H_
> >
> > +#define _LIBC_LIMITS_H_              /* don't recurse to system's 
> headers */
> > +#include <limits.h>          /* MAX_UINT, etc */
> > +
>
> I think it's really a bad idea to use any headers from outside the
> boot context here.  We're dealing with explicit sized ints, so we can
> safely define our own constants giving the limit values.

As I said in the patch changelog, the only headers picked up here are 
libgcc.  The compiler is part of the boot context, and we already pick 
up stdarg from there.   The flag there appears to be what libgcc 
expects the library to use.

I could change the comment to say /* get libgcc header only */ or 
something else you suggest.

milton

Re: [PATCH] boot: find initrd location from device-tree

[Segher Boessenkool]

>>> +     /* if the initrd is above 4G, its untouchable in 32 bit mode */
>>> +     if (initrd_end <= UINT_MAX && initrd_start < initrd_end) {
>>> +             loader_info.initrd_addr = initrd_start;
>>> +             loader_info.initrd_size  = initrd_end - initrd_start;
>>> +     } else {
>>> +             printf("ignoring loader supplied initrd parameters\n");
>>
>> Saying why might be helpful.
>
> Hmm... well, that would mean seperating the && of the if; there are two
> possible reasons. (1) this 32-bit code can't handle such large
> addresses (in which case a 64-bit kernel may work), and (2) end is >=
> start (which means there really isn't any data.  I suppose I could
> reverse the sense of the if, use an else if, and make the final else
> the good path.

"ignoring loader supplied nonsensical initrd parameters"

You do need to fix the comment, though :-)

>>> +#define _LIBC_LIMITS_H_              /* don't recurse to system's
>> headers */
>>> +#include <limits.h>          /* MAX_UINT, etc */
>>> +
>>
>> I think it's really a bad idea to use any headers from outside the
>> boot context here.  We're dealing with explicit sized ints, so we can
>> safely define our own constants giving the limit values.
>
> As I said in the patch changelog, the only headers picked up here are
> libgcc.

Which the kernel doesn't use.  It _should_ be fine
nevertheless, <limits.h> is part of the "freestanding"
headers -- why do you need to do a "libc" trick though?
This doesn't work when not using glibc I guess?


Segher

Re: [PATCH] boot: find initrd location from device-tree

[David Gibson]

On Mon, Jun 18, 2007 at 02:52:12PM +0200, Segher Boessenkool wrote:
> >>> +     /* if the initrd is above 4G, its untouchable in 32 bit mode */
> >>> +     if (initrd_end <= UINT_MAX && initrd_start < initrd_end) {
> >>> +             loader_info.initrd_addr = initrd_start;
> >>> +             loader_info.initrd_size  = initrd_end - initrd_start;
> >>> +     } else {
> >>> +             printf("ignoring loader supplied initrd parameters\n");
> >>
> >> Saying why might be helpful.
> >
> > Hmm... well, that would mean seperating the && of the if; there are two
> > possible reasons. (1) this 32-bit code can't handle such large
> > addresses (in which case a 64-bit kernel may work), and (2) end is >=
> > start (which means there really isn't any data.  I suppose I could
> > reverse the sense of the if, use an else if, and make the final else
> > the good path.
> 
> "ignoring loader supplied nonsensical initrd parameters"
> 
> You do need to fix the comment, though :-)
> 
> >>> +#define _LIBC_LIMITS_H_              /* don't recurse to system's
> >> headers */
> >>> +#include <limits.h>          /* MAX_UINT, etc */
> >>> +
> >>
> >> I think it's really a bad idea to use any headers from outside the
> >> boot context here.  We're dealing with explicit sized ints, so we can
> >> safely define our own constants giving the limit values.
> >
> > As I said in the patch changelog, the only headers picked up here are
> > libgcc.
> 
> Which the kernel doesn't use.  It _should_ be fine
> nevertheless, <limits.h> is part of the "freestanding"
> headers -- why do you need to do a "libc" trick though?
> This doesn't work when not using glibc I guess?

IMO if it needs this define magic to work properly, it's not a good
idea to include it at all.  Especially since avoiding it is easy and
safe in this case.

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

Re: [PATCH] boot: find initrd location from device-tree

[Segher Boessenkool]

>>> As I said in the patch changelog, the only headers picked up here are
>>> libgcc.
>>
>> Which the kernel doesn't use.  It _should_ be fine
>> nevertheless, <limits.h> is part of the "freestanding"
>> headers -- why do you need to do a "libc" trick though?
>> This doesn't work when not using glibc I guess?
>
> IMO if it needs this define magic to work properly, it's not a good
> idea to include it at all.  Especially since avoiding it is easy and
> safe in this case.

100% agreed.


Segher