On 2/17/22 11:10, Sughosh Ganu wrote:
On Thu, 17 Feb 2022 at 13:52, Ilias Apalodimas
<ilias.apalodi...@linaro.org> wrote:
[...]
Yes, we can use --index when we know the index value corresponding to
the firmware image that we need to update. But like I mentioned in my
earlier reply, for A/B updates, we do not know what the index value is
going to be. That is going to be determined at runtime.
I don't think so. See below for alternative approach.
Also, the point I was making is that we can have a capsule which is
consumed by an FMP protocol which has more than one image, and those
images have different ImageTypeId/UpdateImageTypeId.
Yes, but it is a design choice in my first implementation.
I didn't think that we need to "have a capsule which is consumed
by an FMP protocol which has more than one image" as long as we
use DFU framework (and FIT as standard format of aggregation on U-Boot).
But this design can be extended without any hassle, and more
importantly without any regression, no? What kind of a problem does it
create if the FMP can handle more than one image type.
Even as per the UEFI spec, we have the EFI_FIRMWARE_MANAGEMENT_CAPSULE
header for all images to be managed by the FMP protocol which has
multiple images with different UpdateImageTypeId.
Please check the
GenerateCapsule script in EDK2. In case of a multi payload based
capsule, individual parameters like the UpdateImageTypeId are passed
through the json file, where each of the UpdateImageTypeId has a
different value per payload.
2) Each firmware object handled by a given FMP driver can further be
identified by ImageIndex.
My implementation of efi_fmp_find() does (1) and Raw FMP driver does
(2) in efi_firmware_raw_set_image() which takes "image_index" as
a parameter.
Using ImageTypeId as an identifier is simply wrong in my opinion and
doesn't meet the UEFI specification.
So, as per what you are stating, all payloads under a given
EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER should have the same
ImageTypeId, either EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID or
EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID. Same applies for all images in
the EFI_FIRMWARE_IMAGE_DESCRIPTOR. Because, without one of the two
values, > the check in efi_fmp_find to compare the UpdateImageTypeId
with the ImageTypeId retrieved from the image descriptor would simply
fail.
I don't follow your point.
Please elaborate a bit more.
The current implementation of GetImageInfo, passes either of
EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID or
EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID for all the images of the image
descriptor array. So, in case the capsule is generated with a '--guid'
value which is different from these two values, the check in
efi_fmp_find on line 204 will fail.
That is an expected behavior, isn't it?
Yes it is. Do not contest that.
If you want to use a different FMP driver (with another GUID),
you naturally need to add your own FMP driver.
This is where I differ. We can use the same FMP protocol instance for
any type of ImageTypeId. I do not see why we need to define a
different FMP protocol instance for a GUID value other than what has
been defined for u-boot raw and u-boot FIT GUIDs.
I do understand part of your concern a bit.
I thought of using the same ImageType GUID, say IMAGE_TYPE_DFU_GUID, at first
but we needed different GUIDs here simply because we need to determine
the format of payload, FIT format or raw binary.
The platform can give us the image descriptor array, and with that,
the same FMP instance can be used for any type of image(ImageTypeId).
"any type of image"? Really?
The raw FMP instance can certainly handle any type of binary payloads
right. There is no restriction on what type of payload it is as long
as it is all going as a single entity to a given dfu partition.
This means that unless the --guid
value passed to the capsule generation is either of u-boot FIT or
u-boot raw, the current FMP protocol for raw devices cannot be used.
Why do we need that restriction. It should be possible to use the raw
FMP protocol for any other type of image types as well.
I think this interpretation of the UEFI spec is incorrect, since the
spec states that the ImageTypeId and the UpdateImageTypeId are fields
used to identify the firmware component targeted for the update. If
all values in the image descriptor array and the UpdateImageTypeId are
the same, why have this field in the first place for individual
images.
As I said, ImageIndex is for that purpose.
Yes, that is one possible way in the scenario where the ImageIndex is
determined at the capsule generation time. But, for the A/B update
scenario, we do not know the ImageIndex at build time
"Build time" of what?
Of the capsule.
I think that users should know how "dfu_alt_info" is defined
(in other words, where the firmware be located on the target system)
when capsule files are created.
That is true for a non A/B scenario. And that is how it works in the
non A/B updates case. But for A/B updates, since the determination of
the "location" where the firmware image has to be written will be done
only at runtime, we cannot use the --index to differentiate.
Yes, we can :)
You know what I mean -- if we could use the same logic, I would not
have added all that code :)
First of all, my essential assumption in either FIT or RAW FMP driver
is that U-Boot has (somehow conceptually) single firmware blob represented
by DFU or dfu_alt_info. As I said, each object or location in
dfu_alt_info can be further identified by index or "UpdateImageIndex".
Let's assume that we have two locations of firmware, fw1 and fw2, and
that we have two bank A and B.
Then we will define dfu_alt_info as follows:
<loc of fw1 for A>;<loc of fw2 for A>;<loc of fw1 for B>;<loc of fw2 for B>;
|<--- 1st set --->|<--- 2nd set --->|
When you want to update bank A, we can use the first set of dfu_alt_info,
and use the second set of dfu_alt_info for bank B.
At runtime, you should know which bank you're working on, and therefore
you should know the exact physical location from dfu_alt_info.
Please note that you don't have to change the syntax of dfu_alt_info
at all. Simply offset the location with 0 for bank A and with 2 for bank B.
I'll try digging a bit more, but I think the current approach is not
working as it was intended wrt to the EFI spec. My reading of the spec
and specifically section 23.3.2 is that a Capsule consists of an
EFI capsule header and a payload. The payload now has an
EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER which in it's turn can host multiple
firmware images of different type described in
EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER.
An FMP implementation should read the UpdateImageTypeId's used to identify
the image you are updating and from that derive the UpdateImageIndex
which SetImage will use. That would give you the ability to update the
all the firmware components with a single capsule.
Sughosh what about the ESRT table generation? If you use different
UpdateImageTypeId
those should be reflected on the ESRT tables from the OS
That would depend on the values populated in the
EFI_FIRMWARE_IMAGE_DESCRIPTOR array by the GetImageInfo function. The
image descriptor structure has an ImageTypeId field. The value of
ImageTypeId is what will be reflected in the ESRT table.
In the current implementation, all the images in the ESRT table will
show the same ImageTypeId value, either
EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID or
EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID.
This is wrong. According to the the UEFI 2.9 specification the
UpdateImageTypeId is used to "identify (the) device firmware targeted by
this update". It does not identify the format in which the firmware is
delivered.
So this needs to be fixed in the next revision of this patch series.
For each firmware part that can be updated provide a unique GUID.
Best regards
Heinrich
Yea I was mostly asking wrt to A/B updates. Would the correct UUID be
shown in the ESRT instead of EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID?
Yes, the platform would have to define the fill_image_type_guid_array
function which would populate the ImageTypeId values in the
EFI_FIRMWARE_IMAGE_DESCRIPTOR array, and the image GUID values would
then show up as part of the ESRT table.
As part of this patchset, I have added this function for the STM32MP1 DK2 board.
-sughosh
The UpdateImageTypeId value from the capsule is used to compare with
the ImageTypeId values returned by the GetImageInfo function to check
if the given FMP protocol can be used for the update.
-sughosh
[...]
Regards
/Ilias
