[PATCH v8 0/9] Enable EFI capsule generation through binman

This patch series adds support for generation of EFI capsules as part of u-boot build flow. 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 parameters needed for capsule generation are specified as properties under the capsule entry node.
Changes have also been made to the efi capsule update feature testing setup on the sandbox variants. Currently, the capsule files and the keys for testing capsule authentication are generated after u-boot has been built. As part of this patch series, the private and public keys along with the EFI Signature List(ESL) needed for testing the capsule update functionality on the sandbox plaform are placed in the board directory. The test logic has been changed so that the capsules which were generated as part of the test setup are now being generated as part of the build for sandbox platform. The document has been updated to reflect the above changes.
Changes since V7:
This version has dropped the changes for embedding the public key ESL into the DTB as there are discussions currently in progress on the solution. The capsule generation changes OTOH are close to getting merged. Hence the separation of the patches.
The following are changes per individual patches
* Change the file names to highlight good and bad keys as suggested by Simon Glass. * Rebase on top of current upstream. * Drop the ReadEntries method as suggested by Simon Glass. * Add logic to allow specifying a string 'binman-test' for GUIDs in binman tests. * Add a todo comment for getting the capsule contents from the tool. * Move the capsule generation logic to sandbox_capsule.dtsi and include that explicitly in test.dts and sandbox.dts. * Drop the u-boot.dtsi file which kept the capsule and signature nodes. * Remove capsule generation logic from capsule update test setup. * Keep the logic to embed the public key in DTB in the test setup. * Change the name of the file which contains the capsule entry binman nodes.
Sughosh Ganu (9): binman: bintool: Build a tool from a list of commands nuvoton: npcm845-evb: Add a newline at the end of file sandbox: capsule: Add keys and certificates needed for capsule update testing sandbox: Build the mkeficapsule tool for the sandbox variants btool: mkeficapsule: Add a bintool for EFI capsule generation binman: capsule: Add support for generating EFI capsules sandbox: capsule: Generate capsule related files through binman doc: Add documentation to highlight capsule generation related updates sandbox: trace: Increase trace buffer size
.azure-pipelines.yml | 2 +- .gitlab-ci.yml | 2 +- arch/arm/dts/nuvoton-npcm845-evb.dts | 2 +- arch/sandbox/dts/sandbox.dts | 4 + arch/sandbox/dts/sandbox_capsule.dtsi | 340 ++++++++++++++++++ arch/sandbox/dts/test.dts | 4 + board/sandbox/capsule_priv_key_bad.key | 28 ++ board/sandbox/capsule_priv_key_good.key | 28 ++ board/sandbox/capsule_pub_esl_good.esl | Bin 0 -> 831 bytes board/sandbox/capsule_pub_key_bad.crt | 19 + board/sandbox/capsule_pub_key_good.crt | 19 + doc/develop/uefi/uefi.rst | 16 + include/sandbox_efi_capsule.h | 21 ++ test/py/tests/test_efi_capsule/conftest.py | 155 +------- .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- test/py/tests/test_trace.py | 2 +- tools/Kconfig | 6 +- tools/binman/bintool.py | 19 +- tools/binman/btool/mkeficapsule.py | 101 ++++++ tools/binman/entries.rst | 64 ++++ tools/binman/etype/efi_capsule.py | 143 ++++++++ tools/binman/ftest.py | 118 ++++++ tools/binman/test/311_capsule.dts | 21 ++ tools/binman/test/312_capsule_signed.dts | 23 ++ tools/binman/test/313_capsule_version.dts | 22 ++ tools/binman/test/314_capsule_signed_ver.dts | 24 ++ tools/binman/test/315_capsule_oemflags.dts | 22 ++ tools/binman/test/316_capsule_missing_key.dts | 22 ++ .../binman/test/317_capsule_missing_index.dts | 20 ++ .../binman/test/318_capsule_missing_guid.dts | 19 + 30 files changed, 1112 insertions(+), 190 deletions(-) create mode 100644 arch/sandbox/dts/sandbox_capsule.dtsi create mode 100644 board/sandbox/capsule_priv_key_bad.key create mode 100644 board/sandbox/capsule_priv_key_good.key create mode 100644 board/sandbox/capsule_pub_esl_good.esl create mode 100644 board/sandbox/capsule_pub_key_bad.crt create mode 100644 board/sandbox/capsule_pub_key_good.crt create mode 100644 include/sandbox_efi_capsule.h delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its create mode 100644 tools/binman/btool/mkeficapsule.py create mode 100644 tools/binman/etype/efi_capsule.py create mode 100644 tools/binman/test/311_capsule.dts create mode 100644 tools/binman/test/312_capsule_signed.dts create mode 100644 tools/binman/test/313_capsule_version.dts create mode 100644 tools/binman/test/314_capsule_signed_ver.dts create mode 100644 tools/binman/test/315_capsule_oemflags.dts create mode 100644 tools/binman/test/316_capsule_missing_key.dts create mode 100644 tools/binman/test/317_capsule_missing_index.dts create mode 100644 tools/binman/test/318_capsule_missing_guid.dts

