Re: [PATCH v8 1/3] DTBO magic and dtbo format options

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 10786 bytes --]

On Wed, Jun 08, 2016 at 11:30:43AM +0300, Pantelis Antoniou wrote:
> 
> > On Jun 8, 2016, at 09:51 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > 
> > On Thu, Jun 02, 2016 at 08:47:20PM +0300, Pantelis Antoniou wrote:
> >> Introduce a new magic number for dynamic plugin objects,
> >> which is enabled by selecting dtbo/input output options.
> >> 
> >> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> > 
> > So, I don't think we should need an explicit -I dtbo or -O dtbo.
> > Instead we should be able to autoselect the right magic number based
> > on the presence of the /plugin/ flag.  In the other direction we
> > should be able to set the equivalent of the /plugin/ flag based on the
> > magic number.
> 
> Sure but this will break everything since nothing can grok the different
> magic number. Not the kernel unflattener nor any bootloader.

Yes, but I'd prefer to implement the preferred way of doing things
first - with the new magic number, then add the backward compatibility
shim afterwards.

> We need an option to keep the old magic number for compatibility.
> 
> >> ---
> >> Documentation/manual.txt |  8 ++++++++
> >> dtc.c                    | 14 +++++++++++---
> >> dtc.h                    |  4 ++--
> >> fdtdump.c                |  2 +-
> >> flattree.c               | 11 ++++++-----
> >> libfdt/fdt.c             |  2 +-
> >> libfdt/fdt.h             |  3 ++-
> >> tests/mangle-layout.c    |  7 ++++---
> >> 8 files changed, 35 insertions(+), 16 deletions(-)
> >> 
> >> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
> >> index 398de32..6d2811b 100644
> >> --- a/Documentation/manual.txt
> >> +++ b/Documentation/manual.txt
> >> @@ -60,6 +60,9 @@ The currently supported Input Formats are:
> >>     - "dtb": "blob" format.  A flattened device-tree block with
> >>         header in one binary blob.
> >> 
> >> +    - "dtbo" : "blob" format. Identical with "dtb" but meant
> >> +        for use with dynamic-device tree objects.
> >> +
> >>     - "dts": "source" format.  A text file containing a "source"
> >>         for a device-tree.
> >> 
> >> @@ -71,6 +74,8 @@ The currently supported Output Formats are:
> >> 
> >>      - "dtb": "blob" format
> >> 
> >> +     - "dtbo": "blob" format - for dynamic device tree objects/overlays
> >> +
> >>      - "dts": "source" format
> >> 
> >>      - "asm": assembly language file.  A file that can be sourced
> >> @@ -78,6 +83,9 @@ The currently supported Output Formats are:
> >>         then simply be added to your Makefile.  Additionally, the
> >>         assembly file exports some symbols that can be used.
> >> 
> >> +     - "asmo": assembly language file for dynamic device tree objects/overlays.
> >> +        Identical to "asm" for most purposes.
> >> +
> >> 
> >> 3) Command Line
> >> 
> >> diff --git a/dtc.c b/dtc.c
> >> index 1c1f88f..43ba19d 100644
> >> --- a/dtc.c
> >> +++ b/dtc.c
> >> @@ -127,6 +127,8 @@ static const char *guess_type_by_name(const char *fname, const char *fallback)
> >> 		return "dts";
> >> 	if (!strcasecmp(s, ".dtb"))
> >> 		return "dtb";
> >> +	if (!strcasecmp(s, ".dtbo"))
> >> +		return "dtbo";
> >> 	return fallback;
> >> }
> >> 
> >> @@ -157,6 +159,8 @@ static const char *guess_input_format(const char *fname, const char *fallback)
> >> 	magic = fdt32_to_cpu(magic);
> >> 	if (magic == FDT_MAGIC)
> >> 		return "dtb";
> >> +	if (magic == FDT_MAGIC_DTBO)
> >> +		return "dtbo";
> >> 
> >> 	return guess_type_by_name(fname, fallback);
> >> }
> >> @@ -285,7 +289,7 @@ int main(int argc, char *argv[])
> >> 		dti = dt_from_source(arg);
> >> 	else if (streq(inform, "fs"))
> >> 		dti = dt_from_fs(arg);
> >> -	else if(streq(inform, "dtb"))
> >> +	else if (streq(inform, "dtb") || streq(inform, "dtbo"))
> >> 		dti = dt_from_blob(arg);
> >> 	else
> >> 		die("Unknown input format \"%s\"\n", inform);
> >> @@ -319,9 +323,13 @@ int main(int argc, char *argv[])
> >> 	if (streq(outform, "dts")) {
> >> 		dt_to_source(outf, dti);
> >> 	} else if (streq(outform, "dtb")) {
> >> -		dt_to_blob(outf, dti, outversion);
> >> +		dt_to_blob(outf, dti, FDT_MAGIC, outversion);
> >> +	} else if (streq(outform, "dtbo")) {
> >> +		dt_to_blob(outf, dti, FDT_MAGIC_DTBO, outversion);
> >> 	} else if (streq(outform, "asm")) {
> >> -		dt_to_asm(outf, dti, outversion);
> >> +		dt_to_asm(outf, dti, FDT_MAGIC, outversion);
> >> +	} else if (streq(outform, "asmo")) {
> >> +		dt_to_asm(outf, dti, FDT_MAGIC_DTBO, outversion);
> >> 	} else if (streq(outform, "null")) {
> >> 		/* do nothing */
> >> 	} else {
> >> diff --git a/dtc.h b/dtc.h
> >> index 040f8d4..fa66dca 100644
> >> --- a/dtc.h
> >> +++ b/dtc.h
> >> @@ -266,8 +266,8 @@ void process_checks(bool force, struct dt_info *dti);
> >> 
> >> /* Flattened trees */
> >> 
> >> -void dt_to_blob(FILE *f, struct dt_info *dti, int version);
> >> -void dt_to_asm(FILE *f, struct dt_info *dti, int version);
> >> +void dt_to_blob(FILE *f, struct dt_info *dti, fdt32_t magic, int version);
> >> +void dt_to_asm(FILE *f, struct dt_info *dti, fdt32_t magic, int version);
> >> 
> >> struct dt_info *dt_from_blob(const char *fname);
> >> 
> >> diff --git a/fdtdump.c b/fdtdump.c
> >> index 95a6a20..fa5a050 100644
> >> --- a/fdtdump.c
> >> +++ b/fdtdump.c
> >> @@ -199,7 +199,7 @@ int main(int argc, char *argv[])
> >> 			p = memchr(p, smagic[0], endp - p - 4);
> >> 			if (!p)
> >> 				break;
> >> -			if (fdt_magic(p) == FDT_MAGIC) {
> >> +			if (fdt_magic(p) == FDT_MAGIC || fdt_magic(p) == FDT_MAGIC_DTBO) {
> >> 				/* try and validate the main struct */
> >> 				off_t this_len = endp - p;
> >> 				fdt32_t max_version = 17;
> >> diff --git a/flattree.c b/flattree.c
> >> index 45fcb04..59bdc7d 100644
> >> --- a/flattree.c
> >> +++ b/flattree.c
> >> @@ -335,6 +335,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
> >> }
> >> 
> >> static void make_fdt_header(struct fdt_header *fdt,
> >> +			    fdt32_t magic,
> >> 			    struct version_info *vi,
> >> 			    int reservesize, int dtsize, int strsize,
> >> 			    int boot_cpuid_phys)
> >> @@ -345,7 +346,7 @@ static void make_fdt_header(struct fdt_header *fdt,
> >> 
> >> 	memset(fdt, 0xff, sizeof(*fdt));
> >> 
> >> -	fdt->magic = cpu_to_fdt32(FDT_MAGIC);
> >> +	fdt->magic = cpu_to_fdt32(magic);
> >> 	fdt->version = cpu_to_fdt32(vi->version);
> >> 	fdt->last_comp_version = cpu_to_fdt32(vi->last_comp_version);
> >> 
> >> @@ -366,7 +367,7 @@ static void make_fdt_header(struct fdt_header *fdt,
> >> 		fdt->size_dt_struct = cpu_to_fdt32(dtsize);
> >> }
> >> 
> >> -void dt_to_blob(FILE *f, struct dt_info *dti, int version)
> >> +void dt_to_blob(FILE *f, struct dt_info *dti, fdt32_t magic, int version)
> >> {
> >> 	struct version_info *vi = NULL;
> >> 	int i;
> >> @@ -390,7 +391,7 @@ void dt_to_blob(FILE *f, struct dt_info *dti, int version)
> >> 	reservebuf = flatten_reserve_list(dti->reservelist, vi);
> >> 
> >> 	/* Make header */
> >> -	make_fdt_header(&fdt, vi, reservebuf.len, dtbuf.len, strbuf.len,
> >> +	make_fdt_header(&fdt, magic, vi, reservebuf.len, dtbuf.len, strbuf.len,
> >> 			dti->boot_cpuid_phys);
> >> 
> >> 	/*
> >> @@ -460,7 +461,7 @@ static void dump_stringtable_asm(FILE *f, struct data strbuf)
> >> 	}
> >> }
> >> 
> >> -void dt_to_asm(FILE *f, struct dt_info *dti, int version)
> >> +void dt_to_asm(FILE *f, struct dt_info *dti, fdt32_t magic, int version)
> >> {
> >> 	struct version_info *vi = NULL;
> >> 	int i;
> >> @@ -832,7 +833,7 @@ struct dt_info *dt_from_blob(const char *fname)
> >> 	}
> >> 
> >> 	magic = fdt32_to_cpu(magic);
> >> -	if (magic != FDT_MAGIC)
> >> +	if (magic != FDT_MAGIC && magic != FDT_MAGIC_DTBO)
> >> 		die("Blob has incorrect magic number\n");
> >> 
> >> 	rc = fread(&totalsize, sizeof(totalsize), 1, f);
> >> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> >> index 22286a1..28d422c 100644
> >> --- a/libfdt/fdt.c
> >> +++ b/libfdt/fdt.c
> >> @@ -57,7 +57,7 @@
> >> 
> >> int fdt_check_header(const void *fdt)
> >> {
> >> -	if (fdt_magic(fdt) == FDT_MAGIC) {
> >> +	if (fdt_magic(fdt) == FDT_MAGIC || fdt_magic(fdt) == FDT_MAGIC_DTBO) {
> >> 		/* Complete tree */
> >> 		if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
> >> 			return -FDT_ERR_BADVERSION;
> >> diff --git a/libfdt/fdt.h b/libfdt/fdt.h
> >> index 526aedb..493cd55 100644
> >> --- a/libfdt/fdt.h
> >> +++ b/libfdt/fdt.h
> >> @@ -55,7 +55,7 @@
> >> #ifndef __ASSEMBLY__
> >> 
> >> struct fdt_header {
> >> -	fdt32_t magic;			 /* magic word FDT_MAGIC */
> >> +	fdt32_t magic;			 /* magic word FDT_MAGIC[|_DTBO] */
> >> 	fdt32_t totalsize;		 /* total size of DT block */
> >> 	fdt32_t off_dt_struct;		 /* offset to structure */
> >> 	fdt32_t off_dt_strings;		 /* offset to strings */
> >> @@ -93,6 +93,7 @@ struct fdt_property {
> >> #endif /* !__ASSEMBLY */
> >> 
> >> #define FDT_MAGIC	0xd00dfeed	/* 4: version, 4: total size */
> >> +#define FDT_MAGIC_DTBO	0xd00dfdb0	/* DTBO magic */
> >> #define FDT_TAGSIZE	sizeof(fdt32_t)
> >> 
> >> #define FDT_BEGIN_NODE	0x1		/* Start node: full name */
> >> diff --git a/tests/mangle-layout.c b/tests/mangle-layout.c
> >> index a76e51e..d29ebc6 100644
> >> --- a/tests/mangle-layout.c
> >> +++ b/tests/mangle-layout.c
> >> @@ -42,7 +42,8 @@ static void expand_buf(struct bufstate *buf, int newsize)
> >> 	buf->size = newsize;
> >> }
> >> 
> >> -static void new_header(struct bufstate *buf, int version, const void *fdt)
> >> +static void new_header(struct bufstate *buf, fdt32_t magic, int version,
> >> +		       const void *fdt)
> >> {
> >> 	int hdrsize;
> >> 
> >> @@ -56,7 +57,7 @@ static void new_header(struct bufstate *buf, int version, const void *fdt)
> >> 	expand_buf(buf, hdrsize);
> >> 	memset(buf->buf, 0, hdrsize);
> >> 
> >> -	fdt_set_magic(buf->buf, FDT_MAGIC);
> >> +	fdt_set_magic(buf->buf, magic);
> >> 	fdt_set_version(buf->buf, version);
> >> 	fdt_set_last_comp_version(buf->buf, 16);
> >> 	fdt_set_boot_cpuid_phys(buf->buf, fdt_boot_cpuid_phys(fdt));
> >> @@ -145,7 +146,7 @@ int main(int argc, char *argv[])
> >> 	if (fdt_version(fdt) < 17)
> >> 		CONFIG("Input tree must be v17");
> >> 
> >> -	new_header(&buf, version, fdt);
> >> +	new_header(&buf, FDT_MAGIC, version, fdt);
> >> 
> >> 	while (*blockorder) {
> >> 		add_block(&buf, version, *blockorder, fdt);
> > 
> 
> Regards
> 
> — Pantelis
> 
> 

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]