2024-03-22 12:36:08

by Jarkko Sakkinen

[permalink] [raw]
Subject: [PATCH v4] Documentation: tpm_tis

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 <[email protected]>
CC: Daniel P. Smith <[email protected]>
Cc: Lino Sanfilippo <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: Peter Huewe <[email protected]>
Cc: James Bottomley <[email protected]>
Cc: Alexander Steffen <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Randy Dunlap <[email protected]>
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
v4:
- Extended the text to address some of Stefan's concerns with v2.
- Had to unfortunately remove Randy's reviewed-by because of this, given
the amount of text added.
v3:
- Fixed incorrect buffer size:
https://lore.kernel.org/linux-integrity/[email protected]/
v2:
- Fixed errors reported by Randy:
https://lore.kernel.org/all/[email protected]/
- Improved the text a bit to have a better presentation.
---
Documentation/security/tpm/index.rst | 1 +
Documentation/security/tpm/tpm_tis.rst | 46 ++++++++++++++++++++++++++
2 files changed, 47 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..b448ea3db71d
--- /dev/null
+++ b/Documentation/security/tpm/tpm_tis.rst
@@ -0,0 +1,46 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=========================
+TPM FIFO interface driver
+=========================
+
+TCG PTP Specification defines two interface types: FIFO and CRB. The former is
+based on sequenced read and write operations, and the latter is based on a
+buffer containing the full command or response.
+
+FIFO (First-In-First-Out) interface is used by the tpm_tis_core dependent
+drivers. Originally Linux had only a driver called tpm_tis, which covered
+memory mapped (aka MMIO) interface but it was later on extended to cover other
+physical interfaces supported by the TCG standard.
+
+For legacy compliance the original MMIO driver is called tpm_tis and the
+framework for FIFO drivers is named as tpm_tis_core. The postfix "tis" in
+tpm_tis comes from the TPM Interface Specification, which is the hardware
+interface specification for TPM 1.x chips.
+
+Communication is based on a 20 KiB buffer shared by the TPM chip through a
+hardware bus or memory map, depending on the physical wiring. The buffer is
+further split into five equal-size 4 KiB buffers, which provide equivalent
+sets of 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:
+
+- 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.
+
+References
+==========
+
+TCG PC Client Platform TPM Profile (PTP) Specification
+https://trustedcomputinggroup.org/resource/pc-client-platform-tpm-profile-ptp-specification/
--
2.43.0



2024-03-22 22:52:48

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH v4] Documentation: tpm_tis

On Fri, 22 Mar 2024 14:35:36 +0200 Jarkko Sakkinen wrote:
> +TCG PTP Specification defines two interface types: FIFO and CRB. The former is

Could be worth spelling out the PTP part here, I'm guessing
get_maintainer made you CC netdev because it thought it stands
for Precision Time Protocol. And one has to read till the end
to see:

> +TCG PC Client Platform TPM Profile (PTP) Specification

2024-03-23 00:40:07

by Daniel P. Smith

[permalink] [raw]
Subject: Re: [PATCH v4] Documentation: tpm_tis

Hi Jarkko,

On 3/22/24 08:35, 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 <[email protected]>
> CC: Daniel P. Smith <[email protected]>
> Cc: Lino Sanfilippo <[email protected]>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: Peter Huewe <[email protected]>
> Cc: James Bottomley <[email protected]>
> Cc: Alexander Steffen <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: Randy Dunlap <[email protected]>
> Signed-off-by: Jarkko Sakkinen <[email protected]>
> ---
> v4:
> - Extended the text to address some of Stefan's concerns with v2.
> - Had to unfortunately remove Randy's reviewed-by because of this, given
> the amount of text added.
> v3:
> - Fixed incorrect buffer size:
> https://lore.kernel.org/linux-integrity/[email protected]/
> v2:
> - Fixed errors reported by Randy:
> https://lore.kernel.org/all/[email protected]/
> - Improved the text a bit to have a better presentation.
> ---
> Documentation/security/tpm/index.rst | 1 +
> Documentation/security/tpm/tpm_tis.rst | 46 ++++++++++++++++++++++++++
> 2 files changed, 47 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..b448ea3db71d
> --- /dev/null
> +++ b/Documentation/security/tpm/tpm_tis.rst
> @@ -0,0 +1,46 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=========================
> +TPM FIFO interface driver
> +=========================
> +
> +TCG PTP Specification defines two interface types: FIFO and CRB. The former is

