[PATCH v3 2/6] tpm: Support boot measurements

[Ilias Apalodimas]

Hi Simon,

[...]

> > >>
> > > [..]
> > >
> > >> +static int tcg2_log_init(struct udevice *dev, struct tcg2_event_log *elog)
> > >> +{
> > >> +       struct tcg_efi_spec_id_event *ev;
> > > We cannot add EFI things to generic TPM code.
> >
> >
> > Ah, this is NOT an EFI thing even though it is named as such. Please see
> > https://trustedcomputinggroup.org/wp-content/uploads/TCG_ServerManagDomainFWProfile_r1p00_pub.pdf
> > section 9 and
> > https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_Specific_Platform_Profile_for_TPM_2p0_1p04_PUBLIC.pdf
> > section 9
> >
> >
> > Neither of these documents are specific to EFI.
>
> OK, then please drop efi_ from the identifiers. It is terribly confusing.

Why?  The terrible confusing part would be trying to read code not
named after the spec because you don't like EFI.  When looking at code
that's documented by a spec it's far easier to keep the naming as
close as possible, so I'd prefer leaving it as is.

Regards
/Ilias
>
> >
> >
> > >
> > >> +       struct tcg_pcr_event *log;
> > >> +       u32 event_size;
> > >> +       u32 count = 0;
> > >> +       u32 log_size;
> > >> +       u32 active;
> > >> +       u32 mask;
> > >> +       size_t i;
> > >> +       u16 len;
> > >> +       int rc;
> > >> +
> > >> +       rc = tcg2_get_active_pcr_banks(dev, &active);
> > >> +       if (rc)
> > >> +               return rc;
> > >> +
> > >> +       event_size = offsetof(struct tcg_efi_spec_id_event, digest_sizes);
> > >> +       for (i = 0; i < ARRAY_SIZE(tcg2algos); ++i) {
> > >> +               mask = tpm2_algorithm_to_mask(tcg2algos[i]);
> > >> +
> > >> +               if (!(active & mask))
> > >> +                       continue;
> > >> +
> > >> +               switch (tcg2algos[i]) {
> > >> +               case TPM2_ALG_SHA1:
> > >> +               case TPM2_ALG_SHA256:
> > >> +               case TPM2_ALG_SHA384:
> > >> +               case TPM2_ALG_SHA512:
> > >> +                       count++;
> > >> +                       break;
> > >> +               default:
> > >> +                       continue;
> > >> +               }
> > >> +       }
> > >> +
> > >> +       event_size += 1 +
> > >> +               (sizeof(struct tcg_efi_spec_id_event_algorithm_size) * count);
> > >> +       log_size = offsetof(struct tcg_pcr_event, event) + event_size;
> > >> +
> > >> +       if (log_size > elog->log_size) {
> > >> +               printf("%s: log too large: %u > %u\n", __func__, log_size,
> > >> +                      elog->log_size);
> > >> +               return -ENOBUFS;
> > >> +       }
> > >> +
> > >> +       log = (struct tcg_pcr_event *)elog->log;
> > >> +       put_unaligned_le32(0, &log->pcr_index);
> > >> +       put_unaligned_le32(EV_NO_ACTION, &log->event_type);
> > >> +       memset(&log->digest, 0, sizeof(log->digest));
> > >> +       put_unaligned_le32(event_size, &log->event_size);
> > >> +
> > >> +       ev = (struct tcg_efi_spec_id_event *)log->event;
> > >> +       strlcpy((char *)ev->signature, TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03,
> > > Same with all of this.
> > >
> > >> +               sizeof(ev->signature));
> > >> +       put_unaligned_le32(0, &ev->platform_class);
> > >> +       ev->spec_version_minor = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MINOR_TPM2;
> > >> +       ev->spec_version_major = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MAJOR_TPM2;
> > >> +       ev->spec_errata = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_ERRATA_TPM2;
> > > I'm not quite sure what is going on here...is this log in a format
> > > defined by the EFI spec? What if we are not using EFI? How would a
> > > different format be used?
> > >
> > > Put another way, people using a TPM should not pull in EFI things just
> > > to do that.
> >
> >
> > Agreed, however the EFI spec is not involved. These specifications and
> > structures are general to any boot measurement. I would guess EFI was
> > the first to do this and therefore defined some structures that the TCG
> > re-used when writing the specs.
>
> Regards,
> Simon