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

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

Hi Simon,

[...]

> > > > - remove unnecessary const qualifier from smbios_string()
> > > > - create non-const version of next_header()
> > > >
> > > >  include/efi_loader.h          |   2 +
> > > >  include/efi_tcg2.h            |  15 ++++
> > > >  include/smbios.h              |  17 +++-
> > > >  lib/efi_loader/Kconfig        |   1 +
> > > >  lib/efi_loader/efi_boottime.c |   2 +
> > > >  lib/efi_loader/efi_smbios.c   |   2 -
> > > >  lib/efi_loader/efi_tcg2.c     |  84 +++++++++++++++++++
> > > >  lib/smbios-parser.c           | 152 +++++++++++++++++++++++++++++++---
> > > >  8 files changed, 261 insertions(+), 14 deletions(-)
> > >
> > > Where are the tests for this new code, please?
> >
> > 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.

> 
> > 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....

> 
> > and changed the tpm2 spi driver we have,  but I can't test it yet,
> > since I don't have a device for that.
> 
> OK I think we are both going to get one.
> 
> [..]
> 
> Regards,
> SImon

Regards
/Ilias