Assemble the blob in memory before writing it out.
PATCH 1/1 Assemble the blob in memory before writing it out.
This makes padding out the blob if the user requested extra size much easer. The assembly and writing to the file is more straight forward too.
---
Hi David, Jon,
I wasn't happy with David's suggestion on the -S handling. I also wasn't all that wild about how the blob was assembled and then written to the file piecemeal with "ad-hoc" alignment padding. I realized I could fix both by assembling the blob in memory as a "struct data" and then write the whole thing out in one fell swoop.
Makes it a lot simpler and cleaner to my eyes.
Hope y'all agree,
gvb
flattree.c | 82 +++++++++++++++++++++++++++++++++++------------------------- 1 files changed, 48 insertions(+), 34 deletions(-)
flattree.c +48 -34
@@ -310,6 +310,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,return d; } + static void make_bph(struct boot_param_header *bph, struct version_info *vi, int reservesize, int dtsize, int strsize,
@@ -332,19 +333,8 @@ static void make_bph(struct boot_param_header *bph, bph->off_dt_struct =cpu_to_be32 (reserve_off + reservesize); bph->off_dt_strings =cpu_to_be32 (reserve_off + reservesize + dtsize); - bph->totalsize = reserve_off + reservesize + dtsize + strsize; -if (minsize >0 ) { -if (bph->totalsize >= minsize) { -if (quiet <1 ) -fprintf (stderr, -"Warning: blob size %d >= minimum size %d\n" , - bph->totalsize, minsize); + bph->totalsize =cpu_to_be32 (reserve_off + reservesize + dtsize + strsize); - }else - bph->totalsize = minsize; - } - bph->totalsize =cpu_to_be32 (bph->totalsize); -if (vi->flags &FTF_BOOTCPUID ) bph->boot_cpuid_phys =cpu_to_be32 (boot_cpuid_phys);if (vi->flags &FTF_STRTABSIZE )
@@ -358,11 +348,11 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version, {struct version_info *vi =NULL ;int i; -struct data dtbuf = empty_data; -struct data strbuf = empty_data; -struct data reservebuf; +struct data blob = empty_data; +struct data reservebuf = empty_data; +struct data dtbuf = empty_data; +struct data strbuf = empty_data;struct boot_param_header bph; -struct reserve_entry termre = {.address =0 , .size =0 };for (i =0 ; i <ARRAY_SIZE (version_table); i++) {if (version_table[i].version == version)
@@ -371,9 +361,6 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,if (!vi)die ("Unknown device tree blob version %d\n" , version); - dtbuf = empty_data; - strbuf = empty_data; -flatten_tree (bi->dt, &bin_emitter, &dtbuf, &strbuf, vi);bin_emit_cell (&dtbuf,OF_DT_END );
@@ -383,28 +370,46 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,make_bph (&bph, vi, reservebuf.len, dtbuf.len, strbuf.len, boot_cpuid_phys); -fwrite (&bph, vi->hdr_size,1 , f); - -/* Align the reserve map to an 8 byte boundary */ -for (i = vi->hdr_size; i <be32_to_cpu (bph.off_mem_rsvmap); i++) -fputc (0 , f); +/* +* Assemble the blob: start with the header, add with alignment +* the reserve buffer, add the reserve map terminating zeroes, +* the device tree itself, and finally the strings. +*/ + blob =data_append_data (blob, &bph,sizeof (bph)); + blob =data_append_align (blob,8 ); + blob =data_merge (blob, reservebuf); + blob =data_append_zeroes (blob,sizeof (struct reserve_entry )); + blob =data_merge (blob, dtbuf); + blob =data_merge (blob, strbuf);/* -* Reserve map entries. -* Each entry is an (address, size) pair of u64 values. -* Always supply a zero-sized temination entry. +* If the user asked for more space than is used, pad out the blob. */ -fwrite (reservebuf.val, reservebuf.len,1 , f); -fwrite (&termre,sizeof (termre),1 , f); +if (minsize >0 ) { +int padlen = minsize -be32_to_cpu (bph.totalsize); + +fprintf (stderr,"minsize = %d, totalsize = %d, padlen = %d\n" , minsize,be32_to_cpu (bph.totalsize), padlen); +if (padlen >0 ) { + blob =data_append_zeroes (blob, padlen); + bph.totalsize =cpu_to_be32 (minsize); + }else { +if (quiet <1 ) +fprintf (stderr, +"Warning: blob size %d >= minimum size %d\n" , +be32_to_cpu (bph.totalsize), minsize); + } + } -fwrite (dtbuf.val, dtbuf.len,1 , f); -fwrite (strbuf.val, strbuf.len,1 , f); +fwrite (blob.val, blob.len,1 , f);if (ferror (f))die ("Error writing device tree blob: %s\n" ,strerror (errno)); -data_free (dtbuf); -data_free (strbuf); +/* +* data_merge() frees the right-hand element so only the blob +* remains to be freed. +*/ +data_free (blob); }static void dump_stringtable_asm (FILE *f,struct data strbuf)
@@ -447,7 +452,7 @@ void dt_to_asm(FILE *f, struct boot_info *bi, int version, int boot_cpuid_phys)emit_label (f, symprefix,"blob_start" );emit_label (f, symprefix,"header" );fprintf (f,"\t.long\tOF_DT_HEADER /* magic */\n" ); -fprintf (f,"\t.long\t_%s_blob_end - _%s_blob_start /* totalsize */\n" , +fprintf (f,"\t.long\t_%s_blob_abs_end - _%s_blob_start /* totalsize */\n" , symprefix, symprefix);fprintf (f,"\t.long\t_%s_struct_start - _%s_blob_start /* off_dt_struct */\n" , symprefix, symprefix);
@@ -504,6 +509,15 @@ void dt_to_asm(FILE *f, struct boot_info *bi, int version, int boot_cpuid_phys)emit_label (f, symprefix,"blob_end" ); +/* +* If the user asked for more space than is used, pad it out. +*/ +if (minsize >0 ) { +fprintf (f,"\t.space\t%d - (_%s_blob_end - _%s_blob_start), 0\n" , + minsize, symprefix, symprefix); + } +emit_label (f, symprefix,"blob_abs_end" ); +data_free (strbuf); }
Signature · 1 line
1.4.4.4
Quoted 8 lines · Jerry Van Baren
This makes padding out the blob if the user requested extra size much easer. The assembly and writing to the file is more straight forward too.
Signed-off-by Gerald Van Baren <[email protected]>---
Hi David, Jon,
Quoted 7 lines
I wasn't happy with David's suggestion on the -S handling. I also wasn't all that wild about how the blob was assembled and then written to the file piecemeal with "ad-hoc" alignment padding. I realized I could fix both by assembling the blob in memory as a "struct data" and then write the whole thing out in one fell swoop.
Makes it a lot simpler and cleaner to my eyes.
I like it. (In fact my original suggestion about using data_append_zeroes() was based on a mistaken vague notion that we already did it this way).
Signature · 4 lines
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
Quoted 14 lines · Jerry Van Baren
This makes padding out the blob if the user requested extra size much easer. The assembly and writing to the file is more straight forward too.
Signed-off-by Gerald Van Baren <[email protected]>---
Hi David, Jon,
I wasn't happy with David's suggestion on the -S handling. I also wasn't all that wild about how the blob was assembled and then written to the file piecemeal with "ad-hoc" alignment padding. I realized I could fix both by assembling the blob in memory as a "struct data" and then write the whole thing out in one fell swoop.
Quoted 4 lines
Makes it a lot simpler and cleaner to my eyes.
Hope y'all agree,
gvb
Jerry and David,
Sorry. I've been out sick for a couple days here and am just now catching up. I've read through the last week's worth of mail here now, so let me see if I have properly summarized where things stand and what's needed for the DTC:
These patches need to be applied:
[PATCH dtc] Add -o <output file> to the usage message. Sat, 14 Apr 2007 18:16:47 -0400
[PATCH dtc take 3] Fix reserve map output for asm format. Tue, 17 Apr 2007 18:14:41 -0400
[PATCH: dtc take 2] Assemble the blob in memory before writing it out. Wed, 18 Apr 2007 21:59:51 -0400
The last patch there replaces the "Improve -S handling" patches. But does the last patch replace or depend on the second one?
And I agree with David that last patch is a better approach.
Thanks,
jdl
Quoted 38 lines · Jon Loeliger
Quoted 14 lines · Jerry Van Baren
This makes padding out the blob if the user requested extra size much easer. The assembly and writing to the file is more straight forward too.
Signed-off-by Gerald Van Baren <[email protected]>---
Hi David, Jon,
I wasn't happy with David's suggestion on the -S handling. I also wasn't all that wild about how the blob was assembled and then written to the file piecemeal with "ad-hoc" alignment padding. I realized I could fix both by assembling the blob in memory as a "struct data" and then write the whole thing out in one fell swoop.
Quoted 4 lines
Makes it a lot simpler and cleaner to my eyes.
Hope y'all agree,
gvbJerry and David,
Sorry. I've been out sick for a couple days here and am just now catching up. I've read through the last week's worth of mail here now, so let me see if I have properly summarized where things stand and what's needed for the DTC:
These patches need to be applied:
[PATCH dtc] Add -o <output file> to the usage message. Sat, 14 Apr 2007 18:16:47 -0400
[PATCH dtc take 3] Fix reserve map output for asm format. Tue, 17 Apr 2007 18:14:41 -0400
[PATCH: dtc take 2] Assemble the blob in memory before writing it out. Wed, 18 Apr 2007 21:59:51 -0400
Quoted 7 lines
The last patch there replaces the "Improve -S handling" patches. But does the last patch replace or depend on the second one?
And I agree with David that last patch is a better approach.
Thanks,
jdl
Yes, you have that right.
Patch #1 is good to go
[PATCH dtc] Add -o <output file> to the usage message. Sat, 14 Apr 2007 18:16:47 -0400
Patch #2 and #3...
[PATCH dtc take 3] Fix reserve map output for asm format. Tue, 17 Apr 2007 18:14:41 -0400
[PATCH: dtc take 2] Assemble the blob in memory before writing it out. Wed, 18 Apr 2007 21:59:51 -0400
...you force me to make a confession. The third patch depends on the second patch because I snuck in a fix to the ASM output into the third
patch: the last two chunks fix the ASM header output - I padded the blob out but didn't change the size properly in the header. In the third patch above, I added a label "%s_blob_abs_end" and used that to calculate the total blob size (including the padding) in the header:
- fprintf(f, "\t.long\t_%s_blob_end - _%s_blob_start /* totalsize */\n",
+ fprintf(f, "\t.long\t_%s_blob_abs_end - _%s_blob_start /* totalsize */\n",
--------
AAAARGH! Sorry, I see I left a debug statement in flattree.c around line 390:
@@ -383,28 +370,46 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
:
:
+fprintf(stderr, "minsize = %d, totalsize = %d, padlen = %d\n", minsize, be32_to_cpu(bph.totalsize), padlen);
Your option:
1) Take the last two patches as is and remove the fprintf yourself
2) I'll fix it tonight and provide a New Improved patch
Thanks,
gvb
Quoted 20 lines · Jerry Van Baren
Quoted 20 lines
Sorry. I've been out sick for a couple days here and am just now catching up. I've read through the last week's worth of mail here now, so let me see if I have properly summarized where things stand and what's needed for the DTC:
These patches need to be applied:
[PATCH dtc] Add -o <output file> to the usage message. Sat, 14 Apr 2007 18:16:47 -0400
[PATCH dtc take 3] Fix reserve map output for asm format. Tue, 17 Apr 2007 18:14:41 -0400
[PATCH: dtc take 2] Assemble the blob in memory before writing it out. Wed, 18 Apr 2007 21:59:51 -0400
The last patch there replaces the "Improve -S handling" patches. But does the last patch replace or depend on the second one?
Quoted 6 lines
Quoted 4 lines
And I agree with David that last patch is a better approach.
Thanks,
jdlYes, you have that right.
Good. The drugs aren't too strong yet. :-)
Quoted 11 lines
Patch #1 is good to go
[PATCH dtc] Add -o <output file> to the usage message. Sat, 14 Apr 2007 18:16:47 -0400Patch #2 and #3...
[PATCH dtc take 3] Fix reserve map output for asm format. Tue, 17 Apr 2007 18:14:41 -0400
[PATCH: dtc take 2] Assemble the blob in memory before writing it out. Wed, 18 Apr 2007 21:59:51 -0400...you force me to make a confession.
It's good for the soul. :-)
Quoted 3 lines
Your option:
1) Take the last two patches as is and remove the fprintf yourself
2) I'll fix it tonight and provide a New Improved patch
I'll go with Door Number 1! No problem.
Thanks,
jdl
So, like, the other day Jerry Van Baren mumbled:
Quoted 6 lines
Patch #2 and #3...
[PATCH dtc take 3] Fix reserve map output for asm format. Tue, 17 Apr 2007 18:14:41 -0400
[PATCH: dtc take 2] Assemble the blob in memory before writing it out. Wed, 18 Apr 2007 21:59:51 -0400
[snip]
Quoted 5 lines
AAAARGH! Sorry, I see I left a debug statement in
flattree.caround line 390:Your option:
1) Take the last two patches as is and remove the fprintf yourself
Applied #2 and #3 listed above.
Cleaned out the debug fprintf().
Thanks,
jdl
Quoted 16 lines · Jon Loeliger
So, like, the other day Jerry Van Baren mumbled:
Quoted 5 lines
Patch #2 and #3...
[PATCH dtc take 3] Fix reserve map output for asm format. Tue, 17 Apr 2007 18:14:41 -0400
[PATCH: dtc take 2] Assemble the blob in memory before writing it out. Wed, 18 Apr 2007 21:59:51 -0400[snip]
Quoted 5 lines
AAAARGH! Sorry, I see I left a debug statement in
flattree.caround line 390:Your option:
1) Take the last two patches as is and remove the fprintf yourself
Quoted 5 lines
Applied #2 and #3 listed above.
Cleaned out the debugfprintf().Thanks,
jdl
Oh oh, does NOT look good. I lost the rewrite of the header size with the new, bigger size when -S is specified.
Ahh, I think I see the problem, sloppy programming bites me again. Modifying "bph.totalsize" worked before I changed to the struct data
blob. Grrrr. I need to figure out the padding and apply it to the bph
before starting the blob assembly. Before the line:
blob = data_append_data(blob, &bph, sizeof(bph));
Patch will take a bit because I have a meeting to go to :-(.
sorry,
gvb