I believe in the spec, the authors were specific to classify these as
software interfaces. Not sure if you would want to carry that
distinction into this document.

> +based on sequenced read and write operations, and the latter is based on a
> +buffer containing the full command or response.
> +
> +FIFO (First-In-First-Out) interface is used by the tpm_tis_core dependent
> +drivers. Originally Linux had only a driver called tpm_tis, which covered
> +memory mapped (aka MMIO) interface but it was later on extended to cover other
> +physical interfaces supported by the TCG standard.

Would it be worth clarifying here that one of those interfaces is
defined in the Mobile TPM specification, which also refers to its
interface as the CRB interface. In the past, this has caused great
confusion when working with individuals from the embedded community,
e.g., Arm. The Mobile TPM CRB interface, which can also be found being
used by some generations of AMD fTPM, is a doorbell style interface
using general-purpose memory. I would also point out that the Mobile TPM
CRB interface does not provide for the concept of localities.

In relation to the MMIO backed interfaces, I have heard comment that the
software interfaces were not meant to require the physical interface be
MMIO. In fact, in section 9.2, "Hardware Implementation of a TPM in a PC
Client Platform", there is a comment about Locality 4 registers being
accessible via an implementation specific mechanism other than MMIO.
Additionally, there were some discussions about clarifying the PTP on
how the software interfaces might be expected to work for a
general-purpose memory backed implementation.

> +For legacy compliance the original MMIO driver is called tpm_tis and the
> +framework for FIFO drivers is named as tpm_tis_core. The postfix "tis" in
> +tpm_tis comes from the TPM Interface Specification, which is the hardware
> +interface specification for TPM 1.x chips.
> +
> +Communication is based on a 20 KiB buffer shared by the TPM chip through a
> +hardware bus or memory map, depending on the physical wiring. The buffer is
> +further split into five equal-size 4 KiB buffers, which provide equivalent
> +sets of 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 get what you are trying to say, but I find the wording here
could be a bit misleading. Instead of saying they are served in order, I
would suggest saying something to the extent that: there are five
localities, more than one can be requested at a time, but only one will
ever be active. Selection priority when multiple requests are pending is
detailed in the Informative comment on locality priority in Section
6.2.1, "TPM Locality Levels".

> +- Locality 0 has the lowest priority.
> +- Locality 5 has the highest priority.

Do you mean Locality 4?

> +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.
> +
> +References
> +==========
> +
> +TCG PC Client Platform TPM Profile (PTP) Specification
> +https://trustedcomputinggroup.org/resource/pc-client-platform-tpm-profile-ptp-specification/

2024-03-23 18:25:59

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v4] Documentation: tpm_tis

On Sat Mar 23, 2024 at 12:52 AM EET, Jakub Kicinski wrote:
> On Fri, 22 Mar 2024 14:35:36 +0200 Jarkko Sakkinen wrote:
> > +TCG PTP Specification defines two interface types: FIFO and CRB. The former is
>
> Could be worth spelling out the PTP part here, I'm guessing
> get_maintainer made you CC netdev because it thought it stands
> for Precision Time Protocol. And one has to read till the end
> to see:
>
> > +TCG PC Client Platform TPM Profile (PTP) Specification

Thanks! Good remark.

BR, Jarkko

2024-03-23 18:41:06

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v4] Documentation: tpm_tis

