Re: [PATCH v2] Documentation: tpm_tis

[Jarkko Sakkinen]

On Wed Mar 20, 2024 at 6:15 PM EET, Stefan Berger wrote:
>
>
> On 3/20/24 04:56, Jarkko Sakkinen wrote:
> > Based recent discussions on LKML, provide preliminary bits of tpm_tis_core
> > dependent drivers. Includes only bare essentials but can be extended later
> > on case by case. This way some people may even want to read it later on.
> > 
> > Cc: Jonathan Corbet <corbet@xxxxxxx>
> > CC: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> > Cc: Lino Sanfilippo <l.sanfilippo@xxxxxxxxxx>
> > Cc: Jason Gunthorpe <jgg@xxxxxxxx>
> > Cc: Peter Huewe <peterhuewe@xxxxxx>
> > Cc: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
> > Cc: Alexander Steffen <Alexander.Steffen@xxxxxxxxxxxx>
> > Cc: keyrings@xxxxxxxxxxxxxxx
> > Cc: linux-doc@xxxxxxxxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > Cc: linux-integrity@xxxxxxxxxxxxxxx
> > Cc: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
> > Signed-off-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
> > ---
> > v2:
> > - Fixed errors reported by Randy:
> >    https://lore.kernel.org/all/aed28265-d677-491a-a045-24b351854b24@xxxxxxxxxxxxx/
> > - Improved the text a bit to have a better presentation.
> > ---
> >   Documentation/security/tpm/index.rst   |  1 +
> >   Documentation/security/tpm/tpm_tis.rst | 30 ++++++++++++++++++++++++++
> >   2 files changed, 31 insertions(+)
> >   create mode 100644 Documentation/security/tpm/tpm_tis.rst
> > 
> > diff --git a/Documentation/security/tpm/index.rst b/Documentation/security/tpm/index.rst
> > index fc40e9f23c85..f27a17f60a96 100644
> > --- a/Documentation/security/tpm/index.rst
> > +++ b/Documentation/security/tpm/index.rst
> > @@ -5,6 +5,7 @@ Trusted Platform Module documentation
> >   .. toctree::
> >   
> >      tpm_event_log
> > +   tpm_tis
> >      tpm_vtpm_proxy
> >      xen-tpmfront
> >      tpm_ftpm_tee
> > diff --git a/Documentation/security/tpm/tpm_tis.rst b/Documentation/security/tpm/tpm_tis.rst
> > new file mode 100644
> > index 000000000000..b331813b3c45
> > --- /dev/null
> > +++ b/Documentation/security/tpm/tpm_tis.rst
> > @@ -0,0 +1,30 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +=========================
> > +TPM FIFO interface Driver
> > +=========================
> > +
> > +FIFO (First-In-First-Out) is the name of the hardware interface used by the
>
> FIFO is the type. I am surprised you call it a 'name'. I would say TIS 
> is the 'name'.

It's what the official specification calls it [1].


>
> > +tpm_tis_core dependent drivers. The prefix "tis" comes from the TPM Interface
>
> tis is a tla -- a three letter *acronym*. You aren't using it as a 'prefix'.

I don't know what "tla" means.

>
> > +Specification, which is the hardware interface specification for TPM 1x chips.
>
> It's also available for TPM2.
 
Yes, but TIS is the name used by the legacy specification.

>
> > +
> > +Communication is based on a 5 KiB buffer shared by the TPM chip through a
>
> I thought it was typically 4 KiB.

You are basing this on table 9 in [1]?

>
> > +hardware bus or memory map, depending on the physical wiring. The buffer is
> > +further split into five equal-size buffers, which provide equivalent sets of
>
> equal-sized MMIO regions?

I'm not sure what spec you are referring to but [1] defines also other
communication paths.

>
> > +registers for communication between the CPU and TPM. These communication
> > +endpoints are called localities in the TCG terminology.
> > +
> > +When the kernel wants to send commands to the TPM chip, it first reserves
> > +locality 0 by setting the requestUse bit in the TPM_ACCESS register. The bit is
> > +cleared by the chip when the access is granted. Once it completes its
> > +communication, the kernel writes the TPM_ACCESS.activeLocality bit. This
> > +informs the chip that the locality has been relinquished.
> > +
> > +Pending localities are served in order by the chip in descending order, one at
> > +a time:
>
> I think I know what pending localities are because I have worked with 
> this device but I am not sure whether the user can deduce this from the 
> paragraph above. Also, why this particular detail when the driver only 
> uses locality 0 and nobody is competing about access to localities?

This is pretty good summary that is IMHO somewhat useful.

You are welcome to contribute to the documentation but it has to start
from something.

>
> > +
> > +- Locality 0 has the lowest priority.
> > +- Locality 5 has the highest priority.
> > +
> > +Further information on the purpose and meaning of the localities can be found
> > +in section 3.2 of the TCG PC Client Platform TPM Profile Specification.
o


[1] https://trustedcomputinggroup.org/resource/pc-client-platform-tpm-profile-ptp-specification/

BR, Jarkko