| Message ID | 20230805113458.1430239-10-sughosh.ganu@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Integrate EFI capsule tasks into u-boot's build flow | expand |
Hi Sughosh, On Sat, 5 Aug 2023 at 05:35, 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 in the u-boot.dtsi for > the sandbox architecture for generating the capsules. These capsules > are then used for testing the EFI capsule update functionality on the > sandbox platforms. Also add binman nodes for generating the input > files used for these capsules. > > Remove the corresponding logic which was used for generation of these > input files which is now superfluous. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > Changes since V6: > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule > input filenames. > * Generate the capsule input files through binman text entries. > > arch/sandbox/dts/u-boot.dtsi | 363 ++++++++++++++++++ > include/sandbox_efi_capsule.h | 11 + > test/py/tests/test_efi_capsule/conftest.py | 168 +------- > test/py/tests/test_efi_capsule/signature.dts | 10 - > .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- > 5 files changed, 385 insertions(+), 203 deletions(-) > delete mode 100644 test/py/tests/test_efi_capsule/signature.dts > delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its I think you are still getting confused with using filenames when you don't need to. See below... > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi > index 60bd004937..f1b16b41c2 100644 > --- a/arch/sandbox/dts/u-boot.dtsi > +++ b/arch/sandbox/dts/u-boot.dtsi > @@ -7,11 +7,374 @@ > */ > > #ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT > + > +#include <sandbox_efi_capsule.h> > + > / { > #ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE > signature { > capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE); > }; > #endif > + > + binman: binman { > + multiple-images; > + }; > +}; > + > +&binman { > + input1 { > + filename = UBOOT_BIN_IMAGE_OLD; > + text { > + text = "u-boot:Old"; > + }; > + }; > + > + input2 { > + filename = UBOOT_BIN_IMAGE_NEW; > + text { > + text = "u-boot:New"; > + }; > + }; > + > + input3 { > + filename = UBOOT_ENV_IMAGE_OLD; > + text { > + text = "u-boot-env:Old"; > + }; > + }; > + > + input4 { > + filename = UBOOT_ENV_IMAGE_NEW; > + text { > + text = "u-boot-env:New"; > + }; > + }; All of those nodes can be removed > + > + 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>; > + blob { > + filename = UBOOT_BIN_IMAGE_NEW; > + }; instead of this blob node, you should be able to write: text { text = "u-boot:Old"; }; There is no need to provide filenames in every node. > + > + hash-1 { > + algo = "sha1"; > + }; > + }; > + u-boot-env { > + description = "U-Boot environment on SPI Flash"; > + compression = "none"; > + type = "firmware"; > + arch = "sandbox"; > + load = <0>; > + blob { > + filename = UBOOT_ENV_IMAGE_NEW; > + }; same here and below > + > + hash-1 { > + algo = "sha1"; > + }; > + }; > + }; > + }; > + }; > + > + capsule1 { > + filename = "Test01"; > + capsule { > + type = "efi-capsule"; > + image-index = <0x1>; > + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; > + > + blob { > + filename = UBOOT_BIN_IMAGE_NEW; > + }; again this should be a text node same below > + }; > + }; > + > + capsule2 { > + filename = "Test02"; > + capsule { > + type = "efi-capsule"; > + image-index = <0x2>; > + image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID; > + > + blob { > + filename = UBOOT_ENV_IMAGE_NEW; > + }; > + }; > + }; > + > + capsule3 { > + filename = "Test03"; > + capsule { > + type = "efi-capsule"; > + image-index = <0x1>; > + image-type-id = SANDBOX_INCORRECT_GUID; > + > + blob { > + filename = UBOOT_BIN_IMAGE_NEW; > + }; > + }; > + }; > + > + capsule4 { > + filename = "Test04"; > + capsule { > + type = "efi-capsule"; > + image-index = <0x1>; > + image-type-id = SANDBOX_FIT_IMAGE_GUID; > + > + blob { > + filename = [..] > + capsule19 { > + filename = "Test115"; [..] We really need to talk about these tests. So many cases! Can you not reduce this? Regards, Simon
hi Simon, On Sat, 5 Aug 2023 at 20:34, Simon Glass <sjg@chromium.org> wrote: > > Hi Sughosh, > > On Sat, 5 Aug 2023 at 05:35, 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 in the u-boot.dtsi for > > the sandbox architecture for generating the capsules. These capsules > > are then used for testing the EFI capsule update functionality on the > > sandbox platforms. Also add binman nodes for generating the input > > files used for these capsules. > > > > Remove the corresponding logic which was used for generation of these > > input files which is now superfluous. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > --- > > Changes since V6: > > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule > > input filenames. > > * Generate the capsule input files through binman text entries. > > > > arch/sandbox/dts/u-boot.dtsi | 363 ++++++++++++++++++ > > include/sandbox_efi_capsule.h | 11 + > > test/py/tests/test_efi_capsule/conftest.py | 168 +------- > > test/py/tests/test_efi_capsule/signature.dts | 10 - > > .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- > > 5 files changed, 385 insertions(+), 203 deletions(-) > > delete mode 100644 test/py/tests/test_efi_capsule/signature.dts > > delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its > > I think you are still getting confused with using filenames when you > don't need to. See below... No, I don't think so. This is being done on purpose, since I want these files to be created. These are then copied to the efi capsule update tests' data directory, and are then used in testing the feature. Maybe if you want, I can put a comment to that effect. > > > > > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi > > index 60bd004937..f1b16b41c2 100644 > > --- a/arch/sandbox/dts/u-boot.dtsi > > +++ b/arch/sandbox/dts/u-boot.dtsi > > @@ -7,11 +7,374 @@ > > */ > > > > #ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT > > + > > +#include <sandbox_efi_capsule.h> > > + > > / { > > #ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE > > signature { > > capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE); > > }; > > #endif > > + > > + binman: binman { > > + multiple-images; > > + }; > > +}; > > + > > +&binman { > > + input1 { > > + filename = UBOOT_BIN_IMAGE_OLD; > > + text { > > + text = "u-boot:Old"; > > + }; > > + }; > > + > > + input2 { > > + filename = UBOOT_BIN_IMAGE_NEW; > > + text { > > + text = "u-boot:New"; > > + }; > > + }; > > + > > + input3 { > > + filename = UBOOT_ENV_IMAGE_OLD; > > + text { > > + text = "u-boot-env:Old"; > > + }; > > + }; > > + > > + input4 { > > + filename = UBOOT_ENV_IMAGE_NEW; > > + text { > > + text = "u-boot-env:New"; > > + }; > > + }; > > All of those nodes can be removed > > > + > > + 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>; > > + blob { > > + filename = UBOOT_BIN_IMAGE_NEW; > > + }; > > instead of this blob node, you should be able to write: > > text { > text = "u-boot:Old"; > }; > > There is no need to provide filenames in every node. I know, please check above. > > > + > > + hash-1 { > > + algo = "sha1"; > > + }; > > + }; > > + u-boot-env { > > + description = "U-Boot environment on SPI Flash"; > > + compression = "none"; > > + type = "firmware"; > > + arch = "sandbox"; > > + load = <0>; > > + blob { > > + filename = UBOOT_ENV_IMAGE_NEW; > > + }; > > same here and below > > > + > > + hash-1 { > > + algo = "sha1"; > > + }; > > + }; > > + }; > > + }; > > + }; > > + > > + capsule1 { > > + filename = "Test01"; > > + capsule { > > + type = "efi-capsule"; > > + image-index = <0x1>; > > + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; > > + > > + blob { > > + filename = UBOOT_BIN_IMAGE_NEW; > > + }; > > again this should be a text node > > same below > > > + }; > > + }; > > + > > + capsule2 { > > + filename = "Test02"; > > + capsule { > > + type = "efi-capsule"; > > + image-index = <0x2>; > > + image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID; > > + > > + blob { > > + filename = UBOOT_ENV_IMAGE_NEW; > > + }; > > + }; > > + }; > > + > > + capsule3 { > > + filename = "Test03"; > > + capsule { > > + type = "efi-capsule"; > > + image-index = <0x1>; > > + image-type-id = SANDBOX_INCORRECT_GUID; > > + > > + blob { > > + filename = UBOOT_BIN_IMAGE_NEW; > > + }; > > + }; > > + }; > > + > > + capsule4 { > > + filename = "Test04"; > > + capsule { > > + type = "efi-capsule"; > > + image-index = <0x1>; > > + image-type-id = SANDBOX_FIT_IMAGE_GUID; > > + > > + blob { > > + filename = > [..] > > > + capsule19 { > > + filename = "Test115"; > > [..] > > We really need to talk about these tests. So many cases! Can you not > reduce this? Unfortunately no. These capsules are all needed for the feature testing. Which is why I was asking yesterday if you wanted these generated for normal builds as well, or only for CI runs. -sughosh
Hi Sughosh, On Sat, 5 Aug 2023 at 12:01, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > hi Simon, > > On Sat, 5 Aug 2023 at 20:34, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Sat, 5 Aug 2023 at 05:35, 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 in the u-boot.dtsi for > > > the sandbox architecture for generating the capsules. These capsules > > > are then used for testing the EFI capsule update functionality on the > > > sandbox platforms. Also add binman nodes for generating the input > > > files used for these capsules. > > > > > > Remove the corresponding logic which was used for generation of these > > > input files which is now superfluous. > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > --- > > > Changes since V6: > > > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule > > > input filenames. > > > * Generate the capsule input files through binman text entries. > > > > > > arch/sandbox/dts/u-boot.dtsi | 363 ++++++++++++++++++ > > > include/sandbox_efi_capsule.h | 11 + > > > test/py/tests/test_efi_capsule/conftest.py | 168 +------- > > > test/py/tests/test_efi_capsule/signature.dts | 10 - > > > .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- > > > 5 files changed, 385 insertions(+), 203 deletions(-) > > > delete mode 100644 test/py/tests/test_efi_capsule/signature.dts > > > delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its > > > > I think you are still getting confused with using filenames when you > > don't need to. See below... > > No, I don't think so. This is being done on purpose, since I want > these files to be created. These are then copied to the efi capsule > update tests' data directory, and are then used in testing the > feature. Maybe if you want, I can put a comment to that effect. I just traced through this code. I really think this needs to be simplified. You can do it in a patch at the end if you like. But you have the 'u-boot.bin.old' and 'Old' strings in test_efi_capsule_auth2, for example. In the binman world you define UBOOT_BIN_IMAGE_OLD as "u-boot.bin.old", then use that in the sandbox.dtsi Then binman creates the u-boot.bin.old file (unnecessarily in my view) Then in efi_capsule_data() you copy the file to the test directory. So for the last step, you could just create the file again, rather than copying it from the U-Boot build directory. After all, you know the contents. If you like you could put the different contents as binary strings in capsule_defs.py Then you don't need to create the files. There is a lot more I can say about the EFI capsule tests. For now I think we'll have to live with it creating 19 different binman images on sandbox. I think we could put them in an efi_capsules subdir, but that would need to be created somewhere, since binman doesn't do it. I suppose we could make binman automatically create a directory if an entry filename needs it? Anyway, for tests, primarily we need to split things up, so we have, for example: process_capsule_file() which processes the capsule in U-Boot, e.g. using an 'efi' command, then outputs what it did. Then the test can plant the capsule, run that function and check that the output is as expected. This can actually be a unit test, i.e. written in C. Most of the tests can do this. Then we can have one test that checks the whole flow, but it doesn't need to be done for every case, as now. Regards, Simon
hi Simon, On Sun, 6 Aug 2023 at 00:06, Simon Glass <sjg@chromium.org> wrote: > > Hi Sughosh, > > On Sat, 5 Aug 2023 at 12:01, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > hi Simon, > > > > On Sat, 5 Aug 2023 at 20:34, Simon Glass <sjg@chromium.org> wrote: > > > > > > Hi Sughosh, > > > > > > On Sat, 5 Aug 2023 at 05:35, 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 in the u-boot.dtsi for > > > > the sandbox architecture for generating the capsules. These capsules > > > > are then used for testing the EFI capsule update functionality on the > > > > sandbox platforms. Also add binman nodes for generating the input > > > > files used for these capsules. > > > > > > > > Remove the corresponding logic which was used for generation of these > > > > input files which is now superfluous. > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > --- > > > > Changes since V6: > > > > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule > > > > input filenames. > > > > * Generate the capsule input files through binman text entries. > > > > > > > > arch/sandbox/dts/u-boot.dtsi | 363 ++++++++++++++++++ > > > > include/sandbox_efi_capsule.h | 11 + > > > > test/py/tests/test_efi_capsule/conftest.py | 168 +------- > > > > test/py/tests/test_efi_capsule/signature.dts | 10 - > > > > .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- > > > > 5 files changed, 385 insertions(+), 203 deletions(-) > > > > delete mode 100644 test/py/tests/test_efi_capsule/signature.dts > > > > delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its > > > > > > I think you are still getting confused with using filenames when you > > > don't need to. See below... > > > > No, I don't think so. This is being done on purpose, since I want > > these files to be created. These are then copied to the efi capsule > > update tests' data directory, and are then used in testing the > > feature. Maybe if you want, I can put a comment to that effect. > > I just traced through this code. I really think this needs to be > simplified. You can do it in a patch at the end if you like. > > But you have the 'u-boot.bin.old' and 'Old' strings in > test_efi_capsule_auth2, for example. > > In the binman world you define UBOOT_BIN_IMAGE_OLD as > "u-boot.bin.old", then use that in the sandbox.dtsi > > Then binman creates the u-boot.bin.old file (unnecessarily in my view) > Then in efi_capsule_data() you copy the file to the test directory. > > So for the last step, you could just create the file again, rather > than copying it from the U-Boot build directory. After all, you know > the contents. If you like you could put the different contents as > binary strings in capsule_defs.py > > Then you don't need to create the files. Okay. TBH, I thought you would ask me to reuse the files created in binman for the tests as well, which is why I put this logic. I will create these files as part of the tests then. > > There is a lot more I can say about the EFI capsule tests. For now I > think we'll have to live with it creating 19 different binman images > on sandbox. I think we could put them in an efi_capsules subdir, but > that would need to be created somewhere, since binman doesn't do it. I > suppose we could make binman automatically create a directory if an > entry filename needs it? I think this can be taken up as a follow-up. I also was thinking of adding a flag/property to not generate all the map files. I don't think such a property exists currently. The map files really are not needed for the capsules. > > Anyway, for tests, primarily we need to split things up, so we have, > for example: > > process_capsule_file() > > which processes the capsule in U-Boot, e.g. using an 'efi' command, > then outputs what it did. Then the test can plant the capsule, run > that function and check that the output is as expected. This can > actually be a unit test, i.e. written in C. > > Most of the tests can do this. Then we can have one test that checks > the whole flow, but it doesn't need to be done for every case, as now. I believe even Ilias thinks that these tests should be in C. But this is a separate effort, not related to this series. I also have doubts about your observation on not using so many capsule files for tests, but that can be investigated separately. For now, I want to focus on getting these changes in, followed by the generation of capsules through the config file. And FWIW, I am able to use relative paths in the binman tests for generating and testing the capsules generated through the config file -- so that takes care of your objection to using absolute paths. I will send that series once this gets merged. -sughosh > > Regards, > Simon
Hi Sughosh, On Sat, 5 Aug 2023 at 13:12, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > hi Simon, > > On Sun, 6 Aug 2023 at 00:06, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Sat, 5 Aug 2023 at 12:01, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > hi Simon, > > > > > > On Sat, 5 Aug 2023 at 20:34, Simon Glass <sjg@chromium.org> wrote: > > > > > > > > Hi Sughosh, > > > > > > > > On Sat, 5 Aug 2023 at 05:35, 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 in the u-boot.dtsi for > > > > > the sandbox architecture for generating the capsules. These capsules > > > > > are then used for testing the EFI capsule update functionality on the > > > > > sandbox platforms. Also add binman nodes for generating the input > > > > > files used for these capsules. > > > > > > > > > > Remove the corresponding logic which was used for generation of these > > > > > input files which is now superfluous. > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > > --- > > > > > Changes since V6: > > > > > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule > > > > > input filenames. > > > > > * Generate the capsule input files through binman text entries. > > > > > > > > > > arch/sandbox/dts/u-boot.dtsi | 363 ++++++++++++++++++ > > > > > include/sandbox_efi_capsule.h | 11 + > > > > > test/py/tests/test_efi_capsule/conftest.py | 168 +------- > > > > > test/py/tests/test_efi_capsule/signature.dts | 10 - > > > > > .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- > > > > > 5 files changed, 385 insertions(+), 203 deletions(-) > > > > > delete mode 100644 test/py/tests/test_efi_capsule/signature.dts > > > > > delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its > > > > > > > > I think you are still getting confused with using filenames when you > > > > don't need to. See below... > > > > > > No, I don't think so. This is being done on purpose, since I want > > > these files to be created. These are then copied to the efi capsule > > > update tests' data directory, and are then used in testing the > > > feature. Maybe if you want, I can put a comment to that effect. > > > > I just traced through this code. I really think this needs to be > > simplified. You can do it in a patch at the end if you like. > > > > But you have the 'u-boot.bin.old' and 'Old' strings in > > test_efi_capsule_auth2, for example. > > > > In the binman world you define UBOOT_BIN_IMAGE_OLD as > > "u-boot.bin.old", then use that in the sandbox.dtsi > > > > Then binman creates the u-boot.bin.old file (unnecessarily in my view) > > Then in efi_capsule_data() you copy the file to the test directory. > > > > So for the last step, you could just create the file again, rather > > than copying it from the U-Boot build directory. After all, you know > > the contents. If you like you could put the different contents as > > binary strings in capsule_defs.py > > > > Then you don't need to create the files. > > Okay. TBH, I thought you would ask me to reuse the files created in > binman for the tests as well, which is why I put this logic. I will > create these files as part of the tests then. > > > > > There is a lot more I can say about the EFI capsule tests. For now I > > think we'll have to live with it creating 19 different binman images > > on sandbox. I think we could put them in an efi_capsules subdir, but > > that would need to be created somewhere, since binman doesn't do it. I > > suppose we could make binman automatically create a directory if an > > entry filename needs it? > > I think this can be taken up as a follow-up. I also was thinking of > adding a flag/property to not generate all the map files. I don't > think such a property exists currently. The map files really are not > needed for the capsules. Sure, but if you implemented SetImagePos() then it would work. Something to think about when you create the tool to dump capsules. > > > > > Anyway, for tests, primarily we need to split things up, so we have, > > for example: > > > > process_capsule_file() > > > > which processes the capsule in U-Boot, e.g. using an 'efi' command, > > then outputs what it did. Then the test can plant the capsule, run > > that function and check that the output is as expected. This can > > actually be a unit test, i.e. written in C. > > > > Most of the tests can do this. Then we can have one test that checks > > the whole flow, but it doesn't need to be done for every case, as now. > > I believe even Ilias thinks that these tests should be in C. But this > is a separate effort, not related to this series. I also have doubts > about your observation on not using so many capsule files for tests, > but that can be investigated separately. For now, I want to focus on > getting these changes in, followed by the generation of capsules > through the config file. And FWIW, I am able to use relative paths in > the binman tests for generating and testing the capsules generated > through the config file -- so that takes care of your objection to > using absolute paths. I will send that series once this gets merged. OK, yes it is best to get this series in and worry about tdy-ups after that. Regards, Simon
On Sat, Aug 05, 2023 at 09:04:00AM -0600, Simon Glass wrote: > Hi Sughosh, > > On Sat, 5 Aug 2023 at 05:35, 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 in the u-boot.dtsi for > > the sandbox architecture for generating the capsules. These capsules > > are then used for testing the EFI capsule update functionality on the > > sandbox platforms. Also add binman nodes for generating the input > > files used for these capsules. > > > > Remove the corresponding logic which was used for generation of these > > input files which is now superfluous. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > --- > > Changes since V6: > > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule > > input filenames. > > * Generate the capsule input files through binman text entries. > > > > arch/sandbox/dts/u-boot.dtsi | 363 ++++++++++++++++++ > > include/sandbox_efi_capsule.h | 11 + > > test/py/tests/test_efi_capsule/conftest.py | 168 +------- > > test/py/tests/test_efi_capsule/signature.dts | 10 - > > .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- > > 5 files changed, 385 insertions(+), 203 deletions(-) > > delete mode 100644 test/py/tests/test_efi_capsule/signature.dts > > delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its > > I think you are still getting confused with using filenames when you > don't need to. See below... > > > > > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi > > index 60bd004937..f1b16b41c2 100644 > > --- a/arch/sandbox/dts/u-boot.dtsi > > +++ b/arch/sandbox/dts/u-boot.dtsi > > @@ -7,11 +7,374 @@ > > */ > > > > #ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT > > + > > +#include <sandbox_efi_capsule.h> > > + > > / { > > #ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE > > signature { > > capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE); > > }; > > #endif > > + > > + binman: binman { > > + multiple-images; > > + }; > > +}; > > + > > +&binman { > > + input1 { > > + filename = UBOOT_BIN_IMAGE_OLD; > > + text { > > + text = "u-boot:Old"; > > + }; > > + }; > > + > > + input2 { > > + filename = UBOOT_BIN_IMAGE_NEW; > > + text { > > + text = "u-boot:New"; > > + }; > > + }; > > + > > + input3 { > > + filename = UBOOT_ENV_IMAGE_OLD; > > + text { > > + text = "u-boot-env:Old"; > > + }; > > + }; > > + > > + input4 { > > + filename = UBOOT_ENV_IMAGE_NEW; > > + text { > > + text = "u-boot-env:New"; > > + }; > > + }; > > All of those nodes can be removed > > > + > > + 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>; > > + blob { > > + filename = UBOOT_BIN_IMAGE_NEW; > > + }; > > instead of this blob node, you should be able to write: > > text { > text = "u-boot:Old"; > }; > > There is no need to provide filenames in every node. > > > + > > + hash-1 { > > + algo = "sha1"; > > + }; > > + }; > > + u-boot-env { > > + description = "U-Boot environment on SPI Flash"; > > + compression = "none"; > > + type = "firmware"; > > + arch = "sandbox"; > > + load = <0>; > > + blob { > > + filename = UBOOT_ENV_IMAGE_NEW; > > + }; > > same here and below > > > + > > + hash-1 { > > + algo = "sha1"; > > + }; > > + }; > > + }; > > + }; > > + }; > > + > > + capsule1 { > > + filename = "Test01"; > > + capsule { > > + type = "efi-capsule"; > > + image-index = <0x1>; > > + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; > > + > > + blob { > > + filename = UBOOT_BIN_IMAGE_NEW; > > + }; > > again this should be a text node > > same below > > > + }; > > + }; > > + > > + capsule2 { > > + filename = "Test02"; > > + capsule { > > + type = "efi-capsule"; > > + image-index = <0x2>; > > + image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID; > > + > > + blob { > > + filename = UBOOT_ENV_IMAGE_NEW; > > + }; > > + }; > > + }; > > + > > + capsule3 { > > + filename = "Test03"; > > + capsule { > > + type = "efi-capsule"; > > + image-index = <0x1>; > > + image-type-id = SANDBOX_INCORRECT_GUID; > > + > > + blob { > > + filename = UBOOT_BIN_IMAGE_NEW; > > + }; > > + }; > > + }; > > + > > + capsule4 { > > + filename = "Test04"; > > + capsule { > > + type = "efi-capsule"; > > + image-index = <0x1>; > > + image-type-id = SANDBOX_FIT_IMAGE_GUID; > > + > > + blob { > > + filename = > [..] > > > + capsule19 { > > + filename = "Test115"; > > [..] > > We really need to talk about these tests. So many cases! Can you not > reduce this? And why are these tests in the generic looking file and not one of the test dts files? This looks like something that would make implementation in real life confusing and non-obvious.
On Sun, 6 Aug 2023 at 03:48, Tom Rini <trini@konsulko.com> wrote: > > On Sat, Aug 05, 2023 at 09:04:00AM -0600, Simon Glass wrote: > > Hi Sughosh, > > > > On Sat, 5 Aug 2023 at 05:35, 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 in the u-boot.dtsi for > > > the sandbox architecture for generating the capsules. These capsules > > > are then used for testing the EFI capsule update functionality on the > > > sandbox platforms. Also add binman nodes for generating the input > > > files used for these capsules. > > > > > > Remove the corresponding logic which was used for generation of these > > > input files which is now superfluous. > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > --- > > > Changes since V6: > > > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule > > > input filenames. > > > * Generate the capsule input files through binman text entries. > > > > > > arch/sandbox/dts/u-boot.dtsi | 363 ++++++++++++++++++ > > > include/sandbox_efi_capsule.h | 11 + > > > test/py/tests/test_efi_capsule/conftest.py | 168 +------- > > > test/py/tests/test_efi_capsule/signature.dts | 10 - > > > .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- > > > 5 files changed, 385 insertions(+), 203 deletions(-) > > > delete mode 100644 test/py/tests/test_efi_capsule/signature.dts > > > delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its > > > > I think you are still getting confused with using filenames when you > > don't need to. See below... > > > > > > > > > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi > > > index 60bd004937..f1b16b41c2 100644 > > > --- a/arch/sandbox/dts/u-boot.dtsi > > > +++ b/arch/sandbox/dts/u-boot.dtsi <snip> > > > > > + }; > > > + }; > > > + > > > + capsule2 { > > > + filename = "Test02"; > > > + capsule { > > > + type = "efi-capsule"; > > > + image-index = <0x2>; > > > + image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID; > > > + > > > + blob { > > > + filename = UBOOT_ENV_IMAGE_NEW; > > > + }; > > > + }; > > > + }; > > > + > > > + capsule3 { > > > + filename = "Test03"; > > > + capsule { > > > + type = "efi-capsule"; > > > + image-index = <0x1>; > > > + image-type-id = SANDBOX_INCORRECT_GUID; > > > + > > > + blob { > > > + filename = UBOOT_BIN_IMAGE_NEW; > > > + }; > > > + }; > > > + }; > > > + > > > + capsule4 { > > > + filename = "Test04"; > > > + capsule { > > > + type = "efi-capsule"; > > > + image-index = <0x1>; > > > + image-type-id = SANDBOX_FIT_IMAGE_GUID; > > > + > > > + blob { > > > + filename = > > [..] > > > > > + capsule19 { > > > + filename = "Test115"; > > > > [..] > > > > We really need to talk about these tests. So many cases! Can you not > > reduce this? > > And why are these tests in the generic looking file and not one of the > test dts files? This looks like something that would make implementation > in real life confusing and non-obvious. These are not capsules that are being generated for binman tests. Those dts files are indeed under tools/binman/test/. These capsules are the ones used for testing the capsule update feature in the CI run. The idea of having these capsule nodes defined in this file is to have them generated as part of the standard build. -sughosh
On Sun, Aug 06, 2023 at 04:43:06PM +0530, Sughosh Ganu wrote: > On Sun, 6 Aug 2023 at 03:48, Tom Rini <trini@konsulko.com> wrote: > > > > On Sat, Aug 05, 2023 at 09:04:00AM -0600, Simon Glass wrote: > > > Hi Sughosh, > > > > > > On Sat, 5 Aug 2023 at 05:35, 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 in the u-boot.dtsi for > > > > the sandbox architecture for generating the capsules. These capsules > > > > are then used for testing the EFI capsule update functionality on the > > > > sandbox platforms. Also add binman nodes for generating the input > > > > files used for these capsules. > > > > > > > > Remove the corresponding logic which was used for generation of these > > > > input files which is now superfluous. > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > --- > > > > Changes since V6: > > > > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule > > > > input filenames. > > > > * Generate the capsule input files through binman text entries. > > > > > > > > arch/sandbox/dts/u-boot.dtsi | 363 ++++++++++++++++++ > > > > include/sandbox_efi_capsule.h | 11 + > > > > test/py/tests/test_efi_capsule/conftest.py | 168 +------- > > > > test/py/tests/test_efi_capsule/signature.dts | 10 - > > > > .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- > > > > 5 files changed, 385 insertions(+), 203 deletions(-) > > > > delete mode 100644 test/py/tests/test_efi_capsule/signature.dts > > > > delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its > > > > > > I think you are still getting confused with using filenames when you > > > don't need to. See below... > > > > > > > > > > > > > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi > > > > index 60bd004937..f1b16b41c2 100644 > > > > --- a/arch/sandbox/dts/u-boot.dtsi > > > > +++ b/arch/sandbox/dts/u-boot.dtsi > > <snip> > > > > > > > > + }; > > > > + }; > > > > + > > > > + capsule2 { > > > > + filename = "Test02"; > > > > + capsule { > > > > + type = "efi-capsule"; > > > > + image-index = <0x2>; > > > > + image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID; > > > > + > > > > + blob { > > > > + filename = UBOOT_ENV_IMAGE_NEW; > > > > + }; > > > > + }; > > > > + }; > > > > + > > > > + capsule3 { > > > > + filename = "Test03"; > > > > + capsule { > > > > + type = "efi-capsule"; > > > > + image-index = <0x1>; > > > > + image-type-id = SANDBOX_INCORRECT_GUID; > > > > + > > > > + blob { > > > > + filename = UBOOT_BIN_IMAGE_NEW; > > > > + }; > > > > + }; > > > > + }; > > > > + > > > > + capsule4 { > > > > + filename = "Test04"; > > > > + capsule { > > > > + type = "efi-capsule"; > > > > + image-index = <0x1>; > > > > + image-type-id = SANDBOX_FIT_IMAGE_GUID; > > > > + > > > > + blob { > > > > + filename = > > > [..] > > > > > > > + capsule19 { > > > > + filename = "Test115"; > > > > > > [..] > > > > > > We really need to talk about these tests. So many cases! Can you not > > > reduce this? > > > > And why are these tests in the generic looking file and not one of the > > test dts files? This looks like something that would make implementation > > in real life confusing and non-obvious. > > These are not capsules that are being generated for binman tests. > Those dts files are indeed under tools/binman/test/. These capsules > are the ones used for testing the capsule update feature in the CI > run. The idea of having these capsule nodes defined in this file is to > have them generated as part of the standard build. The high level concern I have (so it applies to a few of these patches) is that it's not going to be clear and straightforward on how to use this regularly (for example, if I follow all of this right, I should be able to do a capsule update to push a build to one of my boards, and then run our pytest suite on it, instead of playing games with SD mux boards and so forth) or production (configure product-board so that we can deliver an updated firmware via $mechanism).
Hi Tom, On Mon, 7 Aug 2023 at 12:08, Tom Rini <trini@konsulko.com> wrote: > > On Sun, Aug 06, 2023 at 04:43:06PM +0530, Sughosh Ganu wrote: > > On Sun, 6 Aug 2023 at 03:48, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Sat, Aug 05, 2023 at 09:04:00AM -0600, Simon Glass wrote: > > > > Hi Sughosh, > > > > > > > > On Sat, 5 Aug 2023 at 05:35, 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 in the u-boot.dtsi for > > > > > the sandbox architecture for generating the capsules. These capsules > > > > > are then used for testing the EFI capsule update functionality on the > > > > > sandbox platforms. Also add binman nodes for generating the input > > > > > files used for these capsules. > > > > > > > > > > Remove the corresponding logic which was used for generation of these > > > > > input files which is now superfluous. > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > > --- > > > > > Changes since V6: > > > > > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule > > > > > input filenames. > > > > > * Generate the capsule input files through binman text entries. > > > > > > > > > > arch/sandbox/dts/u-boot.dtsi | 363 ++++++++++++++++++ > > > > > include/sandbox_efi_capsule.h | 11 + > > > > > test/py/tests/test_efi_capsule/conftest.py | 168 +------- > > > > > test/py/tests/test_efi_capsule/signature.dts | 10 - > > > > > .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- > > > > > 5 files changed, 385 insertions(+), 203 deletions(-) > > > > > delete mode 100644 test/py/tests/test_efi_capsule/signature.dts > > > > > delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its > > > > > > > > I think you are still getting confused with using filenames when you > > > > don't need to. See below... > > > > > > > > > > > > > > > > > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi > > > > > index 60bd004937..f1b16b41c2 100644 > > > > > --- a/arch/sandbox/dts/u-boot.dtsi > > > > > +++ b/arch/sandbox/dts/u-boot.dtsi > > > > <snip> > > > > > > > > > > > + }; > > > > > + }; > > > > > + > > > > > + capsule2 { > > > > > + filename = "Test02"; > > > > > + capsule { > > > > > + type = "efi-capsule"; > > > > > + image-index = <0x2>; > > > > > + image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID; > > > > > + > > > > > + blob { > > > > > + filename = UBOOT_ENV_IMAGE_NEW; > > > > > + }; > > > > > + }; > > > > > + }; > > > > > + > > > > > + capsule3 { > > > > > + filename = "Test03"; > > > > > + capsule { > > > > > + type = "efi-capsule"; > > > > > + image-index = <0x1>; > > > > > + image-type-id = SANDBOX_INCORRECT_GUID; > > > > > + > > > > > + blob { > > > > > + filename = UBOOT_BIN_IMAGE_NEW; > > > > > + }; > > > > > + }; > > > > > + }; > > > > > + > > > > > + capsule4 { > > > > > + filename = "Test04"; > > > > > + capsule { > > > > > + type = "efi-capsule"; > > > > > + image-index = <0x1>; > > > > > + image-type-id = SANDBOX_FIT_IMAGE_GUID; > > > > > + > > > > > + blob { > > > > > + filename = > > > > [..] > > > > > > > > > + capsule19 { > > > > > + filename = "Test115"; > > > > > > > > [..] > > > > > > > > We really need to talk about these tests. So many cases! Can you not > > > > reduce this? > > > > > > And why are these tests in the generic looking file and not one of the > > > test dts files? This looks like something that would make implementation > > > in real life confusing and non-obvious. > > > > These are not capsules that are being generated for binman tests. > > Those dts files are indeed under tools/binman/test/. These capsules > > are the ones used for testing the capsule update feature in the CI > > run. The idea of having these capsule nodes defined in this file is to > > have them generated as part of the standard build. > > The high level concern I have (so it applies to a few of these patches) > is that it's not going to be clear and straightforward on how to use > this regularly (for example, if I follow all of this right, I should be > able to do a capsule update to push a build to one of my boards, and > then run our pytest suite on it, instead of playing games with SD mux > boards and so forth) or production (configure product-board so that we > can deliver an updated firmware via $mechanism). Well you can do that today with VBE... Regards, Simon
diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi index 60bd004937..f1b16b41c2 100644 --- a/arch/sandbox/dts/u-boot.dtsi +++ b/arch/sandbox/dts/u-boot.dtsi @@ -7,11 +7,374 @@ */ #ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT + +#include <sandbox_efi_capsule.h> + / { #ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE signature { capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE); }; #endif + + binman: binman { + multiple-images; + }; +}; + +&binman { + input1 { + filename = UBOOT_BIN_IMAGE_OLD; + text { + text = "u-boot:Old"; + }; + }; + + input2 { + filename = UBOOT_BIN_IMAGE_NEW; + text { + text = "u-boot:New"; + }; + }; + + input3 { + filename = UBOOT_ENV_IMAGE_OLD; + text { + text = "u-boot-env:Old"; + }; + }; + + input4 { + filename = UBOOT_ENV_IMAGE_NEW; + text { + text = "u-boot-env:New"; + }; + }; + + 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>; + blob { + filename = UBOOT_BIN_IMAGE_NEW; + }; + + hash-1 { + algo = "sha1"; + }; + }; + u-boot-env { + description = "U-Boot environment on SPI Flash"; + compression = "none"; + type = "firmware"; + arch = "sandbox"; + load = <0>; + blob { + filename = UBOOT_ENV_IMAGE_NEW; + }; + + hash-1 { + algo = "sha1"; + }; + }; + }; + }; + }; + + capsule1 { + filename = "Test01"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; + + blob { + filename = UBOOT_BIN_IMAGE_NEW; + }; + }; + }; + + capsule2 { + filename = "Test02"; + capsule { + type = "efi-capsule"; + image-index = <0x2>; + image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID; + + blob { + filename = UBOOT_ENV_IMAGE_NEW; + }; + }; + }; + + capsule3 { + filename = "Test03"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-type-id = SANDBOX_INCORRECT_GUID; + + blob { + filename = UBOOT_BIN_IMAGE_NEW; + }; + }; + }; + + capsule4 { + filename = "Test04"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-type-id = SANDBOX_FIT_IMAGE_GUID; + + blob { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule5 { + filename = "Test05"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-type-id = SANDBOX_INCORRECT_GUID; + + blob { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule6 { + filename = "Test101"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x5>; + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; + + blob { + filename = UBOOT_BIN_IMAGE_NEW; + }; + }; + }; + + capsule7 { + filename = "Test102"; + capsule { + type = "efi-capsule"; + image-index = <0x2>; + fw-version = <0xa>; + image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID; + + blob { + filename = UBOOT_ENV_IMAGE_NEW; + }; + }; + }; + + capsule8 { + filename = "Test103"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x2>; + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; + + blob { + filename = UBOOT_BIN_IMAGE_NEW; + }; + }; + }; + + capsule9 { + filename = "Test104"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x5>; + image-type-id = SANDBOX_FIT_IMAGE_GUID; + + blob { + filename = UBOOT_FIT_IMAGE; + }; + }; + }; + + capsule10 { + filename = "Test105"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x2>; + image-type-id = 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-type-id = SANDBOX_UBOOT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + public-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + blob { + filename = UBOOT_BIN_IMAGE_NEW; + }; + }; + }; + + capsule12 { + filename = "Test12"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; + private-key = CAPSULE_INVAL_KEY; + public-key-cert = CAPSULE_INVAL_PUB_KEY; + monotonic-count = <0x1>; + + blob { + filename = UBOOT_BIN_IMAGE_NEW; + }; + }; + }; + + capsule13 { + filename = "Test13"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + image-type-id = 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-type-id = 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-type-id = SANDBOX_UBOOT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + public-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + blob { + filename = UBOOT_BIN_IMAGE_NEW; + }; + }; + }; + + capsule16 { + filename = "Test112"; + capsule { + type = "efi-capsule"; + image-index = <0x2>; + fw-version = <0xa>; + image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + public-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + blob { + filename = UBOOT_ENV_IMAGE_NEW; + }; + }; + }; + + capsule17 { + filename = "Test113"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x2>; + image-type-id = SANDBOX_UBOOT_IMAGE_GUID; + private-key = CAPSULE_PRIV_KEY; + public-key-cert = CAPSULE_PUB_KEY; + monotonic-count = <0x1>; + + blob { + filename = UBOOT_BIN_IMAGE_NEW; + }; + }; + }; + + capsule18 { + filename = "Test114"; + capsule { + type = "efi-capsule"; + image-index = <0x1>; + fw-version = <0x5>; + image-type-id = 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-type-id = 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 */ }; #endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */ diff --git a/include/sandbox_efi_capsule.h b/include/sandbox_efi_capsule.h index da71b87a18..8e6128de41 100644 --- a/include/sandbox_efi_capsule.h +++ b/include/sandbox_efi_capsule.h @@ -11,4 +11,15 @@ #define SANDBOX_FIT_IMAGE_GUID "3673b45d-6a7c-46f3-9e60-adabb03f7937" #define SANDBOX_INCORRECT_GUID "058b7d83-50d5-4c47-a195-60d86ad341c4" +#define UBOOT_BIN_IMAGE_NEW "u-boot.bin.new" +#define UBOOT_ENV_IMAGE_NEW "u-boot.env.new" +#define UBOOT_BIN_IMAGE_OLD "u-boot.bin.old" +#define UBOOT_ENV_IMAGE_OLD "u-boot.env.old" +#define UBOOT_FIT_IMAGE "u-boot_bin_env.itb" + +#define CAPSULE_PRIV_KEY "SIGNER.key" +#define CAPSULE_PUB_KEY "SIGNER.crt" +#define CAPSULE_INVAL_KEY "SIGNER2.key" +#define CAPSULE_INVAL_PUB_KEY "SIGNER2.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..d5dc80f5ff 100644 --- a/test/py/tests/test_efi_capsule/conftest.py +++ b/test/py/tests/test_efi_capsule/conftest.py @@ -32,41 +32,22 @@ def efi_capsule_data(request, u_boot_config): check_call('mkdir -p %s' % data_dir, shell=True) check_call('mkdir -p %s' % install_dir, shell=True) - capsule_auth_enabled = u_boot_config.buildconfig.get( - 'config_efi_capsule_authenticate') - if capsule_auth_enabled: - # Create private key (SIGNER.key) and certificate (SIGNER.crt) - check_call('cd %s; ' - 'openssl req -x509 -sha256 -newkey rsa:2048 ' - '-subj /CN=TEST_SIGNER/ -keyout SIGNER.key ' - '-out SIGNER.crt -nodes -days 365' - % data_dir, shell=True) - check_call('cd %s; %scert-to-efi-sig-list SIGNER.crt SIGNER.esl' - % (data_dir, EFITOOLS_PATH), shell=True) - - # Update dtb adding capsule certificate - check_call('cd %s; ' - 'cp %s/test/py/tests/test_efi_capsule/signature.dts .' - % (data_dir, u_boot_config.source_dir), shell=True) - check_call('cd %s; ' - 'dtc -@ -I dts -O dtb -o signature.dtbo signature.dts; ' - 'fdtoverlay -i %s/arch/sandbox/dts/test.dtb ' - '-o test_sig.dtb signature.dtbo' - % (data_dir, u_boot_config.build_dir), shell=True) - - # Create *malicious* private key (SIGNER2.key) and certificate - # (SIGNER2.crt) - check_call('cd %s; ' - 'openssl req -x509 -sha256 -newkey rsa:2048 ' - '-subj /CN=TEST_SIGNER/ -keyout SIGNER2.key ' - '-out SIGNER2.crt -nodes -days 365' - % data_dir, shell=True) - + check_call('cp %s/u-boot.bin.* %s' + % (u_boot_config.build_dir, data_dir), shell=True) + check_call('cp %s/u-boot.env.* %s' + % (u_boot_config.build_dir, data_dir), shell=True) + 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) # Update dtb to add the version information check_call('cd %s; ' 'cp %s/test/py/tests/test_efi_capsule/version.dts .' % (data_dir, u_boot_config.source_dir), shell=True) + + capsule_auth_enabled = u_boot_config.buildconfig.get( + 'config_efi_capsule_authenticate') if capsule_auth_enabled: + check_call('cp %s/arch/sandbox/dts/test.dtb %s/test_sig.dtb' % + (u_boot_config.build_dir, data_dir), shell=True) check_call('cd %s; ' 'dtc -@ -I dts -O dtb -o version.dtbo version.dts; ' 'fdtoverlay -i test_sig.dtb ' @@ -79,133 +60,6 @@ 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 - # 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' % (mnt_point, image_path), shell=True) diff --git a/test/py/tests/test_efi_capsule/signature.dts b/test/py/tests/test_efi_capsule/signature.dts deleted file mode 100644 index 078cfc76c9..0000000000 --- a/test/py/tests/test_efi_capsule/signature.dts +++ /dev/null @@ -1,10 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ - -/dts-v1/; -/plugin/; - -&{/} { - signature { - capsule-key = /incbin/("SIGNER.esl"); - }; -}; 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"; - }; - }; - }; -};
The EFI capsule files can now be generated as part of u-boot build through binman. Add capsule entry nodes in the u-boot.dtsi for the sandbox architecture for generating the capsules. These capsules are then used for testing the EFI capsule update functionality on the sandbox platforms. Also add binman nodes for generating the input files used for these capsules. Remove the corresponding logic which was used for generation of these input files which is now superfluous. Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- Changes since V6: * Use macros defined in sandbox_efi_capsule for GUIDs and capsule input filenames. * Generate the capsule input files through binman text entries. arch/sandbox/dts/u-boot.dtsi | 363 ++++++++++++++++++ include/sandbox_efi_capsule.h | 11 + test/py/tests/test_efi_capsule/conftest.py | 168 +------- test/py/tests/test_efi_capsule/signature.dts | 10 - .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- 5 files changed, 385 insertions(+), 203 deletions(-) delete mode 100644 test/py/tests/test_efi_capsule/signature.dts delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its