[PATCH v2 1/3] efi_loader: add SMBIOS table measurement

[Simon Glass]

Hi Ilias,

On Tue, 28 Sept 2021 at 11:41, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Simon,
>
>
> [...]
> > > > > We've mentioned this in the past.  The sandbox TPM is very limited wrt
> > > > > tpm testing for the EFI TCG protocol.
> > > >
> > > > So let's add some more features? If it helps, think of the sandbox TPM
> > > > as test code, not an emulator. It is a very simple kind of emulator to
> > > > allow tests to work.
> > >
> > > The amount of features needed to test EFI TCG are not minimal.  Since I'll
> > > upstream the mmio tpm anyway,  we'll just test TCG there.  If someone wants
> > > to go ahead and make the sandbox TPM a TIS compliant device that covers the
> > > requirements of the EFI TCG,  I am fine using it.
> >
> > Do you know how many features? There's 250 LOC in this patch.
>
> I haven't checked for a while but back when I tested it
> tpm2_get_capability() was failing on a number of cases.  The EFI TCG
> code expects:
> - TPM2_PT_MAX_COMMAND_SIZE
> - TPM2_PT_MAX_RESPONSE_SIZE
> - TPM2_PT_MANUFACTURER
> - TPM2_PT_PCR_COUNT
> - TPM2_CAP_PCRS
> when querying capabilities.  Ideally we'd also want to extend more
> than 1 PCRs and verify that worked correctly.

That doesn't seem like much. As you say I already added support for
multiple PCRs. I think we should do it and add a test.

>
> >
> > >
> > > >
> > > > > I did send TPM MMIO patches a while back [1].  This would allow us to
> > > > > test everything under QEMU,  but you asked for *another* device to be
> > > > > part of the API I posted (apart from the MMIO).  I've found some time
> > > >
> > > > Yes that is because if you just add a new protocol you have not made
> > > > anything better, just added one more way of doing things.
> > >
> > > Our perspective of 'better' seems to be different.
> > >
> > > I added a TIS API for any driver to use.  I actually did 2 iterations of
> > > the driver.  The first one was replicating all the code and you said 'why
> > > are we replicating code',  which was done already in a bunch of drivers
> > > already...
> > > Then I added an API and a driver using it but you wanted to convert more
> > > *existing* drivers to the API before merging it. But the fact is that if
> > > anyone wants to add a new driver he has to code  ~900 lines instead of the
> > > ~150 needed with the API in place,  not to mention the duplication of bugs
> > > all over the place....
> >
> > It would be like adding a new filesystem in U-Boot with its own new
> > framework for filesystems. It creates technical debt and we don't know
> > if anyone will actually use it.
> >
> > https://xkcd.com/927/
> >
> > I think your API is a great idea but we need some effort to migrate to
> > it, to avoid the problem above. After all, who else is going to do it?
>
> I ordered the SPI TPM, so hopefully, I'll be able to have the MMIO and
> SPI drivers using it!

Snap!

Regards,
Simon