[PATCH v5 1/4] efi_loader: get version information from device tree

[Heinrich Schuchardt]

On 5/10/23 22:46, Simon Glass wrote:
> Hi Heinrich,
>
> On Thu, 27 Apr 2023 at 22:12, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>>
>>
>> Am 28. April 2023 01:35:04 MESZ schrieb Simon Glass <sjg at chromium.org>:
>>> Hi Masahisa,
>>>
>>> On Mon, 10 Apr 2023 at 03:07, Masahisa Kojima
>>> <masahisa.kojima at linaro.org> wrote:
>>>>
>>>> Current FMP->GetImageInfo() always return 0 for the firmware
>>>> version, user can not identify which firmware version is currently
>>>> running through the EFI interface.
>>>>
>>>> This commit gets the version information from device tree,
>>>> then fills the firmware version, lowest supported version
>>>> in FMP->GetImageInfo().
>>>>
>>>> Now FMP->GetImageInfo() and ESRT have the meaningful version number.
>>>>
>>>> Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
>>>> ---
>>>> Changes in v5:
>>>> - newly implement a device tree based versioning
>>>>
>>>>   .../firmware/firmware-version.txt             | 25 ++++++++
>>>>   lib/efi_loader/efi_firmware.c                 | 63 +++++++++++++++++--
>>>>   2 files changed, 84 insertions(+), 4 deletions(-)
>>>>   create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt
>>>>
>>>> diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt b/doc/device-tree-bindings/firmware/firmware-version.txt
>>>> new file mode 100644
>>>> index 0000000000..6112de4a1d
>>>> --- /dev/null
>>>> +++ b/doc/device-tree-bindings/firmware/firmware-version.txt
>>>> @@ -0,0 +1,25 @@
>>>> +firmware-version bindings
>>>> +-------------------------------
>>>> +
>>>> +Required properties:
>>>> +- image-type-id                        : guid for image blob type
>>>> +- image-index                  : image index
>>>> +- fw-version                   : firmware version
>>>> +- lowest-supported-version     : lowest supported version
>>>> +
>>>> +Example:
>>>> +
>>>> +       firmware-version {
>>>> +               image1 {
>>>> +                       image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
>>>
>>> Nit: please use lower-case hex and add a decoder to uuid.c so we can
>>> look it up when debugging.
>>
>> The GUIDs are board specific. No, we should not clutter uuid.c with strings for dozens of boards. Our development aim is to keep U-Boot small and these GUIDs are not printed anywhere.
>>
>> Instead we should define a string per firmware image. We already have a field for it:
>>
>> fw_array[i].fw_name
>
> This is of course madness. NAK to any UUIDs in U-Boot that don't have
> a decoder ring.

I never have heard of a "decoder ring" for UUIDs in U-Boot.

   git grep -A2 -ni decoder | grep -i ring

does not find such a term.

Our development target is to keep the code size of U-Boot small. Hence,
we should not clutter uuid.c with board specific strings. These should
be kept in board specific files.

>
> Let's use a compatible string (vendor,board) and drop these silly UUIDs.

Please, avoid derogatory language like "silly".

The UEFI specification defines EFI_CAPSULE_HEADER. This structure
contains field CapusuleGuid (A GUID that defines the contents of a capsule).

Best regards

Heinrich