Sughosh, On Thu, Mar 24, 2022 at 06:08:55PM +0530, Sughosh Ganu wrote: > > This series is cleaning up the usage of the image GUIDs that are used > in capsule update and the EFI System Resource Table(ESRT). > > Currently, there are two instances of the Firmware Management > Protocol(FMP), one defined for updating the FIT images, and the other > for updating raw images. The FMP code defines two GUID values, one for > all FIT images, and one for raw images. Depending on the FMP instance > used on a platform, the platform needs to use the corresponding image > GUID value for all images on the platform, and also across platforms. > > A few issues are being fixed through the patch series. One, that an > image for a different platform can be flashed on another platform if > both the platforms are using the same FMP instance. So, for e.g. a > capsule generated for the Socionext DeveloperBox platform can be > flashed on the ZynqMP platform, since both the platforms use the > CONFIG_EFI_CAPSULE_FIRMWARE_RAW instance of the FMP. This can be > corrected if each firmware image that can be updated through the > capsule update mechanism has it's own unique image GUID.
Ok although the specification doesn't explicitly require the uniqueness "across platforms". > The second issue that this patch series fixes is the value of FwClass > in the ESRT. With the current logic, all firmware image entries in the > ESRT display the same GUID value -- either the FIT GUID or the raw > GUID. This is not in compliance with the UEFI specification, as the > specification requires all entries to have unique GUID values. Well, this is *not* a problem of fit FMP driver, but a matter of how ESRT is populated. Table 23-16 "ESRT and FMP Fields" says, The ImageTypeId GUID from the Firmware Management Protocol instance for a device is used as the Firmware Class GUID in the ESRT. Where there are multiple identical devices in the system, system firmware must create a mapping to ensure that the ESRT FwClass GUIDs are unique and consistent. The second sentence suggests that UEFI implementation, i.e. efi_esrt_populate(), may and should allow some *mapping*. That said, I basically accept the requirement that you mention above. > The third issue being fixed is the population of the > EFI_FIRMWARE_IMAGE_DESCRIPTOR array. The current code uses the dfu > framework for populating the image descriptor array. However, there > might be other images that are not to be updated through the capsule > update mechanism also registered with the dfu framework. As a result > of this, the ESRT will show up entries of images that are not to be > targeted by the capsule update mechanism. > > These issues are being fixed by defining a structure, efi_fw_images. A > platform can then define image related information like the image GUID > and image name. Every platform that uses capsule update mechanism > needs to define fw_images array. This array will then be used to > populate the image descriptor array, and also in determining if a > particular capsule's payload can be used for updating an image on the > platform. When ESRT support patch was posted, I proposed that we should have a kind of configuration table that defines all the firmware on the system for ESRT, but this proposal was rejected. Your efi_fw_images[] looks quite similar as what I had in mind. Why not define efi_fw_images[] as ESRT itself. (Of course, some fields in an entry can still be populated through GetImageInfo API.) > The first patch of this series adds the fw_images array in all > platforms which are using UEFI capsule updates > > The second patch of the series changes the logic for populating the > image descriptor array, using the information from the fw_images array > defined by the platform. So a FIT image can still work as a single binary of firmware under FIT FMP driver. Correct? > The third patch of the series removes the test cases using the --raw > and --fit parameters, removes test case for FIT images, and adds a > test case for checking that the update happens only with the correct > image GUID value in the capsule. Your change in test_capsule_firmware.py tries to remove FIT FMP driver test and it eventually hides the fact that FIT driver may get broken. I expect that you should maintain FIT FMP driver properly and it be tested as before. # Moreover, you have not yet added capsule authentication support to FIT FMP driver. -Takahiro Akashi > > The fourth patch of the series makes corresponding changes in the > capsule update related documentation. > > The fifth patch of the series removes the now unused FIT and raw image > GUID values from the FMP module. > > The sixth patch of the series removes the --raw and --fit command line > parameters in the mkeficapsule utility. > > > Sughosh Ganu (6): > capsule: Add Image GUIDs for platforms using capsule updates > capsule: FMP: Populate the image descriptor array from platform data > test: capsule: Modify the capsule tests to use GUID values for sandbox > doc: uefi: Update the capsule update related documentation > FMP: Remove GUIDs for FIT and raw images > mkeficapsule: Remove raw and FIT GUID types > > .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c | 19 ++ > .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c | 18 ++ > board/emulation/qemu-arm/qemu-arm.c | 20 +++ > board/kontron/pitx_imx8m/pitx_imx8m.c | 15 +- > board/kontron/sl-mx8mm/sl-mx8mm.c | 14 ++ > board/kontron/sl28/sl28.c | 14 ++ > board/sandbox/sandbox.c | 17 ++ > board/socionext/developerbox/developerbox.c | 23 +++ > board/xilinx/common/board.h | 18 ++ > board/xilinx/zynq/board.c | 18 ++ > board/xilinx/zynqmp/zynqmp.c | 18 ++ > configs/sandbox64_defconfig | 1 - > configs/sandbox_defconfig | 1 - > doc/develop/uefi/uefi.rst | 10 +- > include/configs/imx8mm-cl-iot-gate.h | 10 ++ > include/configs/imx8mp_rsb3720.h | 10 ++ > include/configs/kontron-sl-mx8mm.h | 6 + > include/configs/kontron_pitx_imx8m.h | 6 + > include/configs/kontron_sl28.h | 6 + > include/configs/qemu-arm.h | 10 ++ > include/configs/sandbox.h | 10 ++ > include/configs/synquacer.h | 14 ++ > include/efi_api.h | 8 - > include/efi_loader.h | 18 ++ > lib/efi_loader/efi_firmware.c | 95 +++------- > test/py/tests/test_efi_capsule/conftest.py | 20 +-- > .../test_efi_capsule/test_capsule_firmware.py | 167 ++++++------------ > tools/eficapsule.h | 8 - > tools/mkeficapsule.c | 26 +-- > 29 files changed, 384 insertions(+), 236 deletions(-) > > -- > 2.25.1 > >