Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type

[Heinrich Schuchardt]

Am 10. Juli 2023 08:26:37 MESZ schrieb AKASHI Takahiro <takahiro.akashi at linaro.org>:
>On Sun, Jul 09, 2023 at 10:31:55AM +0200, Heinrich Schuchardt wrote:
>> On 6/16/23 10:28, Stefan Herbrechtsmeier wrote:
>> > From: Malte Schmidt <malte.schmidt at weidmueller.com>
>> > 
>> > The data type of item_offset_list shall be UINT64 according to the UEFI [1]
>> > specifications.
>> > 
>> > In include/efi_api.h the correct data type is used. The bug was probably
>> > never noticed because of little endianness.
>> > 
>> > [1] https://uefi.org/specs/UEFI/2.10/index.html
>> > 
>> > Signed-off-by: Malte Schmidt <malte.schmidt at weidmueller.com>
>> > 
>> > Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.com>
>> > ---
>> > 
>> >   tools/eficapsule.h | 2 +-
>> >   1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/tools/eficapsule.h b/tools/eficapsule.h
>> > index 753fb73313..2099a2e9b8 100644
>> > --- a/tools/eficapsule.h
>> > +++ b/tools/eficapsule.h
>> > @@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header {
>> >   	uint32_t version;
>> >   	uint16_t embedded_driver_count;
>> >   	uint16_t payload_item_count;
>> > -	uint32_t item_offset_list[];
>> > +	uint64_t item_offset_list[];
>> 
>> Defining the same structure in two places it bad practice.
>> https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/11
>
>I had a good reason for adding a tool-specific header, instead of
>using headers under 'include' dir, when I posted v7 of "efi_loader:
>capsule: improve capsule authentication support" patch.
>The cover letter says,
>===
>v7 (Nov 16, 2021)
>  ...
>* define eficapsule.h and include it from mkeficapsule (patch#3)
>  Hopefully, the tool can now compile on non-linux host.
>===
>
>If I correctly remember, this reflects the comment below and the 
>succeeding discussions:
>https://lists.denx.de/pipermail/u-boot/2021-November/465859.html
>
>-Takahiro Akashi

We should be able to factor out a common header file which contains capsule specific structures.

I understand that we have to test building on BSD.

Best regards

Heinrich

>
>
>> Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> 
>> >   } __packed;
>> > 
>> >   /* image_capsule_support */
>>