Add support to build a tool from source with a list of commands. This is useful when a tool can be built with multiple commands instead of a single command.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Simon Glass sjg@chromium.org --- Changes since V7: None
tools/binman/bintool.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index 0b0f56dbbb..3c4ad1adbb 100644 --- a/tools/binman/bintool.py +++ b/tools/binman/bintool.py @@ -328,7 +328,7 @@ class Bintool: return result.stdout
@classmethod - def build_from_git(cls, git_repo, make_target, bintool_path, flags=None): + def build_from_git(cls, git_repo, make_targets, bintool_path, flags=None): """Build a bintool from a git repo
This clones the repo in a temporary directory, builds it with 'make', @@ -336,7 +336,8 @@ class Bintool:
Args: git_repo (str): URL of git repo - make_target (str): Target to pass to 'make' to build the tool + make_targets (list of str): List of targets to pass to 'make' to build + the tool bintool_path (str): Relative path of the tool in the repo, after build is complete flags (list of str): Flags or variables to pass to make, or None @@ -350,12 +351,14 @@ class Bintool: tmpdir = tempfile.mkdtemp(prefix='binmanf.') print(f"- clone git repo '{git_repo}' to '{tmpdir}'") tools.run('git', 'clone', '--depth', '1', git_repo, tmpdir) - print(f"- build target '{make_target}'") - cmd = ['make', '-C', tmpdir, '-j', f'{multiprocessing.cpu_count()}', - make_target] - if flags: - cmd += flags - tools.run(*cmd) + for target in make_targets: + print(f"- build target '{target}'") + cmd = ['make', '-C', tmpdir, '-j', f'{multiprocessing.cpu_count()}', + target] + if flags: + cmd += flags + tools.run(*cmd) + fname = os.path.join(tmpdir, bintool_path) if not os.path.exists(fname): print(f"- File '{fname}' was not produced")

Add a newline at the end of the dts, without which the build fails when including a dtsi file.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- Changes since V7: * Rephrase the commit message
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 +};

Add the private keys and public key certificates which are to be used for capsule authentication while testing the EFI capsule update functionality. There are two pairs of private and public keys, good and bad. The good key pair will be used for signing capsules, whilst the bad key pair is to be used as malicious keys for testing authentication failure cases. The capsule_pub_key_good.crt is also converted to an EFI Signature List(ESL) file, SIGNER.esl, which is embedded in the platform's device-tree for capsule authentication.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- Changes since V7: * Change the file names to highlight good and bad keys as suggested by Simon Glass.
board/sandbox/capsule_priv_key_bad.key | 28 ++++++++++++++++++++++++ board/sandbox/capsule_priv_key_good.key | 28 ++++++++++++++++++++++++ board/sandbox/capsule_pub_esl_good.esl | Bin 0 -> 831 bytes board/sandbox/capsule_pub_key_bad.crt | 19 ++++++++++++++++ board/sandbox/capsule_pub_key_good.crt | 19 ++++++++++++++++ 5 files changed, 94 insertions(+) create mode 100644 board/sandbox/capsule_priv_key_bad.key create mode 100644 board/sandbox/capsule_priv_key_good.key create mode 100644 board/sandbox/capsule_pub_esl_good.esl create mode 100644 board/sandbox/capsule_pub_key_bad.crt create mode 100644 board/sandbox/capsule_pub_key_good.crt
diff --git a/board/sandbox/capsule_priv_key_bad.key b/board/sandbox/capsule_priv_key_bad.key new file mode 100644 index 0000000000..2324f69ebd --- /dev/null +++ b/board/sandbox/capsule_priv_key_bad.key @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCmPw1pGd2xNW0p +lesRXkkek3uwUB06Nt61tnZvpMkBKt4IokqGWz1tZls+Z2CqvwOfcsPZ27cPRYSu +xRnM3YdL4MG6SePV7i/YSNw3rq8CP8zLGtCbNIfsfsNfPQEtPBpw6+7pMJKhjqpV +2U2UQzZEiX4qlnhLpyv2JNJag27yf0feLdJi7HnJ9xdvcXpA1DSGm4y+DDhgYeI8 +DEteEu6s0TYQfnOZSQOeJi+1/Qz0S594uFJB37MyGh/mB15ILb8gva4nA3ayHOBK +0dd+HSiUCGYrLYO7aj+nfzQj9N1qTlzCnC1603bMczU5pkwcODg6xP0Sn11J6RYy +y0c0qzJLAgMBAAECggEABDY2MLoew3IkBltrParAWAUUcFLi95jw92q6BkOHEJg8 +2qia1yCitPUtPodMLmOKF5x4EdgXg5sv2O8MGbWP1VtUKXGh3QJcnRnNmsZ1hXJC +RBcrei2aVLsqf0V2Mg3+GuG8PW3vLWHyZ/Sd6afeuXEYm2Bzrw9J5rfd3dBVKm7f +HBvIyy1ATO/2cbUaEaCLOyhxLhssTI2TIK5SjlsjFLxiQXEi6RyGfBxUCriKZykS +krMdvYh7Tf0uYcv0STmQ5s5Rd+RhRIGCVAdsNBxxJjgBAgqqa/B+kWbcc6o2D41n +yWjErUaBBx3t0A7oT4K4DSTYwMNDVY3fhdd+szsocQKBgQDjnm8LG4UO6OQDm6iX +0vTQTItoAz5TU6GEjHTCfVEqiupD4LKfHhSXwp2hRyzxXO5oNTU9MQCzYd7Npes0 +oVk4Tjo3YDacNPgxqKjODu/Q+tkTH15ydzGr674+YXHfCA1uT5GKOiiF0H1FZgMa +Dk0s+3uWX34vbL4QCu97bUhBewKBgQC6+Z0J9sClgWvvjkglJN3XhRnAacp+WgX7 +bkpgSboXIIsqeqhd1WCLeV7L1pcZgifYBMPojf5LTBqBedL1q3RuqiqQWD/bSIYN +Oc9KCdTjksS8Zo+w+s5zDObDhW9y13H2mKwDqilYBrT4fiA62wPMf1SjEF+RSC6K +ZrQzHO1xcQKBgAILsXnLFIYOx8XUh05eAf9BQNt9c/jxvnjffkklMS6Nsw9LHK/b +aFn40MvbROcia64aFFFpeFUkYwk8HYIKlS+xXEqVHciHnVds6Z94eOVK69qFJKco +tRSTeNE8tPZJLz23j1pLrYOOXSHbidmZGU53MCQo1Yx9kLO6NW7Ji6WzAoGBALP4 +lEoE80Xbn3NEdvkZ1VcfzLvCmKCqMlvjuz+Xd8HPF2VaDznSq01VFAQMmAB7obJy +U8hC9OSxakn6Yy8JS9dBgBrUdxKxaibM4FQZxosOuMPHzMPDhniDkJPemnnmGtIL +/nbAkW8jdYpCjO9Z5PwwC92xYuvKmNGrLgSM8ZhhAoGAfgSZTpASXubM18E3ecfw +5z333wf9qEQgZj7i9MzByFZudyHUhv/FPW1ocUJf36Wu1dfofZg3noSL6oakrm2v +dFDo4PoyCStuF0w9SSzpIld01ZG0t7XqphY0DmshCXIXsqr7Vb4WrbBI7KX+b3Um +BzmROfaSud97NjQ/RA26OZk= +-----END PRIVATE KEY----- diff --git a/board/sandbox/capsule_priv_key_good.key b/board/sandbox/capsule_priv_key_good.key new file mode 100644 index 0000000000..9a37f59796 --- /dev/null +++ b/board/sandbox/capsule_priv_key_good.key @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCwBfaV0P1jRzS6 +13U1T+4VbuMVsxFXhwHJY5z5Fx6v+cWBf3K1ruK+7cEnW55ZHXvNE2JCkjMvISKm +hI/DLJWIPnAus8tFdU/R2u5oJbKI+b6GbuamO/CG9HsXZ58lOC6r2ckjixxovsA9 +SFshccdIv2YrwiVsWeyFpH+rB3/+cFbrgdWpaUc1367GkU/ZCnSRDBvVvzRRI1a4 +y2NogFqbZHXHENpzWNJ3TTXhf9dwM5HFGkmX7SA43Dtazae6CB4EaUKzLYWj3+ae +AQbdvBrupKZQz1PUKn7X6+BGaLujHthvibYppNegPvqbJ1xBbv59CQK+lRULwC05 +NYw5+sIxAgMBAAECggEAHn8h/knjpMAw/BAZP//VrYP1Nwy7u/Dpl9U43JUrXWzG +Uc3dd2nR4id6GBIRCLqJePnbQ9JlqMwyXyxHZhbC34SF1imTVbjh9+dY99VULdQr +NMphDrsCzLbt3pu24HFv8Jk+dniDFwi5cMSo+U3nq4xxrLIp3rBjwLHD5sNZYyEU +9xZnj7ziTn5X8da8iRxNpyzz2kQeVemJ0ahr/IkX718bkakSFMesGkln06vH7rAs +069SeqOPrFEbWYXI5iMktLugl3JZpzasRE48j0M42PuProgvT7jb8B35ZF7kn0jT +MqTIHglsJRWcSY0fAb2lHSAvd2vLLVunxr9PDWZvGQKBgQDVzVTuvo1CrVrQLy+B +tpy2k5mjR3qxAOcoWTnKcMErLe8imWWaxukODenP4XqQIX4Sl+X3BXxOqun0Klap +FEsI7TWSHf0eULFtFj0SCgqfRR+V/nblP05eO2nFXgr5YdNa1bWf/aMHplBo4q9e +bbAr4InUB7IGWL2cWjhOhWuJbQKBgQDSw81cBM+vGPUYH/wlxlTVgZCo2Dg2NHjt +LUBqvOZNr21j2F+w8t1vKmqwhkqpc5HIi3pHjEA5gZLTRtmf4GQyo973I6MGn4bS +eayOd6/+FkAi9DUD+WaF7yctJqeevav6KF2UCiz78OtCAU5Y9jFFJpuOANIztI7m +t7ZCUpMFVQKBgFnAsP7oj3SGQbFTnaXeeztKCx04TJExx9hwXIpXe0AdMF5d9wFa +r0tvG9Bg34rSBJLZoXhpnR2JMl2FyIuCMV219t84J6IqTdF1nH2OKZdi9TeKc28Z +fFSirGxmZkT6hDeFr5FScLYtY2QkhWomseY5hKK1+E4hwrd4SFruN46hAoGBAJgh +nzTBgEtqH1enlrCJhSiLmihV0dVGcNb559pjuXTvoG0GfKPT2gPowRPkCzZe5ia0 +jrHgSWd44MtCA8nEBW8MG9+VyJH6Si3Yh7ZaLB2iX+8bCL1yow8f/c44bZtGW0F5 +K3q1EZ1VW+rL2IqcQhog8P1CGHgb514f0x3yTo71AoGACGdb+Nb6lg8OSJPUcuuH +xsWk6RhkJl9bldTleS+QT3R9zO3FvbTwnCCYJboh5Cq/jVmiA7T+fcVAyEJNHSdm +hxbHdScuiJdNWL9+FczOkylnKH3VEdG3RS5lGdyi6r+miTMs3h8WfzGp4JINysjg +PUFskK36qGjASfkRUn0hizQ= +-----END PRIVATE KEY----- diff --git a/board/sandbox/capsule_pub_esl_good.esl b/board/sandbox/capsule_pub_esl_good.esl new file mode 100644 index 0000000000000000000000000000000000000000..f8cc272309b2f80113c29e22bc9fdd5c767b4667 GIT binary patch literal 831 zcmZ1&d0^?2Da*aux2_hA(f&|m&&&V@%1|1@gOCPI%=`vTjNcb9GchtTi3D3+YdNud z!N;6d=3f<&F-6ONS4$i4vT<s)d9;1!Wn|=LWiSvk6fqEDV-96u=HU);4GxJ9_H_4i z4Kh#==QT1murM$&v@kF?F^ZDlH#RUZHZ(9Wg9_l(*~F-XYzZSP19KB2KLgNRTue=j zj0_uCzfHaHH`(1}*Y#3U|97H!k3}~NhPN}GOrG;oTyFi(qmA`NTh~3>_x7NA^t?#f z>a)U0PLquF6_u8?^dHul+F@6qxB0YdssF`W?=n<3b^P4dmiKI#^@p}E)#B;%RW0;Z z-#n?@Et9eDfUQTgV&QR*{b|~VRC6NVv@WS%&0hbnAnbMH)s>m<ruWw!o9KU&t7IaN z^ws?)fy!Y!PA6wHM9oeqJuYyoIO0;duj#}3>jlOWk4kw?f2&||$2#il@?9KqESXN5 zbz2wTe>RVi?d~3_cT1K9oDaUDRd@aM1GkLbi{)<QcW%>Ma(#i_ui5G`j(PuTIhpoN z73DslYiZhJ`RkA&6Eh<N<Kke0Km%D|xXbdfh_Q&secT;;BI`VRLX*$F#X3)yTwnIl z%Rn9^t;`}}Al86g0Y6BAFeBrC7FGjhAcY+4z_<klJ0n9-ZF-^Byx1)aGPkZ;M8@iK z@4h6QYApWke51U4vh|)B+RHfwcQh8f`Fd8-Ad#bU?mv#SCl^~!ojUbKMpn)-Kd>zH z-bULy5-~dZsg>zZPS-w(zNM;c<#N4ar|5@t2FY2AoF7{4IWYI(=HR-Vl;VtSQGM$z zG&LhNEwesN5|ez&@#Le<mt%k4Ngp{`oU3!I!!G(r+O78=mGAf;yBNLt_LaVmLHB2w zF3Vp(*($;GmPMkjzjWQf=x-~Qozl?NwEilS|Lo7%xGP$f^Riek&1Q@!w>qJ9NJDsL xX})Fc$L0Fj-&QP|CD!3Bu=aCF<a-t|<@nrhQ$HQy*tL7JV9T^r#)tp!0|2#rO&9<G
literal 0 HcmV?d00001
diff --git a/board/sandbox/capsule_pub_key_bad.crt b/board/sandbox/capsule_pub_key_bad.crt new file mode 100644 index 0000000000..2e8e5d5828 --- /dev/null +++ b/board/sandbox/capsule_pub_key_bad.crt @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDDzCCAfegAwIBAgIUWw3vHYnrjoHUXytxSm2eYWzbYVAwDQYJKoZIhvcNAQEL +BQAwFjEUMBIGA1UEAwwLVEVTVF9TSUdORVIwIBcNMjMwODA0MTgwODEyWhgPMzAw +MzEwMDYxODA4MTJaMBYxFDASBgNVBAMMC1RFU1RfU0lHTkVSMIIBIjANBgkqhkiG +9w0BAQEFAAOCAQ8AMIIBCgKCAQEApj8NaRndsTVtKZXrEV5JHpN7sFAdOjbetbZ2 +b6TJASreCKJKhls9bWZbPmdgqr8Dn3LD2du3D0WErsUZzN2HS+DBuknj1e4v2Ejc +N66vAj/MyxrQmzSH7H7DXz0BLTwacOvu6TCSoY6qVdlNlEM2RIl+KpZ4S6cr9iTS +WoNu8n9H3i3SYux5yfcXb3F6QNQ0hpuMvgw4YGHiPAxLXhLurNE2EH5zmUkDniYv +tf0M9EufeLhSQd+zMhof5gdeSC2/IL2uJwN2shzgStHXfh0olAhmKy2Du2o/p380 +I/Tdak5cwpwtetN2zHM1OaZMHDg4OsT9Ep9dSekWMstHNKsySwIDAQABo1MwUTAd +BgNVHQ4EFgQUm9b8SnF811nweXSfGisfpzUHGwgwHwYDVR0jBBgwFoAUm9b8SnF8 +11nweXSfGisfpzUHGwgwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOC +AQEAaOZFOcQzF1MRekcBmIZMaHSWYxOUrVLzBNSNhFD8muYiUAAufrkyTUq0Mmat +w5hAnJ34VGpU1wxQlr/uwH7wpZZnGuj10rAp3tqES0g24AeH1bC9wmRs+rD6dcZR +YmZq6FxtV7Cv3pQX7lhDYbcBj2za3YT6I1+yczskAHR6KYYuJzKJ7XRVCL7ZlYRX +pUMZBQq2eAVWlW/c5iDT3KoGZUD9Of71F7qyUAqMMYafeDxguDz7gKstoXVCklQ+ +I4C7JKmRbrRvMgXx6O1clGhAsRZ0nNAtzi7XT5tD27qFwIPgwv48RWgsmPtzE03S +YGQ5WhYMdHOOjWmcV6MDkCpiSA== +-----END CERTIFICATE----- diff --git a/board/sandbox/capsule_pub_key_good.crt b/board/sandbox/capsule_pub_key_good.crt new file mode 100644 index 0000000000..82d8576a64 --- /dev/null +++ b/board/sandbox/capsule_pub_key_good.crt @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDDzCCAfegAwIBAgIUUzrWhMi7oPFshQP6eFlccqf7exswDQYJKoZIhvcNAQEL +BQAwFjEUMBIGA1UEAwwLVEVTVF9TSUdORVIwIBcNMjMwODA0MTgwNzQyWhgPMzAw +MzEwMDYxODA3NDJaMBYxFDASBgNVBAMMC1RFU1RfU0lHTkVSMIIBIjANBgkqhkiG +9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsAX2ldD9Y0c0utd1NU/uFW7jFbMRV4cByWOc ++Rcer/nFgX9yta7ivu3BJ1ueWR17zRNiQpIzLyEipoSPwyyViD5wLrPLRXVP0dru +aCWyiPm+hm7mpjvwhvR7F2efJTguq9nJI4scaL7APUhbIXHHSL9mK8IlbFnshaR/ +qwd//nBW64HVqWlHNd+uxpFP2Qp0kQwb1b80USNWuMtjaIBam2R1xxDac1jSd001 +4X/XcDORxRpJl+0gONw7Ws2nuggeBGlCsy2Fo9/mngEG3bwa7qSmUM9T1Cp+1+vg +Rmi7ox7Yb4m2KaTXoD76mydcQW7+fQkCvpUVC8AtOTWMOfrCMQIDAQABo1MwUTAd +BgNVHQ4EFgQUHvG7Xchqzwdggky+oyzlpNem8UowHwYDVR0jBBgwFoAUHvG7Xchq +zwdggky+oyzlpNem8UowDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOC +AQEAUn1ncSqeXbQAHNrVOFldLwu70hNlMxf2z4EfH2M7vJgrpwkRuIFw7PXNITBh +CImd/ghm5NGFysrK7BwdHkFvUXZV3rE93BhcLC9leWfky33kW9olIzpE14i5FfBn +ABmaokPhOrzAneGzU35sZHNotlqOrzgpKVkpOWrykhYZ5Qjk8Sz0xvzuG8TJc20s +2og+W8Rm2u/xI9xPxtFbq9vUjvFS35o1pm+vkzpgNdo4YS1PG37BW/aopsooLSk7 +9Rxv5vzNXtQqeZ5qBdKbAVh3OsgqwigTmXVvOX3xpy9r9qiimhaISxCt83RZ7wQW +I19t9pXyxAi6u7MRhJZlAeH/3w== +-----END CERTIFICATE-----

Build the mkeficapsule tool for all the sandbox variants. This tool will be used subsequently for testing capsule generation in binman.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- Changes since V7: None
tools/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/Kconfig b/tools/Kconfig index 6e23f44d55..353a855243 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -91,10 +91,10 @@ config TOOLS_SHA512 Enable SHA512 support in the tools builds
config TOOLS_MKEFICAPSULE - bool "Build efimkcapsule command" - default y if EFI_CAPSULE_ON_DISK + bool "Build mkeficapsule tool" + default y if EFI_CAPSULE_ON_DISK || SANDBOX help - This command allows users to create a UEFI capsule file and, + This tool allows users to create a UEFI capsule file and, optionally sign that file. If you want to enable UEFI capsule update feature on your target, you certainly need this.

On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote:
Build the mkeficapsule tool for all the sandbox variants. This tool will be used subsequently for testing capsule generation in binman.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7: None
tools/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/Kconfig b/tools/Kconfig index 6e23f44d55..353a855243 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -91,10 +91,10 @@ config TOOLS_SHA512 Enable SHA512 support in the tools builds
config TOOLS_MKEFICAPSULE
- bool "Build efimkcapsule command"
- default y if EFI_CAPSULE_ON_DISK
- bool "Build mkeficapsule tool"
- default y if EFI_CAPSULE_ON_DISK || SANDBOX help
This command allows users to create a UEFI capsule file and,
optionally sign that file. If you want to enable UEFI capsule update feature on your target, you certainly need this.This tool allows users to create a UEFI capsule file and,
Sorry, what is this fixing exactly?

On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote:
Build the mkeficapsule tool for all the sandbox variants. This tool will be used subsequently for testing capsule generation in binman.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7: None
tools/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/Kconfig b/tools/Kconfig index 6e23f44d55..353a855243 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -91,10 +91,10 @@ config TOOLS_SHA512 Enable SHA512 support in the tools builds
config TOOLS_MKEFICAPSULE
bool "Build efimkcapsule command"
default y if EFI_CAPSULE_ON_DISK
bool "Build mkeficapsule tool"
default y if EFI_CAPSULE_ON_DISK || SANDBOX help
This command allows users to create a UEFI capsule file and,
This tool allows users to create a UEFI capsule file and, optionally sign that file. If you want to enable UEFI capsule update feature on your target, you certainly need this.
Sorry, what is this fixing exactly?
The tool is required to be supported on the sandbox_spl variant, since that is used for the binman tests in CI. Simon had then asked me to add support for the tool on all sandbox variants. I missed putting his R-b on this patch.
-sughosh

On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote:
On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote:
Build the mkeficapsule tool for all the sandbox variants. This tool will be used subsequently for testing capsule generation in binman.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7: None
tools/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/Kconfig b/tools/Kconfig index 6e23f44d55..353a855243 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -91,10 +91,10 @@ config TOOLS_SHA512 Enable SHA512 support in the tools builds
config TOOLS_MKEFICAPSULE
bool "Build efimkcapsule command"
default y if EFI_CAPSULE_ON_DISK
bool "Build mkeficapsule tool"
default y if EFI_CAPSULE_ON_DISK || SANDBOX help
This command allows users to create a UEFI capsule file and,
This tool allows users to create a UEFI capsule file and, optionally sign that file. If you want to enable UEFI capsule update feature on your target, you certainly need this.
Sorry, what is this fixing exactly?
The tool is required to be supported on the sandbox_spl variant, since that is used for the binman tests in CI. Simon had then asked me to add support for the tool on all sandbox variants. I missed putting his R-b on this patch.
OK, moving forward just depend on: https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri... instead please, thanks.

On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote:
On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote:
Build the mkeficapsule tool for all the sandbox variants. This tool will be used subsequently for testing capsule generation in binman.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7: None
tools/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/Kconfig b/tools/Kconfig index 6e23f44d55..353a855243 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -91,10 +91,10 @@ config TOOLS_SHA512 Enable SHA512 support in the tools builds
config TOOLS_MKEFICAPSULE
bool "Build efimkcapsule command"
default y if EFI_CAPSULE_ON_DISK
bool "Build mkeficapsule tool"
default y if EFI_CAPSULE_ON_DISK || SANDBOX help
This command allows users to create a UEFI capsule file and,
This tool allows users to create a UEFI capsule file and, optionally sign that file. If you want to enable UEFI capsule update feature on your target, you certainly need this.
Sorry, what is this fixing exactly?
The tool is required to be supported on the sandbox_spl variant, since that is used for the binman tests in CI. Simon had then asked me to add support for the tool on all sandbox variants. I missed putting his R-b on this patch.
OK, moving forward just depend on: https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri... instead please, thanks.
I will base my changes on top of your patch. However, we would still need this patch as part of the series, since Simon wants the capsules to be generated for all the sandbox variants. Thanks.
-sughosh

On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote:
On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote:
On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote:
Build the mkeficapsule tool for all the sandbox variants. This tool will be used subsequently for testing capsule generation in binman.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7: None
tools/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/Kconfig b/tools/Kconfig index 6e23f44d55..353a855243 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -91,10 +91,10 @@ config TOOLS_SHA512 Enable SHA512 support in the tools builds
config TOOLS_MKEFICAPSULE
bool "Build efimkcapsule command"
default y if EFI_CAPSULE_ON_DISK
bool "Build mkeficapsule tool"
default y if EFI_CAPSULE_ON_DISK || SANDBOX help
This command allows users to create a UEFI capsule file and,
This tool allows users to create a UEFI capsule file and, optionally sign that file. If you want to enable UEFI capsule update feature on your target, you certainly need this.
Sorry, what is this fixing exactly?
The tool is required to be supported on the sandbox_spl variant, since that is used for the binman tests in CI. Simon had then asked me to add support for the tool on all sandbox variants. I missed putting his R-b on this patch.
OK, moving forward just depend on: https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri... instead please, thanks.
I will base my changes on top of your patch. However, we would still need this patch as part of the series, since Simon wants the capsules to be generated for all the sandbox variants. Thanks.
No, this isn't needed. Any sandbox variant that needs capsules has EFI_CAPSULE_ON_DISK enabled.

On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote:
On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote:
On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote:
On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote:
Build the mkeficapsule tool for all the sandbox variants. This tool will be used subsequently for testing capsule generation in binman.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7: None
tools/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/Kconfig b/tools/Kconfig index 6e23f44d55..353a855243 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -91,10 +91,10 @@ config TOOLS_SHA512 Enable SHA512 support in the tools builds
config TOOLS_MKEFICAPSULE
bool "Build efimkcapsule command"
default y if EFI_CAPSULE_ON_DISK
bool "Build mkeficapsule tool"
default y if EFI_CAPSULE_ON_DISK || SANDBOX help
This command allows users to create a UEFI capsule file and,
This tool allows users to create a UEFI capsule file and, optionally sign that file. If you want to enable UEFI capsule update feature on your target, you certainly need this.
Sorry, what is this fixing exactly?
The tool is required to be supported on the sandbox_spl variant, since that is used for the binman tests in CI. Simon had then asked me to add support for the tool on all sandbox variants. I missed putting his R-b on this patch.
OK, moving forward just depend on: https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri... instead please, thanks.
I will base my changes on top of your patch. However, we would still need this patch as part of the series, since Simon wants the capsules to be generated for all the sandbox variants. Thanks.
No, this isn't needed. Any sandbox variant that needs capsules has EFI_CAPSULE_ON_DISK enabled.
Simon wants the capsules to be generated on all sandbox variants, including those that do not have the EFI_CAPSULE_ON_DISK enabled. Which is why we need to have the tool enabled for all sandbox variants.
-sughosh

Hi Sughosh,
On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org wrote:
On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote:
On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote:
On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote:
On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote:
> Build the mkeficapsule tool for all the sandbox variants. This tool > will be used subsequently for testing capsule generation in binman. > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > --- > Changes since V7: None > > tools/Kconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tools/Kconfig b/tools/Kconfig > index 6e23f44d55..353a855243 100644 > --- a/tools/Kconfig > +++ b/tools/Kconfig > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > Enable SHA512 support in the tools builds > > config TOOLS_MKEFICAPSULE > - bool "Build efimkcapsule command" > - default y if EFI_CAPSULE_ON_DISK > + bool "Build mkeficapsule tool" > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > help > - This command allows users to create a UEFI capsule file and, > + This tool allows users to create a UEFI capsule file and, > optionally sign that file. If you want to enable UEFI capsule > update feature on your target, you certainly need this.
Sorry, what is this fixing exactly?
The tool is required to be supported on the sandbox_spl variant, since that is used for the binman tests in CI. Simon had then asked me to add support for the tool on all sandbox variants. I missed putting his R-b on this patch.
OK, moving forward just depend on: https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri... instead please, thanks.
I will base my changes on top of your patch. However, we would still need this patch as part of the series, since Simon wants the capsules to be generated for all the sandbox variants. Thanks.
No, this isn't needed. Any sandbox variant that needs capsules has EFI_CAPSULE_ON_DISK enabled.
Simon wants the capsules to be generated on all sandbox variants, including those that do not have the EFI_CAPSULE_ON_DISK enabled. Which is why we need to have the tool enabled for all sandbox variants.
I want to avoid #ifdefs in the sandbox .dts so far as possible.
Tom, I'll let you make the final decision.
In any case, the multiple-images thing needs to be fixed.
Regards, Simon

hi Simon,
On Fri, 11 Aug 2023 at 19:56, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org wrote:
On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote:
On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote:
On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote:
On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com wrote: > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote: > > > Build the mkeficapsule tool for all the sandbox variants. This tool > > will be used subsequently for testing capsule generation in binman. > > > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > > --- > > Changes since V7: None > > > > tools/Kconfig | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > index 6e23f44d55..353a855243 100644 > > --- a/tools/Kconfig > > +++ b/tools/Kconfig > > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > > Enable SHA512 support in the tools builds > > > > config TOOLS_MKEFICAPSULE > > - bool "Build efimkcapsule command" > > - default y if EFI_CAPSULE_ON_DISK > > + bool "Build mkeficapsule tool" > > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > > help > > - This command allows users to create a UEFI capsule file and, > > + This tool allows users to create a UEFI capsule file and, > > optionally sign that file. If you want to enable UEFI capsule > > update feature on your target, you certainly need this. > > Sorry, what is this fixing exactly?
The tool is required to be supported on the sandbox_spl variant, since that is used for the binman tests in CI. Simon had then asked me to add support for the tool on all sandbox variants. I missed putting his R-b on this patch.
OK, moving forward just depend on: https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri... instead please, thanks.
I will base my changes on top of your patch. However, we would still need this patch as part of the series, since Simon wants the capsules to be generated for all the sandbox variants. Thanks.
No, this isn't needed. Any sandbox variant that needs capsules has EFI_CAPSULE_ON_DISK enabled.
Simon wants the capsules to be generated on all sandbox variants, including those that do not have the EFI_CAPSULE_ON_DISK enabled. Which is why we need to have the tool enabled for all sandbox variants.
I want to avoid #ifdefs in the sandbox .dts so far as possible.
We cannot avoid the ifdef I we are to drop this patch. Without this patch, the build fails on all sandbox variants which do not have EFI_CAPSULE_ON_DISK enabled if we don't keep the ifdef. So it is we either keep this patch, or we have to keep the ifdef.
Tom, I'll let you make the final decision.
In any case, the multiple-images thing needs to be fixed.
Okay
-sughosh

On Fri, Aug 11, 2023 at 08:26:36AM -0600, Simon Glass wrote:
Hi Sughosh,
On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org wrote:
On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote:
On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote:
On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote:
On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com wrote: > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote: > > > Build the mkeficapsule tool for all the sandbox variants. This tool > > will be used subsequently for testing capsule generation in binman. > > > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > > --- > > Changes since V7: None > > > > tools/Kconfig | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > index 6e23f44d55..353a855243 100644 > > --- a/tools/Kconfig > > +++ b/tools/Kconfig > > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > > Enable SHA512 support in the tools builds > > > > config TOOLS_MKEFICAPSULE > > - bool "Build efimkcapsule command" > > - default y if EFI_CAPSULE_ON_DISK > > + bool "Build mkeficapsule tool" > > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > > help > > - This command allows users to create a UEFI capsule file and, > > + This tool allows users to create a UEFI capsule file and, > > optionally sign that file. If you want to enable UEFI capsule > > update feature on your target, you certainly need this. > > Sorry, what is this fixing exactly?
The tool is required to be supported on the sandbox_spl variant, since that is used for the binman tests in CI. Simon had then asked me to add support for the tool on all sandbox variants. I missed putting his R-b on this patch.
OK, moving forward just depend on: https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri... instead please, thanks.
I will base my changes on top of your patch. However, we would still need this patch as part of the series, since Simon wants the capsules to be generated for all the sandbox variants. Thanks.
No, this isn't needed. Any sandbox variant that needs capsules has EFI_CAPSULE_ON_DISK enabled.
Simon wants the capsules to be generated on all sandbox variants, including those that do not have the EFI_CAPSULE_ON_DISK enabled. Which is why we need to have the tool enabled for all sandbox variants.
I want to avoid #ifdefs in the sandbox .dts so far as possible.
Tom, I'll let you make the final decision.
In any case, the multiple-images thing needs to be fixed.
Sughosh, please update the other sandbox defconfigs to just enable EFI_CAPSULE_ON_DISK.
Simon, this I think is an example of where re-working configs/sandbox64_defconfig configs/sandbox_defconfig configs/sandbox_flattree_defconfig configs/sandbox_noinst_defconfig configs/sandbox_spl_defconfig configs/sandbox_vpl_defconfig
To be configs/sandbox_defconfig + boards/sandbox/flattree.config, noinst.config, spl.config, vpl.config would be helpful. There's the sandbox config itself where EFI_CAPSULE_ON_DISK=y and then every other variant just gets that, and we don't have to tweak N configs.

Hi Tom,
On Fri, 11 Aug 2023 at 09:56, Tom Rini trini@konsulko.com wrote:
On Fri, Aug 11, 2023 at 08:26:36AM -0600, Simon Glass wrote:
Hi Sughosh,
On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org
wrote:
On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote:
On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote:
On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote: > On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com
wrote:
> > > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu
wrote:
> > > > > Build the mkeficapsule tool for all the sandbox variants.
This tool
> > > will be used subsequently for testing capsule generation
in binman.
> > > > > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > > > --- > > > Changes since V7: None > > > > > > tools/Kconfig | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > > index 6e23f44d55..353a855243 100644 > > > --- a/tools/Kconfig > > > +++ b/tools/Kconfig > > > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > > > Enable SHA512 support in the tools builds > > > > > > config TOOLS_MKEFICAPSULE > > > - bool "Build efimkcapsule command" > > > - default y if EFI_CAPSULE_ON_DISK > > > + bool "Build mkeficapsule tool" > > > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > > > help > > > - This command allows users to create a UEFI
capsule file and,
> > > + This tool allows users to create a UEFI capsule
file and,
> > > optionally sign that file. If you want to enable
UEFI capsule
> > > update feature on your target, you certainly need
this.
> > > > Sorry, what is this fixing exactly? > > The tool is required to be supported on the sandbox_spl
variant, since
> that is used for the binman tests in CI. Simon had then asked
me to
> add support for the tool on all sandbox variants. I missed
putting his
> R-b on this patch.
OK, moving forward just depend on:
https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri...
instead please, thanks.
I will base my changes on top of your patch. However, we would
still
need this patch as part of the series, since Simon wants the
capsules
to be generated for all the sandbox variants. Thanks.
No, this isn't needed. Any sandbox variant that needs capsules has EFI_CAPSULE_ON_DISK enabled.
Simon wants the capsules to be generated on all sandbox variants, including those that do not have the EFI_CAPSULE_ON_DISK enabled. Which is why we need to have the tool enabled for all sandbox variants.
I want to avoid #ifdefs in the sandbox .dts so far as possible.
Tom, I'll let you make the final decision.
In any case, the multiple-images thing needs to be fixed.
Sughosh, please update the other sandbox defconfigs to just enable EFI_CAPSULE_ON_DISK.
Simon, this I think is an example of where re-working configs/sandbox64_defconfig configs/sandbox_defconfig configs/sandbox_flattree_defconfig configs/sandbox_noinst_defconfig configs/sandbox_spl_defconfig configs/sandbox_vpl_defconfig
To be configs/sandbox_defconfig + boards/sandbox/flattree.config, noinst.config, spl.config, vpl.config would be helpful. There's the sandbox config itself where EFI_CAPSULE_ON_DISK=y and then every other variant just gets that, and we don't have to tweak N configs.
You mean split configs? So far I am unable to build those...
Regards, Simon

On Sat, Aug 12, 2023 at 07:08:44AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 11 Aug 2023 at 09:56, Tom Rini trini@konsulko.com wrote:
On Fri, Aug 11, 2023 at 08:26:36AM -0600, Simon Glass wrote:
Hi Sughosh,
On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org
wrote:
On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote:
On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote:
On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote: > > On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote: > > On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com
wrote:
> > > > > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu
wrote:
> > > > > > > Build the mkeficapsule tool for all the sandbox variants.
This tool
> > > > will be used subsequently for testing capsule generation
in binman.
> > > > > > > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > > > > --- > > > > Changes since V7: None > > > > > > > > tools/Kconfig | 6 +++--- > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > > > index 6e23f44d55..353a855243 100644 > > > > --- a/tools/Kconfig > > > > +++ b/tools/Kconfig > > > > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > > > > Enable SHA512 support in the tools builds > > > > > > > > config TOOLS_MKEFICAPSULE > > > > - bool "Build efimkcapsule command" > > > > - default y if EFI_CAPSULE_ON_DISK > > > > + bool "Build mkeficapsule tool" > > > > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > > > > help > > > > - This command allows users to create a UEFI
capsule file and,
> > > > + This tool allows users to create a UEFI capsule
file and,
> > > > optionally sign that file. If you want to enable
UEFI capsule
> > > > update feature on your target, you certainly need
this.
> > > > > > Sorry, what is this fixing exactly? > > > > The tool is required to be supported on the sandbox_spl
variant, since
> > that is used for the binman tests in CI. Simon had then asked
me to
> > add support for the tool on all sandbox variants. I missed
putting his
> > R-b on this patch. > > OK, moving forward just depend on: >
https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri...
> instead please, thanks.
I will base my changes on top of your patch. However, we would
still
need this patch as part of the series, since Simon wants the
capsules
to be generated for all the sandbox variants. Thanks.
No, this isn't needed. Any sandbox variant that needs capsules has EFI_CAPSULE_ON_DISK enabled.
Simon wants the capsules to be generated on all sandbox variants, including those that do not have the EFI_CAPSULE_ON_DISK enabled. Which is why we need to have the tool enabled for all sandbox variants.
I want to avoid #ifdefs in the sandbox .dts so far as possible.
Tom, I'll let you make the final decision.
In any case, the multiple-images thing needs to be fixed.
Sughosh, please update the other sandbox defconfigs to just enable EFI_CAPSULE_ON_DISK.
Simon, this I think is an example of where re-working configs/sandbox64_defconfig configs/sandbox_defconfig configs/sandbox_flattree_defconfig configs/sandbox_noinst_defconfig configs/sandbox_spl_defconfig configs/sandbox_vpl_defconfig
To be configs/sandbox_defconfig + boards/sandbox/flattree.config, noinst.config, spl.config, vpl.config would be helpful. There's the sandbox config itself where EFI_CAPSULE_ON_DISK=y and then every other variant just gets that, and we don't have to tweak N configs.
You mean split configs? So far I am unable to build those...
I don't know what you mean by split configs. I mean that I think the only intentional difference between configs/sandbox_defconfig and configs/sandbox64_defconfig is: CONFIG_SANDBOX64=y CONFIG_DEFAULT_DEVICE_TREE="sandbox64"
And everything else is unintentional. And there's lots of other deltas like that between each of the other variants, and sandbox. And that this isn't the first, nor likely the last, time where we need to enable some option on other sandbox config files too, so that CI passes. This would all be avoided by using the config fragments mechanism so that we captured only the intentional delta of a fragment rather than maintaining N nearly identical, but not quite, files.

Hi Tom,
On Sat, 12 Aug 2023 at 08:22, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 07:08:44AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 11 Aug 2023 at 09:56, Tom Rini trini@konsulko.com wrote:
On Fri, Aug 11, 2023 at 08:26:36AM -0600, Simon Glass wrote:
Hi Sughosh,
On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org
wrote:
On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote:
On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote: > On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote: > > > > On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote: > > > On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com
wrote:
> > > > > > > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu
wrote:
> > > > > > > > > Build the mkeficapsule tool for all the sandbox variants.
This tool
> > > > > will be used subsequently for testing capsule generation
in binman.
> > > > > > > > > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > > > > > --- > > > > > Changes since V7: None > > > > > > > > > > tools/Kconfig | 6 +++--- > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > > > > index 6e23f44d55..353a855243 100644 > > > > > --- a/tools/Kconfig > > > > > +++ b/tools/Kconfig > > > > > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > > > > > Enable SHA512 support in the tools builds > > > > > > > > > > config TOOLS_MKEFICAPSULE > > > > > - bool "Build efimkcapsule command" > > > > > - default y if EFI_CAPSULE_ON_DISK > > > > > + bool "Build mkeficapsule tool" > > > > > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > > > > > help > > > > > - This command allows users to create a UEFI
capsule file and,
> > > > > + This tool allows users to create a UEFI capsule
file and,
> > > > > optionally sign that file. If you want to enable
UEFI capsule
> > > > > update feature on your target, you certainly need
this.
> > > > > > > > Sorry, what is this fixing exactly? > > > > > > The tool is required to be supported on the sandbox_spl
variant, since
> > > that is used for the binman tests in CI. Simon had then asked
me to
> > > add support for the tool on all sandbox variants. I missed
putting his
> > > R-b on this patch. > > > > OK, moving forward just depend on: > >
https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri...
> > instead please, thanks. > > I will base my changes on top of your patch. However, we would
still
> need this patch as part of the series, since Simon wants the
capsules
> to be generated for all the sandbox variants. Thanks.
No, this isn't needed. Any sandbox variant that needs capsules has EFI_CAPSULE_ON_DISK enabled.
Simon wants the capsules to be generated on all sandbox variants, including those that do not have the EFI_CAPSULE_ON_DISK enabled. Which is why we need to have the tool enabled for all sandbox variants.
I want to avoid #ifdefs in the sandbox .dts so far as possible.
Tom, I'll let you make the final decision.
In any case, the multiple-images thing needs to be fixed.
Sughosh, please update the other sandbox defconfigs to just enable EFI_CAPSULE_ON_DISK.
Simon, this I think is an example of where re-working configs/sandbox64_defconfig configs/sandbox_defconfig configs/sandbox_flattree_defconfig configs/sandbox_noinst_defconfig configs/sandbox_spl_defconfig configs/sandbox_vpl_defconfig
To be configs/sandbox_defconfig + boards/sandbox/flattree.config, noinst.config, spl.config, vpl.config would be helpful. There's the sandbox config itself where EFI_CAPSULE_ON_DISK=y and then every other variant just gets that, and we don't have to tweak N configs.
You mean split configs? So far I am unable to build those...
I don't know what you mean by split configs. I mean that I think the only intentional difference between configs/sandbox_defconfig and configs/sandbox64_defconfig is: CONFIG_SANDBOX64=y CONFIG_DEFAULT_DEVICE_TREE="sandbox64"
And everything else is unintentional. And there's lots of other deltas like that between each of the other variants, and sandbox. And that this isn't the first, nor likely the last, time where we need to enable some option on other sandbox config files too, so that CI passes. This would all be avoided by using the config fragments mechanism so that we captured only the intentional delta of a fragment rather than maintaining N nearly identical, but not quite, files.
Well we do have other intentional differences, e.g. OF_LIVE. But OK if we can find a way to make fragments work with buildman (amd qconfig), then we could do this.
Regards, Simon

On Sat, Aug 12, 2023 at 08:24:59AM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 08:22, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 07:08:44AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 11 Aug 2023 at 09:56, Tom Rini trini@konsulko.com wrote:
On Fri, Aug 11, 2023 at 08:26:36AM -0600, Simon Glass wrote:
Hi Sughosh,
On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org
wrote:
On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote: > > On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote: > > On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote: > > > > > > On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote: > > > > On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com
wrote:
> > > > > > > > > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu
wrote:
> > > > > > > > > > > Build the mkeficapsule tool for all the sandbox variants.
This tool
> > > > > > will be used subsequently for testing capsule generation
in binman.
> > > > > > > > > > > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > > > > > > --- > > > > > > Changes since V7: None > > > > > > > > > > > > tools/Kconfig | 6 +++--- > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > > > > > index 6e23f44d55..353a855243 100644 > > > > > > --- a/tools/Kconfig > > > > > > +++ b/tools/Kconfig > > > > > > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > > > > > > Enable SHA512 support in the tools builds > > > > > > > > > > > > config TOOLS_MKEFICAPSULE > > > > > > - bool "Build efimkcapsule command" > > > > > > - default y if EFI_CAPSULE_ON_DISK > > > > > > + bool "Build mkeficapsule tool" > > > > > > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > > > > > > help > > > > > > - This command allows users to create a UEFI
capsule file and,
> > > > > > + This tool allows users to create a UEFI capsule
file and,
> > > > > > optionally sign that file. If you want to enable
UEFI capsule
> > > > > > update feature on your target, you certainly need
this.
> > > > > > > > > > Sorry, what is this fixing exactly? > > > > > > > > The tool is required to be supported on the sandbox_spl
variant, since
> > > > that is used for the binman tests in CI. Simon had then asked
me to
> > > > add support for the tool on all sandbox variants. I missed
putting his
> > > > R-b on this patch. > > > > > > OK, moving forward just depend on: > > >
https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri...
> > > instead please, thanks. > > > > I will base my changes on top of your patch. However, we would
still
> > need this patch as part of the series, since Simon wants the
capsules
> > to be generated for all the sandbox variants. Thanks. > > No, this isn't needed. Any sandbox variant that needs capsules has > EFI_CAPSULE_ON_DISK enabled.
Simon wants the capsules to be generated on all sandbox variants, including those that do not have the EFI_CAPSULE_ON_DISK enabled. Which is why we need to have the tool enabled for all sandbox variants.
I want to avoid #ifdefs in the sandbox .dts so far as possible.
Tom, I'll let you make the final decision.
In any case, the multiple-images thing needs to be fixed.
Sughosh, please update the other sandbox defconfigs to just enable EFI_CAPSULE_ON_DISK.
Simon, this I think is an example of where re-working configs/sandbox64_defconfig configs/sandbox_defconfig configs/sandbox_flattree_defconfig configs/sandbox_noinst_defconfig configs/sandbox_spl_defconfig configs/sandbox_vpl_defconfig
To be configs/sandbox_defconfig + boards/sandbox/flattree.config, noinst.config, spl.config, vpl.config would be helpful. There's the sandbox config itself where EFI_CAPSULE_ON_DISK=y and then every other variant just gets that, and we don't have to tweak N configs.
You mean split configs? So far I am unable to build those...
I don't know what you mean by split configs. I mean that I think the only intentional difference between configs/sandbox_defconfig and configs/sandbox64_defconfig is: CONFIG_SANDBOX64=y CONFIG_DEFAULT_DEVICE_TREE="sandbox64"
And everything else is unintentional. And there's lots of other deltas like that between each of the other variants, and sandbox. And that this isn't the first, nor likely the last, time where we need to enable some option on other sandbox config files too, so that CI passes. This would all be avoided by using the config fragments mechanism so that we captured only the intentional delta of a fragment rather than maintaining N nearly identical, but not quite, files.
Well we do have other intentional differences, e.g. OF_LIVE. But OK if we can find a way to make fragments work with buildman (amd qconfig), then we could do this.
Yes, I was noting this in hopes of sparking your interest in figuring out how to handle fragments with buildman. It's similar to how we have the override option today.

Hi Tom,
On Sat, 12 Aug 2023 at 08:28, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 08:24:59AM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 08:22, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 07:08:44AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 11 Aug 2023 at 09:56, Tom Rini trini@konsulko.com wrote:
On Fri, Aug 11, 2023 at 08:26:36AM -0600, Simon Glass wrote:
Hi Sughosh,
On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org
wrote:
> > On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote: > > > > On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote: > > > On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote: > > > > > > > > On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote: > > > > > On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com
wrote:
> > > > > > > > > > > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu
wrote:
> > > > > > > > > > > > > Build the mkeficapsule tool for all the sandbox variants.
This tool
> > > > > > > will be used subsequently for testing capsule generation
in binman.
> > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > > > > > > > --- > > > > > > > Changes since V7: None > > > > > > > > > > > > > > tools/Kconfig | 6 +++--- > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > > > > > > index 6e23f44d55..353a855243 100644 > > > > > > > --- a/tools/Kconfig > > > > > > > +++ b/tools/Kconfig > > > > > > > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > > > > > > > Enable SHA512 support in the tools builds > > > > > > > > > > > > > > config TOOLS_MKEFICAPSULE > > > > > > > - bool "Build efimkcapsule command" > > > > > > > - default y if EFI_CAPSULE_ON_DISK > > > > > > > + bool "Build mkeficapsule tool" > > > > > > > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > > > > > > > help > > > > > > > - This command allows users to create a UEFI
capsule file and,
> > > > > > > + This tool allows users to create a UEFI capsule
file and,
> > > > > > > optionally sign that file. If you want to enable
UEFI capsule
> > > > > > > update feature on your target, you certainly need
this.
> > > > > > > > > > > > Sorry, what is this fixing exactly? > > > > > > > > > > The tool is required to be supported on the sandbox_spl
variant, since
> > > > > that is used for the binman tests in CI. Simon had then asked
me to
> > > > > add support for the tool on all sandbox variants. I missed
putting his
> > > > > R-b on this patch. > > > > > > > > OK, moving forward just depend on: > > > >
https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri...
> > > > instead please, thanks. > > > > > > I will base my changes on top of your patch. However, we would
still
> > > need this patch as part of the series, since Simon wants the
capsules
> > > to be generated for all the sandbox variants. Thanks. > > > > No, this isn't needed. Any sandbox variant that needs capsules has > > EFI_CAPSULE_ON_DISK enabled. > > Simon wants the capsules to be generated on all sandbox variants, > including those that do not have the EFI_CAPSULE_ON_DISK enabled. > Which is why we need to have the tool enabled for all sandbox > variants.
I want to avoid #ifdefs in the sandbox .dts so far as possible.
Tom, I'll let you make the final decision.
In any case, the multiple-images thing needs to be fixed.
Sughosh, please update the other sandbox defconfigs to just enable EFI_CAPSULE_ON_DISK.
Simon, this I think is an example of where re-working configs/sandbox64_defconfig configs/sandbox_defconfig configs/sandbox_flattree_defconfig configs/sandbox_noinst_defconfig configs/sandbox_spl_defconfig configs/sandbox_vpl_defconfig
To be configs/sandbox_defconfig + boards/sandbox/flattree.config, noinst.config, spl.config, vpl.config would be helpful. There's the sandbox config itself where EFI_CAPSULE_ON_DISK=y and then every other variant just gets that, and we don't have to tweak N configs.
You mean split configs? So far I am unable to build those...
I don't know what you mean by split configs. I mean that I think the only intentional difference between configs/sandbox_defconfig and configs/sandbox64_defconfig is: CONFIG_SANDBOX64=y CONFIG_DEFAULT_DEVICE_TREE="sandbox64"
And everything else is unintentional. And there's lots of other deltas like that between each of the other variants, and sandbox. And that this isn't the first, nor likely the last, time where we need to enable some option on other sandbox config files too, so that CI passes. This would all be avoided by using the config fragments mechanism so that we captured only the intentional delta of a fragment rather than maintaining N nearly identical, but not quite, files.
Well we do have other intentional differences, e.g. OF_LIVE. But OK if we can find a way to make fragments work with buildman (amd qconfig), then we could do this.
Yes, I was noting this in hopes of sparking your interest in figuring out how to handle fragments with buildman. It's similar to how we have the override option today.
We need a list of fragments somewhere, so that it is possible to enumerate the different board combinations. Does the main defconfig have a way to specify this, or could we add it?
Regards, Simon

On Sat, Aug 12, 2023 at 11:03:36AM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 08:28, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 08:24:59AM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 08:22, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 07:08:44AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 11 Aug 2023 at 09:56, Tom Rini trini@konsulko.com wrote:
On Fri, Aug 11, 2023 at 08:26:36AM -0600, Simon Glass wrote: > Hi Sughosh, > > On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org
wrote:
> > > > On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote: > > > > > > On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote: > > > > On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote: > > > > > > On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com
wrote:
> > > > > > > > > > > > > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu
wrote:
> > > > > > > > > > > > > > > Build the mkeficapsule tool for all the sandbox variants.
This tool
> > > > > > > > will be used subsequently for testing capsule generation
in binman.
> > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > > > > > > > > --- > > > > > > > > Changes since V7: None > > > > > > > > > > > > > > > > tools/Kconfig | 6 +++--- > > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > > > > > > > index 6e23f44d55..353a855243 100644 > > > > > > > > --- a/tools/Kconfig > > > > > > > > +++ b/tools/Kconfig > > > > > > > > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > > > > > > > > Enable SHA512 support in the tools builds > > > > > > > > > > > > > > > > config TOOLS_MKEFICAPSULE > > > > > > > > - bool "Build efimkcapsule command" > > > > > > > > - default y if EFI_CAPSULE_ON_DISK > > > > > > > > + bool "Build mkeficapsule tool" > > > > > > > > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > > > > > > > > help > > > > > > > > - This command allows users to create a UEFI
capsule file and,
> > > > > > > > + This tool allows users to create a UEFI capsule
file and,
> > > > > > > > optionally sign that file. If you want to enable
UEFI capsule
> > > > > > > > update feature on your target, you certainly need
this.
> > > > > > > > > > > > > > Sorry, what is this fixing exactly? > > > > > > > > > > > > The tool is required to be supported on the sandbox_spl
variant, since
> > > > > > that is used for the binman tests in CI. Simon had then asked
me to
> > > > > > add support for the tool on all sandbox variants. I missed
putting his
> > > > > > R-b on this patch. > > > > > > > > > > OK, moving forward just depend on: > > > > >
https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri...
> > > > > instead please, thanks. > > > > > > > > I will base my changes on top of your patch. However, we would
still
> > > > need this patch as part of the series, since Simon wants the
capsules
> > > > to be generated for all the sandbox variants. Thanks. > > > > > > No, this isn't needed. Any sandbox variant that needs capsules has > > > EFI_CAPSULE_ON_DISK enabled. > > > > Simon wants the capsules to be generated on all sandbox variants, > > including those that do not have the EFI_CAPSULE_ON_DISK enabled. > > Which is why we need to have the tool enabled for all sandbox > > variants. > > I want to avoid #ifdefs in the sandbox .dts so far as possible. > > Tom, I'll let you make the final decision. > > In any case, the multiple-images thing needs to be fixed.
Sughosh, please update the other sandbox defconfigs to just enable EFI_CAPSULE_ON_DISK.
Simon, this I think is an example of where re-working configs/sandbox64_defconfig configs/sandbox_defconfig configs/sandbox_flattree_defconfig configs/sandbox_noinst_defconfig configs/sandbox_spl_defconfig configs/sandbox_vpl_defconfig
To be configs/sandbox_defconfig + boards/sandbox/flattree.config, noinst.config, spl.config, vpl.config would be helpful. There's the sandbox config itself where EFI_CAPSULE_ON_DISK=y and then every other variant just gets that, and we don't have to tweak N configs.
You mean split configs? So far I am unable to build those...
I don't know what you mean by split configs. I mean that I think the only intentional difference between configs/sandbox_defconfig and configs/sandbox64_defconfig is: CONFIG_SANDBOX64=y CONFIG_DEFAULT_DEVICE_TREE="sandbox64"
And everything else is unintentional. And there's lots of other deltas like that between each of the other variants, and sandbox. And that this isn't the first, nor likely the last, time where we need to enable some option on other sandbox config files too, so that CI passes. This would all be avoided by using the config fragments mechanism so that we captured only the intentional delta of a fragment rather than maintaining N nearly identical, but not quite, files.
Well we do have other intentional differences, e.g. OF_LIVE. But OK if we can find a way to make fragments work with buildman (amd qconfig), then we could do this.
Yes, I was noting this in hopes of sparking your interest in figuring out how to handle fragments with buildman. It's similar to how we have the override option today.
We need a list of fragments somewhere, so that it is possible to enumerate the different board combinations. Does the main defconfig have a way to specify this, or could we add it?
No, I don't think that's the right way to go. I was thinking of something along the lines of how --adjust-cfg works, but instead it's a csv of additional targets to pass along with the defconfig name when invoking make.

Hi Tom,
On Sat, 12 Aug 2023 at 16:38, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 11:03:36AM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 08:28, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 08:24:59AM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 08:22, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 07:08:44AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 11 Aug 2023 at 09:56, Tom Rini trini@konsulko.com wrote: > > On Fri, Aug 11, 2023 at 08:26:36AM -0600, Simon Glass wrote: > > Hi Sughosh, > > > > On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org wrote: > > > > > > On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote: > > > > > > > > On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote: > > > > > On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > > > On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote: > > > > > > > On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > > > > > > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote: > > > > > > > > > > > > > > > > > Build the mkeficapsule tool for all the sandbox variants. This tool > > > > > > > > > will be used subsequently for testing capsule generation in binman. > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > > > > > > > > > --- > > > > > > > > > Changes since V7: None > > > > > > > > > > > > > > > > > > tools/Kconfig | 6 +++--- > > > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > > > > > > > > index 6e23f44d55..353a855243 100644 > > > > > > > > > --- a/tools/Kconfig > > > > > > > > > +++ b/tools/Kconfig > > > > > > > > > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > > > > > > > > > Enable SHA512 support in the tools builds > > > > > > > > > > > > > > > > > > config TOOLS_MKEFICAPSULE > > > > > > > > > - bool "Build efimkcapsule command" > > > > > > > > > - default y if EFI_CAPSULE_ON_DISK > > > > > > > > > + bool "Build mkeficapsule tool" > > > > > > > > > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > > > > > > > > > help > > > > > > > > > - This command allows users to create a UEFI capsule file and, > > > > > > > > > + This tool allows users to create a UEFI capsule file and, > > > > > > > > > optionally sign that file. If you want to enable UEFI capsule > > > > > > > > > update feature on your target, you certainly need this. > > > > > > > > > > > > > > > > Sorry, what is this fixing exactly? > > > > > > > > > > > > > > The tool is required to be supported on the sandbox_spl variant, since > > > > > > > that is used for the binman tests in CI. Simon had then asked me to > > > > > > > add support for the tool on all sandbox variants. I missed putting his > > > > > > > R-b on this patch. > > > > > > > > > > > > OK, moving forward just depend on: > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri... > > > > > > instead please, thanks. > > > > > > > > > > I will base my changes on top of your patch. However, we would still > > > > > need this patch as part of the series, since Simon wants the capsules > > > > > to be generated for all the sandbox variants. Thanks. > > > > > > > > No, this isn't needed. Any sandbox variant that needs capsules has > > > > EFI_CAPSULE_ON_DISK enabled. > > > > > > Simon wants the capsules to be generated on all sandbox variants, > > > including those that do not have the EFI_CAPSULE_ON_DISK enabled. > > > Which is why we need to have the tool enabled for all sandbox > > > variants. > > > > I want to avoid #ifdefs in the sandbox .dts so far as possible. > > > > Tom, I'll let you make the final decision. > > > > In any case, the multiple-images thing needs to be fixed. > > Sughosh, please update the other sandbox defconfigs to just enable > EFI_CAPSULE_ON_DISK. > > Simon, this I think is an example of where re-working > configs/sandbox64_defconfig > configs/sandbox_defconfig > configs/sandbox_flattree_defconfig > configs/sandbox_noinst_defconfig > configs/sandbox_spl_defconfig > configs/sandbox_vpl_defconfig > > To be configs/sandbox_defconfig + boards/sandbox/flattree.config, > noinst.config, spl.config, vpl.config would be helpful. There's the > sandbox config itself where EFI_CAPSULE_ON_DISK=y and then every other > variant just gets that, and we don't have to tweak N configs.
You mean split configs? So far I am unable to build those...
I don't know what you mean by split configs. I mean that I think the only intentional difference between configs/sandbox_defconfig and configs/sandbox64_defconfig is: CONFIG_SANDBOX64=y CONFIG_DEFAULT_DEVICE_TREE="sandbox64"
And everything else is unintentional. And there's lots of other deltas like that between each of the other variants, and sandbox. And that this isn't the first, nor likely the last, time where we need to enable some option on other sandbox config files too, so that CI passes. This would all be avoided by using the config fragments mechanism so that we captured only the intentional delta of a fragment rather than maintaining N nearly identical, but not quite, files.
Well we do have other intentional differences, e.g. OF_LIVE. But OK if we can find a way to make fragments work with buildman (amd qconfig), then we could do this.
Yes, I was noting this in hopes of sparking your interest in figuring out how to handle fragments with buildman. It's similar to how we have the override option today.
We need a list of fragments somewhere, so that it is possible to enumerate the different board combinations. Does the main defconfig have a way to specify this, or could we add it?
No, I don't think that's the right way to go. I was thinking of something along the lines of how --adjust-cfg works, but instead it's a csv of additional targets to pass along with the defconfig name when invoking make.
So we would do them one at a time, with the 'name' of the board being some portion of the filename of the config-fragment file?
BTW CSV is not great for humans...perhaps a text file with columns like boards.cfg ?
Regards, Simon

On Sat, Aug 12, 2023 at 06:14:45PM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 16:38, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 11:03:36AM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 08:28, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 08:24:59AM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 08:22, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 07:08:44AM -0600, Simon Glass wrote: > Hi Tom, > > On Fri, 11 Aug 2023 at 09:56, Tom Rini trini@konsulko.com wrote: > > > > On Fri, Aug 11, 2023 at 08:26:36AM -0600, Simon Glass wrote: > > > Hi Sughosh, > > > > > > On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org > wrote: > > > > > > > > On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote: > > > > > > On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > > > > > On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote: > > > > > > > > On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com > wrote: > > > > > > > > > > > > > > > > > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu > wrote: > > > > > > > > > > > > > > > > > > > Build the mkeficapsule tool for all the sandbox variants. > This tool > > > > > > > > > > will be used subsequently for testing capsule generation > in binman. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > > > > > > > > > > --- > > > > > > > > > > Changes since V7: None > > > > > > > > > > > > > > > > > > > > tools/Kconfig | 6 +++--- > > > > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > > > > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > > > > > > > > > index 6e23f44d55..353a855243 100644 > > > > > > > > > > --- a/tools/Kconfig > > > > > > > > > > +++ b/tools/Kconfig > > > > > > > > > > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > > > > > > > > > > Enable SHA512 support in the tools builds > > > > > > > > > > > > > > > > > > > > config TOOLS_MKEFICAPSULE > > > > > > > > > > - bool "Build efimkcapsule command" > > > > > > > > > > - default y if EFI_CAPSULE_ON_DISK > > > > > > > > > > + bool "Build mkeficapsule tool" > > > > > > > > > > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > > > > > > > > > > help > > > > > > > > > > - This command allows users to create a UEFI > capsule file and, > > > > > > > > > > + This tool allows users to create a UEFI capsule > file and, > > > > > > > > > > optionally sign that file. If you want to enable > UEFI capsule > > > > > > > > > > update feature on your target, you certainly need > this. > > > > > > > > > > > > > > > > > > Sorry, what is this fixing exactly? > > > > > > > > > > > > > > > > The tool is required to be supported on the sandbox_spl > variant, since > > > > > > > > that is used for the binman tests in CI. Simon had then asked > me to > > > > > > > > add support for the tool on all sandbox variants. I missed > putting his > > > > > > > > R-b on this patch. > > > > > > > > > > > > > > OK, moving forward just depend on: > > > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri... > > > > > > > instead please, thanks. > > > > > > > > > > > > I will base my changes on top of your patch. However, we would > still > > > > > > need this patch as part of the series, since Simon wants the > capsules > > > > > > to be generated for all the sandbox variants. Thanks. > > > > > > > > > > No, this isn't needed. Any sandbox variant that needs capsules has > > > > > EFI_CAPSULE_ON_DISK enabled. > > > > > > > > Simon wants the capsules to be generated on all sandbox variants, > > > > including those that do not have the EFI_CAPSULE_ON_DISK enabled. > > > > Which is why we need to have the tool enabled for all sandbox > > > > variants. > > > > > > I want to avoid #ifdefs in the sandbox .dts so far as possible. > > > > > > Tom, I'll let you make the final decision. > > > > > > In any case, the multiple-images thing needs to be fixed. > > > > Sughosh, please update the other sandbox defconfigs to just enable > > EFI_CAPSULE_ON_DISK. > > > > Simon, this I think is an example of where re-working > > configs/sandbox64_defconfig > > configs/sandbox_defconfig > > configs/sandbox_flattree_defconfig > > configs/sandbox_noinst_defconfig > > configs/sandbox_spl_defconfig > > configs/sandbox_vpl_defconfig > > > > To be configs/sandbox_defconfig + boards/sandbox/flattree.config, > > noinst.config, spl.config, vpl.config would be helpful. There's the > > sandbox config itself where EFI_CAPSULE_ON_DISK=y and then every other > > variant just gets that, and we don't have to tweak N configs. > > You mean split configs? So far I am unable to build those...
I don't know what you mean by split configs. I mean that I think the only intentional difference between configs/sandbox_defconfig and configs/sandbox64_defconfig is: CONFIG_SANDBOX64=y CONFIG_DEFAULT_DEVICE_TREE="sandbox64"
And everything else is unintentional. And there's lots of other deltas like that between each of the other variants, and sandbox. And that this isn't the first, nor likely the last, time where we need to enable some option on other sandbox config files too, so that CI passes. This would all be avoided by using the config fragments mechanism so that we captured only the intentional delta of a fragment rather than maintaining N nearly identical, but not quite, files.
Well we do have other intentional differences, e.g. OF_LIVE. But OK if we can find a way to make fragments work with buildman (amd qconfig), then we could do this.
Yes, I was noting this in hopes of sparking your interest in figuring out how to handle fragments with buildman. It's similar to how we have the override option today.
We need a list of fragments somewhere, so that it is possible to enumerate the different board combinations. Does the main defconfig have a way to specify this, or could we add it?
No, I don't think that's the right way to go. I was thinking of something along the lines of how --adjust-cfg works, but instead it's a csv of additional targets to pass along with the defconfig name when invoking make.
So we would do them one at a time, with the 'name' of the board being some portion of the filename of the config-fragment file?
BTW CSV is not great for humans...perhaps a text file with columns like boards.cfg ?
I think you're still missing what I'm saying. There should not be a file that lists fragments. Outside of documentation, at least. I was saying csv above because it would make sense to do something like: ./tools/buildman/buildman --add-fragments=64bit,vpl sandbox And that would eventually do: make sandbox_config 64bit.config vpl.config Which has the standard Kconfig merging of configs/sandbox_defconfig boards/sandbox/64bit.config (replaces sandbox64_defconfig) and boards/sandbox/vpl.config And passing multiple files with a comma seems easiest.

Hi Tom,
On Sun, 13 Aug 2023 at 06:40, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 06:14:45PM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 16:38, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 11:03:36AM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 08:28, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 08:24:59AM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 08:22, Tom Rini trini@konsulko.com wrote: > > On Sat, Aug 12, 2023 at 07:08:44AM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Fri, 11 Aug 2023 at 09:56, Tom Rini trini@konsulko.com wrote: > > > > > > On Fri, Aug 11, 2023 at 08:26:36AM -0600, Simon Glass wrote: > > > > Hi Sughosh, > > > > > > > > On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org > > wrote: > > > > > > > > > > On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > > > On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote: > > > > > > > On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > > > > > > > On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote: > > > > > > > > > On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com > > wrote: > > > > > > > > > > > > > > > > > > > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu > > wrote: > > > > > > > > > > > > > > > > > > > > > Build the mkeficapsule tool for all the sandbox variants. > > This tool > > > > > > > > > > > will be used subsequently for testing capsule generation > > in binman. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > > > > > > > > > > > --- > > > > > > > > > > > Changes since V7: None > > > > > > > > > > > > > > > > > > > > > > tools/Kconfig | 6 +++--- > > > > > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > > > > > > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > > > > > > > > > > index 6e23f44d55..353a855243 100644 > > > > > > > > > > > --- a/tools/Kconfig > > > > > > > > > > > +++ b/tools/Kconfig > > > > > > > > > > > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > > > > > > > > > > > Enable SHA512 support in the tools builds > > > > > > > > > > > > > > > > > > > > > > config TOOLS_MKEFICAPSULE > > > > > > > > > > > - bool "Build efimkcapsule command" > > > > > > > > > > > - default y if EFI_CAPSULE_ON_DISK > > > > > > > > > > > + bool "Build mkeficapsule tool" > > > > > > > > > > > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > > > > > > > > > > > help > > > > > > > > > > > - This command allows users to create a UEFI > > capsule file and, > > > > > > > > > > > + This tool allows users to create a UEFI capsule > > file and, > > > > > > > > > > > optionally sign that file. If you want to enable > > UEFI capsule > > > > > > > > > > > update feature on your target, you certainly need > > this. > > > > > > > > > > > > > > > > > > > > Sorry, what is this fixing exactly? > > > > > > > > > > > > > > > > > > The tool is required to be supported on the sandbox_spl > > variant, since > > > > > > > > > that is used for the binman tests in CI. Simon had then asked > > me to > > > > > > > > > add support for the tool on all sandbox variants. I missed > > putting his > > > > > > > > > R-b on this patch. > > > > > > > > > > > > > > > > OK, moving forward just depend on: > > > > > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri... > > > > > > > > instead please, thanks. > > > > > > > > > > > > > > I will base my changes on top of your patch. However, we would > > still > > > > > > > need this patch as part of the series, since Simon wants the > > capsules > > > > > > > to be generated for all the sandbox variants. Thanks. > > > > > > > > > > > > No, this isn't needed. Any sandbox variant that needs capsules has > > > > > > EFI_CAPSULE_ON_DISK enabled. > > > > > > > > > > Simon wants the capsules to be generated on all sandbox variants, > > > > > including those that do not have the EFI_CAPSULE_ON_DISK enabled. > > > > > Which is why we need to have the tool enabled for all sandbox > > > > > variants. > > > > > > > > I want to avoid #ifdefs in the sandbox .dts so far as possible. > > > > > > > > Tom, I'll let you make the final decision. > > > > > > > > In any case, the multiple-images thing needs to be fixed. > > > > > > Sughosh, please update the other sandbox defconfigs to just enable > > > EFI_CAPSULE_ON_DISK. > > > > > > Simon, this I think is an example of where re-working > > > configs/sandbox64_defconfig > > > configs/sandbox_defconfig > > > configs/sandbox_flattree_defconfig > > > configs/sandbox_noinst_defconfig > > > configs/sandbox_spl_defconfig > > > configs/sandbox_vpl_defconfig > > > > > > To be configs/sandbox_defconfig + boards/sandbox/flattree.config, > > > noinst.config, spl.config, vpl.config would be helpful. There's the > > > sandbox config itself where EFI_CAPSULE_ON_DISK=y and then every other > > > variant just gets that, and we don't have to tweak N configs. > > > > You mean split configs? So far I am unable to build those... > > I don't know what you mean by split configs. I mean that I think the > only intentional difference between configs/sandbox_defconfig and > configs/sandbox64_defconfig is: > CONFIG_SANDBOX64=y > CONFIG_DEFAULT_DEVICE_TREE="sandbox64" > > And everything else is unintentional. And there's lots of other deltas > like that between each of the other variants, and sandbox. And that > this isn't the first, nor likely the last, time where we need to enable > some option on other sandbox config files too, so that CI passes. This > would all be avoided by using the config fragments mechanism so that > we captured only the intentional delta of a fragment rather than > maintaining N nearly identical, but not quite, files.
Well we do have other intentional differences, e.g. OF_LIVE. But OK if we can find a way to make fragments work with buildman (amd qconfig), then we could do this.
Yes, I was noting this in hopes of sparking your interest in figuring out how to handle fragments with buildman. It's similar to how we have the override option today.
We need a list of fragments somewhere, so that it is possible to enumerate the different board combinations. Does the main defconfig have a way to specify this, or could we add it?
No, I don't think that's the right way to go. I was thinking of something along the lines of how --adjust-cfg works, but instead it's a csv of additional targets to pass along with the defconfig name when invoking make.
So we would do them one at a time, with the 'name' of the board being some portion of the filename of the config-fragment file?
BTW CSV is not great for humans...perhaps a text file with columns like boards.cfg ?
I think you're still missing what I'm saying. There should not be a file that lists fragments. Outside of documentation, at least. I was saying csv above because it would make sense to do something like: ./tools/buildman/buildman --add-fragments=64bit,vpl sandbox And that would eventually do: make sandbox_config 64bit.config vpl.config Which has the standard Kconfig merging of configs/sandbox_defconfig boards/sandbox/64bit.config (replaces sandbox64_defconfig) and boards/sandbox/vpl.config And passing multiple files with a comma seems easiest.
So is it only possible to add one fragment file to a build?
I see what you are saying, but from my side I am trying to enumerate the boards, since generally I (like) build things without explicitly specifying each board defconfig.
Regards, Simon

On Sun, Aug 13, 2023 at 07:36:45AM -0600, Simon Glass wrote:
Hi Tom,
On Sun, 13 Aug 2023 at 06:40, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 06:14:45PM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 16:38, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 11:03:36AM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 08:28, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 08:24:59AM -0600, Simon Glass wrote: > Hi Tom, > > On Sat, 12 Aug 2023 at 08:22, Tom Rini trini@konsulko.com wrote: > > > > On Sat, Aug 12, 2023 at 07:08:44AM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Fri, 11 Aug 2023 at 09:56, Tom Rini trini@konsulko.com wrote: > > > > > > > > On Fri, Aug 11, 2023 at 08:26:36AM -0600, Simon Glass wrote: > > > > > Hi Sughosh, > > > > > > > > > > On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org > > > wrote: > > > > > > > > > > > > On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > > > > > On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote: > > > > > > > > On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > > > > > > > > > On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote: > > > > > > > > > > On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com > > > wrote: > > > > > > > > > > > > > > > > > > > > > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > Build the mkeficapsule tool for all the sandbox variants. > > > This tool > > > > > > > > > > > > will be used subsequently for testing capsule generation > > > in binman. > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > > > > > > > > > > > > --- > > > > > > > > > > > > Changes since V7: None > > > > > > > > > > > > > > > > > > > > > > > > tools/Kconfig | 6 +++--- > > > > > > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > > > > > > > > > > > index 6e23f44d55..353a855243 100644 > > > > > > > > > > > > --- a/tools/Kconfig > > > > > > > > > > > > +++ b/tools/Kconfig > > > > > > > > > > > > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > > > > > > > > > > > > Enable SHA512 support in the tools builds > > > > > > > > > > > > > > > > > > > > > > > > config TOOLS_MKEFICAPSULE > > > > > > > > > > > > - bool "Build efimkcapsule command" > > > > > > > > > > > > - default y if EFI_CAPSULE_ON_DISK > > > > > > > > > > > > + bool "Build mkeficapsule tool" > > > > > > > > > > > > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > > > > > > > > > > > > help > > > > > > > > > > > > - This command allows users to create a UEFI > > > capsule file and, > > > > > > > > > > > > + This tool allows users to create a UEFI capsule > > > file and, > > > > > > > > > > > > optionally sign that file. If you want to enable > > > UEFI capsule > > > > > > > > > > > > update feature on your target, you certainly need > > > this. > > > > > > > > > > > > > > > > > > > > > > Sorry, what is this fixing exactly? > > > > > > > > > > > > > > > > > > > > The tool is required to be supported on the sandbox_spl > > > variant, since > > > > > > > > > > that is used for the binman tests in CI. Simon had then asked > > > me to > > > > > > > > > > add support for the tool on all sandbox variants. I missed > > > putting his > > > > > > > > > > R-b on this patch. > > > > > > > > > > > > > > > > > > OK, moving forward just depend on: > > > > > > > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri... > > > > > > > > > instead please, thanks. > > > > > > > > > > > > > > > > I will base my changes on top of your patch. However, we would > > > still > > > > > > > > need this patch as part of the series, since Simon wants the > > > capsules > > > > > > > > to be generated for all the sandbox variants. Thanks. > > > > > > > > > > > > > > No, this isn't needed. Any sandbox variant that needs capsules has > > > > > > > EFI_CAPSULE_ON_DISK enabled. > > > > > > > > > > > > Simon wants the capsules to be generated on all sandbox variants, > > > > > > including those that do not have the EFI_CAPSULE_ON_DISK enabled. > > > > > > Which is why we need to have the tool enabled for all sandbox > > > > > > variants. > > > > > > > > > > I want to avoid #ifdefs in the sandbox .dts so far as possible. > > > > > > > > > > Tom, I'll let you make the final decision. > > > > > > > > > > In any case, the multiple-images thing needs to be fixed. > > > > > > > > Sughosh, please update the other sandbox defconfigs to just enable > > > > EFI_CAPSULE_ON_DISK. > > > > > > > > Simon, this I think is an example of where re-working > > > > configs/sandbox64_defconfig > > > > configs/sandbox_defconfig > > > > configs/sandbox_flattree_defconfig > > > > configs/sandbox_noinst_defconfig > > > > configs/sandbox_spl_defconfig > > > > configs/sandbox_vpl_defconfig > > > > > > > > To be configs/sandbox_defconfig + boards/sandbox/flattree.config, > > > > noinst.config, spl.config, vpl.config would be helpful. There's the > > > > sandbox config itself where EFI_CAPSULE_ON_DISK=y and then every other > > > > variant just gets that, and we don't have to tweak N configs. > > > > > > You mean split configs? So far I am unable to build those... > > > > I don't know what you mean by split configs. I mean that I think the > > only intentional difference between configs/sandbox_defconfig and > > configs/sandbox64_defconfig is: > > CONFIG_SANDBOX64=y > > CONFIG_DEFAULT_DEVICE_TREE="sandbox64" > > > > And everything else is unintentional. And there's lots of other deltas > > like that between each of the other variants, and sandbox. And that > > this isn't the first, nor likely the last, time where we need to enable > > some option on other sandbox config files too, so that CI passes. This > > would all be avoided by using the config fragments mechanism so that > > we captured only the intentional delta of a fragment rather than > > maintaining N nearly identical, but not quite, files. > > Well we do have other intentional differences, e.g. OF_LIVE. But OK if > we can find a way to make fragments work with buildman (amd qconfig), > then we could do this.
Yes, I was noting this in hopes of sparking your interest in figuring out how to handle fragments with buildman. It's similar to how we have the override option today.
We need a list of fragments somewhere, so that it is possible to enumerate the different board combinations. Does the main defconfig have a way to specify this, or could we add it?
No, I don't think that's the right way to go. I was thinking of something along the lines of how --adjust-cfg works, but instead it's a csv of additional targets to pass along with the defconfig name when invoking make.
So we would do them one at a time, with the 'name' of the board being some portion of the filename of the config-fragment file?
BTW CSV is not great for humans...perhaps a text file with columns like boards.cfg ?
I think you're still missing what I'm saying. There should not be a file that lists fragments. Outside of documentation, at least. I was saying csv above because it would make sense to do something like: ./tools/buildman/buildman --add-fragments=64bit,vpl sandbox And that would eventually do: make sandbox_config 64bit.config vpl.config Which has the standard Kconfig merging of configs/sandbox_defconfig boards/sandbox/64bit.config (replaces sandbox64_defconfig) and boards/sandbox/vpl.config And passing multiple files with a comma seems easiest.
So is it only possible to add one fragment file to a build?
The example above is two, and yes, N config fragments works (they are merged in listed order).
I see what you are saying, but from my side I am trying to enumerate the boards, since generally I (like) build things without explicitly specifying each board defconfig.
Yes, but that's not possible in this case I think. And I'm really just trying to figure out how we can make CI a little easier. But maybe we can't / don't bother in this case and keep fixing up the sandbox defconfig files as needed.

Hi Tom,
On Sun, 13 Aug 2023 at 08:43, Tom Rini trini@konsulko.com wrote:
On Sun, Aug 13, 2023 at 07:36:45AM -0600, Simon Glass wrote:
Hi Tom,
On Sun, 13 Aug 2023 at 06:40, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 06:14:45PM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 16:38, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 11:03:36AM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 08:28, Tom Rini trini@konsulko.com wrote: > > On Sat, Aug 12, 2023 at 08:24:59AM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Sat, 12 Aug 2023 at 08:22, Tom Rini trini@konsulko.com wrote: > > > > > > On Sat, Aug 12, 2023 at 07:08:44AM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Fri, 11 Aug 2023 at 09:56, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > On Fri, Aug 11, 2023 at 08:26:36AM -0600, Simon Glass wrote: > > > > > > Hi Sughosh, > > > > > > > > > > > > On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org > > > > wrote: > > > > > > > > > > > > > > On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > > > > > > > On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote: > > > > > > > > > On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > > > > > > > > > > > On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote: > > > > > > > > > > > On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > Build the mkeficapsule tool for all the sandbox variants. > > > > This tool > > > > > > > > > > > > > will be used subsequently for testing capsule generation > > > > in binman. > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > > > > > > > > > > > > > --- > > > > > > > > > > > > > Changes since V7: None > > > > > > > > > > > > > > > > > > > > > > > > > > tools/Kconfig | 6 +++--- > > > > > > > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > > > > > > > > > > > > index 6e23f44d55..353a855243 100644 > > > > > > > > > > > > > --- a/tools/Kconfig > > > > > > > > > > > > > +++ b/tools/Kconfig > > > > > > > > > > > > > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > > > > > > > > > > > > > Enable SHA512 support in the tools builds > > > > > > > > > > > > > > > > > > > > > > > > > > config TOOLS_MKEFICAPSULE > > > > > > > > > > > > > - bool "Build efimkcapsule command" > > > > > > > > > > > > > - default y if EFI_CAPSULE_ON_DISK > > > > > > > > > > > > > + bool "Build mkeficapsule tool" > > > > > > > > > > > > > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > > > > > > > > > > > > > help > > > > > > > > > > > > > - This command allows users to create a UEFI > > > > capsule file and, > > > > > > > > > > > > > + This tool allows users to create a UEFI capsule > > > > file and, > > > > > > > > > > > > > optionally sign that file. If you want to enable > > > > UEFI capsule > > > > > > > > > > > > > update feature on your target, you certainly need > > > > this. > > > > > > > > > > > > > > > > > > > > > > > > Sorry, what is this fixing exactly? > > > > > > > > > > > > > > > > > > > > > > The tool is required to be supported on the sandbox_spl > > > > variant, since > > > > > > > > > > > that is used for the binman tests in CI. Simon had then asked > > > > me to > > > > > > > > > > > add support for the tool on all sandbox variants. I missed > > > > putting his > > > > > > > > > > > R-b on this patch. > > > > > > > > > > > > > > > > > > > > OK, moving forward just depend on: > > > > > > > > > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri... > > > > > > > > > > instead please, thanks. > > > > > > > > > > > > > > > > > > I will base my changes on top of your patch. However, we would > > > > still > > > > > > > > > need this patch as part of the series, since Simon wants the > > > > capsules > > > > > > > > > to be generated for all the sandbox variants. Thanks. > > > > > > > > > > > > > > > > No, this isn't needed. Any sandbox variant that needs capsules has > > > > > > > > EFI_CAPSULE_ON_DISK enabled. > > > > > > > > > > > > > > Simon wants the capsules to be generated on all sandbox variants, > > > > > > > including those that do not have the EFI_CAPSULE_ON_DISK enabled. > > > > > > > Which is why we need to have the tool enabled for all sandbox > > > > > > > variants. > > > > > > > > > > > > I want to avoid #ifdefs in the sandbox .dts so far as possible. > > > > > > > > > > > > Tom, I'll let you make the final decision. > > > > > > > > > > > > In any case, the multiple-images thing needs to be fixed. > > > > > > > > > > Sughosh, please update the other sandbox defconfigs to just enable > > > > > EFI_CAPSULE_ON_DISK. > > > > > > > > > > Simon, this I think is an example of where re-working > > > > > configs/sandbox64_defconfig > > > > > configs/sandbox_defconfig > > > > > configs/sandbox_flattree_defconfig > > > > > configs/sandbox_noinst_defconfig > > > > > configs/sandbox_spl_defconfig > > > > > configs/sandbox_vpl_defconfig > > > > > > > > > > To be configs/sandbox_defconfig + boards/sandbox/flattree.config, > > > > > noinst.config, spl.config, vpl.config would be helpful. There's the > > > > > sandbox config itself where EFI_CAPSULE_ON_DISK=y and then every other > > > > > variant just gets that, and we don't have to tweak N configs. > > > > > > > > You mean split configs? So far I am unable to build those... > > > > > > I don't know what you mean by split configs. I mean that I think the > > > only intentional difference between configs/sandbox_defconfig and > > > configs/sandbox64_defconfig is: > > > CONFIG_SANDBOX64=y > > > CONFIG_DEFAULT_DEVICE_TREE="sandbox64" > > > > > > And everything else is unintentional. And there's lots of other deltas > > > like that between each of the other variants, and sandbox. And that > > > this isn't the first, nor likely the last, time where we need to enable > > > some option on other sandbox config files too, so that CI passes. This > > > would all be avoided by using the config fragments mechanism so that > > > we captured only the intentional delta of a fragment rather than > > > maintaining N nearly identical, but not quite, files. > > > > Well we do have other intentional differences, e.g. OF_LIVE. But OK if > > we can find a way to make fragments work with buildman (amd qconfig), > > then we could do this. > > Yes, I was noting this in hopes of sparking your interest in figuring > out how to handle fragments with buildman. It's similar to how we have > the override option today.
We need a list of fragments somewhere, so that it is possible to enumerate the different board combinations. Does the main defconfig have a way to specify this, or could we add it?
No, I don't think that's the right way to go. I was thinking of something along the lines of how --adjust-cfg works, but instead it's a csv of additional targets to pass along with the defconfig name when invoking make.
So we would do them one at a time, with the 'name' of the board being some portion of the filename of the config-fragment file?
BTW CSV is not great for humans...perhaps a text file with columns like boards.cfg ?
I think you're still missing what I'm saying. There should not be a file that lists fragments. Outside of documentation, at least. I was saying csv above because it would make sense to do something like: ./tools/buildman/buildman --add-fragments=64bit,vpl sandbox And that would eventually do: make sandbox_config 64bit.config vpl.config Which has the standard Kconfig merging of configs/sandbox_defconfig boards/sandbox/64bit.config (replaces sandbox64_defconfig) and boards/sandbox/vpl.config And passing multiple files with a comma seems easiest.
So is it only possible to add one fragment file to a build?
The example above is two, and yes, N config fragments works (they are merged in listed order).
OK
I see what you are saying, but from my side I am trying to enumerate the boards, since generally I (like) build things without explicitly specifying each board defconfig.
Yes, but that's not possible in this case I think. And I'm really just trying to figure out how we can make CI a little easier. But maybe we can't / don't bother in this case and keep fixing up the sandbox defconfig files as needed.
Maybe...it sounds like you are really just wanting a way for buildman to manually build a single board with some config added, rather than having it detect and build everything that is in-tree?
Regards, Simon

On Tue, Aug 15, 2023 at 08:44:18AM -0600, Simon Glass wrote:
Hi Tom,
On Sun, 13 Aug 2023 at 08:43, Tom Rini trini@konsulko.com wrote:
On Sun, Aug 13, 2023 at 07:36:45AM -0600, Simon Glass wrote:
[snip]
I see what you are saying, but from my side I am trying to enumerate the boards, since generally I (like) build things without explicitly specifying each board defconfig.
Yes, but that's not possible in this case I think. And I'm really just trying to figure out how we can make CI a little easier. But maybe we can't / don't bother in this case and keep fixing up the sandbox defconfig files as needed.
Maybe...it sounds like you are really just wanting a way for buildman to manually build a single board with some config added, rather than having it detect and build everything that is in-tree?
What I'm trying to do really, is since you didn't seem to see the value in config fragments before, showcase how it would be beneficial to sandbox (and CI) if we used them there.

Hi Tom,
On Tue, 15 Aug 2023 at 08:46, Tom Rini trini@konsulko.com wrote:
On Tue, Aug 15, 2023 at 08:44:18AM -0600, Simon Glass wrote:
Hi Tom,
On Sun, 13 Aug 2023 at 08:43, Tom Rini trini@konsulko.com wrote:
On Sun, Aug 13, 2023 at 07:36:45AM -0600, Simon Glass wrote:
[snip]
I see what you are saying, but from my side I am trying to enumerate the boards, since generally I (like) build things without explicitly specifying each board defconfig.
Yes, but that's not possible in this case I think. And I'm really just trying to figure out how we can make CI a little easier. But maybe we can't / don't bother in this case and keep fixing up the sandbox defconfig files as needed.
Maybe...it sounds like you are really just wanting a way for buildman to manually build a single board with some config added, rather than having it detect and build everything that is in-tree?
What I'm trying to do really, is since you didn't seem to see the value in config fragments before, showcase how it would be beneficial to sandbox (and CI) if we used them there.
Yes, don't worry, I am past that stage and I see their value.
But I think for this to be more broadly useful we need a way to enumerate the combinations permissible for a board and give them names. Things like qconfig and buildman need to know what is going on and cannot infer it from the filesystem...?
Regards, Simon

Hi,
On Thu, 10 Aug 2023 at 09:52, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote:
Build the mkeficapsule tool for all the sandbox variants. This tool will be used subsequently for testing capsule generation in binman.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7: None
tools/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/Kconfig b/tools/Kconfig index 6e23f44d55..353a855243 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -91,10 +91,10 @@ config TOOLS_SHA512 Enable SHA512 support in the tools builds
config TOOLS_MKEFICAPSULE
bool "Build efimkcapsule command"
default y if EFI_CAPSULE_ON_DISK
bool "Build mkeficapsule tool"
default y if EFI_CAPSULE_ON_DISK || SANDBOX help
This command allows users to create a UEFI capsule file and,
This tool allows users to create a UEFI capsule file and, optionally sign that file. If you want to enable UEFI capsule update feature on your target, you certainly need this.
Sorry, what is this fixing exactly?
s/command/tool/ is mixed in with this commit, but the main purpose is to enable it on sandbox.
The commit message really should mention both changes.
Regards, Simon

hi Simon,
On Thu, 10 Aug 2023 at 22:57, Simon Glass sjg@chromium.org wrote:
Hi,
On Thu, 10 Aug 2023 at 09:52, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote:
Build the mkeficapsule tool for all the sandbox variants. This tool will be used subsequently for testing capsule generation in binman.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7: None
tools/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/Kconfig b/tools/Kconfig index 6e23f44d55..353a855243 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -91,10 +91,10 @@ config TOOLS_SHA512 Enable SHA512 support in the tools builds
config TOOLS_MKEFICAPSULE
bool "Build efimkcapsule command"
default y if EFI_CAPSULE_ON_DISK
bool "Build mkeficapsule tool"
default y if EFI_CAPSULE_ON_DISK || SANDBOX help
This command allows users to create a UEFI capsule file and,
This tool allows users to create a UEFI capsule file and, optionally sign that file. If you want to enable UEFI capsule update feature on your target, you certainly need this.
Sorry, what is this fixing exactly?
s/command/tool/ is mixed in with this commit, but the main purpose is to enable it on sandbox.
Sorry, I did not understand this statement. The changes made here are using the same nomenclature(tool) for referring to mkeficapsule.
The commit message really should mention both changes.
Which two changes? The commit message states what the commit is doing, and then states the reason for the change. What more information is needed in the commit message?
-sughosh

Hi Sughosh,
On Fri, 11 Aug 2023 at 05:23, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Thu, 10 Aug 2023 at 22:57, Simon Glass sjg@chromium.org wrote:
Hi,
On Thu, 10 Aug 2023 at 09:52, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote:
Build the mkeficapsule tool for all the sandbox variants. This tool will be used subsequently for testing capsule generation in binman.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7: None
tools/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/Kconfig b/tools/Kconfig index 6e23f44d55..353a855243 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -91,10 +91,10 @@ config TOOLS_SHA512 Enable SHA512 support in the tools builds
config TOOLS_MKEFICAPSULE
bool "Build efimkcapsule command"
default y if EFI_CAPSULE_ON_DISK
bool "Build mkeficapsule tool"
default y if EFI_CAPSULE_ON_DISK || SANDBOX help
This command allows users to create a UEFI capsule file and,
This tool allows users to create a UEFI capsule file and, optionally sign that file. If you want to enable UEFI capsule update feature on your target, you certainly need this.
Sorry, what is this fixing exactly?
s/command/tool/ is mixed in with this commit, but the main purpose is to enable it on sandbox.
Sorry, I did not understand this statement. The changes made here are using the same nomenclature(tool) for referring to mkeficapsule.
The commit message really should mention both changes.
Which two changes? The commit message states what the commit is doing, and then states the reason for the change. What more information is needed in the commit message?
The two changes are:
1. The one the commit message mentions 2. Changing 'command' to 'tool' in the Kconfig
Regards, Simon

hi Simon,
On Fri, 11 Aug 2023 at 19:07, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Fri, 11 Aug 2023 at 05:23, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Thu, 10 Aug 2023 at 22:57, Simon Glass sjg@chromium.org wrote:
Hi,
On Thu, 10 Aug 2023 at 09:52, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote:
Build the mkeficapsule tool for all the sandbox variants. This tool will be used subsequently for testing capsule generation in binman.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7: None
tools/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/Kconfig b/tools/Kconfig index 6e23f44d55..353a855243 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -91,10 +91,10 @@ config TOOLS_SHA512 Enable SHA512 support in the tools builds
config TOOLS_MKEFICAPSULE
bool "Build efimkcapsule command"
default y if EFI_CAPSULE_ON_DISK
bool "Build mkeficapsule tool"
default y if EFI_CAPSULE_ON_DISK || SANDBOX help
This command allows users to create a UEFI capsule file and,
This tool allows users to create a UEFI capsule file and, optionally sign that file. If you want to enable UEFI capsule update feature on your target, you certainly need this.
Sorry, what is this fixing exactly?
s/command/tool/ is mixed in with this commit, but the main purpose is to enable it on sandbox.
Sorry, I did not understand this statement. The changes made here are using the same nomenclature(tool) for referring to mkeficapsule.
The commit message really should mention both changes.
Which two changes? The commit message states what the commit is doing, and then states the reason for the change. What more information is needed in the commit message?
The two changes are:
- The one the commit message mentions
- Changing 'command' to 'tool' in the Kconfig
Okay, will put in a mention for the second point as well. Thanks.
-sughosh

On Fri, Aug 11, 2023 at 07:54:11PM +0530, Sughosh Ganu wrote:
hi Simon,
On Fri, 11 Aug 2023 at 19:07, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Fri, 11 Aug 2023 at 05:23, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Thu, 10 Aug 2023 at 22:57, Simon Glass sjg@chromium.org wrote:
Hi,
On Thu, 10 Aug 2023 at 09:52, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote:
Build the mkeficapsule tool for all the sandbox variants. This tool will be used subsequently for testing capsule generation in binman.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7: None
tools/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/Kconfig b/tools/Kconfig index 6e23f44d55..353a855243 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -91,10 +91,10 @@ config TOOLS_SHA512 Enable SHA512 support in the tools builds
config TOOLS_MKEFICAPSULE
bool "Build efimkcapsule command"
default y if EFI_CAPSULE_ON_DISK
bool "Build mkeficapsule tool"
default y if EFI_CAPSULE_ON_DISK || SANDBOX help
This command allows users to create a UEFI capsule file and,
This tool allows users to create a UEFI capsule file and, optionally sign that file. If you want to enable UEFI capsule update feature on your target, you certainly need this.
Sorry, what is this fixing exactly?
s/command/tool/ is mixed in with this commit, but the main purpose is to enable it on sandbox.
Sorry, I did not understand this statement. The changes made here are using the same nomenclature(tool) for referring to mkeficapsule.
The commit message really should mention both changes.
Which two changes? The commit message states what the commit is doing, and then states the reason for the change. What more information is needed in the commit message?
The two changes are:
- The one the commit message mentions
- Changing 'command' to 'tool' in the Kconfig
Okay, will put in a mention for the second point as well. Thanks.
There is another use of 'command' in CONFIG_TOOLS_MKFWUDATA. So it would be better to separate (2) from (1) and to fix both 'command'.
-Takahiro Akashi
-sughosh

Add a bintool for generating EFI capsules. This calls the mkeficapsule tool which generates the capsules.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- Changes since V7: None
tools/binman/btool/mkeficapsule.py | 101 +++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 tools/binman/btool/mkeficapsule.py
diff --git a/tools/binman/btool/mkeficapsule.py b/tools/binman/btool/mkeficapsule.py new file mode 100644 index 0000000000..61179747ff --- /dev/null +++ b/tools/binman/btool/mkeficapsule.py @@ -0,0 +1,101 @@ +# 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 commandline options to be provided +to the tool +Usage: mkeficapsule [options] <image blob> <output file> +Options: + -g, --guid <guid string> guid for image blob type + -i, --index <index> update image index + -I, --instance <instance> update hardware instance + -v, --fw-version <version> firmware version + -p, --private-key <privkey file> private key file + -c, --certificate <cert file> signer's certificate file + -m, --monotonic-count <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 0x0000 and 0xffff + -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 + commandline, or through a config file. + """ + def __init__(self, name): + super().__init__(name, 'mkeficapsule tool for generating capsules') + + def generate_capsule(self, image_index, image_guid, hardware_instance, + payload, output_fname, priv_key, pub_key, + monotonic_count=0, version=0, oemflags=0): + """Generate a capsule through commandline-provided parameters + + Args: + image_index (int): Unique number for identifying payload image + image_guid (str): GUID used for identifying the image + hardware_instance (int): Optional unique hardware instance of + a device in the system. 0 if not being used + payload (str): Path to the input payload image + output_fname (str): Path to the output capsule file + priv_key (str): Path to the private key + pub_key(str): Path to the public key + monotonic_count (int): Count used when signing an image + version (int): Image version (Optional) + oemflags (int): Optional 16 bit OEM flags + + Returns: + str: Tool output + """ + args = [ + f'--index={image_index}', + f'--guid={image_guid}', + f'--instance={hardware_instance}' + ] + + if version: + args += [f'--fw-version={version}'] + if oemflags: + args += [f'--capoemflag={oemflags}'] + if priv_key and pub_key: + args += [ + f'--monotonic-count={monotonic_count}', + f'--private-key={priv_key}', + f'--certificate={pub_key}' + ] + + args += [ + payload, + output_fname + ] + + return 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 + + cmd = ['tools-only_defconfig', 'tools'] + result = self.build_from_git( + 'https://source.denx.de/u-boot/u-boot.git', + cmd, + 'tools/mkeficapsule') + return result

