Skip to contentSkip to conversation

ftpm: a firmware based TPM driver

PATCH Superseded Sasha Levin 4 participants 12 messages Apr 9, 2019 – Apr 10, 2019

Subsystems (4)

v2 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

v2 cover ftpm: a firmware based TPM driver

Changes since v1:

- Code nits (remove flags, debug statements, close up leaks, etc)
- Add docs under Documentation/

Sasha Levin (3):
ftpm: dt-binding: add dts documentation for fTPM driver
ftpm: firmware TPM running in TEE
ftpm: add documentation for ftpm driver

 .../bindings/security/tpm/tpm_ftpm_tee.txt    |  13 +
 .../devicetree/bindings/vendor-prefixes.txt   |   1 +
 Documentation/security/tpm/index.rst          |   1 +
 Documentation/security/tpm/tpm_ftpm_tee.rst   |  31 ++
 drivers/char/tpm/Kconfig                      |   5 +
 drivers/char/tpm/Makefile                     |   1 +
 drivers/char/tpm/tpm_ftpm_tee.c               | 346 ++++++++++++++++++
 drivers/char/tpm/tpm_ftpm_tee.h               |  52 +++
 8 files changed, 450 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt
 create mode 100644 Documentation/security/tpm/tpm_ftpm_tee.rst
 create mode 100644 drivers/char/tpm/tpm_ftpm_tee.c
 create mode 100644 drivers/char/tpm/tpm_ftpm_tee.h
Signature · 1 line
2.19.1
#1 lore raw

PATCH v2 1/3 ftpm: dt-binding: add dts documentation for fTPM driver

The parameters are similar to the ones used by IBM's vTPM and the various I2C tpm drivers.

---
 .../bindings/security/tpm/tpm_ftpm_tee.txt          | 13 +++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt         |  1 +
 2 files changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt
Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt new file +13 -0
new file mode 100644
index 000000000000..20fca67a56c4
@@ -0,0 +1,13 @@
+Required properties:
+- compatible: should be "microsoft,ftpm"
+- linux,sml-base: 64-bit base address of the reserved memory allocated
+		  for the firmware event log
+- linux,sml-size: size of the memory allocated for the firmware event log
+
+Example:
+
+tpm@0 {
+	compatible = "microsoft,ftpm";
+	linux,sml-base = <0x0 0xC0000000>;
+	linux,sml-size = <0x10000>;
+};
Documentation/devicetree/bindings/vendor-prefixes.txt +1 -0
index 8162b0eb4b50..902798bcb9a5 100644
@@ -249,6 +249,7 @@ micrel	Micrel Inc.
 microchip	Microchip Technology Inc.
 microcrystal	Micro Crystal AG
 micron	Micron Technology Inc.
+microsoft	Microsoft Corporation
 mikroe		MikroElektronika d.o.o.
 minix	MINIX Technology Ltd.
 miramems	MiraMEMS Sensing Technology Co., Ltd.
Signature · 1 line
2.19.1
#2 lore raw

Re: [PATCH v2 1/3] ftpm: dt-binding: add dts documentation for fTPM driver

Quoted 3 lines · Sasha Levin

The parameters are similar to the ones used by IBM's vTPM and the various I2C tpm drivers.

Bindings describe h/w (or firmware interfaces in this case), not drivers.

Quoted 14 lines
---
 .../bindings/security/tpm/tpm_ftpm_tee.txt          | 13 +++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt         |  1 +
 2 files changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt
Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt new file +0 -0
new file mode 100644
index 000000000000..20fca67a56c4

Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt line 5

Quoted 5 lines
Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt new file +5 -0
@@ -0,0 +1,13 @@
+Required properties:
+- compatible: should be "microsoft,ftpm"
+- linux,sml-base: 64-bit base address of the reserved memory allocated
+                 for the firmware event log
+- linux,sml-size: size of the memory allocated for the firmware event log

Firmware is defining linux specific properties? What if I want to run BSD? We should use 'reg' here instead.

What memory is used here? This should be under /reserved-memory if it is part of "main" memory.

Really, I'd prefer to not see this in DT at all. Make the firmware discoverable. Why repeat the mistakes of non-discoverable h/w in s/w interfaces? OP-Tee at least has defined a mechanism to enumerate TEE functions IIRC.

