From d1fffbe3c808a9012a05b048560e17ce43f8ef9e Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 10 Apr 2024 10:38:28 +0200 Subject: sandbox: use sane access rights for files When writing an executable, allowing other users to modify it introduces a security issue. Generally we should avoid giving other users write access to our files by default. Replace chmod(777) by chmod(755) and chmod(644). Fixes: 47f5fcfb4169 ("sandbox: Add os_jump_to_image() to run another executable") Fixes: d9165153caea ("sandbox: add flags for open() call") Fixes: 5c2859cdc302 ("sandbox: Allow reading/writing of RAM buffer") Signed-off-by: Heinrich Schuchardt Reviewed-by: Sean Anderson --- arch/sandbox/cpu/os.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 154a5d77490..d7869b2e368 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -109,7 +109,7 @@ int os_open(const char *pathname, int os_flags) */ flags |= O_CLOEXEC; - return open(pathname, flags, 0777); + return open(pathname, flags, 0644); } int os_close(int fd) @@ -746,7 +746,7 @@ int os_write_ram_buf(const char *fname) struct sandbox_state *state = state_get_current(); int fd, ret; - fd = open(fname, O_CREAT | O_WRONLY, 0777); + fd = open(fname, O_CREAT | O_WRONLY, 0644); if (fd < 0) return -ENOENT; ret = write(fd, state->ram_buf, state->ram_size); @@ -791,7 +791,7 @@ static int make_exec(char *fname, const void *data, int size) if (write(fd, data, size) < 0) return -EIO; close(fd); - if (chmod(fname, 0777)) + if (chmod(fname, 0755)) return -ENOEXEC; return 0; -- cgit v1.3.1 From 07a6c69759eb4a99f044962c8c40c093af534351 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 18 Apr 2024 05:11:13 +0200 Subject: acpi: set creator_revision in acpi_fill_header We should have a single place where we write the default value to the creator revision field. If we ever will have any table created by another tool, we can overwrite the value afterwards. Signed-off-by: Heinrich Schuchardt --- arch/x86/lib/acpi_table.c | 2 -- lib/acpi/acpi_table.c | 2 +- lib/acpi/ssdt.c | 1 - test/dm/acpi.c | 3 +-- 4 files changed, 2 insertions(+), 6 deletions(-) diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c index a42a7e6bbd6..e38ce19ff7c 100644 --- a/arch/x86/lib/acpi_table.c +++ b/arch/x86/lib/acpi_table.c @@ -478,7 +478,6 @@ static int acpi_create_hpet(struct acpi_hpet *hpet) /* Fill out header fields. */ acpi_fill_header(header, "HPET"); - header->creator_revision = ASL_REVISION; header->length = sizeof(struct acpi_hpet); header->revision = acpi_get_table_revision(ACPITAB_HPET); @@ -569,7 +568,6 @@ void acpi_fadt_common(struct acpi_fadt *fadt, struct acpi_facs *facs, memcpy(header->oem_id, OEM_ID, 6); memcpy(header->oem_table_id, OEM_TABLE_ID, 8); memcpy(header->creator_id, ASLC_ID, 4); - header->creator_revision = 1; fadt->x_firmware_ctrl = map_to_sysmem(facs); fadt->x_dsdt = map_to_sysmem(dsdt); diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c index c16ead6a6ec..6dbfdb22dec 100644 --- a/lib/acpi/acpi_table.c +++ b/lib/acpi/acpi_table.c @@ -117,6 +117,7 @@ void acpi_fill_header(struct acpi_table_header *header, char *signature) memcpy(header->oem_table_id, OEM_TABLE_ID, 8); header->oem_revision = OEM_REVISION; memcpy(header->creator_id, ASLC_ID, 4); + header->creator_revision = ASL_REVISION; } void acpi_align(struct acpi_ctx *ctx) @@ -219,7 +220,6 @@ void acpi_create_dbg2(struct acpi_dbg2_header *dbg2, header->revision = acpi_get_table_revision(ACPITAB_DBG2); acpi_fill_header(header, "DBG2"); - header->creator_revision = ASL_REVISION; /* One debug device defined */ dbg2->devices_offset = sizeof(struct acpi_dbg2_header); diff --git a/lib/acpi/ssdt.c b/lib/acpi/ssdt.c index e032e266b3c..df1d739d117 100644 --- a/lib/acpi/ssdt.c +++ b/lib/acpi/ssdt.c @@ -23,7 +23,6 @@ int acpi_write_ssdt(struct acpi_ctx *ctx, const struct acpi_writer *entry) acpi_fill_header(ssdt, "SSDT"); ssdt->revision = acpi_get_table_revision(ACPITAB_SSDT); - ssdt->creator_revision = 1; ssdt->length = sizeof(struct acpi_table_header); acpi_inc(ctx, sizeof(struct acpi_table_header)); diff --git a/test/dm/acpi.c b/test/dm/acpi.c index 4db2171a4b1..7da381f1a54 100644 --- a/test/dm/acpi.c +++ b/test/dm/acpi.c @@ -236,7 +236,6 @@ static int dm_test_acpi_fill_header(struct unit_test_state *uts) hdr.length = 0x11; hdr.revision = 0x22; hdr.checksum = 0x33; - hdr.creator_revision = 0x44; acpi_fill_header(&hdr, "ABCD"); ut_asserteq_mem("ABCD", hdr.signature, sizeof(hdr.signature)); @@ -248,7 +247,7 @@ static int dm_test_acpi_fill_header(struct unit_test_state *uts) sizeof(hdr.oem_table_id)); ut_asserteq(OEM_REVISION, hdr.oem_revision); ut_asserteq_mem(ASLC_ID, hdr.creator_id, sizeof(hdr.creator_id)); - ut_asserteq(0x44, hdr.creator_revision); + ut_asserteq(ASL_REVISION, hdr.creator_revision); return 0; } -- cgit v1.3.1 From eba80858039c403cb3e2ef166c7f1418838d4ec2 Mon Sep 17 00:00:00 2001 From: Sean Anderson Date: Thu, 18 Apr 2024 22:36:30 -0400 Subject: patman: Fix tests if add_maintainers is set to False If add_maintainers is set to False in the user's ~/.patman config, it will cause the custom_get_maintainer_script to fail since that test expects maintainers to be added. Set add_maintainer to True in the .patman config to prevent this. Fixes: 8c042fb7f9f ("patman: add '--get-maintainer-script' argument") Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- tools/patman/func_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py index e3918497cf4..9c016fb5e9a 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -540,7 +540,8 @@ complicated as possible''') with open('.patman', 'w', buffering=1) as f: f.write('[settings]\n' 'get_maintainer_script: dummy-script.sh\n' - 'check_patch: False\n') + 'check_patch: False\n' + 'add_maintainers: True\n') with open('dummy-script.sh', 'w', buffering=1) as f: f.write('#!/usr/bin/env python\n' 'print("hello@there.com")\n') -- cgit v1.3.1 From b4f73931ed1dda09872ebe6ac47cf2fd6d690704 Mon Sep 17 00:00:00 2001 From: Sean Anderson Date: Thu, 18 Apr 2024 22:36:31 -0400 Subject: patman: Add Commit-cc as an alias for Patch-cc Most tags referring to commits (or patches) are named Commit-something. The exception is Patch-cc. Add a Commit-cc alias so we can use whichever one is convenient. Signed-off-by: Sean Anderson --- tools/patman/func_test.py | 5 ++++- tools/patman/patchstream.py | 2 ++ tools/patman/patman.rst | 2 +- .../0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch | 1 + tools/patman/test/test01.txt | 1 + 5 files changed, 9 insertions(+), 2 deletions(-) diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py index 9c016fb5e9a..3b4c9448882 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -211,6 +211,7 @@ class TestFunctional(unittest.TestCase): 'u-boot': ['u-boot@lists.denx.de'], 'simon': [self.leb], 'fred': [self.fred], + 'joe': [self.joe], } text = self._get_text('test01.txt') @@ -259,6 +260,7 @@ class TestFunctional(unittest.TestCase): self.assertEqual('Postfix:\t some-branch', next(lines)) self.assertEqual('Cover: 4 lines', next(lines)) self.assertEqual(' Cc: %s' % self.fred, next(lines)) + self.assertEqual(' Cc: %s' % self.joe, next(lines)) self.assertEqual(' Cc: %s' % self.leb, next(lines)) self.assertEqual(' Cc: %s' % mel, next(lines)) @@ -272,7 +274,8 @@ class TestFunctional(unittest.TestCase): self.assertEqual(('%s %s\0%s' % (args[0], rick, stefan)), cc_lines[0]) self.assertEqual( - '%s %s\0%s\0%s\0%s' % (args[1], self.fred, self.leb, rick, stefan), + '%s %s\0%s\0%s\0%s\0%s' % (args[1], self.fred, self.joe, self.leb, + rick, stefan), cc_lines[1]) expected = ''' diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index e2e2a83e677..ec1ca874fb2 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -475,6 +475,8 @@ class PatchStream: elif name == 'changes': self.in_change = 'Commit' self.change_version = self._parse_version(value, line) + elif name == 'cc': + self.commit.add_cc(value.split(',')) else: self._add_warn('Line %d: Ignoring Commit-%s' % (self.linenum, name)) diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst index f4588c00fc1..9971fa8c0fd 100644 --- a/tools/patman/patman.rst +++ b/tools/patman/patman.rst @@ -350,7 +350,7 @@ Cover-changes: n - This line will only appear in the cover letter -Patch-cc: Their Name +Patch-cc / Commit-cc: Their Name This copies a single patch to another email address. Note that the Cc: used by git send-email is ignored by patman, but will be interpreted by git send-email if you use it. diff --git a/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch b/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch index 56278a6ce9b..55a0d6756aa 100644 --- a/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch +++ b/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch @@ -21,6 +21,7 @@ Series-cc: Stefan Brüns Cover-letter-cc: Lord Mëlchett Series-version: 3 Patch-cc: fred +Commit-cc: joe Series-process-log: sort, uniq Series-changes: 4 - Some changes diff --git a/tools/patman/test/test01.txt b/tools/patman/test/test01.txt index fc3066e50b4..271d9bf043f 100644 --- a/tools/patman/test/test01.txt +++ b/tools/patman/test/test01.txt @@ -49,6 +49,7 @@ Date: Sat Apr 15 15:39:08 2017 -0600 Cover-letter-cc: Lord Mëlchett Series-version: 3 Patch-cc: fred + Commit-cc: joe Series-process-log: sort, uniq Series-changes: 4 - Some changes -- cgit v1.3.1 From 18de1afd4895bb143eb906e52b93db1b1aac6dfc Mon Sep 17 00:00:00 2001 From: Sean Anderson Date: Thu, 18 Apr 2024 22:36:32 -0400 Subject: patman: Add a tag for when a patch gets added to a series When a patch is added to a series after the initial version, there are no changes to note except that it is new. This is typically done to suppress the "(no changes in vN)" message. It's also nice to add a change to the cover letter so reviewers know there is an additional patch. Add a tag to automate this process a bit. There are two nits with the current approach: - It favors '-' as a bullet point, but some people may prefer '*' (or something else) - Tags (e.g. 'patman: ' in 'patman: foo bar') are not stripped. They are probably just noise in most series, but they may be useful for treewide series to distinguish 'gpio: frobnicate' from 'reset: frobnicate', so I've left them in. Suggestions for the above appreciated. Suggested-by: Douglas Anderson Signed-off-by: Sean Anderson Reviewed-by: Douglas Anderson --- tools/patman/func_test.py | 2 ++ tools/patman/patchstream.py | 5 +++++ tools/patman/patman.rst | 13 +++++++++++++ ...t-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch | 1 + tools/patman/test/test01.txt | 1 + 5 files changed, 22 insertions(+) diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py index 3b4c9448882..af6c025a441 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -293,6 +293,7 @@ Changes in v4: change - Some changes - Some notes for the cover letter +- fdt: Correct cast for sandbox in fdtdec_setup_mem_size_base() Simon Glass (2): pci: Correct cast for sandbox @@ -342,6 +343,7 @@ Changes in v4: - Multi line change +- New - Some changes Changes in v2: diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index ec1ca874fb2..a09ae9c7371 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -477,6 +477,11 @@ class PatchStream: self.change_version = self._parse_version(value, line) elif name == 'cc': self.commit.add_cc(value.split(',')) + elif name == 'added-in': + version = self._parse_version(value, line) + self.commit.add_change(version, '- New') + self.series.AddChange(version, None, '- %s' % + self.commit.subject) else: self._add_warn('Line %d: Ignoring Commit-%s' % (self.linenum, name)) diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst index 9971fa8c0fd..63b95a6b161 100644 --- a/tools/patman/patman.rst +++ b/tools/patman/patman.rst @@ -350,6 +350,19 @@ Cover-changes: n - This line will only appear in the cover letter +Commit-added-in: n + Add a change noting the version this commit was added in. This is + equivalent to:: + + Commit-changes: n + - New + + Cover-changes: n + - + + It is a convenient shorthand for suppressing the '(no changes in vN)' + message. + Patch-cc / Commit-cc: Their Name This copies a single patch to another email address. Note that the Cc: used by git send-email is ignored by patman, but will be diff --git a/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch b/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch index 55a0d6756aa..48ea1793b47 100644 --- a/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch +++ b/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch @@ -23,6 +23,7 @@ Series-version: 3 Patch-cc: fred Commit-cc: joe Series-process-log: sort, uniq +Commit-added-in: 4 Series-changes: 4 - Some changes - Multi diff --git a/tools/patman/test/test01.txt b/tools/patman/test/test01.txt index 271d9bf043f..b2d73c5972c 100644 --- a/tools/patman/test/test01.txt +++ b/tools/patman/test/test01.txt @@ -51,6 +51,7 @@ Date: Sat Apr 15 15:39:08 2017 -0600 Patch-cc: fred Commit-cc: joe Series-process-log: sort, uniq + Commit-added-in: 4 Series-changes: 4 - Some changes - Multi -- cgit v1.3.1 From d243b369e95139f77d04007755a4af42a0a03b30 Mon Sep 17 00:00:00 2001 From: Jonathan Liu Date: Sat, 25 May 2024 18:10:53 +1000 Subject: sandbox: enable support for the unlz4 command This does not work with sandbox at present. Fix it up to use map_sysmem() to convert an address to a pointer. Signed-off-by: Jonathan Liu Reviewed-by: Simon Glass Fix conflict and reformat to 80cols: Signed-off-by: Simon Glass --- cmd/unlz4.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/unlz4.c b/cmd/unlz4.c index fc5200117ad..2eadc753e6c 100644 --- a/cmd/unlz4.c +++ b/cmd/unlz4.c @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -26,7 +27,8 @@ static int do_unlz4(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_USAGE; } - ret = ulz4fn((void *)src, src_len, (void *)dst, &dst_len); + ret = ulz4fn(map_sysmem(src, 0), src_len, map_sysmem(dst, dst_len), + &dst_len); if (ret) { printf("Uncompressed err :%d\n", ret); return 1; -- cgit v1.3.1 From a8729a260b53b9a2fce2607ac90744a47f96daef Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Mon, 27 May 2024 22:04:17 +0200 Subject: global_data.h: drop write-only field dm_root_f The dm_root_f field seems to be entirely write-only and hence redundant, unless 'git grep' fails to find some access generated via preprocessor token concatenation or similar. Signed-off-by: Rasmus Villemoes Reviewed-by: Simon Glass --- common/board_r.c | 3 +-- include/asm-generic/global_data.h | 4 ---- test/dm/core.c | 1 - 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/common/board_r.c b/common/board_r.c index c823cd262f1..d4ba245ac69 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -230,8 +230,7 @@ static int initr_dm(void) oftree_reset(); - /* Save the pre-reloc driver model and start a new one */ - gd->dm_root_f = gd->dm_root; + /* Drop the pre-reloc driver model and start a new one */ gd->dm_root = NULL; #ifdef CONFIG_TIMER gd->timer = NULL; diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index fcc3c6e14ca..aa336d63e3a 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -195,10 +195,6 @@ struct global_data { * @dm_root: root instance for Driver Model */ struct udevice *dm_root; - /** - * @dm_root_f: pre-relocation root instance - */ - struct udevice *dm_root_f; /** * @uclass_root_s: * head of core tree when uclasses are not in read-only memory. diff --git a/test/dm/core.c b/test/dm/core.c index 4741c81bcc1..dbad1b317db 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -1006,7 +1006,6 @@ static int dm_test_uclass_before_ready(struct unit_test_state *uts) ut_assertok(uclass_get(UCLASS_TEST, &uc)); gd->dm_root = NULL; - gd->dm_root_f = NULL; memset(&gd->uclass_root, '\0', sizeof(gd->uclass_root)); ut_asserteq_ptr(NULL, uclass_find(UCLASS_TEST)); -- cgit v1.3.1 From 357bfca5e616c7fc003cce1ddda44016660cf75f Mon Sep 17 00:00:00 2001 From: Brandon Maier Date: Tue, 4 Jun 2024 16:16:05 +0000 Subject: tools: binman: fix deprecated Python unittest methods The methods `unittest.assertEquals()` and `unittest.assertRegexpMatches()` are marked deprecated[1]. In Python 3.12 these aliases have been removed, so do a sed to replace them with their new names. [1] https://docs.python.org/3.11/library/unittest.html#deprecated-aliases Signed-off-by: Brandon Maier CC: Simon Glass CC: Alper Nebi Yasak Reviewed-by: Simon Glass --- tools/binman/entry_test.py | 6 ++-- tools/binman/fdt_test.py | 48 ++++++++++++++--------------- tools/binman/ftest.py | 42 ++++++++++++------------- tools/buildman/func_test.py | 74 ++++++++++++++++++++++----------------------- tools/buildman/test.py | 2 +- 5 files changed, 86 insertions(+), 86 deletions(-) diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py index ac6582cf86a..40d74d401a2 100644 --- a/tools/binman/entry_test.py +++ b/tools/binman/entry_test.py @@ -103,7 +103,7 @@ class TestEntry(unittest.TestCase): ent = entry.Entry.Create(None, self.GetNode(), 'missing', missing_etype=True) self.assertTrue(isinstance(ent, Entry_blob)) - self.assertEquals('missing', ent.etype) + self.assertEqual('missing', ent.etype) def testDecompressData(self): """Test the DecompressData() method of the base class""" @@ -111,8 +111,8 @@ class TestEntry(unittest.TestCase): base.compress = 'lz4' bintools = {} base.comp_bintool = base.AddBintool(bintools, '_testing') - self.assertEquals(tools.get_bytes(0, 1024), base.CompressData(b'abc')) - self.assertEquals(tools.get_bytes(0, 1024), base.DecompressData(b'abc')) + self.assertEqual(tools.get_bytes(0, 1024), base.CompressData(b'abc')) + self.assertEqual(tools.get_bytes(0, 1024), base.DecompressData(b'abc')) def testLookupOffset(self): """Test the lookup_offset() method of the base class""" diff --git a/tools/binman/fdt_test.py b/tools/binman/fdt_test.py index 7ef87295463..564c1770820 100644 --- a/tools/binman/fdt_test.py +++ b/tools/binman/fdt_test.py @@ -44,43 +44,43 @@ class TestFdt(unittest.TestCase): fname = self.GetCompiled('045_prop_test.dts') dt = FdtScan(fname) node = dt.GetNode('/binman/intel-me') - self.assertEquals('intel-me', node.name) + self.assertEqual('intel-me', node.name) val = fdt_util.GetString(node, 'filename') - self.assertEquals(str, type(val)) - self.assertEquals('me.bin', val) + self.assertEqual(str, type(val)) + self.assertEqual('me.bin', val) prop = node.props['intval'] - self.assertEquals(fdt.Type.INT, prop.type) - self.assertEquals(3, fdt_util.GetInt(node, 'intval')) + self.assertEqual(fdt.Type.INT, prop.type) + self.assertEqual(3, fdt_util.GetInt(node, 'intval')) prop = node.props['intarray'] - self.assertEquals(fdt.Type.INT, prop.type) - self.assertEquals(list, type(prop.value)) - self.assertEquals(2, len(prop.value)) - self.assertEquals([5, 6], + self.assertEqual(fdt.Type.INT, prop.type) + self.assertEqual(list, type(prop.value)) + self.assertEqual(2, len(prop.value)) + self.assertEqual([5, 6], [fdt_util.fdt32_to_cpu(val) for val in prop.value]) prop = node.props['byteval'] - self.assertEquals(fdt.Type.BYTE, prop.type) - self.assertEquals(chr(8), prop.value) + self.assertEqual(fdt.Type.BYTE, prop.type) + self.assertEqual(chr(8), prop.value) prop = node.props['bytearray'] - self.assertEquals(fdt.Type.BYTE, prop.type) - self.assertEquals(list, type(prop.value)) - self.assertEquals(str, type(prop.value[0])) - self.assertEquals(3, len(prop.value)) - self.assertEquals([chr(1), '#', '4'], prop.value) + self.assertEqual(fdt.Type.BYTE, prop.type) + self.assertEqual(list, type(prop.value)) + self.assertEqual(str, type(prop.value[0])) + self.assertEqual(3, len(prop.value)) + self.assertEqual([chr(1), '#', '4'], prop.value) prop = node.props['longbytearray'] - self.assertEquals(fdt.Type.INT, prop.type) - self.assertEquals(0x090a0b0c, fdt_util.GetInt(node, 'longbytearray')) + self.assertEqual(fdt.Type.INT, prop.type) + self.assertEqual(0x090a0b0c, fdt_util.GetInt(node, 'longbytearray')) prop = node.props['stringval'] - self.assertEquals(fdt.Type.STRING, prop.type) - self.assertEquals('message2', fdt_util.GetString(node, 'stringval')) + self.assertEqual(fdt.Type.STRING, prop.type) + self.assertEqual('message2', fdt_util.GetString(node, 'stringval')) prop = node.props['stringarray'] - self.assertEquals(fdt.Type.STRING, prop.type) - self.assertEquals(list, type(prop.value)) - self.assertEquals(3, len(prop.value)) - self.assertEquals(['another', 'multi-word', 'message'], prop.value) + self.assertEqual(fdt.Type.STRING, prop.type) + self.assertEqual(list, type(prop.value)) + self.assertEqual(3, len(prop.value)) + self.assertEqual(['another', 'multi-word', 'message'], prop.value) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 8a44bc051b3..567849bbab0 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2095,7 +2095,7 @@ class TestFunctional(unittest.TestCase): dtb.Scan() props = self._GetPropTree(dtb, ['size', 'uncomp-size']) orig = self._decompress(data) - self.assertEquals(COMPRESS_DATA, orig) + self.assertEqual(COMPRESS_DATA, orig) # Do a sanity check on various fields image = control.images['image'] @@ -2809,9 +2809,9 @@ class TestFunctional(unittest.TestCase): orig_entry = orig_image.GetEntries()['fdtmap'] entry = image.GetEntries()['fdtmap'] - self.assertEquals(orig_entry.offset, entry.offset) - self.assertEquals(orig_entry.size, entry.size) - self.assertEquals(orig_entry.image_pos, entry.image_pos) + self.assertEqual(orig_entry.offset, entry.offset) + self.assertEqual(orig_entry.size, entry.size) + self.assertEqual(orig_entry.image_pos, entry.image_pos) def testReadImageNoHeader(self): """Test accessing an image's FDT map without an image header""" @@ -3895,7 +3895,7 @@ class TestFunctional(unittest.TestCase): mat = re_line.match(line) vals[mat.group(1)].append(mat.group(2)) - self.assertEquals('FIT description: test-desc', lines[0]) + self.assertEqual('FIT description: test-desc', lines[0]) self.assertIn('Created:', lines[1]) self.assertIn('Image 0 (kernel)', vals) self.assertIn('Hash value', vals) @@ -4012,7 +4012,7 @@ class TestFunctional(unittest.TestCase): fit_pos, fdt_util.fdt32_to_cpu(fnode.props['data-position'].value)) - self.assertEquals(expected_size, len(data)) + self.assertEqual(expected_size, len(data)) actual_pos = len(U_BOOT_DATA) + fit_pos self.assertEqual(U_BOOT_DATA + b'aa', data[actual_pos:actual_pos + external_data_size]) @@ -4431,7 +4431,7 @@ class TestFunctional(unittest.TestCase): props = self._GetPropTree(dtb, ['offset', 'image-pos', 'size', 'uncomp-size']) orig = self._decompress(data) - self.assertEquals(COMPRESS_DATA + U_BOOT_DATA, orig) + self.assertEqual(COMPRESS_DATA + U_BOOT_DATA, orig) # Do a sanity check on various fields image = control.images['image'] @@ -4475,7 +4475,7 @@ class TestFunctional(unittest.TestCase): 'uncomp-size']) orig = self._decompress(data) - self.assertEquals(COMPRESS_DATA + COMPRESS_DATA + U_BOOT_DATA, orig) + self.assertEqual(COMPRESS_DATA + COMPRESS_DATA + U_BOOT_DATA, orig) # Do a sanity check on various fields image = control.images['image'] @@ -4519,7 +4519,7 @@ class TestFunctional(unittest.TestCase): props = self._GetPropTree(dtb, ['offset', 'image-pos', 'size', 'uncomp-size']) orig = self._decompress(data) - self.assertEquals(COMPRESS_DATA + U_BOOT_DATA, orig) + self.assertEqual(COMPRESS_DATA + U_BOOT_DATA, orig) expected = { 'section/blob:offset': 0, 'section/blob:size': len(COMPRESS_DATA), @@ -4545,7 +4545,7 @@ class TestFunctional(unittest.TestCase): props = self._GetPropTree(dtb, ['offset', 'image-pos', 'size', 'uncomp-size']) orig = self._decompress(data) - self.assertEquals(COMPRESS_DATA + U_BOOT_DATA, orig) + self.assertEqual(COMPRESS_DATA + U_BOOT_DATA, orig) expected = { 'section/blob:offset': 0, 'section/blob:size': len(COMPRESS_DATA), @@ -4580,7 +4580,7 @@ class TestFunctional(unittest.TestCase): 'uncomp-size']) base = data[len(U_BOOT_DATA):] - self.assertEquals(U_BOOT_DATA, base[:len(U_BOOT_DATA)]) + self.assertEqual(U_BOOT_DATA, base[:len(U_BOOT_DATA)]) rest = base[len(U_BOOT_DATA):] # Check compressed data @@ -4588,22 +4588,22 @@ class TestFunctional(unittest.TestCase): expect1 = bintool.compress(COMPRESS_DATA + U_BOOT_DATA) data1 = rest[:len(expect1)] section1 = self._decompress(data1) - self.assertEquals(expect1, data1) - self.assertEquals(COMPRESS_DATA + U_BOOT_DATA, section1) + self.assertEqual(expect1, data1) + self.assertEqual(COMPRESS_DATA + U_BOOT_DATA, section1) rest1 = rest[len(expect1):] expect2 = bintool.compress(COMPRESS_DATA + COMPRESS_DATA) data2 = rest1[:len(expect2)] section2 = self._decompress(data2) - self.assertEquals(expect2, data2) - self.assertEquals(COMPRESS_DATA + COMPRESS_DATA, section2) + self.assertEqual(expect2, data2) + self.assertEqual(COMPRESS_DATA + COMPRESS_DATA, section2) rest2 = rest1[len(expect2):] expect_size = (len(U_BOOT_DATA) + len(U_BOOT_DATA) + len(expect1) + len(expect2) + len(U_BOOT_DATA)) - #self.assertEquals(expect_size, len(data)) + #self.assertEqual(expect_size, len(data)) - #self.assertEquals(U_BOOT_DATA, rest2) + #self.assertEqual(U_BOOT_DATA, rest2) self.maxDiff = None expected = { @@ -4695,7 +4695,7 @@ class TestFunctional(unittest.TestCase): u_boot = image.GetEntries()['section'].GetEntries()['u-boot'] - self.assertEquals(U_BOOT_DATA, u_boot.ReadData()) + self.assertEqual(U_BOOT_DATA, u_boot.ReadData()) def testTplNoDtb(self): """Test that an image with tpl/u-boot-tpl-nodtb.bin can be created""" @@ -5526,7 +5526,7 @@ fdt fdtmap Extract the devicetree blob from the fdtmap segments, entry = elf.read_loadable_segments(elf_data) # We assume there are two segments - self.assertEquals(2, len(segments)) + self.assertEqual(2, len(segments)) atf1 = dtb.GetNode('/images/atf-1') _, start, data = segments[0] @@ -6107,7 +6107,7 @@ fdt fdtmap Extract the devicetree blob from the fdtmap data = bintool.compress(COMPRESS_DATA) self.assertNotEqual(COMPRESS_DATA, data) orig = bintool.decompress(data) - self.assertEquals(COMPRESS_DATA, orig) + self.assertEqual(COMPRESS_DATA, orig) def testCompUtilVersions(self): """Test tool version of compression algorithms""" @@ -6125,7 +6125,7 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertNotEqual(COMPRESS_DATA, data) data += tools.get_bytes(0, 64) orig = bintool.decompress(data) - self.assertEquals(COMPRESS_DATA, orig) + self.assertEqual(COMPRESS_DATA, orig) def testCompressDtbZstd(self): """Test that zstd compress of device-tree files failed""" diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py index 6b88ed815d6..0ac9fc7e44f 100644 --- a/tools/buildman/func_test.py +++ b/tools/buildman/func_test.py @@ -807,27 +807,27 @@ CONFIG_LOCALVERSION=y params, warnings = self._boards.scan_defconfigs(src, src) # We should get two boards - self.assertEquals(2, len(params)) + self.assertEqual(2, len(params)) self.assertFalse(warnings) first = 0 if params[0]['target'] == 'board0' else 1 board0 = params[first] board2 = params[1 - first] - self.assertEquals('arm', board0['arch']) - self.assertEquals('armv7', board0['cpu']) - self.assertEquals('-', board0['soc']) - self.assertEquals('Tester', board0['vendor']) - self.assertEquals('ARM Board 0', board0['board']) - self.assertEquals('config0', board0['config']) - self.assertEquals('board0', board0['target']) - - self.assertEquals('powerpc', board2['arch']) - self.assertEquals('ppc', board2['cpu']) - self.assertEquals('mpc85xx', board2['soc']) - self.assertEquals('Tester', board2['vendor']) - self.assertEquals('PowerPC board 1', board2['board']) - self.assertEquals('config2', board2['config']) - self.assertEquals('board2', board2['target']) + self.assertEqual('arm', board0['arch']) + self.assertEqual('armv7', board0['cpu']) + self.assertEqual('-', board0['soc']) + self.assertEqual('Tester', board0['vendor']) + self.assertEqual('ARM Board 0', board0['board']) + self.assertEqual('config0', board0['config']) + self.assertEqual('board0', board0['target']) + + self.assertEqual('powerpc', board2['arch']) + self.assertEqual('ppc', board2['cpu']) + self.assertEqual('mpc85xx', board2['soc']) + self.assertEqual('Tester', board2['vendor']) + self.assertEqual('PowerPC board 1', board2['board']) + self.assertEqual('config2', board2['config']) + self.assertEqual('board2', board2['target']) def test_output_is_new(self): """Test detecting new changes to Kconfig""" @@ -898,7 +898,7 @@ Active aarch64 armv8 - armltd total_compute board2 params_list, warnings = self._boards.build_board_list(config_dir, src) # There should be two boards no warnings - self.assertEquals(2, len(params_list)) + self.assertEqual(2, len(params_list)) self.assertFalse(warnings) # Set an invalid status line in the file @@ -907,12 +907,12 @@ Active aarch64 armv8 - armltd total_compute board2 for line in orig_data.splitlines(keepends=True)] tools.write_file(main, ''.join(lines), binary=False) params_list, warnings = self._boards.build_board_list(config_dir, src) - self.assertEquals(2, len(params_list)) + self.assertEqual(2, len(params_list)) params = params_list[0] if params['target'] == 'board2': params = params_list[1] - self.assertEquals('-', params['status']) - self.assertEquals(["WARNING: Other: unknown status for 'board0'"], + self.assertEqual('-', params['status']) + self.assertEqual(["WARNING: Other: unknown status for 'board0'"], warnings) # Remove the status line (S:) from a file @@ -920,39 +920,39 @@ Active aarch64 armv8 - armltd total_compute board2 if not line.startswith('S:')] tools.write_file(main, ''.join(lines), binary=False) params_list, warnings = self._boards.build_board_list(config_dir, src) - self.assertEquals(2, len(params_list)) - self.assertEquals(["WARNING: -: unknown status for 'board0'"], warnings) + self.assertEqual(2, len(params_list)) + self.assertEqual(["WARNING: -: unknown status for 'board0'"], warnings) # Remove the configs/ line (F:) from a file - this is the last line data = ''.join(orig_data.splitlines(keepends=True)[:-1]) tools.write_file(main, data, binary=False) params_list, warnings = self._boards.build_board_list(config_dir, src) - self.assertEquals(2, len(params_list)) - self.assertEquals(["WARNING: no maintainers for 'board0'"], warnings) + self.assertEqual(2, len(params_list)) + self.assertEqual(["WARNING: no maintainers for 'board0'"], warnings) # Mark a board as orphaned - this should give a warning lines = ['S: Orphaned' if line.startswith('S') else line for line in orig_data.splitlines(keepends=True)] tools.write_file(main, ''.join(lines), binary=False) params_list, warnings = self._boards.build_board_list(config_dir, src) - self.assertEquals(2, len(params_list)) - self.assertEquals(["WARNING: no maintainers for 'board0'"], warnings) + self.assertEqual(2, len(params_list)) + self.assertEqual(["WARNING: no maintainers for 'board0'"], warnings) # Change the maintainer to '-' - this should give a warning lines = ['M: -' if line.startswith('M') else line for line in orig_data.splitlines(keepends=True)] tools.write_file(main, ''.join(lines), binary=False) params_list, warnings = self._boards.build_board_list(config_dir, src) - self.assertEquals(2, len(params_list)) - self.assertEquals(["WARNING: -: unknown status for 'board0'"], warnings) + self.assertEqual(2, len(params_list)) + self.assertEqual(["WARNING: -: unknown status for 'board0'"], warnings) # Remove the maintainer line (M:) from a file lines = [line for line in orig_data.splitlines(keepends=True) if not line.startswith('M:')] tools.write_file(main, ''.join(lines), binary=False) params_list, warnings = self._boards.build_board_list(config_dir, src) - self.assertEquals(2, len(params_list)) - self.assertEquals(["WARNING: no maintainers for 'board0'"], warnings) + self.assertEqual(2, len(params_list)) + self.assertEqual(["WARNING: no maintainers for 'board0'"], warnings) # Move the contents of the second file into this one, removing the # second file, to check multiple records in a single file. @@ -960,14 +960,14 @@ Active aarch64 armv8 - armltd total_compute board2 tools.write_file(main, both_data, binary=False) os.remove(other) params_list, warnings = self._boards.build_board_list(config_dir, src) - self.assertEquals(2, len(params_list)) + self.assertEqual(2, len(params_list)) self.assertFalse(warnings) # Add another record, this should be ignored with a warning extra = '\n\nAnother\nM: Fred\nF: configs/board9_defconfig\nS: other\n' tools.write_file(main, both_data + extra, binary=False) params_list, warnings = self._boards.build_board_list(config_dir, src) - self.assertEquals(2, len(params_list)) + self.assertEqual(2, len(params_list)) self.assertFalse(warnings) # Add another TARGET to the Kconfig @@ -983,8 +983,8 @@ endif tools.write_file(kc_file, orig_kc_data + extra) params_list, warnings = self._boards.build_board_list(config_dir, src, warn_targets=True) - self.assertEquals(2, len(params_list)) - self.assertEquals( + self.assertEqual(2, len(params_list)) + self.assertEqual( ['WARNING: board2_defconfig: Duplicate TARGET_xxx: board2 and other'], warnings) @@ -994,8 +994,8 @@ endif tools.write_file(kc_file, b''.join(lines)) params_list, warnings = self._boards.build_board_list(config_dir, src, warn_targets=True) - self.assertEquals(2, len(params_list)) - self.assertEquals( + self.assertEqual(2, len(params_list)) + self.assertEqual( ['WARNING: board2_defconfig: No TARGET_BOARD2 enabled'], warnings) tools.write_file(kc_file, orig_kc_data) @@ -1004,7 +1004,7 @@ endif data = ''.join(both_data.splitlines(keepends=True)[:-1]) tools.write_file(main, data + 'N: oa.*2\n', binary=False) params_list, warnings = self._boards.build_board_list(config_dir, src) - self.assertEquals(2, len(params_list)) + self.assertEqual(2, len(params_list)) self.assertFalse(warnings) def testRegenBoards(self): diff --git a/tools/buildman/test.py b/tools/buildman/test.py index f92add7a7c5..79164bd1993 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -584,7 +584,7 @@ class TestBuild(unittest.TestCase): if use_network: with test_util.capture_sys_output() as (stdout, stderr): url = self.toolchains.LocateArchUrl('arm') - self.assertRegexpMatches(url, 'https://www.kernel.org/pub/tools/' + self.assertRegex(url, 'https://www.kernel.org/pub/tools/' 'crosstool/files/bin/x86_64/.*/' 'x86_64-gcc-.*-nolibc[-_]arm-.*linux-gnueabi.tar.xz') -- cgit v1.3.1 From 698b60a6a23a29a556d5a00c5eead63fb3fdd4f3 Mon Sep 17 00:00:00 2001 From: Brandon Maier Date: Tue, 4 Jun 2024 16:16:06 +0000 Subject: tools: binman: fix deprecated Python ConfigParser methods The method `ConfigParser.readfp()` is marked deprecated[1]. In Python 3.12 this method have been removed, so replace it with `ConfigParser.read_file()`. [1] https://docs.python.org/3.11/library/configparser.html#configparser.ConfigParser.readfp Signed-off-by: Brandon Maier CC: Simon Glass Reviewed-by: Simon Glass --- tools/buildman/bsettings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/buildman/bsettings.py b/tools/buildman/bsettings.py index e225ac2ca0f..aea724fc559 100644 --- a/tools/buildman/bsettings.py +++ b/tools/buildman/bsettings.py @@ -29,7 +29,7 @@ def setup(fname=''): settings.read(config_fname) def add_file(data): - settings.readfp(io.StringIO(data)) + settings.read_file(io.StringIO(data)) def get_items(section): """Get the items from a section of the config. -- cgit v1.3.1 From a1488750935342f1750bd4f3cbd999688523898e Mon Sep 17 00:00:00 2001 From: Brandon Maier Date: Tue, 4 Jun 2024 16:16:07 +0000 Subject: tools: patman: fix deprecated Python ConfigParser methods The method `ConfigParser.readfp()` is marked deprecated[1]. In Python 3.12 this method have been removed, so replace it with `ConfigParser.read_file()`. [1] https://docs.python.org/3.11/library/configparser.html#configparser.ConfigParser.readfp Signed-off-by: Brandon Maier CC: Simon Glass Reviewed-by: Simon Glass --- tools/patman/settings.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/patman/settings.py b/tools/patman/settings.py index 636983e32da..68c93e313b3 100644 --- a/tools/patman/settings.py +++ b/tools/patman/settings.py @@ -59,25 +59,25 @@ class _ProjectConfigParser(ConfigParser.ConfigParser): # Check to make sure that bogus project gets general alias. >>> config = _ProjectConfigParser("zzz") - >>> config.readfp(StringIO(sample_config)) + >>> config.read_file(StringIO(sample_config)) >>> str(config.get("alias", "enemies")) 'Evil ' # Check to make sure that alias gets overridden by project. >>> config = _ProjectConfigParser("sm") - >>> config.readfp(StringIO(sample_config)) + >>> config.read_file(StringIO(sample_config)) >>> str(config.get("alias", "enemies")) 'Green G. ' # Check to make sure that settings get merged with project. >>> config = _ProjectConfigParser("linux") - >>> config.readfp(StringIO(sample_config)) + >>> config.read_file(StringIO(sample_config)) >>> sorted((str(a), str(b)) for (a, b) in config.items("settings")) [('am_hero', 'True'), ('check_patch_use_tree', 'True'), ('process_tags', 'False')] # Check to make sure that settings works with unknown project. >>> config = _ProjectConfigParser("unknown") - >>> config.readfp(StringIO(sample_config)) + >>> config.read_file(StringIO(sample_config)) >>> sorted((str(a), str(b)) for (a, b) in config.items("settings")) [('am_hero', 'True')] """ -- cgit v1.3.1 From cc560eac51ea19742f4ea166b86a34b1c7ceb31a Mon Sep 17 00:00:00 2001 From: Quentin Schulz Date: Tue, 11 Jun 2024 15:04:24 +0200 Subject: dm: core: fix misleading debug message when matching compatible A driver can have multiple compatible. When the id->compatible matches for that driver, the first compatible supported by the driver is currently returned, which gives the following confusing message: - found match at 'rk3588_syscon': 'rockchip,rk3588-sys-grf' matches 'rockchip,rk3588-pmugrf' Considering that the compatible passed in argument is necessarily the one that exactly matched to enter this code path, there's no need to do some elaborate logic, just print the driver name and the compatible passed in argument. Fixes: d3e773613b6d ("dm: core: Use U-Boot logging instead of pr_debug()") Signed-off-by: Quentin Schulz Reviewed-by: Simon Glass --- drivers/core/lists.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/core/lists.c b/drivers/core/lists.c index 2839a9b7371..942fe4a4e67 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -246,9 +246,8 @@ int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp, } if (entry->of_match) - log_debug(" - found match at '%s': '%s' matches '%s'\n", - entry->name, entry->of_match->compatible, - id->compatible); + log_debug(" - found match at driver '%s' for '%s'\n", + entry->name, id->compatible); ret = device_bind_with_driver_data(parent, entry, name, id ? id->data : 0, node, &dev); -- cgit v1.3.1 From 29010cd31be2bc6f674459137b5dcc054b77c042 Mon Sep 17 00:00:00 2001 From: Quentin Schulz Date: Tue, 11 Jun 2024 15:04:25 +0200 Subject: dm: core: fix signedness in debug messages outp always point to an unsigned type in ofnode_read_u* functions but the format specifier is currently always using signed type. This is an issue since the signed type can only contain half of the unsigned type values above 0. However, this now breaks another usecase. Indeed, ofnode_read_s32_default is actually passing an s32 but it'll be printed as a u32 instead. But since the function is called u32, it makes more sense to have it print an unsigned value. This was discovered because arm,smc-id = <0x82000010>; on RK3588S is above the max signed value and therefore would return a negative signed decimal value instead of its proper unsigned one. Fixes: fa12dfa08a7b ("dm: core: support reading a single indexed u64 value") Fixes: 4bb7075c830c ("dm: core: support reading a single indexed u32 value") Fixes: 7e5196c409f1 ("dm: core: Add ofnode function to read a 64-bit int") Fixes: 9e51204527dc ("dm: core: Add operations on device tree references") Signed-off-by: Quentin Schulz Reviewed-by: Simon Glass --- drivers/core/ofnode.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index 9a5eaaa4d13..9ff46460e7d 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -326,7 +326,7 @@ int ofnode_read_u8(ofnode node, const char *propname, u8 *outp) return -EINVAL; } *outp = *cell; - debug("%#x (%d)\n", *outp, *outp); + debug("%#x (%u)\n", *outp, *outp); return 0; } @@ -357,7 +357,7 @@ int ofnode_read_u16(ofnode node, const char *propname, u16 *outp) return -EINVAL; } *outp = be16_to_cpup(cell); - debug("%#x (%d)\n", *outp, *outp); + debug("%#x (%u)\n", *outp, *outp); return 0; } @@ -409,7 +409,7 @@ int ofnode_read_u32_index(ofnode node, const char *propname, int index, } *outp = fdt32_to_cpu(cell[index]); - debug("%#x (%d)\n", *outp, *outp); + debug("%#x (%u)\n", *outp, *outp); return 0; } @@ -439,7 +439,7 @@ int ofnode_read_u64_index(ofnode node, const char *propname, int index, } *outp = fdt64_to_cpu(cell[index]); - debug("%#llx (%lld)\n", *outp, *outp); + debug("%#llx (%llu)\n", *outp, *outp); return 0; } @@ -479,7 +479,7 @@ int ofnode_read_u64(ofnode node, const char *propname, u64 *outp) return -EINVAL; } *outp = fdt64_to_cpu(cell[0]); - debug("%#llx (%lld)\n", (unsigned long long)*outp, + debug("%#llx (%llu)\n", (unsigned long long)*outp, (unsigned long long)*outp); return 0; -- cgit v1.3.1 From 6afdb1585112ca5171613f3f4957de4acbe1048d Mon Sep 17 00:00:00 2001 From: Quentin Schulz Date: Tue, 11 Jun 2024 15:04:26 +0200 Subject: dm: core: migrate debug() messages to use dm_warn Prior to that, seeing the debug() messages required to enable DM_DEBUG which defines DEBUG (and then _DEBUG) which in turn makes failing assert() calls reset U-Boot which isn't necessarily what is desired. Instead, let's migrate to dm_warn which is using log_debug when unset or log_warn when set. While at it, reword the DM_DEBUG symbol in Kconfig to explain what it now actually does. Signed-off-by: Quentin Schulz Reviewed-by: Simon Glass --- drivers/core/device.c | 2 +- drivers/core/fdtaddr.c | 7 +++-- drivers/core/lists.c | 2 +- drivers/core/of_access.c | 51 +++++++++++++++--------------- drivers/core/of_addr.c | 41 ++++++++++++------------ drivers/core/of_extra.c | 33 ++++++++++---------- drivers/core/ofnode.c | 81 ++++++++++++++++++++++++------------------------ drivers/core/regmap.c | 57 +++++++++++++++++----------------- drivers/core/root.c | 14 ++++----- drivers/core/uclass.c | 4 +-- 10 files changed, 149 insertions(+), 143 deletions(-) diff --git a/drivers/core/device.c b/drivers/core/device.c index 18e2bd02dd5..779f371b9d5 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -58,7 +58,7 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, ret = uclass_get(drv->id, &uc); if (ret) { - debug("Missing uclass for driver %s\n", drv->name); + dm_warn("Missing uclass for driver %s\n", drv->name); return ret; } diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c index 6be8ea0c0a9..9e59968df01 100644 --- a/drivers/core/fdtaddr.c +++ b/drivers/core/fdtaddr.c @@ -15,6 +15,7 @@ #include #include #include +#include DECLARE_GLOBAL_DATA_PTR; @@ -32,19 +33,19 @@ fdt_addr_t devfdt_get_addr_index(const struct udevice *dev, int index) na = fdt_address_cells(gd->fdt_blob, parent); if (na < 1) { - debug("bad #address-cells\n"); + dm_warn("bad #address-cells\n"); return FDT_ADDR_T_NONE; } ns = fdt_size_cells(gd->fdt_blob, parent); if (ns < 0) { - debug("bad #size-cells\n"); + dm_warn("bad #size-cells\n"); return FDT_ADDR_T_NONE; } reg = fdt_getprop(gd->fdt_blob, offset, "reg", &len); if (!reg || (len <= (index * sizeof(fdt32_t) * (na + ns)))) { - debug("Req index out of range\n"); + dm_warn("Req index out of range\n"); return FDT_ADDR_T_NONE; } diff --git a/drivers/core/lists.c b/drivers/core/lists.c index 942fe4a4e67..bd0ab4f16c9 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -144,7 +144,7 @@ int device_bind_driver_to_node(struct udevice *parent, const char *drv_name, drv = lists_driver_lookup_name(drv_name); if (!drv) { - debug("Cannot find driver '%s'\n", drv_name); + dm_warn("Cannot find driver '%s'\n", drv_name); return -ENOENT; } ret = device_bind_with_driver_data(parent, drv, dev_name, 0 /* data */, diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c index 41f2e09b9c2..d05be273e7b 100644 --- a/drivers/core/of_access.c +++ b/drivers/core/of_access.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -489,17 +490,17 @@ int of_read_u8(const struct device_node *np, const char *propname, u8 *outp) { const u8 *val; - debug("%s: %s: ", __func__, propname); + dm_warn("%s: %s: ", __func__, propname); if (!np) return -EINVAL; val = of_find_property_value_of_size(np, propname, sizeof(*outp)); if (IS_ERR(val)) { - debug("(not found)\n"); + dm_warn("(not found)\n"); return PTR_ERR(val); } *outp = *val; - debug("%#x (%d)\n", *outp, *outp); + dm_warn("%#x (%d)\n", *outp, *outp); return 0; } @@ -508,17 +509,17 @@ int of_read_u16(const struct device_node *np, const char *propname, u16 *outp) { const __be16 *val; - debug("%s: %s: ", __func__, propname); + dm_warn("%s: %s: ", __func__, propname); if (!np) return -EINVAL; val = of_find_property_value_of_size(np, propname, sizeof(*outp)); if (IS_ERR(val)) { - debug("(not found)\n"); + dm_warn("(not found)\n"); return PTR_ERR(val); } *outp = be16_to_cpup(val); - debug("%#x (%d)\n", *outp, *outp); + dm_warn("%#x (%d)\n", *outp, *outp); return 0; } @@ -533,14 +534,14 @@ int of_read_u32_array(const struct device_node *np, const char *propname, { const __be32 *val; - debug("%s: %s: ", __func__, propname); + dm_warn("%s: %s: ", __func__, propname); val = of_find_property_value_of_size(np, propname, sz * sizeof(*out_values)); if (IS_ERR(val)) return PTR_ERR(val); - debug("size %zd\n", sz); + dm_warn("size %zd\n", sz); while (sz--) *out_values++ = be32_to_cpup(val++); @@ -552,19 +553,19 @@ int of_read_u32_index(const struct device_node *np, const char *propname, { const __be32 *val; - debug("%s: %s: ", __func__, propname); + dm_warn("%s: %s: ", __func__, propname); if (!np) return -EINVAL; val = of_find_property_value_of_size(np, propname, sizeof(*outp) * (index + 1)); if (IS_ERR(val)) { - debug("(not found)\n"); + dm_warn("(not found)\n"); return PTR_ERR(val); } *outp = be32_to_cpup(val + index); - debug("%#x (%d)\n", *outp, *outp); + dm_warn("%#x (%d)\n", *outp, *outp); return 0; } @@ -574,20 +575,20 @@ int of_read_u64_index(const struct device_node *np, const char *propname, { const __be64 *val; - debug("%s: %s: ", __func__, propname); + dm_warn("%s: %s: ", __func__, propname); if (!np) return -EINVAL; val = of_find_property_value_of_size(np, propname, sizeof(*outp) * (index + 1)); if (IS_ERR(val)) { - debug("(not found)\n"); + dm_warn("(not found)\n"); return PTR_ERR(val); } *outp = be64_to_cpup(val + index); - debug("%#llx (%lld)\n", (unsigned long long)*outp, - (unsigned long long)*outp); + dm_warn("%#llx (%lld)\n", (unsigned long long)*outp, + (unsigned long long)*outp); return 0; } @@ -620,7 +621,7 @@ int of_property_match_string(const struct device_node *np, const char *propname, l = strnlen(p, end - p) + 1; if (p + l > end) return -EILSEQ; - debug("comparing %s with %s\n", string, p); + dm_warn("comparing %s with %s\n", string, p); if (strcmp(string, p) == 0) return i; /* Found it; return index */ } @@ -707,17 +708,17 @@ static int __of_parse_phandle_with_args(const struct device_node *np, if (cells_name || cur_index == index) { node = of_find_node_by_phandle(NULL, phandle); if (!node) { - debug("%s: could not find phandle\n", - np->full_name); + dm_warn("%s: could not find phandle\n", + np->full_name); goto err; } } if (cells_name) { if (of_read_u32(node, cells_name, &count)) { - debug("%s: could not get %s for %s\n", - np->full_name, cells_name, - node->full_name); + dm_warn("%s: could not get %s for %s\n", + np->full_name, cells_name, + node->full_name); goto err; } } else { @@ -729,8 +730,8 @@ static int __of_parse_phandle_with_args(const struct device_node *np, * remaining property data length */ if (list + count > list_end) { - debug("%s: arguments longer than property\n", - np->full_name); + dm_warn("%s: arguments longer than property\n", + np->full_name); goto err; } } @@ -825,8 +826,8 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np, strncpy(ap->stem, stem, stem_len); ap->stem[stem_len] = 0; list_add_tail(&ap->link, &aliases_lookup); - debug("adding DT alias:%s: stem=%s id=%i node=%s\n", - ap->alias, ap->stem, ap->id, of_node_full_name(np)); + dm_warn("adding DT alias:%s: stem=%s id=%i node=%s\n", + ap->alias, ap->stem, ap->id, of_node_full_name(np)); } int of_alias_scan(void) diff --git a/drivers/core/of_addr.c b/drivers/core/of_addr.c index d7913ab3d2f..c893447a1b1 100644 --- a/drivers/core/of_addr.c +++ b/drivers/core/of_addr.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -26,7 +27,7 @@ static struct of_bus *of_match_bus(struct device_node *np); #ifdef DEBUG static void of_dump_addr(const char *s, const __be32 *addr, int na) { - debug("%s", s); + dm_warn("%s", s); while (na--) pr_cont(" %08x", be32_to_cpu(*(addr++))); pr_cont("\n"); @@ -65,9 +66,9 @@ static u64 of_bus_default_map(__be32 *addr, const __be32 *range, s = of_read_number(range + na + pna, ns); da = of_read_number(addr, na); - debug("default map, cp=%llx, s=%llx, da=%llx\n", - (unsigned long long)cp, (unsigned long long)s, - (unsigned long long)da); + dm_warn("default map, cp=%llx, s=%llx, da=%llx\n", + (unsigned long long)cp, (unsigned long long)s, + (unsigned long long)da); if (da < cp || da >= (cp + s)) return OF_BAD_ADDR; @@ -193,17 +194,17 @@ static int of_translate_one(const struct device_node *parent, ranges = of_get_property(parent, rprop, &rlen); if (ranges == NULL && !of_empty_ranges_quirk(parent) && strcmp(rprop, "dma-ranges")) { - debug("no ranges; cannot translate\n"); + dm_warn("no ranges; cannot translate\n"); return 1; } if (ranges == NULL || rlen == 0) { offset = of_read_number(addr, na); memset(addr, 0, pna * 4); - debug("empty ranges; 1:1 translation\n"); + dm_warn("empty ranges; 1:1 translation\n"); goto finish; } - debug("walking ranges...\n"); + dm_warn("walking ranges...\n"); /* Now walk through the ranges */ rlen /= 4; @@ -214,14 +215,14 @@ static int of_translate_one(const struct device_node *parent, break; } if (offset == OF_BAD_ADDR) { - debug("not found !\n"); + dm_warn("not found !\n"); return 1; } memcpy(addr, ranges + na, 4 * pna); finish: of_dump_addr("parent translation for:", addr, pna); - debug("with offset: %llx\n", (unsigned long long)offset); + dm_warn("with offset: %llx\n", (unsigned long long)offset); /* Translate it into parent bus space */ return pbus->translate(addr, offset, pna); @@ -246,7 +247,7 @@ static u64 __of_translate_address(const struct device_node *dev, int na, ns, pna, pns; u64 result = OF_BAD_ADDR; - debug("** translation for device %s **\n", of_node_full_name(dev)); + dm_warn("** translation for device %s **\n", of_node_full_name(dev)); /* Increase refcount at current level */ (void)of_node_get(dev); @@ -260,13 +261,13 @@ static u64 __of_translate_address(const struct device_node *dev, /* Count address cells & copy address locally */ bus->count_cells(dev, &na, &ns); if (!OF_CHECK_COUNTS(na, ns)) { - debug("Bad cell count for %s\n", of_node_full_name(dev)); + dm_warn("Bad cell count for %s\n", of_node_full_name(dev)); goto bail; } memcpy(addr, in_addr, na * 4); - debug("bus is %s (na=%d, ns=%d) on %s\n", bus->name, na, ns, - of_node_full_name(parent)); + dm_warn("bus is %s (na=%d, ns=%d) on %s\n", bus->name, na, ns, + of_node_full_name(parent)); of_dump_addr("translating address:", addr, na); /* Translate */ @@ -278,7 +279,7 @@ static u64 __of_translate_address(const struct device_node *dev, /* If root, we have finished */ if (parent == NULL) { - debug("reached root node\n"); + dm_warn("reached root node\n"); result = of_read_number(addr, na); break; } @@ -287,13 +288,13 @@ static u64 __of_translate_address(const struct device_node *dev, pbus = of_match_bus(parent); pbus->count_cells(dev, &pna, &pns); if (!OF_CHECK_COUNTS(pna, pns)) { - debug("Bad cell count for %s\n", - of_node_full_name(dev)); + dm_warn("Bad cell count for %s\n", + of_node_full_name(dev)); break; } - debug("parent bus is %s (na=%d, ns=%d) on %s\n", pbus->name, - pna, pns, of_node_full_name(parent)); + dm_warn("parent bus is %s (na=%d, ns=%d) on %s\n", pbus->name, + pna, pns, of_node_full_name(parent)); /* Apply bus translation */ if (of_translate_one(dev, bus, pbus, addr, na, ns, pna, rprop)) @@ -358,8 +359,8 @@ int of_get_dma_range(const struct device_node *dev, phys_addr_t *cpu, } if (!dev || !ranges) { - debug("no dma-ranges found for node %s\n", - of_node_full_name(dev)); + dm_warn("no dma-ranges found for node %s\n", + of_node_full_name(dev)); ret = -ENOENT; goto out; } diff --git a/drivers/core/of_extra.c b/drivers/core/of_extra.c index a3ebe9e9c24..bfc1e3441b1 100644 --- a/drivers/core/of_extra.c +++ b/drivers/core/of_extra.c @@ -9,6 +9,7 @@ #include #include #include +#include int ofnode_read_fmap_entry(ofnode node, struct fmap_entry *entry) { @@ -16,13 +17,13 @@ int ofnode_read_fmap_entry(ofnode node, struct fmap_entry *entry) ofnode subnode; if (ofnode_read_u32(node, "image-pos", &entry->offset)) { - debug("Node '%s' has bad/missing 'image-pos' property\n", - ofnode_get_name(node)); + dm_warn("Node '%s' has bad/missing 'image-pos' property\n", + ofnode_get_name(node)); return log_msg_ret("image-pos", -ENOENT); } if (ofnode_read_u32(node, "size", &entry->length)) { - debug("Node '%s' has bad/missing 'size' property\n", - ofnode_get_name(node)); + dm_warn("Node '%s' has bad/missing 'size' property\n", + ofnode_get_name(node)); return log_msg_ret("size", -ENOENT); } entry->used = ofnode_read_s32_default(node, "used", entry->length); @@ -57,17 +58,17 @@ int ofnode_decode_region(ofnode node, const char *prop_name, fdt_addr_t *basep, const fdt_addr_t *cell; int len; - debug("%s: %s: %s\n", __func__, ofnode_get_name(node), prop_name); + dm_warn("%s: %s: %s\n", __func__, ofnode_get_name(node), prop_name); cell = ofnode_get_property(node, prop_name, &len); if (!cell || (len < sizeof(fdt_addr_t) * 2)) { - debug("cell=%p, len=%d\n", cell, len); + dm_warn("cell=%p, len=%d\n", cell, len); return -1; } *basep = fdt_addr_to_cpu(*cell); *sizep = fdt_size_to_cpu(cell[1]); - debug("%s: base=%08lx, size=%lx\n", __func__, (ulong)*basep, - (ulong)*sizep); + dm_warn("%s: base=%08lx, size=%lx\n", __func__, (ulong)*basep, + (ulong)*sizep); return 0; } @@ -85,7 +86,7 @@ int ofnode_decode_memory_region(ofnode config_node, const char *mem_type, if (!ofnode_valid(config_node)) { config_node = ofnode_path("/config"); if (!ofnode_valid(config_node)) { - debug("%s: Cannot find /config node\n", __func__); + dm_warn("%s: Cannot find /config node\n", __func__); return -ENOENT; } } @@ -96,14 +97,14 @@ int ofnode_decode_memory_region(ofnode config_node, const char *mem_type, suffix); mem = ofnode_read_string(config_node, prop_name); if (!mem) { - debug("%s: No memory type for '%s', using /memory\n", __func__, - prop_name); + dm_warn("%s: No memory type for '%s', using /memory\n", __func__, + prop_name); mem = "/memory"; } node = ofnode_path(mem); if (!ofnode_valid(node)) { - debug("%s: Failed to find node '%s'\n", __func__, mem); + dm_warn("%s: Failed to find node '%s'\n", __func__, mem); return -ENOENT; } @@ -112,8 +113,8 @@ int ofnode_decode_memory_region(ofnode config_node, const char *mem_type, * use the first */ if (ofnode_decode_region(node, "reg", &base, &size)) { - debug("%s: Failed to decode memory region %s\n", __func__, - mem); + dm_warn("%s: Failed to decode memory region %s\n", __func__, + mem); return -EINVAL; } @@ -121,8 +122,8 @@ int ofnode_decode_memory_region(ofnode config_node, const char *mem_type, suffix); if (ofnode_decode_region(config_node, prop_name, &offset, &offset_size)) { - debug("%s: Failed to decode memory region '%s'\n", __func__, - prop_name); + dm_warn("%s: Failed to decode memory region '%s'\n", __func__, + prop_name); return -EINVAL; } diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index 9ff46460e7d..4d563b47a5a 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -314,7 +315,7 @@ int ofnode_read_u8(ofnode node, const char *propname, u8 *outp) int len; assert(ofnode_valid(node)); - debug("%s: %s: ", __func__, propname); + dm_warn("%s: %s: ", __func__, propname); if (ofnode_is_np(node)) return of_read_u8(ofnode_to_np(node), propname, outp); @@ -322,11 +323,11 @@ int ofnode_read_u8(ofnode node, const char *propname, u8 *outp) cell = fdt_getprop(gd->fdt_blob, ofnode_to_offset(node), propname, &len); if (!cell || len < sizeof(*cell)) { - debug("(not found)\n"); + dm_warn("(not found)\n"); return -EINVAL; } *outp = *cell; - debug("%#x (%u)\n", *outp, *outp); + dm_warn("%#x (%u)\n", *outp, *outp); return 0; } @@ -345,7 +346,7 @@ int ofnode_read_u16(ofnode node, const char *propname, u16 *outp) int len; assert(ofnode_valid(node)); - debug("%s: %s: ", __func__, propname); + dm_warn("%s: %s: ", __func__, propname); if (ofnode_is_np(node)) return of_read_u16(ofnode_to_np(node), propname, outp); @@ -353,11 +354,11 @@ int ofnode_read_u16(ofnode node, const char *propname, u16 *outp) cell = fdt_getprop(gd->fdt_blob, ofnode_to_offset(node), propname, &len); if (!cell || len < sizeof(*cell)) { - debug("(not found)\n"); + dm_warn("(not found)\n"); return -EINVAL; } *outp = be16_to_cpup(cell); - debug("%#x (%u)\n", *outp, *outp); + dm_warn("%#x (%u)\n", *outp, *outp); return 0; } @@ -390,7 +391,7 @@ int ofnode_read_u32_index(ofnode node, const char *propname, int index, int len; assert(ofnode_valid(node)); - debug("%s: %s: ", __func__, propname); + dm_warn("%s: %s: ", __func__, propname); if (ofnode_is_np(node)) return of_read_u32_index(ofnode_to_np(node), propname, index, @@ -399,17 +400,17 @@ int ofnode_read_u32_index(ofnode node, const char *propname, int index, cell = fdt_getprop(ofnode_to_fdt(node), ofnode_to_offset(node), propname, &len); if (!cell) { - debug("(not found)\n"); + dm_warn("(not found)\n"); return -EINVAL; } if (len < (sizeof(int) * (index + 1))) { - debug("(not large enough)\n"); + dm_warn("(not large enough)\n"); return -EOVERFLOW; } *outp = fdt32_to_cpu(cell[index]); - debug("%#x (%u)\n", *outp, *outp); + dm_warn("%#x (%u)\n", *outp, *outp); return 0; } @@ -429,17 +430,17 @@ int ofnode_read_u64_index(ofnode node, const char *propname, int index, cell = fdt_getprop(ofnode_to_fdt(node), ofnode_to_offset(node), propname, &len); if (!cell) { - debug("(not found)\n"); + dm_warn("(not found)\n"); return -EINVAL; } if (len < (sizeof(u64) * (index + 1))) { - debug("(not large enough)\n"); + dm_warn("(not large enough)\n"); return -EOVERFLOW; } *outp = fdt64_to_cpu(cell[index]); - debug("%#llx (%llu)\n", *outp, *outp); + dm_warn("%#llx (%llu)\n", *outp, *outp); return 0; } @@ -467,7 +468,7 @@ int ofnode_read_u64(ofnode node, const char *propname, u64 *outp) int len; assert(ofnode_valid(node)); - debug("%s: %s: ", __func__, propname); + dm_warn("%s: %s: ", __func__, propname); if (ofnode_is_np(node)) return of_read_u64(ofnode_to_np(node), propname, outp); @@ -475,12 +476,12 @@ int ofnode_read_u64(ofnode node, const char *propname, u64 *outp) cell = fdt_getprop(ofnode_to_fdt(node), ofnode_to_offset(node), propname, &len); if (!cell || len < sizeof(*cell)) { - debug("(not found)\n"); + dm_warn("(not found)\n"); return -EINVAL; } *outp = fdt64_to_cpu(cell[0]); - debug("%#llx (%llu)\n", (unsigned long long)*outp, - (unsigned long long)*outp); + dm_warn("%#llx (%llu)\n", (unsigned long long)*outp, + (unsigned long long)*outp); return 0; } @@ -498,11 +499,11 @@ bool ofnode_read_bool(ofnode node, const char *propname) bool prop; assert(ofnode_valid(node)); - debug("%s: %s: ", __func__, propname); + dm_warn("%s: %s: ", __func__, propname); prop = ofnode_has_property(node, propname); - debug("%s\n", prop ? "true" : "false"); + dm_warn("%s\n", prop ? "true" : "false"); return prop ? true : false; } @@ -513,7 +514,7 @@ const void *ofnode_read_prop(ofnode node, const char *propname, int *sizep) int len; assert(ofnode_valid(node)); - debug("%s: %s: ", __func__, propname); + dm_warn("%s: %s: ", __func__, propname); if (ofnode_is_np(node)) { struct property *prop = of_find_property( @@ -528,7 +529,7 @@ const void *ofnode_read_prop(ofnode node, const char *propname, int *sizep) propname, &len); } if (!val) { - debug("\n"); + dm_warn("\n"); if (sizep) *sizep = -FDT_ERR_NOTFOUND; return NULL; @@ -549,10 +550,10 @@ const char *ofnode_read_string(ofnode node, const char *propname) return NULL; if (strnlen(str, len) >= len) { - debug("\n"); + dm_warn("\n"); return NULL; } - debug("%s\n", str); + dm_warn("%s\n", str); return str; } @@ -572,7 +573,7 @@ ofnode ofnode_find_subnode(ofnode node, const char *subnode_name) ofnode subnode; assert(ofnode_valid(node)); - debug("%s: %s: ", __func__, subnode_name); + dm_warn("%s: %s: ", __func__, subnode_name); if (ofnode_is_np(node)) { struct device_node *np = ofnode_to_np(node); @@ -587,8 +588,8 @@ ofnode ofnode_find_subnode(ofnode node, const char *subnode_name) ofnode_to_offset(node), subnode_name); subnode = noffset_to_ofnode(node, ooffset); } - debug("%s\n", ofnode_valid(subnode) ? - ofnode_get_name(subnode) : ""); + dm_warn("%s\n", ofnode_valid(subnode) ? + ofnode_get_name(subnode) : ""); return subnode; } @@ -597,7 +598,7 @@ int ofnode_read_u32_array(ofnode node, const char *propname, u32 *out_values, size_t sz) { assert(ofnode_valid(node)); - debug("%s: %s: ", __func__, propname); + dm_warn("%s: %s: ", __func__, propname); if (ofnode_is_np(node)) { return of_read_u32_array(ofnode_to_np(node), propname, @@ -669,7 +670,7 @@ ofnode ofnode_get_parent(ofnode node) const char *ofnode_get_name(ofnode node) { if (!ofnode_valid(node)) { - debug("%s node not valid\n", __func__); + dm_warn("%s node not valid\n", __func__); return NULL; } @@ -1030,7 +1031,7 @@ ofnode ofnode_get_aliases_node(const char *name) if (!prop) return ofnode_null(); - debug("%s: node_path: %s\n", __func__, prop); + dm_warn("%s: node_path: %s\n", __func__, prop); return ofnode_path(prop); } @@ -1053,8 +1054,8 @@ static int decode_timing_property(ofnode node, const char *name, length = ofnode_read_size(node, name); if (length < 0) { - debug("%s: could not find property %s\n", - ofnode_get_name(node), name); + dm_warn("%s: could not find property %s\n", + ofnode_get_name(node), name); return length; } @@ -1299,7 +1300,7 @@ int ofnode_read_pci_addr(ofnode node, enum fdt_pci_space type, int len; int ret = -ENOENT; - debug("%s: %s: ", __func__, propname); + dm_warn("%s: %s: ", __func__, propname); /* * If we follow the pci bus bindings strictly, we should check @@ -1316,8 +1317,8 @@ int ofnode_read_pci_addr(ofnode node, enum fdt_pci_space type, int i; for (i = 0; i < num; i++) { - debug("pci address #%d: %08lx %08lx %08lx\n", i, - (ulong)fdt32_to_cpu(cell[0]), + dm_warn("pci address #%d: %08lx %08lx %08lx\n", i, + (ulong)fdt32_to_cpu(cell[0]), (ulong)fdt32_to_cpu(cell[1]), (ulong)fdt32_to_cpu(cell[2])); if ((fdt32_to_cpu(*cell) & type) == type) { @@ -1346,7 +1347,7 @@ int ofnode_read_pci_addr(ofnode node, enum fdt_pci_space type, ret = -EINVAL; fail: - debug("(not found)\n"); + dm_warn("(not found)\n"); return ret; } @@ -1630,7 +1631,7 @@ int ofnode_write_string(ofnode node, const char *propname, const char *value) { assert(ofnode_valid(node)); - debug("%s: %s = %s", __func__, propname, value); + dm_warn("%s: %s = %s", __func__, propname, value); return ofnode_write_prop(node, propname, value, strlen(value) + 1, false); @@ -1743,7 +1744,7 @@ int ofnode_read_bootscript_address(u64 *bootscr_address, u64 *bootscr_offset) uboot = ofnode_path("/options/u-boot"); if (!ofnode_valid(uboot)) { - debug("%s: Missing /u-boot node\n", __func__); + dm_warn("%s: Missing /u-boot node\n", __func__); return -EINVAL; } @@ -1769,7 +1770,7 @@ int ofnode_read_bootscript_flash(u64 *bootscr_flash_offset, uboot = ofnode_path("/options/u-boot"); if (!ofnode_valid(uboot)) { - debug("%s: Missing /u-boot node\n", __func__); + dm_warn("%s: Missing /u-boot node\n", __func__); return -EINVAL; } @@ -1784,7 +1785,7 @@ int ofnode_read_bootscript_flash(u64 *bootscr_flash_offset, return -EINVAL; if (!bootscr_flash_size) { - debug("bootscr-flash-size is zero. Ignoring properties!\n"); + dm_warn("bootscr-flash-size is zero. Ignoring properties!\n"); *bootscr_flash_offset = 0; return -EINVAL; } @@ -1831,7 +1832,7 @@ phy_interface_t ofnode_read_phy_mode(ofnode node) if (!strcmp(mode, phy_interface_strings[i])) return i; - debug("%s: Invalid PHY interface '%s'\n", __func__, mode); + dm_warn("%s: Invalid PHY interface '%s'\n", __func__, mode); return PHY_INTERFACE_MODE_NA; } diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c index 7ff7834bdf0..304d5b02bcd 100644 --- a/drivers/core/regmap.c +++ b/drivers/core/regmap.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -139,8 +140,8 @@ static int init_range(ofnode node, struct regmap_range *range, int addr_len, ret = of_address_to_resource(ofnode_to_np(node), index, &r); if (ret) { - debug("%s: Could not read resource of range %d (ret = %d)\n", - ofnode_get_name(node), index, ret); + dm_warn("%s: Could not read resource of range %d (ret = %d)\n", + ofnode_get_name(node), index, ret); return ret; } @@ -154,8 +155,8 @@ static int init_range(ofnode node, struct regmap_range *range, int addr_len, addr_len, size_len, &sz, true); if (range->start == FDT_ADDR_T_NONE) { - debug("%s: Could not read start of range %d\n", - ofnode_get_name(node), index); + dm_warn("%s: Could not read start of range %d\n", + ofnode_get_name(node), index); return -EINVAL; } @@ -173,15 +174,15 @@ int regmap_init_mem_index(ofnode node, struct regmap **mapp, int index) addr_len = ofnode_read_simple_addr_cells(ofnode_get_parent(node)); if (addr_len < 0) { - debug("%s: Error while reading the addr length (ret = %d)\n", - ofnode_get_name(node), addr_len); + dm_warn("%s: Error while reading the addr length (ret = %d)\n", + ofnode_get_name(node), addr_len); return addr_len; } size_len = ofnode_read_simple_size_cells(ofnode_get_parent(node)); if (size_len < 0) { - debug("%s: Error while reading the size length: (ret = %d)\n", - ofnode_get_name(node), size_len); + dm_warn("%s: Error while reading the size length: (ret = %d)\n", + ofnode_get_name(node), size_len); return size_len; } @@ -250,36 +251,36 @@ int regmap_init_mem(ofnode node, struct regmap **mapp) addr_len = ofnode_read_simple_addr_cells(ofnode_get_parent(node)); if (addr_len < 0) { - debug("%s: Error while reading the addr length (ret = %d)\n", - ofnode_get_name(node), addr_len); + dm_warn("%s: Error while reading the addr length (ret = %d)\n", + ofnode_get_name(node), addr_len); return addr_len; } size_len = ofnode_read_simple_size_cells(ofnode_get_parent(node)); if (size_len < 0) { - debug("%s: Error while reading the size length: (ret = %d)\n", - ofnode_get_name(node), size_len); + dm_warn("%s: Error while reading the size length: (ret = %d)\n", + ofnode_get_name(node), size_len); return size_len; } both_len = addr_len + size_len; if (!both_len) { - debug("%s: Both addr and size length are zero\n", - ofnode_get_name(node)); + dm_warn("%s: Both addr and size length are zero\n", + ofnode_get_name(node)); return -EINVAL; } len = ofnode_read_size(node, "reg"); if (len < 0) { - debug("%s: Error while reading reg size (ret = %d)\n", - ofnode_get_name(node), len); + dm_warn("%s: Error while reading reg size (ret = %d)\n", + ofnode_get_name(node), len); return len; } len /= sizeof(fdt32_t); count = len / both_len; if (!count) { - debug("%s: Not enough data in reg property\n", - ofnode_get_name(node)); + dm_warn("%s: Not enough data in reg property\n", + ofnode_get_name(node)); return -EINVAL; } @@ -424,8 +425,8 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset, void *ptr; if (do_range_check() && range_num >= map->range_count) { - debug("%s: range index %d larger than range count\n", - __func__, range_num); + dm_warn("%s: range index %d larger than range count\n", + __func__, range_num); return -ERANGE; } range = &map->ranges[range_num]; @@ -433,7 +434,7 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset, offset <<= map->reg_offset_shift; if (do_range_check() && (offset + val_len > range->size || offset + val_len < offset)) { - debug("%s: offset/size combination invalid\n", __func__); + dm_warn("%s: offset/size combination invalid\n", __func__); return -ERANGE; } @@ -455,7 +456,7 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset, break; #endif default: - debug("%s: regmap size %zu unknown\n", __func__, val_len); + dm_warn("%s: regmap size %zu unknown\n", __func__, val_len); return -EINVAL; } @@ -564,15 +565,15 @@ int regmap_raw_write_range(struct regmap *map, uint range_num, uint offset, void *ptr; if (range_num >= map->range_count) { - debug("%s: range index %d larger than range count\n", - __func__, range_num); + dm_warn("%s: range index %d larger than range count\n", + __func__, range_num); return -ERANGE; } range = &map->ranges[range_num]; offset <<= map->reg_offset_shift; if (offset + val_len > range->size || offset + val_len < offset) { - debug("%s: offset/size combination invalid\n", __func__); + dm_warn("%s: offset/size combination invalid\n", __func__); return -ERANGE; } @@ -594,7 +595,7 @@ int regmap_raw_write_range(struct regmap *map, uint range_num, uint offset, break; #endif default: - debug("%s: regmap size %zu unknown\n", __func__, val_len); + dm_warn("%s: regmap size %zu unknown\n", __func__, val_len); return -EINVAL; } @@ -630,8 +631,8 @@ int regmap_write(struct regmap *map, uint offset, uint val) u.v64 = val; break; default: - debug("%s: regmap size %zu unknown\n", __func__, - (size_t)map->width); + dm_warn("%s: regmap size %zu unknown\n", __func__, + (size_t)map->width); return -EINVAL; } diff --git a/drivers/core/root.c b/drivers/core/root.c index 4bfd08f4813..7cf6607a9b7 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -207,7 +207,7 @@ static int dm_scan_fdt_node(struct udevice *parent, ofnode parent_node, err = lists_bind_fdt(parent, node, NULL, NULL, pre_reloc_only); if (err && !ret) { ret = err; - debug("%s: ret=%d\n", node_name, ret); + dm_warn("%s: ret=%d\n", node_name, ret); } } @@ -248,7 +248,7 @@ int dm_extended_scan(bool pre_reloc_only) ret = dm_scan_fdt(pre_reloc_only); if (ret) { - debug("dm_scan_fdt() failed: %d\n", ret); + dm_warn("dm_scan_fdt() failed: %d\n", ret); return ret; } @@ -256,8 +256,8 @@ int dm_extended_scan(bool pre_reloc_only) for (i = 0; i < ARRAY_SIZE(nodes); i++) { ret = dm_scan_fdt_ofnode_path(nodes[i], pre_reloc_only); if (ret) { - debug("dm_scan_fdt() scan for %s failed: %d\n", - nodes[i], ret); + dm_warn("dm_scan_fdt() scan for %s failed: %d\n", + nodes[i], ret); return ret; } } @@ -320,14 +320,14 @@ static int dm_scan(bool pre_reloc_only) ret = dm_scan_plat(pre_reloc_only); if (ret) { - debug("dm_scan_plat() failed: %d\n", ret); + dm_warn("dm_scan_plat() failed: %d\n", ret); return ret; } if (CONFIG_IS_ENABLED(OF_REAL)) { ret = dm_extended_scan(pre_reloc_only); if (ret) { - debug("dm_extended_scan() failed: %d\n", ret); + dm_warn("dm_extended_scan() failed: %d\n", ret); return ret; } } @@ -345,7 +345,7 @@ int dm_init_and_scan(bool pre_reloc_only) ret = dm_init(CONFIG_IS_ENABLED(OF_LIVE)); if (ret) { - debug("dm_init() failed: %d\n", ret); + dm_warn("dm_init() failed: %d\n", ret); return ret; } if (!CONFIG_IS_ENABLED(OF_PLATDATA_INST)) { diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index 762536eebc6..7ae0884a75e 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -59,8 +59,8 @@ static int uclass_add(enum uclass_id id, struct uclass **ucp) *ucp = NULL; uc_drv = lists_uclass_lookup(id); if (!uc_drv) { - debug("Cannot find uclass for id %d: please add the UCLASS_DRIVER() declaration for this UCLASS_... id\n", - id); + dm_warn("Cannot find uclass for id %d: please add the UCLASS_DRIVER() declaration for this UCLASS_... id\n", + id); /* * Use a strange error to make this case easier to find. When * a uclass is not available it can prevent driver model from -- cgit v1.3.1 From c449f4f85420478dc6cf0ff1ea3664b38f5e7d81 Mon Sep 17 00:00:00 2001 From: Quentin Schulz Date: Tue, 11 Jun 2024 15:04:27 +0200 Subject: dm: core: fix typo in SPL_DM_WARN prompt text It should read "in SPL" and not "wuth SPL". Signed-off-by: Quentin Schulz Reviewed-by: Simon Glass --- drivers/core/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig index 754649f091e..1a7be4d9b4d 100644 --- a/drivers/core/Kconfig +++ b/drivers/core/Kconfig @@ -56,7 +56,7 @@ config DM_WARN out - it will do nothing when called. config SPL_DM_WARN - bool "Enable warnings in driver model wuth SPL" + bool "Enable warnings in driver model in SPL" depends on SPL_DM help Enable this to see warnings related to driver model in SPL -- cgit v1.3.1 From f0a5d2dfaae6b895e7eb02c67271cc3eba5ee6c8 Mon Sep 17 00:00:00 2001 From: Ilias Apalodimas Date: Mon, 17 Jun 2024 18:14:18 +0300 Subject: sandbox: cleanup linker scripts and sections commit 6e2228fb052b ("Merge patch series "Clean up arm linker scripts"") was cleaning up linker scripts for armv7 and v8 in a similar fashion. Several commits in the past -- e.g commit d0b5d9da5de2 ("arm: make _end compiler-generated") was moving symbols to be compiler generated. They were defined as c variables in its own section to force the compiler emit relative a reference. However, defining those in the linker script will do the same thing since [0]. So let's remove the special sections from the linker scripts, the variable definitions from sections.c, and define them as a symbols. It's worth noting that the linker was discarding the symbols in the older binary completely since the symbol definition had an extra _. - new binary $~ aarch64-linux-gnu-readelf -sW u-boot | grep efi_runtim 246: 000000000004acbe 13 FUNC LOCAL DEFAULT 14 vbe_req_efi_runtime_rand 3198: 0000000000318690 16 OBJECT LOCAL DEFAULT 29 efi_runtime_mmio 6359: 00000000000dedff 217 FUNC LOCAL DEFAULT 14 efi_runtime_relocate 7942: 00000000003074c0 136 OBJECT GLOBAL HIDDEN 29 efi_runtime_services 8869: 0000000000305e20 0 NOTYPE GLOBAL DEFAULT 27 __efi_runtime_rel_stop 9159: 0000000000305e20 0 NOTYPE GLOBAL DEFAULT 27 __efi_runtime_stop 9410: 0000000000305e20 0 NOTYPE GLOBAL DEFAULT 27 __efi_runtime_start 10137: 00000000005981bd 0 NOTYPE WEAK HIDDEN 33 efi_runtime.c.de5bed54 10470: 0000000000305e20 0 NOTYPE GLOBAL DEFAULT 27 __efi_runtime_rel_start - old binary $~ aarch64-linux-gnu-readelf -sW u-boot.old | grep efi_runtim 246: 000000000004acbe 13 FUNC LOCAL DEFAULT 14 vbe_req_efi_runtime_rand 3198: 0000000000318690 16 OBJECT LOCAL DEFAULT 29 efi_runtime_mmio 6359: 00000000000dedff 221 FUNC LOCAL DEFAULT 14 efi_runtime_relocate 7942: 00000000003074c0 136 OBJECT GLOBAL HIDDEN 29 efi_runtime_services 10135: 0000000000598320 0 NOTYPE WEAK HIDDEN 33 efi_runtime.c.de5bed54 $~ bloat-o-meter u-bool.old u-boot add/remove: 0/0 grow/shrink: 1/1 up/down: 7/-4 (3) Function old new delta efi_memory_init 343 350 +7 efi_runtime_relocate 221 217 -4 Total: Before=2009902, After=2009905, chg +0.00% [0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for shared object") Tested-by: Heinrich Schuchardt # sandbox_defconfig on amd64, arm64, riscv64 Reviewed-by: Simon Glass Fixes: commit aac53d3d96a2 ("sandbox: Rename EFI runtime sections") Signed-off-by: Ilias Apalodimas --- arch/sandbox/cpu/u-boot.lds | 20 ++++---------------- arch/sandbox/lib/Makefile | 2 +- arch/sandbox/lib/sections.c | 13 ------------- 3 files changed, 5 insertions(+), 30 deletions(-) delete mode 100644 arch/sandbox/lib/sections.c diff --git a/arch/sandbox/cpu/u-boot.lds b/arch/sandbox/cpu/u-boot.lds index 52f13af3742..6ee8095b6cb 100644 --- a/arch/sandbox/cpu/u-boot.lds +++ b/arch/sandbox/cpu/u-boot.lds @@ -19,30 +19,18 @@ SECTIONS *(_u_boot_sandbox_getopt_end) } - efi_runtime_start : { - *(___efi_runtime_start) - } - efi_runtime : { + __efi_runtime_start = .; *(efi_runtime_text) *(efi_runtime_data) - } - - efi_runtime_stop : { - *(___efi_runtime_stop) - } - - efi_runtime_rel_start : { - *(___efi_runtime_rel_start) + __efi_runtime_stop = .; } efi_runtime_rel : { + __efi_runtime_rel_start = .; *(.relefi_runtime_text) *(.relefi_runtime_data) - } - - efi_runtime_rel_stop : { - *(___efi_runtime_rel_stop) + __efi_runtime_rel_stop = .; } .dynsym : diff --git a/arch/sandbox/lib/Makefile b/arch/sandbox/lib/Makefile index a2bc5a7ee60..d7d15a50bb6 100644 --- a/arch/sandbox/lib/Makefile +++ b/arch/sandbox/lib/Makefile @@ -5,7 +5,7 @@ # (C) Copyright 2002-2006 # Wolfgang Denk, DENX Software Engineering, wd@denx.de. -obj-y += fdt_fixup.o interrupts.o sections.o +obj-y += fdt_fixup.o interrupts.o obj-$(CONFIG_PCI) += pci_io.o obj-$(CONFIG_CMD_BOOTM) += bootm.o obj-$(CONFIG_CMD_BOOTZ) += bootm.o diff --git a/arch/sandbox/lib/sections.c b/arch/sandbox/lib/sections.c deleted file mode 100644 index 2f2f3fbfdb8..00000000000 --- a/arch/sandbox/lib/sections.c +++ /dev/null @@ -1,13 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * Copyright 2013 Albert ARIBAUD - * - */ -#include - -char __efi_runtime_start[0] __section("___efi_runtime_start"); -char __efi_runtime_stop[0] __section("___efi_runtime_stop"); -char __efi_runtime_rel_start[0] - __section("___efi_runtime_rel_start"); -char __efi_runtime_rel_stop[0] - __section("___efi_runtime_rel_stop"); -- cgit v1.3.1 From 4f02196558a40a6538eb09debffde0f1a23b97be Mon Sep 17 00:00:00 2001 From: Ilias Apalodimas Date: Tue, 18 Jun 2024 20:28:20 +0300 Subject: configs: enable setvariable at runtime on sandbox We currently don't have any boards enabling CONFIG_EFI_RT_VOLATILE_STORE. We do have EFI selftests testing the feature though, so enable it in all the sandbox platforms and test the functionality properly Signed-off-by: Ilias Apalodimas --- configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + 2 files changed, 2 insertions(+) diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index 2bd4eeaade9..dd0582d2a0c 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -267,6 +267,7 @@ CONFIG_CMD_DHRYSTONE=y CONFIG_TPM=y CONFIG_ERRNO_STR=y CONFIG_GETOPT=y +CONFIG_EFI_RT_VOLATILE_STORE=y CONFIG_EFI_SECURE_BOOT=y CONFIG_TEST_FDTDEC=y CONFIG_UNIT_TEST=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 93b52f2de5c..da8c1976d7b 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -348,6 +348,7 @@ CONFIG_ECDSA_VERIFY=y CONFIG_TPM=y CONFIG_ERRNO_STR=y CONFIG_GETOPT=y +CONFIG_EFI_RT_VOLATILE_STORE=y CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y CONFIG_EFI_CAPSULE_ON_DISK=y CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y -- cgit v1.3.1 From e344f836fe039701844ae1693f0e196d0915d5c6 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:55:02 -0600 Subject: binman: efi: Correct entry docs Somehow the class documentation has got out of sync with the generated entries.rst file. Regenerating it causes errors, so correct these and regenerate the entries.rst file. Signed-off-by: Simon Glass Fixes: 809f28e7213 ("binman: capsule: Use dumped capsule header...") --- tools/binman/entries.rst | 58 ++++++++++++++++----------------- tools/binman/etype/efi_capsule.py | 40 +++++++++++------------ tools/binman/etype/efi_empty_capsule.py | 22 +++++++------ 3 files changed, 61 insertions(+), 59 deletions(-) diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index 254afe76074..dc42a9cb97d 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -470,11 +470,11 @@ updating the EC on startup via software sync. .. _etype_efi_capsule: -Entry: capsule: Entry for generating EFI Capsule files ------------------------------------------------------- +Entry: efi-capsule: Generate EFI capsules +----------------------------------------- -The parameters needed for generation of the capsules can be provided -as properties in the entry. +The parameters needed for generation of the capsules can +be provided as properties in the entry. Properties / Entry arguments: - image-index: Unique number for identifying corresponding @@ -495,9 +495,9 @@ Properties / Entry arguments: file. Mandatory property for generating signed capsules. - oem-flags - OEM flags to be passed through capsule header. - Since this is a subclass of Entry_section, all properties of the parent - class also apply here. Except for the properties stated as mandatory, the - rest of the properties are optional. +Since this is a subclass of Entry_section, all properties of the parent +class also apply here. Except for the properties stated as mandatory, the +rest of the properties are optional. For more details on the description of the capsule format, and the capsule update functionality, refer Section 8.5 and Chapter 23 in the `UEFI @@ -510,17 +510,17 @@ provided as a subnode of the capsule entry. A typical capsule entry node would then look something like this:: capsule { - type = "efi-capsule"; - image-index = <0x1>; - /* Image GUID for testing capsule update */ - image-guid = SANDBOX_UBOOT_IMAGE_GUID; - hardware-instance = <0x0>; - private-key = "path/to/the/private/key"; - public-key-cert = "path/to/the/public-key-cert"; - oem-flags = <0x8000>; + type = "efi-capsule"; + image-index = <0x1>; + /* Image GUID for testing capsule update */ + image-guid = SANDBOX_UBOOT_IMAGE_GUID; + hardware-instance = <0x0>; + private-key = "path/to/the/private/key"; + public-key-cert = "path/to/the/public-key-cert"; + oem-flags = <0x8000>; - u-boot { - }; + u-boot { + }; }; In the above example, the capsule payload is the U-Boot image. The @@ -534,8 +534,8 @@ payload using the blob-ext subnode. .. _etype_efi_empty_capsule: -Entry: efi-empty-capsule: Entry for generating EFI Empty Capsule files ----------------------------------------------------------------------- +Entry: efi-empty-capsule: Generate EFI empty capsules +----------------------------------------------------- The parameters needed for generation of the empty capsules can be provided as properties in the entry. @@ -551,22 +551,22 @@ update functionality, refer Section 8.5 and Chapter 23 in the `UEFI specification`_. For more information on the empty capsule, refer the sections 2.3.2 and 2.3.3 in the `Dependable Boot specification`_. -A typical accept empty capsule entry node would then look something -like this:: +A typical accept empty capsule entry node would then look something like +this:: empty-capsule { - type = "efi-empty-capsule"; - /* GUID of the image being accepted */ - image-type-id = SANDBOX_UBOOT_IMAGE_GUID; - capsule-type = "accept"; + type = "efi-empty-capsule"; + /* GUID of image being accepted */ + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; + capsule-type = "accept"; }; -A typical revert empty capsule entry node would then look something -like this:: +A typical revert empty capsule entry node would then look something like +this:: empty-capsule { - type = "efi-empty-capsule"; - capsule-type = "revert"; + type = "efi-empty-capsule"; + capsule-type = "revert"; }; The empty capsules do not have any input payload image. diff --git a/tools/binman/etype/efi_capsule.py b/tools/binman/etype/efi_capsule.py index e3203717822..751f654bf31 100644 --- a/tools/binman/etype/efi_capsule.py +++ b/tools/binman/etype/efi_capsule.py @@ -36,23 +36,23 @@ class Entry_efi_capsule(Entry_section): be provided as properties in the entry. Properties / Entry arguments: - - image-index: Unique number for identifying corresponding - payload image. Number between 1 and descriptor count, i.e. - the total number of firmware images that can be updated. Mandatory - property. - - image-guid: Image GUID which will be used for identifying the - updatable image on the board. Mandatory property. - - hardware-instance: Optional number for identifying unique - hardware instance of a device in the system. Default value of 0 - for images where value is not to be used. - - fw-version: Value of image version that can be put on the capsule - through the Firmware Management Protocol(FMP) header. - - monotonic-count: Count used when signing an image. - - private-key: Path to PEM formatted .key private key file. Mandatory - property for generating signed capsules. - - public-key-cert: Path to PEM formatted .crt public key certificate - file. Mandatory property for generating signed capsules. - - oem-flags - OEM flags to be passed through capsule header. + - image-index: Unique number for identifying corresponding + payload image. Number between 1 and descriptor count, i.e. + the total number of firmware images that can be updated. Mandatory + property. + - image-guid: Image GUID which will be used for identifying the + updatable image on the board. Mandatory property. + - hardware-instance: Optional number for identifying unique + hardware instance of a device in the system. Default value of 0 + for images where value is not to be used. + - fw-version: Value of image version that can be put on the capsule + through the Firmware Management Protocol(FMP) header. + - monotonic-count: Count used when signing an image. + - private-key: Path to PEM formatted .key private key file. Mandatory + property for generating signed capsules. + - public-key-cert: Path to PEM formatted .crt public key certificate + file. Mandatory property for generating signed capsules. + - oem-flags - OEM flags to be passed through capsule header. Since this is a subclass of Entry_section, all properties of the parent class also apply here. Except for the properties stated as mandatory, the @@ -66,9 +66,9 @@ class Entry_efi_capsule(Entry_section): properties in the entry. The payload to be used in the capsule is to be provided as a subnode of the capsule entry. - A typical capsule entry node would then look something like this + A typical capsule entry node would then look something like this:: - capsule { + capsule { type = "efi-capsule"; image-index = <0x1>; /* Image GUID for testing capsule update */ @@ -80,7 +80,7 @@ class Entry_efi_capsule(Entry_section): u-boot { }; - }; + }; In the above example, the capsule payload is the U-Boot image. The capsule entry would read the contents of the payload and put them diff --git a/tools/binman/etype/efi_empty_capsule.py b/tools/binman/etype/efi_empty_capsule.py index 064bf9a77f0..1d99fbfb3bb 100644 --- a/tools/binman/etype/efi_empty_capsule.py +++ b/tools/binman/etype/efi_empty_capsule.py @@ -19,31 +19,33 @@ class Entry_efi_empty_capsule(Entry_section): be provided as properties in the entry. Properties / Entry arguments: - - image-guid: Image GUID which will be used for identifying the - updatable image on the board. Mandatory for accept capsule. - - capsule-type - String to indicate type of capsule to generate. Valid - values are 'accept' and 'revert'. + - image-guid: Image GUID which will be used for identifying the + updatable image on the board. Mandatory for accept capsule. + - capsule-type - String to indicate type of capsule to generate. Valid + values are 'accept' and 'revert'. For more details on the description of the capsule format, and the capsule update functionality, refer Section 8.5 and Chapter 23 in the `UEFI specification`_. For more information on the empty capsule, refer the sections 2.3.2 and 2.3.3 in the `Dependable Boot specification`_. - A typical accept empty capsule entry node would then look something like this + A typical accept empty capsule entry node would then look something like + this:: - empty-capsule { + empty-capsule { type = "efi-empty-capsule"; /* GUID of image being accepted */ image-type-id = SANDBOX_UBOOT_IMAGE_GUID; capsule-type = "accept"; - }; + }; - A typical revert empty capsule entry node would then look something like this + A typical revert empty capsule entry node would then look something like + this:: - empty-capsule { + empty-capsule { type = "efi-empty-capsule"; capsule-type = "revert"; - }; + }; The empty capsules do not have any input payload image. -- cgit v1.3.1 From e1b59477059449bfdfb2882ebf0d3c1b9d156b3b Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:55:03 -0600 Subject: binman: Regenerate nxp docs Regenerate the entries.rst file to include this recent addition. Note that more docs are needed here, to actually describe the entry type. Note also that the entry type needs Binman tests added. Signed-off-by: Simon Glass --- tools/binman/entries.rst | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index dc42a9cb97d..1b9b868e33f 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -1521,6 +1521,28 @@ byte. +.. _etype_nxp_imx8mcst: + +Entry: nxp-imx8mcst: NXP i.MX8M CST .cfg file generator and cst invoker +----------------------------------------------------------------------- + +Properties / Entry arguments: + - nxp,loader-address - loader address (SPL text base) + + + +.. _etype_nxp_imx8mimage: + +Entry: nxp-imx8mimage: NXP i.MX8M imx8mimage .cfg file generator and mkimage invoker +------------------------------------------------------------------------------------ + +Properties / Entry arguments: + - nxp,boot-from - device to boot from (e.g. 'sd') + - nxp,loader-address - loader address (SPL text base) + - nxp,rom-version - BootROM version ('2' for i.MX8M Nano and Plus) + + + .. _etype_opensbi: Entry: opensbi: RISC-V OpenSBI fw_dynamic blob -- cgit v1.3.1 From 638aa113e083b2d33740a620f9d9a0002d7303f5 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:55:04 -0600 Subject: binman: ti: Regenerate entry docs Correct formatting errors in the documentation. Regenerate the entries.rst file to include this recent addition. Signed-off-by: Simon Glass --- tools/binman/entries.rst | 35 ++++++++++++++++++++++++++++++++ tools/binman/etype/ti_secure.py | 45 +++++++++++++++++++++-------------------- 2 files changed, 58 insertions(+), 22 deletions(-) diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index 1b9b868e33f..bdda1ef2855 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -1951,6 +1951,12 @@ Properties / Entry arguments: - content: List of phandles to entries to sign - keyfile: Filename of file containing key to sign binary with - sha: Hash function to be used for signing + - auth-in-place: This is an integer field that contains two pieces + of information: + + - Lower Byte - Remains 0x02 as per our use case + ( 0x02: Move the authenticated binary back to the header ) + - Upper Byte - The Host ID of the core owning the firewall Output files: - input. - input file passed to openssl @@ -1959,6 +1965,35 @@ Output files: - cert. - output file generated by openssl (which is used as the entry contents) +Depending on auth-in-place information in the inputs, we read the +firewall nodes that describe the configurations of firewall that TIFS +will be doing after reading the certificate. + +The syntax of the firewall nodes are as such:: + + firewall-257-0 { + id = <257>; /* The ID of the firewall being configured */ + region = <0>; /* Region number to configure */ + + control = /* The control register */ + <(FWCTRL_EN | FWCTRL_LOCK | FWCTRL_BG | FWCTRL_CACHE)>; + + permissions = /* The permission registers */ + <((FWPRIVID_ALL << FWPRIVID_SHIFT) | + FWPERM_SECURE_PRIV_RWCD | + FWPERM_SECURE_USER_RWCD | + FWPERM_NON_SECURE_PRIV_RWCD | + FWPERM_NON_SECURE_USER_RWCD)>; + + /* More defines can be found in k3-security.h */ + + start_address = /* The Start Address of the firewall */ + <0x0 0x0>; + end_address = /* The End Address of the firewall */ + <0xff 0xffffffff>; + }; + + openssl signs the provided data, using the TI templated config file and writes the signature in this entry. This allows verification that the data is genuine. diff --git a/tools/binman/etype/ti_secure.py b/tools/binman/etype/ti_secure.py index 704dcf8a381..420ee263e4f 100644 --- a/tools/binman/etype/ti_secure.py +++ b/tools/binman/etype/ti_secure.py @@ -53,10 +53,11 @@ class Entry_ti_secure(Entry_x509_cert): - keyfile: Filename of file containing key to sign binary with - sha: Hash function to be used for signing - auth-in-place: This is an integer field that contains two pieces - of information - Lower Byte - Remains 0x02 as per our use case - ( 0x02: Move the authenticated binary back to the header ) - Upper Byte - The Host ID of the core owning the firewall + of information: + + - Lower Byte - Remains 0x02 as per our use case + ( 0x02: Move the authenticated binary back to the header ) + - Upper Byte - The Host ID of the core owning the firewall Output files: - input. - input file passed to openssl @@ -69,29 +70,29 @@ class Entry_ti_secure(Entry_x509_cert): firewall nodes that describe the configurations of firewall that TIFS will be doing after reading the certificate. - The syntax of the firewall nodes are as such: + The syntax of the firewall nodes are as such:: - firewall-257-0 { - id = <257>; /* The ID of the firewall being configured */ - region = <0>; /* Region number to configure */ + firewall-257-0 { + id = <257>; /* The ID of the firewall being configured */ + region = <0>; /* Region number to configure */ - control = /* The control register */ - <(FWCTRL_EN | FWCTRL_LOCK | FWCTRL_BG | FWCTRL_CACHE)>; + control = /* The control register */ + <(FWCTRL_EN | FWCTRL_LOCK | FWCTRL_BG | FWCTRL_CACHE)>; - permissions = /* The permission registers */ - <((FWPRIVID_ALL << FWPRIVID_SHIFT) | - FWPERM_SECURE_PRIV_RWCD | - FWPERM_SECURE_USER_RWCD | - FWPERM_NON_SECURE_PRIV_RWCD | - FWPERM_NON_SECURE_USER_RWCD)>; + permissions = /* The permission registers */ + <((FWPRIVID_ALL << FWPRIVID_SHIFT) | + FWPERM_SECURE_PRIV_RWCD | + FWPERM_SECURE_USER_RWCD | + FWPERM_NON_SECURE_PRIV_RWCD | + FWPERM_NON_SECURE_USER_RWCD)>; - /* More defines can be found in k3-security.h */ + /* More defines can be found in k3-security.h */ - start_address = /* The Start Address of the firewall */ - <0x0 0x0>; - end_address = /* The End Address of the firewall */ - <0xff 0xffffffff>; - }; + start_address = /* The Start Address of the firewall */ + <0x0 0x0>; + end_address = /* The End Address of the firewall */ + <0xff 0xffffffff>; + }; openssl signs the provided data, using the TI templated config file and -- cgit v1.3.1 From d552564e1a0a8096aa25812e5ca874967d34f09a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:55:05 -0600 Subject: binman: Update the entrydocs header Reduce the length of the underline for this header, to match the heading itself. Signed-off-by: Simon Glass --- tools/binman/entry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 42e0b7b9145..2ed65800d22 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -812,7 +812,7 @@ class Entry(object): as missing """ print('''Binman Entry Documentation -=========================== +========================== This file describes the entry types supported by binman. These entry types can be placed in an image one by one to build up a final firmware image. It is -- cgit v1.3.1 From 404936e5731ee366a513b0452e2306e799de59cb Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:55:06 -0600 Subject: binman: Support an assumed size for missing binaries Binman has a the useful feature of handling missing external blobs gracefully, including allowing them to be missing, deciding whether the resulting image is functional or not and faking blobs when this is necessary for particular tools (e.g. mkimage). This feature is widely used in CI. One drawback is that if U-Boot grows too large to fit along with the required blobs, then this is not discovered until someone does a 'real' build which includes the blobs. Add a 'assume-size' property to entries to allow Binman to reserve a given size for missing external blobs. Signed-off-by: Simon Glass --- tools/binman/binman.rst | 7 +++++++ tools/binman/entry.py | 1 + tools/binman/etype/blob.py | 7 ++++++- tools/binman/ftest.py | 28 ++++++++++++++++++++++++++++ tools/binman/test/326_assume_size.dts | 16 ++++++++++++++++ tools/binman/test/327_assume_size_ok.dts | 16 ++++++++++++++++ 6 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/326_assume_size.dts create mode 100644 tools/binman/test/327_assume_size_ok.dts diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index 230e055667f..872e9746c8c 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -711,6 +711,13 @@ missing-msg: information about what needs to be fixed. See missing-blob-help for the message for each tag. +assume-size: + Sets the assumed size of a blob entry if it is missing. This allows for a + check that the rest of the image fits into the available space, even when + the contents are not available. If the entry is missing, Binman will use + this assumed size for the entry size, including creating a fake file of that + size if requested. + no-expanded: By default binman substitutes entries with expanded versions if available, so that a `u-boot` entry type turns into `u-boot-expanded`, for example. The diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 2ed65800d22..219d5dcecab 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -315,6 +315,7 @@ class Entry(object): self.overlap = fdt_util.GetBool(self._node, 'overlap') if self.overlap: self.required_props += ['offset', 'size'] + self.assume_size = fdt_util.GetInt(self._node, 'assume-size', 0) # This is only supported by blobs and sections at present self.compress = fdt_util.GetString(self._node, 'compress', 'none') diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py index 064fae50365..041e1122953 100644 --- a/tools/binman/etype/blob.py +++ b/tools/binman/etype/blob.py @@ -48,11 +48,16 @@ class Entry_blob(Entry): self.external and (self.optional or self.section.GetAllowMissing())) # Allow the file to be missing if not self._pathname: + if not fake_size and self.assume_size: + fake_size = self.assume_size self._pathname, faked = self.check_fake_fname(self._filename, fake_size) self.missing = True if not faked: - self.SetContents(b'') + content_size = 0 + if self.assume_size: # Ensure we get test coverage on next line + content_size = self.assume_size + self.SetContents(tools.get_bytes(0, content_size)) return True self.ReadBlobContents() diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 567849bbab0..e4da04030a5 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -7460,5 +7460,33 @@ fdt fdtmap Extract the devicetree blob from the fdtmap with self.assertRaises(ValueError) as e: self._DoReadFile('323_capsule_accept_revert_missing.dts') + def test_assume_size(self): + """Test handling of the assume-size property for external blob""" + with self.assertRaises(ValueError) as e: + self._DoTestFile('326_assume_size.dts', allow_missing=True, + allow_fake_blobs=True) + self.assertIn("contents size 0xa (10) exceeds section size 0x9 (9)", + str(e.exception)) + + def test_assume_size_ok(self): + """Test handling of the assume-size where it fits OK""" + with test_util.capture_sys_output() as (stdout, stderr): + self._DoTestFile('327_assume_size_ok.dts', allow_missing=True, + allow_fake_blobs=True) + err = stderr.getvalue() + self.assertRegex( + err, + "Image '.*' has faked external blobs and is non-functional: .*") + + def test_assume_size_no_fake(self): + """Test handling of the assume-size where it fits OK""" + with test_util.capture_sys_output() as (stdout, stderr): + self._DoTestFile('327_assume_size_ok.dts', allow_missing=True) + err = stderr.getvalue() + self.assertRegex( + err, + "Image '.*' is missing external blobs and is non-functional: .*") + + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/326_assume_size.dts b/tools/binman/test/326_assume_size.dts new file mode 100644 index 00000000000..4c5f8b418d8 --- /dev/null +++ b/tools/binman/test/326_assume_size.dts @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <9>; + blob-ext { + filename = "assume_blob"; + assume-size = <10>; + }; + }; +}; diff --git a/tools/binman/test/327_assume_size_ok.dts b/tools/binman/test/327_assume_size_ok.dts new file mode 100644 index 00000000000..00ed726f872 --- /dev/null +++ b/tools/binman/test/327_assume_size_ok.dts @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <10>; + blob-ext { + filename = "assume_blob"; + assume-size = <10>; + }; + }; +}; -- cgit v1.3.1 From 0f851e234172cf3d874ae16d61a641cb9d793c73 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:55:07 -0600 Subject: binman: Make Intel ME default to position 0x1000 This cannot ever go at offset 0 since the descriptor is there. Use a better offset for the ME, as used by link and coral, for example. This matters when we start using assumed sizes for missing blobs. Signed-off-by: Simon Glass --- tools/binman/etype/intel_descriptor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/binman/etype/intel_descriptor.py b/tools/binman/etype/intel_descriptor.py index 7fe88a9ec1a..3ce967fe81a 100644 --- a/tools/binman/etype/intel_descriptor.py +++ b/tools/binman/etype/intel_descriptor.py @@ -59,7 +59,7 @@ class Entry_intel_descriptor(Entry_blob_ext): if self.missing: # Return zero offsets so that these entries get placed somewhere if self.HasSibling('intel-me'): - info['intel-me'] = [0, None] + info['intel-me'] = [0x1000, None] return info offset = self.data.find(FD_SIGNATURE) if offset == -1: -- cgit v1.3.1 From 35f04c92139dbd0abb054cbb9f539a6c7530d266 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:55:08 -0600 Subject: x86: Set up some assumed sizes for binary blobs Add assumed sizes so that Binman can check that the U-Boot binaries do not grow too large. Signed-off-by: Simon Glass --- arch/x86/dts/u-boot.dtsi | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/arch/x86/dts/u-boot.dtsi b/arch/x86/dts/u-boot.dtsi index e0de3318091..fdd28979e0b 100644 --- a/arch/x86/dts/u-boot.dtsi +++ b/arch/x86/dts/u-boot.dtsi @@ -24,9 +24,11 @@ #ifdef CONFIG_HAVE_INTEL_ME intel-descriptor { filename = CONFIG_FLASH_DESCRIPTOR_FILE; + assume-size = <0x1000>; }; intel-me { filename = CONFIG_INTEL_ME_FILE; + assume-size = <0x1ff000>; }; #endif #ifdef CONFIG_TPL @@ -87,6 +89,7 @@ #ifdef CONFIG_HAVE_MRC intel-mrc { offset = ; + assume-size = <0x2fc94>; }; #endif #ifdef CONFIG_FSP_VERSION1 @@ -98,6 +101,7 @@ #ifdef CONFIG_FSP_VERSION2 intel-descriptor { filename = CONFIG_FLASH_DESCRIPTOR_FILE; + assume-size = <4096>; }; intel-ifwi { filename = CONFIG_IFWI_INPUT_FILE; @@ -139,6 +143,7 @@ intel-vga { filename = CONFIG_VGA_BIOS_FILE; offset = ; + assume-size = <0x10000>; }; #endif #ifdef CONFIG_HAVE_VBT -- cgit v1.3.1 From 49b158a49218701879430ba4ae460bc77e78b7e9 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:55:09 -0600 Subject: buildman: Make mrproper an argument to _reconfigure() Pass this in so the caller can change it independently of the member variable. Signed-off-by: Simon Glass --- tools/buildman/builderthread.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index a8599c0bb2a..5d4426bf0d1 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -240,7 +240,7 @@ class BuilderThread(threading.Thread): return args, cwd, src_dir def _reconfigure(self, commit, brd, cwd, args, env, config_args, config_out, - cmd_list): + cmd_list, mrproper): """Reconfigure the build Args: @@ -251,11 +251,12 @@ class BuilderThread(threading.Thread): env (dict): Environment strings config_args (list of str): defconfig arg for this board cmd_list (list of str): List to add the commands to, for logging + mrproper (bool): True to run mrproper first Returns: CommandResult object """ - if self.mrproper: + if mrproper: result = self.make(commit, brd, 'mrproper', cwd, 'mrproper', *args, env=env) config_out.write(result.combined) @@ -419,7 +420,8 @@ class BuilderThread(threading.Thread): cmd_list = [] if do_config or adjust_cfg: result = self._reconfigure( - commit, brd, cwd, args, env, config_args, config_out, cmd_list) + commit, brd, cwd, args, env, config_args, config_out, cmd_list, + self.mrproper) do_config = False # No need to configure next time if adjust_cfg: cfgutil.adjust_cfg_file(cfg_file, adjust_cfg) -- cgit v1.3.1 From 3f972a465548b76b3ce76d99d7acf03492d3e1f7 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:55:10 -0600 Subject: buildman: Make mrproper an argument to _config_and_build() Pass this in so the caller can change it independently of the member variable. Signed-off-by: Simon Glass --- tools/buildman/builderthread.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 5d4426bf0d1..ff63f9397e6 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -381,7 +381,7 @@ class BuilderThread(threading.Thread): commit = 'current' return commit - def _config_and_build(self, commit_upto, brd, work_dir, do_config, + def _config_and_build(self, commit_upto, brd, work_dir, do_config, mrproper, config_only, adjust_cfg, commit, out_dir, out_rel_dir, result): """Do the build, configuring first if necessary @@ -391,6 +391,7 @@ class BuilderThread(threading.Thread): brd (Board): Board to create arguments for work_dir (str): Directory to which the source will be checked out do_config (bool): True to run a make _defconfig on the source + mrproper (bool): True to run mrproper first config_only (bool): Only configure the source, do not build it adjust_cfg (list of str): See the cfgutil module and run_commit() commit (Commit): Commit only being built @@ -421,7 +422,7 @@ class BuilderThread(threading.Thread): if do_config or adjust_cfg: result = self._reconfigure( commit, brd, cwd, args, env, config_args, config_out, cmd_list, - self.mrproper) + mrproper) do_config = False # No need to configure next time if adjust_cfg: cfgutil.adjust_cfg_file(cfg_file, adjust_cfg) @@ -500,8 +501,9 @@ class BuilderThread(threading.Thread): if self.toolchain: commit = self._checkout(commit_upto, work_dir) result, do_config = self._config_and_build( - commit_upto, brd, work_dir, do_config, config_only, - adjust_cfg, commit, out_dir, out_rel_dir, result) + commit_upto, brd, work_dir, do_config, self.mrproper, + config_only, adjust_cfg, commit, out_dir, out_rel_dir, + result) result.already_done = False result.toolchain = self.toolchain -- cgit v1.3.1 From 3187da4bea64f94055239761fc0f58a68c8e3236 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:55:11 -0600 Subject: buildman: Make mrproper an argument to run_commit() Pass this in so the caller can change it independently of the member variable. Signed-off-by: Simon Glass --- tools/buildman/builderthread.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index ff63f9397e6..0a7ff2e083e 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -448,9 +448,9 @@ class BuilderThread(threading.Thread): result.cmd_list = cmd_list return result, do_config - def run_commit(self, commit_upto, brd, work_dir, do_config, config_only, - force_build, force_build_failures, work_in_output, - adjust_cfg): + def run_commit(self, commit_upto, brd, work_dir, do_config, mrproper, + config_only, force_build, force_build_failures, + work_in_output, adjust_cfg): """Build a particular commit. If the build is already done, and we are not forcing a build, we skip @@ -461,6 +461,7 @@ class BuilderThread(threading.Thread): brd (Board): Board to build work_dir (str): Directory to which the source will be checked out do_config (bool): True to run a make _defconfig on the source + mrproper (bool): True to run mrproper first config_only (bool): Only configure the source, do not build it force_build (bool): Force a build even if one was previously done force_build_failures (bool): Force a bulid if the previous result @@ -501,7 +502,7 @@ class BuilderThread(threading.Thread): if self.toolchain: commit = self._checkout(commit_upto, work_dir) result, do_config = self._config_and_build( - commit_upto, brd, work_dir, do_config, self.mrproper, + commit_upto, brd, work_dir, do_config, mrproper, config_only, adjust_cfg, commit, out_dir, out_rel_dir, result) result.already_done = False @@ -692,7 +693,8 @@ class BuilderThread(threading.Thread): force_build = False for commit_upto in range(0, len(job.commits), job.step): result, request_config = self.run_commit(commit_upto, brd, - work_dir, do_config, self.builder.config_only, + work_dir, do_config, self.mrproper, + self.builder.config_only, force_build or self.builder.force_build, self.builder.force_build_failures, job.work_in_output, job.adjust_cfg) @@ -703,8 +705,8 @@ class BuilderThread(threading.Thread): # with a reconfig. if self.builder.force_config_on_failure: result, request_config = self.run_commit(commit_upto, - brd, work_dir, True, False, True, False, - job.work_in_output, job.adjust_cfg) + brd, work_dir, True, self.mrproper, False, True, + False, job.work_in_output, job.adjust_cfg) did_config = True if not self.builder.force_reconfig: do_config = request_config @@ -748,7 +750,7 @@ class BuilderThread(threading.Thread): else: # Just build the currently checked-out build result, request_config = self.run_commit(None, brd, work_dir, True, - self.builder.config_only, True, + self.mrproper, self.builder.config_only, True, self.builder.force_build_failures, job.work_in_output, job.adjust_cfg) result.commit_upto = 0 -- cgit v1.3.1 From 7e93bd30b1e2a00a46bb60f7122f79801bc0590b Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:55:12 -0600 Subject: buildman: Avoid rebuilding when --mrproper is used When this flag is enabled, 'make mrproper' is always used when reconfiguring, so there is no point in doing it again. Update this. Signed-off-by: Simon Glass --- tools/buildman/builderthread.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 0a7ff2e083e..c0b1067e3f7 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -700,7 +700,7 @@ class BuilderThread(threading.Thread): job.work_in_output, job.adjust_cfg) failed = result.return_code or result.stderr did_config = do_config - if failed and not do_config: + if failed and not do_config and not self.mrproper: # If our incremental build failed, try building again # with a reconfig. if self.builder.force_config_on_failure: -- cgit v1.3.1 From 8941477e02717a7104f8400363979fc3831a4041 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:55:13 -0600 Subject: buildman: Add a flag to force mrproper on failure When a file is removed by a commit (e.g. include/common.h yay!) it can cause incremental build failures since one of the dependency files from a previous build may mention the file. Add an option to run 'make mrproper' automatically when a build fails. This can be used to automatically resolve the problem, without always adding the large overhead of 'make mrproper' to every build. Signed-off-by: Simon Glass --- tools/buildman/builder.py | 18 ++++++++++-------- tools/buildman/builderthread.py | 6 ++++-- tools/buildman/buildman.rst | 3 ++- tools/buildman/cmdline.py | 4 +++- tools/buildman/control.py | 1 + 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index f35175b4598..c4384f53e8d 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -256,14 +256,14 @@ class Builder: def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, gnu_make='make', checkout=True, show_unknown=True, step=1, no_subdirs=False, full_path=False, verbose_build=False, - mrproper=False, per_board_out_dir=False, - config_only=False, squash_config_y=False, - warnings_as_errors=False, work_in_output=False, - test_thread_exceptions=False, adjust_cfg=None, - allow_missing=False, no_lto=False, reproducible_builds=False, - force_build=False, force_build_failures=False, - force_reconfig=False, in_tree=False, - force_config_on_failure=False, make_func=None): + mrproper=False, fallback_mrproper=False, + per_board_out_dir=False, config_only=False, + squash_config_y=False, warnings_as_errors=False, + work_in_output=False, test_thread_exceptions=False, + adjust_cfg=None, allow_missing=False, no_lto=False, + reproducible_builds=False, force_build=False, + force_build_failures=False, force_reconfig=False, + in_tree=False, force_config_on_failure=False, make_func=None): """Create a new Builder object Args: @@ -283,6 +283,7 @@ class Builder: PATH verbose_build: Run build with V=1 and don't use 'make -s' mrproper: Always run 'make mrproper' when configuring + fallback_mrproper: Run 'make mrproper' and retry on build failure per_board_out_dir: Build in a separate persistent directory per board rather than a thread-specific directory config_only: Only configure each build, don't build it @@ -352,6 +353,7 @@ class Builder: self.force_reconfig = force_reconfig self.in_tree = in_tree self.force_config_on_failure = force_config_on_failure + self.fallback_mrproper = fallback_mrproper if not self.squash_config_y: self.config_filenames += EXTRA_CONFIG_FILENAMES diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index c0b1067e3f7..bbe2f6f0d24 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -705,8 +705,10 @@ class BuilderThread(threading.Thread): # with a reconfig. if self.builder.force_config_on_failure: result, request_config = self.run_commit(commit_upto, - brd, work_dir, True, self.mrproper, False, True, - False, job.work_in_output, job.adjust_cfg) + brd, work_dir, True, + self.mrproper or self.builder.fallback_mrproper, + False, True, False, job.work_in_output, + job.adjust_cfg) did_config = True if not self.builder.force_reconfig: do_config = request_config diff --git a/tools/buildman/buildman.rst b/tools/buildman/buildman.rst index aae2477b5c3..bd0482af5f7 100644 --- a/tools/buildman/buildman.rst +++ b/tools/buildman/buildman.rst @@ -995,7 +995,8 @@ By default, buildman doesn't execute 'make mrproper' prior to building the first commit for each board. This reduces the amount of work 'make' does, and hence speeds up the build. To force use of 'make mrproper', use -the -m flag. This flag will slow down any buildman invocation, since it increases the amount -of work done on any build. +of work done on any build. An alternative is to use the --fallback-mrproper +flag, which retries the build with 'make mrproper' only after a build failure. One possible application of buildman is as part of a continual edit, build, edit, build, ... cycle; repeatedly applying buildman to the same change or diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 03211bd5aa5..8dc5a8787b5 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -90,7 +90,9 @@ def add_upto_m(parser): parser.add_argument('--list-tool-chains', action='store_true', default=False, help='List available tool chains (use -v to see probing detail)') parser.add_argument('-m', '--mrproper', action='store_true', - default=False, help="Run 'make mrproper before reconfiguring") + default=False, help="Run 'make mrproper' before reconfiguring") + parser.add_argument('--fallback-mrproper', action='store_true', + default=False, help="Run 'make mrproper' and retry on build failure") parser.add_argument( '-M', '--allow-missing', action='store_true', default=False, help='Tell binman to allow missing blobs and generate fake ones as needed') diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 8f6850c5211..f2dd87814c3 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -656,6 +656,7 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, no_subdirs=args.no_subdirs, full_path=args.full_path, verbose_build=args.verbose_build, mrproper=args.mrproper, + fallback_mrproper=args.fallback_mrproper, per_board_out_dir=args.per_board_out_dir, config_only=args.config_only, squash_config_y=not args.preserve_config_y, -- cgit v1.3.1 From 5d679f801d05fb728678c23d75d0113512e43cca Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:55:15 -0600 Subject: buildman: Add a way to limit the number of buildmans Buildman uses all available CPUs by default, so running more than one or two concurrent processes is not normally useful. However in some CI cases we want to be able to run several jobs at once to save time. For example, in a lab situation we may want to run a test on 20 boards at a time, since only the build step actually takes much CPU. Add an option which allows such a limit. When buildman starts up, it waits until the number of running processes goes below the limit, then claims a spot in the list. The list is maintained with a temporary file. Note that the temp file is user-specific, since it is hard to create a locked temporary file which can be accessed by any user. In most cases, only one user is running jobs on a machine, so this should not matter. Signed-off-by: Simon Glass --- tools/buildman/buildman.rst | 5 ++ tools/buildman/cmdline.py | 2 + tools/buildman/control.py | 140 ++++++++++++++++++++++++++++++++++++++++- tools/buildman/pyproject.toml | 6 +- tools/buildman/test.py | 121 +++++++++++++++++++++++++++++++++++ tools/u_boot_pylib/terminal.py | 7 ++- 6 files changed, 277 insertions(+), 4 deletions(-) diff --git a/tools/buildman/buildman.rst b/tools/buildman/buildman.rst index bd0482af5f7..b8ff3bf1ab2 100644 --- a/tools/buildman/buildman.rst +++ b/tools/buildman/buildman.rst @@ -1286,6 +1286,11 @@ then buildman hangs. Failing to handle any eventuality is a bug in buildman and should be reported. But you can use -T0 to disable threading and hopefully figure out the root cause of the build failure. +For situations where buildman is invoked from multiple running processes, it is +sometimes useful to have buildman wait until the others have finished. Use the +--process-limit option for this: --process-limit 1 will allow only one buildman +to process jobs at a time. + Build summary ------------- diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 8dc5a8787b5..544a391a464 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -129,6 +129,8 @@ def add_after_m(parser): default=False, help="Use an O= (output) directory per board rather than per thread") parser.add_argument('--print-arch', action='store_true', default=False, help="Print the architecture for a board (ARCH=)") + parser.add_argument('--process-limit', type=int, + default=0, help='Limit to number of buildmans running at once') parser.add_argument('-r', '--reproducible-builds', action='store_true', help='Set SOURCE_DATE_EPOCH=0 to suuport a reproducible build') parser.add_argument('-R', '--regen-board-list', type=str, diff --git a/tools/buildman/control.py b/tools/buildman/control.py index f2dd87814c3..464835c5be5 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -7,10 +7,13 @@ This holds the main control logic for buildman, when not running tests. """ +import getpass import multiprocessing import os import shutil import sys +import tempfile +import time from buildman import boards from buildman import bsettings @@ -21,10 +24,23 @@ from patman import gitutil from patman import patchstream from u_boot_pylib import command from u_boot_pylib import terminal -from u_boot_pylib.terminal import tprint +from u_boot_pylib import tools +from u_boot_pylib.terminal import print_clear, tprint TEST_BUILDER = None +# Space-separated list of buildman process IDs currently running jobs +RUNNING_FNAME = f'buildmanq.{getpass.getuser()}' + +# Lock file for access to RUNNING_FILE +LOCK_FNAME = f'{RUNNING_FNAME}.lock' + +# Wait time for access to lock (seconds) +LOCK_WAIT_S = 10 + +# Wait time to start running +RUN_WAIT_S = 300 + def get_plural(count): """Returns a plural 's' if count is not 1""" return 's' if count != 1 else '' @@ -578,6 +594,125 @@ def calc_adjust_cfg(adjust_cfg, reproducible_builds): return adjust_cfg +def read_procs(tmpdir=tempfile.gettempdir()): + """Read the list of running buildman processes + + If the list is corrupted, returns an empty list + + Args: + tmpdir (str): Temporary directory to use (for testing only) + """ + running_fname = os.path.join(tmpdir, RUNNING_FNAME) + procs = [] + if os.path.exists(running_fname): + items = tools.read_file(running_fname, binary=False).split() + try: + procs = [int(x) for x in items] + except ValueError: # Handle invalid format + pass + return procs + + +def check_pid(pid): + """Check for existence of a unix PID + + https://stackoverflow.com/questions/568271/how-to-check-if-there-exists-a-process-with-a-given-pid-in-python + + Args: + pid (int): PID to check + + Returns: + True if it exists, else False + """ + try: + os.kill(pid, 0) + except OSError: + return False + else: + return True + + +def write_procs(procs, tmpdir=tempfile.gettempdir()): + """Write the list of running buildman processes + + Args: + tmpdir (str): Temporary directory to use (for testing only) + """ + running_fname = os.path.join(tmpdir, RUNNING_FNAME) + tools.write_file(running_fname, ' '.join([str(p) for p in procs]), + binary=False) + + # Allow another user to access the file + os.chmod(running_fname, 0o666) + +def wait_for_process_limit(limit, tmpdir=tempfile.gettempdir(), + pid=os.getpid()): + """Wait until the number of buildman processes drops to the limit + + This uses FileLock to protect a 'running' file, which contains a list of + PIDs of running buildman processes. The number of PIDs in the file indicates + the number of running processes. + + When buildman starts up, it calls this function to wait until it is OK to + start the build. + + On exit, no attempt is made to remove the PID from the file, since other + buildman processes will notice that the PID is no-longer valid, and ignore + it. + + Two timeouts are provided: + LOCK_WAIT_S: length of time to wait for the lock; if this occurs, the + lock is busted / removed before trying again + RUN_WAIT_S: length of time to wait to be allowed to run; if this occurs, + the build starts, with the PID being added to the file. + + Args: + limit (int): Maximum number of buildman processes, including this one; + must be > 0 + tmpdir (str): Temporary directory to use (for testing only) + pid (int): Current process ID (for testing only) + """ + from filelock import Timeout, FileLock + + running_fname = os.path.join(tmpdir, RUNNING_FNAME) + lock_fname = os.path.join(tmpdir, LOCK_FNAME) + lock = FileLock(lock_fname) + + # Allow another user to access the file + col = terminal.Color() + tprint('Waiting for other buildman processes...', newline=False, + colour=col.RED) + + claimed = False + deadline = time.time() + RUN_WAIT_S + while True: + try: + with lock.acquire(timeout=LOCK_WAIT_S): + os.chmod(lock_fname, 0o666) + procs = read_procs(tmpdir) + + # Drop PIDs which are not running + procs = list(filter(check_pid, procs)) + + # If we haven't hit the limit, add ourself + if len(procs) < limit: + tprint('done...', newline=False) + claimed = True + if time.time() >= deadline: + tprint('timeout...', newline=False) + claimed = True + if claimed: + write_procs(procs + [pid], tmpdir) + break + + except Timeout: + tprint('failed to get lock: busting...', newline=False) + os.remove(lock_fname) + + time.sleep(1) + tprint('starting build', newline=False) + print_clear() + def do_buildman(args, toolchains=None, make_func=None, brds=None, clean_dir=False, test_thread_exceptions=False): """The main control code for buildman @@ -677,5 +812,8 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, TEST_BUILDER = builder + if args.process_limit: + wait_for_process_limit(args.process_limit) + return run_builder(builder, series.commits if series else None, brds.get_selected_dict(), args) diff --git a/tools/buildman/pyproject.toml b/tools/buildman/pyproject.toml index fe0f6421b53..68bfa45c3f4 100644 --- a/tools/buildman/pyproject.toml +++ b/tools/buildman/pyproject.toml @@ -8,7 +8,11 @@ version = "0.0.6" authors = [ { name="Simon Glass", email="sjg@chromium.org" }, ] -dependencies = ["u_boot_pylib >= 0.0.6", "patch-manager >= 0.0.6"] +dependencies = [ + "filelock >= 3.0.12", + "u_boot_pylib >= 0.0.6", + "patch-manager >= 0.0.6" +] description = "Buildman build tool for U-Boot" readme = "README.rst" requires-python = ">=3.7" diff --git a/tools/buildman/test.py b/tools/buildman/test.py index 79164bd1993..bfad3093030 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -2,12 +2,14 @@ # Copyright (c) 2012 The Chromium OS Authors. # +from filelock import FileLock import os import shutil import sys import tempfile import time import unittest +from unittest.mock import patch from buildman import board from buildman import boards @@ -156,6 +158,11 @@ class TestBuild(unittest.TestCase): if not os.path.isdir(self.base_dir): os.mkdir(self.base_dir) + self.cur_time = 0 + self.valid_pids = [] + self.finish_time = None + self.finish_pid = None + def tearDown(self): shutil.rmtree(self.base_dir) @@ -747,6 +754,120 @@ class TestBuild(unittest.TestCase): self.assertEqual([ ['MARY="mary"', 'Missing expected line: CONFIG_MARY="mary"']], result) + def get_procs(self): + running_fname = os.path.join(self.base_dir, control.RUNNING_FNAME) + items = tools.read_file(running_fname, binary=False).split() + return [int(x) for x in items] + + def get_time(self): + return self.cur_time + + def inc_time(self, amount): + self.cur_time += amount + + # Handle a process exiting + if self.finish_time == self.cur_time: + self.valid_pids = [pid for pid in self.valid_pids + if pid != self.finish_pid] + + def kill(self, pid, signal): + if pid not in self.valid_pids: + raise OSError('Invalid PID') + + def test_process_limit(self): + """Test wait_for_process_limit() function""" + tmpdir = self.base_dir + + with (patch('time.time', side_effect=self.get_time), + patch('time.sleep', side_effect=self.inc_time), + patch('os.kill', side_effect=self.kill)): + # Grab the process. Since there is no other profcess, this should + # immediately succeed + control.wait_for_process_limit(1, tmpdir=tmpdir, pid=1) + lines = terminal.get_print_test_lines() + self.assertEqual(0, self.cur_time) + self.assertEqual('Waiting for other buildman processes...', + lines[0].text) + self.assertEqual(self._col.RED, lines[0].colour) + self.assertEqual(False, lines[0].newline) + self.assertEqual(True, lines[0].bright) + + self.assertEqual('done...', lines[1].text) + self.assertEqual(None, lines[1].colour) + self.assertEqual(False, lines[1].newline) + self.assertEqual(True, lines[1].bright) + + self.assertEqual('starting build', lines[2].text) + self.assertEqual([1], control.read_procs(tmpdir)) + self.assertEqual(None, lines[2].colour) + self.assertEqual(False, lines[2].newline) + self.assertEqual(True, lines[2].bright) + + # Try again, with a different PID...this should eventually timeout + # and start the build anyway + self.cur_time = 0 + self.valid_pids = [1] + control.wait_for_process_limit(1, tmpdir=tmpdir, pid=2) + lines = terminal.get_print_test_lines() + self.assertEqual('Waiting for other buildman processes...', + lines[0].text) + self.assertEqual('timeout...', lines[1].text) + self.assertEqual(None, lines[1].colour) + self.assertEqual(False, lines[1].newline) + self.assertEqual(True, lines[1].bright) + self.assertEqual('starting build', lines[2].text) + self.assertEqual([1, 2], control.read_procs(tmpdir)) + self.assertEqual(control.RUN_WAIT_S, self.cur_time) + + # Check lock-busting + self.cur_time = 0 + self.valid_pids = [1, 2] + lock_fname = os.path.join(tmpdir, control.LOCK_FNAME) + lock = FileLock(lock_fname) + lock.acquire(timeout=1) + control.wait_for_process_limit(1, tmpdir=tmpdir, pid=3) + lines = terminal.get_print_test_lines() + self.assertEqual('Waiting for other buildman processes...', + lines[0].text) + self.assertEqual('failed to get lock: busting...', lines[1].text) + self.assertEqual(None, lines[1].colour) + self.assertEqual(False, lines[1].newline) + self.assertEqual(True, lines[1].bright) + self.assertEqual('timeout...', lines[2].text) + self.assertEqual('starting build', lines[3].text) + self.assertEqual([1, 2, 3], control.read_procs(tmpdir)) + self.assertEqual(control.RUN_WAIT_S, self.cur_time) + lock.release() + + # Check handling of dead processes. Here we have PID 2 as a running + # process, even though the PID file contains 1, 2 and 3. So we can + # add one more PID, to make 2 and 4 + self.cur_time = 0 + self.valid_pids = [2] + control.wait_for_process_limit(2, tmpdir=tmpdir, pid=4) + lines = terminal.get_print_test_lines() + self.assertEqual('Waiting for other buildman processes...', + lines[0].text) + self.assertEqual('done...', lines[1].text) + self.assertEqual('starting build', lines[2].text) + self.assertEqual([2, 4], control.read_procs(tmpdir)) + self.assertEqual(0, self.cur_time) + + # Try again, with PID 2 quitting at time 50. This allows the new + # build to start + self.cur_time = 0 + self.valid_pids = [2, 4] + self.finish_pid = 2 + self.finish_time = 50 + control.wait_for_process_limit(2, tmpdir=tmpdir, pid=5) + lines = terminal.get_print_test_lines() + self.assertEqual('Waiting for other buildman processes...', + lines[0].text) + self.assertEqual('done...', lines[1].text) + self.assertEqual('starting build', lines[2].text) + self.assertEqual([4, 5], control.read_procs(tmpdir)) + self.assertEqual(self.finish_time, self.cur_time) + if __name__ == "__main__": unittest.main() diff --git a/tools/u_boot_pylib/terminal.py b/tools/u_boot_pylib/terminal.py index 40d79f8ac07..2cd5a54ab52 100644 --- a/tools/u_boot_pylib/terminal.py +++ b/tools/u_boot_pylib/terminal.py @@ -164,8 +164,11 @@ def print_clear(): global last_print_len if last_print_len: - print('\r%s\r' % (' '* last_print_len), end='', flush=True) - last_print_len = None + if print_test_mode: + print_test_list.append(PrintLine(None, None, None, None)) + else: + print('\r%s\r' % (' '* last_print_len), end='', flush=True) + last_print_len = None def set_print_test_mode(enable=True): """Go into test mode, where all printing is recorded""" -- cgit v1.3.1 From e70bac90ff134d77ff25c00c7136d43a8ffd615d Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:56:17 -0600 Subject: buildman: Add python3-coverage Add this package so we can run code-coverage tests for Binman. Signed-off-by: Simon Glass Reviewed-by: Tom Rini --- tools/buildman/requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/buildman/requirements.txt b/tools/buildman/requirements.txt index 4a31e69e4cb..564e54898a4 100644 --- a/tools/buildman/requirements.txt +++ b/tools/buildman/requirements.txt @@ -1,3 +1,4 @@ +coverage==6.2 jsonschema==4.17.3 pyyaml==6.0 yamllint==1.26.3 -- cgit v1.3.1 From fa77b510725dcc6c9e0222c5868b88ce22f77d86 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:56:18 -0600 Subject: buildman: Add python3-pycryptodome This is used by some Binman entry types, so add it to allow more tests to pass. Signed-off-by: Simon Glass Reviewed-by: Tom Rini --- tools/buildman/requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/buildman/requirements.txt b/tools/buildman/requirements.txt index 564e54898a4..052d0ed5c6f 100644 --- a/tools/buildman/requirements.txt +++ b/tools/buildman/requirements.txt @@ -1,4 +1,5 @@ coverage==6.2 jsonschema==4.17.3 +pycryptodome==3.20 pyyaml==6.0 yamllint==1.26.3 -- cgit v1.3.1 From 57abd7c5492539c9585fb02bcf9fa9610a24d54a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:56:19 -0600 Subject: buildman: Fix a few typos in toolchain code Fix 'Thie' and capitalise 'unicode'. Signed-off-by: Simon Glass Suggested-by: Heinrich Schuchardt --- tools/buildman/toolchain.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py index 79c7c11a110..324ad0e0821 100644 --- a/tools/buildman/toolchain.py +++ b/tools/buildman/toolchain.py @@ -175,9 +175,9 @@ class Toolchain: def MakeEnvironment(self, full_path): """Returns an environment for using the toolchain. - Thie takes the current environment and adds CROSS_COMPILE so that + This takes the current environment and adds CROSS_COMPILE so that the tool chain will operate correctly. This also disables localized - output and possibly unicode encoded output of all build tools by + output and possibly Unicode encoded output of all build tools by adding LC_ALL=C. Note that os.environb is used to obtain the environment, since in some -- cgit v1.3.1 From 6c0a3cf75f72370deec3ee516a9dd377397af207 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:56:20 -0600 Subject: buildman: Always use the full path in CROSS_COMPILE The feature to set the toolchain path does not seem to be needed. It causes problems with venv (see [1]). Let's remove it. Add some tests while we are here. It does not look like any docs changes are needed for this. [1] https://patchwork.ozlabs.org/project/uboot/patch/20240621131423.2363294-6-sjg@chromium.org/ Signed-off-by: Simon Glass Suggested-by: Tom Rini Reviewed-by: Tom Rini Reviewed-by: Andrejs Cainikovs --- tools/buildman/bsettings.py | 3 ++ tools/buildman/builder.py | 5 +-- tools/buildman/builderthread.py | 4 +-- tools/buildman/cmdline.py | 2 -- tools/buildman/control.py | 6 ++-- tools/buildman/test.py | 75 +++++++++++++++++++++++++++++++++++++++++ tools/buildman/toolchain.py | 20 +++++------ 7 files changed, 92 insertions(+), 23 deletions(-) diff --git a/tools/buildman/bsettings.py b/tools/buildman/bsettings.py index aea724fc559..a7358cfc08a 100644 --- a/tools/buildman/bsettings.py +++ b/tools/buildman/bsettings.py @@ -31,6 +31,9 @@ def setup(fname=''): def add_file(data): settings.read_file(io.StringIO(data)) +def add_section(name): + settings.add_section(name) + def get_items(section): """Get the items from a section of the config. diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index c4384f53e8d..c794177f2ba 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -255,7 +255,7 @@ class Builder: def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, gnu_make='make', checkout=True, show_unknown=True, step=1, - no_subdirs=False, full_path=False, verbose_build=False, + no_subdirs=False, verbose_build=False, mrproper=False, fallback_mrproper=False, per_board_out_dir=False, config_only=False, squash_config_y=False, warnings_as_errors=False, @@ -279,8 +279,6 @@ class Builder: step: 1 to process every commit, n to process every nth commit no_subdirs: Don't create subdirectories when building current source for a single board - full_path: Return the full path in CROSS_COMPILE and don't set - PATH verbose_build: Run build with V=1 and don't use 'make -s' mrproper: Always run 'make mrproper' when configuring fallback_mrproper: Run 'make mrproper' and retry on build failure @@ -337,7 +335,6 @@ class Builder: self._step = step self._error_lines = 0 self.no_subdirs = no_subdirs - self.full_path = full_path self.verbose_build = verbose_build self.config_only = config_only self.squash_config_y = squash_config_y diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index bbe2f6f0d24..4e085d6d675 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -406,7 +406,7 @@ class BuilderThread(threading.Thread): the next incremental build """ # Set up the environment and command line - env = self.toolchain.MakeEnvironment(self.builder.full_path) + env = self.toolchain.MakeEnvironment() mkdir(out_dir) args, cwd, src_dir = self._build_args(brd, out_dir, out_rel_dir, @@ -574,7 +574,7 @@ class BuilderThread(threading.Thread): outf.write(f'{result.return_code}') # Write out the image and function size information and an objdump - env = result.toolchain.MakeEnvironment(self.builder.full_path) + env = result.toolchain.MakeEnvironment() with open(os.path.join(build_dir, 'out-env'), 'wb') as outf: for var in sorted(env.keys()): outf.write(b'%s="%s"' % (var, env[var])) diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 544a391a464..2673e749d58 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -123,8 +123,6 @@ def add_after_m(parser): help="Override host toochain to use for sandbox (e.g. 'clang-7')") parser.add_argument('-Q', '--quick', action='store_true', default=False, help='Do a rough build, with limited warning resolution') - parser.add_argument('-p', '--full-path', action='store_true', - default=False, help="Use full toolchain path in CROSS_COMPILE") parser.add_argument('-P', '--per-board-out-dir', action='store_true', default=False, help="Use an O= (output) directory per board rather than per thread") parser.add_argument('--print-arch', action='store_true', diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 464835c5be5..037854dfc07 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -788,10 +788,8 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, builder = Builder(toolchains, output_dir, git_dir, args.threads, args.jobs, checkout=True, show_unknown=args.show_unknown, step=args.step, - no_subdirs=args.no_subdirs, full_path=args.full_path, - verbose_build=args.verbose_build, - mrproper=args.mrproper, - fallback_mrproper=args.fallback_mrproper, + no_subdirs=args.no_subdirs, verbose_build=args.verbose_build, + mrproper=args.mrproper, fallback_mrproper=args.fallback_mrproper, per_board_out_dir=args.per_board_out_dir, config_only=args.config_only, squash_config_y=not args.preserve_config_y, diff --git a/tools/buildman/test.py b/tools/buildman/test.py index bfad3093030..ac0a7ab06d6 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -148,6 +148,7 @@ class TestBuild(unittest.TestCase): self.toolchains.Add('arm-linux-gcc', test=False) self.toolchains.Add('sparc-linux-gcc', test=False) self.toolchains.Add('powerpc-linux-gcc', test=False) + self.toolchains.Add('/path/to/aarch64-linux-gcc', test=False) self.toolchains.Add('gcc', test=False) # Avoid sending any output @@ -868,6 +869,80 @@ class TestBuild(unittest.TestCase): self.assertEqual([4, 5], control.read_procs(tmpdir)) self.assertEqual(self.finish_time, self.cur_time) + def call_make_environment(self, tchn, in_env=None): + """Call Toolchain.MakeEnvironment() and process the result + + Args: + tchn (Toolchain): Toolchain to use + in_env (dict): Input environment to use, None to use current env + + Returns: + tuple: + dict: Changes that MakeEnvironment has made to the environment + key: Environment variable that was changed + value: New value (for PATH this only includes components + which were added) + str: Full value of the new PATH variable + """ + env = tchn.MakeEnvironment(env=in_env) + + # Get the original environment + orig_env = dict(os.environb if in_env is None else in_env) + orig_path = orig_env[b'PATH'].split(b':') + + # Find new variables + diff = dict((k, env[k]) for k in env if orig_env.get(k) != env[k]) + + # Find new / different path components + diff_path = None + new_path = None + if b'PATH' in diff: + new_path = diff[b'PATH'].split(b':') + diff_paths = [p for p in new_path if p not in orig_path] + diff_path = b':'.join(p for p in new_path if p not in orig_path) + if diff_path: + diff[b'PATH'] = diff_path + else: + del diff[b'PATH'] + return diff, new_path + + def test_toolchain_env(self): + """Test PATH and other environment settings for toolchains""" + # Use a toolchain which has a path + tchn = self.toolchains.Select('aarch64') + + # Normal case + diff = self.call_make_environment(tchn)[0] + self.assertEqual( + {b'CROSS_COMPILE': b'/path/to/aarch64-linux-', b'LC_ALL': b'C'}, + diff) + + # When overriding the toolchain, only LC_ALL should be set + tchn.override_toolchain = True + diff = self.call_make_environment(tchn)[0] + self.assertEqual({b'LC_ALL': b'C'}, diff) + + # Test that virtualenv is handled correctly + tchn.override_toolchain = False + sys.prefix = '/some/venv' + env = dict(os.environb) + env[b'PATH'] = b'/some/venv/bin:other/things' + tchn.path = '/my/path' + diff, diff_path = self.call_make_environment(tchn, env) + + self.assertNotIn(b'PATH', diff) + self.assertEqual(None, diff_path) + self.assertEqual( + {b'CROSS_COMPILE': b'/my/path/aarch64-linux-', b'LC_ALL': b'C'}, + diff) + + # Handle a toolchain wrapper + tchn.path = '' + bsettings.add_section('toolchain-wrapper') + bsettings.set_item('toolchain-wrapper', 'my-wrapper', 'fred') + diff = self.call_make_environment(tchn)[0] + self.assertEqual( + {b'CROSS_COMPILE': b'fred aarch64-linux-', b'LC_ALL': b'C'}, diff) if __name__ == "__main__": unittest.main() diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py index 324ad0e0821..739acf3ec53 100644 --- a/tools/buildman/toolchain.py +++ b/tools/buildman/toolchain.py @@ -90,7 +90,7 @@ class Toolchain: if self.arch == 'sandbox' and override_toolchain: self.gcc = override_toolchain - env = self.MakeEnvironment(False) + env = self.MakeEnvironment() # As a basic sanity check, run the C compiler with --version cmd = [fname, '--version'] @@ -172,7 +172,7 @@ class Toolchain: else: raise ValueError('Unknown arg to GetEnvArgs (%d)' % which) - def MakeEnvironment(self, full_path): + def MakeEnvironment(self, env=None): """Returns an environment for using the toolchain. This takes the current environment and adds CROSS_COMPILE so that @@ -188,25 +188,23 @@ class Toolchain: 569-570: surrogates not allowed Args: - full_path: Return the full path in CROSS_COMPILE and don't set - PATH + env (dict of bytes): Original environment, used for testing + Returns: Dict containing the (bytes) environment to use. This is based on the - current environment, with changes as needed to CROSS_COMPILE, PATH - and LC_ALL. + current environment, with changes as needed to CROSS_COMPILE and + LC_ALL. """ - env = dict(os.environb) + env = dict(env or os.environb) + wrapper = self.GetWrapper() if self.override_toolchain: # We'll use MakeArgs() to provide this pass - elif full_path: + else: env[b'CROSS_COMPILE'] = tools.to_bytes( wrapper + os.path.join(self.path, self.cross)) - else: - env[b'CROSS_COMPILE'] = tools.to_bytes(wrapper + self.cross) - env[b'PATH'] = tools.to_bytes(self.path) + b':' + env[b'PATH'] env[b'LC_ALL'] = b'C' -- cgit v1.3.1 From 0318126236da1474ae1b6e349d83991fb0cda8ea Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 23 Jun 2024 11:56:21 -0600 Subject: u_boot_pylib: Use correct coverage tool within venv When running within a Python venv we must use the 'coverage' tool (which is within the venv) so that the venv packages are used in preference to system packages. Otherwise the coverage tests run in a different environment from the normal tests and may fail due to missing packages. Handle this by detecting the venv and changing the tool name. Signed-off-by: Simon Glass --- tools/u_boot_pylib/test_util.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tools/u_boot_pylib/test_util.py b/tools/u_boot_pylib/test_util.py index f18d385d995..857ce58c98c 100644 --- a/tools/u_boot_pylib/test_util.py +++ b/tools/u_boot_pylib/test_util.py @@ -60,12 +60,17 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None prefix = '' if build_dir: prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir - cmd = ('%spython3-coverage run ' - '--omit "%s" %s %s %s %s' % (prefix, ','.join(glob_list), + + # Detect a Python virtualenv and use 'coverage' instead + covtool = ('python3-coverage' if sys.prefix == sys.base_prefix else + 'coverage') + + cmd = ('%s%s run ' + '--omit "%s" %s %s %s %s' % (prefix, covtool, ','.join(glob_list), prog, extra_args or '', test_cmd, single_thread or '-P1')) os.system(cmd) - stdout = command.output('python3-coverage', 'report') + stdout = command.output(covtool, 'report') lines = stdout.splitlines() if required: # Convert '/path/to/name.py' just the module name 'name' -- cgit v1.3.1 From c85a05a5d87ce6f077c41d7e63a4a7953ddb351c Mon Sep 17 00:00:00 2001 From: Vincent Stehlé Date: Thu, 27 Jun 2024 19:06:29 +0200 Subject: bootstd: cros: store partition type in an efi_guid_t MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scan_part() function uses a struct uuid to store the little-endian partition type GUID, but this structure should be used only to contain a big-endian UUID. Use an efi_guid_t instead. Signed-off-by: Vincent Stehlé Cc: Simon Glass Cc: Tom Rini Reviewed-by: Simon Glass --- boot/bootmeth_cros.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/boot/bootmeth_cros.c b/boot/bootmeth_cros.c index 645b8bed102..1d5fd8b193d 100644 --- a/boot/bootmeth_cros.c +++ b/boot/bootmeth_cros.c @@ -147,7 +147,7 @@ static int scan_part(struct udevice *blk, int partnum, { struct blk_desc *desc = dev_get_uclass_plat(blk); struct vb2_keyblock *hdr; - struct uuid type; + efi_guid_t type; ulong num_blks; int ret; @@ -160,7 +160,7 @@ static int scan_part(struct udevice *blk, int partnum, /* Check for kernel partition type */ log_debug("part %x: type=%s\n", partnum, info->type_guid); - if (uuid_str_to_bin(info->type_guid, (u8 *)&type, UUID_STR_FORMAT_GUID)) + if (uuid_str_to_bin(info->type_guid, type.b, UUID_STR_FORMAT_GUID)) return log_msg_ret("typ", -EINVAL); if (memcmp(&cros_kern_type, &type, sizeof(type))) -- cgit v1.3.1