Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type
Am 10. Juli 2023 08:26:37 MESZ schrieb AKASHI Takahiro : >On Sun, Jul 09, 2023 at 10:31:55AM +0200, Heinrich Schuchardt wrote: >> On 6/16/23 10:28, Stefan Herbrechtsmeier wrote: >> > From: Malte Schmidt >> > >> > The data type of item_offset_list shall be UINT64 according to the UEFI [1] >> > specifications. >> > >> > In include/efi_api.h the correct data type is used. The bug was probably >> > never noticed because of little endianness. >> > >> > [1] https://uefi.org/specs/UEFI/2.10/index.html >> > >> > Signed-off-by: Malte Schmidt >> > >> > Signed-off-by: Stefan Herbrechtsmeier >> > >> > --- >> > >> > tools/eficapsule.h | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/tools/eficapsule.h b/tools/eficapsule.h >> > index 753fb73313..2099a2e9b8 100644 >> > --- a/tools/eficapsule.h >> > +++ b/tools/eficapsule.h >> > @@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header { >> >uint32_t version; >> >uint16_t embedded_driver_count; >> >uint16_t payload_item_count; >> > - uint32_t item_offset_list[]; >> > + uint64_t item_offset_list[]; >> >> Defining the same structure in two places it bad practice. >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/11 > >I had a good reason for adding a tool-specific header, instead of >using headers under 'include' dir, when I posted v7 of "efi_loader: >capsule: improve capsule authentication support" patch. >The cover letter says, >=== >v7 (Nov 16, 2021) > ... >* define eficapsule.h and include it from mkeficapsule (patch#3) > Hopefully, the tool can now compile on non-linux host. >=== > >If I correctly remember, this reflects the comment below and the >succeeding discussions: >https://lists.denx.de/pipermail/u-boot/2021-November/465859.html > >-Takahiro Akashi We should be able to factor out a common header file which contains capsule specific structures. I understand that we have to test building on BSD. Best regards Heinrich > > >> Reviewed-by: Heinrich Schuchardt >> >> > } __packed; >> > >> > /* image_capsule_support */ >>
Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type
On Sun, Jul 09, 2023 at 10:31:55AM +0200, Heinrich Schuchardt wrote: > On 6/16/23 10:28, Stefan Herbrechtsmeier wrote: > > From: Malte Schmidt > > > > The data type of item_offset_list shall be UINT64 according to the UEFI [1] > > specifications. > > > > In include/efi_api.h the correct data type is used. The bug was probably > > never noticed because of little endianness. > > > > [1] https://uefi.org/specs/UEFI/2.10/index.html > > > > Signed-off-by: Malte Schmidt > > > > Signed-off-by: Stefan Herbrechtsmeier > > > > --- > > > > tools/eficapsule.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/eficapsule.h b/tools/eficapsule.h > > index 753fb73313..2099a2e9b8 100644 > > --- a/tools/eficapsule.h > > +++ b/tools/eficapsule.h > > @@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header { > > uint32_t version; > > uint16_t embedded_driver_count; > > uint16_t payload_item_count; > > - uint32_t item_offset_list[]; > > + uint64_t item_offset_list[]; > > Defining the same structure in two places it bad practice. > https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/11 I had a good reason for adding a tool-specific header, instead of using headers under 'include' dir, when I posted v7 of "efi_loader: capsule: improve capsule authentication support" patch. The cover letter says, === v7 (Nov 16, 2021) ... * define eficapsule.h and include it from mkeficapsule (patch#3) Hopefully, the tool can now compile on non-linux host. === If I correctly remember, this reflects the comment below and the succeeding discussions: https://lists.denx.de/pipermail/u-boot/2021-November/465859.html -Takahiro Akashi > Reviewed-by: Heinrich Schuchardt > > > } __packed; > > > > /* image_capsule_support */ >
Re: [PATCH v3 00/19] binman: Simple templating feature and mkimage conversion
On 10.07.23 04:40, Simon Glass wrote: > This series converts the mkimage entry type to be a section, i.e. based on > the entry_Section class. This makes it more consistent in its behaviour, > e.g. allowing symbol writing and expanded entries. > > A simple templating feature is also introduced, to reduce duplication > when a set of entries must be used in multiple images. > > In this version the nodes from the template are placed before any other > nodes, meaning that the template sets the node order. This seems more > consistent than other mechanisms. > > Changes in v3: > - Add new patch for elf to return number of written symbols > - Add new patch with more detail on how ObtainContents() works > - Fix up some tests which now need SPL and TPL > - Avoid calling ObtainContents() when not needed > - Fix 'specific' typo > - Add a new devicetree file especially for node copying > - Correct logic for merging nodes in order > - Tidy up some comments > - Adjust to use the new example file > - Drop duplicate dts-v1 header > - Add new test case for templating in a FIT > - Add new patch to support writing symbols inside a mkimage image > > Changes in v2: > - Drop now-unnecessary methods in mkimage etype > - Correct ordering of template nodes > - Fix 'preseverd' and 'inserter' typos > > Marek Vasut (1): > binman: Convert mkimage to Entry_section > > Simon Glass (18): > binman: Correct coverage gap in control > binman: Init align_default in entry_Section > binman: Use GetEntries() to obtain section contents > binman: Read _multiple_data_files in the correct place > binman: Allow disabling symbol writing > stm32mp15: Avoid writing symbols in SPL > binman: Update elf to return number of written symbols > binman: Add more detail on how ObtainContents() works > binman: Provide a way to specify the fdt-list directly > binman: Drop __bss_size variable in bss_data.c > binman: Correct handling of zero bss size > dtoc: Support copying the contents of a node into another > dtoc: Allow inserting a list of nodes into another > binman: Support simple templates > binman: Support templating with multiple images > binman: Add a test for templating in a FIT > binman: Support templates at any level > binman: Support writing symbols inside a mkimage image > > arch/arm/dts/stm32mp15-u-boot.dtsi | 1 + > tools/binman/binman.rst| 89 + > tools/binman/control.py| 34 +++- > tools/binman/elf.py| 13 +- > tools/binman/elf_test.py | 13 +- > tools/binman/entries.rst | 6 + > tools/binman/entry.py | 11 +- > tools/binman/etype/blob_phase.py | 5 + > tools/binman/etype/fit.py | 9 + > tools/binman/etype/mkimage.py | 110 --- > tools/binman/etype/section.py | 54 +++-- > tools/binman/etype/u_boot_spl_bss_pad.py | 2 +- > tools/binman/etype/u_boot_tpl_bss_pad.py | 2 +- > tools/binman/etype/u_boot_vpl_bss_pad.py | 2 +- > tools/binman/ftest.py | 218 - > tools/binman/test/282_symbols_disable.dts | 25 +++ > tools/binman/test/283_mkimage_special.dts | 24 +++ > tools/binman/test/284_fit_fdt_list.dts | 58 ++ > tools/binman/test/285_spl_expand.dts | 13 ++ > tools/binman/test/286_template.dts | 42 > tools/binman/test/287_template_multi.dts | 27 +++ > tools/binman/test/288_template_fit.dts | 37 > tools/binman/test/289_template_section.dts | 52 + > tools/binman/test/290_mkimage_sym.dts | 27 +++ > tools/binman/test/Makefile | 5 +- > tools/binman/test/bss_data.c | 3 +- > tools/binman/test/bss_data_zero.c | 16 ++ > tools/binman/test/bss_data_zero.lds| 15 ++ > tools/binman/test/embed_data.lds | 1 + > tools/dtoc/fdt.py | 122 +++- > tools/dtoc/test/dtoc_test_copy.dts | 86 > tools/dtoc/test_fdt.py | 93 + > 32 files changed, 1147 insertions(+), 68 deletions(-) > create mode 100644 tools/binman/test/282_symbols_disable.dts > create mode 100644 tools/binman/test/283_mkimage_special.dts > create mode 100644 tools/binman/test/284_fit_fdt_list.dts > create mode 100644 tools/binman/test/285_spl_expand.dts > create mode 100644 tools/binman/test/286_template.dts > create mode 100644 tools/binman/test/287_template_multi.dts > create mode 100644 tools/binman/test/288_template_fit.dts > create mode 100644 tools/binman/test/289_template_section.dts > create mode 100644 tools/binman/test/290_mkimage_sym.dts > create mode 100644 tools/binman/test/bss_data_zero.c > create mode 100644 tools/binman/test/bss_data_zero.lds > create mode 100644 tools/dtoc/test/dtoc_test_copy.dts > Works much better! What does not work yet: /dts-v1/; / { b
[PATCH v3 19/19] binman: Support writing symbols inside a mkimage image
Add support for writing symbols and determining the assumed position of binaries inside a mkimage image. This is useful as an example for other entry types which might want to do the same thing. Signed-off-by: Simon Glass --- Changes in v3: - Add new patch to support writing symbols inside a mkimage image tools/binman/entry.py | 2 - tools/binman/etype/mkimage.py | 36 +++ tools/binman/ftest.py | 64 +++ tools/binman/test/290_mkimage_sym.dts | 27 +++ 4 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 tools/binman/test/290_mkimage_sym.dts diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 0d4cb94f7002..42e0b7b91450 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1314,8 +1314,6 @@ features to produce new behaviours. """ data = b'' for entry in entries: -# First get the input data and put it in a file -entry.ObtainContents(fake_size=fake_size) data += entry.GetData() uniq = self.GetUniqueName() fname = tools.get_output_filename(f'{prefix}.{uniq}') diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index 493761da59f5..9d60c85722b3 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -268,3 +268,39 @@ class Entry_mkimage(Entry_section): def CheckEntries(self): pass + +def ProcessContents(self): +# The blob may have changed due to WriteSymbols() +ok = super().ProcessContents() +data = self.BuildSectionData(True) +ok2 = self.ProcessContentsUpdate(data) +return ok and ok2 + +def SetImagePos(self, image_pos): +"""Set the position in the image + +This sets each subentry's offsets, sizes and positions-in-image +according to where they ended up in the packed mkimage file. + +NOTE: This assumes a legacy mkimage and assumes that the images are +written to the output in order. SoC-specific mkimage handling may not +conform to this, in which case these values may be wrong. + +Args: +image_pos (int): Position of this entry in the image +""" +# The mkimage header consists of 0x40 bytes, following by a table of +# offsets for each file +upto = 0x40 + +# Skip the 0-terminated list of offsets (assume a single image) +upto += 4 + 4 +for entry in self.GetEntries().values(): +entry.SetOffsetSize(upto, None) + +# Give up if any entries lack a size +if entry.size is None: +return +upto += entry.size + +super().SetImagePos(image_pos) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index e96223cbd892..e53181afb78a 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6820,6 +6820,70 @@ fdt fdtmapExtract the devicetree blob from the fdtmap second = U_BOOT_DATA + b'#' + VGA_DATA + U_BOOT_DTB_DATA self.assertEqual(U_BOOT_IMG_DATA + first + second + first, data) +def testMkimageSymbols(self): +"""Test using mkimage to build an image with symbols in it""" +self._SetupSplElf('u_boot_binman_syms') +data = self._DoReadFile('290_mkimage_sym.dts') + +image = control.images['image'] +entries = image.GetEntries() +self.assertIn('u-boot', entries) +u_boot = entries['u-boot'] + +mkim = entries['mkimage'] +mkim_entries = mkim.GetEntries() +self.assertIn('u-boot-spl', mkim_entries) +spl = mkim_entries['u-boot-spl'] +self.assertIn('u-boot-spl2', mkim_entries) +spl2 = mkim_entries['u-boot-spl2'] + +# skip the mkimage header and the area sizes +mk_data = data[mkim.offset + 0x40:] +size, term = struct.unpack('>LL', mk_data[:8]) + +# There should be only one image, so check that the zero terminator is +# present +self.assertEqual(0, term) + +content = mk_data[8:8 + size] + +# The image should contain the symbols from u_boot_binman_syms.c +# Note that image_pos is adjusted by the base address of the image, +# which is 0x10 in our test image +spl_data = content[:0x18] +content = content[0x1b:] + +# After the header is a table of offsets for each image. There should +# only be one image, then a 0 terminator, so figure out the real start +# of the image data +base = 0x40 + 8 + +# Check symbols in both u-boot-spl and u-boot-spl2 +for i in range(2): +vals = struct.unpack('; + #size-cells = <1>; + + binman { + u-boot-dtb { + }; + + mkimage { + args = "-n test -T script"; + + u-boot-spl { +
[PATCH v3 18/19] binman: Support templates at any level
Allow templates to be used inside a section, not just in the top-level /binman node. Signed-off-by: Simon Glass --- (no changes since v1) tools/binman/control.py| 5 ++- tools/binman/ftest.py | 8 tools/binman/test/289_template_section.dts | 52 ++ 3 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 tools/binman/test/289_template_section.dts diff --git a/tools/binman/control.py b/tools/binman/control.py index f92c152285ce..25e66814837d 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -493,8 +493,8 @@ def _ProcessTemplates(parent): Processing involves copying each subnode of the template node into the target node. -For now this is not done recursively, so templates must be at the top level -of the binman image. +This is done recursively, so templates can be at any level of the binman +image, e.g. inside a section. See 'Templates' in the Binman documnentation for details. """ @@ -502,6 +502,7 @@ def _ProcessTemplates(parent): tmpl = fdt_util.GetPhandleList(node, 'insert-template') if tmpl: node.copy_subnodes_from_phandles(tmpl) +_ProcessTemplates(node) def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded): """Prepare the images to be processed and select the device tree diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index dfca6316624c..e96223cbd892 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6812,6 +6812,14 @@ fdt fdtmapExtract the devicetree blob from the fdtmap tools.write_file(fname, fit_data) out = tools.run('dumpimage', '-l', fname) +def testTemplateSection(self): +"""Test using a template in a section (not at top level)""" +TestFunctional._MakeInputFile('vga2.bin', b'#' + VGA_DATA) +data = self._DoReadFile('289_template_section.dts') +first = U_BOOT_DATA + VGA_DATA + U_BOOT_DTB_DATA +second = U_BOOT_DATA + b'#' + VGA_DATA + U_BOOT_DTB_DATA +self.assertEqual(U_BOOT_IMG_DATA + first + second + first, data) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/289_template_section.dts b/tools/binman/test/289_template_section.dts new file mode 100644 index ..8a744a0cf686 --- /dev/null +++ b/tools/binman/test/289_template_section.dts @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot-img { + }; + + common_part: template { + u-boot { + }; + + intel-vga { + filename = "vga.bin"; + }; + }; + + first { + type = "section"; + insert-template = <&common_part>; + + u-boot-dtb { + }; + }; + + section { + second { + type = "section"; + insert-template = <&common_part>; + + u-boot-dtb { + }; + + intel-vga { + filename = "vga2.bin"; + }; + }; + }; + + second { + type = "section"; + insert-template = <&common_part>; + + u-boot-dtb { + }; + }; + }; +}; -- 2.41.0.390.g38632f3daf-goog
[PATCH v3 17/19] binman: Add a test for templating in a FIT
Add this as a separate test case. Signed-off-by: Simon Glass --- Changes in v3: - Add new test case for templating in a FIT tools/binman/ftest.py | 7 + tools/binman/test/288_template_fit.dts | 37 ++ 2 files changed, 44 insertions(+) create mode 100644 tools/binman/test/288_template_fit.dts diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index dd6075b871d4..dfca6316624c 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6805,6 +6805,13 @@ fdt fdtmapExtract the devicetree blob from the fdtmap data = tools.read_file(image_fname) self.assertEqual(b'blobother', data) +def testTemplateFit(self): +"""Test using a template in a FIT""" +fit_data = self._DoReadFile('288_template_fit.dts') +fname = os.path.join(self._indir, 'fit_data.fit') +tools.write_file(fname, fit_data) +out = tools.run('dumpimage', '-l', fname) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/288_template_fit.dts b/tools/binman/test/288_template_fit.dts new file mode 100644 index ..d84dca4ea41e --- /dev/null +++ b/tools/binman/test/288_template_fit.dts @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + binman: binman { + multiple-images; + + my_template: template { + fit@0 { + images { + kernel-1 { + }; + kernel-2 { + }; + }; + }; + }; + + image { + filename = "image.bin"; + insert-template = <&my_template>; + + fit@0 { + description = "desc"; + configurations { + }; + images { + kernel-3 { + }; + kernel-4 { + }; + }; + }; + }; + }; +}; -- 2.41.0.390.g38632f3daf-goog
[PATCH v3 16/19] binman: Support templating with multiple images
Allow a template to appear in the top level description when using multiple images. Signed-off-by: Simon Glass --- Changes in v3: - Drop duplicate dts-v1 header tools/binman/control.py | 5 +++-- tools/binman/ftest.py| 12 +++ tools/binman/test/287_template_multi.dts | 27 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 tools/binman/test/287_template_multi.dts diff --git a/tools/binman/control.py b/tools/binman/control.py index e9c4a65a75a1..f92c152285ce 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -57,8 +57,9 @@ def _ReadImageDesc(binman_node, use_expanded): images = OrderedDict() if 'multiple-images' in binman_node.props: for node in binman_node.subnodes: -images[node.name] = Image(node.name, node, - use_expanded=use_expanded) +if 'template' not in node.name: +images[node.name] = Image(node.name, node, + use_expanded=use_expanded) else: images['image'] = Image('image', binman_node, use_expanded=use_expanded) return images diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index fc5d8a839e6b..dd6075b871d4 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6793,6 +6793,18 @@ fdt fdtmapExtract the devicetree blob from the fdtmap second = U_BOOT_DATA + b'#' + VGA_DATA + U_BOOT_DTB_DATA self.assertEqual(U_BOOT_IMG_DATA + first + second, data) +def testTemplateBlobMulti(self): +"""Test using a template with 'multiple-images' enabled""" +TestFunctional._MakeInputFile('my-blob.bin', b'blob') +TestFunctional._MakeInputFile('my-blob2.bin', b'other') +retcode = self._DoTestFile('287_template_multi.dts') + +self.assertEqual(0, retcode) +image = control.images['image'] +image_fname = tools.get_output_filename('my-image.bin') +data = tools.read_file(image_fname) +self.assertEqual(b'blobother', data) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/287_template_multi.dts b/tools/binman/test/287_template_multi.dts new file mode 100644 index ..122bfccd5652 --- /dev/null +++ b/tools/binman/test/287_template_multi.dts @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; +/ { + binman: binman { + multiple-images; + + my_template: template { + blob-ext@0 { + filename = "my-blob.bin"; + offset = <0>; + }; + blob-ext@8 { + offset = <8>; + }; + }; + + image { + pad-byte = <0x40>; + filename = "my-image.bin"; + insert-template = <&my_template>; + blob-ext@8 { + filename = "my-blob2.bin"; + }; + }; + }; +}; -- 2.41.0.390.g38632f3daf-goog
[PATCH v3 15/19] binman: Support simple templates
Collections can used to collect the contents of other entries into a single entry, but they result in a single entry, with the original entries 'left behind' in their old place. It is useful to be able to specific a set of entries ones and have it used in multiple images, or parts of an image. Implement this mechanism. Signed-off-by: Simon Glass --- (no changes since v2) Changes in v2: - Correct ordering of template nodes - Fix 'preseverd' and 'inserter' typos tools/binman/binman.rst| 82 ++ tools/binman/control.py| 26 ++ tools/binman/etype/section.py | 3 +- tools/binman/ftest.py | 8 +++ tools/binman/test/286_template.dts | 42 +++ 5 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/286_template.dts diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index a4b31fe5397b..73d38e6b64bb 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -727,6 +727,13 @@ optional: Note that missing, optional blobs do not produce a non-zero exit code from binman, although it does show a warning about the missing external blob. +insert-template: +This is not strictly speaking an entry property, since it is processed early +in Binman before the entries are read. It is a list of phandles of nodes to +include in the current (target) node. For each node, its subnodes and their +properties are brought into the target node. See Templates_ below for +more information. + The attributes supported for images and sections are described below. Several are similar to those for entries. @@ -1172,6 +1179,81 @@ If you are having trouble figuring out what is going on, you can use arch/arm/dts/u-boot.dtsi ... found: "arch/arm/dts/juno-r2-u-boot.dtsi" +Templates += + +Sometimes multiple images need to be created which have all have a common +part. For example, a board may generate SPI and eMMC images which both include +a FIT. Since the FIT includes many entries, it is tedious to repeat them twice +in the image description. + +Templates provide a simple way to handle this:: + +binman { +multiple-images; +common_part: template-1 { +fit { +... lots of entries in here +}; + +text { +text = "base image"; +}; +}; + +spi-image { +filename = "image-spi.bin"; +insert-template = <&fit>; + +/* things specific to SPI follow */ +footer { +]; + +text { +text = "SPI image"; +}; +}; + +mmc-image { +filename = "image-mmc.bin"; +insert-template = <&fit>; + +/* things specific to MMC follow */ +footer { +]; + +text { +text = "MMC image"; +}; +}; +}; + +The template node name must start with 'template', so it is not considered to be +an image itself. + +The mechanism is very simple. For each phandle in the 'insert-templates' +property, the source node is looked up. Then the subnodes of that source node +are copied into the target node, i.e. the one containing the `insert-template` +property. + +If the target node has a node with the same name as a template, its properties +override corresponding properties in the template. This allows the template to +be uses as a base, with the node providing updates to the properties as needed. +The overriding happens recursively. + +Template nodes appear first in each node that they are inserted into and +ordering of template nodes is preserved. Other nodes come afterwards. If a +template node also appears in the target node, then the template node sets the +order. Thus the template can be used to set the ordering, even if the target +node provides all the properties. In the above example, `fit` and `text` appear +first in the `spi-image` and `mmc-image` images, followed by `footer`. + +Where there are multiple template nodes, they are inserted in that order. so +the first template node appears first, then the second. + +Note that template nodes are not removed from the binman description at present. + + Updating an ELF file diff --git a/tools/binman/control.py b/tools/binman/control.py index 7e2dd3541b96..e9c4a65a75a1 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -22,6 +22,7 @@ from binman import bintool from binman import cbfs_util from binman import elf from binman import entry +from dtoc import fdt_util from u_boot_pylib import command from u_boot_pylib import tools from u_boot_pylib import tout @@ -478,6 +479,29 @@ def SignEntries(image_fname, input_fname, privatekey_fname, algo, entry_paths, AfterReplace(image, allow_resize=True, write_map=write_map) +def _ProcessTemplates(parent): +"""Handle an
[PATCH v3 14/19] dtoc: Allow inserting a list of nodes into another
Provide a way to specify a phandle list of nodes which are to be inserted into an existing node. Signed-off-by: Simon Glass --- Changes in v3: - Adjust to use the new example file tools/dtoc/fdt.py | 19 +++ tools/dtoc/test/dtoc_test_copy.dts | 13 - tools/dtoc/test_fdt.py | 25 - 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index dcd78b9e5fbf..4375d3923fbf 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -732,6 +732,25 @@ class Node: for node in reversed(src.subnodes): dst.copy_node(node) +def copy_subnodes_from_phandles(self, phandle_list): +"""Copy subnodes of a list of nodes into another node + +Args: +phandle_list (list of int): List of phandles of nodes to copy + +For each node in the phandle list, its subnodes and their properties are +copied recursively. Note that it does not copy the node itself, nor its +properties. +""" +# Process in reverse order, since new nodes are inserted at the start of +# the destination's node list. We want them to appear in order of the +# phandle list +for phandle in phandle_list.__reversed__(): +parent = self.GetFdt().LookupPhandle(phandle) +tout.debug(f'adding template {parent.path} to node {self.path}') +for node in parent.subnodes.__reversed__(): +self.copy_node(node) + class Fdt: """Provides simple access to a flat device tree blob using libfdts. diff --git a/tools/dtoc/test/dtoc_test_copy.dts b/tools/dtoc/test/dtoc_test_copy.dts index bf2b50e92d71..1939256349fe 100644 --- a/tools/dtoc/test/dtoc_test_copy.dts +++ b/tools/dtoc/test/dtoc_test_copy.dts @@ -10,6 +10,7 @@ / { #address-cells = <1>; #size-cells = <1>; + copy-list = <&another &base>; dest { bootph-all; @@ -45,7 +46,7 @@ }; }; - base { + base: base { compatible = "sandbox,i2c"; bootph-all; #address-cells = <1>; @@ -72,4 +73,14 @@ }; }; }; + + another: another { + earlier { + wibble = <2>; + }; + + later { + fibble = <3>; + }; + }; }; diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index b3593cbee8b2..0ee4956591f9 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -316,7 +316,8 @@ class TestNode(unittest.TestCase): chk = dtb.GetNode('/dest/base') self.assertTrue(chk) self.assertEqual( -{'compatible', 'bootph-all', '#address-cells', '#size-cells'}, +{'compatible', 'bootph-all', '#address-cells', '#size-cells', + 'phandle'}, chk.props.keys()) # Check the first property @@ -376,6 +377,28 @@ class TestNode(unittest.TestCase): dst = new_dtb.GetNode('/dest') do_copy_checks(new_dtb, dst, expect_none=False) +def test_copy_subnodes_from_phandles(self): +"""Test copy_node() function""" +dtb = fdt.FdtScan(find_dtb_file('dtoc_test_copy.dts')) + +orig = dtb.GetNode('/') +node_list = fdt_util.GetPhandleList(orig, 'copy-list') + +dst = dtb.GetNode('/dest') +dst.copy_subnodes_from_phandles(node_list) + +pmic = dtb.GetNode('/dest/over') +self.assertTrue(pmic) + +subn = dtb.GetNode('/dest/first@0') +self.assertTrue(subn) +self.assertEqual({'a-prop', 'b-prop', 'reg'}, subn.props.keys()) + +self.assertEqual( +['/dest/earlier', '/dest/later', '/dest/over', '/dest/first@0', + '/dest/second', '/dest/existing', '/dest/base'], +[n.path for n in dst.subnodes]) + class TestProp(unittest.TestCase): """Test operation of the Prop class""" -- 2.41.0.390.g38632f3daf-goog
[PATCH v3 13/19] dtoc: Support copying the contents of a node into another
This permits implementation of a simple templating system, where a node can be reused as a base for others. For now this adds new subnodes after any existing ones. Signed-off-by: Simon Glass --- Changes in v3: - Add a new devicetree file especially for node copying - Correct logic for merging nodes in order - Tidy up some comments tools/dtoc/fdt.py | 103 - tools/dtoc/test/dtoc_test_copy.dts | 75 + tools/dtoc/test_fdt.py | 70 3 files changed, 245 insertions(+), 3 deletions(-) create mode 100644 tools/dtoc/test/dtoc_test_copy.dts diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index a8e05349a720..dcd78b9e5fbf 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -13,6 +13,7 @@ from dtoc import fdt_util import libfdt from libfdt import QUIET_NOTFOUND from u_boot_pylib import tools +from u_boot_pylib import tout # This deals with a device tree, presenting it as an assortment of Node and # Prop objects, representing nodes and properties, respectively. This file @@ -264,6 +265,13 @@ class Prop: fdt_obj.setprop(node.Offset(), self.name, self.bytes) self.dirty = False +def purge(self): +"""Set a property offset to None + +The property remains in the tree structure and will be recreated when +the FDT is synced +""" +self._offset = None class Node: """A device tree node @@ -534,8 +542,8 @@ class Node: """ return self.AddData(prop_name, struct.pack('>I', val)) -def AddSubnode(self, name): -"""Add a new subnode to the node +def Subnode(self, name): +"""Create new subnode for the node Args: name: name of node to add @@ -544,10 +552,72 @@ class Node: New subnode that was created """ path = self.path + '/' + name -subnode = Node(self._fdt, self, None, name, path) +return Node(self._fdt, self, None, name, path) + +def AddSubnode(self, name): +"""Add a new subnode to the node, after all other subnodes + +Args: +name: name of node to add + +Returns: +New subnode that was created +""" +subnode = self.Subnode(name) self.subnodes.append(subnode) return subnode +def insert_subnode(self, name): +"""Add a new subnode to the node, before all other subnodes + +This deletes other subnodes and sets their offset to None, so that they +will be recreated after this one. + +Args: +name: name of node to add + +Returns: +New subnode that was created +""" +# Deleting a node invalidates the offsets of all following nodes, so +# process in reverse order so that the offset of each node remains valid +# until deletion. +for subnode in reversed(self.subnodes): +subnode.purge(True) +subnode = self.Subnode(name) +self.subnodes.insert(0, subnode) +return subnode + +def purge(self, delete_it=False): +"""Purge this node, setting offset to None and deleting from FDT""" +if self._offset is not None: +if delete_it: +CheckErr(self._fdt._fdt_obj.del_node(self.Offset()), + "Node '%s': delete" % self.path) +self._offset = None +self._fdt.Invalidate() + +for prop in self.props.values(): +prop.purge() + +for subnode in self.subnodes: +subnode.purge(False) + +def move_to_first(self): +"""Move the current node to first in its parent's node list""" +parent = self.parent +if parent.subnodes and parent.subnodes[0] == self: +return +for subnode in reversed(parent.subnodes): +subnode.purge(True) + +new_subnodes = [self] +for subnode in parent.subnodes: +#subnode.purge(False) +if subnode != self: +new_subnodes.append(subnode) +parent.subnodes = new_subnodes + def Delete(self): """Delete a node @@ -635,6 +705,33 @@ class Node: prop.Sync(auto_resize) return added +def copy_node(self, src): +"""Copy a node and all its subnodes into this node + +Args: +src (Node): Node to copy + +This works recursively. + +The new node is put before all other nodes. If the node already +exists, just its subnodes and properties are copied, placing them before +any existing subnodes. Properties which exist in the destination node +already are not copied. +""" +dst = self.FindNode(src.name) +if dst: +dst.move_to_first() +else: +dst = self.insert_subnode(src.name) +for name, src_prop in src.props.items(): +if
[PATCH v3 12/19] binman: Correct handling of zero bss size
Fix the check for the __bss_size symbol, since it may be 0. Unfortunately there was no test coverage for this. Signed-off-by: Simon Glass --- (no changes since v1) tools/binman/elf_test.py | 5 + tools/binman/etype/u_boot_spl_bss_pad.py | 2 +- tools/binman/etype/u_boot_tpl_bss_pad.py | 2 +- tools/binman/etype/u_boot_vpl_bss_pad.py | 2 +- tools/binman/ftest.py| 12 tools/binman/test/285_spl_expand.dts | 13 + tools/binman/test/Makefile | 5 - tools/binman/test/bss_data_zero.c| 16 tools/binman/test/bss_data_zero.lds | 15 +++ tools/binman/test/embed_data.lds | 1 + 10 files changed, 69 insertions(+), 4 deletions(-) create mode 100644 tools/binman/test/285_spl_expand.dts create mode 100644 tools/binman/test/bss_data_zero.c create mode 100644 tools/binman/test/bss_data_zero.lds diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index 2fb3f6f28ff1..cc95b424b338 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -371,6 +371,11 @@ class TestElf(unittest.TestCase): elf.GetSymbolOffset(fname, 'embed') self.assertIn('__image_copy_start', str(e.exception)) +def test_get_symbol_address(self): +fname = self.ElfTestFile('embed_data') +addr = elf.GetSymbolAddress(fname, 'region_size') +self.assertEqual(0, addr) + if __name__ == '__main__': unittest.main() diff --git a/tools/binman/etype/u_boot_spl_bss_pad.py b/tools/binman/etype/u_boot_spl_bss_pad.py index 1ffeb3911fd8..4af4045d3702 100644 --- a/tools/binman/etype/u_boot_spl_bss_pad.py +++ b/tools/binman/etype/u_boot_spl_bss_pad.py @@ -38,7 +38,7 @@ class Entry_u_boot_spl_bss_pad(Entry_blob): def ObtainContents(self): fname = tools.get_input_filename('spl/u-boot-spl') bss_size = elf.GetSymbolAddress(fname, '__bss_size') -if not bss_size: +if bss_size is None: self.Raise('Expected __bss_size symbol in spl/u-boot-spl') self.SetContents(tools.get_bytes(0, bss_size)) return True diff --git a/tools/binman/etype/u_boot_tpl_bss_pad.py b/tools/binman/etype/u_boot_tpl_bss_pad.py index 29c6a9541296..46d2cd58f7e2 100644 --- a/tools/binman/etype/u_boot_tpl_bss_pad.py +++ b/tools/binman/etype/u_boot_tpl_bss_pad.py @@ -38,7 +38,7 @@ class Entry_u_boot_tpl_bss_pad(Entry_blob): def ObtainContents(self): fname = tools.get_input_filename('tpl/u-boot-tpl') bss_size = elf.GetSymbolAddress(fname, '__bss_size') -if not bss_size: +if bss_size is None: self.Raise('Expected __bss_size symbol in tpl/u-boot-tpl') self.SetContents(tools.get_bytes(0, bss_size)) return True diff --git a/tools/binman/etype/u_boot_vpl_bss_pad.py b/tools/binman/etype/u_boot_vpl_bss_pad.py index bba38ccf9e93..12b286a71987 100644 --- a/tools/binman/etype/u_boot_vpl_bss_pad.py +++ b/tools/binman/etype/u_boot_vpl_bss_pad.py @@ -38,7 +38,7 @@ class Entry_u_boot_vpl_bss_pad(Entry_blob): def ObtainContents(self): fname = tools.get_input_filename('vpl/u-boot-vpl') bss_size = elf.GetSymbolAddress(fname, '__bss_size') -if not bss_size: +if bss_size is None: self.Raise('Expected __bss_size symbol in vpl/u-boot-vpl') self.SetContents(tools.get_bytes(0, bss_size)) return True diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index ed1940ed9f71..dc9a95d341e5 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6773,6 +6773,18 @@ fdt fdtmapExtract the devicetree blob from the fdtmap self.assertEqual(U_BOOT_NODTB_DATA, data[-len(U_BOOT_NODTB_DATA):]) fit_data = data[len(U_BOOT_DATA):-len(U_BOOT_NODTB_DATA)] +def testSplEmptyBss(self): +"""Test an expanded SPL with a zero-size BSS""" +# ELF file with a '__bss_size' symbol +self._SetupSplElf(src_fname='bss_data_zero') + +entry_args = { +'spl-bss-pad': 'y', +'spl-dtb': 'y', +} +data = self._DoReadFileDtb('285_spl_expand.dts', + use_expanded=True, entry_args=entry_args)[0] + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/285_spl_expand.dts b/tools/binman/test/285_spl_expand.dts new file mode 100644 index ..9c88ccb287b1 --- /dev/null +++ b/tools/binman/test/285_spl_expand.dts @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot-spl { + }; + }; +}; diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile index cd66a3038be2..4d152eee9c09 100644 --- a/tools/binman/test/Makefile +++ b/tools/binman/test/Makefile @@ -32,7 +32,7 @@ LDS_BINMAN_EMBED := -T $(SRC)u_boot_bin
[PATCH v3 11/19] binman: Drop __bss_size variable in bss_data.c
This is not needed since the linker script sets it up. Drop the variable to avoid confusion. Fix the prototype for main() while we are here. Signed-off-by: Simon Glass --- (no changes since v1) tools/binman/test/bss_data.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/binman/test/bss_data.c b/tools/binman/test/bss_data.c index 4f9b64cef9ef..7047a3bb014d 100644 --- a/tools/binman/test/bss_data.c +++ b/tools/binman/test/bss_data.c @@ -7,9 +7,8 @@ */ int bss_data[10]; -int __bss_size = sizeof(bss_data); -int main() +int main(void) { bss_data[2] = 2; -- 2.41.0.390.g38632f3daf-goog
[PATCH v3 09/19] binman: Convert mkimage to Entry_section
From: Marek Vasut This is needed to handle mkimage with inner section located itself in a section. Signed-off-by: Marek Vasut Use BuildSectionData() instead of ObtainContents(), add tests and a few other minor fixes: Signed-off-by: Simon Glass --- Changes in v3: - Fix up some tests which now need SPL and TPL - Avoid calling ObtainContents() when not needed Changes in v2: - Drop now-unnecessary methods in mkimage etype tools/binman/entry.py | 6 +- tools/binman/etype/mkimage.py | 71 ++- tools/binman/ftest.py | 69 +- tools/binman/test/283_mkimage_special.dts | 24 4 files changed, 135 insertions(+), 35 deletions(-) create mode 100644 tools/binman/test/283_mkimage_special.dts diff --git a/tools/binman/entry.py b/tools/binman/entry.py index f20f32213a9b..0d4cb94f7002 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1314,10 +1314,8 @@ features to produce new behaviours. """ data = b'' for entry in entries: -# First get the input data and put it in a file. If not available, -# try later. -if not entry.ObtainContents(fake_size=fake_size): -return None, None, None +# First get the input data and put it in a file +entry.ObtainContents(fake_size=fake_size) data += entry.GetData() uniq = self.GetUniqueName() fname = tools.get_output_filename(f'{prefix}.{uniq}') diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index cb3e10672ad7..493761da59f5 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -8,10 +8,11 @@ from collections import OrderedDict from binman.entry import Entry +from binman.etype.section import Entry_section from dtoc import fdt_util from u_boot_pylib import tools -class Entry_mkimage(Entry): +class Entry_mkimage(Entry_section): """Binary produced by mkimage Properties / Entry arguments: @@ -121,10 +122,8 @@ class Entry_mkimage(Entry): """ def __init__(self, section, etype, node): super().__init__(section, etype, node) -self._mkimage_entries = OrderedDict() self._imagename = None -self._filename = fdt_util.GetString(self._node, 'filename') -self.align_default = None +self._multiple_data_files = False def ReadNode(self): super().ReadNode() @@ -135,41 +134,55 @@ class Entry_mkimage(Entry): 'data-to-imagename') if self._data_to_imagename and self._node.FindNode('imagename'): self.Raise('Cannot use both imagename node and data-to-imagename') -self.ReadEntries() def ReadEntries(self): """Read the subnodes to find out what should go in this image""" for node in self._node.subnodes: -entry = Entry.Create(self, node) +if self.IsSpecialSubnode(node): +continue +entry = Entry.Create(self, node, + expanded=self.GetImage().use_expanded, + missing_etype=self.GetImage().missing_etype) entry.ReadNode() +entry.SetPrefix(self._name_prefix) if entry.name == 'imagename': self._imagename = entry else: -self._mkimage_entries[entry.name] = entry +self._entries[entry.name] = entry -def ObtainContents(self): +def BuildSectionData(self, required): +"""Build mkimage entry contents + +Runs mkimage to build the entry contents + +Args: +required (bool): True if the data must be present, False if it is OK +to return None + +Returns: +bytes: Contents of the section +""" # Use a non-zero size for any fake files to keep mkimage happy # Note that testMkimageImagename() relies on this 'mkimage' parameter fake_size = 1024 if self._multiple_data_files: fnames = [] uniq = self.GetUniqueName() -for entry in self._mkimage_entries.values(): -if not entry.ObtainContents(fake_size=fake_size): -return False -if entry._pathname: -fnames.append(entry._pathname) +for entry in self._entries.values(): +# Put the contents in a temporary file +ename = f'mkimage-in-{uniq}-{entry.name}' +fname = tools.get_output_filename(ename) +data = entry.GetData(required) +tools.write_file(fname, data) +fnames.append(fname) input_fname = ":".join(fnames) +data = b'' else: data, input_fname, uniq = self.collect_contents_to_file( -self._mkimage
[PATCH v3 10/19] binman: Provide a way to specify the fdt-list directly
Sometimes multiple boards are built with binman and it is useful to specify a different FDT list for each. At present this is not possible without providing multiple values of the of-list entryarg (which is not supported in the U-Boot build system). Allow a fit,fdt-list-val string-list property to be used instead. Signed-off-by: Simon Glass --- Changes in v3: - Fix 'specific' typo tools/binman/entries.rst | 6 +++ tools/binman/etype/fit.py | 9 tools/binman/ftest.py | 14 ++- tools/binman/test/284_fit_fdt_list.dts | 58 ++ 4 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/284_fit_fdt_list.dts diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index b71af801fdad..b55f424620a3 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -615,6 +615,12 @@ The top-level 'fit' node supports the following special properties: `of-list` meaning that `-a of-list="dtb1 dtb2..."` should be passed to binman. +fit,fdt-list-val +As an alternative to fit,fdt-list the list of device tree files +can be provided in this property as a string list, e.g.:: + +fit,fdt-list-val = "dtb1", "dtb2"; + Substitutions ~ diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index c395706ece5f..ef4d0667578d 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -81,6 +81,12 @@ class Entry_fit(Entry_section): `of-list` meaning that `-a of-list="dtb1 dtb2..."` should be passed to binman. +fit,fdt-list-val +As an alternative to fit,fdt-list the list of device tree files +can be provided in this property as a string list, e.g.:: + +fit,fdt-list-val = "dtb1", "dtb2"; + Substitutions ~ @@ -361,6 +367,9 @@ class Entry_fit(Entry_section): [EntryArg(self._fit_list_prop.value, str)]) if fdts is not None: self._fdts = fdts.split() +else: +self._fdts = fdt_util.GetStringList(self._node, 'fit,fdt-list-val') + self._fit_default_dt = self.GetEntryArgsOrProps([EntryArg('default-dt', str)])[0] diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 60e69443c400..ed1940ed9f71 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6761,6 +6761,18 @@ fdt fdtmapExtract the devicetree blob from the fdtmap # Just check that the data appears in the file somewhere self.assertIn(U_BOOT_DATA, data) +def testFitFdtList(self): +"""Test an image with an FIT with the fit,fdt-list-val option""" +entry_args = { +'default-dt': 'test-fdt2', +} +data = self._DoReadFileDtb( +'284_fit_fdt_list.dts', +entry_args=entry_args, +extra_indirs=[os.path.join(self._indir, TEST_FDT_SUBDIR)])[0] +self.assertEqual(U_BOOT_NODTB_DATA, data[-len(U_BOOT_NODTB_DATA):]) +fit_data = data[len(U_BOOT_DATA):-len(U_BOOT_NODTB_DATA)] + -if __name__ == "_s_main__": +if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/284_fit_fdt_list.dts b/tools/binman/test/284_fit_fdt_list.dts new file mode 100644 index ..8885313f5b88 --- /dev/null +++ b/tools/binman/test/284_fit_fdt_list.dts @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot { + }; + fit { + description = "test-desc"; + #address-cells = <1>; + fit,fdt-list-val = "test-fdt1", "test-fdt2"; + + images { + kernel { + description = "Vanilla Linux kernel"; + type = "kernel"; + arch = "ppc"; + os = "linux"; + compression = "gzip"; + load = <>; + entry = <>; + hash-1 { + algo = "crc32"; + }; + hash-2 { + algo = "sha1"; + }; + u-boot { + }; + }; + @fdt-SEQ { + description = "fdt-NAME.dtb"; +
[PATCH v3 07/19] binman: Update elf to return number of written symbols
Update the LookupAndWriteSymbols() function to return the number of symbols written. Also add some logging for when debugging is not enabled. Signed-off-by: Simon Glass --- Changes in v3: - Add new patch for elf to return number of written symbols tools/binman/elf.py | 13 +++-- tools/binman/elf_test.py | 8 +--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/tools/binman/elf.py b/tools/binman/elf.py index 5816284c32a2..4219001feac3 100644 --- a/tools/binman/elf.py +++ b/tools/binman/elf.py @@ -248,6 +248,9 @@ def LookupAndWriteSymbols(elf_fname, entry, section, is_elf=False, entry: Entry to process section: Section which can be used to lookup symbol values base_sym: Base symbol marking the start of the image + +Returns: +int: Number of symbols written """ if not base_sym: base_sym = '__image_copy_start' @@ -269,12 +272,13 @@ def LookupAndWriteSymbols(elf_fname, entry, section, is_elf=False, if not syms: tout.debug('LookupAndWriteSymbols: no syms') -return +return 0 base = syms.get(base_sym) if not base and not is_elf: tout.debug('LookupAndWriteSymbols: no base') -return +return 0 base_addr = 0 if is_elf else base.address +count = 0 for name, sym in syms.items(): if name.startswith('_binman'): msg = ("Section '%s': Symbol '%s'\n in entry '%s'" % @@ -307,6 +311,11 @@ def LookupAndWriteSymbols(elf_fname, entry, section, is_elf=False, (msg, name, offset, value, len(value_bytes))) entry.data = (entry.data[:offset] + value_bytes + entry.data[offset + sym.size:]) +count += 1 +if count: +tout.detail( +f"Section '{section.GetPath()}': entry '{entry.GetPath()}' : {count} symbols") +return count def GetSymbolValue(sym, data, msg): """Get the value of a symbol diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index c98083961b53..2fb3f6f28ff1 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -141,7 +141,8 @@ class TestElf(unittest.TestCase): entry = FakeEntry(10) section = FakeSection() elf_fname = self.ElfTestFile('u_boot_binman_syms_bad') -elf.LookupAndWriteSymbols(elf_fname, entry, section) +count = elf.LookupAndWriteSymbols(elf_fname, entry, section) +self.assertEqual(0, count) def testBadSymbolSize(self): """Test that an attempt to use an 8-bit symbol are detected @@ -162,7 +163,7 @@ class TestElf(unittest.TestCase): def testNoValue(self): """Test the case where we have no value for the symbol -This should produce -1 values for all thress symbols, taking up the +This should produce -1 values for all three symbols, taking up the first 16 bytes of the image. """ if not elf.ELF_TOOLS: @@ -170,7 +171,8 @@ class TestElf(unittest.TestCase): entry = FakeEntry(28) section = FakeSection(sym_value=None) elf_fname = self.ElfTestFile('u_boot_binman_syms') -elf.LookupAndWriteSymbols(elf_fname, entry, section) +count = elf.LookupAndWriteSymbols(elf_fname, entry, section) +self.assertEqual(5, count) expected = (struct.pack('
[PATCH v3 08/19] binman: Add more detail on how ObtainContents() works
This area of binman can be a bit confusing. Add some more comments to help. Signed-off-by: Simon Glass --- Changes in v3: - Add new patch with more detail on how ObtainContents() works tools/binman/entry.py | 3 +++ tools/binman/etype/section.py | 32 +++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 328b5bc568a9..f20f32213a9b 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -474,6 +474,9 @@ class Entry(object): def ObtainContents(self, skip_entry=None, fake_size=0): """Figure out the contents of an entry. +For missing blobs (where allow-missing is enabled), the contents are set +to b'' and self.missing is set to True. + Args: skip_entry (Entry): Entry to skip when obtaining section contents fake_size (int): Size of fake file to create if needed diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index d56cc11d1023..d9b9e428024a 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -316,12 +316,15 @@ class Entry_section(Entry): This should be overridden by subclasses which want to build their own data structure for the section. +Missing entries will have be given empty (or fake) data, so are +processed normally here. + Args: required: True if the data must be present, False if it is OK to return None Returns: -Contents of the section (bytes) +Contents of the section (bytes), None if not available """ section_data = bytearray() @@ -711,6 +714,33 @@ class Entry_section(Entry): def GetEntryContents(self, skip_entry=None): """Call ObtainContents() for each entry in the section +The overall goal of this function is to read in any available data in +this entry and any subentries. This includes reading in blobs, setting +up objects which have predefined contents, etc. + +Since entry types which contain entries call ObtainContents() on all +those entries too, the result is that ObtainContents() is called +recursively for the whole tree below this one. + +Entries with subentries are generally not *themselves& processed here, +i.e. their ObtainContents() implementation simply obtains contents of +their subentries, skipping their own contents. For example, the +implementation here (for entry_Section) does not attempt to pack the +entries into a final result. That is handled later. + +Generally, calling this results in SetContents() being called for each +entry, so that the 'data' and 'contents_size; properties are set, and +subsequent calls to GetData() will return value data. + +Where 'allow_missing' is set, this can result in the 'missing' property +being set to True if there is no data. This is handled by setting the +data to b''. This function will still return success. Future calls to +GetData() for this entry will return b'', or in the case where the data +is faked, GetData() will return that fake data. + +Args: +skip_entry: (single) Entry to skip, or None to process all entries + Note that this may set entry.absent to True if the entry is not actually needed """ -- 2.41.0.390.g38632f3daf-goog
[PATCH v3 06/19] stm32mp15: Avoid writing symbols in SPL
These boards use SPL in a mkimage entry and apparently access the symbol containing the image position of U-Boot, but put U-Boot in another image. This means that binman is unable to fill in the symbol correctly in the SPL binary. This doesn't matter at present since mkimage doesn't support symbol writing. But with the upcoming conversion to a section, it will. So add a property to disable symbol writing. Signed-off-by: Simon Glass --- (no changes since v1) arch/arm/dts/stm32mp15-u-boot.dtsi | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/dts/stm32mp15-u-boot.dtsi b/arch/arm/dts/stm32mp15-u-boot.dtsi index d872c6fc5679..573dd4d3ed56 100644 --- a/arch/arm/dts/stm32mp15-u-boot.dtsi +++ b/arch/arm/dts/stm32mp15-u-boot.dtsi @@ -226,6 +226,7 @@ mkimage { args = "-T stm32image -a 0x2ffc2500 -e 0x2ffc2500"; u-boot-spl { + no-write-symbols; }; }; }; -- 2.41.0.390.g38632f3daf-goog
[PATCH v3 05/19] binman: Allow disabling symbol writing
Some boards don't use symbol writing but do access the symbols in SPL. Provide an option to work around this. Signed-off-by: Simon Glass --- (no changes since v1) tools/binman/binman.rst | 7 ++ tools/binman/entry.py | 4 +++- tools/binman/etype/blob_phase.py | 5 tools/binman/ftest.py | 28 +++ tools/binman/test/282_symbols_disable.dts | 25 5 files changed, 64 insertions(+), 5 deletions(-) create mode 100644 tools/binman/test/282_symbols_disable.dts diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index 23cbb99b4b0b..a4b31fe5397b 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -831,6 +831,13 @@ write-symbols: binman. This is automatic for certain entry types, e.g. `u-boot-spl`. See binman_syms_ for more information. +no-write-symbols: +Disables symbol writing for this entry. This can be used in entry types +where symbol writing is automatic. For example, if `u-boot-spl` refers to +the `u_boot_any_image_pos` symbol but U-Boot is not available in the image +containing SPL, this can be used to disable the writing. Quite likely this +indicates a bug in your setup. + elf-filename: Sets the file name of a blob's associated ELF file. For example, if the blob is `zephyr.bin` then the ELF file may be `zephyr.elf`. This allows diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 39456906a477..328b5bc568a9 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -158,6 +158,7 @@ class Entry(object): self.offset_from_elf = None self.preserve = False self.build_done = False +self.no_write_symbols = False @staticmethod def FindEntryClass(etype, expanded): @@ -321,6 +322,7 @@ class Entry(object): 'offset-from-elf') self.preserve = fdt_util.GetBool(self._node, 'preserve') +self.no_write_symbols = fdt_util.GetBool(self._node, 'no-write-symbols') def GetDefaultFilename(self): return None @@ -695,7 +697,7 @@ class Entry(object): Args: section: Section containing the entry """ -if self.auto_write_symbols: +if self.auto_write_symbols and not self.no_write_symbols: # Check if we are writing symbols into an ELF file is_elf = self.GetDefaultFilename() == self.elf_fname elf.LookupAndWriteSymbols(self.elf_fname, self, section.GetImage(), diff --git a/tools/binman/etype/blob_phase.py b/tools/binman/etype/blob_phase.py index b937158756fd..951d9934050e 100644 --- a/tools/binman/etype/blob_phase.py +++ b/tools/binman/etype/blob_phase.py @@ -52,3 +52,8 @@ class Entry_blob_phase(Entry_section): # Read entries again, now that we have some self.ReadEntries() + +# Propagate the no-write-symbols property +if self.no_write_symbols: +for entry in self._entries.values(): +entry.no_write_symbols = True diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 43b4f850a691..dabb3f689fdb 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1452,7 +1452,7 @@ class TestFunctional(unittest.TestCase): self.assertEqual(U_BOOT_SPL_NODTB_DATA, data[:len(U_BOOT_SPL_NODTB_DATA)]) def checkSymbols(self, dts, base_data, u_boot_offset, entry_args=None, - use_expanded=False): + use_expanded=False, no_write_symbols=False): """Check the image contains the expected symbol values Args: @@ -1481,9 +1481,14 @@ class TestFunctional(unittest.TestCase): sym_values = struct.pack('; + #size-cells = <1>; + + binman { + pad-byte = <0xff>; + u-boot-spl { + no-write-symbols; + }; + + u-boot { + offset = <0x38>; + no-expanded; + }; + + u-boot-spl2 { + type = "u-boot-spl"; + no-write-symbols; + }; + }; +}; -- 2.41.0.390.g38632f3daf-goog
[PATCH v3 03/19] binman: Use GetEntries() to obtain section contents
Some section types don't have a simple _entries list. Use the GetEntries() method in GetEntryContents() and other places to handle this. This makes the behaviour more consistent. Signed-off-by: Simon Glass --- (no changes since v1) tools/binman/etype/section.py | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 77250a7525c6..d56cc11d1023 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -720,7 +720,7 @@ class Entry_section(Entry): next_todo.append(entry) return entry -todo = self._entries.values() +todo = self.GetEntries().values() for passnum in range(3): threads = state.GetThreads() next_todo = [] @@ -893,7 +893,7 @@ class Entry_section(Entry): allow_missing: True if allowed, False if not allowed """ self.allow_missing = allow_missing -for entry in self._entries.values(): +for entry in self.GetEntries().values(): entry.SetAllowMissing(allow_missing) def SetAllowFakeBlob(self, allow_fake): @@ -903,7 +903,7 @@ class Entry_section(Entry): allow_fake: True if allowed, False if not allowed """ super().SetAllowFakeBlob(allow_fake) -for entry in self._entries.values(): +for entry in self.GetEntries().values(): entry.SetAllowFakeBlob(allow_fake) def CheckMissing(self, missing_list): @@ -915,7 +915,7 @@ class Entry_section(Entry): Args: missing_list: List of Entry objects to be added to """ -for entry in self._entries.values(): +for entry in self.GetEntries().values(): entry.CheckMissing(missing_list) def CheckFakedBlobs(self, faked_blobs_list): @@ -926,7 +926,7 @@ class Entry_section(Entry): Args: faked_blobs_list: List of Entry objects to be added to """ -for entry in self._entries.values(): +for entry in self.GetEntries().values(): entry.CheckFakedBlobs(faked_blobs_list) def CheckOptional(self, optional_list): @@ -937,7 +937,7 @@ class Entry_section(Entry): Args: optional_list (list): List of Entry objects to be added to """ -for entry in self._entries.values(): +for entry in self.GetEntries().values(): entry.CheckOptional(optional_list) def check_missing_bintools(self, missing_list): @@ -949,7 +949,7 @@ class Entry_section(Entry): missing_list: List of Bintool objects to be added to """ super().check_missing_bintools(missing_list) -for entry in self._entries.values(): +for entry in self.GetEntries().values(): entry.check_missing_bintools(missing_list) def _CollectEntries(self, entries, entries_by_name, add_entry): @@ -999,12 +999,12 @@ class Entry_section(Entry): entry.Raise(f'Missing required properties/entry args: {missing}') def CheckAltFormats(self, alt_formats): -for entry in self._entries.values(): +for entry in self.GetEntries().values(): entry.CheckAltFormats(alt_formats) def AddBintools(self, btools): super().AddBintools(btools) -for entry in self._entries.values(): +for entry in self.GetEntries().values(): entry.AddBintools(btools) def read_elf_segments(self): -- 2.41.0.390.g38632f3daf-goog
[PATCH v3 04/19] binman: Read _multiple_data_files in the correct place
Move this to the ReadEntries() function where it belongs. Signed-off-by: Simon Glass --- (no changes since v1) tools/binman/etype/mkimage.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index e028c4407081..cb3e10672ad7 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -121,7 +121,6 @@ class Entry_mkimage(Entry): """ def __init__(self, section, etype, node): super().__init__(section, etype, node) -self._multiple_data_files = fdt_util.GetBool(self._node, 'multiple-data-files') self._mkimage_entries = OrderedDict() self._imagename = None self._filename = fdt_util.GetString(self._node, 'filename') @@ -129,6 +128,8 @@ class Entry_mkimage(Entry): def ReadNode(self): super().ReadNode() +self._multiple_data_files = fdt_util.GetBool(self._node, + 'multiple-data-files') self._args = fdt_util.GetArgs(self._node, 'args') self._data_to_imagename = fdt_util.GetBool(self._node, 'data-to-imagename') -- 2.41.0.390.g38632f3daf-goog
[PATCH v3 01/19] binman: Correct coverage gap in control
Add a pragma to deal with the code-coverage gap which drops binman down to 90% coverage. Fixes: de65b122a25 (tools: Fall back to importlib_resources on Python 3.6) Signed-off-by: Simon Glass --- (no changes since v1) tools/binman/control.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/binman/control.py b/tools/binman/control.py index 68597c4e7792..7e2dd3541b96 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -9,7 +9,7 @@ from collections import OrderedDict import glob try: import importlib.resources -except ImportError: +except ImportError: # pragma: no cover # for Python 3.6 import importlib_resources import os -- 2.41.0.390.g38632f3daf-goog
[PATCH v3 02/19] binman: Init align_default in entry_Section
This should be set up in the init function, to avoid a warning about a property not set up there. Fix it. Signed-off-by: Simon Glass --- (no changes since v1) tools/binman/etype/section.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index c36edd13508b..77250a7525c6 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -168,6 +168,7 @@ class Entry_section(Entry): self._end_4gb = False self._ignore_missing = False self._filename = None +self.align_default = 0 def IsSpecialSubnode(self, node): """Check if a node is a special one used by the section itself -- 2.41.0.390.g38632f3daf-goog
[PATCH v3 00/19] binman: Simple templating feature and mkimage conversion
This series converts the mkimage entry type to be a section, i.e. based on the entry_Section class. This makes it more consistent in its behaviour, e.g. allowing symbol writing and expanded entries. A simple templating feature is also introduced, to reduce duplication when a set of entries must be used in multiple images. In this version the nodes from the template are placed before any other nodes, meaning that the template sets the node order. This seems more consistent than other mechanisms. Changes in v3: - Add new patch for elf to return number of written symbols - Add new patch with more detail on how ObtainContents() works - Fix up some tests which now need SPL and TPL - Avoid calling ObtainContents() when not needed - Fix 'specific' typo - Add a new devicetree file especially for node copying - Correct logic for merging nodes in order - Tidy up some comments - Adjust to use the new example file - Drop duplicate dts-v1 header - Add new test case for templating in a FIT - Add new patch to support writing symbols inside a mkimage image Changes in v2: - Drop now-unnecessary methods in mkimage etype - Correct ordering of template nodes - Fix 'preseverd' and 'inserter' typos Marek Vasut (1): binman: Convert mkimage to Entry_section Simon Glass (18): binman: Correct coverage gap in control binman: Init align_default in entry_Section binman: Use GetEntries() to obtain section contents binman: Read _multiple_data_files in the correct place binman: Allow disabling symbol writing stm32mp15: Avoid writing symbols in SPL binman: Update elf to return number of written symbols binman: Add more detail on how ObtainContents() works binman: Provide a way to specify the fdt-list directly binman: Drop __bss_size variable in bss_data.c binman: Correct handling of zero bss size dtoc: Support copying the contents of a node into another dtoc: Allow inserting a list of nodes into another binman: Support simple templates binman: Support templating with multiple images binman: Add a test for templating in a FIT binman: Support templates at any level binman: Support writing symbols inside a mkimage image arch/arm/dts/stm32mp15-u-boot.dtsi | 1 + tools/binman/binman.rst| 89 + tools/binman/control.py| 34 +++- tools/binman/elf.py| 13 +- tools/binman/elf_test.py | 13 +- tools/binman/entries.rst | 6 + tools/binman/entry.py | 11 +- tools/binman/etype/blob_phase.py | 5 + tools/binman/etype/fit.py | 9 + tools/binman/etype/mkimage.py | 110 --- tools/binman/etype/section.py | 54 +++-- tools/binman/etype/u_boot_spl_bss_pad.py | 2 +- tools/binman/etype/u_boot_tpl_bss_pad.py | 2 +- tools/binman/etype/u_boot_vpl_bss_pad.py | 2 +- tools/binman/ftest.py | 218 - tools/binman/test/282_symbols_disable.dts | 25 +++ tools/binman/test/283_mkimage_special.dts | 24 +++ tools/binman/test/284_fit_fdt_list.dts | 58 ++ tools/binman/test/285_spl_expand.dts | 13 ++ tools/binman/test/286_template.dts | 42 tools/binman/test/287_template_multi.dts | 27 +++ tools/binman/test/288_template_fit.dts | 37 tools/binman/test/289_template_section.dts | 52 + tools/binman/test/290_mkimage_sym.dts | 27 +++ tools/binman/test/Makefile | 5 +- tools/binman/test/bss_data.c | 3 +- tools/binman/test/bss_data_zero.c | 16 ++ tools/binman/test/bss_data_zero.lds| 15 ++ tools/binman/test/embed_data.lds | 1 + tools/dtoc/fdt.py | 122 +++- tools/dtoc/test/dtoc_test_copy.dts | 86 tools/dtoc/test_fdt.py | 93 + 32 files changed, 1147 insertions(+), 68 deletions(-) create mode 100644 tools/binman/test/282_symbols_disable.dts create mode 100644 tools/binman/test/283_mkimage_special.dts create mode 100644 tools/binman/test/284_fit_fdt_list.dts create mode 100644 tools/binman/test/285_spl_expand.dts create mode 100644 tools/binman/test/286_template.dts create mode 100644 tools/binman/test/287_template_multi.dts create mode 100644 tools/binman/test/288_template_fit.dts create mode 100644 tools/binman/test/289_template_section.dts create mode 100644 tools/binman/test/290_mkimage_sym.dts create mode 100644 tools/binman/test/bss_data_zero.c create mode 100644 tools/binman/test/bss_data_zero.lds create mode 100644 tools/dtoc/test/dtoc_test_copy.dts -- 2.41.0.390.g38632f3daf-goog
Re: [PATCH 0/4] introduce EFI_RAM_DISK_PROTOCOL
On Mon, Jul 10, 2023 at 11:13:12AM +0900, Masahisa Kojima wrote: > On Fri, 7 Jul 2023 at 18:12, AKASHI Takahiro > wrote: > > > > On Fri, Jul 07, 2023 at 05:19:33PM +0900, Masahisa Kojima wrote: > > > Hi Akashi-san, > > > > > > On Fri, 7 Jul 2023 at 16:16, AKASHI Takahiro > > > wrote: > > > > > > > > On Fri, Jul 07, 2023 at 08:29:12AM +0200, Heinrich Schuchardt wrote: > > > > > On 7/7/23 06:00, Masahisa Kojima wrote: > > > > > > This series introduces the EFI_RAM_DISK_PROTOCOL implementation. > > > > > > The major purpose of this series is a preparation for EFI HTTP(S) > > > > > > boot. > > > > > > > > > > > > Now U-Boot can download the distro installer ISO image > > > > > > via wget or tftpboot commands, but U-Boot can not mount > > > > > > the downloaded ISO image. > > > > > > By calling EFI_RAM_DISK_PROTOCOL->register API, user can > > > > > > mount the ISO image and boot the distro installer. > > > > > > > > > > If I understand you correctly, with your design RAM disks will only > > > > > exist in the EFI sub-system. > > > > > > > > Probably, not. As Kojima-san's "dm tree" shows, there is a U-Boot > > > > block device (and associated partition devices) thanks to your > > > > efi_driver framework. > > > > > > Read/Write the RAM disk is expected to be called from the EFI context, so > > > > An associated U-Boot block device should work as well on top of your > > RAW_DISK_PROTOCOL via a generated RAM disk object (efi_disk). > > That is why SYMPLE_FILE_SYSTEM_PROTOCOL, which solely relies on U-Boot's > > proper > > filesystem subsystem, is installed to the RAM disk object. > > I now realize that my RAM_DISK_PROTOCOL implementation happens to work > thanks to the block cache. > When I disable CONFIG_BLOCK_CACHE and load some EFI application(it > does set 'app_gd') > before calling RAM_DISK_PROTOCOL service, RAM_DISK_PROTOCOL does not work. > ConnectController fails. > > > > > > native U-Boot can not access the RAM disk. > > > > I believe that, in theory, "fatls efidisk 0 1" (or efi_disk as an > > interface?) > > should work even under your current implementation. > > "fatls efiloader 0:2" does not work. > I think "efiloader" device is designed to be accessed from EFI application > only > even if it is listed by "dm tree". I don't believe so. (the interface name may be "efi_blk" or "efiblk", though.) If the command fails, it's a bug. Must be fixed. > I understand that ramdisk as UCLASS_BLK driver is required as Heinrich said. That said, I agree to starting with UCLASS_BLK from the viewpoint of U-Boot driver model/UEFI integration. -Takahiro Akashi > > Thanks, > Masahisa Kojima > > > > > -Takahiro Akashi > > > > > # Honestly speaking, I'm not sure how U-Boot prohibits the access to > > > the EFI RAM disk > > > because the udevices are created for the RAM disk. > > > > > > Thanks, > > > Masahisa Kojima > > > > > > > > > > > > We strive to synchronize what is happening on the driver model level > > > > > and > > > > > on the UEFI level. I would prefer a design where we have a UCLASS_BLK > > > > > driver ram disks and the EFI_RAM_DISK_PROTOCOL is a one method of > > > > > managing ram disk devices. > > > > > > > > That said, I agree to starting with U-Boot block device implementation, > > > > UEFI_DISK comes automatically. It benefits both U-Boot proper and > > > > UEFI subsystem equally as well. > > > > (That is also what I meant to say in my first response.) > > > > > > > > > A big issue we have is RAM management. malloc() can only assign > > > > > limited > > > > > amount of memory which is probably too small for the RAM disk you are > > > > > looking at. We manage page sized memory in the EFI sub-system but this > > > > > is not integrated with the LMB memory checks. > > > > > > > > Not sure, is it enough simply to add some restrictions on the start size > > > > and the size when a memory region is specified for a raw disk? > > > > > > > > -Takahiro Akashi > > > > > > > > > The necessary sequence of development is probably: > > > > > > > > > > * Rework memory management > > > > > * Implement ramdisks as UCLASS_BLK driver > > > > > * Implement the EFI_RAM_DISK_PROTOCOL based on the UCLASS_BLK driver. > > > > > > > > > > Best regards > > > > > > > > > > Heinrich > > > > > > > > > > > > > > > > > Note that the installation process may not proceed > > > > > > after the distro installer calls ExitBootServices() > > > > > > since there is no hand-off process for the block device of > > > > > > memory mapped ISO image. > > > > > > > > > > > > The EFI_RAM_DISK_PROTOCOL was tested with the > > > > > > debian network installer[1] on qemu_arm64 platform. > > > > > > The example procedure is as follows. > > > > > > => tftpboot 4100 mini.iso > > > > > > => efidebug disk load 4100 $filesize > > > > > > > > > > > > After these commands, ISO image is mounted like: > > > > > > > > > > > > => efidebug dh > > > > > > > > > > > > 7eec5910 (efiblk#0) > > > > > > > > >
Re: [PATCH 0/4] introduce EFI_RAM_DISK_PROTOCOL
On Fri, 7 Jul 2023 at 18:12, AKASHI Takahiro wrote: > > On Fri, Jul 07, 2023 at 05:19:33PM +0900, Masahisa Kojima wrote: > > Hi Akashi-san, > > > > On Fri, 7 Jul 2023 at 16:16, AKASHI Takahiro > > wrote: > > > > > > On Fri, Jul 07, 2023 at 08:29:12AM +0200, Heinrich Schuchardt wrote: > > > > On 7/7/23 06:00, Masahisa Kojima wrote: > > > > > This series introduces the EFI_RAM_DISK_PROTOCOL implementation. > > > > > The major purpose of this series is a preparation for EFI HTTP(S) > > > > > boot. > > > > > > > > > > Now U-Boot can download the distro installer ISO image > > > > > via wget or tftpboot commands, but U-Boot can not mount > > > > > the downloaded ISO image. > > > > > By calling EFI_RAM_DISK_PROTOCOL->register API, user can > > > > > mount the ISO image and boot the distro installer. > > > > > > > > If I understand you correctly, with your design RAM disks will only > > > > exist in the EFI sub-system. > > > > > > Probably, not. As Kojima-san's "dm tree" shows, there is a U-Boot > > > block device (and associated partition devices) thanks to your > > > efi_driver framework. > > > > Read/Write the RAM disk is expected to be called from the EFI context, so > > An associated U-Boot block device should work as well on top of your > RAW_DISK_PROTOCOL via a generated RAM disk object (efi_disk). > That is why SYMPLE_FILE_SYSTEM_PROTOCOL, which solely relies on U-Boot's > proper > filesystem subsystem, is installed to the RAM disk object. I now realize that my RAM_DISK_PROTOCOL implementation happens to work thanks to the block cache. When I disable CONFIG_BLOCK_CACHE and load some EFI application(it does set 'app_gd') before calling RAM_DISK_PROTOCOL service, RAM_DISK_PROTOCOL does not work. ConnectController fails. > > > native U-Boot can not access the RAM disk. > > I believe that, in theory, "fatls efidisk 0 1" (or efi_disk as an interface?) > should work even under your current implementation. "fatls efiloader 0:2" does not work. I think "efiloader" device is designed to be accessed from EFI application only even if it is listed by "dm tree". I understand that ramdisk as UCLASS_BLK driver is required as Heinrich said. Thanks, Masahisa Kojima > > -Takahiro Akashi > > > # Honestly speaking, I'm not sure how U-Boot prohibits the access to > > the EFI RAM disk > > because the udevices are created for the RAM disk. > > > > Thanks, > > Masahisa Kojima > > > > > > > > > We strive to synchronize what is happening on the driver model level and > > > > on the UEFI level. I would prefer a design where we have a UCLASS_BLK > > > > driver ram disks and the EFI_RAM_DISK_PROTOCOL is a one method of > > > > managing ram disk devices. > > > > > > That said, I agree to starting with U-Boot block device implementation, > > > UEFI_DISK comes automatically. It benefits both U-Boot proper and > > > UEFI subsystem equally as well. > > > (That is also what I meant to say in my first response.) > > > > > > > A big issue we have is RAM management. malloc() can only assign limited > > > > amount of memory which is probably too small for the RAM disk you are > > > > looking at. We manage page sized memory in the EFI sub-system but this > > > > is not integrated with the LMB memory checks. > > > > > > Not sure, is it enough simply to add some restrictions on the start size > > > and the size when a memory region is specified for a raw disk? > > > > > > -Takahiro Akashi > > > > > > > The necessary sequence of development is probably: > > > > > > > > * Rework memory management > > > > * Implement ramdisks as UCLASS_BLK driver > > > > * Implement the EFI_RAM_DISK_PROTOCOL based on the UCLASS_BLK driver. > > > > > > > > Best regards > > > > > > > > Heinrich > > > > > > > > > > > > > > Note that the installation process may not proceed > > > > > after the distro installer calls ExitBootServices() > > > > > since there is no hand-off process for the block device of > > > > > memory mapped ISO image. > > > > > > > > > > The EFI_RAM_DISK_PROTOCOL was tested with the > > > > > debian network installer[1] on qemu_arm64 platform. > > > > > The example procedure is as follows. > > > > > => tftpboot 4100 mini.iso > > > > > => efidebug disk load 4100 $filesize > > > > > > > > > > After these commands, ISO image is mounted like: > > > > > > > > > > => efidebug dh > > > > > > > > > > 7eec5910 (efiblk#0) > > > > > > > > > > /RamDisk(0x4100,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0) > > > > >Block IO > > > > > > > > > > 7eec6140 (efiblk#0:1) > > > > > > > > > > /RamDisk(0x4100,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(1,0x01,0,0x0,0x33800) > > > > >Block IO > > > > > > > > > > 7eec62b0 (efiblk#0:2) > > > > > > > > > > /RamDisk(0x4100,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(2,0x01,0,0x33800,0x1) > > > > >Block IO > > > > >System Partition > > > > >Simple File System
Re: [PATCH 07/10] test: dm: add SCMI base protocol test
On Fri, Jul 07, 2023 at 11:35:49AM -0600, Simon Glass wrote: > Hi, > > On Tue, 4 Jul 2023 at 03:35, AKASHI Takahiro > wrote: > > > > Hi Simon, > > > > On Mon, Jul 03, 2023 at 02:30:57PM +0100, Simon Glass wrote: > > > Hi, > > > > > > On Mon, 3 Jul 2023 at 01:57, AKASHI Takahiro > > > wrote: > > > > > > > > On Thu, Jun 29, 2023 at 08:09:58PM +0100, Simon Glass wrote: > > > > > Hi AKASHI, > > > > > > > > > > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro > > > > > wrote: > > > > > > > > > > > > Added is a new unit test for SCMI base protocol, which will > > > > > > exercise all > > > > > > the commands provided by the protocol, except > > > > > > SCMI_BASE_NOTIFY_ERRORS. > > > > > > $ ut dm scmi_base > > > > > > It is assumed that test.dtb is used as sandbox's device tree. > > > > > > > > > > > > Signed-off-by: AKASHI Takahiro > > > > > > --- > > > > > > test/dm/scmi.c | 112 > > > > > > + > > > > > > 1 file changed, 112 insertions(+) > > > > > > > > > > > > diff --git a/test/dm/scmi.c b/test/dm/scmi.c > > > > > > index 881be3171b7c..563017bb63e0 100644 > > > > > > --- a/test/dm/scmi.c > > > > > > +++ b/test/dm/scmi.c > > > > > > @@ -16,6 +16,9 @@ > > > > > > #include > > > > > > #include > > > > > > #include > > > > > > +#include > > > > > > +#include > > > > > > +#include > > > > > > #include > > > > > > #include > > > > > > #include > > > > > > @@ -95,6 +98,115 @@ static int dm_test_scmi_sandbox_agent(struct > > > > > > unit_test_state *uts) > > > > > > } > > > > > > DM_TEST(dm_test_scmi_sandbox_agent, UT_TESTF_SCAN_FDT); > > > > > > > > > > > > +static int dm_test_scmi_base(struct unit_test_state *uts) > > > > > > +{ > > > > > > + struct udevice *agent_dev, *base; > > > > > > + struct scmi_agent_priv *priv; > > > > > > + const struct scmi_base_ops *ops; > > > > > > + u32 version, num_agents, num_protocols, impl_version; > > > > > > + u32 attributes, agent_id; > > > > > > + char vendor[SCMI_BASE_NAME_LENGTH_MAX], > > > > > > +agent_name[SCMI_BASE_NAME_LENGTH_MAX]; > > > > > > + u8 *protocols; > > > > > > + int ret; > > > > > > + > > > > > > + /* preparation */ > > > > > > + ut_assertok(uclass_get_device_by_name(UCLASS_SCMI_AGENT, > > > > > > "scmi", > > > > > > + &agent_dev)); > > > > > > + ut_assertnonnull(agent_dev); > > > > > > + ut_assertnonnull(priv = dev_get_uclass_plat(agent_dev)); > > > > > > + ut_assertnonnull(base = scmi_get_protocol(agent_dev, > > > > > > + > > > > > > SCMI_PROTOCOL_ID_BASE)); > > > > > > + ut_assertnonnull(ops = dev_get_driver_ops(base)); > > > > > > + > > > > > > + /* version */ > > > > > > + ret = (*ops->protocol_version)(base, &version); > > > > > > > > > > Can you add uclass helpers to call each of the methods? That is how it > > > > > is commonly done. You should not be calling ops->xxx directly here. > > > > > > > > Yes, I will add inline functions instead. > > > > > > I don't mean inline...see all the other uclasses which define a > > > > Okay, I will *real* functions. > > > > > function which is implemented in the uclass. It is confusing when one > > > uclass does something different. People might copy this style and then > > > the code base diverges. Did you not notice this when looking around > > > the source tree? > > > > But one concern came up in my mind. > > Contrary to ordinary "device controllers", there exists only a single > > implementation of driver for each of "udevice"'s associated with SCMI > > protocols including the base protocol. > > > > So if I follow your suggestion, the code (base.c) might look like: > > === > > static int __scmi_base_discover_vendor(struct udevice *dev, u8 *vendor) > > { > > ... > > } > > > > struct scmi_base_ops scmi_base_ops = { > > > > .base_discover_vendor = __scmi_base_discover_vendor, > > > > } > > > > int scmi_base_discover_vendor(struct udevice *dev, u8 *vendor) > > { > > struct scmi_base_ops *ops; > > > > ops = scmi_base_dev_ops(dev); > > > > return ops->base_discover_vendor(dev, vendor); > > } > > === > > > > We will have to have similar definitions for every operation in ops. > > It looks quite weird to me as there are always pairs of functions, > > like __scmi_base_discover_vendor() and scmi_base_discover_vendor(). > > Yes I understand that you only have one driver at present. Is there > not a sandbox driver? No. Please remember that SCMI protocol drivers on U-Boot are nothing but stubs that makes a call to SCMI servers, supporting common communication channel interfaces for different transports (either OP-TEE, SMCCC or mailbox). Sandbox driver, if is properly named, is also implemented as a sort of transport layer, where a invocation is replaced with a function call which mimicks one of specific commands in SCMI protocol on behalf
[GIT PULL] Please pull fsl-qoirq-2023-7-6 for next
Hi Tom, Please pull fsl-qoriq-2023-7-6 for next - Enable DM Serial for ls1043ardb and ls1046ardb/afrwy Fixed secure boot on LS-CH2 platforms - https://source.denx.de/u-boot/custodians/u-boot-fsl-qoriq/-/pipelines/16795 Thanks, Peng. The following changes since commit e80f4079b3a3db0961b73fa7a96e6c90242d8d25: Merge tag 'v2023.07-rc6' into next (2023-07-05 11:28:55 -0400) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-fsl-qoriq.git tags/fsl-qoriq-2023-7-6 for you to fetch changes up to fcf75435c8fd443547f5d4192d1cf70fb8a84034: LFU-544: Kconfig.nxp: Fixed secure boot on LS-CH2 platforms (2023-07-06 13:04:56 +0800) Camelia Groza (7): arch: arm: dts: ls1043a: sync serial nodes with Linux arch: arm: dts: ls1043a: tag serial nodes with bootph-all configs: ls1043ardb: enable DM_SERIAL arch: arm: dts: ls1046a: sync serial nodes with Linux arch: arm: dts: ls1046a: tag serial nodes with bootph-all configs: ls1046ardb: enable DM_SERIAL configs: ls1046afrwy: enable DM_SERIAL Kshitiz Varshney (1): LFU-544: Kconfig.nxp: Fixed secure boot on LS-CH2 platforms arch/Kconfig.nxp| 2 +- arch/arm/dts/fsl-ls1043a-qds.dtsi | 2 +- arch/arm/dts/fsl-ls1043a-rdb-u-boot.dtsi| 5 + arch/arm/dts/fsl-ls1043a-rdb.dts| 6 +- arch/arm/dts/fsl-ls1043a-u-boot.dtsi| 19 +++ arch/arm/dts/fsl-ls1043a.dtsi | 16 +++- arch/arm/dts/fsl-ls1046a-frwy-u-boot.dtsi | 5 + arch/arm/dts/fsl-ls1046a-frwy.dts | 22 +- arch/arm/dts/fsl-ls1046a-qds.dtsi | 2 +- arch/arm/dts/fsl-ls1046a-rdb-u-boot.dtsi| 5 + arch/arm/dts/fsl-ls1046a-rdb.dts| 14 +- arch/arm/dts/fsl-ls1046a-u-boot.dtsi| 19 +++ arch/arm/dts/fsl-ls1046a.dtsi | 28 +++- configs/ls1043ardb_SECURE_BOOT_defconfig| 4 +++- configs/ls1043ardb_defconfig| 4 +++- configs/ls1043ardb_nand_SECURE_BOOT_defconfig | 4 +++- configs/ls1043ardb_nand_defconfig | 3 ++- configs/ls1043ardb_sdcard_SECURE_BOOT_defconfig | 4 +++- configs/ls1043ardb_sdcard_defconfig | 3 ++- configs/ls1043ardb_tfa_SECURE_BOOT_defconfig| 4 +++- configs/ls1043ardb_tfa_defconfig| 4 +++- configs/ls1046afrwy_tfa_SECURE_BOOT_defconfig | 4 +++- configs/ls1046afrwy_tfa_defconfig | 4 +++- configs/ls1046ardb_emmc_defconfig | 3 ++- configs/ls1046ardb_qspi_SECURE_BOOT_defconfig | 4 +++- configs/ls1046ardb_qspi_defconfig | 4 +++- configs/ls1046ardb_qspi_spl_defconfig | 3 ++- configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig | 4 +++- configs/ls1046ardb_sdcard_defconfig | 3 ++- configs/ls1046ardb_tfa_SECURE_BOOT_defconfig| 4 +++- configs/ls1046ardb_tfa_defconfig| 4 +++- 31 files changed, 174 insertions(+), 38 deletions(-) create mode 100644 arch/arm/dts/fsl-ls1043a-rdb-u-boot.dtsi create mode 100644 arch/arm/dts/fsl-ls1043a-u-boot.dtsi create mode 100644 arch/arm/dts/fsl-ls1046a-frwy-u-boot.dtsi create mode 100644 arch/arm/dts/fsl-ls1046a-rdb-u-boot.dtsi create mode 100644 arch/arm/dts/fsl-ls1046a-u-boot.dtsi
Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware
On Fri, Jul 07, 2023 at 11:35:52AM -0600, Simon Glass wrote: > Hi, > > On Tue, 4 Jul 2023 at 02:26, AKASHI Takahiro > wrote: > > > > On Mon, Jul 03, 2023 at 02:30:54PM +0100, Simon Glass wrote: > > > Hi, > > > > > > On Mon, 3 Jul 2023 at 01:55, AKASHI Takahiro > > > wrote: > > > > > > > > On Thu, Jun 29, 2023 at 08:10:00PM +0100, Simon Glass wrote: > > > > > Hi AKASHI, > > > > > > > > > > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro > > > > > wrote: > > > > > > > > > > > > This command, "scmi", provides a command line interface to various > > > > > > SCMI > > > > > > protocols. It supports at least initially SCMI base protocol with > > > > > > the sub > > > > > > command, "base", and is intended mainly for debug purpose. > > > > > > > > > > > > Signed-off-by: AKASHI Takahiro > > > > > > --- > > > > > > cmd/Kconfig | 8 ++ > > > > > > cmd/Makefile | 1 + > > > > > > cmd/scmi.c | 373 > > > > > > +++ > > > > > > 3 files changed, 382 insertions(+) > > > > > > create mode 100644 cmd/scmi.c > > > > > > > > > > > > diff --git a/cmd/Kconfig b/cmd/Kconfig > > > > > > index 02e54f1e50fe..065470a76295 100644 > > > > > > --- a/cmd/Kconfig > > > > > > +++ b/cmd/Kconfig > > > > > > @@ -2504,6 +2504,14 @@ config CMD_CROS_EC > > > > > > a number of sub-commands for performing EC tasks such as > > > > > > updating its flash, accessing a small saved context area > > > > > > and talking to the I2C bus behind the EC (if there is > > > > > > one). > > > > > > + > > > > > > +config CMD_SCMI > > > > > > + bool "Enable scmi command" > > > > > > + depends on SCMI_FIRMWARE > > > > > > + default n > > > > > > + help > > > > > > + This command provides user interfaces to several SCMI > > > > > > protocols, > > > > > > + including Base protocol. > > > > > > > > > > Please mention what is SCMI > > > > > > > > I will give a full spelling. > > > > > > > > > > endmenu > > > > > > > > > > > > menu "Filesystem commands" > > > > > > diff --git a/cmd/Makefile b/cmd/Makefile > > > > > > index 6c37521b4e2b..826e0b74b587 100644 > > > > > > --- a/cmd/Makefile > > > > > > +++ b/cmd/Makefile > > > > > > @@ -156,6 +156,7 @@ obj-$(CONFIG_CMD_SATA) += sata.o > > > > > > obj-$(CONFIG_CMD_NVME) += nvme.o > > > > > > obj-$(CONFIG_SANDBOX) += sb.o > > > > > > obj-$(CONFIG_CMD_SF) += sf.o > > > > > > +obj-$(CONFIG_CMD_SCMI) += scmi.o > > > > > > obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o > > > > > > obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o > > > > > > obj-$(CONFIG_CMD_SEAMA) += seama.o > > > > > > diff --git a/cmd/scmi.c b/cmd/scmi.c > > > > > > new file mode 100644 > > > > > > index ..c97f77e97d95 > > > > > > --- /dev/null > > > > > > +++ b/cmd/scmi.c > > > > > > @@ -0,0 +1,373 @@ > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > +/* > > > > > > + * SCMI utility command > > > > > > + * > > > > > > + * Copyright (c) 2023 Linaro Limited > > > > > > + * Author: AKASHI Takahiro > > > > > > + */ > > > > > > + > > > > > > +#include > > > > > > +#include > > > > > > +#include > > > > > > +#include /* uclass_get_device */ > > > > > > > > > > Goes before linux/bitfield.h > > > > > > > > Can you tell me why? > > > > > > It is just a convention: > > > https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files > > > > Okay. > > > > > > > > > > > > +#include > > > > > > +#include > > > > > > +#include > > > > > > +#include > > > > > > +#include > > > > > > +#include > > > > > > + > > > > > > +static struct udevice *agent; > > > > > > +static struct udevice *base_proto; > > > > > > +static const struct scmi_base_ops *ops; > > > > > > + > > > > > > +struct { > > > > > > + enum scmi_std_protocol id; > > > > > > + const char *name; > > > > > > +} protocol_name[] = { > > > > > > + {SCMI_PROTOCOL_ID_BASE, "Base"}, > > > > > > + {SCMI_PROTOCOL_ID_POWER_DOMAIN, "Power domain management"}, > > > > > > + {SCMI_PROTOCOL_ID_SYSTEM, "System power management"}, > > > > > > + {SCMI_PROTOCOL_ID_PERF, "Performance domain management"}, > > > > > > + {SCMI_PROTOCOL_ID_CLOCK, "Clock management"}, > > > > > > + {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"}, > > > > > > + {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"}, > > > > > > + {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain > > > > > > management"}, > > > > > > +}; > > > > > > + > > > > > > +/** > > > > > > + * scmi_convert_id_to_string() - get the name of SCMI protocol > > > > > > + * > > > > > > + * @id:Protocol ID > > > > > > + * > > > > > > + * Get the printable name of the protocol, @id > > > > > > + * > > > > > > + * Return: Name string on success, NULL on failure > > > > > > + */ > > > > > > +static const char *scmi_convert_id_to_string(enum > > > > > > scmi_std_protocol id) > > > > > > > > > > scmi_lookup_id? > > > > > > >
[BUG] fdt_pack_reg in common/fdt_support.c can cause crash from unaligned access
Hi, I'm trying to port U-Boot to a new board (Samsung JACKPOTLTE, ARMv8, Exynos7885) but when CONFIG_ARCH_FIXUP_FDT_MEMORY is enabled, the bootm command leads to an unaligned memory access, which results in a synchronous abort. After a long debugging session, I concluded that fdt_pack_reg in common/fdt_support.c writes to unaligned addresses in its for loop. In the case of address_cells being 2, and size_cells being 1, the buffer pointer gets incremented by 12 in each loop, making the second iteration (i=1) write a 64bit value to a non 64bit aligned address. Turning the alignment check enable bit (A) off in SCTLR makes the function work as intended. I couldn't find code that touches this bit, but I may have missed something. I don't think writing in two parts should be the fix, but something should be done about this. As far as I understand, any arm64 board that has this bit turned on, either from previous code or just the initial status of the bit after power on, could crash here. This is on top of the latest commit as of now (0beb649053b86b2cfd5cf55a0fc68bc2fe91a430) What should be done here? Best regards, David
[PATCH] common: Kconfig: Fix CMD_BMP/BMP dependency
Using `default y` will not select BMP when CMD_BMP has been enabled, if it was already configured. By using `select`, if `CMD_BMP` is turned on, it will force the presence of `BMP`. Fixes: 072b0e16c482114d242580dd7a3197db5966705f Signed-off-by: Samuel Dionne-Riel --- cmd/Kconfig| 1 + common/Kconfig | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index 02e54f1e50..94c54b2359 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1988,6 +1988,7 @@ config CMD_2048 config CMD_BMP bool "Enable 'bmp' command" depends on VIDEO + select BMP help This provides a way to obtain information about a BMP-format image and to display it. BMP (which presumably stands for BitMaP) is a diff --git a/common/Kconfig b/common/Kconfig index bbabadb35e..d0117e3dc8 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -1157,7 +1157,7 @@ config IO_TRACE config BMP bool "Enable bmp image display" - default y if CMD_BMP + default n help Enable bmp functions to display bmp image and get bmp info. -- 2.41.0
[PATCH v2 6/6] arm: mvebu: Remove unused alias from RC AC5X dts
The sar-reg0 alias was left over from an earlier iteration of the patches adding support for this board. Remove the unused alias. Fixes: 6cc8b5db40 ("arm: mvebu: Add RD-AC5X board") Signed-off-by: Chris Packham --- arch/arm/dts/ac5-98dx35xx-rd.dts | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/arm/dts/ac5-98dx35xx-rd.dts b/arch/arm/dts/ac5-98dx35xx-rd.dts index d9f217cd4a5f..1dc85bb7ef24 100644 --- a/arch/arm/dts/ac5-98dx35xx-rd.dts +++ b/arch/arm/dts/ac5-98dx35xx-rd.dts @@ -31,7 +31,6 @@ usb0 = &usb0; usb1 = &usb1; pinctrl0 = &pinctrl0; - sar-reg0 = "/config-space/sar-reg"; }; usb1phy: usb-phy { -- 2.41.0
[PATCH v2 5/6] arm: mvebu: Add Allied Telesis x240 board
The x240 and SE240 are a series of L2+ switches from Allied Telesis. There are a number of them in the range but as far as U-Boot is concerned all the CPU block components are the same so there's only one board defined. Signed-off-by: Chris Packham --- Notes: Changes in v2: - drop CONFIG_DEBUG_UART arch/arm/dts/Makefile | 3 +- arch/arm/dts/ac5-98dx35xx-atl-x240.dts | 212 + arch/arm/mach-mvebu/Kconfig| 7 + board/alliedtelesis/x240/MAINTAINERS | 7 + board/alliedtelesis/x240/Makefile | 6 + board/alliedtelesis/x240/x240.c| 13 ++ configs/x240_defconfig | 86 ++ include/configs/x240.h | 37 + 8 files changed, 370 insertions(+), 1 deletion(-) create mode 100644 arch/arm/dts/ac5-98dx35xx-atl-x240.dts create mode 100644 board/alliedtelesis/x240/MAINTAINERS create mode 100644 board/alliedtelesis/x240/Makefile create mode 100644 board/alliedtelesis/x240/x240.c create mode 100644 configs/x240_defconfig create mode 100644 include/configs/x240.h diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 480269fa6065..38d878a0f853 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -303,7 +303,8 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ cn9132-db-B.dtb \ cn9130-crb-A.dtb\ cn9130-crb-B.dtb\ - ac5-98dx35xx-rd.dtb + ac5-98dx35xx-rd.dtb \ + ac5-98dx35xx-atl-x240.dtb endif dtb-$(CONFIG_ARCH_SYNQUACER) += synquacer-sc2a11-developerbox.dtb diff --git a/arch/arm/dts/ac5-98dx35xx-atl-x240.dts b/arch/arm/dts/ac5-98dx35xx-atl-x240.dts new file mode 100644 index ..820ec18b4355 --- /dev/null +++ b/arch/arm/dts/ac5-98dx35xx-atl-x240.dts @@ -0,0 +1,212 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) + +/dts-v1/; + +#include +#include +#include "ac5-98dx35xx.dtsi" + +/ { + model = "Allied Telesis x240"; + compatible = "alliedtelesis,x240", "marvell,ac5x", "marvell,ac5"; + + aliases { + serial0 = &uart0; + spiflash0 = &spiflash0; + gpio0 = &gpio0; + gpio1 = &gpio1; + spi0 = &spi0; + i2c0 = &i2c0; + usb0 = &usb0; + pinctrl0 = &pinctrl0; + }; + + chosen { + stdout-path = "serial0:115200n8"; + }; + + boot-board { + compatible = "atl,boot-board"; + present-gpio = <&gpio1 6 GPIO_ACTIVE_HIGH>; + override-gpio = <&gpio1 2 GPIO_ACTIVE_HIGH>; + }; + + gpio-leds { + compatible = "gpio-leds"; + + fault { + label = "fault:red"; + gpios = <&system_gpio 11 GPIO_ACTIVE_LOW>; + default-state = "on"; + }; + }; +}; + +&nand { + pinctrl-names = "default"; + pinctrl-0 = <&nand_pins>; + + nand-ecc-strength = <4>; + nand-ecc-step-size = <512>; + status = "okay"; + + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + partition@user { + reg = <0x 0x1000>; + label = "user"; + }; + }; +}; + +&uart0 { + status = "okay"; +}; + +&usb0 { + status = "okay"; +}; + +&i2c0 { + status = "okay"; + + mux@71 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "nxp,pca9546"; + reg = <0x71>; + i2c-mux-idle-disconnect; + reset-gpios = <&gpio1 4 GPIO_ACTIVE_LOW>; /* MPP36 */ + status = "okay"; + + i2c@1 { + #address-cells = <1>; + #size-cells = <0>; + reg = <1>; + + hwmon@2e { + compatible = "adi,adt7476"; + reg = <0x2e>; + }; + + rtc@68 { + compatible = "adi,max31331"; + reg = <0x68>; + }; + + system_gpio: gpio@27 { + compatible = "nxp,pca9555"; + gpio-controller; + #gpio-cells= <2>; + reg = <0x27>; + interrupt-parent = <&gpio0>; + interrupts = <25 IRQ_TYPE_LEVEL_LOW>; /* MPP25 */ + }; + }; + }; +}; + +&spi0 { + status = "okay"; + spiflash0: flash@0 { + compatible = "jedec,spi-nor"; + spi-max-frequency = <
[PATCH v2 4/6] mtd: nand: pxa3xx: Enable devbus/nand arbiter on Armada 8K
The CN9130 SoC (an ARMADA 8K type) has both a NAND Flash Controller and a generic local bus controller (Device Bus Controller) that share common pins. With a board design that incorporates both a NAND flash and uses the Device Bus (in our case for an SRAM) accessing the Device Bus device fails unless the NfArbiterEn bit is set. Setting the bit enables arbitration between the Device Bus and the NAND flash. Since there is no obvious downside in enabling this for designs that don't require arbitration, we always enable it. Signed-off-by: Chris Packham --- drivers/mtd/nand/raw/pxa3xx_nand.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/nand/raw/pxa3xx_nand.c b/drivers/mtd/nand/raw/pxa3xx_nand.c index 9dee580ab9c2..d502e967f92c 100644 --- a/drivers/mtd/nand/raw/pxa3xx_nand.c +++ b/drivers/mtd/nand/raw/pxa3xx_nand.c @@ -125,6 +125,7 @@ DECLARE_GLOBAL_DATA_PTR; /* System control register and bit to enable NAND on some SoCs */ #define GENCONF_SOC_DEVICE_MUX 0x208 #define GENCONF_SOC_DEVICE_MUX_NFC_EN BIT(0) +#define GENCONF_SOC_DEVICE_MUX_NFC_DEVBUS_ARB_EN BIT(27) /* * This should be large enough to read 'ONFI' and 'JEDEC'. @@ -1739,7 +1740,7 @@ static int alloc_nand_resource(struct udevice *dev, struct pxa3xx_nand_info *inf return PTR_ERR(sysctrl_base); regmap_read(sysctrl_base, GENCONF_SOC_DEVICE_MUX, ®); - reg |= GENCONF_SOC_DEVICE_MUX_NFC_EN; + reg |= GENCONF_SOC_DEVICE_MUX_NFC_EN | GENCONF_SOC_DEVICE_MUX_NFC_DEVBUS_ARB_EN; regmap_write(sysctrl_base, GENCONF_SOC_DEVICE_MUX, reg); } -- 2.41.0
[PATCH v2 3/6] mtd: nand: pxa3xx: Add support for the Marvell AC5 SoC
The NAND flash controller (NFC) on the AC5/AC5X SoC is the same as the NFC used on other Marvell SoCs. It does have the additional restriction of only supporting SDR timing modes up to 3. Signed-off-by: Chris Packham --- drivers/mtd/nand/raw/pxa3xx_nand.c | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/mtd/nand/raw/pxa3xx_nand.c b/drivers/mtd/nand/raw/pxa3xx_nand.c index fcd1b9c63614..9dee580ab9c2 100644 --- a/drivers/mtd/nand/raw/pxa3xx_nand.c +++ b/drivers/mtd/nand/raw/pxa3xx_nand.c @@ -167,6 +167,7 @@ enum pxa3xx_nand_variant { PXA3XX_NAND_VARIANT_PXA, PXA3XX_NAND_VARIANT_ARMADA370, PXA3XX_NAND_VARIANT_ARMADA_8K, + PXA3XX_NAND_VARIANT_AC5, }; struct pxa3xx_nand_host { @@ -391,6 +392,10 @@ static const struct udevice_id pxa3xx_nand_dt_ids[] = { .compatible = "marvell,armada-8k-nand-controller", .data = PXA3XX_NAND_VARIANT_ARMADA_8K, }, + { + .compatible = "marvell,mvebu-ac5-pxa3xx-nand", + .data = PXA3XX_NAND_VARIANT_AC5, + }, {} }; @@ -505,6 +510,9 @@ static int pxa3xx_nand_init_timings(struct pxa3xx_nand_host *host) if (mode < 0) mode = 0; + if (info->variant == PXA3XX_NAND_VARIANT_AC5) + mode = min(mode, 3); + timings = onfi_async_timing_mode_to_sdr_timings(mode); if (IS_ERR(timings)) return PTR_ERR(timings); @@ -730,7 +738,8 @@ static irqreturn_t pxa3xx_nand_irq(struct pxa3xx_nand_info *info) /* NDCB3 register is available in NFCv2 (Armada 370/XP SoC) */ if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370 || - info->variant == PXA3XX_NAND_VARIANT_ARMADA_8K) + info->variant == PXA3XX_NAND_VARIANT_ARMADA_8K || + info->variant == PXA3XX_NAND_VARIANT_AC5) nand_writel(info, NDCB0, info->ndcb3); } @@ -1579,7 +1588,8 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd) /* Device detection must be done with ECC disabled */ if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370 || - info->variant == PXA3XX_NAND_VARIANT_ARMADA_8K) + info->variant == PXA3XX_NAND_VARIANT_ARMADA_8K || + info->variant == PXA3XX_NAND_VARIANT_AC5) nand_writel(info, NDECCCTRL, 0x0); if (nand_scan_ident(mtd, 1, NULL)) @@ -1630,7 +1640,8 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd) */ if (mtd->writesize > info->chunk_size) { if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370 || - info->variant == PXA3XX_NAND_VARIANT_ARMADA_8K) { + info->variant == PXA3XX_NAND_VARIANT_ARMADA_8K || + info->variant == PXA3XX_NAND_VARIANT_AC5) { chip->cmdfunc = nand_cmdfunc_extended; } else { dev_err(mtd->dev, -- 2.41.0
[PATCH v2 2/6] arm: mvebu: ac5: Define mvebu_get_nand_clock()
The NF_CLK for the AC5 SoC runs at 400MHz. There's no strapping or gating require so just add a mvebu_get_nand_clock() that returns this value. Signed-off-by: Chris Packham --- arch/arm/mach-mvebu/alleycat5/soc.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/arm/mach-mvebu/alleycat5/soc.c b/arch/arm/mach-mvebu/alleycat5/soc.c index dc69f46eedb2..734b0a87dd49 100644 --- a/arch/arm/mach-mvebu/alleycat5/soc.c +++ b/arch/arm/mach-mvebu/alleycat5/soc.c @@ -255,6 +255,12 @@ void soc_print_clock_info(void) printf("\tMSS %4d MHz\n", 200); } +/* Return NAND clock in Hz */ +u32 mvebu_get_nand_clock(void) +{ + return 400 * 100; +} + /* * Override of __weak int mach_cpu_init(void) : * SoC/machine dependent CPU setup -- 2.41.0
[PATCH v2 1/6] arm: mvebu: ac5: Add nand-controller node
The AC5/AC5X SoC has a NAND flash controller. Add this to the SoC device tree. Signed-off-by: Chris Packham --- arch/arm/dts/ac5-98dx25xx.dtsi | 9 + 1 file changed, 9 insertions(+) diff --git a/arch/arm/dts/ac5-98dx25xx.dtsi b/arch/arm/dts/ac5-98dx25xx.dtsi index 3c68355f323a..f53b4781d7fd 100644 --- a/arch/arm/dts/ac5-98dx25xx.dtsi +++ b/arch/arm/dts/ac5-98dx25xx.dtsi @@ -251,6 +251,15 @@ status = "disabled"; }; + nand: nand-controller@805b { + compatible = "marvell,mvebu-ac5-pxa3xx-nand"; + reg = <0x0 0x805b 0x0 0x54>; + #address-cells = <0x0001>; + marvell,nand-enable-arbiter; + num-cs = <0x0001>; + status = "disabled"; + }; + gic: interrupt-controller@8060 { compatible = "arm,gic-v3"; #interrupt-cells = <3>; -- 2.41.0
[PATCH v2 0/6] Support for AC5X NAND and AT-x240 board
This series adds support for the NAND flash controller on the AC5X SoC and adds support for the Allied Telesis x240 board. I've also included 2 unrelated changes. "arm: mvebu: Remove unused alias from RC AC5X dts" removes an unused alias from the dts. This was in the neighborhood of the x240 so I included it. "mtd: nand: pxa3xx: Enable devbus/nand arbiter on Armada 8K" allows using the NFC and device bus at the same time. This is needed for another board (CN9130 based) that I'll hopefully get upstream in the near future. I figured I'd include it now since I was touching the NAND driver. Chris Packham (6): arm: mvebu: ac5: Add nand-controller node arm: mvebu: ac5: Define mvebu_get_nand_clock() mtd: nand: pxa3xx: Add support for the Marvell AC5 SoC mtd: nand: pxa3xx: Enable devbus/nand arbiter on Armada 8K arm: mvebu: Add Allied Telesis x240 board arm: mvebu: Remove unused alias from RC AC5X dts arch/arm/dts/Makefile | 3 +- arch/arm/dts/ac5-98dx25xx.dtsi | 9 ++ arch/arm/dts/ac5-98dx35xx-atl-x240.dts | 212 + arch/arm/dts/ac5-98dx35xx-rd.dts | 1 - arch/arm/mach-mvebu/Kconfig| 7 + arch/arm/mach-mvebu/alleycat5/soc.c| 6 + board/alliedtelesis/x240/MAINTAINERS | 7 + board/alliedtelesis/x240/Makefile | 6 + board/alliedtelesis/x240/x240.c| 13 ++ configs/x240_defconfig | 86 ++ drivers/mtd/nand/raw/pxa3xx_nand.c | 20 ++- include/configs/x240.h | 37 + 12 files changed, 401 insertions(+), 6 deletions(-) create mode 100644 arch/arm/dts/ac5-98dx35xx-atl-x240.dts create mode 100644 board/alliedtelesis/x240/MAINTAINERS create mode 100644 board/alliedtelesis/x240/Makefile create mode 100644 board/alliedtelesis/x240/x240.c create mode 100644 configs/x240_defconfig create mode 100644 include/configs/x240.h -- 2.41.0
Re: [bug report] sunxi: H6: no ethernet on Orange Pi One Plus
On Sun, Jul 09, 2023 at 04:32:22PM +, Anne Macedo wrote: > On Tue, Jul 04, 2023 at 11:22:29PM +, Anne Macedo wrote: > > Hey! > > > > I'm trying to bake Linux images for the Orange Pi One Plus using Yocto. > > Everything works fine, except for Ethernet. > > > > On the u-boot prompt: > > > > => dhcp > > No ethernet found. > > > > After adding: > > > > CONFIG_SPL_SPI_SUNXI=y > > CONFIG_SUN8I_EMAC=y > > > > to configs/orangepi_one_plus_defconfig, I started seeing this error: > > > > => dhcp > > sun8i_emac_eth_start: Timeout > > > > I saw this other bug report but I couldn't really understand what has > > been made to fix this issue [1]. > > > > More context here [2]. > > > > [1] https://lists.denx.de/pipermail/u-boot/2021-June/451357.html > > [2] https://github.com/linux-sunxi/meta-sunxi/issues/387 > > > > Regards, > > Anne Quick update > > Just wanted to share a summary of my findings about the ethernet on the > Orange Pi One Plus (Allwinner H6). > > 1. PMIC should not be disabled. I tested and if I disable PMIC, MAC > never turns on, even if I force the gpio PD6 pin to be on. When building > tfa, use SUNXI_SETUP_REGULATORS=1 or just don't pass it. > > 2. These configs are needed on configs/orangepi_one_plus_defconfig > > CONFIG_SPL_SPI_SUNXI=y CONFIG_SPL_SPI_SUNXI may not be needed in this context, I don't know why I mentioned it in the first place. > CONFIG_SUN8I_EMAC=y > > 3. With this config, there's this strange behavior where ethernet is > only detected after a crash: > > # Fresh boot > Net: Could not get PHY for ethernet@502: addr 1 > No ethernet found. > > # Forcing the board to crash > => mii dump > "Synchronous Abort" handler, esr 0x9644 > > Code: 3221 d5033fbf 91408013 f9481a60 (b9004801) > Resetting CPU ... > > resetting ... > > # Reboot > Net: eth0: ethernet@502 > > 4. I'm testing CONFIG_MACPWR="PD6" and that successfully enabled MAC on > u-boot (I see the LEDs turning on). However, I see the behaviour from #3 > but crash-rebooting doesn't seem to enable ethernet... According to this patch [1], we should use DM driver, so I believe CONFIG_MACPWR is deprecated. I then added these configs: # Required by regulator CONFIG_DM=y # Required by regulator fixed CONFIG_DM_REGULATOR=y # Required by the ethernet's definition on the dts CONFIG_DM_REGULATOR_FIXED=y # This adds a handy regulator cmd on u-boot CONFIG_CMD_REGULATOR=y With the regulator command, I turned it on and saw the LEDs turning on and dhcp "trying" to work (it doesn't get an IP though). => regulator list | Device | regulator-name | Parent | vcc5v | vcc-5v | root_driver | gmac-3v3| vcc-gmac-3v3| root_driver => regulator dev vcc-gmac-3v3 dev: vcc-gmac-3v3 @ gmac-3v3 => regulator enable => dhcp BOOTP broadcast 1 BOOTP broadcast 2 For some reason, vcc-gmac-3v3 is disabled by default, and it only seems to start manually for me. It sometimes start when I try a mii dump or a dhcp, but there's a lot of phy errors that make it unusable. [1] https://lists.denx.de/pipermail/u-boot/2022-December/501397.html > > Regards, > Anne
Re: [bug report] sunxi: H6: no ethernet on Orange Pi One Plus
On Tue, Jul 04, 2023 at 11:22:29PM +, Anne Macedo wrote: > Hey! > > I'm trying to bake Linux images for the Orange Pi One Plus using Yocto. > Everything works fine, except for Ethernet. > > On the u-boot prompt: > > => dhcp > No ethernet found. > > After adding: > > CONFIG_SPL_SPI_SUNXI=y > CONFIG_SUN8I_EMAC=y > > to configs/orangepi_one_plus_defconfig, I started seeing this error: > > => dhcp > sun8i_emac_eth_start: Timeout > > I saw this other bug report but I couldn't really understand what has > been made to fix this issue [1]. > > More context here [2]. > > [1] https://lists.denx.de/pipermail/u-boot/2021-June/451357.html > [2] https://github.com/linux-sunxi/meta-sunxi/issues/387 > > Regards, > Anne Just wanted to share a summary of my findings about the ethernet on the Orange Pi One Plus (Allwinner H6). 1. PMIC should not be disabled. I tested and if I disable PMIC, MAC never turns on, even if I force the gpio PD6 pin to be on. When building tfa, use SUNXI_SETUP_REGULATORS=1 or just don't pass it. 2. These configs are needed on configs/orangepi_one_plus_defconfig CONFIG_SPL_SPI_SUNXI=y CONFIG_SUN8I_EMAC=y 3. With this config, there's this strange behavior where ethernet is only detected after a crash: # Fresh boot Net: Could not get PHY for ethernet@502: addr 1 No ethernet found. # Forcing the board to crash => mii dump "Synchronous Abort" handler, esr 0x9644 Code: 3221 d5033fbf 91408013 f9481a60 (b9004801) Resetting CPU ... resetting ... # Reboot Net: eth0: ethernet@502 4. I'm testing CONFIG_MACPWR="PD6" and that successfully enabled MAC on u-boot (I see the LEDs turning on). However, I see the behaviour from #3 but crash-rebooting doesn't seem to enable ethernet... Regards, Anne
Re: Pull request efi-2023-07-rc7
On Sun, Jul 09, 2023 at 12:38:46PM +0200, Heinrich Schuchardt wrote: > Dear Tom, > > The following changes since commit 0beb649053b86b2cfd5cf55a0fc68bc2fe91a430: > > MAINTAINERS: correct at91 tree link (2023-07-07 11:37:09 -0400) > > are available in the Git repository at: > > https://source.denx.de/u-boot/custodians/u-boot-efi.git > tags/efi-2023-07-rc7 > > for you to fetch changes up to e05a4b12a056fd7fe22e85715c8f5e5e811fb551: > > mkeficapsule: fix efi_firmware_management_capsule_header data type > (2023-07-09 10:32:28 +0200) > > Gitlab CI showed no issues: > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/16815 > > > Pull request efi-2023-07-rc7 > > Documentation: > > * Fix links to Linux kernel documentation > > UEFI: > > * Fix memory leak in efidebug dh subcommand > * Fix underflow when calculating remaining variable store size > * Increase default variable store size to 64 KiB > * mkeficapsule: fix efi_firmware_management_capsule_header data type None of these look like boot regression fixes, nor security fixes that we should have already applied earlier this cycle, so I'm going to hold this off for v2023.07 unless you strongly object. -- Tom signature.asc Description: PGP signature
RE: [PATCH 1/2] cmd: thordown: Add proper dependency for CMD_THOR_DOWNLOAD
Hi Heinrich, > -Original Message- > From: Heinrich Schuchardt > Sent: Sunday, July 9, 2023 7:09 PM > To: Soma, Ashok Reddy ; u- > b...@lists.denx.de > Cc: s...@chromium.org; ilias.apalodi...@linaro.org; rfried@gmail.com; > seanedm...@microsoft.com; tob...@waldekranz.com; s...@denx.de; > j...@metanate.com; Simek, Michal ; git (AMD- > Xilinx) > Subject: Re: [PATCH 1/2] cmd: thordown: Add proper dependency for > CMD_THOR_DOWNLOAD > > > > Am 9. Juli 2023 15:09:57 MESZ schrieb Ashok Reddy Soma > : > >When CONFIG_CMD_USB and CONFIG_USB are disabled some compilation > errors > >are seen as below. > > Thanks for your patch. > > Currently we have no documentation for the thordown command. We > should create a man page in /docs/usage/cmd/. > > Do you have any description of the usage of the command? No, I was not working with thor download command I was disabling CONFIG_CMD_USB and CONFIG_USB and saw some compilation errors from cmd/thordown.c. So, added dependency and sent patch. Thanks, Ashok > > Best regards > > Heinrich > > > > > > >cmd/thordown.o: in function `usb_gadget_initialize': > >include/linux/usb/gadget.h:981: undefined reference to `board_usb_init' > >cmd/thordown.o: in function `do_thor_down': > >cmd/thordown.c:68: undefined reference to `g_dnl_unregister' > >cmd/thordown.o: in function `usb_gadget_release': > >include/linux/usb/gadget.h:986: undefined reference to > `board_usb_cleanup' > >cmd/thordown.o: in function `do_thor_down': > >cmd/thordown.c:41: undefined reference to `g_dnl_register' > >cmd/thordown.c:48: undefined reference to `thor_init' > >cmd/thordown.c:56: undefined reference to `thor_handle' > >gnu/aarch64/lin/aarch64-linux/bin/aarch64-linux-gnu-ld.bfd: line 4: 8485 > >Segmentation fault (core dumped) $CC --sysroot=$LIBC > >--no-warn-rwx-segment "$@" > >Makefile:1779: recipe for target 'u-boot' failed > >make: *** [u-boot] Error 139 > >make: *** Deleting file 'u-boot' > > > >Add dependency of CMD_USB for CONFIG_CMD_THOR_DOWNLOAD to fix > the errors. > > > >Signed-off-by: Ashok Reddy Soma > >--- > > > > cmd/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > >diff --git a/cmd/Kconfig b/cmd/Kconfig > >index 02e54f1e50..b44df9d67a 100644 > >--- a/cmd/Kconfig > >+++ b/cmd/Kconfig > >@@ -526,6 +526,7 @@ config CMD_SPL_WRITE_SIZE > > > > config CMD_THOR_DOWNLOAD > > bool "thor - TIZEN 'thor' download" > >+depends on CMD_USB > > select DFU > > help > > Implements the 'thor' download protocol. This is a way of
Re: [PATCH v3 02/11] capsule: authenticate: Add capsule public key in platform's dtb
Am 9. Juli 2023 15:33:17 MESZ schrieb Sughosh Ganu : >The EFI capsule authentication logic in u-boot expects the public key >in the form of an EFI Signature List(ESL) to be provided as part of >the platform's dtb. Currently, the embedding of the ESL file into the >dtb needs to be done manually. > >Add a signature node in the u-boot dtsi file and include the public >key through the capsule-key property. This file is per architecture, >and is currently being added for sandbox and arm architectures. It The device-tree compiler can pick up files from /include/. If the dtsi file is not architecture specific, we should avoid code duplication. We should treat all EFI architectures the same. Best regards Heinrich >will have to be added for other architectures which need to enable >capsule authentication support. > >The path to the ESL file is specified through the >CONFIG_EFI_CAPSULE_ESL_FILE symbol. > >Signed-off-by: Sughosh Ganu >--- >Changes since V2: >* Add the public key ESL file through the u-boot.dtsi. >* Add the dtsi files for sandbox and arm architectures. >* Add a check in the Makefile that the ESL file path is not empty. > > arch/arm/dts/u-boot.dtsi | 17 + > arch/sandbox/dts/u-boot.dtsi | 17 + > lib/efi_loader/Kconfig | 11 +++ > lib/efi_loader/Makefile | 7 +++ > 4 files changed, 52 insertions(+) > create mode 100644 arch/arm/dts/u-boot.dtsi > create mode 100644 arch/sandbox/dts/u-boot.dtsi > >diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi >new file mode 100644 >index 00..60bd004937 >--- /dev/null >+++ b/arch/arm/dts/u-boot.dtsi >@@ -0,0 +1,17 @@ >+// SPDX-License-Identifier: GPL-2.0+ >+/* >+ * Devicetree file with miscellaneous nodes that will be included >+ * at build time into the DTB. Currently being used for including >+ * capsule related information. >+ * >+ */ >+ >+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT >+/ { >+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE >+ signature { >+ capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE); >+ }; >+#endif >+}; >+#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */ >diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi >new file mode 100644 >index 00..60bd004937 >--- /dev/null >+++ b/arch/sandbox/dts/u-boot.dtsi >@@ -0,0 +1,17 @@ >+// SPDX-License-Identifier: GPL-2.0+ >+/* >+ * Devicetree file with miscellaneous nodes that will be included >+ * at build time into the DTB. Currently being used for including >+ * capsule related information. >+ * >+ */ >+ >+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT >+/ { >+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE >+ signature { >+ capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE); >+ }; >+#endif >+}; >+#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */ >diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig >index c5835e6ef6..1326a1d109 100644 >--- a/lib/efi_loader/Kconfig >+++ b/lib/efi_loader/Kconfig >@@ -234,6 +234,17 @@ config EFI_CAPSULE_MAX > Select the max capsule index value used for capsule report > variables. This value is used to create CapsuleMax variable. > >+config EFI_CAPSULE_ESL_FILE >+ string "Path to the EFI Signature List File" >+ default "" >+ depends on EFI_CAPSULE_AUTHENTICATE >+ help >+Provides the absolute path to the EFI Signature List >+file which will be embedded in the platform's device >+tree and used for capsule authentication at the time >+of capsule update. >+ >+ > config EFI_DEVICE_PATH_TO_TEXT > bool "Device path to text protocol" > default y >diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile >index 13a35eae6c..9fb04720d9 100644 >--- a/lib/efi_loader/Makefile >+++ b/lib/efi_loader/Makefile >@@ -86,3 +86,10 @@ obj-$(CONFIG_EFI_ECPT) += efi_conformance.o > > EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE)) > $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE) >+ >+ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y) >+EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_ESL_FILE)) >+ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","") >+$(error .esl cerificate not found. Configure your CONFIG_EFI_CAPSULE_ESL_FILE) >+endif >+endif
Re: [PATCH 1/2] cmd: thordown: Add proper dependency for CMD_THOR_DOWNLOAD
Am 9. Juli 2023 15:09:57 MESZ schrieb Ashok Reddy Soma : >When CONFIG_CMD_USB and CONFIG_USB are disabled some compilation errors >are seen as below. Thanks for your patch. Currently we have no documentation for the thordown command. We should create a man page in /docs/usage/cmd/. Do you have any description of the usage of the command? Best regards Heinrich > >cmd/thordown.o: in function `usb_gadget_initialize': >include/linux/usb/gadget.h:981: undefined reference to `board_usb_init' >cmd/thordown.o: in function `do_thor_down': >cmd/thordown.c:68: undefined reference to `g_dnl_unregister' >cmd/thordown.o: in function `usb_gadget_release': >include/linux/usb/gadget.h:986: undefined reference to `board_usb_cleanup' >cmd/thordown.o: in function `do_thor_down': >cmd/thordown.c:41: undefined reference to `g_dnl_register' >cmd/thordown.c:48: undefined reference to `thor_init' >cmd/thordown.c:56: undefined reference to `thor_handle' >gnu/aarch64/lin/aarch64-linux/bin/aarch64-linux-gnu-ld.bfd: line 4: 8485 >Segmentation fault (core dumped) $CC --sysroot=$LIBC >--no-warn-rwx-segment "$@" >Makefile:1779: recipe for target 'u-boot' failed >make: *** [u-boot] Error 139 >make: *** Deleting file 'u-boot' > >Add dependency of CMD_USB for CONFIG_CMD_THOR_DOWNLOAD to fix the errors. > >Signed-off-by: Ashok Reddy Soma >--- > > cmd/Kconfig | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/cmd/Kconfig b/cmd/Kconfig >index 02e54f1e50..b44df9d67a 100644 >--- a/cmd/Kconfig >+++ b/cmd/Kconfig >@@ -526,6 +526,7 @@ config CMD_SPL_WRITE_SIZE > > config CMD_THOR_DOWNLOAD > bool "thor - TIZEN 'thor' download" >+ depends on CMD_USB > select DFU > help > Implements the 'thor' download protocol. This is a way of
[PATCH v3 11/11] sandbox: capsule: Generate capsule related files through binman
The EFI capsule files can now be generated as part of u-boot build. This is done through binman. Add capsule entry nodes in the u-boot.dtsi for the sandbox architecture for generating the capsules. Remove the corresponding generation of capsules from the capsule update conftest file. The capsules are generated through the config file for the sandbox variant, and through explicit parameters for the sandbox_flattree variant. Also generate the FIT image used for testing the capsule update feature on the sandbox_flattree variant through binman. Remove the now superfluous its file which was used for generating this FIT image. Signed-off-by: Sughosh Ganu --- Changes since V2: * New patch for generating the capsules and capsule input files through binman. arch/sandbox/dts/u-boot.dtsi | 143 ++ test/py/tests/test_efi_capsule/conftest.py| 62 .../tests/test_efi_capsule/uboot_bin_env.its | 36 - 3 files changed, 143 insertions(+), 98 deletions(-) delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi index 60bd004937..292fb86a50 100644 --- a/arch/sandbox/dts/u-boot.dtsi +++ b/arch/sandbox/dts/u-boot.dtsi @@ -13,5 +13,148 @@ capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE); }; #endif + + binman: binman { + multiple-images; + }; +}; + +&binman { + itb { + filename = "/tmp/capsules/uboot_bin_env.itb"; + + fit { + description = "Automatic U-Boot environment update"; + #address-cells = <2>; + + images { + u-boot-bin { + description = "U-Boot binary on SPI Flash"; + data = /incbin/("/tmp/capsules/u-boot.bin.new"); + compression = "none"; + type = "firmware"; + arch = "sandbox"; + load = <0>; + hash-1 { + algo = "sha1"; + }; + }; + u-boot-env { + description = "U-Boot environment on SPI Flash"; + data = /incbin/("/tmp/capsules/u-boot.env.new"); + compression = "none"; + type = "firmware"; + arch = "sandbox"; + load = <0>; + hash-1 { + algo = "sha1"; + }; + }; + }; + }; + }; + +#ifdef CONFIG_EFI_USE_CAPSULE_CFG_FILE + capsule1 { + capsule { + cfg-file = CONFIG_EFI_CAPSULE_CFG_FILE; + }; + }; +#else + capsule2 { + capsule { + image-index = <0x1>; + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8"; + filename = "/tmp/capsules/u-boot.bin.new"; + capsule = "/tmp/capsules/Test01"; + }; + }; + + capsule3 { + capsule { + image-index = <0x2>; + image-type-id = "5A7021F5-FEF2-48B4-AABA-832E777418C0"; + filename = "/tmp/capsules/u-boot.env.new"; + capsule = "/tmp/capsules/Test02"; + }; + }; + + capsule4 { + capsule { + image-index = <0x1>; + image-type-id = "058B7D83-50D5-4C47-A195-60D86AD341C4"; + filename = "/tmp/capsules/u-boot.bin.new"; + capsule = "/tmp/capsules/Test03"; + }; + }; + + capsule5 { + capsule { + image-index = <0x1>; + image-type-id = "3673B45D-6A7C-46F3-9E60-ADABB03F7937"; + filename = "/tmp/capsules/uboot_bin_env.itb"; + capsule = "/tmp/capsules/Test04"; + }; + }; + + capsule6 { + capsule { + image-index = <0x1>; + image-type-id = "058B7D83-50D5-4C47-A195-60D86AD341C4"; + filename = "/tmp/capsules/uboot_bin_env.itb"; + capsule = "/tmp/capsules/Test05"; + }; + }; + +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE + capsule7 { + capsule { + im
[PATCH v3 10/11] sandbox: capsule: Add a config file for generating capsules
Support has been added to the mkeficapsule tool to generate capsules by parsing the capsule parameters through a config file. Add a config file for generating capsules. These capsules will be used for testing the capsule update feature on sandbox platform. Enable generation of capsules through the config file on the sandbox variant. Signed-off-by: Sughosh Ganu --- Changes since V2: * New patch to add the capsule generation config file for sandbox. .azure-pipelines.yml | 1 + .gitlab-ci.yml| 1 + configs/sandbox_defconfig | 2 + test/py/conftest.py | 5 ++ .../test_efi_capsule/sandbox_capsule_cfg.txt | 75 +++ 5 files changed, 84 insertions(+) create mode 100644 test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 75075bbd07..cc196bf98c 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -403,6 +403,7 @@ stages: echo -n "u-boot:New" >/tmp/capsules/u-boot.bin.new; echo -n "u-boot-env:Old" >/tmp/capsules/u-boot.env.old; echo -n "u-boot-env:New" >/tmp/capsules/u-boot.env.new; + cp test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt /tmp/capsules/; if [[ "${TEST_PY_BD}" == "sandbox" ]] || [[ "${TEST_PY_BD}" == "sandbox_flattree" ]]; then openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER.key -out /tmp/capsules/SIGNER.crt -nodes -days 365; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER2.key -out /tmp/capsules/SIGNER2.crt -nodes -days 365; diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 577eebd678..614bf61962 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -42,6 +42,7 @@ stages: - echo -n "u-boot:New" >/tmp/capsules/u-boot.bin.new; - echo -n "u-boot-env:Old" >/tmp/capsules/u-boot.env.old; - echo -n "u-boot-env:New" >/tmp/capsules/u-boot.env.new; +- cp test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt /tmp/capsules/; - if [[ "${TEST_PY_BD}" == "sandbox" ]] || [[ "${TEST_PY_BD}" == "sandbox_flattree" ]]; then openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER.key -out /tmp/capsules/SIGNER.crt -nodes -days 365; openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER2.key -out /tmp/capsules/SIGNER2.crt -nodes -days 365; diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index d8a2386bb0..0f4c59e1a8 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -340,6 +340,8 @@ CONFIG_EFI_CAPSULE_ON_DISK=y CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y CONFIG_EFI_CAPSULE_AUTHENTICATE=y CONFIG_EFI_CAPSULE_ESL_FILE="/tmp/capsules/SIGNER.esl" +CONFIG_EFI_CAPSULE_CFG_FILE="/tmp/capsules/sandbox_capsule_cfg.txt" +CONFIG_EFI_USE_CAPSULE_CFG_FILE=y CONFIG_EFI_SECURE_BOOT=y CONFIG_TEST_FDTDEC=y CONFIG_UNIT_TEST=y diff --git a/test/py/conftest.py b/test/py/conftest.py index 661ed74fae..f32ab1a70c 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -161,6 +161,11 @@ def setup_capsule_build(source_dir, build_dir, board_type, log): ) run_command(name, cmd, source_dir) +capsule_cfg_file = 'test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt' +name = 'cp' +cmd = ( ' cp %s %s' % (capsule_cfg_file, capsule_sig_dir)) +run_command(name, cmd, source_dir) + gen_capsule_payloads(capsule_sig_dir) def run_build(config, source_dir, build_dir, board_type, log): diff --git a/test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt b/test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt new file mode 100644 index 00..4e5065d538 --- /dev/null +++ b/test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt @@ -0,0 +1,75 @@ +{ + image-index: 1 + image-guid: 09D7CF52-0720-4710-91D1-08469B7FE9C8 + payload: /tmp/capsules/u-boot.bin.new + capsule: /tmp/capsules/Test01 +} +{ + image-index: 2 + image-guid: 5A7021F5-FEF2-48B4-AABA-832E777418C0 + payload: /tmp/capsules/u-boot.env.new + capsule: /tmp/capsules/Test02 +} +{ + image-index: 1 + image-guid: 058B7D83-50D5-4C47-A195-60D86AD341C4 + payload: /tmp/capsules/u-boot.bin.new + capsule: /tmp/capsules/Test03 + +} +{ + image-index: 1 + image-guid: 3673B45D-6A7C-46F3-9E60-ADABB03F7937 + payload: /tmp/capsules/uboot_bin_env.itb + capsule: /tmp/capsules/Test04 + +} +{ + image-index: 1 + image-guid: 058B7D83-50D5-4C47-A195-60D86AD341C4 + payload: /tmp/capsules/uboot_bin_env.itb + capsule: /tmp/capsules/Test05 + +} +{ + image-index: 1 + image-guid: 058B7D83-50D5-4C47-A195-60D86AD341C4 + payload: /tmp/capsules/uboot_bin_env.itb + capsule: /tmp/capsules/Test05 +} +{ +
[PATCH v3 09/11] test: capsule: Remove public key embed logic from capsule update test
The embedding of the public key EFI Signature List(ESL) file into the platform's DTB is now done at the time of u-boot build. Remove this logic from the capsule update test' configuration. Include the public key for the sandbox and sandbox_flattree variant as part of the build. Signed-off-by: Sughosh Ganu --- Changes since V2: * New patch for removing the capsule key and ESL generation logic from the capsule test config file. configs/sandbox_defconfig| 1 + configs/sandbox_flattree_defconfig | 1 + test/py/tests/test_efi_capsule/conftest.py | 30 +++- test/py/tests/test_efi_capsule/signature.dts | 10 --- 4 files changed, 6 insertions(+), 36 deletions(-) delete mode 100644 test/py/tests/test_efi_capsule/signature.dts diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 1ec44d5b33..d8a2386bb0 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -339,6 +339,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y CONFIG_EFI_CAPSULE_ON_DISK=y CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y CONFIG_EFI_CAPSULE_AUTHENTICATE=y +CONFIG_EFI_CAPSULE_ESL_FILE="/tmp/capsules/SIGNER.esl" CONFIG_EFI_SECURE_BOOT=y CONFIG_TEST_FDTDEC=y CONFIG_UNIT_TEST=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index e7657d40dc..8d60744771 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -226,6 +226,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y CONFIG_EFI_CAPSULE_ON_DISK=y CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y CONFIG_EFI_CAPSULE_AUTHENTICATE=y +CONFIG_EFI_CAPSULE_ESL_FILE="/tmp/capsules/SIGNER.esl" CONFIG_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y diff --git a/test/py/tests/test_efi_capsule/conftest.py b/test/py/tests/test_efi_capsule/conftest.py index a337e62936..9b0f7e635d 100644 --- a/test/py/tests/test_efi_capsule/conftest.py +++ b/test/py/tests/test_efi_capsule/conftest.py @@ -25,42 +25,20 @@ def efi_capsule_data(request, u_boot_config): image_path = u_boot_config.persistent_data_dir + '/test_efi_capsule.img' try: +capsules_path_dir = '/tmp/capsules/' # Create a target device check_call('dd if=/dev/zero of=./spi.bin bs=1MiB count=16', shell=True) check_call('rm -rf %s' % mnt_point, shell=True) check_call('mkdir -p %s' % data_dir, shell=True) check_call('mkdir -p %s' % install_dir, shell=True) +check_call('cp %s/* %s ' % (capsules_path_dir, data_dir), shell=True) capsule_auth_enabled = u_boot_config.buildconfig.get( 'config_efi_capsule_authenticate') if capsule_auth_enabled: -# Create private key (SIGNER.key) and certificate (SIGNER.crt) -check_call('cd %s; ' - 'openssl req -x509 -sha256 -newkey rsa:2048 ' -'-subj /CN=TEST_SIGNER/ -keyout SIGNER.key ' -'-out SIGNER.crt -nodes -days 365' - % data_dir, shell=True) -check_call('cd %s; %scert-to-efi-sig-list SIGNER.crt SIGNER.esl' - % (data_dir, EFITOOLS_PATH), shell=True) - -# Update dtb adding capsule certificate -check_call('cd %s; ' - 'cp %s/test/py/tests/test_efi_capsule/signature.dts .' - % (data_dir, u_boot_config.source_dir), shell=True) -check_call('cd %s; ' - 'dtc -@ -I dts -O dtb -o signature.dtbo signature.dts; ' - 'fdtoverlay -i %s/arch/sandbox/dts/test.dtb ' -'-o test_sig.dtb signature.dtbo' - % (data_dir, u_boot_config.build_dir), shell=True) - -# Create *malicious* private key (SIGNER2.key) and certificate -# (SIGNER2.crt) -check_call('cd %s; ' - 'openssl req -x509 -sha256 -newkey rsa:2048 ' -'-subj /CN=TEST_SIGNER/ -keyout SIGNER2.key ' -'-out SIGNER2.crt -nodes -days 365' - % data_dir, shell=True) +check_call('cp %s/arch/sandbox/dts/test.dtb %s/test_sig.dtb' % + (u_boot_config.build_dir, data_dir), shell=True) # Create capsule files # two regions: one for u-boot.bin and the other for u-boot.env diff --git a/test/py/tests/test_efi_capsule/signature.dts b/test/py/tests/test_efi_capsule/signature.dts deleted file mode 100644 index 078cfc76c9..00 --- a/test/py/tests/test_efi_capsule/signature.dts +++ /dev/null @@ -1,10 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ - -/dts-v1/; -/plugin/; - -&{/} { - signature { - capsule-key = /incbin/("SIGNER.esl"); - }; -}; -- 2.34.1
[PATCH v3 08/11] test: py: Setup capsule files for testing
Support has being added through earlier commits to build capsules and embed the public key needed for capsule authentication as part of u-boot build. >From the testing point-of-view, this means the input files needed for the above have to be setup before invoking the build. Set this up in the pytest configuration file for testing the capsule update feature. Signed-off-by: Sughosh Ganu --- Changes since V2: * New patch for setting up the capsule files in the pytest setup before initiation of u-boot build. test/py/conftest.py | 87 + 1 file changed, 87 insertions(+) diff --git a/test/py/conftest.py b/test/py/conftest.py index fc9dd3a83f..661ed74fae 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -80,6 +80,89 @@ def pytest_addoption(parser): help='Run sandbox under gdbserver. The argument is the channel '+ 'over which gdbserver should communicate, e.g. localhost:1234') +def setup_capsule_build(source_dir, build_dir, board_type, log): +"""Setup the platform's build for testing capsule updates + +This generates the payload/input files needed for testing the +capsule update functionality, along with the keys for signing +the capsules. An EFI Signature List(ESL) file, which houses the +public key for capsule authentication is generated as +well. + +The ESL file is subsequently embedded into the platform's +dtb during the u-boot build, to be used for capsule +authentication. + +Two sets of keys are generated, namely SIGNER and SIGNER2. +The SIGNER2 key pair is used as a malicious key for testing the +the capsule authentication functionality. + +All the generated files are placed under the /tmp/capsules/ +directory. + +Args: +soruce_dir (str): Directory containing source code +build_dir (str): Directory to build in +board_type (str): board_type parameter (e.g. 'sandbox') +log (Logfile): Log file to use + +Returns: +Nothing. +""" +def run_command(name, cmd, source_dir): +with log.section(name): +if isinstance(cmd, str): +cmd = cmd.split() +runner = log.get_runner(name, None) +runner.run(cmd, cwd=source_dir) +runner.close() +log.status_pass('OK') + +def gen_capsule_payloads(capsule_dir): +fname = '%su-boot.bin.old' % capsule_dir +with open(fname, 'w') as fd: +fd.write('u-boot:Old') + +fname = '%su-boot.bin.new' % capsule_dir +with open(fname, 'w') as fd: +fd.write('u-boot:New') + +fname = '%su-boot.env.old' % capsule_dir +with open(fname, 'w') as fd: +fd.write('u-boot-env:Old') + +fname = '%su-boot.env.new' % capsule_dir +with open(fname, 'w') as fd: +fd.write('u-boot-env:New') + +capsule_sig_dir = '/tmp/capsules/' +sig_name = 'SIGNER' +mkdir_p(capsule_sig_dir) +name = 'openssl' +cmd = ( 'openssl req -x509 -sha256 -newkey rsa:2048 ' +'-subj /CN=TEST_SIGNER/ -keyout %s%s.key ' +'-out %s%s.crt -nodes -days 365' +% (capsule_sig_dir, sig_name, capsule_sig_dir, sig_name) + ) +run_command(name, cmd, source_dir) + +name = 'cert-to-efi-sig-list' +cmd = ( 'cert-to-efi-sig-list %s%s.crt %s%s.esl' +% (capsule_sig_dir, sig_name, capsule_sig_dir, sig_name) + ) +run_command(name, cmd, source_dir) + +sig_name = 'SIGNER2' +name = 'openssl' +cmd = ( 'openssl req -x509 -sha256 -newkey rsa:2048 ' +'-subj /CN=TEST_SIGNER/ -keyout %s%s.key ' +'-out %s%s.crt -nodes -days 365' +% (capsule_sig_dir, sig_name, capsule_sig_dir, sig_name) + ) +run_command(name, cmd, source_dir) + +gen_capsule_payloads(capsule_sig_dir) + def run_build(config, source_dir, build_dir, board_type, log): """run_build: Build U-Boot @@ -90,6 +173,10 @@ def run_build(config, source_dir, build_dir, board_type, log): board_type (str): board_type parameter (e.g. 'sandbox') log (Logfile): Log file to use """ +capsule_boards = ( 'sandbox', 'sandbox64', 'sandbox_flattree' ) +if board_type in capsule_boards: +setup_capsule_build(source_dir, build_dir, board_type, log) + if config.getoption('buildman'): if build_dir != source_dir: dest_args = ['-o', build_dir, '-w'] -- 2.34.1
[PATCH v3 07/11] CI: capsule: Setup the files needed for capsule update testing
Support has being added through earlier commits to build capsules and embed the public key needed for capsule authentication as part of u-boot build. >From the testing point-of-view, this means the input files needed for generating the above have to be setup before invoking the build. Set this up in the CI configuration files for testing the capsule update feature. Signed-off-by: Sughosh Ganu --- Changes since V2: * New patch setting up the capsule files needed for CI run .azure-pipelines.yml | 21 + .gitlab-ci.yml | 19 +++ 2 files changed, 40 insertions(+) diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 96b2ab4d75..75075bbd07 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -398,6 +398,17 @@ stages: wget -O - https://github.com/riscv/opensbi/releases/download/v0.9/opensbi-0.9-rv-bin.tar.xz | tar -C /tmp -xJ; export OPENSBI=/tmp/opensbi-0.9-rv-bin/share/opensbi/lp64/generic/firmware/fw_dynamic.bin; fi + mkdir -p /tmp/capsules/; + echo -n "u-boot:Old" >/tmp/capsules/u-boot.bin.old; + echo -n "u-boot:New" >/tmp/capsules/u-boot.bin.new; + echo -n "u-boot-env:Old" >/tmp/capsules/u-boot.env.old; + echo -n "u-boot-env:New" >/tmp/capsules/u-boot.env.new; + if [[ "${TEST_PY_BD}" == "sandbox" ]] || [[ "${TEST_PY_BD}" == "sandbox_flattree" ]]; then + openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER.key -out /tmp/capsules/SIGNER.crt -nodes -days 365; + openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER2.key -out /tmp/capsules/SIGNER2.crt -nodes -days 365; + cert-to-efi-sig-list /tmp/capsules/SIGNER.crt /tmp/capsules/SIGNER.esl; + fi + # the below corresponds to .gitlab-ci.yml "script" cd ${WORK_DIR} export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD}; @@ -582,6 +593,16 @@ stages: cd ${WORK_DIR} # make environment variables available as tests are running inside a container export BUILDMAN="${BUILDMAN}" + if [[ "${BUILDMAN}" == "sandbox" ]] || [[ "${BUILDMAN}" == "sandbox x86" ]]; then + if [ ! -d "/tmp/capsules/" ]; then + mkdir -p /tmp/capsules/; + openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER.key -out /tmp/capsules/SIGNER.crt -n +odes -days 365; + openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER2.key -out /tmp/capsules/SIGNER2.crt +-nodes -days 365; + cert-to-efi-sig-list /tmp/capsules/SIGNER.crt /tmp/capsules/SIGNER.esl; + fi + fi git config --global --add safe.directory ${WORK_DIR} EOF cat << "EOF" >> build.sh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e6c6ab3586..577eebd678 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -37,6 +37,17 @@ stages: export OPENSBI=/tmp/opensbi-0.9-rv-bin/share/opensbi/lp64/generic/firmware/fw_dynamic.bin; fi +- mkdir -p /tmp/capsules/; +- echo -n "u-boot:Old" >/tmp/capsules/u-boot.bin.old; +- echo -n "u-boot:New" >/tmp/capsules/u-boot.bin.new; +- echo -n "u-boot-env:Old" >/tmp/capsules/u-boot.env.old; +- echo -n "u-boot-env:New" >/tmp/capsules/u-boot.env.new; +- if [[ "${TEST_PY_BD}" == "sandbox" ]] || [[ "${TEST_PY_BD}" == "sandbox_flattree" ]]; then + openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER.key -out /tmp/capsules/SIGNER.crt -nodes -days 365; + openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER2.key -out /tmp/capsules/SIGNER2.crt -nodes -days 365; + cert-to-efi-sig-list /tmp/capsules/SIGNER.crt /tmp/capsules/SIGNER.esl; + fi + after_script: - cp -v /tmp/${TEST_PY_BD}/*.{html,css} . - rm -rf /tmp/uboot-test-hooks /tmp/venv @@ -131,6 +142,14 @@ build all other platforms: stage: world build script: - ret=0; + if [ ! -d "/tmp/capsules/" ]; then +mkdir -p /tmp/capsules/; +openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER.key -out /tmp/capsules/SIGNER.crt -nodes -days + 365; +openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER2.key -out /tmp/capsules/SIGNER2.crt -nodes -da +ys 365; +cert-to-efi-sig-list /tmp/capsules/SIGNER.crt /tmp/capsules/SIGNER.esl; + fi git config --global --add safe.directory "${CI_PROJECT_DIR}"; ./tools/buildman/buildman -o /tmp -PEWM -x arm,powerpc || ret=$?; if [[ $ret -ne 0 ]]; then -- 2.34.1
[PATCH v3 06/11] binman: capsule: Add support for generating capsules
Add support in binman for generating capsules. The capsule parameters can be specified either through a config file or through the capsule binman entry. Signed-off-by: Sughosh Ganu --- Changes since V2: * New patch which generates capsules through binman replacing the earlier make target. tools/binman/btool/mkeficapsule.py | 91 + tools/binman/entries.rst | 27 tools/binman/etype/capsule.py | 102 + 3 files changed, 220 insertions(+) create mode 100644 tools/binman/btool/mkeficapsule.py create mode 100644 tools/binman/etype/capsule.py diff --git a/tools/binman/btool/mkeficapsule.py b/tools/binman/btool/mkeficapsule.py new file mode 100644 index 00..9f656c12cf --- /dev/null +++ b/tools/binman/btool/mkeficapsule.py @@ -0,0 +1,91 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2023 Linaro Limited +# +"""Bintool implementation for mkeficapsule tool + +mkeficapsule is a tool used for generating EFI capsules. + +The following are the command-line options to be provided +to the tool +Usage: mkeficapsule [options] +Options: + -g, --guid guid for image blob type + -i, --index update image index + -I, --instanceupdate hardware instance + -p, --private-key private key file + -c, --certificate signer's certificate file + -m, --monotonic-count monotonic count + -d, --dump_sig dump signature (*.p7) + -A, --fw-accept firmware accept capsule, requires GUID, no image blob + -R, --fw-revert firmware revert capsule, takes no GUID, no image blob + -o, --capoemflag Capsule OEM Flag, an integer between 0x and 0x + -f, --cfg-file config file with capsule parameters + -h, --help print a help message + +""" + +from binman import bintool + +class Bintoolmkeficapsule(bintool.Bintool): +"""Handles the 'mkeficapsule' tool + +This bintool is used for generating the EFI capsules. The +capsule generation parameters can either be specified through +command-line, or through a config file. + +""" +def __init__(self, name): +super().__init__(name, 'mkeficapsule tool for generating capsules') + +def capsule_cfg_file(self, cfg_file): + +args = [ +f'--cfg-file={cfg_file}' +] +self.run_cmd(*args) + +def cmdline_capsule(self, image_index, image_guid, hardware_instance, +payload, output_fname): + +args = [ +f'--index={image_index}', +f'--guid={image_guid}', +f'--instance={hardware_instance}', +payload, +output_fname +] +self.run_cmd(*args) + +def cmdline_auth_capsule(self, image_index, image_guid, hardware_instance, + monotonic_count, priv_key, pub_key, + payload, output_fname): + +args = [ +f'--index={image_index}', +f'--guid={image_guid}', +f'--instance={hardware_instance}', +f'--monotonic-count={monotonic_count}', +f'--private-key={priv_key}', +f'--certificate={pub_key}', +payload, +output_fname +] +self.run_cmd(*args) + +def fetch(self, method): +"""Fetch handler for mkeficapsule + +This builds the tool from source + +Returns: +tuple: +str: Filename of fetched file to copy to a suitable directory +str: Name of temp directory to remove, or None +""" +if method != bintool.FETCH_BUILD: +return None +result = self.build_from_git( +'https://source.denx.de/u-boot/u-boot.git', +'tools', +'tools/mkeficapsule') +return result diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index b71af801fd..9a263e8691 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -283,6 +283,33 @@ entry; similarly for SPL. +.. _etype_capsule: + +Entry: capsule: Entry for generating EFI Capsule files +-- + +This is an entry for generating EFI capsules. + +The parameters needed for generation of the capsules can either be +provided separately, or through a config file. + +Properties / Entry arguments: +- cfg-file: Config file for providing capsule + parameters. +- image-index: Unique number for identifying + corresponding payload image. +- image-type-id: Image GUID which will be used + for identifying the image. +- hardware-instance: Optional number for identifying + unique hardware instance of a device in the system. +- monotomic-count: Count used when signing an image. +- private-key: Path to private key file. +- pub-key-cert: Path to public key certificate file. +- filename: Path to the i
[PATCH v3 05/11] doc: Add documentation to describe capsule config file format
The UEFI capsule can be generated either through command-line parameters, or, by specifying those in a config file. Add documentation to describe the format of the config file. Signed-off-by: Sughosh Ganu --- Changes since V2: None doc/develop/uefi/uefi.rst | 64 +++ 1 file changed, 64 insertions(+) diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index c04e62f3a5..ddf8e20cb0 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -442,6 +442,70 @@ following command can be issued --guid c1b629f1-ce0e-4894-82bf-f0a38387e630 \ optee.bin optee.capsule +Or alternatively, the capsule can be generated through a make target + +.. code-block:: bash + +$ make capsule + +Issuing the above make command requires specifying the capsule +parameters through a config file instead. The Kconfig symbol +CONFIG_EFI_CAPSULE_CFG_FILE is to be used for specifying the path to +the config file. + +The config file describes the parameters that are used for generating +one or more capsules. The parameters for a given capsule file are +specified within curly braces, in the form of "key:value" pairs. All +the parameters that are currently supported by the mkeficapsule tool +can be specified through the config file. + +The following are some example payload parameters specified through +the config file. + +.. code-block:: none + + { + image-guid: 02f4d760-cfd5-43bd-8e2d-a42acb33c660 + hardware-instance: 0 + monotonic-count: 1 + payload: u-boot.bin + image-index: 1 + private-key: /path/to/priv/key + pub-key-cert: /path/to/pub/key + capsule: u-boot.capsule + } + { + image-guid: 4ce292da-1dd8-428d-a1c2-77743ef8b96e + hardware-instance: 0 + payload: u-boot.itb + image-index: 2 + oemflags: 0x8000 + capsule: fit.capsule + } + { + capsule-type: accept + image-guid: 4ce292da-1dd8-428d-a1c2-77743ef8b96e + capsule: accept.capsule + } + { + capsule-type: revert + capsule: revert.capsule + } + +The following are the keys that specify the capsule parameters + +..code-block:: none + +image-guid: Image GUID +image-index: Image index value +private-key: Path to the private key file used for capsule signing +pub-key-cert: Path to the public key crt file used for capsule signing +payload: Path to the capsule payload file +capsule: Path to the output capsule file that is generated +hardware-instance: Hardware Instance value +monotonic-count: Monotonic count value +capsule-type: Specifies capsule type. normal(default), accept or revert +oemflags: 16bit Oemflags value to be used(populated in capsule header) Enabling Capsule Authentication *** -- 2.34.1
[PATCH v3 04/11] tools: mkeficapsule: Add support for parsing capsule params from config file
Add support for specifying the parameters needed for capsule generation through a config file, instead of passing them through command-line. Parameters for more than a single capsule file can be specified, resulting in generation of multiple capsules through a single invocation of the command. This path is to be used for generating capsules through a make target, with the parameters being parsed from the config file. Signed-off-by: Sughosh Ganu --- Changes since V2: * Add a Kconfig boolean symbol CONFIG_EFI_USE_CAPSULE_CFG_FILE which can be used to generate capsules through config file or parameters. tools/Kconfig | 16 ++ tools/Makefile | 1 + tools/eficapsule.h | 110 tools/mkeficapsule.c | 84 + tools/mkeficapsule_parse.c | 345 + 5 files changed, 526 insertions(+), 30 deletions(-) create mode 100644 tools/mkeficapsule_parse.c diff --git a/tools/Kconfig b/tools/Kconfig index 539708f277..9b744aba31 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -98,6 +98,22 @@ config TOOLS_MKEFICAPSULE optionally sign that file. If you want to enable UEFI capsule update feature on your target, you certainly need this. +config EFI_CAPSULE_CFG_FILE + string "Path to the EFI Capsule Config File" + default "" + help + Path to the EFI capsule config file which provides the + parameters needed to build capsule(s). Parameters can be + provided for multiple payloads resulting in corresponding + capsule images being generated. + +config EFI_USE_CAPSULE_CFG_FILE + bool "Use the config file for generating capsules" + help + Boolean option used to specify if the EFI capsules are to + be generated through parameters specified via the config + file or through command line. + menuconfig FSPI_CONF_HEADER bool "FlexSPI Header Configuration" help diff --git a/tools/Makefile b/tools/Makefile index d793cf3bec..ef366f3d61 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -250,6 +250,7 @@ HOSTLDLIBS_mkeficapsule += \ HOSTLDLIBS_mkeficapsule += \ $(shell pkg-config --libs uuid 2> /dev/null || echo "-luuid") hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule +mkeficapsule-objs := mkeficapsule.o mkeficapsule_parse.o # We build some files with extra pedantic flags to try to minimize things # that won't build on some weird host compiler -- though there are lots of diff --git a/tools/eficapsule.h b/tools/eficapsule.h index 072a4b5598..42e66c6d6a 100644 --- a/tools/eficapsule.h +++ b/tools/eficapsule.h @@ -52,6 +52,38 @@ typedef struct { /* flags */ #define CAPSULE_FLAGS_PERSIST_ACROSS_RESET 0x0001 +enum capsule_type { + CAPSULE_NORMAL_BLOB = 0, + CAPSULE_ACCEPT, + CAPSULE_REVERT, +}; + +/** + * struct efi_capsule_params - Capsule parameters + * @image_guid: Guid value of the payload input image + * @image_index: Image index value + * @hardware_instance: Hardware instance to be used for the image + * @monotonic_count: Monotonic count value to be used for signed capsule + * @privkey_file: Path to private key used in capsule signing + * @cert_file: Path to public key certificate used in capsule signing + * @input_file: Path to payload input image + * @capsule_file: Path to the output capsule file + * @oemflags: Oemflags to be populated in the capsule header + * @capsule: Capsule Type, normal or accept or revert + */ +struct efi_capsule_params { + efi_guid_t *image_guid; + unsigned long image_index; + unsigned long hardware_instance; + uint64_t monotonic_count; + char *privkey_file; + char *cert_file; + char *input_file; + char *capsule_file; + unsigned long oemflags; + enum capsule_type capsule; +}; + struct efi_capsule_header { efi_guid_t capsule_guid; uint32_t header_size; @@ -113,4 +145,82 @@ struct efi_firmware_image_authentication { struct win_certificate_uefi_guid auth_info; } __packed; +/** + * capsule_with_cfg_file() - Generate capsule from config file + * @cfg_file: Path to the config file + * + * Parse the capsule parameters from the config file and use the + * parameters for generating one or more capsules. + * + * Return: None + * + */ +void capsule_with_cfg_file(const char *cfg_file); + +/** + * convert_uuid_to_guid() - convert UUID to GUID + * @buf: UUID binary + * + * UUID and GUID have the same data structure, but their binary + * formats are different due to the endianness. See lib/uuid.c. + * Since uuid_parse() can handle only UUID, this function must + * be called to get correct data for GUID when parsing a string. + * + * The correct data will be returned in @buf. + */ +void convert_uuid_to_guid(unsigned char *buf); + +/** + * create_empty_capsule() - Generate an empty capsule + * @path: Path to the empty capsule file to be generated + * @guid: Guid value o
[PATCH v3 02/11] capsule: authenticate: Add capsule public key in platform's dtb
The EFI capsule authentication logic in u-boot expects the public key in the form of an EFI Signature List(ESL) to be provided as part of the platform's dtb. Currently, the embedding of the ESL file into the dtb needs to be done manually. Add a signature node in the u-boot dtsi file and include the public key through the capsule-key property. This file is per architecture, and is currently being added for sandbox and arm architectures. It will have to be added for other architectures which need to enable capsule authentication support. The path to the ESL file is specified through the CONFIG_EFI_CAPSULE_ESL_FILE symbol. Signed-off-by: Sughosh Ganu --- Changes since V2: * Add the public key ESL file through the u-boot.dtsi. * Add the dtsi files for sandbox and arm architectures. * Add a check in the Makefile that the ESL file path is not empty. arch/arm/dts/u-boot.dtsi | 17 + arch/sandbox/dts/u-boot.dtsi | 17 + lib/efi_loader/Kconfig | 11 +++ lib/efi_loader/Makefile | 7 +++ 4 files changed, 52 insertions(+) create mode 100644 arch/arm/dts/u-boot.dtsi create mode 100644 arch/sandbox/dts/u-boot.dtsi diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi new file mode 100644 index 00..60bd004937 --- /dev/null +++ b/arch/arm/dts/u-boot.dtsi @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Devicetree file with miscellaneous nodes that will be included + * at build time into the DTB. Currently being used for including + * capsule related information. + * + */ + +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT +/ { +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE + signature { + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE); + }; +#endif +}; +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */ diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi new file mode 100644 index 00..60bd004937 --- /dev/null +++ b/arch/sandbox/dts/u-boot.dtsi @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Devicetree file with miscellaneous nodes that will be included + * at build time into the DTB. Currently being used for including + * capsule related information. + * + */ + +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT +/ { +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE + signature { + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE); + }; +#endif +}; +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */ diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index c5835e6ef6..1326a1d109 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -234,6 +234,17 @@ config EFI_CAPSULE_MAX Select the max capsule index value used for capsule report variables. This value is used to create CapsuleMax variable. +config EFI_CAPSULE_ESL_FILE + string "Path to the EFI Signature List File" + default "" + depends on EFI_CAPSULE_AUTHENTICATE + help + Provides the absolute path to the EFI Signature List + file which will be embedded in the platform's device + tree and used for capsule authentication at the time + of capsule update. + + config EFI_DEVICE_PATH_TO_TEXT bool "Device path to text protocol" default y diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 13a35eae6c..9fb04720d9 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -86,3 +86,10 @@ obj-$(CONFIG_EFI_ECPT) += efi_conformance.o EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE)) $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE) + +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y) +EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_ESL_FILE)) +ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","") +$(error .esl cerificate not found. Configure your CONFIG_EFI_CAPSULE_ESL_FILE) +endif +endif -- 2.34.1
[PATCH v3 03/11] doc: capsule: Document the new mechanism to embed ESL file into dtb
Update the document to specify how the EFI Signature List(ESL) file can be embedded into the platform's dtb as part of the u-boot build. Signed-off-by: Sughosh Ganu --- Changes since V2: * Highlight the need to use the u-boot.dtsi file for embedding the public key ESL into the DTB. doc/develop/uefi/uefi.rst | 22 +- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index ffe25ca231..c04e62f3a5 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -495,20 +495,16 @@ and used by the steps highlighted below. ... } -You can do step-4 manually with +You can perform step-4 by defining the Kconfig symbol +CONFIG_EFI_CAPSULE_ESL_FILE. Once this has been done, the signature +node can be added to the u-boot.dtsi file. For reference, check the +u-boot.dtsi file for the sandbox architecture. If this node has not +been added to the architecture's u-boot.dtsi file, this needs to be +done. The node has currently been added for the sandbox and arm +architectures' in the u-boot.dtsi file. Once the u-boot.dtsi file has +been added with the signature node, the esl file will automatically +get embedded into the platform's dtb as part of u-boot build. -.. code-block:: console - -$ dtc -@ -I dts -O dtb -o signature.dtbo signature.dts -$ fdtoverlay -i orig.dtb -o new.dtb -v signature.dtbo - -where signature.dts looks like:: - -&{/} { -signature { -capsule-key = /incbin/("CRT.esl"); -}; -}; Executing the boot manager ~~ -- 2.34.1
[PATCH v3 01/11] nuvoton: npcm845-evb: Add a newline at the end of file
Add a newline at the end of the dts, without which the build fails when including the u-boot.dtsi file. Signed-off-by: Sughosh Ganu --- Changes since V2: * New patch arch/arm/dts/nuvoton-npcm845-evb.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/dts/nuvoton-npcm845-evb.dts b/arch/arm/dts/nuvoton-npcm845-evb.dts index 3cab7807e3..a93666cb41 100644 --- a/arch/arm/dts/nuvoton-npcm845-evb.dts +++ b/arch/arm/dts/nuvoton-npcm845-evb.dts @@ -354,4 +354,4 @@ &r1en_pins &r1oen_pins >; -}; \ No newline at end of file +}; -- 2.34.1
[PATCH v3 00/11] Integrate EFI capsule tasks into u-boot's build flow
This patchset aims to bring two capsule related tasks under the u-boot build flow. One is the embedding of the public key into the platform's dtb. The public key is in the form of an EFI Signature List(ESL) file and is used for capsule authentication. This is being achieved by adding the signature node containing the capsule public key in the architecture's u-boot.dtsi file. Currently, the u-boot.dtsi file has been added for the sandbox and arm architectures. The path to the ESL file is being provided through a Kconfig symbol(CONFIG_EFI_CAPSULE_ESL_FILE). Changes have also been made to the test flow so that the keys used for signing the capsule, and the ESL file, are generated prior to invoking the u-boot's build, which enables embedding the ESL file into the dtb as part of the u-boot build. The other task is related to generation of capsules. Support is being added to generate capsules by specifying the capsule parameters in a config file. Calling the mkeficapsule tool then results in generation of the corresponding capsule files. The capsules can be generated as part of u-boot build, and this is being achieved through binman, by adding a capsule entry type. The capsules can be generated either by specifying the capsule parameters in a config file, or through specifying them as properties under the capsule entry node. If using the config file, the path to the config file is to be specified through a Kconfig symbol(CONFIG_EFI_CAPSULE_CFG_FILE). Changes have also been made to the efi capsule update feature testing setup on the sandbox variants. Currently, the capsule files and the public key ESL file are generated after u-boot has been built. This logic has been changed so that the capsule input files along with the keys needed for capsule signing and authentication are generated prior to initiation of the u-boot build. The placement of all the files needed for generation of capsules, along with the generated capsule files is under the /tmp/capsules/ directory. Currently, the capsule update feature is tested on the sandbox and sandbox_flattree variants in CI. The capsule generation through config file is enabled for the sandbox variant, with the sandbox_flattree variant generating capsules through the command-line parameters. The document has been updated to reflect the above changes. Changes since V2: This version embeds the capsule auth related public key through the u-boot.dtsi file. The capsule generation has been moved to binman. The changes in the test setup have been split into multiple patches, instead of a single monolithic patch. * Add the public key ESL file through the u-boot.dtsi * Add the dtsi files for sandbox and arm architectures * Add a check in the Makefile that the ESL file path is not empty. * Highlight the need to use the u-boot.dtsi file for embedding the public key ESL into the DTB. * Add a Kconfig boolean symbol CONFIG_EFI_USE_CAPSULE_CFG_FILE which can be used to generate capsules through config file or parameters. * New patch which generates capsules through binman replacing the earlier make target. * New patch setting up the capsule files needed for CI run * New patch for setting up the capsule files in the pytest setup before initiation of u-boot build. * New patch for removing the capsule key and ESL generation logic from the capsule test config file. * New patch to add the capsule generation config file for sandbox. * New patch for generating the capsules and capsule input files through binman. Sughosh Ganu (11): nuvoton: npcm845-evb: Add a newline at the end of file capsule: authenticate: Add capsule public key in platform's dtb doc: capsule: Document the new mechanism to embed ESL file into dtb tools: mkeficapsule: Add support for parsing capsule params from config file doc: Add documentation to describe capsule config file format binman: capsule: Add support for generating capsules CI: capsule: Setup the files needed for capsule update testing test: py: Setup capsule files for testing test: capsule: Remove public key embed logic from capsule update test sandbox: capsule: Add a config file for generating capsules sandbox: capsule: Generate capsule related files through binman .azure-pipelines.yml | 22 ++ .gitlab-ci.yml| 20 + arch/arm/dts/nuvoton-npcm845-evb.dts | 2 +- arch/arm/dts/u-boot.dtsi | 17 + arch/sandbox/dts/u-boot.dtsi | 160 configs/sandbox_defconfig | 3 + configs/sandbox_flattree_defconfig| 1 + doc/develop/uefi/uefi.rst | 86 - lib/efi_loader/Kconfig| 11 + lib/efi_loader/Makefile | 7 + test/py/conftest.py | 92 + test/py/tests/test_efi_capsule/conftest.py| 92 + .../test_efi_capsule/sandbox_capsule_cfg.txt | 75 test/py/tests/test_efi_
[PATCH 1/2] cmd: thordown: Add proper dependency for CMD_THOR_DOWNLOAD
When CONFIG_CMD_USB and CONFIG_USB are disabled some compilation errors are seen as below. cmd/thordown.o: in function `usb_gadget_initialize': include/linux/usb/gadget.h:981: undefined reference to `board_usb_init' cmd/thordown.o: in function `do_thor_down': cmd/thordown.c:68: undefined reference to `g_dnl_unregister' cmd/thordown.o: in function `usb_gadget_release': include/linux/usb/gadget.h:986: undefined reference to `board_usb_cleanup' cmd/thordown.o: in function `do_thor_down': cmd/thordown.c:41: undefined reference to `g_dnl_register' cmd/thordown.c:48: undefined reference to `thor_init' cmd/thordown.c:56: undefined reference to `thor_handle' gnu/aarch64/lin/aarch64-linux/bin/aarch64-linux-gnu-ld.bfd: line 4: 8485 Segmentation fault (core dumped) $CC --sysroot=$LIBC --no-warn-rwx-segment "$@" Makefile:1779: recipe for target 'u-boot' failed make: *** [u-boot] Error 139 make: *** Deleting file 'u-boot' Add dependency of CMD_USB for CONFIG_CMD_THOR_DOWNLOAD to fix the errors. Signed-off-by: Ashok Reddy Soma --- cmd/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/Kconfig b/cmd/Kconfig index 02e54f1e50..b44df9d67a 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -526,6 +526,7 @@ config CMD_SPL_WRITE_SIZE config CMD_THOR_DOWNLOAD bool "thor - TIZEN 'thor' download" + depends on CMD_USB select DFU help Implements the 'thor' download protocol. This is a way of -- 2.17.1
[PATCH 2/2] zynqmp: config: Add proper dependencies for USB
When CONFIG_CMD_USB and CONFIG_USB are disabled, still some compilation errors are seen as below. In file included from include/configs/xilinx_zynqmp.h:173, from include/config.h:3, from include/common.h:16, from env/common.c:10: include/config_distro_bootcmd.h:302:9: error: expected '}' before 'BOOT_TARGET_DEVICES_references_USB_without_CONFIG_CMD_USB' 302 | BOOT_TARGET_DEVICES_references_USB_without_CONFIG_CMD_USB | ^ include/config_distro_bootcmd.h:302:9: note: in definition of macro 'BOOTENV_DEV_NAME_USB' 302 | BOOT_TARGET_DEVICES_references_USB_without_CONFIG_CMD_USB | ^ include/configs/xilinx_zynqmp.h:77:41: note: in expansion of macro 'BOOTENV_DEV_NAME' 77 | # define BOOT_TARGET_DEVICES_USB(func) func(USB, usb, 0) func(USB, usb, 1) | ^~~~ include/configs/xilinx_zynqmp.h:168:9: note: in expansion of macro 'BOOT_TARGET_DEVICES_USB' 168 | BOOT_TARGET_DEVICES_USB(func) \ | ^~~ include/config_distro_bootcmd.h:454:25: note: in expansion of macro 'BOOT_TARGET_DEVICES' 454 | "boot_targets=" BOOT_TARGET_DEVICES(BOOTENV_DEV_NAME) "\0" | ^~~ include/config_distro_bootcmd.h:474:9: note: in expansion of macro 'BOOTENV_BOOT_TARGETS' 474 | BOOTENV_BOOT_TARGETS \ | ^~~~ include/configs/xilinx_zynqmp.h:179:9: note: in expansion of macro 'BOOTENV' 179 | BOOTENV | ^~~ include/env_default.h:120:9: note: in expansion of macro 'CFG_EXTRA_ENV_SETTINGS' 120 | CFG_EXTRA_ENV_SETTINGS | ^~ In file included from env/common.c:32: include/env_default.h:27:36: note: to match this '{' 27 | const char default_environment[] = { |^ scripts/Makefile.build:256: recipe for target 'env/common.o' failed make[1]: *** [env/common.o] Error 1 Makefile:1853: recipe for target 'env' failed make: *** [env] Error 2 make: *** Waiting for unfinished jobs Add CONFIG_USB_STORAGE as dependency for USB related macro's such as BOOT_TARGET_DEVICES_USB() and DFU_DEFAULT_POLL_TIMEOUT and CONFIG_THOR_RESET_OFF. Remove CONFIG_ZYNQMP_USB from Kconfig and also from defconfig since it is not used anywhere else. Signed-off-by: Ashok Reddy Soma --- arch/arm/mach-zynqmp/Kconfig | 3 --- configs/xilinx_zynqmp_virt_defconfig | 1 - include/configs/xilinx_zynqmp.h | 4 ++-- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig index fd6f07715a..26b80b7882 100644 --- a/arch/arm/mach-zynqmp/Kconfig +++ b/arch/arm/mach-zynqmp/Kconfig @@ -84,9 +84,6 @@ config ZYNQMP_SPL_PM_CFG_OBJ_FILE Leave this option empty if your PMU firmware has a hard-coded configuration object or you are loading it by any other means. -config ZYNQMP_USB - bool "Configure ZynqMP USB" - config ZYNQMP_NO_DDR bool "Disable DDR MMU mapping" help diff --git a/configs/xilinx_zynqmp_virt_defconfig b/configs/xilinx_zynqmp_virt_defconfig index c4bbde2206..6bda4f8453 100644 --- a/configs/xilinx_zynqmp_virt_defconfig +++ b/configs/xilinx_zynqmp_virt_defconfig @@ -17,7 +17,6 @@ CONFIG_ENV_OFFSET_REDUND=0x1E8 CONFIG_SPL_SPI_FLASH_SUPPORT=y CONFIG_SPL_SPI=y CONFIG_CMD_FRU=y -CONFIG_ZYNQMP_USB=y CONFIG_SYS_LOAD_ADDR=0x800 CONFIG_AHCI=y CONFIG_SYS_MEMTEST_START=0x diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h index 011f0034c5..44f8914b80 100644 --- a/include/configs/xilinx_zynqmp.h +++ b/include/configs/xilinx_zynqmp.h @@ -29,7 +29,7 @@ /* Miscellaneous configurable options */ -#if defined(CONFIG_ZYNQMP_USB) +#if defined(CONFIG_USB_STORAGE) #define DFU_DEFAULT_POLL_TIMEOUT 300 # define PARTS_DEFAULT \ @@ -73,7 +73,7 @@ # define BOOT_TARGET_DEVICES_SCSI(func) #endif -#if defined(CONFIG_ZYNQMP_USB) +#if defined(CONFIG_USB_STORAGE) # define BOOT_TARGET_DEVICES_USB(func) func(USB, usb, 0) func(USB, usb, 1) #else # define BOOT_TARGET_DEVICES_USB(func) -- 2.17.1
[PATCH 0/2] Fix dependencies of USB Kconfig options
When USB device driver CONFIG_USB and CONFIG_CMD_USB are disabled, some compilation issues are seen. Also CMD_THOR_DOWNLOAD should depend on CONFIG_CMD_USB. Add dependencies to resolve those issues and compile properly. Also remove unused config CONFIG_ZYNQMP_USB. Ashok Reddy Soma (2): cmd: thordown: Add proper dependency for CMD_THOR_DOWNLOAD zynqmp: config: Add proper dependencies for USB arch/arm/mach-zynqmp/Kconfig | 3 --- cmd/Kconfig | 1 + configs/xilinx_zynqmp_virt_defconfig | 1 - include/configs/xilinx_zynqmp.h | 4 ++-- 4 files changed, 3 insertions(+), 6 deletions(-) -- 2.17.1
Re: [PATCH v3 u-boot] mmc: spl: Make partition choice in default_spl_mmc_emmc_boot_partition() more explicit
On Saturday 08 July 2023 11:28:34 Tom Rini wrote: > On Sat, Jul 08, 2023 at 10:30:47AM +0200, Pali Rohár wrote: > > On Friday 07 July 2023 19:43:34 Tom Rini wrote: > > > On Sat, Jul 08, 2023 at 12:54:38AM +0200, Pali Rohár wrote: > > > > > > > To make eMMC partition choosing in default_spl_mmc_emmc_boot_partition() > > > > function better understandable, rewrite it via explicit switch-case code > > > > pattern. > > > > > > > > Also indicate an error when eMMC EXT_CSD[179] register is configured by > > > > user to value which is not suitable for eMMC booting and SPL do not know > > > > how to interpret it. > > > > > > > > Signed-off-by: Pali Rohár > > > > > > I did some quick local testing to check on the size impact and this is > > > indeed quite manageable, thanks for reworking things! > > > > > > Reviewed-by: Tom Rini > > > > Unfortunately no, sama5d2_xplained_emmc, sama5d2_xplained_mmc and > > sama5d2_xplained_qspiflash are failing with v3; but not with v2. > > > > Seems that these boards have their SPL at limit and adding any new > > puts() increase size over the limit. > > > > So I think that there is no other way than just adding a new config > > option to hide this new puts(). > > We should enable LTO on those platforms then and buy us some space. > > -- > Tom Ok, but then this is really out of what I have a free time to do...
Re: imx8mp: issues getting dtb with pcie enabled to boot
On Fri, Jul 7, 2023 at 8:48 PM Sahaj Sarup wrote: > For now the fix seems to be `clocks = <&hsio_blk_ctrl 0>;` Please submit the fix to Linux. > this gets pcie working under linux but not under u-boot, i'm guessing pcie > for imx8 is not yet implemented? That's correct.
[PATCH v2] x86: Update docs link in bootparam header
After Linux commit ff61f0791ce9, x86 documentation was moved to arch/x86 and the link in bootparam.h was broken. Signed-off-by: Paul Barker --- arch/x86/include/asm/bootparam.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/bootparam.h b/arch/x86/include/asm/bootparam.h index ea816ca74698..ac4865300f1b 100644 --- a/arch/x86/include/asm/bootparam.h +++ b/arch/x86/include/asm/bootparam.h @@ -62,7 +62,7 @@ struct setup_indirect { /** * struct setup_header - Information needed by Linux to boot * - * See https://www.kernel.org/doc/html/latest/x86/boot.html + * See https://www.kernel.org/doc/html/latest/arch/x86/boot.html */ struct setup_header { __u8setup_sects; base-commit: 0beb649053b86b2cfd5cf55a0fc68bc2fe91a430 -- 2.34.1
Re: [PATCH] x86: Update docs link in bootparam header
On 09/07/2023 02:47, Heinrich Schuchardt wrote: On 7/7/23 17:41, Tom Rini wrote: On Fri, Jul 07, 2023 at 07:51:42AM +0100, Paul Barker wrote: After Linux commit ff61f0791ce9, x86 documentation was moved to arch/x86 and the link in bootparam.h was broken. Signed-off-by: Paul Barker --- arch/x86/include/asm/bootparam.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/bootparam.h b/arch/x86/include/asm/bootparam.h index ea816ca74698..1f8ca569b880 100644 --- a/arch/x86/include/asm/bootparam.h +++ b/arch/x86/include/asm/bootparam.h @@ -62,7 +62,7 @@ struct setup_indirect { /** * struct setup_header - Information needed by Linux to boot * - * See https://www.kernel.org/doc/html/latest/x86/boot.html + * See https://docs.kernel.org/arch/x86/boot.html This is also now: https://www.kernel.org/doc/html/latest/arch/x86/boot.html And tree-wide we have two examples of docs.kernel.org and the rest are www.kernel.org/doc/html/latest for the base. We should be consistent here, so I'm deferring to Heinrich. In Linux' /Documentation/ they always uses https://www.kernel.org/doc/html/latest. So let's stick to this. Ok, I'll send a v2. Thanks, Paul
Pull request efi-2023-07-rc7
Dear Tom, The following changes since commit 0beb649053b86b2cfd5cf55a0fc68bc2fe91a430: MAINTAINERS: correct at91 tree link (2023-07-07 11:37:09 -0400) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-efi.git tags/efi-2023-07-rc7 for you to fetch changes up to e05a4b12a056fd7fe22e85715c8f5e5e811fb551: mkeficapsule: fix efi_firmware_management_capsule_header data type (2023-07-09 10:32:28 +0200) Gitlab CI showed no issues: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/16815 Pull request efi-2023-07-rc7 Documentation: * Fix links to Linux kernel documentation UEFI: * Fix memory leak in efidebug dh subcommand * Fix underflow when calculating remaining variable store size * Increase default variable store size to 64 KiB * mkeficapsule: fix efi_firmware_management_capsule_header data type Alper Nebi Yasak (2): efi_loader: Avoid underflow when calculating remaining var store size efi_loader: Increase default variable store size to 64KiB Heinrich Schuchardt (1): doc: harmonize Linux kernel documentation links Malte Schmidt (1): mkeficapsule: fix efi_firmware_management_capsule_header data type Masahisa Kojima (1): cmd: efidebug: add missing efi_free_pool for dh subcommand Paul Barker (1): x86: Update docs link in bootparam header arch/x86/include/asm/bootparam.h | 2 +- cmd/efidebug.c | 1 + doc/develop/docstyle.rst | 2 +- doc/usage/blkmap.rst | 2 +- lib/efi_loader/Kconfig | 5 +++-- lib/efi_loader/efi_var_mem.c | 4 tools/eficapsule.h | 2 +- 7 files changed, 12 insertions(+), 6 deletions(-)
Re: [PATCH v2] x86: Update docs link in bootparam header
On 7/9/23 11:44, Paul Barker wrote: After Linux commit ff61f0791ce9, x86 documentation was moved to arch/x86 and the link in bootparam.h was broken. Signed-off-by: Paul Barker Reviewed-by: Heinrich Schuchardt --- arch/x86/include/asm/bootparam.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/bootparam.h b/arch/x86/include/asm/bootparam.h index ea816ca74698..ac4865300f1b 100644 --- a/arch/x86/include/asm/bootparam.h +++ b/arch/x86/include/asm/bootparam.h @@ -62,7 +62,7 @@ struct setup_indirect { /** * struct setup_header - Information needed by Linux to boot * - * See https://www.kernel.org/doc/html/latest/x86/boot.html + * See https://www.kernel.org/doc/html/latest/arch/x86/boot.html */ struct setup_header { __u8setup_sects; base-commit: 0beb649053b86b2cfd5cf55a0fc68bc2fe91a430
Re: [PATCH] efi_loader: make efi_delete_handle() follow the EFI spec
On 6/26/23 12:04, Ilias Apalodimas wrote: The EFI doesn't allow removal of handles, unless all hosted protocols are cleanly removed. Our efi_delete_handle() is a bit intrusive. Although it does try to delete protocols before removing a handle, it doesn't care if that fails. Instead it only returns an error if the handle is invalid. On top of that none of the callers of that function check the return code. So let's rewrite this in a way that fits the EFI spec better. Instead of forcing the handle removal, gracefully uninstall all the handle protocols. According to the EFI spec when the last protocol is removed the handle will be deleted. Also switch all the callers and check the return code. Some callers can't do anything useful apart from reporting an error. The disk related functions on the other hand, can prevent a medium that is being used by EFI from removal. The only function that doesn't check the result is efi_delete_image(). But that function needs a bigger rework anyway, so we can clean it up in the future Signed-off-by: Ilias Apalodimas --- Heinrich this needs to be applied on top of https://lore.kernel.org/u-boot/20230620061932.113292-1-ilias.apalodi...@linaro.org/ cmd/bootefi.c | 19 --- include/efi_loader.h | 2 +- lib/efi_driver/efi_uclass.c | 13 ++--- lib/efi_loader/efi_boottime.c | 26 ++ lib/efi_loader/efi_disk.c | 17 + 5 files changed, 42 insertions(+), 35 deletions(-) diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 8aa15a64c836..60b9e950ffdc 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -606,20 +606,6 @@ failure: return ret; } -/** - * bootefi_run_finish() - finish up after running an EFI test - * - * @loaded_image_info: Pointer to a struct which holds the loaded image info - * @image_obj: Pointer to a struct which holds the loaded image object - */ -static void bootefi_run_finish(struct efi_loaded_image_obj *image_obj, - struct efi_loaded_image *loaded_image_info) -{ - efi_restore_gd(); - free(loaded_image_info->load_options); - efi_delete_handle(&image_obj->header); -} - /** * do_efi_selftest() - execute EFI selftest * @@ -638,7 +624,10 @@ static int do_efi_selftest(void) /* Execute the test */ ret = EFI_CALL(efi_selftest(&image_obj->header, &systab)); - bootefi_run_finish(image_obj, loaded_image_info); + efi_restore_gd(); + free(loaded_image_info->load_options); + if (efi_delete_handle(&image_obj->header) != EFI_SUCCESS) + log_err("Failed to delete loaded image handle\n"); return ret != EFI_SUCCESS; } diff --git a/include/efi_loader.h b/include/efi_loader.h index e530bcf15f76..43ccf49abec4 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -618,7 +618,7 @@ void efi_add_handle(efi_handle_t obj); /* Create handle */ efi_status_t efi_create_handle(efi_handle_t *handle); /* Delete handle */ -void efi_delete_handle(efi_handle_t obj); +efi_status_t efi_delete_handle(efi_handle_t obj); /* Call this to validate a handle and find the EFI object for it */ struct efi_object *efi_search_obj(const efi_handle_t handle); /* Locate device_path handle */ diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c index 45f935198874..d8755541fc14 100644 --- a/lib/efi_driver/efi_uclass.c +++ b/lib/efi_driver/efi_uclass.c @@ -285,10 +285,8 @@ static efi_status_t efi_add_driver(struct driver *drv) bp->ops = ops; ret = efi_create_handle(&bp->bp.driver_binding_handle); - if (ret != EFI_SUCCESS) { - free(bp); - goto out; - } + if (ret != EFI_SUCCESS) + goto err; bp->bp.image_handle = bp->bp.driver_binding_handle; ret = efi_add_protocol(bp->bp.driver_binding_handle, &efi_guid_driver_binding_protocol, bp); @@ -299,11 +297,12 @@ static efi_status_t efi_add_driver(struct driver *drv) if (ret != EFI_SUCCESS) goto err; } -out: - return ret; + return ret; err: - efi_delete_handle(bp->bp.driver_binding_handle); + if (bp->bp.driver_binding_handle && + efi_delete_handle(bp->bp.driver_binding_handle) != EFI_SUCCESS) + log_err("Failed to delete handle\n"); You already log errors in efi_delete_handle(), please, avoid duplicate log messages increasing code size (applies to several locations in this patch). Best regards Heinrich free(bp); return ret; } diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index d75a3336e3f1..5123055d7f5d 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -59,6 +59,10 @@ static efi_handle_t current_image; static volatile gd_t *efi_gd, *app_gd; #endif +static efi_status_t efi_uninstall_protocol +
Re: [PATCH 2/4 v2] efi_loader: check the status of disconnected drivers
On 6/20/23 08:19, Ilias Apalodimas wrote: efi_uninstall_protocol() calls efi_disconnect_all_drivers() but never checks the return value. Instead it tries to identify protocols that are still open after closing the ones that were opened with EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL, EFI_OPEN_PROTOCOL_GET_PROTOCOL and EFI_OPEN_PROTOCOL_TEST_PROTOCOL. Instead of doing that, check the return value early and exit if disconnecting the drivers failed. Also reconnect all the drivers of a handle if protocols are still found on the handle after disconnecting controllers and closing the remaining protocols. While at it fix a memory leak and properly free the opened protocol information when closing a protocol. Signed-off-by: Ilias Apalodimas Reviewed-by: Heinrich Schuchardt
Re: [PATCH 1/4 v2] efi_loader: reconnect drivers on failure
On 6/20/23 08:19, Ilias Apalodimas wrote: efi_disconnect_controller() doesn't reconnect drivers in case of failure. Reconnect the disconnected drivers properly Signed-off-by: Ilias Apalodimas Reviewed-by: Heinrich Schuchardt
Re: [PATCH v10 3/4] Boot var automatic management for removable medias
On 6/19/23 23:23, Raymond Mao wrote: Changes for complying to EFI spec §3.5.1.1 'Removable Media Boot Behavior'. Boot variables can be automatically generated during a removable media is probed. At the same time, unused boot variables will be detected and removed. Please note that currently the function 'efi_disk_remove' has no ability to distinguish below two scenarios a) Unplugging of a removable media under U-Boot b) U-Boot exiting and booting an OS Thus currently the boot variables management is not added into 'efi_disk_remove' to avoid boot options being added/erased repeatedly under scenario b) during power cycles See TODO comments under function 'efi_disk_remove' for more details Signed-off-by: Raymond Mao --- Changes in v3 - Split the patch into moving and renaming functions and individual patches for each changed functionality Changes in v5 - Move function call of efi_bootmgr_update_media_device_boot_option() from efi_init_variables() to efi_init_obj_list() Changes in v6 - Revert unrelated changes Changes in v7 - adapt the return code of function efi_bootmgr_update_media_device_boot_option() Changes in v8 - add a note in the commit message for future reference Changes in v9 - amend the note text in the commit message - add a TODO comment in 'efi_disk_remove' Changes in v10 - fix typo and build failures with 'CONFIG_CMD_BOOTEFI_BOOTMGR=n' lib/efi_loader/efi_disk.c | 18 ++ lib/efi_loader/efi_setup.c | 7 +++ 2 files changed, 25 insertions(+) diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index d2256713a8..911a4adfb1 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -687,6 +687,13 @@ int efi_disk_probe(void *ctx, struct event *event) return -1; } + /* only do the boot option management when UEFI sub-system is initialized */ + if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) && efi_obj_list_initialized == EFI_SUCCESS) { + ret = efi_bootmgr_update_media_device_boot_option(); + if (ret != EFI_SUCCESS) + return -1; + } + return 0; } @@ -773,6 +780,17 @@ int efi_disk_remove(void *ctx, struct event *event) return efi_disk_delete_part(dev); else return 0; + + /* +* TODO A flag to distinguish below 2 different scenarios of this +* function call is needed: +* a) Unplugging of a removable media under U-Boot +* b) U-Boot exiting and booting an OS +* In case of scenario a), efi_bootmgr_update_media_device_boot_option() +* needs to be invoked here to update the boot options and remove the +* unnecessary ones. +*/ As in future we want to integrate more EFI drivers with the driver model such a status should be in a global variable. It will have to be set in ExitBootServices() before invoking dm_remove_devices_flags(). Reviewed-by: Heinrich Schuchardt + } /** diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index 58d4e13402..69c8b27730 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -245,6 +245,13 @@ efi_status_t efi_init_obj_list(void) if (ret != EFI_SUCCESS) goto out; + if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) { + /* update boot option after variable service initialized */ + ret = efi_bootmgr_update_media_device_boot_option(); + if (ret != EFI_SUCCESS) + goto out; + } + /* Define supported languages */ ret = efi_init_platform_lang(); if (ret != EFI_SUCCESS)
Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type
On 6/16/23 10:28, Stefan Herbrechtsmeier wrote: From: Malte Schmidt The data type of item_offset_list shall be UINT64 according to the UEFI [1] specifications. In include/efi_api.h the correct data type is used. The bug was probably never noticed because of little endianness. [1] https://uefi.org/specs/UEFI/2.10/index.html Signed-off-by: Malte Schmidt Signed-off-by: Stefan Herbrechtsmeier --- tools/eficapsule.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/eficapsule.h b/tools/eficapsule.h index 753fb73313..2099a2e9b8 100644 --- a/tools/eficapsule.h +++ b/tools/eficapsule.h @@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header { uint32_t version; uint16_t embedded_driver_count; uint16_t payload_item_count; - uint32_t item_offset_list[]; + uint64_t item_offset_list[]; Defining the same structure in two places it bad practice. https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/11 Reviewed-by: Heinrich Schuchardt } __packed; /* image_capsule_support */