[RFC PATCH v3 9/9] mkeficapsule: Add support for generating empty capsules

[Masami Hiramatsu]

Hi,

2022年1月24日(月) 11:08 AKASHI Takahiro <takahiro.akashi at linaro.org>:

> > > > +     memcpy(&header.capsule_guid, &capsule_guid, sizeof(efi_guid_t));
> > > > +     header.header_size = sizeof(header);
> > > > +     header.flags = 0;
> > > > +
> > > > +     if (fw_accept) {
> > > > +             header.capsule_image_size = sizeof(header) + sizeof(efi_guid_t);
> > > > +     } else {
> > > > +             header.capsule_image_size = sizeof(header);
> > > > +     }
> > >
> > > I wonder why we don't need GUID in revert case (and why need GUID
> > > in fw case. Since we want to add A/B update, there seems to be
> > > no ambiguity.
> >
> > The revert capsule is used not as a rejection of a specific individual
> > image, but for reverting the platform to the other bank. Which does
> > not require a image specific GUID.
>
> If so, why not apply the same rule to *accept* case to make the change
> permanent?

Perhaps, we can make a special "acceptance" capsule file which accept
all image types. I guess originally it is considered to test each image by
the image provider and each provider decides the firmware is acceptable
or not. For example, TF-A test program passed the test but U-Boot test
program doesn't. In that case, we may need a partial acceptance flags.

However, at least the DeveloperBox has only one image type and maybe
most of platforms doesn't want to split it. Thus the acceptance image-type
uuid can be optional.

Thank you,

>
> -Takahiro Akashi
>
> > >
> > > > +     if (write_capsule_file(f, &header, sizeof(header),
> > > > +                            "Capsule header"))
> > > > +             goto err;
> > > > +
> > > > +     if (fw_accept) {
> > > > +             memcpy(&payload, guid, sizeof(efi_guid_t));
> > > > +             if (write_capsule_file(f, &payload, sizeof(payload),
> > > > +                                    "FW Accept Capsule Payload"))
> > > > +                     goto err;
> > > > +     }
> > > > +
> > > > +     ret = 0;
> > > > +
> > > > +err:
> > > > +     if (f)
> > > > +             fclose(f);
> > > > +
> > > > +     return ret;
> > > > +}
> > > > +
> > > >  /**
> > > >   * main - main entry function of mkeficapsule
> > > >   * @argc:    Number of arguments
> > > > @@ -616,6 +679,7 @@ int main(int argc, char **argv)
> > > >       unsigned char uuid_buf[16];
> > > >       unsigned long index, instance;
> > > >       uint64_t mcount;
> > > > +     unsigned char accept_fw_capsule, revert_fw_capsule;
> > > >       char *privkey_file, *cert_file;
> > > >       int c, idx;
> > > >
> > > > @@ -625,6 +689,8 @@ int main(int argc, char **argv)
> > > >       mcount = 0;
> > > >       privkey_file = NULL;
> > > >       cert_file = NULL;
> > > > +     accept_fw_capsule = 0;
> > > > +     revert_fw_capsule = 0;
> > > >       dump_sig = 0;
> > > >       for (;;) {
> > > >               c = getopt_long(argc, argv, opts_short, options, &idx);
> > > > @@ -691,22 +757,38 @@ int main(int argc, char **argv)
> > > >                       dump_sig = 1;
> > > >                       break;
> > > >  #endif /* CONFIG_TOOLS_LIBCRYPTO */
> > > > +             case 'A':
> > > > +                     accept_fw_capsule = 1;
> > > > +                     break;
> > > > +             case 'R':
> > > > +                     revert_fw_capsule = 1;
> > > > +                     break;
> > > >               case 'h':
> > > >                       print_usage();
> > > >                       exit(EXIT_SUCCESS);
> > > >               }
> > > >       }
> > > >
> > > > +     empty_capsule = (accept_fw_capsule || revert_fw_capsule);
> > >
> > > Please check that two options are exclusive here.
> >
> > Okay
> >
> > >
> > > >       /* check necessary parameters */
> > > > -     if ((argc != optind + 2) || !guid ||
> > > > -         ((privkey_file && !cert_file) ||
> > > > +     if ((!empty_capsule && argc != optind + 2) ||
> > > > +         (empty_capsule && argc != optind + 1) ||
> > > > +         (!revert_fw_capsule && !guid) || ((privkey_file && !cert_file) ||
> > > >            (!privkey_file && cert_file))) {
> > > >               print_usage();
> > > >               exit(EXIT_FAILURE);
> > > >       }
> > > >
> > > > -     if (create_fwbin(argv[argc - 1], argv[argc - 2], guid, index, instance,
> > > > -                      mcount, privkey_file, cert_file) < 0) {
> > > > +     if (empty_capsule) {
> > > > +             if (create_empty_capsule(argv[argc - 1], guid,
> > > > +                                      accept_fw_capsule ? 1 : 0) < 0) {
> > > > +                     printf("Creating empty capsule failed\n");
> > >
> > > To stderr
> >
> > Okay, will check.
> >
> > -sughosh
> >
> > >
> > > -Takahiro Akashi
> > >
> > > > +                     exit(EXIT_FAILURE);
> > > > +             }
> > > > +     } else  if (create_fwbin(argv[argc - 1], argv[argc - 2], guid,
> > > > +                              index, instance, mcount, privkey_file,
> > > > +                              cert_file) < 0) {
> > > >               fprintf(stderr, "Creating firmware capsule failed\n");
> > > >               exit(EXIT_FAILURE);
> > > >       }
> > > > --
> > > > 2.17.1
> > > >



--
Masami Hiramatsu