On Sat Mar 23, 2024 at 2:39 AM EET, Daniel P. Smith wrote:
> Hi Jarkko,
>
> On 3/22/24 08:35, 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 <[email protected]>
> > CC: Daniel P. Smith <[email protected]>
> > Cc: Lino Sanfilippo <[email protected]>
> > Cc: Jason Gunthorpe <[email protected]>
> > Cc: Peter Huewe <[email protected]>
> > Cc: James Bottomley <[email protected]>
> > Cc: Alexander Steffen <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: Randy Dunlap <[email protected]>
> > Signed-off-by: Jarkko Sakkinen <[email protected]>
> > ---
> > v4:
> > - Extended the text to address some of Stefan's concerns with v2.
> > - Had to unfortunately remove Randy's reviewed-by because of this, given
> > the amount of text added.
> > v3:
> > - Fixed incorrect buffer size:
> > https://lore.kernel.org/linux-integrity/[email protected]/
> > v2:
> > - Fixed errors reported by Randy:
> > https://lore.kernel.org/all/[email protected]/
> > - Improved the text a bit to have a better presentation.
> > ---
> > Documentation/security/tpm/index.rst | 1 +
> > Documentation/security/tpm/tpm_tis.rst | 46 ++++++++++++++++++++++++++
> > 2 files changed, 47 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..b448ea3db71d
> > --- /dev/null
> > +++ b/Documentation/security/tpm/tpm_tis.rst
> > @@ -0,0 +1,46 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +=========================
> > +TPM FIFO interface driver
> > +=========================
> > +
> > +TCG PTP Specification defines two interface types: FIFO and CRB. The former is
>
> I believe in the spec, the authors were specific to classify these as
> software interfaces. Not sure if you would want to carry that
> distinction into this document.
>
> > +based on sequenced read and write operations, and the latter is based on a
> > +buffer containing the full command or response.
> > +
> > +FIFO (First-In-First-Out) interface is used by the tpm_tis_core dependent
> > +drivers. Originally Linux had only a driver called tpm_tis, which covered
> > +memory mapped (aka MMIO) interface but it was later on extended to cover other
> > +physical interfaces supported by the TCG standard.
>
> Would it be worth clarifying here that one of those interfaces is
> defined in the Mobile TPM specification, which also refers to its
> interface as the CRB interface. In the past, this has caused great
> confusion when working with individuals from the embedded community,
> e.g., Arm. The Mobile TPM CRB interface, which can also be found being
> used by some generations of AMD fTPM, is a doorbell style interface
> using general-purpose memory. I would also point out that the Mobile TPM
> CRB interface does not provide for the concept of localities.

I don't necessarily disagree but it is out of scope for this. I'm not
sure tho why "mobile" CRB would ever need that sort of separate
dicussion.

Some CRB implementations have localities some don't, and also fTPM
implementations on x86 vary, no need to state that separately for
mobile.

> In relation to the MMIO backed interfaces, I have heard comment that the
> software interfaces were not meant to require the physical interface be
> MMIO. In fact, in section 9.2, "Hardware Implementation of a TPM in a PC
> Client Platform", there is a comment about Locality 4 registers being
> accessible via an implementation specific mechanism other than MMIO.
> Additionally, there were some discussions about clarifying the PTP on
> how the software interfaces might be expected to work for a
> general-purpose memory backed implementation.

So what is the error in the current want, i.e. what do you want to
change? I think this type of stuff can be extended but I don't want
to pollute this with too much detail at this point.

Only what matters for daily kernel development as reminder is what
this type of doc's should contain.

BR, Jarkko

2024-03-24 02:14:58

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v4] Documentation: tpm_tis

On Sat Mar 23, 2024 at 8:40 PM EET, Jarkko Sakkinen wrote:
> > Would it be worth clarifying here that one of those interfaces is
> > defined in the Mobile TPM specification, which also refers to its
> > interface as the CRB interface. In the past, this has caused great
> > confusion when working with individuals from the embedded community,
> > e.g., Arm. The Mobile TPM CRB interface, which can also be found being
> > used by some generations of AMD fTPM, is a doorbell style interface
> > using general-purpose memory. I would also point out that the Mobile TPM
> > CRB interface does not provide for the concept of localities.
>
> I don't necessarily disagree but it is out of scope for this. I'm not
> sure tho why "mobile" CRB would ever need that sort of separate
> dicussion.
>
> Some CRB implementations have localities some don't, and also fTPM
> implementations on x86 vary, no need to state that separately for
> mobile.

I.e. the variance exist but it is not "mobile" specific.

E.g. when I developed tpm_crb in 2014 at that time Intel PTT only
had a single locality (AFAIK later multiple localities were added
to support TXT).

In all cases this tpm_crb discussion is not really part of tpm_tis
discussion.

BR, Jarkko