Implement the -a option to pad dtb aligned

PATCH Superseded Tim Wang 2 participants 3 messages Jul 1, 2016 – Jul 7, 2016 2 messages not in the archive

Share & tools

Tim Wang

author

2016-07-01 09:30 UTC #0 lore raw

PATCH v4 1/1 Implement the -a option to pad dtb aligned

There is one condition that need cat the dtb files into one dtb.img which can support several boards use same SoC platform.

And the original dtb file size is not aligned to any base. This may cause "Synchronous Abort" when load from a unligned address on some SoC machine, such as ARM.

So this patch implement the -a <aligned number> option to pad zero at the end of dtb files and make the dtb size aligned to <aligned number>.

Then, the aligned dtbs can cat together and load without "Synchronous Abort".

Signed-off-by Tim Wang <timwang-XOWFcpiLszpWk0Htik3J/[email protected]>

---
 dtc.c      | 17 ++++++++++++++++-
 dtc.h      |  1 +
 flattree.c | 10 +++++++++-
 3 files changed, 26 insertions(+), 2 deletions(-)

dtc.c +16 -1

index 5fa23c4..d47089e 100644
@@ -30,8 +30,14 @@ int quiet;		/* Level of quietness */
 int reservenum;		/* Number of memory reservation slots */
 int minsize;		/* Minimum blob size */
 int padsize;		/* Additional padding to blob */
+int alignsize;		/* Additional padding to blob accroding to the alignsize */
 int phandle_format = PHANDLE_BOTH;	/* Use linux,phandle or phandle properties */
 
