From 86aeacca6792128cf16cd054a69018346a7aa601 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Fri, 15 Dec 2023 10:42:30 +0100 Subject: [PATCH 01/24] buildman: type cotaining %s/cotaining/containing/ Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- tools/buildman/boards.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/buildman/boards.py b/tools/buildman/boards.py index 341a5056dfd..3c2822715f3 100644 --- a/tools/buildman/boards.py +++ b/tools/buildman/boards.py @@ -119,7 +119,7 @@ class Expr: """Set up a new Expr object. Args: - expr (str): String cotaining regular expression to store + expr (str): String containing regular expression to store """ self._expr = expr self._re = re.compile(expr) From 69c3705be9fe80a084408c5f8db7bceec4c720dd Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 16 Dec 2023 00:26:04 +0100 Subject: [PATCH 02/24] binman: used-before-assignment in ftest.py Pytest 7.4.3 complains if a variable is used in a finally clause without having been initialized before the try clause. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- tools/binman/ftest.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index a273120d9f9..a4ac520cbbb 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2842,12 +2842,14 @@ class TestFunctional(unittest.TestCase): fdt_size = entries['section'].GetEntries()['u-boot-dtb'].size fdtmap_offset = entries['fdtmap'].offset + tmpdir = None try: tmpdir, updated_fname = self._SetupImageInTmpdir() with test_util.capture_sys_output() as (stdout, stderr): self._DoBinman('ls', '-i', updated_fname) finally: - shutil.rmtree(tmpdir) + if tmpdir: + shutil.rmtree(tmpdir) lines = stdout.getvalue().splitlines() expected = [ 'Name Image-pos Size Entry-type Offset Uncomp-size', @@ -2868,12 +2870,14 @@ class TestFunctional(unittest.TestCase): def testListCmdFail(self): """Test failing to list an image""" self._DoReadFile('005_simple.dts') + tmpdir = None try: tmpdir, updated_fname = self._SetupImageInTmpdir() with self.assertRaises(ValueError) as e: self._DoBinman('ls', '-i', updated_fname) finally: - shutil.rmtree(tmpdir) + if tmpdir: + shutil.rmtree(tmpdir) self.assertIn("Cannot find FDT map in image", str(e.exception)) def _RunListCmd(self, paths, expected): @@ -3002,13 +3006,15 @@ class TestFunctional(unittest.TestCase): self._CheckLz4() self._DoReadFileRealDtb('130_list_fdtmap.dts') fname = os.path.join(self._indir, 'output.extact') + tmpdir = None try: tmpdir, updated_fname = self._SetupImageInTmpdir() with test_util.capture_sys_output() as (stdout, stderr): self._DoBinman('extract', '-i', updated_fname, 'u-boot', '-f', fname) finally: - shutil.rmtree(tmpdir) + if tmpdir: + shutil.rmtree(tmpdir) data = tools.read_file(fname) self.assertEqual(U_BOOT_DATA, data) @@ -5185,12 +5191,14 @@ fdt fdtmap Extract the devicetree blob from the fdtmap data = self._DoReadFileRealDtb('207_fip_ls.dts') hdr, fents = fip_util.decode_fip(data) + tmpdir = None try: tmpdir, updated_fname = self._SetupImageInTmpdir() with test_util.capture_sys_output() as (stdout, stderr): self._DoBinman('ls', '-i', updated_fname) finally: - shutil.rmtree(tmpdir) + if tmpdir: + shutil.rmtree(tmpdir) lines = stdout.getvalue().splitlines() expected = [ 'Name Image-pos Size Entry-type Offset Uncomp-size', @@ -5395,12 +5403,14 @@ fdt fdtmap Extract the devicetree blob from the fdtmap use_real_dtb=True, extra_indirs=[os.path.join(self._indir, TEST_FDT_SUBDIR)]) + tmpdir = None try: tmpdir, updated_fname = self._SetupImageInTmpdir() with test_util.capture_sys_output() as (stdout, stderr): self._RunBinman('ls', '-i', updated_fname) finally: - shutil.rmtree(tmpdir) + if tmpdir: + shutil.rmtree(tmpdir) def testFitSubentryUsesBintool(self): """Test that binman FIT subentries can use bintools""" From 2c714d682d896a24fe00498f09e3d624a5ee24b2 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 17 Dec 2023 09:36:14 -0700 Subject: [PATCH 03/24] x86: coral: Align bootph SPI-flash subnodes with parent The subnode has different tags from the parents, which is not correct. Fix the subnode. Signed-off-by: Simon Glass --- arch/x86/dts/chromebook_coral.dts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/x86/dts/chromebook_coral.dts b/arch/x86/dts/chromebook_coral.dts index 8bfb2c0d19d..2412801302e 100644 --- a/arch/x86/dts/chromebook_coral.dts +++ b/arch/x86/dts/chromebook_coral.dts @@ -369,12 +369,14 @@ rw-mrc-cache { label = "rw-mrc-cache"; reg = <0x008e0000 0x00010000>; - bootph-all; + bootph-some-ram; + bootph-pre-ram; }; rw-var-mrc-cache { label = "rw-mrc-cache"; reg = <0x008f0000 0x0001000>; - bootph-all; + bootph-some-ram; + bootph-pre-ram; }; }; }; From 233a61373bfc474bbf48e6e278f835818083ef84 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 17 Dec 2023 09:36:15 -0700 Subject: [PATCH 04/24] fdtgrep: Tidy up a few type warnings and comments Align the code with the upstream version at fdt-tools which had a few tweaks before being applied. Signed-off-by: Simon Glass --- tools/fdtgrep.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c index 7eabcab4399..f5493aaed3d 100644 --- a/tools/fdtgrep.c +++ b/tools/fdtgrep.c @@ -375,8 +375,9 @@ static int display_fdt_by_regions(struct display_info *disp, const void *blob, const char *str; int str_base = fdt_off_dt_strings(blob); - for (offset = 0; offset < fdt_size_dt_strings(blob); - offset += strlen(str) + 1) { + for (offset = 0; + offset < (int)fdt_size_dt_strings(blob); + offset += strlen(str) + 1) { str = fdt_string(blob, offset); int len = strlen(str) + 1; int show; @@ -431,7 +432,7 @@ static int dump_fdt_regions(struct display_info *disp, const void *blob, { struct fdt_header *fdt; int size, struct_start; - int ptr; + unsigned int ptr; int i; /* Set up a basic header (even if we don't actually write it) */ @@ -683,10 +684,10 @@ static int fdtgrep_find_regions(const void *fdt, return new_count; } else if (new_count <= max_regions) { /* - * The alias regions will now be at the end of the list. - * Sort the regions by offset to get things into the - * right order - */ + * The alias regions will now be at the end of the list. + * Sort the regions by offset to get things into the + * right order + */ count = new_count; qsort(region, count, sizeof(struct fdt_region), h_cmp_region); @@ -880,7 +881,7 @@ static int do_fdtgrep(struct display_info *disp, const char *filename) size = fdt_totalsize(fdt); } - if (size != fwrite(fdt, 1, size, disp->fout)) { + if ((size_t)size != fwrite(fdt, 1, size, disp->fout)) { fprintf(stderr, "Write failure, %d bytes\n", size); free(fdt); ret = 1; @@ -934,7 +935,7 @@ static const char usage_synopsis[] = static const char usage_short_opts[] = "haAc:b:C:defg:G:HIlLmn:N:o:O:p:P:rRsStTv" USAGE_COMMON_SHORT_OPTS; -static struct option const usage_long_opts[] = { +static const struct option usage_long_opts[] = { {"show-address", no_argument, NULL, 'a'}, {"colour", no_argument, NULL, 'A'}, {"include-node-with-prop", a_argument, NULL, 'b'}, From 9dab5bd3f8c4b9f09e1bcb7aefc090210d1fd52e Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 17 Dec 2023 09:36:16 -0700 Subject: [PATCH 05/24] fdtgrep: Correct ordering of flags Two of the flags are out of order, so fix this. Also adjust the ordering of one flag in the main switch() Signed-off-by: Simon Glass --- tools/fdtgrep.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c index f5493aaed3d..41d8b41252d 100644 --- a/tools/fdtgrep.c +++ b/tools/fdtgrep.c @@ -953,6 +953,8 @@ static const struct option usage_long_opts[] = { {"include-mem", no_argument, NULL, 'm'}, {"include-node", a_argument, NULL, 'n'}, {"exclude-node", a_argument, NULL, 'N'}, + {"out", a_argument, NULL, 'o'}, + {"out-format", a_argument, NULL, 'O'}, {"include-prop", a_argument, NULL, 'p'}, {"exclude-prop", a_argument, NULL, 'P'}, {"remove-strings", no_argument, NULL, 'r'}, @@ -961,8 +963,6 @@ static const struct option usage_long_opts[] = { {"skip-supernodes", no_argument, NULL, 'S'}, {"show-stringtab", no_argument, NULL, 't'}, {"show-aliases", no_argument, NULL, 'T'}, - {"out", a_argument, NULL, 'o'}, - {"out-format", a_argument, NULL, 'O'}, {"invert-match", no_argument, NULL, 'v'}, USAGE_COMMON_LONG_OPTS, }; @@ -984,6 +984,8 @@ static const char * const usage_opts_help[] = { "Include mem_rsvmap section in binary output", "Node to include in grep", "Node to exclude in grep", + "-o ", + "-O ", "Property to include in grep", "Property to exclude in grep", "Remove unused strings from string table", @@ -992,8 +994,6 @@ static const char * const usage_opts_help[] = { "Don't include supernodes of matching nodes", "Include string table in binary output", "Include matching aliases in output", - "-o ", - "-O ", "Invert the sense of matching (select non-matching lines)", USAGE_COMMON_OPTS_HELP }; @@ -1125,6 +1125,9 @@ static void scan_args(struct display_info *disp, int argc, char *argv[]) case 'H': disp->header = 1; break; + case 'I': + disp->show_dts_version = 1; + break; case 'l': disp->region_list = 1; break; @@ -1180,9 +1183,6 @@ static void scan_args(struct display_info *disp, int argc, char *argv[]) case 'v': disp->invert = 1; break; - case 'I': - disp->show_dts_version = 1; - break; } if (type && value_add(disp, &disp->value_head, type, inc, From f3acd206859ed516f79ba83da587196f66fe57d2 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 17 Dec 2023 09:36:17 -0700 Subject: [PATCH 06/24] fdtgrep: Correct references to fdt_find_regions() The function name is actually fdtgrep_find_regions() so update the name in comments accordinging. Signed-off-by: Simon Glass --- tools/fdtgrep.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c index 41d8b41252d..b56d2fe21c9 100644 --- a/tools/fdtgrep.c +++ b/tools/fdtgrep.c @@ -576,10 +576,10 @@ static int check_type_include(void *priv, int type, const char *data, int size) } /** - * h_include() - Include handler function for fdt_find_regions() + * h_include() - Include handler function for fdtgrep_find_regions() * * This function decides whether to include or exclude a node, property or - * compatible string. The function is defined by fdt_find_regions(). + * compatible string. The function is defined by fdtgrep_find_regions(). * * The algorithm is documented in the code - disp->invert is 0 for normal * operation, and 1 to invert the sense of all matches. @@ -822,7 +822,7 @@ static int do_fdtgrep(struct display_info *disp, const char *filename) region, max_regions, path, sizeof(path), disp->flags); if (count < 0) { - report_error("fdt_find_regions", count); + report_error("fdtgrep_find_regions", count); free(region); return -1; } From b1823ed1715910b1af68815a27c33c2c992e685c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 17 Dec 2023 09:36:18 -0700 Subject: [PATCH 07/24] fdtgrep: Tidy up comment for h_include() Copy the comment from fdt_first_region() so that it is clear what value this function returns. Signed-off-by: Simon Glass --- tools/fdtgrep.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c index b56d2fe21c9..a6cdc326709 100644 --- a/tools/fdtgrep.c +++ b/tools/fdtgrep.c @@ -576,15 +576,21 @@ static int check_type_include(void *priv, int type, const char *data, int size) } /** - * h_include() - Include handler function for fdtgrep_find_regions() + * h_include() - Include handler function for fdt_first_region() * * This function decides whether to include or exclude a node, property or - * compatible string. The function is defined by fdtgrep_find_regions(). + * compatible string. The function is defined by fdt_first_region(). * * The algorithm is documented in the code - disp->invert is 0 for normal * operation, and 1 to invert the sense of all matches. * - * See + * @priv: Private pointer as passed to fdtgrep_find_regions() + * @fdt: Pointer to FDT blob + * @offset: Offset of this node / property + * @type: Type of this part, FDT_IS_... + * @data: Pointer to data (node name, property name, compatible string) + * @size: Size of data, or 0 if none + * Return: 0 to exclude, 1 to include, -1 if no information is available */ static int h_include(void *priv, const void *fdt, int offset, int type, const char *data, int size) From 490afe74287fef246320c6473f74b2fc2a62c745 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 17 Dec 2023 09:36:19 -0700 Subject: [PATCH 08/24] fdtgrep: Simplify code to inverting the match The code to invert the match in h_include() is a bit convoluted. Simplify it by using disp->invert only once. Signed-off-by: Simon Glass --- tools/fdtgrep.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c index a6cdc326709..b06a1a7a838 100644 --- a/tools/fdtgrep.c +++ b/tools/fdtgrep.c @@ -634,14 +634,8 @@ static int h_include(void *priv, const void *fdt, int offset, int type, inc = 0; } - switch (inc) { - case 1: - inc = !disp->invert; - break; - case 0: - inc = disp->invert; - break; - } + if (inc != -1 && disp->invert) + inc = !inc; debug(" - returning %d\n", inc); return inc; From 61a695e451fb6abe65de193c70dd981af6fb7a51 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 17 Dec 2023 09:36:20 -0700 Subject: [PATCH 09/24] fdtgrep: Move property checking into a function The h_include() function includes a piece which checks if a node contains a property being searched for. Move this into its own function to reduce the size of the h_include() function. Signed-off-by: Simon Glass --- tools/fdtgrep.c | 48 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c index b06a1a7a838..ca639a2d9f4 100644 --- a/tools/fdtgrep.c +++ b/tools/fdtgrep.c @@ -575,6 +575,40 @@ static int check_type_include(void *priv, int type, const char *data, int size) return 0; } +/** + * check_props() - Check if a node has properties that we want to include + * + * Calls check_type_include() for each property in the nodn, returning 1 if + * that function returns 1 for any of them + * + * @disp: Display structure, holding info about our options + * @fdt: Devicetree blob to check + * @node: Node offset to check + * @inc: Current value of the 'include' variable (see h_include()) + * Return: 0 to exclude, 1 to include, -1 if no information is available + */ +static int check_props(struct display_info *disp, const void *fdt, int node, + int inc) +{ + int offset; + + for (offset = fdt_first_property_offset(fdt, node); + offset > 0 && inc != 1; + offset = fdt_next_property_offset(fdt, offset)) { + const struct fdt_property *prop; + const char *str; + + prop = fdt_get_property_by_offset(fdt, offset, NULL); + if (!prop) + continue; + str = fdt_string(fdt, fdt32_to_cpu(prop->nameoff)); + inc = check_type_include(disp, FDT_NODE_HAS_PROP, str, + strlen(str)); + } + + return inc; +} + /** * h_include() - Include handler function for fdt_first_region() * @@ -617,19 +651,7 @@ static int h_include(void *priv, const void *fdt, int offset, int type, (disp->types_inc & FDT_NODE_HAS_PROP)) { debug(" - checking node '%s'\n", fdt_get_name(fdt, offset, NULL)); - for (offset = fdt_first_property_offset(fdt, offset); - offset > 0 && inc != 1; - offset = fdt_next_property_offset(fdt, offset)) { - const struct fdt_property *prop; - const char *str; - - prop = fdt_get_property_by_offset(fdt, offset, NULL); - if (!prop) - continue; - str = fdt_string(fdt, fdt32_to_cpu(prop->nameoff)); - inc = check_type_include(priv, FDT_NODE_HAS_PROP, str, - strlen(str)); - } + inc = check_props(disp, fdt, offset, inc); if (inc == -1) inc = 0; } From 0b2e47be2c6b37e6026e36ed3a8656a7ed598ce1 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 17 Dec 2023 09:36:21 -0700 Subject: [PATCH 10/24] sandbox: Correct SPL condition for building devicetree With sandbox, CONFIG_SANDBOX is y so the current rule ends up building the devicetree for only those SPL builds where it is unwanted. Correct the condition. This allows sandbox_vpl to produce a u-boot-vpl.dtb file. Fixes: e7fb789612e ("sandbox: Remove OF_HOSTFILE") Signed-off-by: Simon Glass --- scripts/Makefile.spl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index e450ffd5d5e..407fc52376a 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -314,7 +314,7 @@ endif # - we have either OF_SEPARATE or OF_HOSTFILE build_dtb := ifneq ($(CONFIG_$(SPL_TPL_)OF_REAL),) -ifeq ($(CONFIG_OF_SEPARATE)$(CONFIG_SANDBOX),y) +ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_SANDBOX),) build_dtb := y endif endif From 7a06cc2027c0169c462da63a68fa269c0d59a950 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 17 Dec 2023 09:36:22 -0700 Subject: [PATCH 11/24] fdtgrep: Allow propagating properties up to supernodes The existing bootph binding is defined such that properties in a subnode are also implied in the supernode also, as in this example: buttons { /* bootph,pre-ram is implied by btn1 */ compatible = "gpio-keys"; btn1 { bootph,pre-ram; gpios = <&gpio_a 3 0>; label = "button1"; linux,code = ; }; Provide an option to implement this in fdtgrep. Signed-off-by: Simon Glass --- tools/fdtgrep.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c index ca639a2d9f4..f1ff1946bd4 100644 --- a/tools/fdtgrep.c +++ b/tools/fdtgrep.c @@ -63,6 +63,7 @@ struct display_info { int types_inc; /* Mask of types that we include (FDT_IS...) */ int types_exc; /* Mask of types that we exclude (FDT_IS...) */ int invert; /* Invert polarity of match */ + int props_up; /* Imply properties up to supernodes */ struct value_node *value_head; /* List of values to match */ const char *output_fname; /* Output filename */ FILE *fout; /* File to write dts/dtb output */ @@ -606,6 +607,16 @@ static int check_props(struct display_info *disp, const void *fdt, int node, strlen(str)); } + /* if requested, check all subnodes for this property too */ + if (inc != 1 && disp->props_up) { + int subnode; + + for (subnode = fdt_first_subnode(fdt, node); + subnode > 0 && inc != 1; + subnode = fdt_next_subnode(fdt, subnode)) + inc = check_props(disp, fdt, subnode, inc); + } + return inc; } @@ -955,7 +966,7 @@ static const char usage_synopsis[] = case '?': usage("unknown option"); static const char usage_short_opts[] = - "haAc:b:C:defg:G:HIlLmn:N:o:O:p:P:rRsStTv" + "haAc:b:C:defg:G:HIlLmn:N:o:O:p:P:rRsStTuv" USAGE_COMMON_SHORT_OPTS; static const struct option usage_long_opts[] = { {"show-address", no_argument, NULL, 'a'}, @@ -985,6 +996,7 @@ static const struct option usage_long_opts[] = { {"skip-supernodes", no_argument, NULL, 'S'}, {"show-stringtab", no_argument, NULL, 't'}, {"show-aliases", no_argument, NULL, 'T'}, + {"props-up-to-supernode", no_argument, NULL, 'u'}, {"invert-match", no_argument, NULL, 'v'}, USAGE_COMMON_LONG_OPTS, }; @@ -1016,6 +1028,7 @@ static const char * const usage_opts_help[] = { "Don't include supernodes of matching nodes", "Include string table in binary output", "Include matching aliases in output", + "Add -p properties to supernodes too", "Invert the sense of matching (select non-matching lines)", USAGE_COMMON_OPTS_HELP }; @@ -1202,6 +1215,9 @@ static void scan_args(struct display_info *disp, int argc, char *argv[]) case 'T': disp->add_aliases = 1; break; + case 'u': + disp->props_up = 1; + break; case 'v': disp->invert = 1; break; From aca95282c1b72c41d8e72984b1dceb15f396b2f8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 19 Dec 2023 07:21:25 -0700 Subject: [PATCH 12/24] Makefile: Use the fdtgrep -u flag Use this flag so that the bootph binding is obeyed correctly. Add a comment about what is going on. Signed-off-by: Simon Glass Fixes: https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/12 Reviewed-by: Tom Rini --- scripts/Makefile.lib | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 16bbc277a9f..1ca84195c99 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -635,8 +635,19 @@ else fdtgrep_props := -b bootph-all -b bootph-pre-ram $(migrate_spl) endif endif + +# This rule produces the .dtb for an SPL build. +# +# The first fdtgrep keeps nodes with the above properties (with -u ensuring that +# the properties are implied in all parents of a matching node). The root node +# is always included, along with /chosen and /config nodes. Referenced aliases +# (i.e. properties in /aliases which point to an incldued node) are also +# included. +# +# The second fdtgrep removes all bootph properties along with unused strings +# and any properties in CONFIG_OF_SPL_REMOVE_PROPS quiet_cmd_fdtgrep = FDTGREP $@ - cmd_fdtgrep = $(objtree)/tools/fdtgrep $(fdtgrep_props) -RT $< \ + cmd_fdtgrep = $(objtree)/tools/fdtgrep $(fdtgrep_props) -u -RT $< \ -n /chosen -n /config -O dtb | \ $(objtree)/tools/fdtgrep -r -O dtb - -o $@ \ -P bootph-all -P bootph-pre-ram -P bootph-pre-sram \ From e748e4b780057872b4a7192db87476adaa8b501c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 27 Dec 2023 13:06:59 -0800 Subject: [PATCH 13/24] bloblist: Update the tag numbering Align bloblist tags with the FW handoff spec v0.9. The most common ones are from 0. TF related ones are from 0x100. All non-standard ones from 0xfff000. Added new defined tags: BLOBLISTT_OPTEE_PAGABLE_PART for TF. BLOBLISTT_TPM_EVLOG and BLOBLISTT_TPM_CRB_BASE for TPM. Signed-off-by: Simon Glass Co-developed-by: Raymond Mao Signed-off-by: Raymond Mao Reviewed-by: Ilias Apalodimas --- common/bloblist.c | 18 ++++++++++--- include/bloblist.h | 67 +++++++++++++++++++++++++--------------------- test/bloblist.c | 4 +-- 3 files changed, 52 insertions(+), 37 deletions(-) diff --git a/common/bloblist.c b/common/bloblist.c index a22f6c12b0c..5606487f5bf 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -36,16 +36,26 @@ static struct tag_name { enum bloblist_tag_t tag; const char *name; } tag_name[] = { - { BLOBLISTT_NONE, "(none)" }, + { BLOBLISTT_VOID, "(void)" }, /* BLOBLISTT_AREA_FIRMWARE_TOP */ + { BLOBLISTT_CONTROL_FDT, "Control FDT" }, + { BLOBLISTT_HOB_BLOCK, "HOB block" }, + { BLOBLISTT_HOB_LIST, "HOB list" }, + { BLOBLISTT_ACPI_TABLES, "ACPI tables for x86" }, + { BLOBLISTT_TPM_EVLOG, "TPM event log defined by TCG EFI" }, + { BLOBLISTT_TPM_CRB_BASE, "TPM Command Response Buffer address" }, /* BLOBLISTT_AREA_FIRMWARE */ - { BLOBLISTT_ACPI_GNVS, "ACPI GNVS" }, - { BLOBLISTT_INTEL_VBT, "Intel Video-BIOS table" }, { BLOBLISTT_TPM2_TCG_LOG, "TPM v2 log space" }, { BLOBLISTT_TCPA_LOG, "TPM log space" }, - { BLOBLISTT_ACPI_TABLES, "ACPI tables for x86" }, + { BLOBLISTT_ACPI_GNVS, "ACPI GNVS" }, + + /* BLOBLISTT_AREA_TF */ + { BLOBLISTT_OPTEE_PAGABLE_PART, "OP-TEE pagable part" }, + + /* BLOBLISTT_AREA_OTHER */ + { BLOBLISTT_INTEL_VBT, "Intel Video-BIOS table" }, { BLOBLISTT_SMBIOS_TABLES, "SMBIOS tables for x86" }, { BLOBLISTT_VBOOT_CTX, "Chrome OS vboot context" }, diff --git a/include/bloblist.h b/include/bloblist.h index 080cc46a126..92dbfda21b4 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -81,7 +81,7 @@ enum { /* Supported tags - add new ones to tag_name in bloblist.c */ enum bloblist_tag_t { - BLOBLISTT_NONE = 0, + BLOBLISTT_VOID = 0, /* * Standard area to allocate blobs used across firmware components, for @@ -89,42 +89,36 @@ enum bloblist_tag_t { * projects. */ BLOBLISTT_AREA_FIRMWARE_TOP = 0x1, + /* + * Devicetree for use by firmware. On some platforms this is passed to + * the OS also + */ + BLOBLISTT_CONTROL_FDT = 1, + BLOBLISTT_HOB_BLOCK = 2, + BLOBLISTT_HOB_LIST = 3, + BLOBLISTT_ACPI_TABLES = 4, + BLOBLISTT_TPM_EVLOG = 5, + BLOBLISTT_TPM_CRB_BASE = 6, /* Standard area to allocate blobs used across firmware components */ - BLOBLISTT_AREA_FIRMWARE = 0x100, + BLOBLISTT_AREA_FIRMWARE = 0x10, + BLOBLISTT_TPM2_TCG_LOG = 0x10, /* TPM v2 log space */ + BLOBLISTT_TCPA_LOG = 0x11, /* TPM log space */ /* * Advanced Configuration and Power Interface Global Non-Volatile * Sleeping table. This forms part of the ACPI tables passed to Linux. */ - BLOBLISTT_ACPI_GNVS = 0x100, - BLOBLISTT_INTEL_VBT = 0x101, /* Intel Video-BIOS table */ - BLOBLISTT_TPM2_TCG_LOG = 0x102, /* TPM v2 log space */ - BLOBLISTT_TCPA_LOG = 0x103, /* TPM log space */ - BLOBLISTT_ACPI_TABLES = 0x104, /* ACPI tables for x86 */ - BLOBLISTT_SMBIOS_TABLES = 0x105, /* SMBIOS tables for x86 */ - BLOBLISTT_VBOOT_CTX = 0x106, /* Chromium OS verified boot context */ + BLOBLISTT_ACPI_GNVS = 0x12, - /* - * Project-specific tags are permitted here. Projects can be open source - * or not, but the format of the data must be fuily documented in an - * open source project, including all fields, bits, etc. Naming should - * be: BLOBLISTT__ - */ - BLOBLISTT_PROJECT_AREA = 0x8000, - BLOBLISTT_U_BOOT_SPL_HANDOFF = 0x8000, /* Hand-off info from SPL */ - BLOBLISTT_VBE = 0x8001, /* VBE per-phase state */ - BLOBLISTT_U_BOOT_VIDEO = 0x8002, /* Video information from SPL */ + /* Standard area to allocate blobs used for Trusted Firmware */ + BLOBLISTT_AREA_TF = 0x100, + BLOBLISTT_OPTEE_PAGABLE_PART = 0x100, - /* - * Vendor-specific tags are permitted here. Projects can be open source - * or not, but the format of the data must be fuily documented in an - * open source project, including all fields, bits, etc. Naming should - * be BLOBLISTT__ - */ - BLOBLISTT_VENDOR_AREA = 0xc000, - - /* Tags after this are not allocated for now */ - BLOBLISTT_EXPANSION = 0x10000, + /* Other standard area to allocate blobs */ + BLOBLISTT_AREA_OTHER = 0x200, + BLOBLISTT_INTEL_VBT = 0x200, /* Intel Video-BIOS table */ + BLOBLISTT_SMBIOS_TABLES = 0x201, /* SMBIOS tables for x86 */ + BLOBLISTT_VBOOT_CTX = 0x202, /* Chromium OS verified boot context */ /* * Tags from here are on reserved for private use within a single @@ -133,9 +127,20 @@ enum bloblist_tag_t { * implementation, but cannot be used in upstream code. Allocate a * tag in one of the areas above if you want that. * - * This area may move in future. + * Project-specific tags are permitted here. Projects can be open source + * or not, but the format of the data must be fuily documented in an + * open source project, including all fields, bits, etc. Naming should + * be: BLOBLISTT__ + * + * Vendor-specific tags are also permitted. Projects can be open source + * or not, but the format of the data must be fuily documented in an + * open source project, including all fields, bits, etc. Naming should + * be BLOBLISTT__ */ - BLOBLISTT_PRIVATE_AREA = 0xffff0000, + BLOBLISTT_PRIVATE_AREA = 0xfff000, + BLOBLISTT_U_BOOT_SPL_HANDOFF = 0xfff000, /* Hand-off info from SPL */ + BLOBLISTT_VBE = 0xfff001, /* VBE per-phase state */ + BLOBLISTT_U_BOOT_VIDEO = 0xfff002, /* Video info from SPL */ }; /** diff --git a/test/bloblist.c b/test/bloblist.c index 720be7e244f..efa1e32afd7 100644 --- a/test/bloblist.c +++ b/test/bloblist.c @@ -291,9 +291,9 @@ static int bloblist_test_cmd_list(struct unit_test_state *uts) console_record_reset(); run_command("bloblist list", 0); ut_assert_nextline("Address Size Tag Name"); - ut_assert_nextline("%08lx %8x 8000 SPL hand-off", + ut_assert_nextline("%08lx %8x fff000 SPL hand-off", (ulong)map_to_sysmem(data), TEST_SIZE); - ut_assert_nextline("%08lx %8x 106 Chrome OS vboot context", + ut_assert_nextline("%08lx %8x 202 Chrome OS vboot context", (ulong)map_to_sysmem(data2), TEST_SIZE2); ut_assert_console_end(); ut_unsilence_console(uts); From 1a2e02f955f98395142415d7c6cc14e4df903969 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 27 Dec 2023 13:07:00 -0800 Subject: [PATCH 14/24] bloblist: Adjust API to align in powers of 2 The updated bloblist structure stores the alignment as a power-of-two value in its structures. Adjust the API to use this, to avoid needing to calling ilog2(). Update the bloblist alignment from 16 bytes to 8 bytes. Drop a stale comment while we are here. Signed-off-by: Simon Glass Co-developed-by: Raymond Mao Signed-off-by: Raymond Mao Reviewed-by: Simon Glass Reviewed-by: Ilias Apalodimas --- arch/x86/lib/tables.c | 3 ++- common/bloblist.c | 24 +++++++++++------------- include/bloblist.h | 12 +++++++----- test/bloblist.c | 4 ++-- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c index 5b5070f7ca5..d43e77d3730 100644 --- a/arch/x86/lib/tables.c +++ b/arch/x86/lib/tables.c @@ -16,6 +16,7 @@ #include #include #include +#include DECLARE_GLOBAL_DATA_PTR; @@ -104,7 +105,7 @@ int write_tables(void) if (!gd->arch.table_end) gd->arch.table_end = rom_addr; rom_addr = (ulong)bloblist_add(table->tag, size, - table->align); + ilog2(table->align)); if (!rom_addr) return log_msg_ret("bloblist", -ENOBUFS); diff --git a/common/bloblist.c b/common/bloblist.c index 5606487f5bf..1e78a3d4b3e 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -26,8 +26,6 @@ * start address of the data in each blob is aligned as required. Note that * each blob's *data* is aligned to BLOBLIST_ALIGN regardless of the alignment * of the bloblist itself or the blob header. - * - * So far, only BLOBLIST_ALIGN alignment is supported. */ DECLARE_GLOBAL_DATA_PTR; @@ -128,24 +126,24 @@ static struct bloblist_rec *bloblist_findrec(uint tag) return NULL; } -static int bloblist_addrec(uint tag, int size, int align, +static int bloblist_addrec(uint tag, int size, int align_log2, struct bloblist_rec **recp) { struct bloblist_hdr *hdr = gd->bloblist; struct bloblist_rec *rec; int data_start, new_alloced; - if (!align) - align = BLOBLIST_ALIGN; + if (!align_log2) + align_log2 = BLOBLIST_ALIGN_LOG2; /* Figure out where the new data will start */ data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*rec); /* Align the address and then calculate the offset from ->alloced */ - data_start = ALIGN(data_start, align) - map_to_sysmem(hdr); + data_start = ALIGN(data_start, 1U << align_log2) - map_to_sysmem(hdr); /* Calculate the new allocated total */ - new_alloced = data_start + ALIGN(size, align); + new_alloced = data_start + ALIGN(size, 1U << align_log2); if (new_alloced > hdr->size) { log_err("Failed to allocate %x bytes size=%x, need size=%x\n", @@ -169,7 +167,7 @@ static int bloblist_addrec(uint tag, int size, int align, } static int bloblist_ensurerec(uint tag, struct bloblist_rec **recp, int size, - int align) + int align_log2) { struct bloblist_rec *rec; @@ -182,7 +180,7 @@ static int bloblist_ensurerec(uint tag, struct bloblist_rec **recp, int size, } else { int ret; - ret = bloblist_addrec(tag, size, align, &rec); + ret = bloblist_addrec(tag, size, align_log2, &rec); if (ret) return ret; } @@ -204,22 +202,22 @@ void *bloblist_find(uint tag, int size) return (void *)rec + rec->hdr_size; } -void *bloblist_add(uint tag, int size, int align) +void *bloblist_add(uint tag, int size, int align_log2) { struct bloblist_rec *rec; - if (bloblist_addrec(tag, size, align, &rec)) + if (bloblist_addrec(tag, size, align_log2, &rec)) return NULL; return (void *)rec + rec->hdr_size; } -int bloblist_ensure_size(uint tag, int size, int align, void **blobp) +int bloblist_ensure_size(uint tag, int size, int align_log2, void **blobp) { struct bloblist_rec *rec; int ret; - ret = bloblist_ensurerec(tag, &rec, size, align); + ret = bloblist_ensurerec(tag, &rec, size, align_log2); if (ret) return ret; *blobp = (void *)rec + rec->hdr_size; diff --git a/include/bloblist.h b/include/bloblist.h index 92dbfda21b4..5ad1337d1fb 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -76,7 +76,9 @@ enum { BLOBLIST_VERSION = 0, BLOBLIST_MAGIC = 0xb00757a3, - BLOBLIST_ALIGN = 16, + + BLOBLIST_ALIGN_LOG2 = 3, + BLOBLIST_ALIGN = 1 << BLOBLIST_ALIGN_LOG2, }; /* Supported tags - add new ones to tag_name in bloblist.c */ @@ -254,11 +256,11 @@ void *bloblist_find(uint tag, int size); * * @tag: Tag to add (enum bloblist_tag_t) * @size: Size of the blob - * @align: Alignment of the blob (in bytes), 0 for default + * @align_log2: Alignment of the blob (in bytes log2), 0 for default * Return: pointer to the newly added block, or NULL if there is not enough * space for the blob */ -void *bloblist_add(uint tag, int size, int align); +void *bloblist_add(uint tag, int size, int align_log2); /** * bloblist_ensure_size() - Find or add a blob @@ -268,11 +270,11 @@ void *bloblist_add(uint tag, int size, int align); * @tag: Tag to add (enum bloblist_tag_t) * @size: Size of the blob * @blobp: Returns a pointer to blob on success - * @align: Alignment of the blob (in bytes), 0 for default + * @align_log2: Alignment of the blob (in bytes log2), 0 for default * Return: 0 if OK, -ENOSPC if it is missing and could not be added due to lack * of space, or -ESPIPE it exists but has the wrong size */ -int bloblist_ensure_size(uint tag, int size, int align, void **blobp); +int bloblist_ensure_size(uint tag, int size, int align_log2, void **blobp); /** * bloblist_ensure() - Find or add a blob diff --git a/test/bloblist.c b/test/bloblist.c index efa1e32afd7..8b435e27ca3 100644 --- a/test/bloblist.c +++ b/test/bloblist.c @@ -336,7 +336,7 @@ static int bloblist_test_align(struct unit_test_state *uts) /* Check larger alignment */ for (i = 0; i < 3; i++) { - int align = 32 << i; + int align = 5 - i; data = bloblist_add(3 + i, i * 4, align); ut_assertnonnull(data); @@ -351,7 +351,7 @@ static int bloblist_test_align(struct unit_test_state *uts) ut_assertok(bloblist_new(TEST_ADDR + BLOBLIST_ALIGN, TEST_BLOBLIST_SIZE, 0)); - data = bloblist_add(1, 5, BLOBLIST_ALIGN * 2); + data = bloblist_add(1, 5, BLOBLIST_ALIGN_LOG2 + 1); ut_assertnonnull(data); addr = map_to_sysmem(data); ut_asserteq(0, addr & (BLOBLIST_ALIGN * 2 - 1)); From 5a53e56011f11d2abbc23217a979c5b42274056c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 27 Dec 2023 13:07:01 -0800 Subject: [PATCH 15/24] bloblist: Change the magic value This uses a new value with spec v0.9 so change it. Signed-off-by: Simon Glass Co-developed-by: Raymond Mao Signed-off-by: Raymond Mao Reviewed-by: Ilias Apalodimas Reviewed-by: Simon Glass --- include/bloblist.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/bloblist.h b/include/bloblist.h index 5ad1337d1fb..72c785411d0 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -75,7 +75,7 @@ enum { BLOBLIST_VERSION = 0, - BLOBLIST_MAGIC = 0xb00757a3, + BLOBLIST_MAGIC = 0x4a0fb10b, BLOBLIST_ALIGN_LOG2 = 3, BLOBLIST_ALIGN = 1 << BLOBLIST_ALIGN_LOG2, From 0b9f77f140483569e2ed4622704ef23fef527df8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 27 Dec 2023 13:07:02 -0800 Subject: [PATCH 16/24] bloblist: Set version to 1 The new bloblist for v0.9 has version 1 so update this value. Signed-off-by: Simon Glass Co-developed-by: Raymond Mao Signed-off-by: Raymond Mao Reviewed-by: Ilias Apalodimas Reviewed-by: Simon Glass --- include/bloblist.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/bloblist.h b/include/bloblist.h index 72c785411d0..7eff709ec8d 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -74,7 +74,7 @@ #include enum { - BLOBLIST_VERSION = 0, + BLOBLIST_VERSION = 1, BLOBLIST_MAGIC = 0x4a0fb10b, BLOBLIST_ALIGN_LOG2 = 3, From 1f06ed41cc18943e379894c5a3c3f4e6b9de73e3 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 27 Dec 2023 13:07:03 -0800 Subject: [PATCH 17/24] bloblist: Access record hdr_size and tag via a function Convert accesses to tag and hdr_size via function for grouping tag and hdr_size together later. Signed-off-by: Simon Glass Co-developed-by: Raymond Mao Signed-off-by: Raymond Mao Reviewed-by: Ilias Apalodimas --- common/bloblist.c | 38 +++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/common/bloblist.c b/common/bloblist.c index 1e78a3d4b3e..168993e0a7b 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -84,13 +84,23 @@ static struct bloblist_rec *bloblist_first_blob(struct bloblist_hdr *hdr) return (struct bloblist_rec *)((void *)hdr + hdr->hdr_size); } +static inline uint rec_hdr_size(struct bloblist_rec *rec) +{ + return rec->hdr_size; +} + +static inline uint rec_tag(struct bloblist_rec *rec) +{ + return rec->tag; +} + static ulong bloblist_blob_end_ofs(struct bloblist_hdr *hdr, struct bloblist_rec *rec) { ulong offset; offset = (void *)rec - (void *)hdr; - offset += rec->hdr_size + ALIGN(rec->size, BLOBLIST_ALIGN); + offset += rec_hdr_size(rec) + ALIGN(rec->size, BLOBLIST_ALIGN); return offset; } @@ -119,7 +129,7 @@ static struct bloblist_rec *bloblist_findrec(uint tag) return NULL; foreach_rec(rec, hdr) { - if (rec->tag == tag) + if (rec_tag(rec) == tag) return rec; } @@ -158,7 +168,7 @@ static int bloblist_addrec(uint tag, int size, int align_log2, rec->spare = 0; /* Zero the record data */ - memset((void *)rec + rec->hdr_size, '\0', rec->size); + memset((void *)rec + rec_hdr_size(rec), '\0', rec->size); hdr->alloced = new_alloced; *recp = rec; @@ -199,7 +209,7 @@ void *bloblist_find(uint tag, int size) if (size && size != rec->size) return NULL; - return (void *)rec + rec->hdr_size; + return (void *)rec + rec_hdr_size(rec); } void *bloblist_add(uint tag, int size, int align_log2) @@ -209,7 +219,7 @@ void *bloblist_add(uint tag, int size, int align_log2) if (bloblist_addrec(tag, size, align_log2, &rec)) return NULL; - return (void *)rec + rec->hdr_size; + return (void *)rec + rec_hdr_size(rec); } int bloblist_ensure_size(uint tag, int size, int align_log2, void **blobp) @@ -220,7 +230,7 @@ int bloblist_ensure_size(uint tag, int size, int align_log2, void **blobp) ret = bloblist_ensurerec(tag, &rec, size, align_log2); if (ret) return ret; - *blobp = (void *)rec + rec->hdr_size; + *blobp = (void *)rec + rec_hdr_size(rec); return 0; } @@ -232,7 +242,7 @@ void *bloblist_ensure(uint tag, int size) if (bloblist_ensurerec(tag, &rec, size, 0)) return NULL; - return (void *)rec + rec->hdr_size; + return (void *)rec + rec_hdr_size(rec); } int bloblist_ensure_size_ret(uint tag, int *sizep, void **blobp) @@ -245,7 +255,7 @@ int bloblist_ensure_size_ret(uint tag, int *sizep, void **blobp) *sizep = rec->size; else if (ret) return ret; - *blobp = (void *)rec + rec->hdr_size; + *blobp = (void *)rec + rec_hdr_size(rec); return 0; } @@ -281,7 +291,7 @@ static int bloblist_resize_rec(struct bloblist_hdr *hdr, /* Zero the new part of the blob */ if (expand_by > 0) { - memset((void *)rec + rec->hdr_size + rec->size, '\0', + memset((void *)rec + rec_hdr_size(rec) + rec->size, '\0', new_size - rec->size); } @@ -315,8 +325,9 @@ static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr) chksum = crc32(0, (unsigned char *)hdr, offsetof(struct bloblist_hdr, chksum)); foreach_rec(rec, hdr) { - chksum = crc32(chksum, (void *)rec, rec->hdr_size); - chksum = crc32(chksum, (void *)rec + rec->hdr_size, rec->size); + chksum = crc32(chksum, (void *)rec, rec_hdr_size(rec)); + chksum = crc32(chksum, (void *)rec + rec_hdr_size(rec), + rec->size); } return chksum; @@ -424,8 +435,9 @@ void bloblist_show_list(void) for (rec = bloblist_first_blob(hdr); rec; rec = bloblist_next_blob(hdr, rec)) { printf("%08lx %8x %4x %s\n", - (ulong)map_to_sysmem((void *)rec + rec->hdr_size), - rec->size, rec->tag, bloblist_tag_name(rec->tag)); + (ulong)map_to_sysmem((void *)rec + rec_hdr_size(rec)), + rec->size, rec_tag(rec), + bloblist_tag_name(rec_tag(rec))); } } From 47e1047b0cc25269124737696971ab0c3187666d Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 27 Dec 2023 13:07:04 -0800 Subject: [PATCH 18/24] bloblist: Drop spare value from bloblist record Drop spare value from bloblist record header. For now it is still present in the header, with an underscore, so that tests continue to pass. Signed-off-by: Simon Glass Co-developed-by: Raymond Mao Signed-off-by: Raymond Mao --- common/bloblist.c | 1 - include/bloblist.h | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/common/bloblist.c b/common/bloblist.c index 168993e0a7b..88e2a0f5c06 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -165,7 +165,6 @@ static int bloblist_addrec(uint tag, int size, int align_log2, rec->tag = tag; rec->hdr_size = data_start - hdr->alloced; rec->size = size; - rec->spare = 0; /* Zero the record data */ memset((void *)rec + rec_hdr_size(rec), '\0', rec->size); diff --git a/include/bloblist.h b/include/bloblist.h index 7eff709ec8d..68f97395b78 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -205,13 +205,12 @@ struct bloblist_hdr { * record's data starts at this offset from the start of the record * @size: Size of record in bytes, excluding the header size. This does not * need to be aligned (e.g. 3 is OK). - * @spare: Spare space for other things */ struct bloblist_rec { u32 tag; u32 hdr_size; u32 size; - u32 spare; + u32 _spare; }; /** From 997dac6edebe5b82f4d6f9b90753f0af6e9d04f0 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 27 Dec 2023 13:07:05 -0800 Subject: [PATCH 19/24] bloblist: Checksum the entire bloblist Use a sinple 8-bit checksum for bloblist, as specified by the spec version 0.9. Spec v0.9 specifies that the entire bloblist area is checksummed, including unused portions. Update the code to follow this. Signed-off-by: Simon Glass Co-developed-by: Raymond Mao Signed-off-by: Raymond Mao Reviewed-by: Ilias Apalodimas --- common/bloblist.c | 13 ++++--------- include/bloblist.h | 5 ++--- test/bloblist.c | 10 ++++++++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/common/bloblist.c b/common/bloblist.c index 88e2a0f5c06..705d9c6ae99 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -318,16 +319,10 @@ int bloblist_resize(uint tag, int new_size) static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr) { - struct bloblist_rec *rec; - u32 chksum; + u8 chksum; - chksum = crc32(0, (unsigned char *)hdr, - offsetof(struct bloblist_hdr, chksum)); - foreach_rec(rec, hdr) { - chksum = crc32(chksum, (void *)rec, rec_hdr_size(rec)); - chksum = crc32(chksum, (void *)rec + rec_hdr_size(rec), - rec->size); - } + chksum = table_compute_checksum(hdr, hdr->alloced); + chksum += hdr->chksum; return chksum; } diff --git a/include/bloblist.h b/include/bloblist.h index 68f97395b78..d2dcad69a1b 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -174,11 +174,10 @@ enum bloblist_tag_t { * sizeof(bloblist_hdr) since we need at least that much space to store a * valid bloblist * @spare: Spare space (for future use) - * @chksum: CRC32 for the entire bloblist allocated area. Since any of the + * @chksum: checksum for the entire bloblist allocated area. Since any of the * blobs can be altered after being created, this checksum is only valid * when the bloblist is finalised before jumping to the next stage of boot. - * Note that chksum is last to make it easier to exclude it from the - * checksum calculation. + * This is the value needed to make all checksummed bytes sum to 0 */ struct bloblist_hdr { u32 magic; diff --git a/test/bloblist.c b/test/bloblist.c index 8b435e27ca3..49ac4b92aee 100644 --- a/test/bloblist.c +++ b/test/bloblist.c @@ -237,12 +237,18 @@ static int bloblist_test_checksum(struct unit_test_state *uts) *data2 -= 1; /* - * Changing data outside the range of valid data should not affect - * the checksum. + * Changing data outside the range of valid data should affect the + * checksum. */ ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); data[TEST_SIZE]++; + ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); + data[TEST_SIZE]--; + ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); + data2[TEST_SIZE2]++; + ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); + data[TEST_SIZE]--; ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); return 0; From f9ef9fb033d5ea29d0a72349fea9d0e55a528d36 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 27 Dec 2023 13:07:06 -0800 Subject: [PATCH 20/24] bloblist: Handle alignment with a void entry Rather than setting the alignment using the header size, add an entirely new entry to cover the gap left by the alignment. Signed-off-by: Simon Glass Co-developed-by: Raymond Mao Signed-off-by: Raymond Mao Reviewed-by: Simon Glass Reviewed-by: Ilias Apalodimas --- common/bloblist.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/common/bloblist.c b/common/bloblist.c index 705d9c6ae99..73dbbc01c08 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -142,7 +142,7 @@ static int bloblist_addrec(uint tag, int size, int align_log2, { struct bloblist_hdr *hdr = gd->bloblist; struct bloblist_rec *rec; - int data_start, new_alloced; + int data_start, aligned_start, new_alloced; if (!align_log2) align_log2 = BLOBLIST_ALIGN_LOG2; @@ -151,10 +151,25 @@ static int bloblist_addrec(uint tag, int size, int align_log2, data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*rec); /* Align the address and then calculate the offset from ->alloced */ - data_start = ALIGN(data_start, 1U << align_log2) - map_to_sysmem(hdr); + aligned_start = ALIGN(data_start, 1U << align_log2) - data_start; + + /* If we need to create a dummy record, create it */ + if (aligned_start) { + int void_size = aligned_start - sizeof(*rec); + struct bloblist_rec *vrec; + int ret; + + ret = bloblist_addrec(BLOBLISTT_VOID, void_size, 0, &vrec); + if (ret) + return log_msg_ret("void", ret); + + /* start the record after that */ + data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*vrec); + } /* Calculate the new allocated total */ - new_alloced = data_start + ALIGN(size, 1U << align_log2); + new_alloced = data_start - map_to_sysmem(hdr) + + ALIGN(size, 1U << align_log2); if (new_alloced > hdr->size) { log_err("Failed to allocate %x bytes size=%x, need size=%x\n", @@ -164,7 +179,7 @@ static int bloblist_addrec(uint tag, int size, int align_log2, rec = (void *)hdr + hdr->alloced; rec->tag = tag; - rec->hdr_size = data_start - hdr->alloced; + rec->hdr_size = sizeof(struct bloblist_rec); rec->size = size; /* Zero the record data */ From b6e83826ef1f4d04d350e4d2c03e3b28ab1b0ae4 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 27 Dec 2023 13:07:07 -0800 Subject: [PATCH 21/24] bloblist: Reduce blob-header size The v0.9 spec provides for an 8-byte header for each blob, with fewer fields. The blob data start address should be aligned to the alignment specified by the bloblist header. Update the implementation to match this. Signed-off-by: Simon Glass Co-developed-by: Raymond Mao Signed-off-by: Raymond Mao --- common/bloblist.c | 23 +++++++++++++++-------- include/bloblist.h | 33 ++++++++++++++++++++++----------- test/bloblist.c | 16 ++++++++-------- 3 files changed, 45 insertions(+), 27 deletions(-) diff --git a/common/bloblist.c b/common/bloblist.c index 73dbbc01c08..1c97d61e4aa 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -87,12 +87,14 @@ static struct bloblist_rec *bloblist_first_blob(struct bloblist_hdr *hdr) static inline uint rec_hdr_size(struct bloblist_rec *rec) { - return rec->hdr_size; + return (rec->tag_and_hdr_size & BLOBLISTR_HDR_SIZE_MASK) >> + BLOBLISTR_HDR_SIZE_SHIFT; } static inline uint rec_tag(struct bloblist_rec *rec) { - return rec->tag; + return (rec->tag_and_hdr_size & BLOBLISTR_TAG_MASK) >> + BLOBLISTR_TAG_SHIFT; } static ulong bloblist_blob_end_ofs(struct bloblist_hdr *hdr, @@ -101,7 +103,13 @@ static ulong bloblist_blob_end_ofs(struct bloblist_hdr *hdr, ulong offset; offset = (void *)rec - (void *)hdr; - offset += rec_hdr_size(rec) + ALIGN(rec->size, BLOBLIST_ALIGN); + /* + * The data section of next TE should start from an address aligned + * to 1 << hdr->align_log2. + */ + offset += rec_hdr_size(rec) + rec->size; + offset = round_up(offset + rec_hdr_size(rec), 1 << hdr->align_log2); + offset -= rec_hdr_size(rec); return offset; } @@ -145,7 +153,7 @@ static int bloblist_addrec(uint tag, int size, int align_log2, int data_start, aligned_start, new_alloced; if (!align_log2) - align_log2 = BLOBLIST_ALIGN_LOG2; + align_log2 = BLOBLIST_BLOB_ALIGN_LOG2; /* Figure out where the new data will start */ data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*rec); @@ -178,8 +186,7 @@ static int bloblist_addrec(uint tag, int size, int align_log2, } rec = (void *)hdr + hdr->alloced; - rec->tag = tag; - rec->hdr_size = sizeof(struct bloblist_rec); + rec->tag_and_hdr_size = tag | sizeof(*rec) << BLOBLISTR_HDR_SIZE_SHIFT; rec->size = size; /* Zero the record data */ @@ -283,8 +290,8 @@ static int bloblist_resize_rec(struct bloblist_hdr *hdr, int new_alloced; /* New value for @hdr->alloced */ ulong next_ofs; /* Offset of the record after @rec */ - expand_by = ALIGN(new_size - rec->size, BLOBLIST_ALIGN); - new_alloced = ALIGN(hdr->alloced + expand_by, BLOBLIST_ALIGN); + expand_by = ALIGN(new_size - rec->size, BLOBLIST_BLOB_ALIGN); + new_alloced = ALIGN(hdr->alloced + expand_by, BLOBLIST_BLOB_ALIGN); if (new_size < 0) { log_debug("Attempt to shrink blob size below 0 (%x)\n", new_size); diff --git a/include/bloblist.h b/include/bloblist.h index d2dcad69a1b..7024d7bf9e5 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -24,11 +24,11 @@ * which would add to code size. For Thumb-2 the code size needed in SPL is * approximately 940 bytes (e.g. for chromebook_bob). * - * 5. Bloblist uses 16-byte alignment internally and is designed to start on a - * 16-byte boundary. Its headers are multiples of 16 bytes. This makes it easier - * to deal with data structures which need this level of alignment, such as ACPI - * tables. For use in SPL and TPL the alignment can be relaxed, since it can be - * relocated to an aligned address in U-Boot proper. + * 5. Bloblist uses 8-byte alignment internally and is designed to start on a + * 8-byte boundary. Its headers are 8 bytes long. It is possible to achieve + * larger alignment (e.g. 16 bytes) by adding a dummy header, For use in SPL and + * TPL the alignment can be relaxed, since it can be relocated to an aligned + * address in U-Boot proper. * * 6. Bloblist is designed to be passed to Linux as reserved memory. While linux * doesn't understand the bloblist header, it can be passed the indivdual blobs. @@ -77,6 +77,9 @@ enum { BLOBLIST_VERSION = 1, BLOBLIST_MAGIC = 0x4a0fb10b, + BLOBLIST_BLOB_ALIGN_LOG2 = 3, + BLOBLIST_BLOB_ALIGN = 1 << BLOBLIST_BLOB_ALIGN_LOG2, + BLOBLIST_ALIGN_LOG2 = 3, BLOBLIST_ALIGN = 1 << BLOBLIST_ALIGN_LOG2, }; @@ -199,17 +202,25 @@ struct bloblist_hdr { * * NOTE: Only exported for testing purposes. Do not use this struct. * - * @tag: Tag indicating what the record contains - * @hdr_size: Size of this header, normally sizeof(struct bloblist_rec). The - * record's data starts at this offset from the start of the record + * @tag_and_hdr_size: Tag indicating what the record contains (bottom 24 bits), and + * size of this header (top 8 bits), normally sizeof(struct bloblist_rec). + * The record's data starts at this offset from the start of the record * @size: Size of record in bytes, excluding the header size. This does not * need to be aligned (e.g. 3 is OK). */ struct bloblist_rec { - u32 tag; - u32 hdr_size; + u32 tag_and_hdr_size; u32 size; - u32 _spare; +}; + +enum { + BLOBLISTR_TAG_SHIFT = 0, + BLOBLISTR_TAG_MASK = 0xffffffU << BLOBLISTR_TAG_SHIFT, + BLOBLISTR_HDR_SIZE_SHIFT = 24, + BLOBLISTR_HDR_SIZE_MASK = 0xffU << BLOBLISTR_HDR_SIZE_SHIFT, + + BLOBLIST_HDR_SIZE = sizeof(struct bloblist_hdr), + BLOBLIST_REC_HDR_SIZE = sizeof(struct bloblist_rec), }; /** diff --git a/test/bloblist.c b/test/bloblist.c index 49ac4b92aee..e6070041d36 100644 --- a/test/bloblist.c +++ b/test/bloblist.c @@ -272,8 +272,8 @@ static int bloblist_test_cmd_info(struct unit_test_state *uts) run_command("bloblist info", 0); ut_assert_nextline("base: %lx", (ulong)map_to_sysmem(hdr)); ut_assert_nextline("size: 400 1 KiB"); - ut_assert_nextline("alloced: 70 112 Bytes"); - ut_assert_nextline("free: 390 912 Bytes"); + ut_assert_nextline("alloced: 58 88 Bytes"); + ut_assert_nextline("free: 3a8 936 Bytes"); ut_assert_console_end(); ut_unsilence_console(uts); @@ -331,12 +331,12 @@ static int bloblist_test_align(struct unit_test_state *uts) data = bloblist_add(i, size, 0); ut_assertnonnull(data); addr = map_to_sysmem(data); - ut_asserteq(0, addr & (BLOBLIST_ALIGN - 1)); + ut_asserteq(0, addr & (BLOBLIST_BLOB_ALIGN - 1)); /* Only the bytes in the blob data should be zeroed */ for (j = 0; j < size; j++) ut_asserteq(0, data[j]); - for (; j < BLOBLIST_ALIGN; j++) + for (; j < BLOBLIST_BLOB_ALIGN; j++) ut_asserteq(ERASE_BYTE, data[j]); } @@ -351,7 +351,7 @@ static int bloblist_test_align(struct unit_test_state *uts) } /* Check alignment with an bloblist starting on a smaller alignment */ - hdr = map_sysmem(TEST_ADDR + BLOBLIST_ALIGN, TEST_BLOBLIST_SIZE); + hdr = map_sysmem(TEST_ADDR + BLOBLIST_BLOB_ALIGN, TEST_BLOBLIST_SIZE); memset(hdr, ERASE_BYTE, TEST_BLOBLIST_SIZE); memset(hdr, '\0', sizeof(*hdr)); ut_assertok(bloblist_new(TEST_ADDR + BLOBLIST_ALIGN, TEST_BLOBLIST_SIZE, @@ -360,7 +360,7 @@ static int bloblist_test_align(struct unit_test_state *uts) data = bloblist_add(1, 5, BLOBLIST_ALIGN_LOG2 + 1); ut_assertnonnull(data); addr = map_to_sysmem(data); - ut_asserteq(0, addr & (BLOBLIST_ALIGN * 2 - 1)); + ut_asserteq(0, addr & (BLOBLIST_BLOB_ALIGN * 2 - 1)); return 0; } @@ -448,7 +448,7 @@ static int bloblist_test_grow(struct unit_test_state *uts) hdr = ptr; ut_asserteq(sizeof(struct bloblist_hdr) + sizeof(struct bloblist_rec) * 2 + small_size * 2 + - BLOBLIST_ALIGN, + BLOBLIST_BLOB_ALIGN, hdr->alloced); return 0; @@ -583,7 +583,7 @@ static int bloblist_test_resize_last(struct unit_test_state *uts) ut_asserteq((u8)ERASE_BYTE, *((u8 *)hdr + alloced_val + 4)); /* Check that the new top of the allocated blobs has not been touched */ - alloced_val += BLOBLIST_ALIGN; + alloced_val += BLOBLIST_BLOB_ALIGN; ut_asserteq(alloced_val, hdr->alloced); ut_asserteq((u8)ERASE_BYTE, *((u8 *)hdr + hdr->alloced)); From b86b2d940caf27aa80b7c657e48770349e15491b Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 27 Dec 2023 13:07:08 -0800 Subject: [PATCH 22/24] bloblist: Adjust the bloblist header The v0.9 spec provides for a 24-byte header. Update the implementation to match this. Rename the fields of the bloblist header to align to the spec. Adds an alignment field into the bloblist header. Update the related bloblist APIs and UT testcases. Signed-off-by: Simon Glass Co-developed-by: Raymond Mao Signed-off-by: Raymond Mao Reviewed-by: Ilias Apalodimas --- common/bloblist.c | 86 +++++++++++++++++++++++++++------------------- include/bloblist.h | 44 ++++++++++++++---------- test/bloblist.c | 37 ++++++++++---------- 3 files changed, 95 insertions(+), 72 deletions(-) diff --git a/common/bloblist.c b/common/bloblist.c index 1c97d61e4aa..6e019087ff9 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -80,7 +80,7 @@ const char *bloblist_tag_name(enum bloblist_tag_t tag) static struct bloblist_rec *bloblist_first_blob(struct bloblist_hdr *hdr) { - if (hdr->alloced <= hdr->hdr_size) + if (hdr->used_size <= hdr->hdr_size) return NULL; return (struct bloblist_rec *)((void *)hdr + hdr->hdr_size); } @@ -119,7 +119,7 @@ static struct bloblist_rec *bloblist_next_blob(struct bloblist_hdr *hdr, { ulong offset = bloblist_blob_end_ofs(hdr, rec); - if (offset >= hdr->alloced) + if (offset >= hdr->used_size) return NULL; return (struct bloblist_rec *)((void *)hdr + offset); } @@ -156,9 +156,9 @@ static int bloblist_addrec(uint tag, int size, int align_log2, align_log2 = BLOBLIST_BLOB_ALIGN_LOG2; /* Figure out where the new data will start */ - data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*rec); + data_start = map_to_sysmem(hdr) + hdr->used_size + sizeof(*rec); - /* Align the address and then calculate the offset from ->alloced */ + /* Align the address and then calculate the offset from used size */ aligned_start = ALIGN(data_start, 1U << align_log2) - data_start; /* If we need to create a dummy record, create it */ @@ -172,19 +172,20 @@ static int bloblist_addrec(uint tag, int size, int align_log2, return log_msg_ret("void", ret); /* start the record after that */ - data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*vrec); + data_start = map_to_sysmem(hdr) + hdr->used_size + sizeof(*vrec); } /* Calculate the new allocated total */ new_alloced = data_start - map_to_sysmem(hdr) + ALIGN(size, 1U << align_log2); - if (new_alloced > hdr->size) { - log_err("Failed to allocate %x bytes size=%x, need size=%x\n", - size, hdr->size, new_alloced); + if (new_alloced > hdr->total_size) { + log_err("Failed to allocate %x bytes\n", size); + log_err("Used size=%x, total size=%x\n", + hdr->used_size, hdr->total_size); return log_msg_ret("bloblist add", -ENOSPC); } - rec = (void *)hdr + hdr->alloced; + rec = (void *)hdr + hdr->used_size; rec->tag_and_hdr_size = tag | sizeof(*rec) << BLOBLISTR_HDR_SIZE_SHIFT; rec->size = size; @@ -192,7 +193,7 @@ static int bloblist_addrec(uint tag, int size, int align_log2, /* Zero the record data */ memset((void *)rec + rec_hdr_size(rec), '\0', rec->size); - hdr->alloced = new_alloced; + hdr->used_size = new_alloced; *recp = rec; return 0; @@ -287,29 +288,30 @@ static int bloblist_resize_rec(struct bloblist_hdr *hdr, int new_size) { int expand_by; /* Number of bytes to expand by (-ve to contract) */ - int new_alloced; /* New value for @hdr->alloced */ + int new_alloced; ulong next_ofs; /* Offset of the record after @rec */ expand_by = ALIGN(new_size - rec->size, BLOBLIST_BLOB_ALIGN); - new_alloced = ALIGN(hdr->alloced + expand_by, BLOBLIST_BLOB_ALIGN); + new_alloced = ALIGN(hdr->used_size + expand_by, BLOBLIST_BLOB_ALIGN); if (new_size < 0) { log_debug("Attempt to shrink blob size below 0 (%x)\n", new_size); return log_msg_ret("size", -EINVAL); } - if (new_alloced > hdr->size) { - log_err("Failed to allocate %x bytes size=%x, need size=%x\n", - new_size, hdr->size, new_alloced); + if (new_alloced > hdr->total_size) { + log_err("Failed to allocate %x bytes\n", new_size); + log_err("Used size=%x, total size=%x\n", + hdr->used_size, hdr->total_size); return log_msg_ret("alloc", -ENOSPC); } /* Move the following blobs up or down, if this is not the last */ next_ofs = bloblist_blob_end_ofs(hdr, rec); - if (next_ofs != hdr->alloced) { + if (next_ofs != hdr->used_size) { memmove((void *)hdr + next_ofs + expand_by, (void *)hdr + next_ofs, new_alloced - next_ofs); } - hdr->alloced = new_alloced; + hdr->used_size = new_alloced; /* Zero the new part of the blob */ if (expand_by > 0) { @@ -343,7 +345,7 @@ static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr) { u8 chksum; - chksum = table_compute_checksum(hdr, hdr->alloced); + chksum = table_compute_checksum(hdr, hdr->used_size); chksum += hdr->chksum; return chksum; @@ -363,8 +365,8 @@ int bloblist_new(ulong addr, uint size, uint flags) hdr->hdr_size = sizeof(*hdr); hdr->flags = flags; hdr->magic = BLOBLIST_MAGIC; - hdr->size = size; - hdr->alloced = hdr->hdr_size; + hdr->used_size = hdr->hdr_size; + hdr->total_size = size; hdr->chksum = 0; gd->bloblist = hdr; @@ -381,8 +383,13 @@ int bloblist_check(ulong addr, uint size) return log_msg_ret("Bad magic", -ENOENT); if (hdr->version != BLOBLIST_VERSION) return log_msg_ret("Bad version", -EPROTONOSUPPORT); - if (size && hdr->size != size) - return log_msg_ret("Bad size", -EFBIG); + if (!hdr->total_size || (size && hdr->total_size != size)) + return log_msg_ret("Bad total size", -EFBIG); + if (hdr->used_size > hdr->total_size) + return log_msg_ret("Bad used size", -ENOENT); + if (hdr->hdr_size != sizeof(struct bloblist_hdr)) + return log_msg_ret("Bad header size", -ENOENT); + chksum = bloblist_calc_chksum(hdr); if (hdr->chksum != chksum) { log_err("Checksum %x != %x\n", hdr->chksum, chksum); @@ -398,7 +405,7 @@ int bloblist_finish(void) struct bloblist_hdr *hdr = gd->bloblist; hdr->chksum = bloblist_calc_chksum(hdr); - log_debug("Finished bloblist size %lx at %lx\n", (ulong)hdr->size, + log_debug("Finished bloblist size %lx at %lx\n", (ulong)hdr->used_size, (ulong)map_to_sysmem(hdr)); return 0; @@ -413,33 +420,40 @@ ulong bloblist_get_size(void) { struct bloblist_hdr *hdr = gd->bloblist; - return hdr->size; + return hdr->used_size; } -void bloblist_get_stats(ulong *basep, ulong *sizep, ulong *allocedp) +ulong bloblist_get_total_size(void) +{ + struct bloblist_hdr *hdr = gd->bloblist; + + return hdr->total_size; +} + +void bloblist_get_stats(ulong *basep, ulong *tsizep, ulong *usizep) { struct bloblist_hdr *hdr = gd->bloblist; *basep = map_to_sysmem(gd->bloblist); - *sizep = hdr->size; - *allocedp = hdr->alloced; + *tsizep = hdr->total_size; + *usizep = hdr->used_size; } static void show_value(const char *prompt, ulong value) { - printf("%s:%*s %-5lx ", prompt, 8 - (int)strlen(prompt), "", value); + printf("%s:%*s %-5lx ", prompt, 10 - (int)strlen(prompt), "", value); print_size(value, "\n"); } void bloblist_show_stats(void) { - ulong base, size, alloced; + ulong base, tsize, usize; - bloblist_get_stats(&base, &size, &alloced); - printf("base: %lx\n", base); - show_value("size", size); - show_value("alloced", alloced); - show_value("free", size - alloced); + bloblist_get_stats(&base, &tsize, &usize); + printf("base: %lx\n", base); + show_value("total size", tsize); + show_value("used size", usize); + show_value("free", tsize - usize); } void bloblist_show_list(void) @@ -463,7 +477,7 @@ void bloblist_reloc(void *to, uint to_size, void *from, uint from_size) memcpy(to, from, from_size); hdr = to; - hdr->size = to_size; + hdr->total_size = to_size; } int bloblist_init(void) @@ -493,7 +507,7 @@ int bloblist_init(void) addr, ret); } else { /* Get the real size, if it is not what we expected */ - size = gd->bloblist->size; + size = gd->bloblist->total_size; } } if (ret) { diff --git a/include/bloblist.h b/include/bloblist.h index 7024d7bf9e5..4ec4b3d449e 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -166,32 +166,33 @@ enum bloblist_tag_t { * from the last. * * @magic: BLOBLIST_MAGIC + * @chksum: checksum for the entire bloblist allocated area. Since any of the + * blobs can be altered after being created, this checksum is only valid + * when the bloblist is finalized before jumping to the next stage of boot. + * This is the value needed to make all checksummed bytes sum to 0 * @version: BLOBLIST_VERSION * @hdr_size: Size of this header, normally sizeof(struct bloblist_hdr). The * first bloblist_rec starts at this offset from the start of the header - * @flags: Space for BLOBLISTF... flags (none yet) - * @size: Total size of the bloblist (non-zero if valid) including this header. - * The bloblist extends for this many bytes from the start of this header. - * When adding new records, the bloblist can grow up to this size. - * @alloced: Total size allocated so far for this bloblist. This starts out as + * @align_log2: Power of two of the maximum alignment required by this list + * @used_size: Size allocated so far for this bloblist. This starts out as * sizeof(bloblist_hdr) since we need at least that much space to store a * valid bloblist + * @total_size: The number of total bytes that the bloblist can occupy. + * Any blob producer must check if there is sufficient space before adding + * a record to the bloblist. + * @flags: Space for BLOBLISTF... flags (none yet) * @spare: Spare space (for future use) - * @chksum: checksum for the entire bloblist allocated area. Since any of the - * blobs can be altered after being created, this checksum is only valid - * when the bloblist is finalised before jumping to the next stage of boot. - * This is the value needed to make all checksummed bytes sum to 0 */ struct bloblist_hdr { u32 magic; - u32 version; - u32 hdr_size; + u8 chksum; + u8 version; + u8 hdr_size; + u8 align_log2; + u32 used_size; + u32 total_size; u32 flags; - - u32 size; - u32 alloced; u32 spare; - u32 chksum; }; /** @@ -363,10 +364,10 @@ int bloblist_finish(void); * This returns useful information about the bloblist * * @basep: Returns base address of bloblist - * @sizep: Returns the number of bytes used in the bloblist - * @allocedp: Returns the total space allocated to the bloblist + * @tsizep: Returns the total number of bytes of the bloblist + * @usizep: Returns the number of used bytes of the bloblist */ -void bloblist_get_stats(ulong *basep, ulong *sizep, ulong *allocedp); +void bloblist_get_stats(ulong *basep, ulong *tsizep, ulong *usizep); /** * bloblist_get_base() - Get the base address of the bloblist @@ -382,6 +383,13 @@ ulong bloblist_get_base(void); */ ulong bloblist_get_size(void); +/** + * bloblist_get_total_size() - Get the total size of the bloblist + * + * Return: the size in bytes + */ +ulong bloblist_get_total_size(void); + /** * bloblist_show_stats() - Show information about the bloblist * diff --git a/test/bloblist.c b/test/bloblist.c index e6070041d36..2b06ce844f8 100644 --- a/test/bloblist.c +++ b/test/bloblist.c @@ -78,7 +78,7 @@ static int bloblist_test_init(struct unit_test_state *uts) ut_asserteq(-EPROTONOSUPPORT, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); - ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0x10, 0)); + ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0xc, 0)); ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE, 0)); ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); @@ -107,7 +107,8 @@ static int bloblist_test_blob(struct unit_test_state *uts) hdr = clear_bloblist(); ut_assertnull(bloblist_find(TEST_TAG, TEST_BLOBLIST_SIZE)); ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); - ut_asserteq(TEST_BLOBLIST_SIZE, bloblist_get_size()); + ut_asserteq(sizeof(struct bloblist_hdr), bloblist_get_size()); + ut_asserteq(TEST_BLOBLIST_SIZE, bloblist_get_total_size()); ut_asserteq(TEST_ADDR, bloblist_get_base()); ut_asserteq(map_to_sysmem(hdr), TEST_ADDR); @@ -205,9 +206,9 @@ static int bloblist_test_checksum(struct unit_test_state *uts) ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); hdr->flags++; - hdr->size--; + hdr->total_size--; ut_asserteq(-EFBIG, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); - hdr->size++; + hdr->total_size++; hdr->spare++; ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); @@ -270,10 +271,10 @@ static int bloblist_test_cmd_info(struct unit_test_state *uts) ut_silence_console(uts); console_record_reset(); run_command("bloblist info", 0); - ut_assert_nextline("base: %lx", (ulong)map_to_sysmem(hdr)); - ut_assert_nextline("size: 400 1 KiB"); - ut_assert_nextline("alloced: 58 88 Bytes"); - ut_assert_nextline("free: 3a8 936 Bytes"); + ut_assert_nextline("base: %lx", (ulong)map_to_sysmem(hdr)); + ut_assert_nextline("total size: 400 1 KiB"); + ut_assert_nextline("used size: 50 80 Bytes"); + ut_assert_nextline("free: 3b0 944 Bytes"); ut_assert_console_end(); ut_unsilence_console(uts); @@ -427,7 +428,7 @@ static int bloblist_test_grow(struct unit_test_state *uts) ut_asserteq(sizeof(struct bloblist_hdr) + sizeof(struct bloblist_rec) * 2 + small_size * 2, - hdr->alloced); + hdr->used_size); /* Resize the first one */ ut_assertok(bloblist_resize(TEST_TAG, small_size + 4)); @@ -449,7 +450,7 @@ static int bloblist_test_grow(struct unit_test_state *uts) ut_asserteq(sizeof(struct bloblist_hdr) + sizeof(struct bloblist_rec) * 2 + small_size * 2 + BLOBLIST_BLOB_ALIGN, - hdr->alloced); + hdr->used_size); return 0; } @@ -479,7 +480,7 @@ static int bloblist_test_shrink(struct unit_test_state *uts) hdr = ptr; ut_asserteq(sizeof(struct bloblist_hdr) + sizeof(struct bloblist_rec) * 2 + small_size * 2, - hdr->alloced); + hdr->used_size); /* Resize the first one */ new_size = small_size - BLOBLIST_ALIGN - 4; @@ -499,7 +500,7 @@ static int bloblist_test_shrink(struct unit_test_state *uts) ut_asserteq(sizeof(struct bloblist_hdr) + sizeof(struct bloblist_rec) * 2 + small_size * 2 - BLOBLIST_ALIGN, - hdr->alloced); + hdr->used_size); return 0; } @@ -527,12 +528,12 @@ static int bloblist_test_resize_fail(struct unit_test_state *uts) hdr = ptr; ut_asserteq(sizeof(struct bloblist_hdr) + sizeof(struct bloblist_rec) * 2 + small_size * 2, - hdr->alloced); + hdr->used_size); /* Resize the first one, to check the boundary conditions */ ut_asserteq(-EINVAL, bloblist_resize(TEST_TAG, -1)); - new_size = small_size + (hdr->size - hdr->alloced); + new_size = small_size + (hdr->total_size - hdr->used_size); ut_asserteq(-ENOSPC, bloblist_resize(TEST_TAG, new_size + 1)); ut_assertok(bloblist_resize(TEST_TAG, new_size)); @@ -564,9 +565,9 @@ static int bloblist_test_resize_last(struct unit_test_state *uts) /* Check the byte after the last blob */ alloced_val = sizeof(struct bloblist_hdr) + sizeof(struct bloblist_rec) * 2 + small_size * 2; - ut_asserteq(alloced_val, hdr->alloced); + ut_asserteq(alloced_val, hdr->used_size); ut_asserteq_ptr((void *)hdr + alloced_val, blob2 + small_size); - ut_asserteq((u8)ERASE_BYTE, *((u8 *)hdr + hdr->alloced)); + ut_asserteq((u8)ERASE_BYTE, *((u8 *)hdr + hdr->used_size)); /* Resize the second one, checking nothing changes */ ut_asserteq(0, bloblist_resize(TEST_TAG2, small_size + 4)); @@ -584,8 +585,8 @@ static int bloblist_test_resize_last(struct unit_test_state *uts) /* Check that the new top of the allocated blobs has not been touched */ alloced_val += BLOBLIST_BLOB_ALIGN; - ut_asserteq(alloced_val, hdr->alloced); - ut_asserteq((u8)ERASE_BYTE, *((u8 *)hdr + hdr->alloced)); + ut_asserteq(alloced_val, hdr->used_size); + ut_asserteq((u8)ERASE_BYTE, *((u8 *)hdr + hdr->used_size)); return 0; } From 7d790a80b67958b49da591d32662c4b168737012 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 27 Dec 2023 13:07:09 -0800 Subject: [PATCH 23/24] bloblist: Add alignment to bloblist_new() Allow the alignment to be specified when creating a bloblist. Signed-off-by: Simon Glass Co-developed-by: Raymond Mao Signed-off-by: Raymond Mao Reviewed-by: Ilias Apalodimas --- common/bloblist.c | 5 +++-- include/bloblist.h | 3 ++- test/bloblist.c | 40 ++++++++++++++++++++++------------------ 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/common/bloblist.c b/common/bloblist.c index 6e019087ff9..2d373910b6d 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -351,7 +351,7 @@ static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr) return chksum; } -int bloblist_new(ulong addr, uint size, uint flags) +int bloblist_new(ulong addr, uint size, uint flags, uint align_log2) { struct bloblist_hdr *hdr; @@ -367,6 +367,7 @@ int bloblist_new(ulong addr, uint size, uint flags) hdr->magic = BLOBLIST_MAGIC; hdr->used_size = hdr->hdr_size; hdr->total_size = size; + hdr->align_log2 = align_log2 ? align_log2 : BLOBLIST_BLOB_ALIGN_LOG2; hdr->chksum = 0; gd->bloblist = hdr; @@ -522,7 +523,7 @@ int bloblist_init(void) } log_debug("Creating new bloblist size %lx at %lx\n", size, addr); - ret = bloblist_new(addr, size, 0); + ret = bloblist_new(addr, size, 0, 0); } else { log_debug("Found existing bloblist size %lx at %lx\n", size, addr); diff --git a/include/bloblist.h b/include/bloblist.h index 4ec4b3d449e..145e5c0242c 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -330,10 +330,11 @@ int bloblist_resize(uint tag, int new_size); * @addr: Address of bloblist * @size: Initial size for bloblist * @flags: Flags to use for bloblist + * @align_log2: Log base 2 of maximum alignment provided by this bloblist * Return: 0 if OK, -EFAULT if addr is not aligned correctly, -ENOSPC is the * area is not large enough */ -int bloblist_new(ulong addr, uint size, uint flags); +int bloblist_new(ulong addr, uint size, uint flags, uint align_log2); /** * bloblist_check() - Check if a bloblist exists diff --git a/test/bloblist.c b/test/bloblist.c index 2b06ce844f8..17d9dd03d07 100644 --- a/test/bloblist.c +++ b/test/bloblist.c @@ -72,15 +72,15 @@ static int bloblist_test_init(struct unit_test_state *uts) hdr = clear_bloblist(); ut_asserteq(-ENOENT, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); ut_asserteq_ptr(NULL, bloblist_check_magic(TEST_ADDR)); - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); ut_asserteq_ptr(hdr, bloblist_check_magic(TEST_ADDR)); hdr->version++; ut_asserteq(-EPROTONOSUPPORT, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); - ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0xc, 0)); - ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE, 0)); - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0xc, 0, 0)); + ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE, 0, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); ut_assertok(bloblist_finish()); @@ -106,7 +106,7 @@ static int bloblist_test_blob(struct unit_test_state *uts) /* At the start there should be no records */ hdr = clear_bloblist(); ut_assertnull(bloblist_find(TEST_TAG, TEST_BLOBLIST_SIZE)); - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); ut_asserteq(sizeof(struct bloblist_hdr), bloblist_get_size()); ut_asserteq(TEST_BLOBLIST_SIZE, bloblist_get_total_size()); ut_asserteq(TEST_ADDR, bloblist_get_base()); @@ -145,7 +145,7 @@ static int bloblist_test_blob_ensure(struct unit_test_state *uts) /* At the start there should be no records */ clear_bloblist(); - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); /* Test with an empty bloblist */ size = TEST_SIZE; @@ -177,7 +177,7 @@ static int bloblist_test_bad_blob(struct unit_test_state *uts) void *data; hdr = clear_bloblist(); - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); data = hdr + 1; data += sizeof(struct bloblist_rec); ut_asserteq_addr(data, bloblist_ensure(TEST_TAG, TEST_SIZE)); @@ -193,7 +193,7 @@ static int bloblist_test_checksum(struct unit_test_state *uts) char *data, *data2; hdr = clear_bloblist(); - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); ut_assertok(bloblist_finish()); ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); @@ -218,6 +218,10 @@ static int bloblist_test_checksum(struct unit_test_state *uts) ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); hdr->chksum--; + hdr->align_log2++; + ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); + hdr->align_log2--; + /* Make sure the checksum changes when we add blobs */ data = bloblist_add(TEST_TAG, TEST_SIZE, 0); ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); @@ -263,7 +267,7 @@ static int bloblist_test_cmd_info(struct unit_test_state *uts) char *data, *data2; hdr = clear_bloblist(); - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); data = bloblist_ensure(TEST_TAG, TEST_SIZE); data2 = bloblist_ensure(TEST_TAG2, TEST_SIZE2); @@ -289,7 +293,7 @@ static int bloblist_test_cmd_list(struct unit_test_state *uts) char *data, *data2; hdr = clear_bloblist(); - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); data = bloblist_ensure(TEST_TAG, TEST_SIZE); data2 = bloblist_ensure(TEST_TAG2, TEST_SIZE2); @@ -319,7 +323,7 @@ static int bloblist_test_align(struct unit_test_state *uts) /* At the start there should be no records */ hdr = clear_bloblist(); - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); ut_assertnull(bloblist_find(TEST_TAG, TEST_BLOBLIST_SIZE)); /* Check the default alignment */ @@ -356,7 +360,7 @@ static int bloblist_test_align(struct unit_test_state *uts) memset(hdr, ERASE_BYTE, TEST_BLOBLIST_SIZE); memset(hdr, '\0', sizeof(*hdr)); ut_assertok(bloblist_new(TEST_ADDR + BLOBLIST_ALIGN, TEST_BLOBLIST_SIZE, - 0)); + 0, 0)); data = bloblist_add(1, 5, BLOBLIST_ALIGN_LOG2 + 1); ut_assertnonnull(data); @@ -377,7 +381,7 @@ static int bloblist_test_reloc(struct unit_test_state *uts) ulong new_addr; ulong new_size; - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); old_ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE); /* Add one blob and then one that won't fit */ @@ -416,7 +420,7 @@ static int bloblist_test_grow(struct unit_test_state *uts) memset(hdr, ERASE_BYTE, TEST_BLOBLIST_SIZE); /* Create two blobs */ - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); blob1 = bloblist_add(TEST_TAG, small_size, 0); ut_assertnonnull(blob1); ut_assertok(check_zero(blob1, small_size)); @@ -468,7 +472,7 @@ static int bloblist_test_shrink(struct unit_test_state *uts) ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE); /* Create two blobs */ - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); blob1 = bloblist_add(TEST_TAG, small_size, 0); ut_assertnonnull(blob1); strcpy(blob1, test1_str); @@ -518,7 +522,7 @@ static int bloblist_test_resize_fail(struct unit_test_state *uts) ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE); /* Create two blobs */ - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); blob1 = bloblist_add(TEST_TAG, small_size, 0); ut_assertnonnull(blob1); @@ -555,7 +559,7 @@ static int bloblist_test_resize_last(struct unit_test_state *uts) hdr = ptr; /* Create two blobs */ - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); blob1 = bloblist_add(TEST_TAG, small_size, 0); ut_assertnonnull(blob1); @@ -600,7 +604,7 @@ static int bloblist_test_blob_maxsize(struct unit_test_state *uts) /* At the start there should be no records */ clear_bloblist(); - ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); /* Add a blob that takes up all space */ size = TEST_BLOBLIST_SIZE - sizeof(struct bloblist_hdr) - From e266d2731145681a55d862360f1b61690b0c6820 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 27 Dec 2023 13:07:10 -0800 Subject: [PATCH 24/24] bloblist: Update documentation and header comment Align the documentation with the v0.9 spec. Signed-off-by: Simon Glass Signed-off-by: Raymond Mao Reviewed-by: Ilias Apalodimas --- doc/develop/bloblist.rst | 4 +++- include/bloblist.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/develop/bloblist.rst b/doc/develop/bloblist.rst index 81643c7674b..28431039adc 100644 --- a/doc/develop/bloblist.rst +++ b/doc/develop/bloblist.rst @@ -14,6 +14,8 @@ structure defined by the code that owns it. For the design goals of bloblist, please see the comments at the top of the `bloblist.h` header file. +Bloblist is an implementation with the `Firmware Handoff`_ protocol. + Passing state through the boot process -------------------------------------- @@ -99,7 +101,7 @@ API documentation ----------------- .. kernel-doc:: include/bloblist.h - +.. _`Firmware Handoff`: https://github.com/FirmwareHandoff/firmware_handoff Simon Glass sjg@chromium.org diff --git a/include/bloblist.h b/include/bloblist.h index 145e5c0242c..84fc9438191 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -66,6 +66,7 @@ * * Copyright 2018 Google, Inc * Written by Simon Glass + * Adjusted July 2023 to match Firmware handoff specification, Release 0.9 */ #ifndef __BLOBLIST_H