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

From: David Gibson <[email-masked]>
Date: Mon, 18 Jun 2007 13:18:22 +1000

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 <[email-masked]>
> --- 
> 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 <[email-masked]>
> + *
> + */
> +
> +#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
> [email-masked]
> 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