Hi Sughosh,
On Thu, 10 Aug 2023 at 08:24, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add a bintool for generating EFI capsules. This calls the mkeficapsule tool which generates the capsules.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7: None
tools/binman/btool/mkeficapsule.py | 101 +++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 tools/binman/btool/mkeficapsule.py
I think you lost a review tag here...you can use 'patman status' to view and collect them.
Regards, Simon

Add support in binman for generating EFI capsules. The capsule parameters can be specified through the capsule binman entry. Also add test cases in binman for testing capsule generation.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- Changes since V7: * Rebase on top of current upstream. * Drop the ReadEntries method as suggested by Simon Glass. * Add logic to allow specifying a string 'binman-test' for GUIDs in binman tests. * Add a todo comment for getting the capsule contents from the tool.
tools/binman/entries.rst | 64 ++++++++ tools/binman/etype/efi_capsule.py | 143 ++++++++++++++++++ tools/binman/ftest.py | 118 +++++++++++++++ tools/binman/test/311_capsule.dts | 21 +++ tools/binman/test/312_capsule_signed.dts | 23 +++ tools/binman/test/313_capsule_version.dts | 22 +++ tools/binman/test/314_capsule_signed_ver.dts | 24 +++ tools/binman/test/315_capsule_oemflags.dts | 22 +++ tools/binman/test/316_capsule_missing_key.dts | 22 +++ .../binman/test/317_capsule_missing_index.dts | 20 +++ .../binman/test/318_capsule_missing_guid.dts | 19 +++ 11 files changed, 498 insertions(+) create mode 100644 tools/binman/etype/efi_capsule.py create mode 100644 tools/binman/test/311_capsule.dts create mode 100644 tools/binman/test/312_capsule_signed.dts create mode 100644 tools/binman/test/313_capsule_version.dts create mode 100644 tools/binman/test/314_capsule_signed_ver.dts create mode 100644 tools/binman/test/315_capsule_oemflags.dts create mode 100644 tools/binman/test/316_capsule_missing_key.dts create mode 100644 tools/binman/test/317_capsule_missing_index.dts create mode 100644 tools/binman/test/318_capsule_missing_guid.dts
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index e7dfe6b2a3..801bd94674 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -468,6 +468,70 @@ updating the EC on startup via software sync.
+.. _etype_efi_capsule: + +Entry: capsule: Entry for generating EFI Capsule files +------------------------------------------------------ + +The parameters needed for generation of the capsules can be provided +as properties in the entry. + +Properties / Entry arguments: + - image-index: Unique number for identifying corresponding + payload image. Number between 1 and descriptor count, i.e. + the total number of firmware images that can be updated. Mandatory + property. + - image-guid: Image GUID which will be used for identifying the + updatable image on the board. Mandatory property. + - hardware-instance: Optional number for identifying unique + hardware instance of a device in the system. Default value of 0 + for images where value is not to be used. + - fw-version: Value of image version that can be put on the capsule + through the Firmware Management Protocol(FMP) header. + - monotonic-count: Count used when signing an image. + - private-key: Path to PEM formatted .key private key file. Mandatory + property for generating signed capsules. + - public-key-cert: Path to PEM formatted .crt public key certificate + file. Mandatory property for generating signed capsules. + - oem-flags - OEM flags to be passed through capsule header. + + Since this is a subclass of Entry_section, all properties of the parent + class also apply here. Except for the properties stated as mandatory, the + rest of the properties are optional. + +For more details on the description of the capsule format, and the capsule +update functionality, refer Section 8.5 and Chapter 23 in the `UEFI +specification`_. + +The capsule parameters like image index and image GUID are passed as +properties in the entry. The payload to be used in the capsule is to be +provided as a subnode of the capsule entry. + +A typical capsule entry node would then look something like this:: + + capsule { + type = "efi-capsule"; + image-index = <0x1>; + /* Image GUID for testing capsule update */ + image-guid = SANDBOX_UBOOT_IMAGE_GUID; + hardware-instance = <0x0>; + private-key = "path/to/the/private/key"; + public-key-cert = "path/to/the/public-key-cert"; + oem-flags = <0x8000>; + + u-boot { + }; + }; + +In the above example, the capsule payload is the U-Boot image. The +capsule entry would read the contents of the payload and put them +into the capsule. Any external file can also be specified as the +payload using the blob-ext subnode. + +.. _`UEFI specification`: https://uefi.org/sites/default/files/resources/UEFI_Spec_2_10_Aug29.pdf + + + .. _etype_encrypted:
Entry: encrypted: Externally built encrypted binary blob diff --git a/tools/binman/etype/efi_capsule.py b/tools/binman/etype/efi_capsule.py new file mode 100644 index 0000000000..006eb630ad --- /dev/null +++ b/tools/binman/etype/efi_capsule.py @@ -0,0 +1,143 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2023 Linaro Limited +# +# Entry-type module for producing a 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_capsule(Entry_section): + """Generate EFI capsules + + The parameters needed for generation of the capsules can + be provided as properties in the entry. + + Properties / Entry arguments: + - image-index: Unique number for identifying corresponding + payload image. Number between 1 and descriptor count, i.e. + the total number of firmware images that can be updated. Mandatory + property. + - image-guid: Image GUID which will be used for identifying the + updatable image on the board. Mandatory property. + - hardware-instance: Optional number for identifying unique + hardware instance of a device in the system. Default value of 0 + for images where value is not to be used. + - fw-version: Value of image version that can be put on the capsule + through the Firmware Management Protocol(FMP) header. + - monotonic-count: Count used when signing an image. + - private-key: Path to PEM formatted .key private key file. Mandatory + property for generating signed capsules. + - public-key-cert: Path to PEM formatted .crt public key certificate + file. Mandatory property for generating signed capsules. + - oem-flags - OEM flags to be passed through capsule header. + + Since this is a subclass of Entry_section, all properties of the parent + class also apply here. Except for the properties stated as mandatory, the + rest of the properties are optional. + + For more details on the description of the capsule format, and the capsule + update functionality, refer Section 8.5 and Chapter 23 in the `UEFI + specification`_. + + The capsule parameters like image index and image GUID are passed as + properties in the entry. The payload to be used in the capsule is to be + provided as a subnode of the capsule entry. + + A typical capsule entry node would then look something like this + + capsule { + type = "efi-capsule"; + image-index = <0x1>; + /* Image GUID for testing capsule update */ + image-guid = SANDBOX_UBOOT_IMAGE_GUID; + hardware-instance = <0x0>; + private-key = "path/to/the/private/key"; + public-key-cert = "path/to/the/public-key-cert"; + oem-flags = <0x8000>; + + u-boot { + }; + }; + + In the above example, the capsule payload is the U-Boot image. The + capsule entry would read the contents of the payload and put them + into the capsule. Any external file can also be specified as the + payload using the blob-ext subnode. + + .. _`UEFI specification`: https://uefi.org/sites/default/files/resources/UEFI_Spec_2_10_Aug29.pdf + """ + def __init__(self, section, etype, node): + super().__init__(section, etype, node) + self.required_props = ['image-index', 'image-guid'] + self.image_index = 0 + self.image_guid = '' + self.hardware_instance = 0 + self.monotonic_count = 0 + self.fw_version = 0 + self.oem_flags = 0 + self.private_key = '' + self.public_key_cert = '' + self.auth = 0 + + def ReadNode(self): + super().ReadNode() + + self.image_index = fdt_util.GetInt(self._node, 'image-index') + self.image_guid = fdt_util.GetString(self._node, 'image-guid') + self.fw_version = fdt_util.GetInt(self._node, 'fw-version') + self.hardware_instance = fdt_util.GetInt(self._node, 'hardware-instance') + self.monotonic_count = fdt_util.GetInt(self._node, 'monotonic-count') + self.oem_flags = fdt_util.GetInt(self._node, 'oem-flags') + + self.private_key = fdt_util.GetString(self._node, 'private-key') + self.public_key_cert = fdt_util.GetString(self._node, 'public-key-cert') + if ((self.private_key and not self.public_key_cert) or (self.public_key_cert and not self.private_key)): + self.Raise('Both private key and public key certificate need to be provided') + elif not (self.private_key and self.public_key_cert): + self.auth = 0 + else: + self.auth = 1 + + def BuildSectionData(self, required): + def get_binman_test_guid(type_str): + TYPE_TO_GUID = { + 'binman-test' : '09d7cf52-0720-4710-91d1-08469b7fe9c8' + } + return TYPE_TO_GUID[type_str] + + private_key = '' + public_key_cert = '' + if self.auth: + if not os.path.isabs(self.private_key): + private_key = tools.get_input_filename(self.private_key) + if not os.path.isabs(self.public_key_cert): + public_key_cert = tools.get_input_filename(self.public_key_cert) + data, payload, uniq = self.collect_contents_to_file( + self._entries.values(), 'capsule_in') + outfile = self._filename if self._filename else 'capsule.%s' % uniq + capsule_fname = tools.get_output_filename(outfile) + guid = self.image_guid + if self.image_guid == "binman-test": + guid = get_binman_test_guid('binman-test') + + ret = self.mkeficapsule.generate_capsule(self.image_index, + guid, + self.hardware_instance, + payload, + capsule_fname, + private_key, + public_key_cert, + self.monotonic_count, + self.fw_version, + self.oem_flags) + if ret is not None: + os.remove(payload) + 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 1293e9dbf4..8e419645a6 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -48,6 +48,7 @@ U_BOOT_VPL_DATA = b'vpl76543210fedcbazywxyz_' BLOB_DATA = b'89' ME_DATA = b'0abcd' VGA_DATA = b'vga' +EFI_CAPSULE_DATA = b'efi' U_BOOT_DTB_DATA = b'udtb' U_BOOT_SPL_DTB_DATA = b'spldtb' U_BOOT_TPL_DTB_DATA = b'tpldtb' @@ -119,6 +120,11 @@ COMP_BINTOOLS = ['bzip2', 'gzip', 'lz4', 'lzma_alone', 'lzop', 'xz', 'zstd']
TEE_ADDR = 0x5678
+# Firmware Management Protocol(FMP) GUID +FW_MGMT_GUID = 'edd5cb6d2de8444cbda17194199ad92a' +# Image GUID specified in the DTS +CAPSULE_IMAGE_GUID = '52cfd7092007104791d108469b7fe9c8' + class TestFunctional(unittest.TestCase): """Functional tests for binman
@@ -215,6 +221,7 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('scp.bin', SCP_DATA) TestFunctional._MakeInputFile('rockchip-tpl.bin', ROCKCHIP_TPL_DATA) TestFunctional._MakeInputFile('ti_unsecure.bin', TI_UNSECURE_DATA) + TestFunctional._MakeInputFile('capsule_input.bin', EFI_CAPSULE_DATA)
# Add a few .dtb files for testing TestFunctional._MakeInputFile('%s/test-fdt1.dtb' % TEST_FDT_SUBDIR, @@ -7216,5 +7223,116 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertRegex(err, "Image 'image'.*missing bintools.*: bootgen")
+ def _CheckCapsule(self, data, signed_capsule=False, version_check=False, + capoemflags=False): + fmp_signature = "4d535331" # 'M', 'S', 'S', '1' + fmp_size = "10" + fmp_fw_version = "02" + oemflag = "0080" + + payload_data = EFI_CAPSULE_DATA + + # TODO - Currently, these offsets for capsule fields are hardcoded. + # There are plans to add support to the mkeficapsule tool to dump + # the capsule contents which can then be used for capsule + # verification. + + # Firmware Management Protocol(FMP) GUID - offset(0 - 32) + self.assertEqual(FW_MGMT_GUID, data.hex()[:32]) + # Image GUID - offset(96 - 128) + self.assertEqual(CAPSULE_IMAGE_GUID, data.hex()[96:128]) + + if capoemflags: + # OEM Flags - offset(40 - 44) + self.assertEqual(oemflag, data.hex()[40:44]) + if signed_capsule and version_check: + # FMP header signature - offset(4770 - 4778) + self.assertEqual(fmp_signature, data.hex()[4770:4778]) + # FMP header size - offset(4778 - 4780) + self.assertEqual(fmp_size, data.hex()[4778:4780]) + # firmware version - offset(4786 - 4788) + self.assertEqual(fmp_fw_version, data.hex()[4786:4788]) + # payload offset signed capsule(4802 - 4808) + self.assertEqual(payload_data.hex(), data.hex()[4802:4808]) + elif signed_capsule: + # payload offset signed capsule(4770 - 4776) + self.assertEqual(payload_data.hex(), data.hex()[4770:4776]) + elif version_check: + # FMP header signature - offset(184 - 192) + self.assertEqual(fmp_signature, data.hex()[184:192]) + # FMP header size - offset(192 - 194) + self.assertEqual(fmp_size, data.hex()[192:194]) + # firmware version - offset(200 - 202) + self.assertEqual(fmp_fw_version, data.hex()[200:202]) + # payload offset for non-signed capsule with version header(216 - 222) + self.assertEqual(payload_data.hex(), data.hex()[216:222]) + else: + # payload offset for non-signed capsule with no version header(184 - 190) + self.assertEqual(payload_data.hex(), data.hex()[184:190]) + + def testCapsuleGen(self): + """Test generation of EFI capsule""" + data = self._DoReadFile('311_capsule.dts') + + self._CheckCapsule(data) + + def testSignedCapsuleGen(self): + """Test generation of EFI capsule""" + data = tools.read_file(self.TestFile("key.key")) + self._MakeInputFile("key.key", data) + data = tools.read_file(self.TestFile("key.pem")) + self._MakeInputFile("key.crt", data) + + data = self._DoReadFile('312_capsule_signed.dts') + + self._CheckCapsule(data, signed_capsule=True) + + def testCapsuleGenVersionSupport(self): + """Test generation of EFI capsule with version support""" + data = self._DoReadFile('313_capsule_version.dts') + + self._CheckCapsule(data, version_check=True) + + def testCapsuleGenSignedVer(self): + """Test generation of signed EFI capsule with version information""" + data = tools.read_file(self.TestFile("key.key")) + self._MakeInputFile("key.key", data) + data = tools.read_file(self.TestFile("key.pem")) + self._MakeInputFile("key.crt", data) + + data = self._DoReadFile('314_capsule_signed_ver.dts') + + self._CheckCapsule(data, signed_capsule=True, version_check=True) + + def testCapsuleGenCapOemFlags(self): + """Test generation of EFI capsule with OEM Flags set""" + data = self._DoReadFile('315_capsule_oemflags.dts') + + self._CheckCapsule(data, capoemflags=True) + + def testCapsuleGenKeyMissing(self): + """Test that binman errors out on missing key""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('316_capsule_missing_key.dts') + + self.assertIn("Both private key and public key certificate need to be provided", + str(e.exception)) + + def testCapsuleGenIndexMissing(self): + """Test that binman errors out on missing image index""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('317_capsule_missing_index.dts') + + self.assertIn("entry is missing properties: image-index", + str(e.exception)) + + def testCapsuleGenGuidMissing(self): + """Test that binman errors out on missing image GUID""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('318_capsule_missing_guid.dts') + + self.assertIn("entry is missing properties: image-guid", + str(e.exception)) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/311_capsule.dts b/tools/binman/test/311_capsule.dts new file mode 100644 index 0000000000..8eb4250b14 --- /dev/null +++ b/tools/binman/test/311_capsule.dts @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + /* Image GUID for testing capsule update */ + image-guid = "binman-test"; + hardware-instance = <0x0>; + + blob { + filename = "capsule_input.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/312_capsule_signed.dts b/tools/binman/test/312_capsule_signed.dts new file mode 100644 index 0000000000..d1c76e269c --- /dev/null +++ b/tools/binman/test/312_capsule_signed.dts @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + /* Image GUID for testing capsule update */ + image-guid = "binman-test"; + hardware-instance = <0x0>; + private-key = "key.key"; + public-key-cert = "key.crt"; + + blob { + filename = "capsule_input.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/313_capsule_version.dts b/tools/binman/test/313_capsule_version.dts new file mode 100644 index 0000000000..bafef3609e --- /dev/null +++ b/tools/binman/test/313_capsule_version.dts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + fw-version = <0x2>; + /* Image GUID for testing capsule update */ + image-guid = "binman-test"; + hardware-instance = <0x0>; + + blob { + filename = "capsule_input.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/314_capsule_signed_ver.dts b/tools/binman/test/314_capsule_signed_ver.dts new file mode 100644 index 0000000000..85c784bba4 --- /dev/null +++ b/tools/binman/test/314_capsule_signed_ver.dts @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + fw-version = <0x2>; + /* Image GUID for testing capsule update */ + image-guid = "binman-test"; + hardware-instance = <0x0>; + private-key = "key.key"; + public-key-cert = "key.crt"; + + blob { + filename = "capsule_input.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/315_capsule_oemflags.dts b/tools/binman/test/315_capsule_oemflags.dts new file mode 100644 index 0000000000..f736e8758f --- /dev/null +++ b/tools/binman/test/315_capsule_oemflags.dts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + /* Image GUID for testing capsule update */ + image-guid = "binman-test"; + hardware-instance = <0x0>; + oem-flags = <0x8000>; + + blob { + filename = "capsule_input.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/316_capsule_missing_key.dts b/tools/binman/test/316_capsule_missing_key.dts new file mode 100644 index 0000000000..2080b50e3d --- /dev/null +++ b/tools/binman/test/316_capsule_missing_key.dts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + /* Image GUID for testing capsule update */ + image-guid = "binman-test"; + hardware-instance = <0x0>; + private-key = "tools/binman/test/key.key"; + + blob { + filename = "capsule_input.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/317_capsule_missing_index.dts b/tools/binman/test/317_capsule_missing_index.dts new file mode 100644 index 0000000000..aadb61f647 --- /dev/null +++ b/tools/binman/test/317_capsule_missing_index.dts @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + /* Image GUID for testing capsule update */ + image-guid = "binman-test"; + hardware-instance = <0x0>; + + blob { + filename = "capsule_input.bin"; + }; + }; + }; +}; diff --git a/tools/binman/test/318_capsule_missing_guid.dts b/tools/binman/test/318_capsule_missing_guid.dts new file mode 100644 index 0000000000..d76afba853 --- /dev/null +++ b/tools/binman/test/318_capsule_missing_guid.dts @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + efi-capsule { + image-index = <0x1>; + hardware-instance = <0x0>; + + blob { + filename = "capsule_input.bin"; + }; + }; + }; +};

