From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Wed, 25 Nov 2020 16:32:08 +0900 Subject: [PATCH v9 08/11] tools: add mkeficapsule command for UEFI capsule update In-Reply-To: References: <20201117002805.13902-1-takahiro.akashi@linaro.org> <20201117002805.13902-9-takahiro.akashi@linaro.org> <9a55236d-87a8-6ef2-2142-e826e1952ff1@gmx.de> <20201125010506.GB8300@laputa> Message-ID: <20201125073208.GB62993@laputa> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 : > >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 > >> > --- > >> > 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 > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > + > >> > +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 > >> > +#include > >> > + > >> > +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] \n" > >> > + "Options:\n" > >> > + "\t--fit new FIT image file\n" > >> > + "\t--raw new raw image file\n" > >> > + "\t--index update image index\n" > >> > + "\t--instance update hardware instance\n" > >> > + "\t--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 > 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 > >> > + */ > >> > +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; > >> > +} > >> > > >> >