+static int is_power_of_2(int x)
+{
+	return (x > 0) && ((x & (x - 1)) == 0);
+}
+
 static void fill_fullpaths(struct node *tree, const char *prefix)
 {
 	struct node *child;

@@ -53,7 +59,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
 #define FDT_VERSION(version)	_FDT_VERSION(version)
 #define _FDT_VERSION(version)	#version
 static const char usage_synopsis[] = "dtc [options] <input file>";
-static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:fb:i:H:sW:E:hv";
+static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:hv";
 static struct option const usage_long_opts[] = {
 	{"quiet",            no_argument, NULL, 'q'},
 	{"in-format",         a_argument, NULL, 'I'},

@@ -64,6 +70,7 @@ static struct option const usage_long_opts[] = {
 	{"reserve",           a_argument, NULL, 'R'},
 	{"space",             a_argument, NULL, 'S'},
 	{"pad",               a_argument, NULL, 'p'},
+	{"align",             a_argument, NULL, 'a'},
 	{"boot-cpu",          a_argument, NULL, 'b'},
 	{"force",            no_argument, NULL, 'f'},
 	{"include",           a_argument, NULL, 'i'},

@@ -91,6 +98,7 @@ static const char * const usage_opts_help[] = {
 	"\n\tMake space for <number> reserve map entries (for dtb and asm output)",
 	"\n\tMake the blob at least <bytes> long (extra space)",
 	"\n\tAdd padding to the blob of <bytes> long (extra space)",
+	"\n\tMake the blob align to the <bytes> (extra space)",
 	"\n\tSet the physical boot cpu",
 	"\n\tTry to produce output even if the input tree has errors",
 	"\n\tAdd a path to search for include files",

@@ -169,6 +177,7 @@ int main(int argc, char *argv[])
 	reservenum = 0;
 	minsize    = 0;
 	padsize    = 0;
+	alignsize  = 0;
 
 	while ((opt = util_getopt_long()) != EOF) {
 		switch (opt) {

@@ -196,6 +205,12 @@ int main(int argc, char *argv[])
 		case 'p':
 			padsize = strtol(optarg, NULL, 0);
 			break;
+		case 'a':
+			alignsize = strtol(optarg, NULL, 0);
+			if (!is_power_of_2(alignsize))
+				die("Invalid argument \"%s\" to -H option\n",
+				    optarg);
+			break;
 		case 'f':
 			force = true;
 			break;

dtc.h +1 -0

index 56212c8..32009bc 100644
@@ -53,6 +53,7 @@ extern int quiet;		/* Level of quietness */
 extern int reservenum;		/* Number of memory reservation slots */
 extern int minsize;		/* Minimum blob size */
 extern int padsize;		/* Additional padding to blob */
+extern int alignsize;		/* Additional padding to blob accroding to the alignsize */
 extern int phandle_format;	/* Use linux,phandle or phandle properties */
 
 #define PHANDLE_LEGACY	0x1

flattree.c +9 -1

index ec14954..1cdd426 100644
@@ -398,15 +398,21 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version)
 	 */
 	if (minsize > 0) {
 		padlen = minsize - fdt32_to_cpu(fdt.totalsize);
-		if ((padlen < 0) && (quiet < 1))
+		if ((padlen < 0) && (quiet < 1)) {
 			fprintf(stderr,
 				"Warning: blob size %d >= minimum size %d\n",
 				fdt32_to_cpu(fdt.totalsize), minsize);
+			padlen = 0;
+		}
 	}
 
 	if (padsize > 0)
 		padlen = padsize;
 
+	if (alignsize > 0)
+		padlen = ALIGN(fdt32_to_cpu(fdt.totalsize) + padlen, alignsize)
+			- fdt32_to_cpu(fdt.totalsize);
+
 	if (padlen > 0) {
 		int tsize = fdt32_to_cpu(fdt.totalsize);
 		tsize += padlen;

@@ -572,6 +578,8 @@ void dt_to_asm(FILE *f, struct boot_info *bi, int version)
 	if (padsize > 0) {
 		fprintf(f, "\t.space\t%d, 0\n", padsize);
 	}
+	if (alignsize > 0)
+		asm_emit_align(f, alignsize);
 	emit_label(f, symprefix, "blob_abs_end");
 
 	data_free(strbuf);

Signature · 1 line

1.9.1

A message that is not in the archive (sent off-list or not yet received): 1467365408-923-1-git-send-email-timwang-XOWFcpiLszpWk0Htik3J/[email protected]

David Gibson

2016-07-07 04:45 UTC #2 lore raw

Quoted 75 lines · Tim Wang

There is one condition that need cat the dtb files into one dtb.img which can support several boards use same SoC platform.

And the original dtb file size is not aligned to any base. This may cause "Synchronous Abort" when load from a unligned address on some SoC machine, such as ARM.

So this patch implement the -a <aligned number> option to pad zero at the end of dtb files and make the dtb size aligned to <aligned number>.

Then, the aligned dtbs can cat together and load without "Synchronous Abort".

Signed-off-by Tim Wang <timwang-XOWFcpiLszpWk0Htik3J/[email protected]>

---
 dtc.c      | 17 ++++++++++++++++-
 dtc.h      |  1 +
 flattree.c | 10 +++++++++-
 3 files changed, 26 insertions(+), 2 deletions(-)

dtc.c +10 -1

index 5fa23c4..d47089e 100644
@@ -30,8 +30,14 @@ int quiet;		/* Level of quietness */
 int reservenum;		/* Number of memory reservation slots */
 int minsize;		/* Minimum blob size */
 int padsize;		/* Additional padding to blob */
+int alignsize;		/* Additional padding to blob accroding to the alignsize */
 int phandle_format = PHANDLE_BOTH;	/* Use linux,phandle or phandle properties */
 
+static int is_power_of_2(int x)
+{
+	return (x > 0) && ((x & (x - 1)) == 0);
+}
+
 static void fill_fullpaths(struct node *tree, const char *prefix)
 {
 	struct node *child;

@@ -53,7 +59,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
 #define FDT_VERSION(version)	_FDT_VERSION(version)
 #define _FDT_VERSION(version)	#version
 static const char usage_synopsis[] = "dtc [options] <input file>";
-static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:fb:i:H:sW:E:hv";
+static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:hv";
 static struct option const usage_long_opts[] = {
 	{"quiet",            no_argument, NULL, 'q'},
 	{"in-format",         a_argument, NULL, 'I'},

@@ -64,6 +70,7 @@ static struct option const usage_long_opts[] = {
 	{"reserve",           a_argument, NULL, 'R'},
 	{"space",             a_argument, NULL, 'S'},
 	{"pad",               a_argument, NULL, 'p'},
+	{"align",             a_argument, NULL, 'a'},
 	{"boot-cpu",          a_argument, NULL, 'b'},
 	{"force",            no_argument, NULL, 'f'},
 	{"include",           a_argument, NULL, 'i'},

@@ -91,6 +98,7 @@ static const char * const usage_opts_help[] = {
 	"\n\tMake space for <number> reserve map entries (for dtb and asm output)",
 	"\n\tMake the blob at least <bytes> long (extra space)",
 	"\n\tAdd padding to the blob of <bytes> long (extra space)",
+	"\n\tMake the blob align to the <bytes> (extra space)",
 	"\n\tSet the physical boot cpu",
 	"\n\tTry to produce output even if the input tree has errors",
 	"\n\tAdd a path to search for include files",

@@ -169,6 +177,7 @@ int main(int argc, char *argv[])
 	reservenum = 0;
 	minsize    = 0;
 	padsize    = 0;
+	alignsize  = 0;
 
 	while ((opt = util_getopt_long()) != EOF) {
 		switch (opt) {

dtc.c int main(int argc, char *argv[]) · line 211

Quoted 7 lines

dtc.c +4 -0

@@ -196,6 +205,12 @@ int main(int argc, char *argv[])
 		case 'p':
 			padsize = strtol(optarg, NULL, 0);
 			break;
+		case 'a':
+			alignsize = strtol(optarg, NULL, 0);
+			if (!is_power_of_2(alignsize))
+				die("Invalid argument \"%s\" to -H option\n",

You have the wrong option name here. Also, since the argument isn't a string, I wouldn't suggest the \"s.

Quoted 22 lines

dtc.c +2 -0

+				    optarg);
+			break;
 		case 'f':
 			force = true;
 			break;

dtc.h +1 -0

index 56212c8..32009bc 100644
@@ -53,6 +53,7 @@ extern int quiet;		/* Level of quietness */
 extern int reservenum;		/* Number of memory reservation slots */
 extern int minsize;		/* Minimum blob size */
 extern int padsize;		/* Additional padding to blob */
+extern int alignsize;		/* Additional padding to blob accroding to the alignsize */
 extern int phandle_format;	/* Use linux,phandle or phandle properties */
 
 #define PHANDLE_LEGACY	0x1

flattree.c +0 -0

index ec14954..1cdd426 100644

flattree.c void dt_to_blob(FILE *f, struct boot_info *bi, int version) · line 405

Quoted 9 lines

flattree.c +2 -1

@@ -398,15 +398,21 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version)
 	 */
 	if (minsize > 0) {
 		padlen = minsize - fdt32_to_cpu(fdt.totalsize);
-		if ((padlen < 0) && (quiet < 1))
+		if ((padlen < 0) && (quiet < 1)) {
 			fprintf(stderr,
 				"Warning: blob size %d >= minimum size %d\n",
 				fdt32_to_cpu(fdt.totalsize), minsize);
+			padlen = 0;

Here, your behaviour will change if you add the -q option, which is incorrect. You'll need to split the if condition.

flattree.c void dt_to_blob(FILE *f, struct boot_info *bi, int version) · line 415

Quoted 10 lines

flattree.c +5 -0

+		}
 	}
 
 	if (padsize > 0)
 		padlen = padsize;
 
+	if (alignsize > 0)
+		padlen = ALIGN(fdt32_to_cpu(fdt.totalsize) + padlen, alignsize)
+			- fdt32_to_cpu(fdt.totalsize);
+

This looks correct now.

Quoted 12 lines

flattree.c +2 -0

 	if (padlen > 0) {
 		int tsize = fdt32_to_cpu(fdt.totalsize);
 		tsize += padlen;

@@ -572,6 +578,8 @@ void dt_to_asm(FILE *f, struct boot_info *bi, int version)
 	if (padsize > 0) {
 		fprintf(f, "\t.space\t%d, 0\n", padsize);
 	}
+	if (alignsize > 0)
+		asm_emit_align(f, alignsize);
 	emit_label(f, symprefix, "blob_abs_end");
 
 	data_free(strbuf);

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

Wang Tim（王艇艇）

author

2016-07-07 09:56 UTC #4 lore raw

Hi David,
Thanks for your carefully review!
I'll refine my patch in next patch version according your suggestions.

Best Regards
Tim Wang(王艇艇)

-----Original Message-----
From: David Gibson [mailto:[email protected]]
Sent: Thursday, July 07, 2016 12:45 PM
To: Wang Tim（王艇艇）
Cc: [email protected]; [email protected]
Subject: Re: [PATCH v4] Implement the -a option to pad dtb aligned

Quoted 75 lines · Tim Wang

There is one condition that need cat the dtb files into one dtb.img which can support several boards use same SoC platform.

And the original dtb file size is not aligned to any base. This may cause "Synchronous Abort" when load from a unligned address on some SoC machine, such as ARM.

So this patch implement the -a <aligned number> option to pad zero at the end of dtb files and make the dtb size aligned to <aligned number>.

Then, the aligned dtbs can cat together and load without "Synchronous Abort".

Signed-off-by Tim Wang <[email protected]>

---
 dtc.c      | 17 ++++++++++++++++-
 dtc.h      |  1 +
 flattree.c | 10 +++++++++-
 3 files changed, 26 insertions(+), 2 deletions(-)

dtc.c +10 -1

index 5fa23c4..d47089e 100644
@@ -30,8 +30,14 @@ int quiet;		/* Level of quietness */
 int reservenum;		/* Number of memory reservation slots */
 int minsize;		/* Minimum blob size */
 int padsize;		/* Additional padding to blob */
+int alignsize;		/* Additional padding to blob accroding to the alignsize */
 int phandle_format = PHANDLE_BOTH;	/* Use linux,phandle or phandle properties */
 
+static int is_power_of_2(int x)
+{
+	return (x > 0) && ((x & (x - 1)) == 0);
+}
+
 static void fill_fullpaths(struct node *tree, const char *prefix)
 {
 	struct node *child;

@@ -53,7 +59,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
 #define FDT_VERSION(version)	_FDT_VERSION(version)
 #define _FDT_VERSION(version)	#version
 static const char usage_synopsis[] = "dtc [options] <input file>";
-static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:fb:i:H:sW:E:hv";
+static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:hv";
 static struct option const usage_long_opts[] = {
 	{"quiet",            no_argument, NULL, 'q'},
 	{"in-format",         a_argument, NULL, 'I'},

@@ -64,6 +70,7 @@ static struct option const usage_long_opts[] = {
 	{"reserve",           a_argument, NULL, 'R'},
 	{"space",             a_argument, NULL, 'S'},
 	{"pad",               a_argument, NULL, 'p'},
+	{"align",             a_argument, NULL, 'a'},
 	{"boot-cpu",          a_argument, NULL, 'b'},
 	{"force",            no_argument, NULL, 'f'},
 	{"include",           a_argument, NULL, 'i'},

@@ -91,6 +98,7 @@ static const char * const usage_opts_help[] = {
 	"\n\tMake space for <number> reserve map entries (for dtb and asm output)",
 	"\n\tMake the blob at least <bytes> long (extra space)",
 	"\n\tAdd padding to the blob of <bytes> long (extra space)",
+	"\n\tMake the blob align to the <bytes> (extra space)",
 	"\n\tSet the physical boot cpu",
 	"\n\tTry to produce output even if the input tree has errors",
 	"\n\tAdd a path to search for include files",

@@ -169,6 +177,7 @@ int main(int argc, char *argv[])
 	reservenum = 0;
 	minsize    = 0;
 	padsize    = 0;
+	alignsize  = 0;
 
 	while ((opt = util_getopt_long()) != EOF) {
 		switch (opt) {

dtc.c int main(int argc, char *argv[]) · line 211

Quoted 7 lines

dtc.c +4 -0

@@ -196,6 +205,12 @@ int main(int argc, char *argv[])
 		case 'p':
 			padsize = strtol(optarg, NULL, 0);
 			break;
+		case 'a':
+			alignsize = strtol(optarg, NULL, 0);
+			if (!is_power_of_2(alignsize))
+				die("Invalid argument \"%s\" to -H option\n",

You have the wrong option name here. Also, since the argument isn't a string, I wouldn't suggest the \"s.

Quoted 22 lines

dtc.c +2 -0

+				    optarg);
+			break;
 		case 'f':
 			force = true;
 			break;

dtc.h +1 -0

index 56212c8..32009bc 100644
@@ -53,6 +53,7 @@ extern int quiet;		/* Level of quietness */
 extern int reservenum;		/* Number of memory reservation slots */
 extern int minsize;		/* Minimum blob size */
 extern int padsize;		/* Additional padding to blob */
+extern int alignsize;		/* Additional padding to blob accroding to the alignsize */
 extern int phandle_format;	/* Use linux,phandle or phandle properties */
 
 #define PHANDLE_LEGACY	0x1

flattree.c +0 -0

index ec14954..1cdd426 100644

flattree.c void dt_to_blob(FILE *f, struct boot_info *bi, int version) · line 405

Quoted 9 lines

flattree.c +2 -1

@@ -398,15 +398,21 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version)
 	 */
 	if (minsize > 0) {
 		padlen = minsize - fdt32_to_cpu(fdt.totalsize);
-		if ((padlen < 0) && (quiet < 1))
+		if ((padlen < 0) && (quiet < 1)) {
 			fprintf(stderr,
 				"Warning: blob size %d >= minimum size %d\n",
 				fdt32_to_cpu(fdt.totalsize), minsize);
+			padlen = 0;

Here, your behaviour will change if you add the -q option, which is incorrect. You'll need to split the if condition.

flattree.c void dt_to_blob(FILE *f, struct boot_info *bi, int version) · line 415

Quoted 10 lines

flattree.c +5 -0

+		}
 	}
 
 	if (padsize > 0)
 		padlen = padsize;
 
+	if (alignsize > 0)
+		padlen = ALIGN(fdt32_to_cpu(fdt.totalsize) + padlen, alignsize)
+			- fdt32_to_cpu(fdt.totalsize);
+

This looks correct now.

Quoted 12 lines

flattree.c +2 -0

 	if (padlen > 0) {
 		int tsize = fdt32_to_cpu(fdt.totalsize);
 		tsize += padlen;

@@ -572,6 +578,8 @@ void dt_to_asm(FILE *f, struct boot_info *bi, int version)
 	if (padsize > 0) {
 		fprintf(f, "\t.space\t%d, 0\n", padsize);
 	}
+	if (alignsize > 0)
+		asm_emit_align(f, alignsize);
 	emit_label(f, symprefix, "blob_abs_end");
 
 	data_free(strbuf);

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