On Thu, 10 Aug 2023 at 08:24, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add support in binman for generating EFI capsules. The capsule parameters can be specified through the capsule binman entry. Also add test cases in binman for testing capsule generation.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7:
- Rebase on top of current upstream.
- Drop the ReadEntries method as suggested by Simon Glass.
- Add logic to allow specifying a string 'binman-test' for GUIDs in binman tests.
- Add a todo comment for getting the capsule contents from the tool.
tools/binman/entries.rst | 64 ++++++++ tools/binman/etype/efi_capsule.py | 143 ++++++++++++++++++ tools/binman/ftest.py | 118 +++++++++++++++ tools/binman/test/311_capsule.dts | 21 +++ tools/binman/test/312_capsule_signed.dts | 23 +++ tools/binman/test/313_capsule_version.dts | 22 +++ tools/binman/test/314_capsule_signed_ver.dts | 24 +++ tools/binman/test/315_capsule_oemflags.dts | 22 +++ tools/binman/test/316_capsule_missing_key.dts | 22 +++ .../binman/test/317_capsule_missing_index.dts | 20 +++ .../binman/test/318_capsule_missing_guid.dts | 19 +++ 11 files changed, 498 insertions(+) create mode 100644 tools/binman/etype/efi_capsule.py create mode 100644 tools/binman/test/311_capsule.dts create mode 100644 tools/binman/test/312_capsule_signed.dts create mode 100644 tools/binman/test/313_capsule_version.dts create mode 100644 tools/binman/test/314_capsule_signed_ver.dts create mode 100644 tools/binman/test/315_capsule_oemflags.dts create mode 100644 tools/binman/test/316_capsule_missing_key.dts create mode 100644 tools/binman/test/317_capsule_missing_index.dts create mode 100644 tools/binman/test/318_capsule_missing_guid.dts
Reviewed-by: Simon Glass sjg@chromium.org

