hi Simon,
On Sun, 8 Oct 2023 at 04:41, Simon Glass sjg@chromium.org wrote:
Hi Sugosh,
On Wed, 4 Oct 2023 at 05:27, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add support in binman for generating EFI empty capsules. These capsules are used in the FWU A/B update feature. Also add test cases in binman for the corresponding code coverage.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
tools/binman/etype/efi_empty_capsule.py | 91 +++++++++++++++++++ tools/binman/ftest.py | 52 +++++++++++ tools/binman/test/319_capsule_accept.dts | 16 ++++ tools/binman/test/320_capsule_revert.dts | 14 +++ .../test/321_capsule_accept_missing_guid.dts | 14 +++ .../binman/test/322_capsule_accept_revert.dts | 17 ++++ 6 files changed, 204 insertions(+) create mode 100644 tools/binman/etype/efi_empty_capsule.py create mode 100644 tools/binman/test/319_capsule_accept.dts create mode 100644 tools/binman/test/320_capsule_revert.dts create mode 100644 tools/binman/test/321_capsule_accept_missing_guid.dts create mode 100644 tools/binman/test/322_capsule_accept_revert.dts
diff --git a/tools/binman/etype/efi_empty_capsule.py b/tools/binman/etype/efi_empty_capsule.py new file mode 100644 index 0000000000..d2c781627b --- /dev/null +++ b/tools/binman/etype/efi_empty_capsule.py @@ -0,0 +1,91 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2023 Linaro Limited +# +# Entry-type module for producing an empty EFI capsule +#
+import os
+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_efi_empty_capsule(Entry_section):
Do you think this could subclass Entry_efi_capsule? They seem to do similar things. You could call generate_capsule() or generate_empty_capsule(). depending on whether any data is present (and perhaps require an operation if no data).
I'm not sure about this, just an idea.
Okay, let me try this out. If it is getting too confusing or unclear, I will keep the two entry types separate.
- """Generate EFI empty capsules
- The parameters needed for generation of the empty capsules can
- be provided as properties in the entry.
- Properties / Entry arguments:
- image-guid: Image GUID which will be used for identifying the
updatable image on the board. Mandatory for accept capsule.
- accept-capsule - Boolean property to generate an accept capsule.
image-type-id
- revert-capsule - Boolean property to generate a revert capsule
- For more details on the description of the capsule format, and the capsule
- update functionality, refer Section 8.5 and Chapter 23 in the `UEFI
- specification`_. For more information on the empty capsule, refer the
- sections 2.3.2 and 2.3.3 in the `Dependable Boot specification`_.
- A typical accept empty capsule entry node would then look something like this
- empty-capsule {
type = "efi-empty-capsule";/* Image GUID for testing capsule update */image-type-id = SANDBOX_UBOOT_IMAGE_GUID;accept-capsule;- };
- A typical revert empty capsule entry node would then look something like this
- empty-capsule {
type = "efi-empty-capsule";revert-capsule;- };
- The empty capsules do not have any input payload image.
- .. _`UEFI specification`: https://uefi.org/sites/default/files/resources/UEFI_Spec_2_10_Aug29.pdf
- .. _`Dependable Boot specification`: https://git.codelinaro.org/linaro/dependable-boot/mbfw/uploads/6f7ddfe3be24e...
- """
- def __init__(self, section, etype, node):
super().__init__(section, etype, node)self.accept = 0self.revert = 0- def ReadNode(self):
super().ReadNode()self.image_guid = fdt_util.GetString(self._node, 'image-guid')self.accept = fdt_util.GetBool(self._node, 'accept-capsule')self.revert = fdt_util.GetBool(self._node, 'revert-capsule')Perhaps it should be
operation = "accept" / "revert" ?
Using two conflicting bools is not great.
Will change
if self.accept and not self.image_guid:self.Raise('Image GUID needed for generating accept capsule')if self.accept and self.revert:self.Raise('Need to enable either Accept or Revert capsule')- def BuildSectionData(self, required):
def get_binman_test_guid(type_str):TYPE_TO_GUID = {'binman-test' : '09d7cf52-0720-4710-91d1-08469b7fe9c8'Can you put this in a shared file somewhere, used by this and the efi_capsule.py module?
Okay. I will put this in a separate global function, and use it in the other module.
}return TYPE_TO_GUID[type_str]uniq = self.GetUniqueName()outfile = self._filename if self._filename else 'capsule.%s' % uniqcapsule_fname = tools.get_output_filename(outfile)guid = self.image_guidif self.image_guid == "binman-test":guid = get_binman_test_guid('binman-test')ret = self.mkeficapsule.generate_empty_capsule(self.accept, self.revert,guid, capsule_fname)if ret is not None:return tools.read_file(capsule_fname)- def AddBintools(self, btools):
self.mkeficapsule = self.AddBintool(btools, 'mkeficapsule')diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 2b8871ab7e..9286de28e4 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -126,6 +126,9 @@ FW_MGMT_GUID = '6DCBD5ED-E82D-4C44-BDA1-7194199AD92A' CAPSULE_IMAGE_GUID = '09D7CF52-0720-4710-91D1-08469B7FE9C8' # Windows cert GUID WIN_CERT_TYPE_EFI_GUID = '4AAFD29D-68DF-49EE-8AA9-347D375665A7' +# Empty capsule GUIDs +EMPTY_CAPSULE_ACCEPT_GUID = '0C996046-BCC0-4D04-85EC-E1FCEDF1C6F8' +EMPTY_CAPSULE_REVERT_GUID = 'ACD58B4B-C0E8-475F-99B5-6B3F7E07AAF0'
class TestFunctional(unittest.TestCase): """Functional tests for binman @@ -7283,6 +7286,27 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
self.assertEqual(payload_data_len, int(hdr['Payload Image Size']))
- def _CheckEmptyCapsule(self, data, accept_capsule=False):
if accept_capsule:capsule_hdr_guid = EMPTY_CAPSULE_ACCEPT_GUIDelse:capsule_hdr_guid = EMPTY_CAPSULE_REVERT_GUIDhdr = self._GetCapsuleHeaders(data)self.assertEqual(capsule_hdr_guid,hdr['EFI_CAPSULE_HDR.CAPSULE_GUID'])if accept_capsule:capsule_size = "0000002C"else:capsule_size = "0000001C"self.assertEqual(capsule_size,hdr['EFI_CAPSULE_HDR.CAPSULE_IMAGE_SIZE'])if accept_capsule:self.assertEqual(CAPSULE_IMAGE_GUID, hdr['ACCEPT_IMAGE_GUID'])- def testCapsuleGen(self): """Test generation of EFI capsule""" data = self._DoReadFile('311_capsule.dts')
@@ -7347,5 +7371,33 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertIn("entry is missing properties: image-guid", str(e.exception))
- def testCapsuleGenAcceptCapsule(self):
"""Test generationg of accept EFI capsule"""data = self._DoReadFile('319_capsule_accept.dts')self._CheckEmptyCapsule(data, accept_capsule=True)- def testCapsuleGenRevertCapsule(self):
"""Test generationg of revert EFI capsule"""data = self._DoReadFile('320_capsule_revert.dts')self._CheckEmptyCapsule(data)- def testCapsuleGenAcceptGuidMissing(self):
"""Test that binman errors out on missing image GUID for accept capsule"""with self.assertRaises(ValueError) as e:self._DoReadFile('321_capsule_accept_missing_guid.dts')self.assertIn("Image GUID needed for generating accept capsule",str(e.exception))- def testCapsuleGenAcceptOrRevert(self):
"""Test that both accept and revert capsule are not specified"""with self.assertRaises(ValueError) as e:self._DoReadFile('322_capsule_accept_revert.dts')self.assertIn("Need to enable either Accept or Revert capsule",str(e.exception))if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/319_capsule_accept.dts b/tools/binman/test/319_capsule_accept.dts new file mode 100644 index 0000000000..4d6c005019 --- /dev/null +++ b/tools/binman/test/319_capsule_accept.dts @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+/ {
#address-cells = <1>;#size-cells = <1>;binman {efi-empty-capsule {/* Image GUID for testing capsule update */image-guid = "binman-test";accept-capsule;};};+}; diff --git a/tools/binman/test/320_capsule_revert.dts b/tools/binman/test/320_capsule_revert.dts new file mode 100644 index 0000000000..eeaa2793a5 --- /dev/null +++ b/tools/binman/test/320_capsule_revert.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+/ {
#address-cells = <1>;#size-cells = <1>;binman {efi-empty-capsule {revert-capsule;};};+}; diff --git a/tools/binman/test/321_capsule_accept_missing_guid.dts b/tools/binman/test/321_capsule_accept_missing_guid.dts new file mode 100644 index 0000000000..6f7062e83e --- /dev/null +++ b/tools/binman/test/321_capsule_accept_missing_guid.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+/ {
#address-cells = <1>;#size-cells = <1>;I wonder if those two lines are needed? Perhaps it generates a warning?
Tbh, I just copied them from another file which I was using as a reference when I introduced the capsule changes. I will check if removing them causes any issues.
-sughosh
binman {efi-empty-capsule {accept-capsule;};};+}; diff --git a/tools/binman/test/322_capsule_accept_revert.dts b/tools/binman/test/322_capsule_accept_revert.dts new file mode 100644 index 0000000000..c68e76a669 --- /dev/null +++ b/tools/binman/test/322_capsule_accept_revert.dts @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+/ {
#address-cells = <1>;#size-cells = <1>;binman {efi-empty-capsule {/* Image GUID for testing capsule update */image-guid = "binman-test";accept-capsule;revert-capsule;};};+};
2.34.1
Regards, Simon