Rob

#3 lore raw

Re: [PATCH v2 1/3] ftpm: dt-binding: add dts documentation for fTPM driver

Quoted 15 lines · Rob Herring
Quoted 3 lines · Sasha Levin

The parameters are similar to the ones used by IBM's vTPM and the various I2C tpm drivers.

Bindings describe h/w (or firmware interfaces in this case), not drivers.

Quoted 8 lines
---
 .../bindings/security/tpm/tpm_ftpm_tee.txt          | 13 +++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt         |  1 +
 2 files changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt
Quoted 14 lines

Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt line 5

Quoted 11 lines
Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt new file +5 -0
new file mode 100644
index 000000000000..20fca67a56c4
@@ -0,0 +1,13 @@
+Required properties:
+- compatible: should be "microsoft,ftpm"
+- linux,sml-base: 64-bit base address of the reserved memory allocated
+                 for the firmware event log
+- linux,sml-size: size of the memory allocated for the firmware event log

Firmware is defining linux specific properties? What if I want to run BSD? We should use 'reg' here instead.

This is based on already existing code that defines these names, see tpm_read_log_of() in drivers/char/tpm/eventlog/of.c .

These properties were described similarily by other interfaces (see Documentation/devicetree/bindings/security/tpm/ibmvtpm.txt or Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt for example).

We could rename them all if you'd like, I was just trying to follow the existing code.

Quoted 2 lines

What memory is used here? This should be under /reserved-memory if it is part of "main" memory.

That's my understanding, yes.

Quoted 4 lines

Really, I'd prefer to not see this in DT at all. Make the firmware discoverable. Why repeat the mistakes of non-discoverable h/w in s/w interfaces? OP-Tee at least has defined a mechanism to enumerate TEE functions IIRC.

Sadly the firmware already exists as-is on live hardware, there is a paper describing it back from 2016 and we're stuck having to support that.

--
Thanks,
Sasha

#4 lore raw

Re: [PATCH v2 1/3] ftpm: dt-binding: add dts documentation for fTPM driver

Quoted 32 lines · Sasha Levin
Quoted 15 lines · Rob Herring
Quoted 3 lines · Sasha Levin

The parameters are similar to the ones used by IBM's vTPM and the various I2C tpm drivers.

Bindings describe h/w (or firmware interfaces in this case), not drivers.

Quoted 8 lines
---
 .../bindings/security/tpm/tpm_ftpm_tee.txt          | 13 +++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt         |  1 +
 2 files changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt
Quoted 14 lines

Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt line 5

Quoted 11 lines
Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt new file +5 -0
new file mode 100644
index 000000000000..20fca67a56c4
@@ -0,0 +1,13 @@
+Required properties:
+- compatible: should be "microsoft,ftpm"
+- linux,sml-base: 64-bit base address of the reserved memory allocated
+                 for the firmware event log
+- linux,sml-size: size of the memory allocated for the firmware event log

Firmware is defining linux specific properties? What if I want to run BSD? We should use 'reg' here instead.

Quoted 2 lines

This is based on already existing code that defines these names, see tpm_read_log_of() in drivers/char/tpm/eventlog/of.c .

BTW, that probably needs updating to handle endianness correctly.

Quoted 12 lines

These properties were described similarily by other interfaces (see Documentation/devicetree/bindings/security/tpm/ibmvtpm.txt or Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt for example).

We could rename them all if you'd like, I was just trying to follow the existing code.

Quoted 2 lines

What memory is used here? This should be under /reserved-memory if it is part of "main" memory.

That's my understanding, yes.

Quoted 8 lines
Quoted 4 lines

Really, I'd prefer to not see this in DT at all. Make the firmware discoverable. Why repeat the mistakes of non-discoverable h/w in s/w interfaces? OP-Tee at least has defined a mechanism to enumerate TEE functions IIRC.

Sadly the firmware already exists as-is on live hardware, there is a paper describing it back from 2016 and we're stuck having to support that.

Does the firmware depend on this binding or the DT just gets statically populated with the address/size the firmware hardcodes? I'd prefer not to propagate the IBM binding that we were kind of stuck with. I had similar comments on it...

Are there other parts of the TEE firmware that need to be described in DT?