The EFI capsule files can now be generated as part of u-boot build through binman. Add capsule entry nodes for the sandbox architecture for generating the capsules. These capsules are then used for testing the EFI capsule update functionality on the sandbox platforms.
Remove the corresponding logic in the test setup which was used for generation of these capsule which is now superfluous.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- Changes since V7: * Move the capsule generation logic to sandbox_capsule.dtsi and include that explicitly in test.dts and sandbox.dts. * Drop the u-boot.dtsi file which kept the capsule and signature nodes. * Remove capsule generation logic from capsule update test setup. * Keep the logic to embed the public key in DTB in the test setup.
arch/sandbox/dts/sandbox.dts | 4 + arch/sandbox/dts/sandbox_capsule.dtsi | 340 ++++++++++++++++++ arch/sandbox/dts/test.dts | 4 + include/sandbox_efi_capsule.h | 21 ++ test/py/tests/test_efi_capsule/conftest.py | 155 +------- .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- 6 files changed, 385 insertions(+), 175 deletions(-) create mode 100644 arch/sandbox/dts/sandbox_capsule.dtsi create mode 100644 include/sandbox_efi_capsule.h delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts index 12d3eff5fa..8d234ed216 100644 --- a/arch/sandbox/dts/sandbox.dts +++ b/arch/sandbox/dts/sandbox.dts @@ -106,3 +106,7 @@ #if IS_ENABLED(CONFIG_SUPPORT_VPL) #include "sandbox_vpl.dtsi" #endif + +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT +#include "sandbox_capsule.dtsi" +#endif diff --git a/arch/sandbox/dts/sandbox_capsule.dtsi b/arch/sandbox/dts/sandbox_capsule.dtsi new file mode 100644 index 0000000000..c38fbd33b1 --- /dev/null +++ b/arch/sandbox/dts/sandbox_capsule.dtsi @@ -0,0 +1,340 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Devicetree file with binman nodes for generating capsules. + */ + +#include <sandbox_efi_capsule.h> + +/ { + binman: binman { + multiple-images; + }; +}; + +&binman { + itb { + filename = UBOOT_FIT_IMAGE; + + fit { + description = "Automatic U-Boot environment update"; + #address-cells = <2>; + + images { + u-boot-bin { + description = "U-Boot binary on SPI Flash"; + compression = "none"; + type = "firmware"; + arch = "sandbox"; + load = <0>; + text { + text = "u-boot:New"; + }; + + hash-1 { + algo = "sha1"; + }; + }; + u-boot-env { + description = "U-Boot environment on SPI Flash"; + compression = "none"; + type = "firmware"; + arch = "sandbox"; + load = <0>; + text { + text = "u-boot-env:New"; + }; + + hash-1 { + algo = "sha1"; + }; + }; + }; + }; + }; + + capsule1 { + filename = "Test01"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-guid = SANDBOX_UBOOT_IMAGE_GUID; + + text { + text = "u-boot:New"; + }; + }; + }; + + capsule2 { + filename = "Test02"; + capsule { + type = "efi-capsule"; + image-index = <0x2>; + image-guid = SANDBOX_UBOOT_ENV_IMAGE_GUID; + + text { + text = "u-boot-env:New"; + }; + }; + }; + + capsule3 { + filename = "Test03"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-guid = SANDBOX_INCORRECT_GUID; + + text { + text = "u-boot:New"; + }; + }; + }; + + capsule4 { + filename = "Test04"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-guid = SANDBOX_FIT_IMAGE_GUID; + + blob { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule5 { + filename = "Test05"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-guid = SANDBOX_INCORRECT_GUID; + + blob { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule6 { + filename = "Test101"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x5>; + image-guid = SANDBOX_UBOOT_IMAGE_GUID; + + text { + text = "u-boot:New"; + }; + }; + }; + + capsule7 { + filename = "Test102"; + capsule { + type = "efi-capsule"; + image-index = <0x2>; + fw-version = <0xa>; + image-guid = SANDBOX_UBOOT_ENV_IMAGE_GUID; + + text { + text = "u-boot-env:New"; + }; + }; + }; + + capsule8 { + filename = "Test103"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x2>; + image-guid = SANDBOX_UBOOT_IMAGE_GUID; + + text { + text = "u-boot:New"; + }; + }; + }; + + capsule9 { + filename = "Test104"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x5>; + image-guid = SANDBOX_FIT_IMAGE_GUID; + + blob { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule10 { + filename = "Test105"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x2>; + image-guid = SANDBOX_FIT_IMAGE_GUID; + + blob { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE + capsule11 { + filename = "Test11"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-guid = SANDBOX_UBOOT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + public-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + text { + text = "u-boot:New"; + }; + }; + }; + + capsule12 { + filename = "Test12"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-guid = SANDBOX_UBOOT_IMAGE_GUID; + private-key = CAPSULE_INVAL_KEY; + public-key-cert = CAPSULE_INVAL_PUB_KEY; + monotonic-count = <0x1>; + + text { + text = "u-boot:New"; + }; + }; + }; + + capsule13 { + filename = "Test13"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-guid = SANDBOX_FIT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + public-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + blob { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule14 { + filename = "Test14"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-guid = SANDBOX_FIT_IMAGE_GUID; + private-key = CAPSULE_INVAL_KEY; + public-key-cert = CAPSULE_INVAL_PUB_KEY; + monotonic-count = <0x1>; + + blob { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule15 { + filename = "Test111"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x5>; + image-guid = SANDBOX_UBOOT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + public-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + text { + text = "u-boot:New"; + }; + }; + }; + + capsule16 { + filename = "Test112"; + capsule { + type = "efi-capsule"; + image-index = <0x2>; + fw-version = <0xa>; + image-guid = SANDBOX_UBOOT_ENV_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + public-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + text { + text = "u-boot-env:New"; + }; + }; + }; + + capsule17 { + filename = "Test113"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x2>; + image-guid = SANDBOX_UBOOT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + public-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + text { + text = "u-boot:New"; + }; + }; + }; + + capsule18 { + filename = "Test114"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x5>; + image-guid = SANDBOX_FIT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + public-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + blob { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule19 { + filename = "Test115"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x2>; + image-guid = SANDBOX_FIT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + public-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + blob { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */ +}; diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index f351d5cb84..acbe0046ad 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -1849,3 +1849,7 @@ #endif
#include "cedit.dtsi" + +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE +#include "sandbox_capsule.dtsi" +#endif diff --git a/include/sandbox_efi_capsule.h b/include/sandbox_efi_capsule.h new file mode 100644 index 0000000000..3e288e8a84 --- /dev/null +++ b/include/sandbox_efi_capsule.h @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2023, Linaro Limited + */ + +#if !defined(_SANDBOX_EFI_CAPSULE_H_) +#define _SANDBOX_EFI_CAPSULE_H_ + +#define SANDBOX_UBOOT_IMAGE_GUID "09d7cf52-0720-4710-91d1-08469b7fe9c8" +#define SANDBOX_UBOOT_ENV_IMAGE_GUID "5a7021f5-fef2-48b4-aaba-832e777418c0" +#define SANDBOX_FIT_IMAGE_GUID "3673b45d-6a7c-46f3-9e60-adabb03f7937" +#define SANDBOX_INCORRECT_GUID "058b7d83-50d5-4c47-a195-60d86ad341c4" + +#define UBOOT_FIT_IMAGE "u-boot_bin_env.itb" + +#define CAPSULE_PRIV_KEY "capsule_priv_key_good.key" +#define CAPSULE_PUB_KEY "capsule_pub_key_good.crt" +#define CAPSULE_INVAL_KEY "capsule_priv_key_bad.key" +#define CAPSULE_INVAL_PUB_KEY "capsule_pub_key_bad.crt" + +#endif /* _SANDBOX_EFI_CAPSULE_H_ */ diff --git a/test/py/tests/test_efi_capsule/conftest.py b/test/py/tests/test_efi_capsule/conftest.py index 054be1ee97..7eead9bc64 100644 --- a/test/py/tests/test_efi_capsule/conftest.py +++ b/test/py/tests/test_efi_capsule/conftest.py @@ -34,15 +34,20 @@ def efi_capsule_data(request, u_boot_config):
capsule_auth_enabled = u_boot_config.buildconfig.get( 'config_efi_capsule_authenticate') + key_dir = u_boot_config.source_dir + '/board/sandbox' 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) + # Get the keys from the board directory + check_call('cp %s/capsule_priv_key_good.key %s/SIGNER.key' + % (key_dir, data_dir), shell=True) + check_call('cp %s/capsule_pub_key_good.crt %s/SIGNER.crt' + % (key_dir, data_dir), shell=True) + check_call('cp %s/capsule_pub_esl_good.esl %s/SIGNER.esl' + % (key_dir, data_dir), shell=True) + + check_call('cp %s/capsule_priv_key_bad.key %s/SIGNER2.key' + % (key_dir, data_dir), shell=True) + check_call('cp %s/capsule_pub_key_bad.crt %s/SIGNER2.crt' + % (key_dir, data_dir), shell=True)
# Update dtb adding capsule certificate check_call('cd %s; ' @@ -54,14 +59,6 @@ def efi_capsule_data(request, u_boot_config): '-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) - # Update dtb to add the version information check_call('cd %s; ' 'cp %s/test/py/tests/test_efi_capsule/version.dts .' @@ -79,132 +76,12 @@ def efi_capsule_data(request, u_boot_config): '-o test_ver.dtb version.dtbo' % (data_dir, u_boot_config.build_dir), shell=True)
- # Create capsule files + + check_call('cp %s/u-boot_bin_env.itb %s ' % (u_boot_config.build_dir, data_dir), shell=True) + check_call('cp %s/Test* %s ' % (u_boot_config.build_dir, data_dir), shell=True) # two regions: one for u-boot.bin and the other for u-boot.env check_call('cd %s; echo -n u-boot:Old > u-boot.bin.old; echo -n u-boot:New > u-boot.bin.new; echo -n u-boot-env:Old > u-boot.env.old; echo -n u-boot-env:New > u-boot.env.new' % data_dir, shell=True) - check_call('sed -e "s?BINFILE1?u-boot.bin.new?" -e "s?BINFILE2?u-boot.env.new?" %s/test/py/tests/test_efi_capsule/uboot_bin_env.its > %s/uboot_bin_env.its' % - (u_boot_config.source_dir, data_dir), - shell=True) - check_call('cd %s; %s/tools/mkimage -f uboot_bin_env.its uboot_bin_env.itb' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 u-boot.bin.new Test01' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 2 --guid 5A7021F5-FEF2-48B4-AABA-832E777418C0 u-boot.env.new Test02' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid 058B7D83-50D5-4C47-A195-60D86AD341C4 u-boot.bin.new Test03' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 uboot_bin_env.itb Test04' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --guid 058B7D83-50D5-4C47-A195-60D86AD341C4 uboot_bin_env.itb Test05' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --fw-version 5 ' - '--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 u-boot.bin.new Test101' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 2 --fw-version 10 ' - '--guid 5A7021F5-FEF2-48B4-AABA-832E777418C0 u-boot.env.new Test102' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --fw-version 2 ' - '--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 u-boot.bin.new Test103' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --fw-version 5 ' - '--guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 uboot_bin_env.itb Test104' % - (data_dir, u_boot_config.build_dir), - shell=True) - check_call('cd %s; %s/tools/mkeficapsule --index 1 --fw-version 2 ' - '--guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 uboot_bin_env.itb Test105' % - (data_dir, u_boot_config.build_dir), - shell=True) - - if capsule_auth_enabled: - # raw firmware signed with proper key - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--private-key SIGNER.key --certificate SIGNER.crt ' - '--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 ' - 'u-boot.bin.new Test11' - % (data_dir, u_boot_config.build_dir), - shell=True) - # raw firmware signed with *mal* key - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--private-key SIGNER2.key ' - '--certificate SIGNER2.crt ' - '--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 ' - 'u-boot.bin.new Test12' - % (data_dir, u_boot_config.build_dir), - shell=True) - # FIT firmware signed with proper key - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--private-key SIGNER.key --certificate SIGNER.crt ' - '--guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 ' - 'uboot_bin_env.itb Test13' - % (data_dir, u_boot_config.build_dir), - shell=True) - # FIT firmware signed with *mal* key - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--private-key SIGNER2.key ' - '--certificate SIGNER2.crt ' - '--guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 ' - 'uboot_bin_env.itb Test14' - % (data_dir, u_boot_config.build_dir), - shell=True) - # raw firmware signed with proper key with version information - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--fw-version 5 ' - '--private-key SIGNER.key --certificate SIGNER.crt ' - '--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 ' - 'u-boot.bin.new Test111' - % (data_dir, u_boot_config.build_dir), - shell=True) - # raw firmware signed with proper key with version information - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 2 --monotonic-count 1 ' - '--fw-version 10 ' - '--private-key SIGNER.key --certificate SIGNER.crt ' - '--guid 5A7021F5-FEF2-48B4-AABA-832E777418C0 ' - 'u-boot.env.new Test112' - % (data_dir, u_boot_config.build_dir), - shell=True) - # raw firmware signed with proper key with lower version information - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--fw-version 2 ' - '--private-key SIGNER.key --certificate SIGNER.crt ' - '--guid 09D7CF52-0720-4710-91D1-08469B7FE9C8 ' - 'u-boot.bin.new Test113' - % (data_dir, u_boot_config.build_dir), - shell=True) - # FIT firmware signed with proper key with version information - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--fw-version 5 ' - '--private-key SIGNER.key --certificate SIGNER.crt ' - '--guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 ' - 'uboot_bin_env.itb Test114' - % (data_dir, u_boot_config.build_dir), - shell=True) - # FIT firmware signed with proper key with lower version information - check_call('cd %s; ' - '%s/tools/mkeficapsule --index 1 --monotonic-count 1 ' - '--fw-version 2 ' - '--private-key SIGNER.key --certificate SIGNER.crt ' - '--guid 3673B45D-6A7C-46F3-9E60-ADABB03F7937 ' - 'uboot_bin_env.itb Test115' - % (data_dir, u_boot_config.build_dir), - shell=True)
# Create a disk image with EFI system partition check_call('virt-make-fs --partition=gpt --size=+1M --type=vfat %s %s' % diff --git a/test/py/tests/test_efi_capsule/uboot_bin_env.its b/test/py/tests/test_efi_capsule/uboot_bin_env.its deleted file mode 100644 index fc65907481..0000000000 --- a/test/py/tests/test_efi_capsule/uboot_bin_env.its +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Automatic software update for U-Boot - * Make sure the flashing addresses ('load' prop) is correct for your board! - */ - -/dts-v1/; - -/ { - description = "Automatic U-Boot environment update"; - #address-cells = <2>; - - images { - u-boot-bin { - description = "U-Boot binary on SPI Flash"; - data = /incbin/("BINFILE1"); - compression = "none"; - type = "firmware"; - arch = "sandbox"; - load = <0>; - hash-1 { - algo = "sha1"; - }; - }; - u-boot-env { - description = "U-Boot environment on SPI Flash"; - data = /incbin/("BINFILE2"); - compression = "none"; - type = "firmware"; - arch = "sandbox"; - load = <0>; - hash-1 { - algo = "sha1"; - }; - }; - }; -};

