Re: [PATCH v7 12/13] mkeficapsule: Add support for setting OEM flags in capsule header

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Sughosh,

On Thu, 14 Jul 2022 at 21:40, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> Add support for setting OEM flags in the capsule header. As per the
> UEFI specification, bits 0-15 of the flags member of the capsule
> header can be defined per capsule GUID.
>
> The oemflags will be used for the FWU Multi Bank update feature, as
> specified by the Dependable Boot specification[1]. Bit
> 15 of the flags member will be used to determine if the
> acceptance/rejection of the updated images is to be done by the
> firmware or an external component like the OS.

Have we documented bit15 in the documentation? If not please add it.

>
> [1] - https://git.codelinaro.org/linaro/dependable-boot/mbfw/uploads/6f7ddfe3be24e18d4319e108a758d02e/mbfw.pdf
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> Changes since V6: None
>
>  doc/mkeficapsule.1   |  4 ++++
>  tools/mkeficapsule.c | 17 ++++++++++++++---
>  2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1
> index 77ca061efd..6fb2dd0810 100644
> --- a/doc/mkeficapsule.1
> +++ b/doc/mkeficapsule.1
> @@ -72,6 +72,10 @@ Generate a firmware acceptance empty capsule
>  .BI "-R\fR,\fB --fw-revert "
>  Generate a firmware revert empty capsule
>
> +.TP
> +.BI "-o\fR,\fB --capoemflag "
> +Capsule OEM flag, value between 0x0000 to 0xffff
> +
>  .TP
>  .BR -h ", " --help
>  Print a help message
> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> index 244c80e1f7..237c1218fd 100644
> --- a/tools/mkeficapsule.c
> +++ b/tools/mkeficapsule.c
> @@ -29,7 +29,7 @@ static const char *tool_name = "mkeficapsule";
>  efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
>  efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
>
> -static const char *opts_short = "g:i:I:v:p:c:m:dhAR";
> +static const char *opts_short = "g:i:I:v:p:c:m:o:dhAR";
>
>  enum {
>         CAPSULE_NORMAL_BLOB = 0,
> @@ -47,6 +47,7 @@ static struct option options[] = {
>         {"dump-sig", no_argument, NULL, 'd'},
>         {"fw-accept", no_argument, NULL, 'A'},
>         {"fw-revert", no_argument, NULL, 'R'},
> +       {"capoemflag", required_argument, NULL, 'o'},
>         {"help", no_argument, NULL, 'h'},
>         {NULL, 0, NULL, 0},
>  };
> @@ -65,6 +66,7 @@ static void print_usage(void)
>                 "\t-d, --dump_sig              dump signature (*.p7)\n"
>                 "\t-A, --fw-accept  firmware accept capsule, requires GUID, no image blob\n"
>                 "\t-R, --fw-revert  firmware revert capsule, takes no GUID, no image blob\n"
> +               "\t-o, --capoemflag Capsule OEM Flag, an integer between 0x0000 and 0xffff\n"
>                 "\t-h, --help                  print a help message\n",
>                 tool_name);
>  }
> @@ -387,6 +389,7 @@ static void free_sig_data(struct auth_context *ctx)
>   * @mcount:    Monotonic count in authentication information
>   * @private_file:      Path to a private key file
>   * @cert_file: Path to a certificate file
> + * @oemflags:  Capsule OEM Flags, bits 0-15
>   *
>   * This function actually does the job of creating an uefi capsule file.
>   * All the arguments must be supplied.
> @@ -399,7 +402,8 @@ static void free_sig_data(struct auth_context *ctx)
>   */
>  static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>                         unsigned long index, unsigned long instance,
> -                       uint64_t mcount, char *privkey_file, char *cert_file)
> +                       uint64_t mcount, char *privkey_file, char *cert_file,
> +                       uint16_t oemflags)
>  {
>         struct efi_capsule_header header;
>         struct efi_firmware_management_capsule_header capsule;
> @@ -464,6 +468,8 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>         header.header_size = sizeof(header);
>         /* TODO: The current implementation ignores flags */
>         header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET;
> +       if (oemflags)
> +               header.flags |= oemflags;
>         header.capsule_image_size = sizeof(header)
>                                         + sizeof(capsule) + sizeof(uint64_t)
>                                         + sizeof(image)
> @@ -635,6 +641,7 @@ int main(int argc, char **argv)
>         unsigned char uuid_buf[16];
>         unsigned long index, instance;
>         uint64_t mcount;
> +       uint16_t oemflags;
>         char *privkey_file, *cert_file;
>         int c, idx;
>
> @@ -646,6 +653,7 @@ int main(int argc, char **argv)
>         cert_file = NULL;
>         dump_sig = 0;
>         capsule_type = CAPSULE_NORMAL_BLOB;
> +       oemflags = 0;
>         for (;;) {
>                 c = getopt_long(argc, argv, opts_short, options, &idx);
>                 if (c == -1)
> @@ -699,6 +707,9 @@ int main(int argc, char **argv)
>                 case 'R':
>                         capsule_type |= CAPSULE_REVERT;
>                         break;
> +               case 'o':
> +                       oemflags = strtoul(optarg, NULL, 0);
> +                       break;
>                 default:
>                         print_usage();
>                         exit(EXIT_SUCCESS);
> @@ -732,7 +743,7 @@ int main(int argc, char **argv)
>                 }
>         } else  if (create_fwbin(argv[argc - 1], argv[argc - 2], guid,
>                                  index, instance, mcount, privkey_file,
> -                                cert_file) < 0) {
> +                                cert_file, oemflags) < 0) {
>                 fprintf(stderr, "Creating firmware capsule failed\n");
>                 exit(EXIT_FAILURE);
>         }
> --
> 2.34.1
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>