Rob

#5 lore raw

Re: [PATCH v2 1/3] ftpm: dt-binding: add dts documentation for fTPM driver

Quoted 31 lines · Rob Herring
Quoted 30 lines · Sasha Levin
Quoted 27 lines · Rob Herring
Quoted 3 lines · Sasha Levin

The parameters are similar to the ones used by IBM's vTPM and the various I2C tpm drivers.

Bindings describe h/w (or firmware interfaces in this case), not drivers.

Quoted 12 lines
 .../bindings/security/tpm/tpm_ftpm_tee.txt          | 13 +++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt         |  1 +
 2 files changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt
Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt new file +1 -0
new file mode 100644
index 000000000000..20fca67a56c4
+++ b/Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt

Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt line 5

Quoted 5 lines
Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt new file +5 -0
@@ -0,0 +1,13 @@
+Required properties:
+- compatible: should be "microsoft,ftpm"
+- linux,sml-base: 64-bit base address of the reserved memory allocated
+                 for the firmware event log
+- linux,sml-size: size of the memory allocated for the firmware event log

Firmware is defining linux specific properties? What if I want to run BSD? We should use 'reg' here instead.

Quoted 4 lines
Quoted 2 lines

This is based on already existing code that defines these names, see tpm_read_log_of() in drivers/char/tpm/eventlog/of.c .

BTW, that probably needs updating to handle endianness correctly.

IIRC this legacy IBM code has broken endianness in the firmware..

All that stuff in read_log_of, and the related DT stuff, is historical IBM special case-ness and should not be copied into new things.

Jason

#6 lore raw

Re: [PATCH v2 1/3] ftpm: dt-binding: add dts documentation for fTPM driver

Quoted 35 lines · Jason Gunthorpe
Quoted 34 lines · Rob Herring
Quoted 32 lines · Sasha Levin
Quoted 14 lines · Rob Herring
Quoted 3 lines · Sasha Levin

The parameters are similar to the ones used by IBM's vTPM and the various I2C tpm drivers.

Bindings describe h/w (or firmware interfaces in this case), not drivers.

Quoted 7 lines
 .../bindings/security/tpm/tpm_ftpm_tee.txt          | 13 +++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt         |  1 +
 2 files changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt
Quoted 13 lines

Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt line 5

Quoted 10 lines
Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt new file +6 -0
new file mode 100644
index 000000000000..20fca67a56c4
+++ b/Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt
@@ -0,0 +1,13 @@
+Required properties:
+- compatible: should be "microsoft,ftpm"
+- linux,sml-base: 64-bit base address of the reserved memory allocated
+                 for the firmware event log
+- linux,sml-size: size of the memory allocated for the firmware event log

Firmware is defining linux specific properties? What if I want to run BSD? We should use 'reg' here instead.

This is based on already existing code that defines these names, see tpm_read_log_of() in drivers/char/tpm/eventlog/of.c .

Quoted 6 lines
Quoted 1 line

BTW, that probably needs updating to handle endianness correctly.

IIRC this legacy IBM code has broken endianness in the firmware..

All that stuff in read_log_of, and the related DT stuff, is historical IBM special case-ness and should not be copied into new things.

The fTPM driver does not use it on it's own, so I guess I can just drop this patch then?

--
Thanks,
Sasha

#7 lore raw

Re: [PATCH v2 1/3] ftpm: dt-binding: add dts documentation for fTPM driver

Quoted 38 lines · Sasha Levin
Quoted 37 lines · Jason Gunthorpe
Quoted 35 lines · Rob Herring
Quoted 30 lines · Sasha Levin
Quoted 14 lines · Rob Herring
Quoted 3 lines · Sasha Levin

The parameters are similar to the ones used by IBM's vTPM and the various I2C tpm drivers.

Bindings describe h/w (or firmware interfaces in this case), not drivers.

Quoted 7 lines
 .../bindings/security/tpm/tpm_ftpm_tee.txt          | 13 +++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt         |  1 +
 2 files changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt
Quoted 13 lines

Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt line 5

Quoted 10 lines
Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt new file +6 -0
new file mode 100644
index 000000000000..20fca67a56c4
+++ b/Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt
@@ -0,0 +1,13 @@
+Required properties:
+- compatible: should be "microsoft,ftpm"
+- linux,sml-base: 64-bit base address of the reserved memory allocated
+                 for the firmware event log
+- linux,sml-size: size of the memory allocated for the firmware event log