Hi Sughosh,
On Thu, 10 Aug 2023 at 08:24, Sughosh Ganu sughosh.ganu@linaro.org wrote:
The EFI capsule files can now be generated as part of u-boot build through binman. Add capsule entry nodes for the sandbox architecture for generating the capsules. These capsules are then used for testing the EFI capsule update functionality on the sandbox platforms.
Remove the corresponding logic in the test setup which was used for generation of these capsule which is now superfluous.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7:
- Move the capsule generation logic to sandbox_capsule.dtsi and include that explicitly in test.dts and sandbox.dts.
- Drop the u-boot.dtsi file which kept the capsule and signature nodes.
- Remove capsule generation logic from capsule update test setup.
- Keep the logic to embed the public key in DTB in the test setup.
arch/sandbox/dts/sandbox.dts | 4 + arch/sandbox/dts/sandbox_capsule.dtsi | 340 ++++++++++++++++++ arch/sandbox/dts/test.dts | 4 + include/sandbox_efi_capsule.h | 21 ++ test/py/tests/test_efi_capsule/conftest.py | 155 +------- .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- 6 files changed, 385 insertions(+), 175 deletions(-) create mode 100644 arch/sandbox/dts/sandbox_capsule.dtsi create mode 100644 include/sandbox_efi_capsule.h delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts index 12d3eff5fa..8d234ed216 100644 --- a/arch/sandbox/dts/sandbox.dts +++ b/arch/sandbox/dts/sandbox.dts @@ -106,3 +106,7 @@ #if IS_ENABLED(CONFIG_SUPPORT_VPL) #include "sandbox_vpl.dtsi" #endif
+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT +#include "sandbox_capsule.dtsi" +#endif
I believe you've missed the vpl fix. You need to update sandbox_vpl.dtsi along the lines I indicated, so you don't need the extra #ifdef here.
diff --git a/arch/sandbox/dts/sandbox_capsule.dtsi b/arch/sandbox/dts/sandbox_capsule.dtsi new file mode 100644 index 0000000000..c38fbd33b1 --- /dev/null +++ b/arch/sandbox/dts/sandbox_capsule.dtsi @@ -0,0 +1,340 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Devicetree file with binman nodes for generating capsules.
- */
+#include <sandbox_efi_capsule.h>
+/ {
binman: binman {
multiple-images;
This is what triggers the need for an update. It is just too confusing to have one sandbox build use multiple images and not others.
So this line needs to move to a previous patch:
- add it to sandbox.dts, test.dts - tidy up sandbox_vpl.dtsi
};
+};
+&binman {
itb {
filename = UBOOT_FIT_IMAGE;
fit {
description = "Automatic U-Boot environment update";
#address-cells = <2>;
images {
u-boot-bin {
description = "U-Boot binary on SPI Flash";
compression = "none";
type = "firmware";
arch = "sandbox";
load = <0>;
text {
text = "u-boot:New";
};
hash-1 {
algo = "sha1";
};
};
u-boot-env {
description = "U-Boot environment on SPI Flash";
compression = "none";
type = "firmware";
arch = "sandbox";
load = <0>;
text {
text = "u-boot-env:New";
};
hash-1 {
algo = "sha1";
};
};
};
};
};
capsule1 {
filename = "Test01";
capsule {
type = "efi-capsule";
Can you use efi-capsule for the name, so you can omit this line?
[..]
Regards, Simon

hi Simon,
On Thu, 10 Aug 2023 at 22:57, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Thu, 10 Aug 2023 at 08:24, Sughosh Ganu sughosh.ganu@linaro.org wrote:
The EFI capsule files can now be generated as part of u-boot build through binman. Add capsule entry nodes for the sandbox architecture for generating the capsules. These capsules are then used for testing the EFI capsule update functionality on the sandbox platforms.
Remove the corresponding logic in the test setup which was used for generation of these capsule which is now superfluous.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7:
- Move the capsule generation logic to sandbox_capsule.dtsi and include that explicitly in test.dts and sandbox.dts.
- Drop the u-boot.dtsi file which kept the capsule and signature nodes.
- Remove capsule generation logic from capsule update test setup.
- Keep the logic to embed the public key in DTB in the test setup.
arch/sandbox/dts/sandbox.dts | 4 + arch/sandbox/dts/sandbox_capsule.dtsi | 340 ++++++++++++++++++ arch/sandbox/dts/test.dts | 4 + include/sandbox_efi_capsule.h | 21 ++ test/py/tests/test_efi_capsule/conftest.py | 155 +------- .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- 6 files changed, 385 insertions(+), 175 deletions(-) create mode 100644 arch/sandbox/dts/sandbox_capsule.dtsi create mode 100644 include/sandbox_efi_capsule.h delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts index 12d3eff5fa..8d234ed216 100644 --- a/arch/sandbox/dts/sandbox.dts +++ b/arch/sandbox/dts/sandbox.dts @@ -106,3 +106,7 @@ #if IS_ENABLED(CONFIG_SUPPORT_VPL) #include "sandbox_vpl.dtsi" #endif
+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT +#include "sandbox_capsule.dtsi" +#endif
I believe you've missed the vpl fix. You need to update sandbox_vpl.dtsi along the lines I indicated, so you don't need the extra #ifdef here.
No, I did not miss the fix for the vpl issue that you had suggested. But since we are no longer working with a u-boot.dtsi which gets included for every variant, I put this logic which is similar to what is done for including the sandbox_vpl.dtsi above. My thought behind this was that the capsule generation is not needed on the variants which do not enable capsule support, like sandbox_{spl,vpl} etc. Do you want to build capsules for all variants, including the ones which do not enable capsule update functionality?
-sughosh
diff --git a/arch/sandbox/dts/sandbox_capsule.dtsi b/arch/sandbox/dts/sandbox_capsule.dtsi new file mode 100644 index 0000000000..c38fbd33b1 --- /dev/null +++ b/arch/sandbox/dts/sandbox_capsule.dtsi @@ -0,0 +1,340 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Devicetree file with binman nodes for generating capsules.
- */
+#include <sandbox_efi_capsule.h>
+/ {
binman: binman {
multiple-images;
This is what triggers the need for an update. It is just too confusing to have one sandbox build use multiple images and not others.
So this line needs to move to a previous patch:
- add it to sandbox.dts, test.dts
- tidy up sandbox_vpl.dtsi
};
+};
+&binman {
itb {
filename = UBOOT_FIT_IMAGE;
fit {
description = "Automatic U-Boot environment update";
#address-cells = <2>;
images {
u-boot-bin {
description = "U-Boot binary on SPI Flash";
compression = "none";
type = "firmware";
arch = "sandbox";
load = <0>;
text {
text = "u-boot:New";
};
hash-1 {
algo = "sha1";
};
};
u-boot-env {
description = "U-Boot environment on SPI Flash";
compression = "none";
type = "firmware";
arch = "sandbox";
load = <0>;
text {
text = "u-boot-env:New";
};
hash-1 {
algo = "sha1";
};
};
};
};
};
capsule1 {
filename = "Test01";
capsule {
type = "efi-capsule";
Can you use efi-capsule for the name, so you can omit this line?
[..]
Regards, Simon

Hi Sughosh,
On Thu, 10 Aug 2023 at 12:42, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Thu, 10 Aug 2023 at 22:57, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Thu, 10 Aug 2023 at 08:24, Sughosh Ganu sughosh.ganu@linaro.org wrote:
The EFI capsule files can now be generated as part of u-boot build through binman. Add capsule entry nodes for the sandbox architecture for generating the capsules. These capsules are then used for testing the EFI capsule update functionality on the sandbox platforms.
Remove the corresponding logic in the test setup which was used for generation of these capsule which is now superfluous.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V7:
- Move the capsule generation logic to sandbox_capsule.dtsi and include that explicitly in test.dts and sandbox.dts.
- Drop the u-boot.dtsi file which kept the capsule and signature nodes.
- Remove capsule generation logic from capsule update test setup.
- Keep the logic to embed the public key in DTB in the test setup.
arch/sandbox/dts/sandbox.dts | 4 + arch/sandbox/dts/sandbox_capsule.dtsi | 340 ++++++++++++++++++ arch/sandbox/dts/test.dts | 4 + include/sandbox_efi_capsule.h | 21 ++ test/py/tests/test_efi_capsule/conftest.py | 155 +------- .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- 6 files changed, 385 insertions(+), 175 deletions(-) create mode 100644 arch/sandbox/dts/sandbox_capsule.dtsi create mode 100644 include/sandbox_efi_capsule.h delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts index 12d3eff5fa..8d234ed216 100644 --- a/arch/sandbox/dts/sandbox.dts +++ b/arch/sandbox/dts/sandbox.dts @@ -106,3 +106,7 @@ #if IS_ENABLED(CONFIG_SUPPORT_VPL) #include "sandbox_vpl.dtsi" #endif
+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT +#include "sandbox_capsule.dtsi" +#endif
I believe you've missed the vpl fix. You need to update sandbox_vpl.dtsi along the lines I indicated, so you don't need the extra #ifdef here.
No, I did not miss the fix for the vpl issue that you had suggested. But since we are no longer working with a u-boot.dtsi which gets included for every variant, I put this logic which is similar to what is done for including the sandbox_vpl.dtsi above. My thought behind this was that the capsule generation is not needed on the variants which do not enable capsule support, like sandbox_{spl,vpl} etc. Do you want to build capsules for all variants, including the ones which do not enable capsule update functionality?
Yes please. Sorry I wasn't clear about that.
Regards, Simon

The EFI capsules can now be generated as part of u-boot build, through binman. Highlight these changes in the documentation.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org --- Changess since V7: * Change the name of the file which contains the capsule entry binman nodes.
doc/develop/uefi/uefi.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index a7a41f2fac..93266da45e 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -318,6 +318,9 @@ Run the following command --guid <image GUID> \ <capsule_file_name>
+Capsule with firmware version +***************************** + The UEFI specification does not define the firmware versioning mechanism. EDK II reference implementation inserts the FMP Payload Header right before the payload. It coutains the fw_version and lowest supported version, @@ -345,6 +348,19 @@ add --fw-version option in mkeficapsule tool. If the --fw-version option is not set, FMP Payload Header is not inserted and fw_version is set as 0.
+Capsule Generation through binman +********************************* + +Support has also been added to generate capsules during u-boot build +through binman. This requires the platform's DTB to be populated with +the capsule entry nodes for binman. The capsules then can be generated +by specifying the capsule parameters either through a config file, or +by specifying them as properties in the capsule entry node. + +Check the arch/sandbox/dts/sandbox_capsule.dtsi file for the sandbox +platform as reference for how to generate capsules through binman as +part of u-boot build. + Performing the update *********************

On 10.08.23 16:23, Sughosh Ganu wrote:
The EFI capsules can now be generated as part of u-boot build, through binman. Highlight these changes in the documentation.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Acked-by: Heinrich Schuchardt xypron.glpk@gmx.de
Changess since V7:
Change the name of the file which contains the capsule entry binman nodes.
doc/develop/uefi/uefi.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index a7a41f2fac..93266da45e 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -318,6 +318,9 @@ Run the following command --guid <image GUID> \ <capsule_file_name>
+Capsule with firmware version +*****************************
- The UEFI specification does not define the firmware versioning mechanism. EDK II reference implementation inserts the FMP Payload Header right before the payload. It coutains the fw_version and lowest supported version,
@@ -345,6 +348,19 @@ add --fw-version option in mkeficapsule tool. If the --fw-version option is not set, FMP Payload Header is not inserted and fw_version is set as 0.
+Capsule Generation through binman +*********************************
+Support has also been added to generate capsules during u-boot build +through binman. This requires the platform's DTB to be populated with +the capsule entry nodes for binman. The capsules then can be generated +by specifying the capsule parameters either through a config file, or +by specifying them as properties in the capsule entry node.
+Check the arch/sandbox/dts/sandbox_capsule.dtsi file for the sandbox +platform as reference for how to generate capsules through binman as +part of u-boot build.
- Performing the update

When running the trace test on the sandbox platform, the current size of 16MiB is no longer large enough for capturing the entire trace history, and results in truncation. Use a size of 32MiB for the trace buffer on the sandbox platform while running the trace test.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Simon Glass sjg@chromium.org --- Changes since V7: None
.azure-pipelines.yml | 2 +- .gitlab-ci.yml | 2 +- test/py/tests/test_trace.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 31850ae571..1e937455c6 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -275,7 +275,7 @@ stages: TEST_PY_BD: "sandbox" BUILD_ENV: "FTRACE=1 NO_LTO=1" TEST_PY_TEST_SPEC: "trace" - OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a CONFIG_TRACE_EARLY_SIZE=0x01000000" + OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a CONFIG_TRACE_EARLY_SIZE=0x01000000 -a CONFIG_TRACE_BUFFER_SIZE=0x02000000" coreboot: TEST_PY_BD: "coreboot" TEST_PY_ID: "--id qemu" diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 8010afae95..3e41299658 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -315,7 +315,7 @@ sandbox trace_test.py: TEST_PY_BD: "sandbox" BUILD_ENV: "FTRACE=1 NO_LTO=1" TEST_PY_TEST_SPEC: "trace" - OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a CONFIG_TRACE_EARLY_SIZE=0x01000000" + OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a CONFIG_TRACE_EARLY_SIZE=0x01000000 -a CONFIG_TRACE_BUFFER_SIZE=0x02000000" <<: *buildman_and_testpy_dfn
evb-ast2500 test.py: diff --git a/test/py/tests/test_trace.py b/test/py/tests/test_trace.py index ac3e95925e..ad2250920d 100644 --- a/test/py/tests/test_trace.py +++ b/test/py/tests/test_trace.py @@ -61,7 +61,7 @@ def collect_trace(cons):
# Read out the trace data addr = 0x02000000 - size = 0x01000000 + size = 0x02000000 out = cons.run_command(f'trace calls {addr:x} {size:x}') print(out) fname = os.path.join(TMPDIR, 'trace')
participants (5)
-
Heinrich Schuchardt
-
Simon Glass
-
Sughosh Ganu
-
Takahiro Akashi
-
Tom Rini