Am 25. November 2020 08:32:08 MEZ schrieb AKASHI Takahiro 
<takahiro.aka...@linaro.org>:
>Heinrich,
>
>On Wed, Nov 25, 2020 at 07:42:31AM +0100, Heinrich Schuchardt wrote:
>> Am 25. November 2020 02:05:06 MEZ schrieb AKASHI Takahiro
><takahiro.aka...@linaro.org>:
>> >Heinrich,
>> >
>> >On Tue, Nov 24, 2020 at 09:23:50PM +0100, Heinrich Schuchardt wrote:
>> >> On 11/17/20 1:28 AM, AKASHI Takahiro wrote:
>> >> > This is a utility mainly for test purpose.
>> >> >    mkeficapsule -f: create a test capsule file for FIT image
>> >firmware
>> >> > 
>> >> > Having said that, you will be able to customize the code to fit
>> >> > your specific requirements for your platform.
>> >> > 
>> >> > Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
>> >> > ---
>> >> >   tools/Makefile       |   2 +
>> >> >   tools/mkeficapsule.c | 238
>> >+++++++++++++++++++++++++++++++++++++++++++
>> >> >   2 files changed, 240 insertions(+)
>> >> >   create mode 100644 tools/mkeficapsule.c
>> >> > 
>> >> > diff --git a/tools/Makefile b/tools/Makefile
>> >> > index 51123fd92983..66d9376803e3 100644
>> >> > --- a/tools/Makefile
>> >> > +++ b/tools/Makefile
>> >> > @@ -218,6 +218,8 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs
>> >> >   hostprogs-$(CONFIG_ASN1_COMPILER)     += asn1_compiler
>> >> >   HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include
>> >> > 
>> >> > +hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule
>> >> > +
>> >> >   # We build some files with extra pedantic flags to try to
>> >minimize things
>> >> >   # that won't build on some weird host compiler -- though there
>> >are lots of
>> >> >   # exceptions for files that aren't complaint.
>> >> > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
>> >> > new file mode 100644
>> >> > index 000000000000..db95426457cc
>> >> > --- /dev/null
>> >> > +++ b/tools/mkeficapsule.c
>> >> > @@ -0,0 +1,238 @@
>> >> > +// SPDX-License-Identifier: GPL-2.0
>> >> > +/*
>> >> > + * Copyright 2018 Linaro Limited
>> >> > + *             Author: AKASHI Takahiro
>> >> > + */
>> >> > +
>> >> > +#include <getopt.h>
>> >> > +#include <malloc.h>
>> >> > +#include <stdbool.h>
>> >> > +#include <stdio.h>
>> >> > +#include <stdlib.h>
>> >> > +#include <string.h>
>> >> > +#include <linux/types.h>
>> >> > +#include <sys/stat.h>
>> >> > +#include <sys/types.h>
>> >> > +
>> >> > +typedef __u8 u8;
>> >> > +typedef __u16 u16;
>> >> > +typedef __u32 u32;
>> >> > +typedef __u64 u64;
>> >> > +typedef __s16 s16;
>> >> > +typedef __s32 s32;
>> >> > +
>> >> > +#define aligned_u64 __aligned_u64
>> >> > +
>> >> > +#ifndef __packed
>> >> > +#define __packed __attribute__((packed))
>> >> > +#endif
>> >> > +
>> >> > +#include <efi.h>
>> >> > +#include <efi_api.h>
>> >> > +
>> >> > +static const char *tool_name = "mkeficapsule";
>> >> > +
>> >> > +efi_guid_t efi_guid_fm_capsule =
>> >EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
>> >> > +efi_guid_t efi_guid_image_type_uboot_fit =
>> >> > +               EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID;
>> >> > +efi_guid_t efi_guid_image_type_uboot_raw =
>> >> > +               EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
>> >> > +
>> >> > +static struct option options[] = {
>> >> > +       {"fit", required_argument, NULL, 'f'},
>> >> > +       {"raw", required_argument, NULL, 'r'},
>> >> > +       {"index", required_argument, NULL, 'i'},
>> >> > +       {"instance", required_argument, NULL, 'I'},
>> >> > +       {"version", required_argument, NULL, 'v'},
>> >> > +       {"help", no_argument, NULL, 'h'},
>> >> > +       {NULL, 0, NULL, 0},
>> >> > +};
>> >> > +
>> >> > +static void print_usage(void)
>> >> > +{
>> >> > +       printf("Usage: %s [options] <output file>\n"
>> >> > +              "Options:\n"
>> >> > +              "\t--fit <fit image>      new FIT image file\n"
>> >> > +              "\t--raw <raw image>      new raw image file\n"
>> >> > +              "\t--index <index>        update image index\n"
>> >> > +              "\t--instance <instance>  update hardware instance\n"
>> >> > +              "\t--version <version>    firmware version\n"
>> >> > +              "\t--help                 print a help message\n",
>> >> > +              tool_name);
>> >> > +}
>> >> > +
>> >> > +static int create_fwbin(char *path, char *bin, efi_guid_t
>*guid,
>> >> > +                       unsigned long version, unsigned long index,
>> >> > +                       unsigned long instance)
>> >> > +{
>> >> > +       struct efi_capsule_header header;
>> >> > +       struct efi_firmware_management_capsule_header capsule;
>> >> > +       struct efi_firmware_management_capsule_image_header image;
>> >> > +       FILE *f, *g;
>> >> > +       struct stat bin_stat;
>> >> > +       u8 *data;
>> >> > +       size_t size;
>> >> > +
>> >> > +#ifdef DEBUG
>> >> > +       printf("For output: %s\n", path);
>> >> > +       printf("\tbin: %s\n\ttype: %pUl\n" bin, guid);
>> >> > +       printf("\tversion: %ld\n\tindex: %ld\n\tinstance: %ld\n",
>> >> > +              version, index, instance);
>> >> > +#endif
>> >> > +
>> >> > +       g = fopen(bin, "r");
>> >> > +       if (!g) {
>> >> > +               printf("cannot open %s\n", bin);
>> >> > +               return -1;
>> >> > +       }
>> >> > +       if (stat(bin, &bin_stat) < 0) {
>> >> > +               printf("cannot determine the size of %s\n", bin);
>> >> > +               goto err_1;
>> >> > +       }
>> >> > +       data = malloc(bin_stat.st_size);
>> >> > +       if (!data) {
>> >> > +               printf("cannot allocate memory: %lx\n", 
>> >> > bin_stat.st_size);
>> >> > +               goto err_1;
>> >> > +       }
>> >> > +       f = fopen(path, "w");
>> >> > +       if (!f) {
>> >> > +               printf("cannot open %s\n", path);
>> >> > +               goto err_2;
>> >> > +       }
>> >> > +       header.capsule_guid = efi_guid_fm_capsule;
>> >> > +       header.header_size = sizeof(header);
>> >> > +       header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET; /* TODO */
>> >> > +       header.capsule_image_size = sizeof(header)
>> >> > +                                       + sizeof(capsule) + sizeof(u64)
>> >> > +                                       + sizeof(image)
>> >> > +                                       + bin_stat.st_size;
>> >> > +
>> >> > +       size = fwrite(&header, 1, sizeof(header), f);
>> >> > +       if (size < sizeof(header)) {
>> >> > +               printf("write failed (%lx)\n", size);
>> >> > +               goto err_3;
>> >> > +       }
>> >> > +
>> >> > +       capsule.version = 0x00000001;
>> >> > +       capsule.embedded_driver_count = 0;
>> >> > +       capsule.payload_item_count = 1;
>> >> > +       capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64);
>> >> 
>> >> With the complete series applied, building sandbox_defconfig:
>> >> 
>> >> tools/mkeficapsule.c: In function ‘main’:
>> >> tools/mkeficapsule.c:120:26: warning: array subscript 0 is outside
>> >array
>> >> bounds of ‘u64[]’ {aka ‘long long unsigned int[]’}
>[-Warray-bounds]
>> >>   120 |  capsule.item_offset_list[0] = sizeof(capsule) +
>sizeof(u64);
>> >>       |  ~~~~~~~~~~~~~~~~~~~~~~~~^~~
>> >> 
>> >> Please, ensure that the code compiles without warnings.
>> >
>> >Fixing this warning would be easy, but I didn't see it
>> >with gcc9.2 (from Linaro/Arm) or even in the results of Travis CI.
>> >
>> >So I wonder if it is mandatory that the code compiles without
>warnings,
>> >and if so, which compiler and which version are required?
>
>First, can you please make a comment here against my question?
>
>> >
>> >-Takahiro Akashi
>> >
>> 
>> Our settings for gitlab CI and Travis CI are that all warnings are
>treated as errors.
>
>As I said, I've never seen this warning/error in Travis CI.
>I don't know how we can confirm the result of gitlab CI.
>
>-Takahiro Akashi