Firmware is defining linux specific properties? What if I want to run BSD? We should use 'reg' here instead.

Quoted 2 lines

This is based on already existing code that defines these names, see tpm_read_log_of() in drivers/char/tpm/eventlog/of.c .

BTW, that probably needs updating to handle endianness correctly.

Quoted 7 lines
Quoted 4 lines

IIRC this legacy IBM code has broken endianness in the firmware..

All that stuff in read_log_of, and the related DT stuff, is historical IBM special case-ness and should not be copied into new things.

The fTPM driver does not use it on it's own, so I guess I can just drop this patch then?

If your ARM system boots via EFI then it should pass the firmware event log through EFI mechanisms, IIRC.

Otherwise maybe you could make a case for using this, but only if the old IBM stuff is perfectly emulated, bugs and all.

Do you even have an event log?

Jason

#8 lore raw

RE: [PATCH v2 1/3] ftpm: dt-binding: add dts documentation for fTPM driver

Quoted 77 lines

-----Original Message-----
From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, April 10, 2019 10:57 AM
To: Sasha Levin <[email protected]>
Cc: Rob Herring <[email protected]>; Peter Huewe <[email protected]>; Jarkko Sakkinen <[email protected]>; Mark Rutland <[email protected]>; Jonathan Corbet <[email protected]>; Arnd Bergmann <[email protected]>; Greg Kroah-Hartman <[email protected]>; linux- [email protected]; Linux Doc Mailing List <[email protected]>; [email protected]; Microsoft Linux Kernel List <linux- [email protected]>; Thirupathaiah Annapureddy <[email protected]>; Bryan Kelly (CSI) <[email protected]>
Subject: Re: [PATCH v2 1/3] ftpm: dt-binding: add dts documentation for fTPM driver

Quoted 3 lines · Sasha Levin
Quoted 2 lines · Jason Gunthorpe
Quoted 1 line · Rob Herring

On Wed, Apr 10, 2019 at 11:19 AM Sasha Levin <[email protected]>

Quoted 3 lines · wrote:
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines · Rob Herring
Quoted 1 line

On Tue, Apr 9, 2019 at 1:50 PM Sasha Levin <[email protected]>

Quoted 2 lines · wrote:
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

The parameters are similar to the ones used by IBM's vTPM and

the

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line

various I2C tpm drivers.

Bindings describe h/w (or firmware interfaces in this case), not

drivers.

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
 .../bindings/security/tpm/tpm_ftpm_tee.txt          | 13

+++++++++++++

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
 .../devicetree/bindings/vendor-prefixes.txt         |  1 +
 2 files changed, 14 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

diff --git

a/Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt b/Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

new file mode 100644
index 000000000000..20fca67a56c4
+++

b/Documentation/devicetree/bindings/security/tpm/tpm_ftpm_tee.txt

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

Review line 3

Quoted 4 lines
(quoted diff) +3 -0
@@ -0,0 +1,13 @@
+Required properties:
+- compatible: should be "microsoft,ftpm"
+- linux,sml-base: 64-bit base address of the reserved memory

allocated

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

Review line 5

Quoted 2 lines
(quoted diff) +2 -0
+                 for the firmware event log
+- linux,sml-size: size of the memory allocated for the firmware

event log

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

Firmware is defining linux specific properties? What if I want to

run

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line

BSD? We should use 'reg' here instead.

This is based on already existing code that defines these names,

see

Quoted 11 lines
Quoted 8 lines
Quoted 3 lines
Quoted 1 line

tpm_read_log_of() in drivers/char/tpm/eventlog/of.c .

BTW, that probably needs updating to handle endianness correctly.

IIRC this legacy IBM code has broken endianness in the firmware..

All that stuff in read_log_of, and the related DT stuff, is historical IBM special case-ness and should not be copied into new things.

The fTPM driver does not use it on it's own, so I guess I can just drop this patch then?

Quoted 8 lines

If your ARM system boots via EFI then it should pass the firmware event log through EFI mechanisms, IIRC.

Otherwise maybe you could make a case for using this, but only if the old IBM stuff is perfectly emulated, bugs and all.

