Hi,
2022年1月24日(月) 11:08 AKASHI Takahiro <takahiro.aka...@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
