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