Do you even have an event log?

Our ARM system is booting using U-boot and Device Tree.
So the ACPI/EFI table mechanism to pass binary boot measurements won't be applicable.

We have event logs working and we are able to successfully read them using /sys/kernel/security/tpm0/binary_bios_measurements and parse them

--Thiru

Quoted 1 line

Jason

#9 lore raw

PATCH v2 2/3 ftpm: firmware TPM running in TEE

In reply to Sasha Levin

This patch adds support for a software-only implementation of a TPM running in TEE.

There is extensive documentation of the design here:
https://www.microsoft.com/en-us/research/publication/ftpm-software-implementation-tpm-chip/ .

As well as reference code for the firmware available here:
https://github.com/Microsoft/ms-tpm-20-ref/tree/master/Samples/ARM32-FirmwareTPM

---
 drivers/char/tpm/Kconfig        |   5 +
 drivers/char/tpm/Makefile       |   1 +
 drivers/char/tpm/tpm_ftpm_tee.c | 346 ++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm_ftpm_tee.h |  52 +++++
 4 files changed, 404 insertions(+)
 create mode 100644 drivers/char/tpm/tpm_ftpm_tee.c
 create mode 100644 drivers/char/tpm/tpm_ftpm_tee.h
drivers/char/tpm/Kconfig +5 -0
index 536e55d3919f..5638726641eb 100644
@@ -164,6 +164,11 @@ config TCG_VTPM_PROXY
 	  /dev/vtpmX and a server-side file descriptor on which the vTPM
 	  can receive commands.
 
+config TCG_FTPM_TEE
+	tristate "TEE based fTPM Interface"
+	depends on TEE
+	---help---
+	  This driver proxies for fTPM running in TEE
 
 source "drivers/char/tpm/st33zp24/Kconfig"
 endif # TCG_TPM
drivers/char/tpm/Makefile +1 -0
index a01c4cab902a..c354cdff9c62 100644
@@ -33,3 +33,4 @@ obj-$(CONFIG_TCG_TIS_ST33ZP24) += st33zp24/
 obj-$(CONFIG_TCG_XEN) += xen-tpmfront.o
 obj-$(CONFIG_TCG_CRB) += tpm_crb.o
 obj-$(CONFIG_TCG_VTPM_PROXY) += tpm_vtpm_proxy.o
+obj-$(CONFIG_TCG_FTPM_TEE) += tpm_ftpm_tee.o
drivers/char/tpm/tpm_ftpm_tee.c new file +346 -0

346 lines not on this page · raw message

drivers/char/tpm/tpm_ftpm_tee.h new file +52 -0

52 lines not on this page · raw message

Signature · 1 line
2.19.1
#10 lore raw

Re: [PATCH v2 2/3] ftpm: firmware TPM running in TEE

