linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Milton Miller <miltonm@bga.com>
Cc: linuxppc-dev@ozlabs.org, Paul Mackerras <paulus@samba.org>,
	Rob Landley <rob@landley.net>
Subject: Re: [PATCH 8/15] bootwrapper: convert flatdevtree to version 16
Date: Mon, 24 Sep 2007 13:36:12 +1000	[thread overview]
Message-ID: <20070924033612.GI8058@localhost.localdomain> (raw)
In-Reply-To: <boot-8-08.miltonm@bga.com>

On Fri, Sep 21, 2007 at 06:05:06PM -0500, Milton Miller wrote:
> kexec-tools still produces a version 2 device tree, while the
> libraries in the wrapper only support version 16 and later.
> 
> Add a routine to convert a v2 flat device tree to a v16 one inplace
> by inserting OF_DT_NOP and chomping full path.  Make space for new
> headers by moving and then chomping the OF_DT_NOPs.
> 
> Signed-off-by: Milton Miller <miltonm@bga.com>
> ---
> vs 12175
> Rediffed Makefile, ops, kexec.c
> 
> Index: kernel/arch/powerpc/boot/flatdevtree_conv.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ kernel/arch/powerpc/boot/flatdevtree_conv.c	2007-09-20 17:49:04.000000000 -0500
> @@ -0,0 +1,280 @@
> +/*
> + * 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 IBM Corporation 2007
> + *
> + * Authors: Milton Miller <miltonm@bga.com>
> + */
> +#include "flatdevtree.h"
> +#include "stdio.h"
> +#include "ops.h"
> +
> +#define MIN_VERSION 2
> +#define OUT_VERSION 16

Should output version 17.  In any case, don't try to be so general -
just convert v123 (all basically the same) to latest (i.e. v17)
without all the #if nonsense.

> +#define OUT_COMPAT 16
> +
> +#ifdef NO_CHECK
> +static int check_v123_tree(u32 *start, u32 *limit)
> +{
> +	return 0;
> +}
> +#else
> +/**
> + * check_v123_tree - check integrety of a version 1, 2, or 3 tree
> + * @start: the start of the device tree struct
> + * @limit: the end of the region for the struct
> + * structural checks on device_tree
> + */
> +static int check_v123_tree(u32 *start, u32 *limit)

What is the point of this check?  If the device tree is corrupt, we're
stuffed anyway, so why bother?

> +{
> +	u32 len;
> +	int depth = 0;
> +	u32 *dtp = start;
> +
> +	while (dtp < limit)
> +		switch (*dtp) {
> +		case OF_DT_END:
> +			if (depth)
> +				return -1;
> +			return ++dtp - start;
> +		case OF_DT_NOP:
> +			dtp++;
> +			break;
> +		case OF_DT_END_NODE:
> +			dtp++;
> +			depth--;
> +			break;
> +		case OF_DT_BEGIN_NODE:
> +			len = strlen((char *)(++dtp));
> +			/* check path is suffix to previous? */
> +			dtp += 1 + (len / 4);
> +			depth++;
> +			break;
> +		case OF_DT_PROP:
> +			len = dtp[1];
> +			dtp += 3;
> +			if ((len >= 8) && ((long)dtp & 4))
> +				dtp++;
> +			dtp += (len + 3) / 4;
> +			break;
> +		default:
> +			return -1;
> +		}
> +	return -1;	/* no OF_DT_END */
> +}
> +#endif
> +
> +/**
> + * nop_to_v16 - add %OF_DT_NOP to hide alignment differences
> + * @dtp: pointer to the beginning of the struct area to modify
> + * insert %OF_DT_NOP into the dt_struct @dtp to make it v16 from v1, 2, or 3.
> + */
> +static int nop_to_v16(u32 *dtp)
> +{
> +	int nops = 0;
> +	char *p, *s;
> +	int len;
> +	u32 *next;
> +
> +	while (*dtp != OF_DT_END)
> +		switch (*dtp) {
> +		case OF_DT_BEGIN_NODE:
> +			/* v2 & v3 names are full path, v16+ is relative */
> +			p = (char *)(++dtp);
> +			len = strlen(p);
> +			next = dtp + 1 + len / 4;
> +
> +			for (s = p + len; *s != '/'; s--)
> +				if (s == p)
> +					fatal("name %s has no '/'", p);
> +
> +			len -= s++ - p;		/* not the slash but the nul */
> +			memmove(p, s, len);
> +			while (len % 4)
> +				p[len++] = '\0';
> +			dtp += len / 4;
> +			while (dtp != next) {
> +				*dtp++ = OF_DT_NOP;
> +				nops++;
> +			}
> +			break;
> +		case OF_DT_PROP:
> +			/* convert from align_8 to align_4 via prefixing nop */
> +			len = dtp[1];
> +			if ((len >= 8) && !((long)dtp & 4)) {
> +				memmove(dtp+1, dtp, 12);
> +				*dtp++ = OF_DT_NOP;
> +				nops++;
> +			}
> +			dtp += 3 + (len + 3)/4;
> +			break;
> +		default:
> +			fatal("%s: unrecognised tag %d at %p\n", __FUNCTION__,
> +				*dtp, dtp);
> +		case OF_DT_NOP:
> +			nops ++;
> +			/* fall through */
> +		case OF_DT_END_NODE:
> +			dtp ++;
> +			break;
> +		}
> +	return nops;
> +}
> +
> +#if MIN_VERSION < 3 || OUT_VERSION > 16
> +/**
> + * move_nops_fwd - move nops in a v16 dt_struct to the beginning
> + * @start - device tree starting address
> + * @count - number of %OF_DT_NOP cells to move
> + */
> +static void move_nops_fwd(u32 *start, int count)