Just make sure that GCC 10+ does not complain locally.

Tom will update the CI in January. I dont want to see a build error then.

Best regards

Heinrich

>
>
>> So we must build without warning.
>> 
>> Best regards
>> 
>> Heinrich
>> 
>> >
>> >> I have been using GCC 10.2 as supplied by Debian Bullseye on an
>ARM64
>> >> system.
>> >> 
>> >> Best regards
>> >> 
>> >> Heinrich
>> >> 
>> >> > +       size = fwrite(&capsule, 1, sizeof(capsule) + sizeof(u64), f);
>> >> > +       if (size < (sizeof(capsule) + sizeof(u64))) {
>> >> > +               printf("write failed (%lx)\n", size);
>> >> > +               goto err_3;
>> >> > +       }
>> >> > +
>> >> > +       image.version = version;
>> >> > +       memcpy(&image.update_image_type_id, guid, sizeof(*guid));
>> >> > +       image.update_image_index = index;
>> >> > +       image.update_image_size = bin_stat.st_size;
>> >> > +       image.update_vendor_code_size = 0; /* none */
>> >> > +       image.update_hardware_instance = instance;
>> >> > +       image.image_capsule_support = 0;
>> >> > +
>> >> > +       size = fwrite(&image, 1, sizeof(image), f);
>> >> > +       if (size < sizeof(image)) {
>> >> > +               printf("write failed (%lx)\n", size);
>> >> > +               goto err_3;
>> >> > +       }
>> >> > +       size = fread(data, 1, bin_stat.st_size, g);
>> >> > +       if (size < bin_stat.st_size) {
>> >> > +               printf("read failed (%lx)\n", size);
>> >> > +               goto err_3;
>> >> > +       }
>> >> > +       size = fwrite(data, 1, bin_stat.st_size, f);
>> >> > +       if (size < bin_stat.st_size) {
>> >> > +               printf("write failed (%lx)\n", size);
>> >> > +               goto err_3;
>> >> > +       }
>> >> > +
>> >> > +       fclose(f);
>> >> > +       fclose(g);
>> >> > +       free(data);
>> >> > +
>> >> > +       return 0;
>> >> > +
>> >> > +err_3:
>> >> > +       fclose(f);
>> >> > +err_2:
>> >> > +       free(data);
>> >> > +err_1:
>> >> > +       fclose(g);
>> >> > +
>> >> > +       return -1;
>> >> > +}
>> >> > +
>> >> > +/*
>> >> > + * Usage:
>> >> > + *   $ mkeficapsule -f <firmware binary> <output file>
>> >> > + */
>> >> > +int main(int argc, char **argv)
>> >> > +{
>> >> > +       char *file;
>> >> > +       efi_guid_t *guid;
>> >> > +       unsigned long index, instance, version;
>> >> > +       int c, idx;
>> >> > +
>> >> > +       file = NULL;
>> >> > +       guid = NULL;
>> >> > +       index = 0;
>> >> > +       instance = 0;
>> >> > +       version = 0;
>> >> > +       for (;;) {
>> >> > +               c = getopt_long(argc, argv, "f:r:i:I:v:h", options, 
>> >> > &idx);
>> >> > +               if (c == -1)
>> >> > +                       break;
>> >> > +
>> >> > +               switch (c) {
>> >> > +               case 'f':
>> >> > +                       if (file) {
>> >> > +                               printf("Image already specified\n");
>> >> > +                               return -1;
>> >> > +                       }
>> >> > +                       file = optarg;
>> >> > +                       guid = &efi_guid_image_type_uboot_fit;
>> >> > +                       break;
>> >> > +               case 'r':
>> >> > +                       if (file) {
>> >> > +                               printf("Image already specified\n");
>> >> > +                               return -1;
>> >> > +                       }
>> >> > +                       file = optarg;
>> >> > +                       guid = &efi_guid_image_type_uboot_raw;
>> >> > +                       break;
>> >> > +               case 'i':
>> >> > +                       index = strtoul(optarg, NULL, 0);
>> >> > +                       break;
>> >> > +               case 'I':
>> >> > +                       instance = strtoul(optarg, NULL, 0);
>> >> > +                       break;
>> >> > +               case 'v':
>> >> > +                       version = strtoul(optarg, NULL, 0);
>> >> > +                       break;
>> >> > +               case 'h':
>> >> > +                       print_usage();
>> >> > +                       return 0;
>> >> > +               }
>> >> > +       }
>> >> > +
>> >> > +       /* need a output file */
>> >> > +       if (argc != optind + 1) {
>> >> > +               print_usage();
>> >> > +               return -1;
>> >> > +       }
>> >> > +
>> >> > +       /* need a fit image file or raw image file */
>> >> > +       if (!file) {
>> >> > +               print_usage();
>> >> > +               return -1;
>> >> > +       }
>> >> > +
>> >> > +       if (create_fwbin(argv[optind], file, guid, version, index,
>> >instance)
>> >> > +                       < 0) {
>> >> > +               printf("Creating firmware capsule failed\n");
>> >> > +               return -1;
>> >> > +       }
>> >> > +
>> >> > +       return 0;
>> >> > +}
>> >> > 
>> >> 
>> 

Reply via email to