Quoted 12 lines · Sasha Levin
drivers/char/tpm/tpm_ftpm_tee.c new file +12 -0
+/*
+ * Undo what has been done in ftpm_tee_probe
+ */
+static void ftpm_tee_deinit(struct ftpm_tee_private *pvt_data)
+{
+	/* Release the chip */
+	tpm_chip_unregister(pvt_data->chip);
+
+	/* frees chip */
+	if (pvt_data->chip)
+		put_device(&pvt_data->chip->dev);
+

drivers/char/tpm/tpm_ftpm_tee.c line 221

Quoted 10 lines
drivers/char/tpm/tpm_ftpm_tee.c new file +10 -0
+	if (pvt_data->ctx) {
+		/* Free the shared memory pool */
+		tee_shm_free(pvt_data->shm);
+
+		/* close the existing session with fTPM TA*/
+		tee_client_close_session(pvt_data->ctx, pvt_data->session);
+
+		/* close the context with TEE driver */
+		tee_client_close_context(pvt_data->ctx);
+	}

None of these if's are necessary, remove is only called if probe succeeds. Would also make more sense to put this code into remove instead of having it call one function..

Quoted 45 lines
drivers/char/tpm/tpm_ftpm_tee.h new file +41 -0
new file mode 100644
index 000000000000..c1dd416d27c9
+++ b/drivers/char/tpm/tpm_ftpm_tee.h
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) Microsoft Corporation
+ */
+
+#ifndef __TPM_FTPM_TEE_H__
+#define __TPM_FTPM_TEE_H__
+
+#include <linux/tee_drv.h>
+#include <linux/uuid.h>
+#include <linux/tpm.h>
+
+/* The TAFs ID implemented in this TA */
+#define FTPM_OPTEE_TA_SUBMIT_COMMAND  (0)
+#define FTPM_OPTEE_TA_EMULATE_PPI     (1)
+
+/* max. buffer size supported by fTPM  */
+#define  MAX_COMMAND_SIZE       4096
+#define  MAX_RESPONSE_SIZE      4096
+
+/**
+ * struct ftpm_tee_private - fTPM's private data
+ * @chip:     struct tpm_chip instance registered with tpm framework.
+ * @state:    internal state
+ * @session:  fTPM TA session identifier.
+ * @resp_len: cached response buffer length.
+ * @resp_buf: cached response buffer.
+ * @ctx:      TEE context handler.
+ * @shm:      Memory pool shared with fTPM TA in TEE.
+ */
+struct ftpm_tee_private {
+	struct tpm_chip *chip;
+	u32 session;
+	size_t resp_len;
+	u8 resp_buf[MAX_RESPONSE_SIZE];
+	struct tee_context *ctx;
+	struct tee_shm *shm;
+};
+
+/*

drivers/char/tpm/tpm_ftpm_tee.h line 50

Quoted 10 lines
drivers/char/tpm/tpm_ftpm_tee.h new file +10 -0
+ * Note: ftpm_tee_tpm_op_recv and ftpm_tee_tpm_op_send are called from the
+ * same routine tpm_try_transmit in tpm-interface.c. These calls are protected
+ * by chip->tpm_mutex => There is no need for protecting any data shared
+ * between these routines ex: struct ftpm_tee_private
+ */
+
+/* TA_FTPM_UUID: BC50D971-D4C9-42C4-82CB-343FB7F37896 */
+static const uuid_t ftpm_ta_uuid =
+	UUID_INIT(0xBC50D971, 0xD4C9, 0x42C4,
+		  0x82, 0xCB, 0x34, 0x3F, 0xB7, 0xF3, 0x78, 0x96);

Don't put static variables in header files

Jason

#11 lore raw

PATCH v2 3/3 ftpm: add documentation for ftpm driver

In reply to Sasha Levin

This patch adds basic documentation to describe the new fTPM driver.

---
 Documentation/security/tpm/index.rst        |  1 +
 Documentation/security/tpm/tpm_ftpm_tee.rst | 31 +++++++++++++++++++++
 2 files changed, 32 insertions(+)
 create mode 100644 Documentation/security/tpm/tpm_ftpm_tee.rst
Documentation/security/tpm/index.rst +1 -0
index af77a7bbb070..15783668644f 100644
@@ -4,4 +4,5 @@ Trusted Platform Module documentation
 
 .. toctree::
 
+   tpm_ftpm_tee
    tpm_vtpm_proxy
Documentation/security/tpm/tpm_ftpm_tee.rst new file +31 -0
new file mode 100644
index 000000000000..3f19b12d9c57
@@ -0,0 +1,31 @@
+=============================================
+Firmware TPM Driver
+=============================================
+
+| Authors:
+| Thirupathaiah Annapureddy <[email protected]>
+| Sasha Levin <[email protected]>
+
+This document describes the firmware Trusted Platform Module (fTPM)
+device driver.
+
+Introduction
+============
+
+This driver is a shim for a firmware implemented in ARM's TrustZone
+environment. The driver allows programs to interact with the TPM in the same
+way the would interact with a hardware TPM.
+
+Design
+======
+
+The driver acts as a thin layer that passes commands to and from a TPM
+implemented in firmware. The driver itself doesn't contain much logic and is
+used more like a dumb pipe between firmware and kernel/userspace.
+
+The firmware itself is based on the following paper:
+https://www.microsoft.com/en-us/research/wp-content/uploads/2017/06/ftpm1.pdf
+
+When the driver is loaded it will expose ``/dev/tpmX`` character devices to
+userspace which will enable userspace to communicate with the firmware tpm
+through this device.
Signature · 1 line
2.19.1