What on earth is the point of this.  The NOPs are perfectly valid
scattered within the tree, why go to all this trouble to shuffle them
about.

> +{
> +	u32 *dtp = start;
> +	int len;
> +	while (count)
> +		switch (*dtp) {
> +		case OF_DT_NOP:
> +			memmove(start+1,start,(dtp-start) * 4);
> +			*start++ = OF_DT_NOP;
> +			dtp++;
> +			count--;
> +			break;
> +		case OF_DT_END_NODE:
> +			dtp++;
> +			break;
> +		case OF_DT_BEGIN_NODE:
> +			len = strlen((char *)(++dtp));
> +			dtp += 1 + len / 4;
> +			break;
> +		case OF_DT_PROP:
> +			len = dtp[1];
> +			dtp += 3 + (len + 3) / 4;
> +			break;
> +		case OF_DT_END:
> +			fatal("Not enough nops -- need %d more\n", count);
> +			return;
> +		default:
> +			fatal("%s: unknown tag %d at %p", __FUNCTION__, *dtp, dtp)
> +		}
> +}
> +#endif
> +
> +/**
> + * conv_flattree_inplace upgrade the version of a boot_param_header
> + * @tree: pointer to the device tree header to convert
> + *
> + * Converts a v1, 2, 3 device tree (of at least MIN_VERSION)
> + * in place to OUT_VERSION (16) format, usable by flatdevtree.c
> + */
> +void conv_flattree_inplace(struct boot_param_header *tree)
> +{
> +	u32 *dtp;
> +	u32 need = 0, nops;
> +	int slen;
> +
> +	if (tree->magic != OF_DT_HEADER)
> +		fatal("%s: no magic", __FUNCTION__);

More pointless tests...

> +	if (tree->last_comp_version > 3)
> +		return;			/* don't know what to do */

Rather, don't need to do anything.

> +
> +	if (tree->version < MIN_VERSION) {
> +		printf("%s: Warning: can't handle version %d tree\n",
> +				__FUNCTION__, tree->version);
> +		return;
> +	}
> +
> +	if (tree->version < 2)
> +		need++;		/* boot_cpu_id */
> +
> +	if (tree->version < 3)
> +		need++;		/* dt_string_size */
> +
> +	if (OUT_VERSION > 16)
> +		need++;		/* dt_struct_size */
> +
> +	dtp = (void *)tree + tree->off_dt_struct;
> +
> +	slen = check_v123_tree(dtp, (void *)tree + tree->totalsize);
> +	if (slen < 0)
> +		fatal("device tree check failed\n");
> +
> +	nops = nop_to_v16(dtp);
> +
> +	if (need & 1)		/* keep 8 byte alignment of mem reserve */
> +		need++;
> +
> +	if (need > nops)
> +		fatal("Didn't find enough space to add new header fields\n\r"
> +			"(needed %d found %d tree %p)", need, nops, tree);
> +
> +	/* ok now compress the dtb struct */
> +	move_nops_fwd(dtp, need);
> +	dtp += need;
> +
> +	/*
> +	 * move mem_rsvmap and dt_strings if they are before dt_struct
> +	 * onto our nops .  Adjust start addresses for the 3 sections.
> +	 */

Hrm.  Do we really need to worry about this case.  You may be
producing v2 trees in kexec-tools, but do they actually have the
blocks out of order?  dtc certainly never produced them that way.

> +	if ((tree->off_mem_rsvmap < tree->off_dt_struct) ||
> +		(tree->off_dt_strings < tree->off_dt_struct)) {
> +		int start, end;
> +		void *ptr;
> +
> +		if (tree->off_mem_rsvmap < tree->off_dt_strings)
> +			start = tree->off_mem_rsvmap;
> +		else
> +			start = tree->off_dt_strings;
> +
> +		end = tree->off_dt_struct;
> +		ptr = (void *)tree + start;
> +
> +		memmove(ptr + 4 * need, ptr, end - start);
> +
> +		if (tree->off_mem_rsvmap < tree->off_dt_struct)
> +			tree->off_mem_rsvmap += 4 * need;
> +		if (tree->off_dt_strings < tree->off_dt_struct)
> +			tree->off_dt_strings += 4 * need;
> +	}
> +	tree->off_dt_struct += 4 * need;
> +
> +	/* ok now we have space to extend the header. */
> +	if (tree->version < 2 && MIN_VERSION < 2) {
> +		tree->boot_cpuid_phys = 0;	/* default, caller can fix */
> +	}
> +
> +	/* calculate size of dt_strings_size */
> +	if (tree->version < 3 && MIN_VERSION < 3) {
> +		int end = tree->totalsize;
> +
> +		if (tree->off_dt_strings < tree->off_mem_rsvmap)
> +			end = tree->off_mem_rsvmap;
> +
> +		if ((tree->off_dt_strings < tree->off_dt_struct) &&
> +				(end > tree->off_dt_struct))
> +			end = tree->off_dt_struct;
> +
> +		tree->dt_strings_size = end - tree->off_dt_strings;
> +	}
> +
> +#if OUT_VERSION > 16
> +	tree->dt_struct_size = 4 * slen;
> +#endif
> +
> +	tree->version = OUT_VERSION;
> +	tree->last_comp_version = OUT_COMPAT;
> +
> +	return;
> +}
> Index: kernel/arch/powerpc/boot/Makefile
> ===================================================================
> --- kernel.orig/arch/powerpc/boot/Makefile	2007-09-20 17:42:24.000000000 -0500
> +++ kernel/arch/powerpc/boot/Makefile	2007-09-20 17:49:04.000000000 -0500
> @@ -42,7 +42,7 @@ $(addprefix $(obj)/,$(zlib) gunzip_util.
>  	$(addprefix $(obj)/,$(zliblinuxheader)) $(addprefix $(obj)/,$(zlibheader))
>  
>  src-wlib := string.S crt0.S stdio.c main.c flatdevtree.c flatdevtree_misc.c \
> -		marshal.c memranges.c kexec.c \
> +		flatdevtree_conv.c marshal.c memranges.c kexec.c \
>  		ns16550.c serial.c simple_alloc.c div64.S util.S \
>  		gunzip_util.c elf_util.c $(zlib) devtree.c oflib.c ofconsole.c \
>  		4xx.c ebony.c mv64x60.c mpsc.c mv64x60_i2c.c cuboot.c bamboo.c \
> Index: kernel/arch/powerpc/boot/ops.h
> ===================================================================
> --- kernel.orig/arch/powerpc/boot/ops.h	2007-09-20 17:42:24.000000000 -0500
> +++ kernel/arch/powerpc/boot/ops.h	2007-09-20 17:49:04.000000000 -0500
> @@ -81,7 +81,10 @@ struct loader_info {
>  };
>  extern struct loader_info loader_info;
>  
> +struct boot_param_header;
> +
>  void start(void);
> +void conv_flattree_inplace(struct boot_param_header *tree);
>  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);
> Index: kernel/arch/powerpc/boot/kexec.c
> ===================================================================
> --- kernel.orig/arch/powerpc/boot/kexec.c	2007-09-20 17:42:24.000000000 -0500
> +++ kernel/arch/powerpc/boot/kexec.c	2007-09-20 17:49:04.000000000 -0500
> @@ -99,6 +99,7 @@ void kexec_platform_init(struct boot_par
>  	move_slaves_up();
>  
>  	setup_initial_heap();
> +	conv_flattree_inplace(dt_blob);
>  	init_flat_tree(dt_blob);
>  	/*
>  	 * drivers can malloc and read the tree, but not realloc later
> _______________________________________________
> 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

  reply	other threads:[~2007-09-24  3:36 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-21 23:02 [PATCH 0/15] bootwrapper: kexec and external payloads Milton Miller
2007-09-21 23:03 ` [PATCH 1/15] boot: find initrd location from device-tree Milton Miller
2007-09-24  2:58   ` David Gibson
2007-09-24  5:50     ` Rob Landley
2007-09-24  8:02     ` Milton Miller
2007-09-25  3:27       ` David Gibson
2007-09-26  5:49         ` Milton Miller
2007-09-21 23:03 ` [PATCH 2/15] boot: record header bytes in gunzip_start Milton Miller
2007-09-24  2:59   ` David Gibson
2007-09-21 23:03 ` [PATCH 3/15] boot: simplfy gunzip_finish Milton Miller
2007-09-21 23:03 ` [PATCH 4/15] bootwrapper: smp support code Milton Miller
2007-09-21 23:04 ` [PATCH 5/15] bootwrapper: occuppied memory ranges Milton Miller
2007-09-24  3:09   ` David Gibson
2007-09-24  9:33     ` Milton Miller
2007-09-21 23:04 ` [PATCH 6/15] bootwrapper: help for 64 bit cpus Milton Miller
2007-09-24  3:14   ` David Gibson
2007-09-21 23:04 ` [PATCH 7/15] bootwrapper: Add kexec callable zImage wrapper Milton Miller
2007-09-24  3:23   ` David Gibson
2007-09-21 23:05 ` [PATCH 8/15] bootwrapper: convert flatdevtree to version 16 Milton Miller
2007-09-24  3:36   ` David Gibson [this message]
2007-09-24  6:54     ` Milton Miller
2007-09-25  3:46       ` David Gibson
2007-09-26 16:19         ` Milton Miller
2007-09-27  2:45           ` David Gibson
2007-09-27 15:44             ` Milton Miller
2007-09-28  2:40               ` David Gibson
2007-09-28 15:16                 ` Milton Miller
2007-10-03  5:29                   ` David Gibson
2007-09-21 23:05 ` [PATCH 9/15] bootwrapper: rtas support Milton Miller
2007-09-24  3:46   ` David Gibson
2007-09-21 23:05 ` [PATCH 10/15] bootwrapper: add cpio file extraction library Milton Miller
2007-09-21 23:06 ` [PATCH 11/15] bootwrapper: allow vmlinuz to be an external payload Milton Miller
2007-09-21 23:06 ` [PATCH 12/15] bootwrapper: kexec extract vmlinux from initramfs Milton Miller
2007-09-21 23:06 ` [PATCH 13/15] bootwrapper: attach an empty vmlinux Milton Miller
2007-09-24  4:03   ` David Gibson
2007-09-21 23:08 ` [PATCH 14/15] boot: add a hook to start cpus Milton Miller
2007-09-21 23:08 ` [PATCH 15/15] bootwrapper: recheck for command line after fixups Milton Miller
2007-09-21 23:08 ` [PATCH 1/2] qemu platform, v2 Milton Miller
2007-09-22  9:55   ` Christoph Hellwig
2007-09-22 19:16     ` Rob Landley
2007-09-23  4:27       ` Paul Mackerras
2007-09-23 22:01         ` Rob Landley
2007-09-28 16:53         ` Segher Boessenkool
2007-09-28 20:14           ` Rob Landley
2007-10-01  5:33           ` David Gibson
2007-10-17 20:28             ` Grant Likely
2007-10-17 23:09               ` Rob Landley
2007-10-18  9:59               ` Matt Sealey
2007-10-18 17:19                 ` Milton Miller
2007-10-18 17:29                   ` Grant Likely
2007-10-19  6:28                     ` Rob Landley
2007-09-24  4:00     ` David Gibson
2007-09-24  7:46       ` Christoph Hellwig
2007-09-24  9:48         ` Milton Miller
2007-09-21 23:08 ` [PATCH 2/2] qemu platform rom, v2 Milton Miller
  -- strict thread matches above, loose matches on Subject: below --
2007-07-10 22:07 [PATCH 0/15] bootwrapper: support for kexec to zImage Milton Miller
2007-07-10 22:10 ` [PATCH 8/15] bootwrapper: convert flatdevtree to version 16 Milton Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070924033612.GI8058@localhost.localdomain \
    --to=david@gibson.dropbear.id.au \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=miltonm@bga.com \
    --cc=paulus@samba.org \
    --cc=rob@landley.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).