[PATCH 1/2 v2] tpm2: Introduce TIS tpm core

[Simon Glass]

Hi Ilias,

On Mon, 12 Jul 2021 at 08:03, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> > > > >         TPM_STS_SELF_TEST_DONE          = 1 << 2,
>
> [...]
>
> > > > >         TPM_STS_RESPONSE_RETRY          = 1 << 1,
> > > > > +       TPM_STS_READ_ZERO               = 0x23
> > > >
> > > > Does this below in another patch?
> > > >
> > >
> > > It's a general tpm2 update. I can move it to the driver patch if it makes
> > > more sense.
> > >
> > > > >  };
> > > > >
> > > > >  enum {
> > > > > --
> > > > > 2.32.0.rc0
> > > > >
> > > >
> > > > I feel that this API could be useful in reducing code duplication, but
> > > > in fact it has just created more, so far as I can see from this series
> > > > :-) So I think you should convert at least one driver to show its
> > > > value (and not make things any worse).
> > >
> > > The mmio tpm driver uses it and instead of ~700 lines (like the tpmv2 spi
> > > driver) it drops down to ~100.  I don't have access to any other TPM
> > > hardware to rewrite any of those.
> >
> > Yes, but I hope you see my point, that you have added a new interface.
> > It is definitely better than adding a new driver and duplicating all
> > the code, but it is still one more copy and in fact, the code is
> > duplicated.
> >
>
> I get the point but I don't exactly agree here.  It's not duplicated code.
> We need to plug in the mmio driver.  The original code was just doing what
> every TPM does.  It carried the TIS relevant code in the new driver.
> The new approach defines an API for everyone to use and the new driver uses it.
> So I don't see the duplication here.  You need the TIS code one way or the
> other.  Now it's on a common interface for everyone to use.

Well how about converting a TPM blindly and then we'll find someone to test it?

>
> > Can you get access to TPM hardware? I see that you have offered to be
> > the maintainer for this subsystem, so I think that would be useful.
> > Can sandbox use your new API?
>
> It depends, is the sandbox TIS compatible? If it is sure we could use it.

At present sandbox implements the tpm_ops API. So if we did that we
would need to tear it apart to insert this new API as well.

> I offered to maintain the drivers because I wrote the API and I have an
> idea of how TPMs should work.  If that means I'll have to go and get every
> hardware we support, I'll just volunteer into maintaining the TIS layer.
> Moreover I dont see why I should start porting drivers to use that API.
> People decided to duplicate that code in every driver (in fact multiple times).

See https://xkcd.com/927/ :-)

>
> I am happy to work with you on the cr50 i2c driver if that would help.

Sure that might be easier as I can definitely test it.

Regards,
Siomn