hi Simon, On Sun, 6 Aug 2023 at 00:06, Simon Glass <[email protected]> wrote: > > Hi Sughosh, > > On Sat, 5 Aug 2023 at 12:01, Sughosh Ganu <[email protected]> wrote: > > > > hi Simon, > > > > On Sat, 5 Aug 2023 at 20:34, Simon Glass <[email protected]> wrote: > > > > > > Hi Sughosh, > > > > > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu <[email protected]> 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 <[email protected]> > > > > --- > > > > 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
