[U-Boot] [PATCH v2 00/19] Introduce SPI TPM v2.0 support

[U-Boot] [PATCH v2 00/19] Introduce SPI TPM v2.0 support

[Miquel Raynal]

Current U-Boot supports TPM v1.2 specification. The new specification
(v2.0) is not backward compatible and renames/introduces several
functions.

This series introduces a new SPI driver following the TPM v2.0
specification. It has been tested on a ST TPM but should be usable with
others v2.0 compliant chips.

Then, basic functionalities are introduced one by one for the v2.0
specification. The INIT command now can receive a parameter to
distinguish further TPMv1/TPMv2 commands. After that, the library itself
will know which one is pertinent and will return a special error if the
desired command is not supported for the selected specification.

Available commands for v2.0 TPMs are:
* STARTUP
* SELF TEST
* CLEAR
* PCR EXTEND
* PCR READ
* GET CAPABILITY
* DICTIONARY ATTACK LOCK RESET
* DICTIONARY ATTACK CHANGE PARAMETERS
* HIERARCHY CHANGE AUTH

Two commands have been written but could not be tested (unsupported by
the TPM chosen):
* PCR CHANGE AUTH POLICY
* PCR CHANGE AUTH VALUE

With this set of function, minimal TPMv2.0 handling is possible with the
following sequence.

* First, initialize the TPM stack in U-Boot: "TPM2" is a new parameter
  to discern the format of the commands:

> tpm init TPM2

* Then send the STARTUP command to the TPM. The flag is slightly
  different between the revisions.

> tpm startup TPM2_SU_CLEAR

* To enable full TPM capabilities, continue the tests (or do them all
  again). It seems like self_test_full always waits for the operation to
  finish, while continue_self_test returns a busy state if called to
  early.

> tpm continue_self_test
> tpm self_test_full

* Manage passwords (force_clear also resets a lot of internal stuff).
  Olderly, TAKE OWNERSHIP == CLEAR + CHANGE AUTH. LOCKOUT is an example,
  ENDORSEMENT and PLATFORM hierarchies are available too:

> tpm force_clear TPM2_RH_LOCKOUT [<pw>]
> tpm change_auth TPM2_RH_LOCKOUT <new_pw> [<old_pw>]

* Dictionary Attack Mitigation (DAM) parameters can be changed. It is
  possible to reset the failure counter and disable the lockout (values
  erased after a CLEAR). It is then possible to check the parameters
  have been correctly applied.

> tpm dam_reset_counter [<pw>]
> tpm dam_set_parameters 0xffff 1 0 [<pw>]
> tpm get_capability 0x0006 0x020e 0x4000000 4

* PCR policy may be changed (untested).
  PCR can be extended (no protection against packet replay yet).
  PCR can be read (the counter with the number of "extensions" is also
  given).

> tpm pcr_setauthpolicy 0 12345678901234567890123456789012 [<pw>]
> tpm pcr_read 0 0x4000000
> tpm pcr_extend 0 0x4000000

Regular testing may be done through the test/py/ framework when using
real hardware, there is no sandbox support for now.

Thanks,
Miquèl


Miquel Raynal (19):
  tpm: add Revision ID field in the chip structure
  tpm: rename tpm_tis_infineon in tpm_tis_infineon_i2c
  tpm: add support for TPMv2 SPI modules
  tpm: fix indentation in command list before adding more
  tpm: prepare support for TPMv2 commands
  tpm: add macros for TPMv2 commands
  tpm: add possible traces to analyze buffers returned by the TPM
  tpm: handle different buffer sizes
  tpm: add TPM2_Startup command support
  tpm: add TPM2_SelfTest command support
  tpm: add TPM2_Clear command support
  tpm: rename the _extend() function to be _pcr_event()
  tpm: add TPM2_PCR_Extend command support
  tpm: add TPM2_PCR_Read command support
  tpm: add TPM2_GetCapability command support
  tpm: add dictionary attack mitigation commands support
  tpm: add TPM2_HierarchyChangeAuth command support
  tpm: add PCR authentication commands support
  test/py: add TPMv2.0 test suite

 cmd/tpm.c                                          | 360 +++++++++--
 cmd/tpm_test.c                                     |  10 +-
 drivers/tpm/Kconfig                                |  13 +-
 drivers/tpm/Makefile                               |   3 +-
 drivers/tpm/tpm_tis.h                              |   4 +
 .../{tpm_tis_infineon.c => tpm_tis_infineon_i2c.c} |   2 +-
 drivers/tpm/tpm_tis_spi.c                          | 656 +++++++++++++++++++++
 include/tpm.h                                      | 183 +++++-
 lib/tpm.c                                          | 654 ++++++++++++++++++--
 test/py/tests/test_tpm2.py                         | 254 ++++++++
 10 files changed, 1993 insertions(+), 146 deletions(-)
 rename drivers/tpm/{tpm_tis_infineon.c => tpm_tis_infineon_i2c.c} (99%)
 create mode 100644 drivers/tpm/tpm_tis_spi.c
 create mode 100644 test/py/tests/test_tpm2.py

-- 
2.14.1

[U-Boot] [PATCH v2 01/19] tpm: add Revision ID field in the chip structure

[Miquel Raynal]

TPM are shipped with a few read-only register from which we can retrieve
for instance:
- vendor ID
- product ID
- revision ID

Product and vendor ID share the same register and are already referenced
in the tpm_chip structure. Add the revision ID entry which is missing.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/tpm/tpm_tis.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tpm/tpm_tis.h b/drivers/tpm/tpm_tis.h
index 25b152b321..2b81f3be50 100644
--- a/drivers/tpm/tpm_tis.h
+++ b/drivers/tpm/tpm_tis.h
@@ -41,6 +41,7 @@ struct tpm_chip {
 	int is_open;
 	int locality;
 	u32 vend_dev;
+	u8 rid;
 	unsigned long timeout_a, timeout_b, timeout_c, timeout_d;  /* msec */
 	ulong chip_type;
 };
-- 
2.14.1

[U-Boot] [PATCH v2 02/19] tpm: rename tpm_tis_infineon in tpm_tis_infineon_i2c

[Miquel Raynal]

As the chips driven by tpm_tis_infineon.c are only I2C chips, rename the
driver with the _i2c suffix to prepare the venue of its _spi cousin.

Also change the driver name in the U_BOOT_DRIVER structure.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/tpm/Kconfig                                        | 4 ++--
 drivers/tpm/Makefile                                       | 2 +-
 drivers/tpm/{tpm_tis_infineon.c => tpm_tis_infineon_i2c.c} | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)
 rename drivers/tpm/{tpm_tis_infineon.c => tpm_tis_infineon_i2c.c} (99%)

diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig
index 2a64bc49c3..a98570ee77 100644
--- a/drivers/tpm/Kconfig
+++ b/drivers/tpm/Kconfig
@@ -22,7 +22,7 @@ config TPM_ATMEL_TWI
 	  to the device using the standard TPM Interface Specification (TIS)
 	  protocol
 
-config TPM_TIS_INFINEON
+config TPM_TIS_INFINEON_I2C
 	bool "Enable support for Infineon SLB9635/45 TPMs on I2C"
 	depends on TPM && DM_I2C
 	help
@@ -33,7 +33,7 @@ config TPM_TIS_INFINEON
 
 config TPM_TIS_I2C_BURST_LIMITATION
 	bool "Enable I2C burst length limitation"
-	depends on TPM_TIS_INFINEON
+	depends on TPM_TIS_INFINEON_I2C
 	help
 	  Some broken TPMs have a limitation on the number of bytes they can
 	  receive in one message. Enable this option to allow you to set this
diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
index c42a93f267..5a19a58f43 100644
--- a/drivers/tpm/Makefile
+++ b/drivers/tpm/Makefile
@@ -6,7 +6,7 @@
 obj-$(CONFIG_TPM) += tpm-uclass.o
 
 obj-$(CONFIG_TPM_ATMEL_TWI) += tpm_atmel_twi.o
-obj-$(CONFIG_TPM_TIS_INFINEON) += tpm_tis_infineon.o
+obj-$(CONFIG_TPM_TIS_INFINEON_I2C) += tpm_tis_infineon_i2c.o
 obj-$(CONFIG_TPM_TIS_LPC) += tpm_tis_lpc.o
 obj-$(CONFIG_TPM_TIS_SANDBOX) += tpm_tis_sandbox.o
 obj-$(CONFIG_TPM_ST33ZP24_I2C) += tpm_tis_st33zp24_i2c.o
diff --git a/drivers/tpm/tpm_tis_infineon.c b/drivers/tpm/tpm_tis_infineon_i2c.c
similarity index 99%
rename from drivers/tpm/tpm_tis_infineon.c
rename to drivers/tpm/tpm_tis_infineon_i2c.c
index 41b748e7a2..c29c2d1106 100644
--- a/drivers/tpm/tpm_tis_infineon.c
+++ b/drivers/tpm/tpm_tis_infineon_i2c.c
@@ -629,7 +629,7 @@ static const struct udevice_id tpm_tis_i2c_ids[] = {
 };
 
 U_BOOT_DRIVER(tpm_tis_i2c) = {
-	.name   = "tpm_tis_infineon",
+	.name   = "tpm_tis_infineon_i2c",
 	.id     = UCLASS_TPM,
 	.of_match = tpm_tis_i2c_ids,
 	.ops    = &tpm_tis_i2c_ops,
-- 
2.14.1

[U-Boot] [PATCH v2 03/19] tpm: add support for TPMv2 SPI modules

[Miquel Raynal]

Add the tpm_tis_spi driver that should support any TPMv2 compliant (SPI)
module.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/tpm/Kconfig       |   9 +
 drivers/tpm/Makefile      |   1 +
 drivers/tpm/tpm_tis.h     |   3 +
 drivers/tpm/tpm_tis_spi.c | 656 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 669 insertions(+)
 create mode 100644 drivers/tpm/tpm_tis_spi.c

diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig
index a98570ee77..cc57008b6a 100644
--- a/drivers/tpm/Kconfig
+++ b/drivers/tpm/Kconfig
@@ -46,6 +46,15 @@ config TPM_TIS_I2C_BURST_LIMITATION_LEN
 	help
 	  Use this to set the burst limitation length
 
+config TPM_TIS_SPI
+	bool "Enable support for TPMv2 SPI chips"
+	depends on TPM && DM_SPI
+	help
+	  This driver supports TPMv2 devices connected on the SPI bus.
+	  The usual TPM operations and the 'tpm' command can be used to talk
+	  to the device using the standard TPM Interface Specification (TIS)
+	  protocol
+
 config TPM_TIS_LPC
 	bool "Enable support for Infineon SLB9635/45 TPMs on LPC"
 	depends on TPM && X86
diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
index 5a19a58f43..a753b24230 100644
--- a/drivers/tpm/Makefile
+++ b/drivers/tpm/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_TPM) += tpm-uclass.o
 
 obj-$(CONFIG_TPM_ATMEL_TWI) += tpm_atmel_twi.o
 obj-$(CONFIG_TPM_TIS_INFINEON_I2C) += tpm_tis_infineon_i2c.o
+obj-$(CONFIG_TPM_TIS_SPI) += tpm_tis_spi.o
 obj-$(CONFIG_TPM_TIS_LPC) += tpm_tis_lpc.o
 obj-$(CONFIG_TPM_TIS_SANDBOX) += tpm_tis_sandbox.o
 obj-$(CONFIG_TPM_ST33ZP24_I2C) += tpm_tis_st33zp24_i2c.o
diff --git a/drivers/tpm/tpm_tis.h b/drivers/tpm/tpm_tis.h
index 2b81f3be50..1f4b0c24bd 100644
--- a/drivers/tpm/tpm_tis.h
+++ b/drivers/tpm/tpm_tis.h
@@ -37,6 +37,9 @@ enum tpm_timeout {
 #define TPM_RSP_SIZE_BYTE	2
 #define TPM_RSP_RC_BYTE		6
 
+/* Number of xfer retries */
+#define TPM_XFER_RETRY		10
+
 struct tpm_chip {
 	int is_open;
 	int locality;
diff --git a/drivers/tpm/tpm_tis_spi.c b/drivers/tpm/tpm_tis_spi.c
new file mode 100644
index 0000000000..17f6cfa85c
--- /dev/null
+++ b/drivers/tpm/tpm_tis_spi.c
@@ -0,0 +1,656 @@
+/*
+ * Author:
+ * Miquel Raynal <miquel.raynal@bootlin.com>
+ *
+ * Description:
+ * SPI-level driver for TCG/TIS TPM (trusted platform module).
+ * Specifications at www.trustedcomputinggroup.org
+ *
+ * This device driver implements the TPM interface as defined in
+ * the TCG SPI protocol stack version 2.0.
+ *
+ * It is based on the U-Boot driver tpm_tis_infineon_i2c.c.
+ *
+ * SPDX-License-Identifier:	GPL-2.0
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <fdtdec.h>
+#include <spi.h>
+#include <tpm.h>
+#include <linux/errno.h>
+#include <linux/compiler.h>
+#include <linux/types.h>
+#include <linux/unaligned/be_byteshift.h>
+
+#include "tpm_tis.h"
+#include "tpm_internal.h"
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define TPM_ACCESS(l)			(0x0000 | ((l) << 12))
+#define TPM_INT_ENABLE(l)               (0x0008 | ((l) << 12))
+#define TPM_STS(l)			(0x0018 | ((l) << 12))
+#define TPM_DATA_FIFO(l)		(0x0024 | ((l) << 12))
+#define TPM_DID_VID(l)			(0x0F00 | ((l) << 12))
+#define TPM_RID(l)			(0x0F04 | ((l) << 12))
+
+#define MAX_SPI_FRAMESIZE 64
+
+/*
+ * tpm_tis_spi_read() - read from TPM register
+ * @addr: register address to read from
+ * @buffer: provided by caller
+ * @len: number of bytes to read
+ *
+ * Read len bytes from TPM register and put them into
+ * buffer (little-endian format, i.e. first byte is put into buffer[0]).
+ *
+ * NOTE: TPM is big-endian for multi-byte values. Multi-byte
+ * values have to be swapped.
+ *
+ * Return -EIO on error, 0 on success.
+ */
+static int tpm_tis_spi_xfer(struct udevice *dev, u32 addr, const u8 *out,
+			    u8 *in, u16 len)
+{
+	struct spi_slave *slave = dev_get_parent_priv(dev);
+	int transfer_len, ret;
+	u8 tx_buf[MAX_SPI_FRAMESIZE];
+	u8 rx_buf[MAX_SPI_FRAMESIZE];
+
+	if (in && out) {
+		debug("%s: cannot do full duplex\n", __func__);
+		return -EINVAL;
+	}
+
+	ret = spi_claim_bus(slave);
+	if (ret < 0) {
+		debug("%s: could not claim bus\n", __func__);
+		return ret;
+	}
+
+	while (len) {
+		/* Request */
+		transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);
+		tx_buf[0] = (in ? BIT(7) : 0) | (transfer_len - 1);
+		tx_buf[1] = 0xD4;
+		tx_buf[2] = addr >> 8;
+		tx_buf[3] = addr;
+
+		ret = spi_xfer(slave, 4 * 8, tx_buf, rx_buf, SPI_XFER_BEGIN);
+		if (ret < 0) {
+			debug("%s: spi request transfer failed (err: %d)\n",
+			      __func__, ret);
+			goto release_bus;
+		}
+
+		/* Wait state */
+		if (!(rx_buf[3] & 0x1)) {
+			int i;
+
+			rx_buf[0] = 0;
+			for (i = 0; i < TPM_XFER_RETRY; i++) {
+				ret = spi_xfer(slave, 1 * 8, NULL, rx_buf, 0);
+				if (ret < 0) {
+					debug("%s: wait state failed: %d\n",
+					      __func__, ret);
+					goto release_bus;
+				}
+
+				if (rx_buf[0] & 0x1)
+					break;
+			}
+
+			if (i == TPM_RETRY) {
+				debug("%s: timeout on wait state\n", __func__);
+				ret = -ETIMEDOUT;
+				goto release_bus;
+			}
+		}
+
+		/* Read/Write */
+		if (out) {
+			memcpy(tx_buf, out, transfer_len);
+			out += transfer_len;
+		}
+
+		ret = spi_xfer(slave, transfer_len * 8,
+			       out ? tx_buf : NULL,
+			       in ? rx_buf : NULL,
+			       SPI_XFER_END);
+		if (ret < 0) {
+			debug("%s: spi read transfer failed (err: %d)\n",
+			      __func__, ret);
+			goto release_bus;
+		}
+
+		if (in) {
+			memcpy(in, rx_buf, transfer_len);
+			in += transfer_len;
+		}
+
+		len -= transfer_len;
+	}
+
+release_bus:
+	/* If an error occurred, release the chip by deasserting the CS */
+	if (ret < 0)
+		spi_xfer(slave, 0, NULL, NULL, SPI_XFER_END);
+
+	spi_release_bus(slave);
+
+	return ret;
+}
+
+static int tpm_tis_spi_read(struct udevice *dev, u16 addr, u8 *in, u16 len)
+{
+	return tpm_tis_spi_xfer(dev, addr, NULL, in, len);
+}
+
+static __maybe_unused int tpm_tis_spi_read16(struct udevice *dev, u32 addr,
+					     u16 *result)
+{
+	__le16 result_le;
+	int ret;
+
+	ret = tpm_tis_spi_read(dev, addr, (u8 *)&result_le, sizeof(u16));
+	if (!ret)
+		*result = le16_to_cpu(result_le);
+
+	return ret;
+}
+
+static __maybe_unused int tpm_tis_spi_read32(struct udevice *dev, u32 addr,
+					     u32 *result)
+{
+	__le32 result_le;
+	int ret;
+
+	ret = tpm_tis_spi_read(dev, addr, (u8 *)&result_le, sizeof(u32));
+	if (!ret)
+		*result = le32_to_cpu(result_le);
+
+	return ret;
+}
+
+static int tpm_tis_spi_write(struct udevice *dev, u16 addr, const u8 *out,
+			     u16 len)
+{
+	return tpm_tis_spi_xfer(dev, addr, out, NULL, len);
+}
+
+static __maybe_unused int tpm_tis_spi_write32(struct udevice *dev, u32 addr,
+					      u32 value)
+{
+	__le32 value_le = cpu_to_le32(value);
+
+	return tpm_tis_spi_write(dev, addr, (const u8 *)&value_le, sizeof(u32));
+}
+
+static int tpm_tis_spi_check_locality(struct udevice *dev, int loc)
+{
+	const u8 mask = TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID;
+	struct tpm_chip *chip = dev_get_priv(dev);
+	u8 buf;
+	int ret;
+
+	ret = tpm_tis_spi_read(dev, TPM_ACCESS(loc), &buf, 1);
+	if (ret)
+		return ret;
+
+	if ((buf & mask) == mask) {
+		chip->locality = loc;
+		return 0;
+	}
+
+	return -ENOENT;
+}
+
+static void tpm_tis_spi_release_locality(struct udevice *dev, int loc,
+					 bool force)
+{
+	const u8 mask = TPM_ACCESS_REQUEST_PENDING | TPM_ACCESS_VALID;
+	u8 buf;
+
+	if (tpm_tis_spi_read(dev, TPM_ACCESS(loc), &buf, 1) < 0)
+		return;
+
+	if (force || (buf & mask) == mask) {
+		buf = TPM_ACCESS_ACTIVE_LOCALITY;
+		tpm_tis_spi_write(dev, TPM_ACCESS(loc), &buf, 1);
+	}
+}
+
+static int tpm_tis_spi_request_locality(struct udevice *dev, int loc)
+{
+	struct tpm_chip *chip = dev_get_priv(dev);
+	unsigned long start, stop;
+	u8 buf = TPM_ACCESS_REQUEST_USE;
+	int ret;
+
+	ret = tpm_tis_spi_check_locality(dev, loc);
+	if (!ret)
+		return 0;
+
+	if (ret != -ENOENT) {
+		debug("%s: Failed to get locality: %d\n", __func__, ret);
+		return ret;
+	}
+
+	ret = tpm_tis_spi_write(dev, TPM_ACCESS(loc), &buf, 1);
+	if (ret) {
+		debug("%s: Failed to write to TPM: %d\n", __func__, ret);
+		return ret;
+	}
+
+	start = get_timer(0);
+	stop = chip->timeout_a;
+	do {
+		ret = tpm_tis_spi_check_locality(dev, loc);
+		if (!ret)
+			return 0;
+
+		if (ret != -ENOENT) {
+			debug("%s: Failed to get locality: %d\n", __func__,
+			      ret);
+			return ret;
+		}
+
+		mdelay(TPM_TIMEOUT_MS);
+	} while (get_timer(start) < stop);
+	debug("%s: Timeout getting locality: %d\n", __func__, ret);
+
+	return ret;
+}
+
+/*
+ * tpm_tis_spi_status return the TPM_STS register
+ * @dev: the device
+ * @status: storage parameter
+ * @return: 0 on success, a negative error otherwise
+ */
+static u8 tpm_tis_spi_status(struct udevice *dev, u8 *status)
+{
+	struct tpm_chip *chip = dev_get_priv(dev);
+
+	return tpm_tis_spi_read(dev, TPM_STS(chip->locality), status, 1);
+}
+
+static int tpm_tis_spi_wait_for_stat(struct udevice *dev, u8 mask,
+				     unsigned long timeout, u8 *status)
+{
+	unsigned long start = get_timer(0);
+	unsigned long stop = timeout;
+	int ret;
+
+	do {
+		mdelay(TPM_TIMEOUT_MS);
+		ret = tpm_tis_spi_status(dev, status);
+		if (ret)
+			return ret;
+
+		if ((*status & mask) == mask)
+			return 0;
+	} while (get_timer(start) < stop);
+
+	return -ETIMEDOUT;
+}
+
+/*
+ * tpm_tis_spi_get_burstcount return the burstcount address 0x19 0x1A
+ * @param: chip, the chip description
+ * return: the burstcount or -TPM_DRIVER_ERR in case of error.
+ */
+static int tpm_tis_spi_get_burstcount(struct udevice *dev)
+{
+	struct tpm_chip *chip = dev_get_priv(dev);
+	unsigned long start, stop;
+	u32 burstcount, ret;
+
+	/* wait for burstcount */
+	start = get_timer(0);
+	stop = chip->timeout_d;
+	do {
+		ret = tpm_tis_spi_read32(dev, TPM_STS(chip->locality),
+					 &burstcount);
+		if (ret)
+			return -EBUSY;
+
+		burstcount = (burstcount >> 8) & 0xFFFF;
+		if (burstcount)
+			return burstcount;
+
+		mdelay(TPM_TIMEOUT_MS);
+	} while (get_timer(start) < stop);
+
+	return -EBUSY;
+}
+
+/*
+ * tpm_tis_spi_cancel, cancel the current command execution or
+ * set STS to COMMAND READY.
+ * @param: chip, tpm_chip description.
+ */
+static int tpm_tis_spi_cancel(struct udevice *dev)
+{
+	struct tpm_chip *chip = dev_get_priv(dev);
+	u8 data = TPM_STS_COMMAND_READY;
+
+	return tpm_tis_spi_write(dev, TPM_STS(chip->locality), &data, 1);
+}
+
+static int tpm_tis_spi_recv_data(struct udevice *dev, u8 *buf, size_t count)
+{
+	struct tpm_chip *chip = dev_get_priv(dev);
+	int size = 0, burstcnt, len, ret;
+	u8 status;
+
+	while (size < count &&
+	       tpm_tis_spi_wait_for_stat(dev,
+					 TPM_STS_DATA_AVAIL | TPM_STS_VALID,
+					 chip->timeout_c, &status) == 0) {
+		burstcnt = tpm_tis_spi_get_burstcount(dev);
+		if (burstcnt < 0)
+			return burstcnt;
+
+		len = min_t(int, burstcnt, count - size);
+		ret = tpm_tis_spi_read(dev, TPM_DATA_FIFO(chip->locality),
+				       buf + size, len);
+		if (ret < 0)
+			return ret;
+
+		size += len;
+	}
+
+	return size;
+}
+
+static int tpm_tis_spi_recv(struct udevice *dev, u8 *buf, size_t count)
+{
+	struct tpm_chip *chip = dev_get_priv(dev);
+	int size, expected;
+
+	if (!chip)
+		return -ENODEV;
+
+	if (count < TPM_HEADER_SIZE) {
+		size = -EIO;
+		goto out;
+	}
+
+	size = tpm_tis_spi_recv_data(dev, buf, TPM_HEADER_SIZE);
+	if (size < TPM_HEADER_SIZE) {
+		debug("TPM error, unable to read header\n");
+		goto out;
+	}
+
+	expected = get_unaligned_be32(buf + 2);
+	if (expected > count) {
+		size = -EIO;
+		goto out;
+	}
+
+	size += tpm_tis_spi_recv_data(dev, &buf[TPM_HEADER_SIZE],
+				   expected - TPM_HEADER_SIZE);
+	if (size < expected) {
+		debug("TPM error, unable to read remaining bytes of result\n");
+		size = -EIO;
+		goto out;
+	}
+
+out:
+	tpm_tis_spi_cancel(dev);
+	tpm_tis_spi_release_locality(dev, chip->locality, false);
+
+	return size;
+}
+
+static int tpm_tis_spi_send(struct udevice *dev, const u8 *buf, size_t len)
+{
+	struct tpm_chip *chip = dev_get_priv(dev);
+	u32 i, size;
+	u8 status;
+	int burstcnt, ret;
+	u8 data;
+
+	if (!chip)
+		return -ENODEV;
+
+	if (len > TPM_DEV_BUFSIZE)
+		return -E2BIG;  /* Command is too long for our tpm, sorry */
+
+	ret = tpm_tis_spi_request_locality(dev, 0);
+	if (ret < 0)
+		return -EBUSY;
+
+	/*
+	 * Check if the TPM is ready. If not, if not, cancel the pending command
+	 * and poll on the status to be finally ready.
+	 */
+	ret = tpm_tis_spi_status(dev, &status);
+	if (ret)
+		return ret;
+
+	if (!(status & TPM_STS_COMMAND_READY)) {
+		/* Force the transition, usually this will be done@startup */
+		ret = tpm_tis_spi_cancel(dev);
+		if (ret) {
+			debug("%s: Could not cancel previous operation\n",
+			      __func__);
+			goto out_err;
+		}
+
+		ret = tpm_tis_spi_wait_for_stat(dev, TPM_STS_COMMAND_READY,
+						chip->timeout_b, &status);
+		if (ret < 0 || !(status & TPM_STS_COMMAND_READY)) {
+			debug("status %d after wait for stat returned %d\n",
+			      status, ret);
+			goto out_err;
+		}
+	}
+
+	for (i = 0; i < len - 1;) {
+		burstcnt = tpm_tis_spi_get_burstcount(dev);
+		if (burstcnt < 0)
+			return burstcnt;
+
+		size = min_t(int, len - i - 1, burstcnt);
+		ret = tpm_tis_spi_write(dev, TPM_DATA_FIFO(chip->locality),
+					buf + i, size);
+		if (ret < 0)
+			goto out_err;
+
+		i += size;
+	}
+
+	ret = tpm_tis_spi_status(dev, &status);
+	if (ret)
+		goto out_err;
+
+	if ((status & TPM_STS_DATA_EXPECT) == 0) {
+		ret = -EIO;
+		goto out_err;
+	}
+
+	ret = tpm_tis_spi_write(dev, TPM_DATA_FIFO(chip->locality),
+				buf + len - 1, 1);
+	if (ret)
+		goto out_err;
+
+	ret = tpm_tis_spi_status(dev, &status);
+	if (ret)
+		goto out_err;
+
+	if ((status & TPM_STS_DATA_EXPECT) != 0) {
+		ret = -EIO;
+		goto out_err;
+	}
+
+	data = TPM_STS_GO;
+	ret = tpm_tis_spi_write(dev, TPM_STS(chip->locality), &data, 1);
+	if (ret)
+		goto out_err;
+
+	return len;
+
+out_err:
+	tpm_tis_spi_cancel(dev);
+	tpm_tis_spi_release_locality(dev, chip->locality, false);
+
+	return ret;
+}
+
+static int tpm_tis_spi_cleanup(struct udevice *dev)
+{
+	struct tpm_chip *chip = dev_get_priv(dev);
+
+	tpm_tis_spi_cancel(dev);
+	/*
+	 * The TPM needs some time to clean up here,
+	 * so we sleep rather than keeping the bus busy
+	 */
+	mdelay(2);
+	tpm_tis_spi_release_locality(dev, chip->locality, false);
+
+	return 0;
+}
+
+static int tpm_tis_spi_open(struct udevice *dev)
+{
+	struct tpm_chip *chip = dev_get_priv(dev);
+
+	if (chip->is_open)
+		return -EBUSY;
+
+	chip->is_open = 1;
+
+	return 0;
+}
+
+static int tpm_tis_spi_close(struct udevice *dev)
+{
+	struct tpm_chip *chip = dev_get_priv(dev);
+
+	if (chip->is_open) {
+		tpm_tis_spi_release_locality(dev, chip->locality, true);
+		chip->is_open = 0;
+	}
+
+	return 0;
+}
+
+static int tpm_tis_get_desc(struct udevice *dev, char *buf, int size)
+{
+	struct tpm_chip *chip = dev_get_priv(dev);
+
+	if (size < 80)
+		return -ENOSPC;
+
+	return snprintf(buf, size,
+			"%s v2.0: VendorID 0x%04x, DeviceID 0x%04x, RevisionID 0x%02x [%s]",
+			dev->name, chip->vend_dev & 0xFFFF,
+			chip->vend_dev >> 16, chip->rid,
+			(chip->is_open ? "open" : "closed"));
+}
+
+static int tpm_tis_wait_init(struct udevice *dev, int loc)
+{
+	struct tpm_chip *chip = dev_get_priv(dev);
+	unsigned long start, stop;
+	u8 status;
+	int ret;
+
+	start = get_timer(0);
+	stop = chip->timeout_b;
+	do {
+		mdelay(TPM_TIMEOUT_MS);
+
+		ret = tpm_tis_spi_read(dev, TPM_ACCESS(loc), &status, 1);
+		if (ret)
+			break;
+
+		if (status & TPM_ACCESS_VALID)
+			return 0;
+	} while (get_timer(start) < stop);
+
+	return -EIO;
+}
+
+static int tpm_tis_spi_probe(struct udevice *dev)
+{
+	struct tpm_chip *chip = dev_get_priv(dev);
+	int ret;
+
+	chip->chip_type = dev_get_driver_data(dev);
+	chip->locality = 0;
+	chip->timeout_a = TIS_SHORT_TIMEOUT_MS;
+	chip->timeout_b = TIS_LONG_TIMEOUT_MS;
+	chip->timeout_c = TIS_SHORT_TIMEOUT_MS;
+	chip->timeout_d = TIS_SHORT_TIMEOUT_MS;
+
+	ret = tpm_tis_wait_init(dev, chip->locality);
+	if (ret) {
+		debug("%s: no device found\n", __func__);
+		return ret;
+	}
+
+	ret = tpm_tis_spi_request_locality(dev, chip->locality);
+	if (ret) {
+		debug("%s: could not request locality %d\n", __func__,
+		      chip->locality);
+		return ret;
+	}
+
+	ret = tpm_tis_spi_read32(dev, TPM_DID_VID(chip->locality),
+				 &chip->vend_dev);
+	if (ret) {
+		debug("%s: could not retrieve VendorID/DeviceID\n", __func__);
+		return ret;
+	}
+
+	ret = tpm_tis_spi_read(dev, TPM_RID(chip->locality), &chip->rid, 1);
+	if (ret) {
+		debug("%s: could not retrieve RevisionID\n", __func__);
+		return ret;
+	}
+
+	debug("SPI TPMv2.0 found (vid:%04x, did:%04x, rid:%02x)\n",
+	      chip->vend_dev & 0xFFFF, chip->vend_dev >> 16, chip->rid);
+
+	return 0;
+}
+
+static int tpm_tis_spi_remove(struct udevice *dev)
+{
+	struct tpm_chip *chip = dev_get_priv(dev);
+
+	tpm_tis_spi_release_locality(dev, chip->locality, true);
+
+	return 0;
+}
+
+static const struct tpm_ops tpm_tis_spi_ops = {
+	.open		= tpm_tis_spi_open,
+	.close		= tpm_tis_spi_close,
+	.get_desc	= tpm_tis_get_desc,
+	.send		= tpm_tis_spi_send,
+	.recv		= tpm_tis_spi_recv,
+	.cleanup	= tpm_tis_spi_cleanup,
+};
+
+static const struct udevice_id tpm_tis_spi_ids[] = {
+	{ .compatible = "st,st33tphf20-spi" },
+	{ }
+};
+
+U_BOOT_DRIVER(tpm_tis_spi) = {
+	.name   = "tpm_tis_spi",
+	.id     = UCLASS_TPM,
+	.of_match = tpm_tis_spi_ids,
+	.ops    = &tpm_tis_spi_ops,
+	.probe	= tpm_tis_spi_probe,
+	.remove	= tpm_tis_spi_remove,
+	.priv_auto_alloc_size = sizeof(struct tpm_chip),
+};
-- 
2.14.1

[U-Boot] [PATCH v2 04/19] tpm: fix indentation in command list before adding more

[Miquel Raynal]

Prepare the addition of more commands by first indenting correctly the
current list.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/tpm.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index d9b433582c..f456396d75 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -820,45 +820,45 @@ static int do_tpm_list(cmd_tbl_t *cmdtp, int flag, int argc,
 static cmd_tbl_t tpm_commands[] = {
 	U_BOOT_CMD_MKENT(info, 0, 1, do_tpm_info, "", ""),
 	U_BOOT_CMD_MKENT(init, 0, 1,
-			do_tpm_init, "", ""),
+			 do_tpm_init, "", ""),
 	U_BOOT_CMD_MKENT(startup, 0, 1,
-			do_tpm_startup, "", ""),
+			 do_tpm_startup, "", ""),
 	U_BOOT_CMD_MKENT(self_test_full, 0, 1,
-			do_tpm_self_test_full, "", ""),
+			 do_tpm_self_test_full, "", ""),
 	U_BOOT_CMD_MKENT(continue_self_test, 0, 1,
-			do_tpm_continue_self_test, "", ""),
+			 do_tpm_continue_self_test, "", ""),
 	U_BOOT_CMD_MKENT(force_clear, 0, 1,
-			do_tpm_force_clear, "", ""),
+			 do_tpm_force_clear, "", ""),
 	U_BOOT_CMD_MKENT(physical_enable, 0, 1,
-			do_tpm_physical_enable, "", ""),
+			 do_tpm_physical_enable, "", ""),
 	U_BOOT_CMD_MKENT(physical_disable, 0, 1,
-			do_tpm_physical_disable, "", ""),
+			 do_tpm_physical_disable, "", ""),
 	U_BOOT_CMD_MKENT(nv_define_space, 0, 1,
-			do_tpm_nv_define_space, "", ""),
+			 do_tpm_nv_define_space, "", ""),
 	U_BOOT_CMD_MKENT(nv_read_value, 0, 1,
-			do_tpm_nv_read_value, "", ""),
+			 do_tpm_nv_read_value, "", ""),
 	U_BOOT_CMD_MKENT(nv_write_value, 0, 1,
-			do_tpm_nv_write_value, "", ""),
+			 do_tpm_nv_write_value, "", ""),
 	U_BOOT_CMD_MKENT(extend, 0, 1,
-			do_tpm_extend, "", ""),
+			 do_tpm_extend, "", ""),
 	U_BOOT_CMD_MKENT(pcr_read, 0, 1,
-			do_tpm_pcr_read, "", ""),
+			 do_tpm_pcr_read, "", ""),
 	U_BOOT_CMD_MKENT(tsc_physical_presence, 0, 1,
-			do_tpm_tsc_physical_presence, "", ""),
+			 do_tpm_tsc_physical_presence, "", ""),
 	U_BOOT_CMD_MKENT(read_pubek, 0, 1,
-			do_tpm_read_pubek, "", ""),
+			 do_tpm_read_pubek, "", ""),
 	U_BOOT_CMD_MKENT(physical_set_deactivated, 0, 1,
-			do_tpm_physical_set_deactivated, "", ""),
+			 do_tpm_physical_set_deactivated, "", ""),
 	U_BOOT_CMD_MKENT(get_capability, 0, 1,
-			do_tpm_get_capability, "", ""),
+			 do_tpm_get_capability, "", ""),
 	U_BOOT_CMD_MKENT(raw_transfer, 0, 1,
-			do_tpm_raw_transfer, "", ""),
+			 do_tpm_raw_transfer, "", ""),
 	U_BOOT_CMD_MKENT(nv_define, 0, 1,
-			do_tpm_nv_define, "", ""),
+			 do_tpm_nv_define, "", ""),
 	U_BOOT_CMD_MKENT(nv_read, 0, 1,
-			do_tpm_nv_read, "", ""),
+			 do_tpm_nv_read, "", ""),
 	U_BOOT_CMD_MKENT(nv_write, 0, 1,
-			do_tpm_nv_write, "", ""),
+			 do_tpm_nv_write, "", ""),
 #ifdef CONFIG_TPM_AUTH_SESSIONS
 	U_BOOT_CMD_MKENT(oiap, 0, 1,
 			 do_tpm_oiap, "", ""),
-- 
2.14.1

[U-Boot] [PATCH v2 05/19] tpm: prepare support for TPMv2 commands

[Miquel Raynal]

Later choice between v1 and v2 compliant functions will be handled by a
global value in lib/tpm.c that will be accessible through set/get
accessors from lib/cmd.c.

This global value is set during the initialization phase if the TPM2
handle is given to the init command.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/tpm.c     | 37 +++++++++++++++++++++-----
 include/tpm.h | 20 ++++++++++++++
 lib/tpm.c     | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 131 insertions(+), 9 deletions(-)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index f456396d75..1d32028b64 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -89,12 +89,16 @@ static void *parse_byte_string(char *bytes, uint8_t *data, size_t *count_ptr)
  */
 static int report_return_code(int return_code)
 {
-	if (return_code) {
-		printf("Error: %d\n", return_code);
-		return CMD_RET_FAILURE;
-	} else {
+	if (!return_code)
 		return CMD_RET_SUCCESS;
-	}
+
+	if (return_code == -EOPNOTSUPP)
+		printf("TPMv%d error: unavailable command with this spec\n",
+		       tpm_get_specification());
+	else
+		printf("TPM error: %d\n", return_code);
+
+	return CMD_RET_FAILURE;
 }
 
 /**
@@ -427,6 +431,24 @@ static int do_tpm_get_capability(cmd_tbl_t *cmdtp, int flag,
 	return report_return_code(rc);
 }
 
+static int do_tpm_init(cmd_tbl_t *cmdtp, int flag,
+		       int argc, char * const argv[])
+{
+	if (argc > 2)
+		return CMD_RET_USAGE;
+
+	if (argc == 2) {
+		if (!strcasecmp("TPM1", argv[1]))
+			tpm_set_specification(1);
+		else if (!strcasecmp("TPM2", argv[1]))
+			tpm_set_specification(2);
+		else
+			return CMD_RET_USAGE;
+	}
+
+	return report_return_code(tpm_init());
+}
+
 #define TPM_COMMAND_NO_ARG(cmd)				\
 static int do_##cmd(cmd_tbl_t *cmdtp, int flag,		\
 		int argc, char * const argv[])		\
@@ -436,7 +458,6 @@ static int do_##cmd(cmd_tbl_t *cmdtp, int flag,		\
 	return report_return_code(cmd());		\
 }
 
-TPM_COMMAND_NO_ARG(tpm_init)
 TPM_COMMAND_NO_ARG(tpm_self_test_full)
 TPM_COMMAND_NO_ARG(tpm_continue_self_test)
 TPM_COMMAND_NO_ARG(tpm_force_clear)
@@ -902,8 +923,10 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
 "    - Issue TPM command <cmd> with arguments <args...>.\n"
 "Admin Startup and State Commands:\n"
 "  info - Show information about the TPM\n"
-"  init\n"
+"  init [<type>]\n"
 "    - Put TPM into a state where it waits for 'startup' command.\n"
+"      <type> is one of TPM1 (default) or TPM2. This choice impacts the way\n"
+"      other functions will behave.\n"
 "  startup mode\n"
 "    - Issue TPM_Starup command.  <mode> is one of TPM_ST_CLEAR,\n"
 "      TPM_ST_STATE, and TPM_ST_DEACTIVATED.\n"
diff --git a/include/tpm.h b/include/tpm.h
index 760d94865c..0ec3428ea4 100644
--- a/include/tpm.h
+++ b/include/tpm.h
@@ -30,6 +30,11 @@ enum tpm_startup_type {
 	TPM_ST_DEACTIVATED	= 0x0003,
 };
 
+enum tpm2_startup_types {
+	TPM2_SU_CLEAR	= 0x0000,
+	TPM2_SU_STATE	= 0x0001,
+};
+
 enum tpm_physical_presence {
 	TPM_PHYSICAL_PRESENCE_HW_DISABLE	= 0x0200,
 	TPM_PHYSICAL_PRESENCE_CMD_DISABLE	= 0x0100,
@@ -417,6 +422,21 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size,
  */
 int tpm_init(void);
 
+/**
+ * Assign a value to the global is_nfcv2 boolean to discriminate in the lib
+ * between the available specifications.
+ *
+ * @version: 1 or 2, depending on the supported specification
+ */
+int tpm_set_specification(int version);
+
+/**
+ * Return the current value of the specification.
+ *
+ * @return: 1 or 2, depending on the supported specification
+ */
+int tpm_get_specification(void);
+
 /**
  * Issue a TPM_Startup command.
  *
diff --git a/lib/tpm.c b/lib/tpm.c
index 99556b1cf6..38b76b4961 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -15,6 +15,9 @@
 /* Internal error of TPM command library */
 #define TPM_LIB_ERROR	((uint32_t)~0u)
 
+/* Global boolean to discriminate TPMv2.x from TPMv1.x functions */
+static bool is_tpmv2;
+
 /* Useful constants */
 enum {
 	COMMAND_BUFFER_SIZE		= 256,
@@ -262,6 +265,26 @@ static uint32_t tpm_sendrecv_command(const void *command,
 	return tpm_return_code(response);
 }
 
+int tpm_set_specification(int version)
+{
+	if (version == 1) {
+		debug("TPM complies to the v1.x specification\n");
+		is_tpmv2 = false;
+	} else if (version == 2) {
+		debug("TPM complies to the v2.x specification\n");
+		is_tpmv2 = true;
+	} else {
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+int tpm_get_specification(void)
+{
+	return is_tpmv2 ? 2 : 1;
+}
+
 int tpm_init(void)
 {
 	int err;
@@ -338,6 +361,9 @@ uint32_t tpm_nv_define_space(uint32_t index, uint32_t perm, uint32_t size)
 	const size_t size_offset = 77;
 	uint8_t buf[COMMAND_BUFFER_SIZE];
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	if (pack_byte_string(buf, sizeof(buf), "sddd",
 				0, command, sizeof(command),
 				index_offset, index,
@@ -362,6 +388,9 @@ uint32_t tpm_nv_read_value(uint32_t index, void *data, uint32_t count)
 	uint32_t data_size;
 	uint32_t err;
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	if (pack_byte_string(buf, sizeof(buf), "sdd",
 				0, command, sizeof(command),
 				index_offset, index,
@@ -398,6 +427,9 @@ uint32_t tpm_nv_write_value(uint32_t index, const void *data, uint32_t length)
 	size_t response_length = sizeof(response);
 	uint32_t err;
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	if (pack_byte_string(buf, sizeof(buf), "sddds",
 				0, command, sizeof(command),
 				command_size_offset, total_length,
@@ -425,6 +457,9 @@ uint32_t tpm_extend(uint32_t index, const void *in_digest, void *out_digest)
 	size_t response_length = sizeof(response);
 	uint32_t err;
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	if (pack_byte_string(buf, sizeof(buf), "sds",
 				0, command, sizeof(command),
 				index_offset, index,
@@ -454,8 +489,8 @@ uint32_t tpm_pcr_read(uint32_t index, void *data, size_t count)
 	size_t response_length = sizeof(response);
 	uint32_t err;
 
-	if (count < PCR_DIGEST_LENGTH)
-		return TPM_LIB_ERROR;
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
 
 	if (pack_byte_string(buf, sizeof(buf), "sd",
 				0, command, sizeof(command),
@@ -479,6 +514,9 @@ uint32_t tpm_tsc_physical_presence(uint16_t presence)
 	const size_t presence_offset = 10;
 	uint8_t buf[COMMAND_BUFFER_SIZE];
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	if (pack_byte_string(buf, sizeof(buf), "sw",
 				0, command, sizeof(command),
 				presence_offset, presence))
@@ -500,6 +538,9 @@ uint32_t tpm_read_pubek(void *data, size_t count)
 	uint32_t data_size;
 	uint32_t err;
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	err = tpm_sendrecv_command(command, response, &response_length);
 	if (err)
 		return err;
@@ -533,6 +574,9 @@ uint32_t tpm_physical_enable(void)
 		0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x6f,
 	};
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	return tpm_sendrecv_command(command, NULL, NULL);
 }
 
@@ -542,6 +586,9 @@ uint32_t tpm_physical_disable(void)
 		0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x70,
 	};
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	return tpm_sendrecv_command(command, NULL, NULL);
 }
 
@@ -553,6 +600,9 @@ uint32_t tpm_physical_set_deactivated(uint8_t state)
 	const size_t state_offset = 10;
 	uint8_t buf[COMMAND_BUFFER_SIZE];
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	if (pack_byte_string(buf, sizeof(buf), "sb",
 				0, command, sizeof(command),
 				state_offset, state))
@@ -618,6 +668,9 @@ uint32_t tpm_get_permanent_flags(struct tpm_permanent_flags *pflags)
 	uint32_t err;
 	uint32_t data_size;
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	err = tpm_sendrecv_command(command, response, &response_length);
 	if (err)
 		return err;
@@ -648,6 +701,9 @@ uint32_t tpm_get_permissions(uint32_t index, uint32_t *perm)
 	size_t response_length = sizeof(response);
 	uint32_t err;
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	if (pack_byte_string(buf, sizeof(buf), "d", 0, command, sizeof(command),
 			     index_offset, index))
 		return TPM_LIB_ERROR;
@@ -677,6 +733,9 @@ uint32_t tpm_flush_specific(uint32_t key_handle, uint32_t resource_type)
 	size_t response_length = sizeof(response);
 	uint32_t err;
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	if (pack_byte_string(buf, sizeof(buf), "sdd",
 			     0, command, sizeof(command),
 			     key_handle_offset, key_handle,
@@ -835,6 +894,9 @@ uint32_t tpm_terminate_auth_session(uint32_t auth_handle)
 	const size_t req_handle_offset = TPM_REQUEST_HEADER_LENGTH;
 	uint8_t request[COMMAND_BUFFER_SIZE];
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	if (pack_byte_string(request, sizeof(request), "sd",
 			     0, command, sizeof(command),
 			     req_handle_offset, auth_handle))
@@ -848,8 +910,13 @@ uint32_t tpm_terminate_auth_session(uint32_t auth_handle)
 uint32_t tpm_end_oiap(void)
 {
 	uint32_t err = TPM_SUCCESS;
+
 	if (oiap_session.valid)
 		err = tpm_terminate_auth_session(oiap_session.handle);
+
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	return err;
 }
 
@@ -866,6 +933,9 @@ uint32_t tpm_oiap(uint32_t *auth_handle)
 	size_t response_length = sizeof(response);
 	uint32_t err;
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	if (oiap_session.valid)
 		tpm_terminate_auth_session(oiap_session.handle);
 
@@ -904,6 +974,9 @@ uint32_t tpm_load_key2_oiap(uint32_t parent_handle,
 	size_t response_length = sizeof(response);
 	uint32_t err;
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	if (!oiap_session.valid) {
 		err = tpm_oiap(NULL);
 		if (err)
@@ -967,6 +1040,9 @@ uint32_t tpm_get_pub_key_oiap(uint32_t key_handle, const void *usage_auth,
 	size_t response_length = sizeof(response);
 	uint32_t err;
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	if (!oiap_session.valid) {
 		err = tpm_oiap(NULL);
 		if (err)
@@ -1025,6 +1101,9 @@ uint32_t tpm_find_key_sha1(const uint8_t auth[20], const uint8_t
 	size_t buf_len;
 	unsigned int i;
 
+	if (is_tpmv2)
+		return -EOPNOTSUPP;
+
 	/* fetch list of already loaded keys in the TPM */
 	err = tpm_get_capability(TPM_CAP_HANDLE, TPM_RT_KEY, buf, sizeof(buf));
 	if (err)
-- 
2.14.1

[U-Boot] [PATCH v2 06/19] tpm: add macros for TPMv2 commands

[Miquel Raynal]

TPM commands are much easier to handle with these macros that will
transform words or integers into byte string. This way, there is no need
to call pack_byte_string() while all variable length in a command are
known (and at must 4 bytes, which is a lot of them).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 lib/tpm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/tpm.c b/lib/tpm.c
index 38b76b4961..2b329145be 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -15,6 +15,12 @@
 /* Internal error of TPM command library */
 #define TPM_LIB_ERROR	((uint32_t)~0u)
 
+/* To make strings of commands more easily */
+#define __MSB(x) ((x) >> 8)
+#define __LSB(x) ((x) & 0xFF)
+#define U16_TO_ARRAY(x) __MSB(x), __LSB(x)
+#define U32_TO_ARRAY(x) U16_TO_ARRAY((x) >> 16), U16_TO_ARRAY((x) & 0xFFFF)
+
 /* Global boolean to discriminate TPMv2.x from TPMv1.x functions */
 static bool is_tpmv2;
 
-- 
2.14.1

[U-Boot] [PATCH v2 07/19] tpm: add possible traces to analyze buffers returned by the TPM

[Miquel Raynal]

When debugging, it is welcome to get more information about what the TPM
returns. Add the possibility to print the packets received to show their
exact content.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 lib/tpm.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/tpm.c b/lib/tpm.c
index 2b329145be..aa46ec1693 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -249,6 +249,7 @@ static uint32_t tpm_sendrecv_command(const void *command,
 	int err, ret;
 	uint8_t response_buffer[COMMAND_BUFFER_SIZE];
 	size_t response_length;
+	int i;
 
 	if (response) {
 		response_length = *size_ptr;
@@ -260,15 +261,24 @@ static uint32_t tpm_sendrecv_command(const void *command,
 	ret = uclass_first_device_err(UCLASS_TPM, &dev);
 	if (ret)
 		return ret;
+
 	err = tpm_xfer(dev, command, tpm_command_size(command),
 		       response, &response_length);
 
 	if (err < 0)
 		return TPM_LIB_ERROR;
+
 	if (size_ptr)
 		*size_ptr = response_length;
 
-	return tpm_return_code(response);
+	ret = tpm_return_code(response);
+
+	debug("TPM response [ret:%d]: ", ret);
+	for (i = 0; i < response_length; i++)
+		debug("%02x ", ((u8 *)response)[i]);
+	debug("\n");
+
+	return ret;
 }
 
 int tpm_set_specification(int version)
-- 
2.14.1

[U-Boot] [PATCH v2 08/19] tpm: handle different buffer sizes

[Miquel Raynal]

Usual buffer sizes for TPMv1 and TPMv2 are different. Change TPMv1
buffer size definition for that and declare another size for TPMv2
buffers.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/tpm.c     |  5 +++--
 include/tpm.h |  2 ++
 lib/tpm.c     | 60 +++++++++++++++++++++++++++++------------------------------
 3 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index 1d32028b64..3e2bb3b118 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -323,8 +323,9 @@ static int do_tpm_nv_write_value(cmd_tbl_t *cmdtp, int flag,
 static int do_tpm_extend(cmd_tbl_t *cmdtp, int flag,
 		int argc, char * const argv[])
 {
+	uint8_t in_digest[TPM1_DIGEST_LENGTH];
+	uint8_t out_digest[TPM1_DIGEST_LENGTH];
 	uint32_t index, rc;
-	uint8_t in_digest[20], out_digest[20];
 
 	if (argc != 3)
 		return CMD_RET_USAGE;
@@ -337,7 +338,7 @@ static int do_tpm_extend(cmd_tbl_t *cmdtp, int flag,
 	rc = tpm_extend(index, in_digest, out_digest);
 	if (!rc) {
 		puts("PCR value after execution of the command:\n");
-		print_byte_string(out_digest, sizeof(out_digest));
+		print_byte_string(out_digest, TPM1_DIGEST_LENGTH);
 	}
 
 	return report_return_code(rc);
diff --git a/include/tpm.h b/include/tpm.h
index 0ec3428ea4..1a60ef5b36 100644
--- a/include/tpm.h
+++ b/include/tpm.h
@@ -14,6 +14,8 @@
  */
 
 #define TPM_HEADER_SIZE		10
+#define TPM1_DIGEST_LENGTH	20
+#define TPM2_DIGEST_LENGTH	32
 
 enum tpm_duration {
 	TPM_SHORT = 0,
diff --git a/lib/tpm.c b/lib/tpm.c
index aa46ec1693..c0fbba86ff 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -29,8 +29,6 @@ enum {
 	COMMAND_BUFFER_SIZE		= 256,
 	TPM_REQUEST_HEADER_LENGTH	= 10,
 	TPM_RESPONSE_HEADER_LENGTH	= 10,
-	PCR_DIGEST_LENGTH		= 20,
-	DIGEST_LENGTH			= 20,
 	TPM_REQUEST_AUTH_LENGTH		= 45,
 	TPM_RESPONSE_AUTH_LENGTH	= 41,
 	/* some max lengths, valid for RSA keys <= 2048 bits */
@@ -47,8 +45,8 @@ enum {
 struct session_data {
 	int		valid;
 	uint32_t	handle;
-	uint8_t		nonce_even[DIGEST_LENGTH];
-	uint8_t		nonce_odd[DIGEST_LENGTH];
+	uint8_t		nonce_even[TPM1_DIGEST_LENGTH];
+	uint8_t		nonce_odd[TPM1_DIGEST_LENGTH];
 };
 
 static struct session_data oiap_session = {0, };
@@ -469,7 +467,7 @@ uint32_t tpm_extend(uint32_t index, const void *in_digest, void *out_digest)
 	const size_t in_digest_offset = 14;
 	const size_t out_digest_offset = 10;
 	uint8_t buf[COMMAND_BUFFER_SIZE];
-	uint8_t response[TPM_RESPONSE_HEADER_LENGTH + PCR_DIGEST_LENGTH];
+	uint8_t response[TPM_RESPONSE_HEADER_LENGTH + TPM1_DIGEST_LENGTH];
 	size_t response_length = sizeof(response);
 	uint32_t err;
 
@@ -477,18 +475,18 @@ uint32_t tpm_extend(uint32_t index, const void *in_digest, void *out_digest)
 		return -EOPNOTSUPP;
 
 	if (pack_byte_string(buf, sizeof(buf), "sds",
-				0, command, sizeof(command),
-				index_offset, index,
-				in_digest_offset, in_digest,
-				PCR_DIGEST_LENGTH))
+			     0, command, sizeof(command),
+			     index_offset, index,
+			     in_digest_offset, in_digest,
+			     TPM1_DIGEST_LENGTH))
 		return TPM_LIB_ERROR;
 	err = tpm_sendrecv_command(buf, response, &response_length);
 	if (err)
 		return err;
 
 	if (unpack_byte_string(response, response_length, "s",
-				out_digest_offset, out_digest,
-				PCR_DIGEST_LENGTH))
+			       out_digest_offset, out_digest,
+			       TPM1_DIGEST_LENGTH))
 		return TPM_LIB_ERROR;
 
 	return 0;
@@ -516,7 +514,7 @@ uint32_t tpm_pcr_read(uint32_t index, void *data, size_t count)
 	if (err)
 		return err;
 	if (unpack_byte_string(response, response_length, "s",
-				out_digest_offset, data, PCR_DIGEST_LENGTH))
+				out_digest_offset, data, TPM1_DIGEST_LENGTH))
 		return TPM_LIB_ERROR;
 
 	return 0;
@@ -784,7 +782,7 @@ static uint32_t create_request_auth(const void *request, size_t request_len0,
 	struct session_data *auth_session,
 	void *request_auth, const void *auth)
 {
-	uint8_t hmac_data[DIGEST_LENGTH * 3 + 1];
+	uint8_t hmac_data[TPM1_DIGEST_LENGTH * 3 + 1];
 	sha1_context hash_ctx;
 	const size_t command_code_offset = 6;
 	const size_t auth_nonce_odd_offset = 4;
@@ -804,25 +802,25 @@ static uint32_t create_request_auth(const void *request, size_t request_len0,
 	sha1_finish(&hash_ctx, hmac_data);
 
 	sha1_starts(&hash_ctx);
-	sha1_update(&hash_ctx, auth_session->nonce_odd, DIGEST_LENGTH);
+	sha1_update(&hash_ctx, auth_session->nonce_odd, TPM1_DIGEST_LENGTH);
 	sha1_update(&hash_ctx, hmac_data, sizeof(hmac_data));
 	sha1_finish(&hash_ctx, auth_session->nonce_odd);
 
 	if (pack_byte_string(request_auth, TPM_REQUEST_AUTH_LENGTH, "dsb",
 			     0, auth_session->handle,
 			     auth_nonce_odd_offset, auth_session->nonce_odd,
-			     DIGEST_LENGTH,
+			     TPM1_DIGEST_LENGTH,
 			     auth_continue_offset, 1))
 		return TPM_LIB_ERROR;
 	if (pack_byte_string(hmac_data, sizeof(hmac_data), "ss",
-			     DIGEST_LENGTH,
+			     TPM1_DIGEST_LENGTH,
 			     auth_session->nonce_even,
-			     DIGEST_LENGTH,
-			     2 * DIGEST_LENGTH,
+			     TPM1_DIGEST_LENGTH,
+			     2 * TPM1_DIGEST_LENGTH,
 			     request_auth + auth_nonce_odd_offset,
-			     DIGEST_LENGTH + 1))
+			     TPM1_DIGEST_LENGTH + 1))
 		return TPM_LIB_ERROR;
-	sha1_hmac(auth, DIGEST_LENGTH, hmac_data, sizeof(hmac_data),
+	sha1_hmac(auth, TPM1_DIGEST_LENGTH, hmac_data, sizeof(hmac_data),
 		  request_auth + auth_auth_offset);
 
 	return TPM_SUCCESS;
@@ -848,8 +846,8 @@ static uint32_t verify_response_auth(uint32_t command_code,
 	struct session_data *auth_session,
 	const void *response_auth, const void *auth)
 {
-	uint8_t hmac_data[DIGEST_LENGTH * 3 + 1];
-	uint8_t computed_auth[DIGEST_LENGTH];
+	uint8_t hmac_data[TPM1_DIGEST_LENGTH * 3 + 1];
+	uint8_t computed_auth[TPM1_DIGEST_LENGTH];
 	sha1_context hash_ctx;
 	const size_t return_code_offset = 6;
 	const size_t auth_continue_offset = 20;
@@ -874,24 +872,24 @@ static uint32_t verify_response_auth(uint32_t command_code,
 			    - handles_len);
 	sha1_finish(&hash_ctx, hmac_data);
 
-	memcpy(auth_session->nonce_even, response_auth, DIGEST_LENGTH);
+	memcpy(auth_session->nonce_even, response_auth, TPM1_DIGEST_LENGTH);
 	auth_continue = ((uint8_t *)response_auth)[auth_continue_offset];
 	if (pack_byte_string(hmac_data, sizeof(hmac_data), "ssb",
-			     DIGEST_LENGTH,
+			     TPM1_DIGEST_LENGTH,
 			     response_auth,
-			     DIGEST_LENGTH,
-			     2 * DIGEST_LENGTH,
+			     TPM1_DIGEST_LENGTH,
+			     2 * TPM1_DIGEST_LENGTH,
 			     auth_session->nonce_odd,
-			     DIGEST_LENGTH,
-			     3 * DIGEST_LENGTH,
+			     TPM1_DIGEST_LENGTH,
+			     3 * TPM1_DIGEST_LENGTH,
 			     auth_continue))
 		return TPM_LIB_ERROR;
 
-	sha1_hmac(auth, DIGEST_LENGTH, hmac_data, sizeof(hmac_data),
+	sha1_hmac(auth, TPM1_DIGEST_LENGTH, hmac_data, sizeof(hmac_data),
 		  computed_auth);
 
 	if (memcmp(computed_auth, response_auth + auth_auth_offset,
-		   DIGEST_LENGTH))
+		   TPM1_DIGEST_LENGTH))
 		return TPM_AUTHFAIL;
 
 	return TPM_SUCCESS;
@@ -961,7 +959,7 @@ uint32_t tpm_oiap(uint32_t *auth_handle)
 	if (unpack_byte_string(response, response_length, "ds",
 			       res_auth_handle_offset, &oiap_session.handle,
 			       res_nonce_even_offset, &oiap_session.nonce_even,
-			       (uint32_t)DIGEST_LENGTH))
+			       (uint32_t)TPM1_DIGEST_LENGTH))
 		return TPM_LIB_ERROR;
 	oiap_session.valid = 1;
 	if (auth_handle)
-- 
2.14.1

[U-Boot] [PATCH v2 09/19] tpm: add TPM2_Startup command support

[Miquel Raynal]

Add support for the TPM2_Startup command.

Change the command file and the help accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/tpm.c     |  9 +++++++--
 include/tpm.h | 26 +++++++++++++++++++++++++-
 lib/tpm.c     | 35 +++++++++++++++++++++++++----------
 3 files changed, 57 insertions(+), 13 deletions(-)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index 3e2bb3b118..fc9ef9d4a3 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -255,6 +255,10 @@ static int do_tpm_startup(cmd_tbl_t *cmdtp, int flag,
 		mode = TPM_ST_STATE;
 	} else if (!strcasecmp("TPM_ST_DEACTIVATED", argv[1])) {
 		mode = TPM_ST_DEACTIVATED;
+	} else if (!strcasecmp("TPM2_SU_CLEAR", argv[1])) {
+		mode = TPM2_SU_CLEAR;
+	} else if (!strcasecmp("TPM2_SU_STATE", argv[1])) {
+		mode = TPM2_SU_STATE;
 	} else {
 		printf("Couldn't recognize mode string: %s\n", argv[1]);
 		return CMD_RET_FAILURE;
@@ -929,8 +933,9 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
 "      <type> is one of TPM1 (default) or TPM2. This choice impacts the way\n"
 "      other functions will behave.\n"
 "  startup mode\n"
-"    - Issue TPM_Starup command.  <mode> is one of TPM_ST_CLEAR,\n"
-"      TPM_ST_STATE, and TPM_ST_DEACTIVATED.\n"
+"    - Issue TPM_Startup command. <mode> is one of:\n"
+"          * TPM_ST_CLEAR, TPM_ST_STATE, TPM_ST_DEACTIVATED for TPMv1.x.\n"
+"          * TPM2_SU_CLEAR, TPM2_SU_STATE, for TPMv2.x.\n"
 "Admin Testing Commands:\n"
 "  self_test_full\n"
 "    - Test all of the TPM capabilities.\n"
diff --git a/include/tpm.h b/include/tpm.h
index 1a60ef5b36..ba71bac885 100644
--- a/include/tpm.h
+++ b/include/tpm.h
@@ -26,6 +26,11 @@ enum tpm_duration {
 	TPM_DURATION_COUNT,
 };
 
+enum tpm2_structures {
+	TPM2_ST_NO_SESSIONS	= 0x8001,
+	TPM2_ST_SESSIONS	= 0x8002,
+};
+
 enum tpm_startup_type {
 	TPM_ST_CLEAR		= 0x0001,
 	TPM_ST_STATE		= 0x0002,
@@ -37,6 +42,25 @@ enum tpm2_startup_types {
 	TPM2_SU_STATE	= 0x0001,
 };
 
+enum tpm2_command_codes {
+	TPM2_CC_STARTUP		= 0x0144,
+	TPM2_CC_SELF_TEST	= 0x0143,
+	TPM2_CC_GET_CAPABILITY	= 0x017A,
+	TPM2_CC_PCR_READ	= 0x017E,
+	TPM2_CC_PCR_EXTEND	= 0x0182,
+};
+
+enum tpm2_return_codes {
+	TPM2_RC_SUCCESS		= 0x0000,
+	TPM2_RC_HASH		= 0x0083, /* RC_FMT1 */
+	TPM2_RC_HANDLE		= 0x008B,
+	TPM2_RC_INITIALIZE	= 0x0100, /* RC_VER1 */
+	TPM2_RC_DISABLED	= 0x0120,
+	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
+	TPM2_RC_REFERENCE_H0	= 0x0910,
+	TPM2_RC_LOCKOUT		= 0x0921,
+};
+
 enum tpm_physical_presence {
 	TPM_PHYSICAL_PRESENCE_HW_DISABLE	= 0x0200,
 	TPM_PHYSICAL_PRESENCE_CMD_DISABLE	= 0x0100,
@@ -445,7 +469,7 @@ int tpm_get_specification(void);
  * @param mode		TPM startup mode
  * @return return code of the operation
  */
-uint32_t tpm_startup(enum tpm_startup_type mode);
+int tpm_startup(enum tpm_startup_type mode);
 
 /**
  * Issue a TPM_SelfTestFull command.
diff --git a/lib/tpm.c b/lib/tpm.c
index c0fbba86ff..0c57ef8dc7 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -310,20 +310,35 @@ int tpm_init(void)
 	return tpm_open(dev);
 }
 
-uint32_t tpm_startup(enum tpm_startup_type mode)
+int tpm_startup(enum tpm_startup_type mode)
 {
-	const uint8_t command[12] = {
-		0x0, 0xc1, 0x0, 0x0, 0x0, 0xc, 0x0, 0x0, 0x0, 0x99, 0x0, 0x0,
+	const u8 command_v1[12] = {
+		U16_TO_ARRAY(0xc1),
+		U32_TO_ARRAY(12),
+		U32_TO_ARRAY(0x99),
+		U16_TO_ARRAY(mode),
 	};
-	const size_t mode_offset = 10;
-	uint8_t buf[COMMAND_BUFFER_SIZE];
+	const u8 command_v2[12] = {
+		U16_TO_ARRAY(TPM2_ST_NO_SESSIONS),
+		U32_TO_ARRAY(12),
+		U32_TO_ARRAY(TPM2_CC_STARTUP),
+		U16_TO_ARRAY(mode),
+	};
+	int ret;
+
+	if (!is_tpmv2)
+		return tpm_sendrecv_command(command_v1, NULL, NULL);
+
+	ret = tpm_sendrecv_command(command_v2, NULL, NULL);
 
-	if (pack_byte_string(buf, sizeof(buf), "sw",
-				0, command, sizeof(command),
-				mode_offset, mode))
-		return TPM_LIB_ERROR;
+	/*
+	 * Note TPMv2: STARTUP command will return RC_SUCCESS the first time,
+	 * but will return RC_INITIALIZE otherwise.
+	 */
+	if (ret && ret != TPM2_RC_INITIALIZE)
+		return ret;
 
-	return tpm_sendrecv_command(buf, NULL, NULL);
+	return 0;
 }
 
 uint32_t tpm_self_test_full(void)
-- 
2.14.1

[U-Boot] [PATCH v2 10/19] tpm: add TPM2_SelfTest command support

[Miquel Raynal]

Add support for the TPM2_Selftest command.

Change the command file and the help accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/tpm.h |  9 +++++++--
 lib/tpm.c     | 36 ++++++++++++++++++++++++++++--------
 2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/include/tpm.h b/include/tpm.h
index ba71bac885..38d7cb899d 100644
--- a/include/tpm.h
+++ b/include/tpm.h
@@ -31,6 +31,11 @@ enum tpm2_structures {
 	TPM2_ST_SESSIONS	= 0x8002,
 };
 
+enum tpm2_yes_no {
+	TPMI_YES		= 1,
+	TPMI_NO			= 0,
+};
+
 enum tpm_startup_type {
 	TPM_ST_CLEAR		= 0x0001,
 	TPM_ST_STATE		= 0x0002,
@@ -476,14 +481,14 @@ int tpm_startup(enum tpm_startup_type mode);
  *
  * @return return code of the operation
  */
-uint32_t tpm_self_test_full(void);
+int tpm_self_test_full(void);
 
 /**
  * Issue a TPM_ContinueSelfTest command.
  *
  * @return return code of the operation
  */
-uint32_t tpm_continue_self_test(void);
+int tpm_continue_self_test(void);
 
 /**
  * Issue a TPM_NV_DefineSpace command.  The implementation is limited
diff --git a/lib/tpm.c b/lib/tpm.c
index 0c57ef8dc7..3cc2888267 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -341,20 +341,40 @@ int tpm_startup(enum tpm_startup_type mode)
 	return 0;
 }
 
-uint32_t tpm_self_test_full(void)
+int tpm_self_test_full(void)
 {
-	const uint8_t command[10] = {
-		0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x50,
+	const u8 command_v1[10] = {
+		U16_TO_ARRAY(0xc1),
+		U32_TO_ARRAY(10),
+		U32_TO_ARRAY(0x50),
 	};
-	return tpm_sendrecv_command(command, NULL, NULL);
+	const u8 command_v2[12] = {
+		U16_TO_ARRAY(TPM2_ST_NO_SESSIONS),
+		U32_TO_ARRAY(11),
+		U32_TO_ARRAY(TPM2_CC_SELF_TEST),
+		TPMI_YES,
+	};
+
+	return tpm_sendrecv_command(is_tpmv2 ? command_v2 : command_v1, NULL,
+				    NULL);
 }
 
-uint32_t tpm_continue_self_test(void)
+int tpm_continue_self_test(void)
 {
-	const uint8_t command[10] = {
-		0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x53,
+	const u8 command_v1[10] = {
+		U16_TO_ARRAY(0xc1),
+		U32_TO_ARRAY(10),
+		U32_TO_ARRAY(0x53),
 	};
-	return tpm_sendrecv_command(command, NULL, NULL);
+	const u8 command_v2[12] = {
+		U16_TO_ARRAY(TPM2_ST_NO_SESSIONS),
+		U32_TO_ARRAY(11),
+		U32_TO_ARRAY(TPM2_CC_SELF_TEST),
+		TPMI_NO,
+	};
+
+	return tpm_sendrecv_command(is_tpmv2 ? command_v2 : command_v1, NULL,
+				    NULL);
 }
 
 uint32_t tpm_nv_define_space(uint32_t index, uint32_t perm, uint32_t size)
-- 
2.14.1

[U-Boot] [PATCH v2 11/19] tpm: add TPM2_Clear command support

[Miquel Raynal]

Add support for the TPM2_Clear command.

Change the command file and the help accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/tpm.c      | 29 ++++++++++++++++++++++++++---
 cmd/tpm_test.c |  6 +++---
 include/tpm.h  | 21 +++++++++++++++++----
 lib/tpm.c      | 42 ++++++++++++++++++++++++++++++++++++++----
 4 files changed, 84 insertions(+), 14 deletions(-)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index fc9ef9d4a3..32921e1a70 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -454,6 +454,29 @@ static int do_tpm_init(cmd_tbl_t *cmdtp, int flag,
 	return report_return_code(tpm_init());
 }
 
+static int do_tpm_force_clear(cmd_tbl_t *cmdtp, int flag,
+			      int argc, char * const argv[])
+{
+	u32 handle = 0;
+	const char *pw = (argc < 3) ? NULL : argv[2];
+	const ssize_t pw_sz = pw ? strlen(pw) : 0;
+
+	if (argc < 2 || argc > 3)
+		return CMD_RET_USAGE;
+
+	if (pw_sz > TPM2_DIGEST_LENGTH)
+		return -EINVAL;
+
+	if (!strcasecmp("TPM2_RH_LOCKOUT", argv[1]))
+		handle = TPM2_RH_LOCKOUT;
+	else if (!strcasecmp("TPM2_RH_PLATFORM", argv[1]))
+		handle = TPM2_RH_PLATFORM;
+	else
+		return CMD_RET_USAGE;
+
+	return report_return_code(tpm_force_clear(handle, pw, pw_sz));
+}
+
 #define TPM_COMMAND_NO_ARG(cmd)				\
 static int do_##cmd(cmd_tbl_t *cmdtp, int flag,		\
 		int argc, char * const argv[])		\
@@ -465,7 +488,6 @@ static int do_##cmd(cmd_tbl_t *cmdtp, int flag,		\
 
 TPM_COMMAND_NO_ARG(tpm_self_test_full)
 TPM_COMMAND_NO_ARG(tpm_continue_self_test)
-TPM_COMMAND_NO_ARG(tpm_force_clear)
 TPM_COMMAND_NO_ARG(tpm_physical_enable)
 TPM_COMMAND_NO_ARG(tpm_physical_disable)
 
@@ -951,8 +973,9 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
 "  physical_set_deactivated 0|1\n"
 "    - Set deactivated flag.\n"
 "Admin Ownership Commands:\n"
-"  force_clear\n"
-"    - Issue TPM_ForceClear command.\n"
+"  force_clear [<type>]\n"
+"    - Issue TPM_[Force]Clear command, with <type> one of (TPMv2 only):\n"
+"          * TPM2_RH_LOCKOUT, TPM2_RH_PLATFORM.\n"
 "  tsc_physical_presence flags\n"
 "    - Set TPM device's Physical Presence flags to <flags>.\n"
 "The Capability Commands:\n"
diff --git a/cmd/tpm_test.c b/cmd/tpm_test.c
index 37ad2ff33d..da40dbc423 100644
--- a/cmd/tpm_test.c
+++ b/cmd/tpm_test.c
@@ -176,7 +176,7 @@ static int test_fast_enable(void)
 	TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL));
 	printf("\tdisable is %d, deactivated is %d\n", disable, deactivated);
 	for (i = 0; i < 2; i++) {
-		TPM_CHECK(tpm_force_clear());
+		TPM_CHECK(tpm_force_clear(0, NULL, 0));
 		TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL));
 		printf("\tdisable is %d, deactivated is %d\n", disable,
 		       deactivated);
@@ -458,7 +458,7 @@ static int test_write_limit(void)
 	TPM_CHECK(TlclStartupIfNeeded());
 	TPM_CHECK(tpm_self_test_full());
 	TPM_CHECK(tpm_tsc_physical_presence(PRESENCE));
-	TPM_CHECK(tpm_force_clear());
+	TPM_CHECK(tpm_force_clear(0, NULL, 0));
 	TPM_CHECK(tpm_physical_enable());
 	TPM_CHECK(tpm_physical_set_deactivated(0));
 
@@ -477,7 +477,7 @@ static int test_write_limit(void)
 	}
 
 	/* Reset write count */
-	TPM_CHECK(tpm_force_clear());
+	TPM_CHECK(tpm_force_clear(0, NULL, 0));
 	TPM_CHECK(tpm_physical_enable());
 	TPM_CHECK(tpm_physical_set_deactivated(0));
 
diff --git a/include/tpm.h b/include/tpm.h
index 38d7cb899d..2f17166662 100644
--- a/include/tpm.h
+++ b/include/tpm.h
@@ -43,13 +43,23 @@ enum tpm_startup_type {
 };
 
 enum tpm2_startup_types {
-	TPM2_SU_CLEAR	= 0x0000,
-	TPM2_SU_STATE	= 0x0001,
+	TPM2_SU_CLEAR		= 0x0000,
+	TPM2_SU_STATE		= 0x0001,
+};
+
+enum tpm2_handles {
+	TPM2_RH_OWNER		= 0x40000001,
+	TPM2_RS_PW		= 0x40000009,
+	TPM2_RH_LOCKOUT		= 0x4000000A,
+	TPM2_RH_ENDORSEMENT	= 0x4000000B,
+	TPM2_RH_PLATFORM	= 0x4000000C,
 };
 
 enum tpm2_command_codes {
 	TPM2_CC_STARTUP		= 0x0144,
 	TPM2_CC_SELF_TEST	= 0x0143,
+	TPM2_CC_CLEAR		= 0x0126,
+	TPM2_CC_CLEARCONTROL	= 0x0127,
 	TPM2_CC_GET_CAPABILITY	= 0x017A,
 	TPM2_CC_PCR_READ	= 0x017E,
 	TPM2_CC_PCR_EXTEND	= 0x0182,
@@ -567,11 +577,14 @@ uint32_t tpm_tsc_physical_presence(uint16_t presence);
 uint32_t tpm_read_pubek(void *data, size_t count);
 
 /**
- * Issue a TPM_ForceClear command.
+ * Issue a TPM_ForceClear or a TPM2_Clear command.
  *
+ * @param handle	Handle
+ * @param pw		Password
+ * @param pw_sz		Length of the password
  * @return return code of the operation
  */
-uint32_t tpm_force_clear(void);
+int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz);
 
 /**
  * Issue a TPM_PhysicalEnable command.
diff --git a/lib/tpm.c b/lib/tpm.c
index 3cc2888267..9a46ac09e6 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -608,13 +608,47 @@ uint32_t tpm_read_pubek(void *data, size_t count)
 	return 0;
 }
 
-uint32_t tpm_force_clear(void)
+int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz)
 {
-	const uint8_t command[10] = {
-		0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x5d,
+	const u8 command_v1[10] = {
+		U16_TO_ARRAY(0xc1),
+		U32_TO_ARRAY(10),
+		U32_TO_ARRAY(0x5d),
 	};
+	u8 command_v2[COMMAND_BUFFER_SIZE] = {
+		U16_TO_ARRAY(TPM2_ST_SESSIONS),	/* TAG */
+		U32_TO_ARRAY(27 + pw_sz),	/* Length */
+		U32_TO_ARRAY(TPM2_CC_CLEAR),	/* Command code */
 
-	return tpm_sendrecv_command(command, NULL, NULL);
+		/* HANDLE */
+		U32_TO_ARRAY(handle),		/* TPM resource handle */
+
+		/* AUTH_SESSION */
+		U32_TO_ARRAY(9 + pw_sz),	/* Authorization size */
+		U32_TO_ARRAY(TPM2_RS_PW),	/* Session handle */
+		U16_TO_ARRAY(0),		/* Size of <nonce> */
+						/* <nonce> (if any) */
+		0,				/* Attributes: Cont/Excl/Rst */
+		U16_TO_ARRAY(pw_sz),		/* Size of <hmac/password> */
+		/* STRING(pw)			   <hmac/password> (if any) */
+	};
+	unsigned int offset = 27;
+	int ret;
+
+	if (!is_tpmv2)
+		tpm_sendrecv_command(command_v1, NULL, NULL);
+
+	/*
+	 * Fill the command structure starting from the first buffer:
+	 *     - the password (if any)
+	 */
+	ret = pack_byte_string(command_v2, sizeof(command_v2), "s",
+			       offset, pw, pw_sz);
+	offset += pw_sz;
+	if (ret)
+		return TPM_LIB_ERROR;
+
+	return tpm_sendrecv_command(command_v2, NULL, NULL);
 }
 
 uint32_t tpm_physical_enable(void)
-- 
2.14.1

[U-Boot] [PATCH v2 12/19] tpm: rename the _extend() function to be _pcr_event()

[Miquel Raynal]

The function currently called _extend() actually does what the
specification defines as a _pcr_event(). Rename the function
accordingly before implementing the actual _extend() command.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/tpm.c      | 18 ++++++++++--------
 cmd/tpm_test.c |  4 ++--
 include/tpm.h  |  4 ++--
 lib/tpm.c      |  2 +-
 4 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index 32921e1a70..93dcd1a65c 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -324,8 +324,8 @@ static int do_tpm_nv_write_value(cmd_tbl_t *cmdtp, int flag,
 	return report_return_code(rc);
 }
 
-static int do_tpm_extend(cmd_tbl_t *cmdtp, int flag,
-		int argc, char * const argv[])
+static int do_tpm_pcr_event(cmd_tbl_t *cmdtp, int flag,
+			    int argc, char * const argv[])
 {
 	uint8_t in_digest[TPM1_DIGEST_LENGTH];
 	uint8_t out_digest[TPM1_DIGEST_LENGTH];
@@ -333,13 +333,14 @@ static int do_tpm_extend(cmd_tbl_t *cmdtp, int flag,
 
 	if (argc != 3)
 		return CMD_RET_USAGE;
+
 	index = simple_strtoul(argv[1], NULL, 0);
 	if (!parse_byte_string(argv[2], in_digest, NULL)) {
 		printf("Couldn't parse byte string %s\n", argv[2]);
 		return CMD_RET_FAILURE;
 	}
 
-	rc = tpm_extend(index, in_digest, out_digest);
+	rc = tpm_pcr_event(index, in_digest, out_digest);
 	if (!rc) {
 		puts("PCR value after execution of the command:\n");
 		print_byte_string(out_digest, TPM1_DIGEST_LENGTH);
@@ -887,8 +888,8 @@ static cmd_tbl_t tpm_commands[] = {
 			 do_tpm_nv_read_value, "", ""),
 	U_BOOT_CMD_MKENT(nv_write_value, 0, 1,
 			 do_tpm_nv_write_value, "", ""),
-	U_BOOT_CMD_MKENT(extend, 0, 1,
-			 do_tpm_extend, "", ""),
+	U_BOOT_CMD_MKENT(pcr_event, 0, 1,
+			 do_tpm_pcr_event, "", ""),
 	U_BOOT_CMD_MKENT(pcr_read, 0, 1,
 			 do_tpm_pcr_read, "", ""),
 	U_BOOT_CMD_MKENT(tsc_physical_presence, 0, 1,
@@ -1019,9 +1020,10 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
 "    - Read <count> bytes of the public endorsement key to memory\n"
 "      address <addr>\n"
 "Integrity Collection and Reporting Commands:\n"
-"  extend index digest_hex_string\n"
-"    - Add a new measurement to a PCR.  Update PCR <index> with the 20-bytes\n"
-"      <digest_hex_string>\n"
+"  pcr_event <index> <digest_in> <digest_out>\n"
+"    - Add a new measurement to a PCR.  Update PCR <index> with\n"
+"      <digest_in>. It must be a 20 byte digest for TPMv1 or a SHA256\n"
+"      digest of 32 bytes for TPMv2. Value of the PCR is given at <digest_out>\n"
 "  pcr_read index addr count\n"
 "    - Read <count> bytes from PCR <index> to memory address <addr>.\n"
 #ifdef CONFIG_TPM_AUTH_SESSIONS
diff --git a/cmd/tpm_test.c b/cmd/tpm_test.c
index da40dbc423..0bbbdab4ee 100644
--- a/cmd/tpm_test.c
+++ b/cmd/tpm_test.c
@@ -104,7 +104,7 @@ static int test_early_extend(void)
 	tpm_init();
 	TPM_CHECK(tpm_startup(TPM_ST_CLEAR));
 	TPM_CHECK(tpm_continue_self_test());
-	TPM_CHECK(tpm_extend(1, value_in, value_out));
+	TPM_CHECK(tpm_pcr_event(1, value_in, value_out));
 	printf("done\n");
 	return 0;
 }
@@ -439,7 +439,7 @@ static int test_timing(void)
 	TTPM_CHECK(tpm_tsc_physical_presence(PRESENCE), 100);
 	TTPM_CHECK(tpm_nv_write_value(INDEX0, (uint8_t *)&x, sizeof(x)), 100);
 	TTPM_CHECK(tpm_nv_read_value(INDEX0, (uint8_t *)&x, sizeof(x)), 100);
-	TTPM_CHECK(tpm_extend(0, in, out), 200);
+	TTPM_CHECK(tpm_pcr_event(0, in, out), 200);
 	TTPM_CHECK(tpm_set_global_lock(), 50);
 	TTPM_CHECK(tpm_tsc_physical_presence(PHYS_PRESENCE), 100);
 	printf("done\n");
diff --git a/include/tpm.h b/include/tpm.h
index 2f17166662..a863ac6196 100644
--- a/include/tpm.h
+++ b/include/tpm.h
@@ -537,7 +537,7 @@ uint32_t tpm_nv_read_value(uint32_t index, void *data, uint32_t count);
 uint32_t tpm_nv_write_value(uint32_t index, const void *data, uint32_t length);
 
 /**
- * Issue a TPM_Extend command.
+ * Issue a TPM_PCR_Event command.
  *
  * @param index		index of the PCR
  * @param in_digest	160-bit value representing the event to be
@@ -546,7 +546,7 @@ uint32_t tpm_nv_write_value(uint32_t index, const void *data, uint32_t length);
  *			command
  * @return return code of the operation
  */
-uint32_t tpm_extend(uint32_t index, const void *in_digest, void *out_digest);
+int tpm_pcr_event(u32 index, const void *in_digest, void *out_digest);
 
 /**
  * Issue a TPM_PCRRead command.
diff --git a/lib/tpm.c b/lib/tpm.c
index 9a46ac09e6..46250a86cf 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -493,7 +493,7 @@ uint32_t tpm_nv_write_value(uint32_t index, const void *data, uint32_t length)
 	return 0;
 }
 
-uint32_t tpm_extend(uint32_t index, const void *in_digest, void *out_digest)
+int tpm_pcr_event(u32 index, const void *in_digest, void *out_digest)
 {
 	const uint8_t command[34] = {
 		0x0, 0xc1, 0x0, 0x0, 0x0, 0x22, 0x0, 0x0, 0x0, 0x14,
-- 
2.14.1

[U-Boot] [PATCH v2 13/19] tpm: add TPM2_PCR_Extend command support

[Miquel Raynal]

Add support for the TPM2_PCR_Extend command.

Change the command file and the help accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/tpm.c     | 18 ++++++++++++++++++
 include/tpm.h | 18 ++++++++++++++++++
 lib/tpm.c     | 41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index 93dcd1a65c..3f284f0adf 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -349,6 +349,18 @@ static int do_tpm_pcr_event(cmd_tbl_t *cmdtp, int flag,
 	return report_return_code(rc);
 }
 
+static int do_tpm_pcr_extend(cmd_tbl_t *cmdtp, int flag,
+			     int argc, char * const argv[])
+{
+	u32 index = simple_strtoul(argv[1], NULL, 0);
+	void *digest = (void *)simple_strtoul(argv[2], NULL, 0);
+
+	if (argc != 3)
+		return CMD_RET_USAGE;
+
+	return report_return_code(tpm2_pcr_extend(index, digest));
+}
+
 static int do_tpm_pcr_read(cmd_tbl_t *cmdtp, int flag,
 		int argc, char * const argv[])
 {
@@ -890,6 +902,8 @@ static cmd_tbl_t tpm_commands[] = {
 			 do_tpm_nv_write_value, "", ""),
 	U_BOOT_CMD_MKENT(pcr_event, 0, 1,
 			 do_tpm_pcr_event, "", ""),
+	U_BOOT_CMD_MKENT(pcr_extend, 0, 1,
+			 do_tpm_pcr_extend, "", ""),
 	U_BOOT_CMD_MKENT(pcr_read, 0, 1,
 			 do_tpm_pcr_read, "", ""),
 	U_BOOT_CMD_MKENT(tsc_physical_presence, 0, 1,
@@ -1026,6 +1040,10 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
 "      digest of 32 bytes for TPMv2. Value of the PCR is given at <digest_out>\n"
 "  pcr_read index addr count\n"
 "    - Read <count> bytes from PCR <index> to memory address <addr>.\n"
+"  pcr_extend index <digest_in>\n"
+"    - Add a new measurement to a PCR.  Update PCR <index> with\n"
+"      <digest_in>. It must be a 20 byte digest for TPMv1 or a SHA256\n"
+"      digest of 32 bytes for TPMv2.\n"
 #ifdef CONFIG_TPM_AUTH_SESSIONS
 "Authorization Sessions\n"
 "  oiap\n"
diff --git a/include/tpm.h b/include/tpm.h
index a863ac6196..b88ad4b2f4 100644
--- a/include/tpm.h
+++ b/include/tpm.h
@@ -76,6 +76,14 @@ enum tpm2_return_codes {
 	TPM2_RC_LOCKOUT		= 0x0921,
 };
 
+enum tpm_algorithms {
+	TPM2_ALG_XOR		= 0x0A,
+	TPM2_ALG_SHA256		= 0x0B,
+	TPM2_ALG_SHA384		= 0x0C,
+	TPM2_ALG_SHA512		= 0x0D,
+	TPM2_ALG_NULL		= 0x10,
+};
+
 enum tpm_physical_presence {
 	TPM_PHYSICAL_PRESENCE_HW_DISABLE	= 0x0200,
 	TPM_PHYSICAL_PRESENCE_CMD_DISABLE	= 0x0100,
@@ -548,6 +556,16 @@ uint32_t tpm_nv_write_value(uint32_t index, const void *data, uint32_t length);
  */
 int tpm_pcr_event(u32 index, const void *in_digest, void *out_digest);
 
+/**
+ * Issue a TPM_PCR_Extend command.
+ *
+ * @param index		Index of the PCR
+ * @param digest	Value representing the event to be recorded
+ *
+ * @return return code of the operation
+ */
+int tpm2_pcr_extend(u32 index, const uint8_t *digest);
+
 /**
  * Issue a TPM_PCRRead command.
  *
diff --git a/lib/tpm.c b/lib/tpm.c
index 46250a86cf..0cde8695b9 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -527,6 +527,47 @@ int tpm_pcr_event(u32 index, const void *in_digest, void *out_digest)
 	return 0;
 }
 
+int tpm2_pcr_extend(u32 index, const uint8_t *digest)
+{
+	u8 command_v2[COMMAND_BUFFER_SIZE] = {
+		U16_TO_ARRAY(TPM2_ST_SESSIONS),	/* TAG */
+		U32_TO_ARRAY(33 + TPM2_DIGEST_LENGTH), /* Length */
+		U32_TO_ARRAY(TPM2_CC_PCR_EXTEND), /* Command code */
+
+		/* HANDLE */
+		U32_TO_ARRAY(index),		/* Handle (PCR Index) */
+
+		/* AUTH_SESSION */
+		U32_TO_ARRAY(9),		/* Authorization size */
+		U32_TO_ARRAY(TPM2_RS_PW),	/* Session handle */
+		U16_TO_ARRAY(0),		/* Size of <nonce> */
+						/* <nonce> (if any) */
+		0,				/* Attributes: Cont/Excl/Rst */
+		U16_TO_ARRAY(0),		/* Size of <hmac/password> */
+						/* <hmac/password> (if any) */
+		U32_TO_ARRAY(1),		/* Count (number of hashes) */
+		U16_TO_ARRAY(TPM2_ALG_SHA256),	/* Algorithm of the hash */
+		/* STRING(digest)		   Digest */
+	};
+	unsigned int offset = 33;
+	int ret;
+
+	if (!is_tpmv2)
+		return TPM_LIB_ERROR;
+
+	/*
+	 * Fill the command structure starting from the first buffer:
+	 *     - the digest
+	 */
+	ret = pack_byte_string(command_v2, sizeof(command_v2), "s",
+			       offset, digest, TPM2_DIGEST_LENGTH);
+	offset += TPM2_DIGEST_LENGTH;
+	if (ret)
+		return TPM_LIB_ERROR;
+
+	return tpm_sendrecv_command(command_v2,	NULL, NULL);
+}
+
 uint32_t tpm_pcr_read(uint32_t index, void *data, size_t count)
 {
 	const uint8_t command[14] = {
-- 
2.14.1

[U-Boot] [PATCH v2 14/19] tpm: add TPM2_PCR_Read command support

[Miquel Raynal]

Add support for the TPM2_PCR_Read command.

Change the command file and the help accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/tpm.c     | 23 ++++++++++++++---------
 include/tpm.h |  4 ++--
 lib/tpm.c     | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 71 insertions(+), 12 deletions(-)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index 3f284f0adf..6ee72b3032 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -362,21 +362,25 @@ static int do_tpm_pcr_extend(cmd_tbl_t *cmdtp, int flag,
 }
 
 static int do_tpm_pcr_read(cmd_tbl_t *cmdtp, int flag,
-		int argc, char * const argv[])
+			   int argc, char * const argv[])
 {
-	uint32_t index, count, rc;
+	bool is_tpmv2 = (tpm_get_specification() == 2);
+	u32 index, rc;
+	unsigned int updates;
 	void *data;
 
-	if (argc != 4)
+	if (argc != 3)
 		return CMD_RET_USAGE;
+
 	index = simple_strtoul(argv[1], NULL, 0);
 	data = (void *)simple_strtoul(argv[2], NULL, 0);
-	count = simple_strtoul(argv[3], NULL, 0);
 
-	rc = tpm_pcr_read(index, data, count);
+	rc = tpm_pcr_read(index, data, &updates);
 	if (!rc) {
-		puts("Named PCR content:\n");
-		print_byte_string(data, count);
+		printf("Named PCR %u content (known updates: %d):\n", index,
+		       is_tpmv2 ? updates : -1);
+		print_byte_string(data, !is_tpmv2 ? TPM1_DIGEST_LENGTH :
+						    TPM2_DIGEST_LENGTH);
 	}
 
 	return report_return_code(rc);
@@ -1038,12 +1042,13 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
 "    - Add a new measurement to a PCR.  Update PCR <index> with\n"
 "      <digest_in>. It must be a 20 byte digest for TPMv1 or a SHA256\n"
 "      digest of 32 bytes for TPMv2. Value of the PCR is given at <digest_out>\n"
-"  pcr_read index addr count\n"
-"    - Read <count> bytes from PCR <index> to memory address <addr>.\n"
 "  pcr_extend index <digest_in>\n"
 "    - Add a new measurement to a PCR.  Update PCR <index> with\n"
 "      <digest_in>. It must be a 20 byte digest for TPMv1 or a SHA256\n"
 "      digest of 32 bytes for TPMv2.\n"
+"  pcr_read <index> <digest_in>\n"
+"    - Read PCR <index> to memory address <digest_in> (20B with TPMv1, 32B with\n"
+"       TPMv2).\n"
 #ifdef CONFIG_TPM_AUTH_SESSIONS
 "Authorization Sessions\n"
 "  oiap\n"
diff --git a/include/tpm.h b/include/tpm.h
index b88ad4b2f4..2df2ea3c5b 100644
--- a/include/tpm.h
+++ b/include/tpm.h
@@ -571,10 +571,10 @@ int tpm2_pcr_extend(u32 index, const uint8_t *digest);
  *
  * @param index		index of the PCR
  * @param data		output buffer for contents of the named PCR
- * @param count		size of output buffer
+ * @param updates	optional out parameter: number of updates for this PCR
  * @return return code of the operation
  */
-uint32_t tpm_pcr_read(uint32_t index, void *data, size_t count);
+int tpm_pcr_read(u32 index, void *data, unsigned int *updates);
 
 /**
  * Issue a TSC_PhysicalPresence command.  TPM physical presence flag
diff --git a/lib/tpm.c b/lib/tpm.c
index 0cde8695b9..589f9c1004 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -568,7 +568,7 @@ int tpm2_pcr_extend(u32 index, const uint8_t *digest)
 	return tpm_sendrecv_command(command_v2,	NULL, NULL);
 }
 
-uint32_t tpm_pcr_read(uint32_t index, void *data, size_t count)
+int tpm1_pcr_read(u32 index, void *data)
 {
 	const uint8_t command[14] = {
 		0x0, 0xc1, 0x0, 0x0, 0x0, 0xe, 0x0, 0x0, 0x0, 0x15,
@@ -596,6 +596,60 @@ uint32_t tpm_pcr_read(uint32_t index, void *data, size_t count)
 	return 0;
 }
 
+int tpm2_pcr_read(u32 index, void *data, unsigned int *updates)
+{
+	u8 command_v2[COMMAND_BUFFER_SIZE] = {
+		U16_TO_ARRAY(TPM2_ST_NO_SESSIONS), /* TAG */
+		U32_TO_ARRAY(20),		/* Length */
+		U32_TO_ARRAY(TPM2_CC_PCR_READ),	/* Command code */
+
+		/* TPML_PCR_SELECTION */
+		U32_TO_ARRAY(1),		/* Number of selections */
+		U16_TO_ARRAY(TPM2_ALG_SHA256),	/* Algorithm of the hash */
+		3,				/* Array size for selection */
+		/* U32_TO_ARRAY(bitmap(index) << 8) Selected PCR bitmap */
+	};
+	size_t response_len = COMMAND_BUFFER_SIZE;
+	u8 response[COMMAND_BUFFER_SIZE];
+	unsigned int counter = 0;
+	u8 pcr_sel[3] = {};
+	int ret;
+
+	if (!is_tpmv2)
+		return TPM_LIB_ERROR;
+
+	if (index >= 24)
+		return TPM_LIB_ERROR;
+
+	pcr_sel[index / 8] = BIT(index % 8);
+	if (pack_byte_string(command_v2, COMMAND_BUFFER_SIZE, "bbb",
+			     17, pcr_sel[0], 18, pcr_sel[1], 19, pcr_sel[2]))
+		return TPM_LIB_ERROR;
+
+	ret = tpm_sendrecv_command(command_v2, response, &response_len);
+	if (ret)
+		return ret;
+
+	if (unpack_byte_string(response, response_len, "ds",
+			       10, &counter,
+			       response_len - TPM2_DIGEST_LENGTH, data,
+			       TPM2_DIGEST_LENGTH))
+		return TPM_LIB_ERROR;
+
+	if (updates)
+		*updates = counter;
+
+	return 0;
+}
+
+int tpm_pcr_read(u32 index, void *data, unsigned int *updates)
+{
+	if (!is_tpmv2)
+		return tpm1_pcr_read(index, data);
+	else
+		return tpm2_pcr_read(index, data, updates);
+}
+
 uint32_t tpm_tsc_physical_presence(uint16_t presence)
 {
 	const uint8_t command[12] = {
-- 
2.14.1

[U-Boot] [PATCH v2 15/19] tpm: add TPM2_GetCapability command support

[Miquel Raynal]

Add support for the TPM2_GetCapability command.

Change the command file and the help accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/tpm.c     | 31 ++++++++++++++++++++-----------
 include/tpm.h | 14 +++++++-------
 lib/tpm.c     | 39 +++++++++++++++++++++++++++++++++++++--
 3 files changed, 64 insertions(+), 20 deletions(-)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index 6ee72b3032..eab914ce5f 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -433,21 +433,30 @@ static int do_tpm_physical_set_deactivated(cmd_tbl_t *cmdtp, int flag,
 static int do_tpm_get_capability(cmd_tbl_t *cmdtp, int flag,
 		int argc, char * const argv[])
 {
-	uint32_t cap_area, sub_cap, rc;
-	void *cap;
+	u32 capability, property, rc;
+	u8 *data;
 	size_t count;
+	int i, j;
 
 	if (argc != 5)
 		return CMD_RET_USAGE;
-	cap_area = simple_strtoul(argv[1], NULL, 0);
-	sub_cap = simple_strtoul(argv[2], NULL, 0);
-	cap = (void *)simple_strtoul(argv[3], NULL, 0);
+	capability = simple_strtoul(argv[1], NULL, 0);
+	property = simple_strtoul(argv[2], NULL, 0);
+	data = (void *)simple_strtoul(argv[3], NULL, 0);
 	count = simple_strtoul(argv[4], NULL, 0);
 
-	rc = tpm_get_capability(cap_area, sub_cap, cap, count);
+	rc = tpm_get_capability(capability, property, data, count);
 	if (!rc) {
-		puts("capability information:\n");
-		print_byte_string(cap, count);
+		printf("Capabilities read from TPM:\n");
+		for (i = 0; i < count; i++) {
+			printf("Property 0x");
+			for (j = 0; j < 4; j++)
+				printf("%02x", data[(i * 8) + j]);
+			printf(": 0x");
+			for (j = 4; j < 8; j++)
+				printf("%02x", data[(i * 8) + j]);
+			printf("\n");
+		}
 	}
 
 	return report_return_code(rc);
@@ -998,9 +1007,9 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
 "  tsc_physical_presence flags\n"
 "    - Set TPM device's Physical Presence flags to <flags>.\n"
 "The Capability Commands:\n"
-"  get_capability cap_area sub_cap addr count\n"
-"    - Read <count> bytes of TPM capability indexed by <cap_area> and\n"
-"      <sub_cap> to memory address <addr>.\n"
+"  get_capability <cap_area|capability> <sub_cap|property> <addr> <count>\n"
+"    - Read <count> bytes of TPM capability indexed by <cap_area|capability>\n"
+"      and <sub_cap|property> to memory address <addr>.\n"
 #if defined(CONFIG_TPM_FLUSH_RESOURCES) || defined(CONFIG_TPM_LIST_RESOURCES)
 "Resource management functions\n"
 #endif
diff --git a/include/tpm.h b/include/tpm.h
index 2df2ea3c5b..369119fc1b 100644
--- a/include/tpm.h
+++ b/include/tpm.h
@@ -628,17 +628,17 @@ uint32_t tpm_physical_set_deactivated(uint8_t state);
 
 /**
  * Issue a TPM_GetCapability command.  This implementation is limited
- * to query sub_cap index that is 4-byte wide.
+ * to query property index that is 4-byte wide.
  *
- * @param cap_area	partition of capabilities
- * @param sub_cap	further definition of capability, which is
+ * @param capability	partition of capabilities
+ * @param property	further definition of capability, which is
  *			limited to be 4-byte wide
- * @param cap		output buffer for capability information
- * @param count		size of ouput buffer
+ * @param buf		output buffer for capability information
+ * @param propertycount size of output buffer
  * @return return code of the operation
  */
-uint32_t tpm_get_capability(uint32_t cap_area, uint32_t sub_cap,
-		void *cap, size_t count);
+int tpm_get_capability(u32 capability, u32 property, void *buf,
+		       size_t property_count);
 
 /**
  * Issue a TPM_FlushSpecific command for a AUTH ressource.
diff --git a/lib/tpm.c b/lib/tpm.c
index 589f9c1004..f15611ee92 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -789,8 +789,7 @@ uint32_t tpm_physical_set_deactivated(uint8_t state)
 	return tpm_sendrecv_command(buf, NULL, NULL);
 }
 
-uint32_t tpm_get_capability(uint32_t cap_area, uint32_t sub_cap,
-		void *cap, size_t count)
+int tpm1_get_capability(u32 cap_area, u32 sub_cap, void *cap, size_t count)
 {
 	const uint8_t command[22] = {
 		0x0, 0xc1,		/* TPM_TAG */
@@ -829,6 +828,42 @@ uint32_t tpm_get_capability(uint32_t cap_area, uint32_t sub_cap,
 	return 0;
 }
 
+int tpm2_get_capability(u32 capability, u32 property, void *buf,
+			size_t property_count)
+{
+	u8 command_v2[COMMAND_BUFFER_SIZE] = {
+		U16_TO_ARRAY(TPM2_ST_NO_SESSIONS),	/* TAG */
+		U32_TO_ARRAY(22),			/* Length */
+		U32_TO_ARRAY(TPM2_CC_GET_CAPABILITY),	/* Command code */
+
+		U32_TO_ARRAY(capability),		/* Capability */
+		U32_TO_ARRAY(property),			/* Property */
+		U32_TO_ARRAY(property_count),		/* Property count */
+	};
+	u8 response[COMMAND_BUFFER_SIZE];
+	size_t response_len = COMMAND_BUFFER_SIZE;
+	int ret;
+
+	ret = tpm_sendrecv_command(command_v2, response, &response_len);
+	if (ret)
+		return ret;
+
+	memcpy(buf, &response[19], response_len - 19);
+
+	return 0;
+}
+
+int tpm_get_capability(u32 capability, u32 property, void *buf,
+		       size_t property_count)
+{
+	if (!is_tpmv2)
+		return tpm1_get_capability(capability, property, buf,
+					   property_count);
+	else
+		return tpm2_get_capability(capability, property, buf,
+					   property_count);
+}
+
 uint32_t tpm_get_permanent_flags(struct tpm_permanent_flags *pflags)
 {
 	const uint8_t command[22] = {
-- 
2.14.1

[U-Boot] [PATCH v2 16/19] tpm: add dictionary attack mitigation commands support

[Miquel Raynal]

Add support for the TPM2_DictionaryAttackParameters and
TPM2_DictionaryAttackLockReset commands.

Change the command file and the help accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/tpm.c     | 69 +++++++++++++++++++++++++++++++++++++++++++++
 include/tpm.h | 26 +++++++++++++++++
 lib/tpm.c     | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 184 insertions(+)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index eab914ce5f..3648d05581 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -480,6 +480,58 @@ static int do_tpm_init(cmd_tbl_t *cmdtp, int flag,
 	return report_return_code(tpm_init());
 }
 
+static int do_tpm_dam_reset_counter(cmd_tbl_t *cmdtp, int flag,
+				    int argc, char *const argv[])
+{
+	const char *pw = (argc < 2) ? NULL : argv[1];
+	const ssize_t pw_sz = pw ? strlen(pw) : 0;
+
+	if (argc > 2)
+		return CMD_RET_USAGE;
+
+	if (pw_sz > TPM2_DIGEST_LENGTH)
+		return -EINVAL;
+
+	return report_return_code(tpm2_dam_reset_counter(pw, pw_sz));
+}
+
+static int do_tpm_dam_set_parameters(cmd_tbl_t *cmdtp, int flag,
+				     int argc, char *const argv[])
+{
+	const char *pw = (argc < 5) ? NULL : argv[4];
+	const ssize_t pw_sz = pw ? strlen(pw) : 0;
+	/*
+	 * No Dictionary Attack Mitigation (DAM) means:
+	 * maxtries = 0xFFFFFFFF, recovery_time = 1, lockout_recovery = 0
+	 */
+	unsigned long int max_tries;
+	unsigned long int recovery_time;
+	unsigned long int lockout_recovery;
+
+	if (argc < 4 || argc > 5)
+		return CMD_RET_USAGE;
+
+	if (pw_sz > TPM2_DIGEST_LENGTH)
+		return -EINVAL;
+
+	if (strict_strtoul(argv[1], 0, &max_tries))
+		return CMD_RET_USAGE;
+
+	if (strict_strtoul(argv[2], 0, &recovery_time))
+		return CMD_RET_USAGE;
+
+	if (strict_strtoul(argv[3], 0, &lockout_recovery))
+		return CMD_RET_USAGE;
+
+	debug("Changing dictionary attack parameters:\n");
+	debug("- maxTries: %lu\n- recoveryTime: %lu\n- lockoutRecovery: %lu\n",
+	      max_tries, recovery_time, lockout_recovery);
+
+	return report_return_code(tpm2_dam_set_parameters(pw, pw_sz, max_tries,
+							 recovery_time,
+							 lockout_recovery));
+}
+
 static int do_tpm_force_clear(cmd_tbl_t *cmdtp, int flag,
 			      int argc, char * const argv[])
 {
@@ -901,6 +953,10 @@ static cmd_tbl_t tpm_commands[] = {
 			 do_tpm_self_test_full, "", ""),
 	U_BOOT_CMD_MKENT(continue_self_test, 0, 1,
 			 do_tpm_continue_self_test, "", ""),
+	U_BOOT_CMD_MKENT(dam_reset_counter, 0, 1,
+			 do_tpm_dam_reset_counter, "", ""),
+	U_BOOT_CMD_MKENT(dam_set_parameters, 0, 1,
+			 do_tpm_dam_set_parameters, "", ""),
 	U_BOOT_CMD_MKENT(force_clear, 0, 1,
 			 do_tpm_force_clear, "", ""),
 	U_BOOT_CMD_MKENT(physical_enable, 0, 1,
@@ -1010,6 +1066,19 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
 "  get_capability <cap_area|capability> <sub_cap|property> <addr> <count>\n"
 "    - Read <count> bytes of TPM capability indexed by <cap_area|capability>\n"
 "      and <sub_cap|property> to memory address <addr>.\n"
+"Lockout/Dictionary attack Commands:\n"
+"  dam_reset_counter [<password>]\n"
+"    - If the TPM is not in a LOCKOUT state, reset the internal error\n"
+"      counter (TPMv2 only)\n"
+"  dam_set_parameters <maxTries> <recoveryTime> <lockoutRecovery> [<password>]\n"
+"    - If the TPM is not in a LOCKOUT state, set the dictionary attack\n"
+"      parameters:\n"
+"          * maxTries: maximum number of failures before lockout.\n"
+"                          0 means always locking.\n"
+"          * recoveryTime: time before decrementation of the error counter,\n"
+"                          0 means no lockout.\n"
+"          * lockoutRecovery: time of a lockout (before the next try)\n"
+"                          0 means a reboot is needed.\n"
 #if defined(CONFIG_TPM_FLUSH_RESOURCES) || defined(CONFIG_TPM_LIST_RESOURCES)
 "Resource management functions\n"
 #endif
diff --git a/include/tpm.h b/include/tpm.h
index 369119fc1b..4d062584f9 100644
--- a/include/tpm.h
+++ b/include/tpm.h
@@ -60,6 +60,8 @@ enum tpm2_command_codes {
 	TPM2_CC_SELF_TEST	= 0x0143,
 	TPM2_CC_CLEAR		= 0x0126,
 	TPM2_CC_CLEARCONTROL	= 0x0127,
+	TPM2_CC_DAM_RESET	= 0x0139,
+	TPM2_CC_DAM_PARAMETERS	= 0x013A,
 	TPM2_CC_GET_CAPABILITY	= 0x017A,
 	TPM2_CC_PCR_READ	= 0x017E,
 	TPM2_CC_PCR_EXTEND	= 0x0182,
@@ -594,6 +596,30 @@ uint32_t tpm_tsc_physical_presence(uint16_t presence);
  */
 uint32_t tpm_read_pubek(void *data, size_t count);
 
+/**
+ * Issue a TPM2_DictionaryAttackLockReset command.
+ *
+ * @param pw		Password
+ * @param pw_sz		Length of the password
+ * @return return code of the operation
+ */
+int tpm2_dam_reset_counter(const char *pw, const ssize_t pw_sz);
+
+/**
+ * Issue a TPM2_DictionaryAttackLockParameters command.
+ *
+ * @param pw		Password
+ * @param pw_sz		Length of the password
+ * @param max_tries	Count of authorizations before lockout
+ * @param recovery_time Time before decrementation of the failure count
+ * @param lockout_recovery Time to wait after a lockout
+ * @return return code of the operation
+ */
+int tpm2_dam_set_parameters(const char *pw, const ssize_t pw_sz,
+			    unsigned int max_tries,
+			    unsigned int recovery_time,
+			    unsigned int lockout_recovery);
+
 /**
  * Issue a TPM_ForceClear or a TPM2_Clear command.
  *
diff --git a/lib/tpm.c b/lib/tpm.c
index f15611ee92..4987a4ce8c 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -703,6 +703,95 @@ uint32_t tpm_read_pubek(void *data, size_t count)
 	return 0;
 }
 
+int tpm2_dam_reset_counter(const char *pw, const ssize_t pw_sz)
+{
+	u8 command_v2[COMMAND_BUFFER_SIZE] = {
+		U16_TO_ARRAY(TPM2_ST_SESSIONS),	/* TAG */
+		U32_TO_ARRAY(27 + pw_sz),	/* Length */
+		U32_TO_ARRAY(TPM2_CC_DAM_RESET), /* Command code */
+
+		/* HANDLE */
+		U32_TO_ARRAY(TPM2_RH_LOCKOUT),	/* TPM resource handle */
+
+		/* AUTH_SESSION */
+		U32_TO_ARRAY(9 + pw_sz),	/* Authorization size */
+		U32_TO_ARRAY(TPM2_RS_PW),	/* Session handle */
+		U16_TO_ARRAY(0),		/* Size of <nonce> */
+						/* <nonce> (if any) */
+		0,				/* Attributes: Cont/Excl/Rst */
+		U16_TO_ARRAY(pw_sz),		/* Size of <hmac/password> */
+		/* STRING(pw)			   <hmac/password> (if any) */
+	};
+	unsigned int offset = 27;
+	int ret;
+
+	if (!is_tpmv2)
+		return TPM_LIB_ERROR;
+
+	/*
+	 * Fill the command structure starting from the first buffer:
+	 *     - the password (if any)
+	 */
+	ret = pack_byte_string(command_v2, sizeof(command_v2), "s",
+			       offset, pw, pw_sz);
+	offset += pw_sz;
+	if (ret)
+		return TPM_LIB_ERROR;
+
+	return tpm_sendrecv_command(command_v2, NULL, NULL);
+}
+
+int tpm2_dam_set_parameters(const char *pw, const ssize_t pw_sz,
+			    unsigned int max_tries, unsigned int recovery_time,
+			    unsigned int lockout_recovery)
+{
+	u8 command_v2[COMMAND_BUFFER_SIZE] = {
+		U16_TO_ARRAY(TPM2_ST_SESSIONS),	/* TAG */
+		U32_TO_ARRAY(27 + pw_sz + 12),	/* Length */
+		U32_TO_ARRAY(TPM2_CC_DAM_PARAMETERS), /* Command code */
+
+		/* HANDLE */
+		U32_TO_ARRAY(TPM2_RH_LOCKOUT),	/* TPM resource handle */
+
+		/* AUTH_SESSION */
+		U32_TO_ARRAY(9 + pw_sz),	/* Authorization size */
+		U32_TO_ARRAY(TPM2_RS_PW),	/* Session handle */
+		U16_TO_ARRAY(0),		/* Size of <nonce> */
+						/* <nonce> (if any) */
+		0,				/* Attributes: Cont/Excl/Rst */
+		U16_TO_ARRAY(pw_sz),		/* Size of <hmac/password> */
+		/* STRING(pw)			   <hmac/password> (if any) */
+
+		/* LOCKOUT PARAMETERS */
+		/* STRIGIFY32(max_tries)	   Max tries (0, always lock) */
+		/* STRIGIFY32(recovery_time)	   Recovery time (0, no lock) */
+		/* STRIGIFY32(lockout_recovery)	   Lockout recovery */
+	};
+	unsigned int offset = 27;
+	int ret;
+
+	if (!is_tpmv2)
+		return TPM_LIB_ERROR;
+
+	/*
+	 * Fill the command structure starting from the first buffer:
+	 *     - the password (if any)
+	 *     - max tries
+	 *     - recovery time
+	 *     - lockout recovery
+	 */
+	ret = pack_byte_string(command_v2, sizeof(command_v2), "sddd",
+			       offset, pw, pw_sz,
+			       offset + pw_sz, max_tries,
+			       offset + pw_sz + 4, recovery_time,
+			       offset + pw_sz + 8, lockout_recovery);
+	offset += pw_sz + 12;
+	if (ret)
+		return TPM_LIB_ERROR;
+
+	return tpm_sendrecv_command(command_v2, NULL, NULL);
+}
+
 int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz)
 {
 	const u8 command_v1[10] = {
-- 
2.14.1

[U-Boot] [PATCH v2 17/19] tpm: add TPM2_HierarchyChangeAuth command support

[Miquel Raynal]

Add support for the TPM2_HierarchyChangeAuth command.

Change the command file and the help accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/tpm.c     | 34 ++++++++++++++++++++++++++++++++++
 include/tpm.h | 14 ++++++++++++++
 lib/tpm.c     | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 95 insertions(+)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index 3648d05581..132cc7b051 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -555,6 +555,36 @@ static int do_tpm_force_clear(cmd_tbl_t *cmdtp, int flag,
 	return report_return_code(tpm_force_clear(handle, pw, pw_sz));
 }
 
+static int do_tpm_change_auth(cmd_tbl_t *cmdtp, int flag,
+			      int argc, char *const argv[])
+{
+	u32 handle;
+	const char *newpw = argv[2];
+	const char *oldpw = (argc == 3) ? NULL : argv[3];
+	const ssize_t newpw_sz = strlen(newpw);
+	const ssize_t oldpw_sz = oldpw ? strlen(oldpw) : 0;
+
+	if (argc < 3 || argc > 4)
+		return CMD_RET_USAGE;
+
+	if (newpw_sz > TPM2_DIGEST_LENGTH || oldpw_sz > TPM2_DIGEST_LENGTH)
+		return -EINVAL;
+
+	if (!strcasecmp("TPM2_RH_LOCKOUT", argv[1]))
+		handle = TPM2_RH_LOCKOUT;
+	else if (!strcasecmp("TPM2_RH_ENDORSEMENT", argv[1]))
+		handle = TPM2_RH_ENDORSEMENT;
+	else if (!strcasecmp("TPM2_RH_OWNER", argv[1]))
+		handle = TPM2_RH_OWNER;
+	else if (!strcasecmp("TPM2_RH_PLATFORM", argv[1]))
+		handle = TPM2_RH_PLATFORM;
+	else
+		return CMD_RET_USAGE;
+
+	return report_return_code(tpm2_change_auth(handle, newpw, newpw_sz,
+						   oldpw, oldpw_sz));
+}
+
 #define TPM_COMMAND_NO_ARG(cmd)				\
 static int do_##cmd(cmd_tbl_t *cmdtp, int flag,		\
 		int argc, char * const argv[])		\
@@ -959,6 +989,8 @@ static cmd_tbl_t tpm_commands[] = {
 			 do_tpm_dam_set_parameters, "", ""),
 	U_BOOT_CMD_MKENT(force_clear, 0, 1,
 			 do_tpm_force_clear, "", ""),
+	U_BOOT_CMD_MKENT(change_auth, 0, 1,
+			 do_tpm_change_auth, "", ""),
 	U_BOOT_CMD_MKENT(physical_enable, 0, 1,
 			 do_tpm_physical_enable, "", ""),
 	U_BOOT_CMD_MKENT(physical_disable, 0, 1,
@@ -1060,6 +1092,8 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
 "  force_clear [<type>]\n"
 "    - Issue TPM_[Force]Clear command, with <type> one of (TPMv2 only):\n"
 "          * TPM2_RH_LOCKOUT, TPM2_RH_PLATFORM.\n"
+"  change_auth <new_pw> [<old_pw>]\n"
+"    - Change the hierarchy authorizations (TPMv2 only).\n"
 "  tsc_physical_presence flags\n"
 "    - Set TPM device's Physical Presence flags to <flags>.\n"
 "The Capability Commands:\n"
diff --git a/include/tpm.h b/include/tpm.h
index 4d062584f9..cc63f06634 100644
--- a/include/tpm.h
+++ b/include/tpm.h
@@ -60,6 +60,7 @@ enum tpm2_command_codes {
 	TPM2_CC_SELF_TEST	= 0x0143,
 	TPM2_CC_CLEAR		= 0x0126,
 	TPM2_CC_CLEARCONTROL	= 0x0127,
+	TPM2_CC_HIERCHANGEAUTH	= 0x0129,
 	TPM2_CC_DAM_RESET	= 0x0139,
 	TPM2_CC_DAM_PARAMETERS	= 0x013A,
 	TPM2_CC_GET_CAPABILITY	= 0x017A,
@@ -630,6 +631,19 @@ int tpm2_dam_set_parameters(const char *pw, const ssize_t pw_sz,
  */
 int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz);
 
+/**
+ * Issue a TPM2_HierarchyChangeAuthorization command.
+ *
+ * @param handle	Handle
+ * @param newpw		New password
+ * @param newpw_sz	Length of the new password
+ * @param oldpw		Old password
+ * @param oldpw_sz	Length of the old password
+ * @return return code of the operation
+ */
+int tpm2_change_auth(u32 handle, const char *newpw, const ssize_t newpw_sz,
+		     const char *oldpw, const ssize_t oldpw_sz);
+
 /**
  * Issue a TPM_PhysicalEnable command.
  *
diff --git a/lib/tpm.c b/lib/tpm.c
index 4987a4ce8c..c42aa2dbc1 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -835,6 +835,53 @@ int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz)
 	return tpm_sendrecv_command(command_v2, NULL, NULL);
 }
 
+int tpm2_change_auth(u32 handle, const char *newpw, const ssize_t newpw_sz,
+		     const char *oldpw, const ssize_t oldpw_sz)
+{
+	unsigned int offset = 27;
+	u8 command_v2[COMMAND_BUFFER_SIZE] = {
+		U16_TO_ARRAY(TPM2_ST_SESSIONS),	/* TAG */
+		U32_TO_ARRAY(offset + oldpw_sz + 2 + newpw_sz), /* Length */
+		U32_TO_ARRAY(TPM2_CC_HIERCHANGEAUTH), /* Command code */
+
+		/* HANDLE */
+		U32_TO_ARRAY(handle),		/* TPM resource handle */
+
+		/* AUTH_SESSION */
+		U32_TO_ARRAY(9 + oldpw_sz),	/* Authorization size */
+		U32_TO_ARRAY(TPM2_RS_PW),	/* Session handle */
+		U16_TO_ARRAY(0),		/* Size of <nonce> */
+						/* <nonce> (if any) */
+		0,				/* Attributes: Cont/Excl/Rst */
+		U16_TO_ARRAY(oldpw_sz)		/* Size of <hmac/password> */
+		/* STRING(oldpw)		   <hmac/password> (if any) */
+
+		/* TPM2B_AUTH (TPM2B_DIGEST) */
+		/* U16_TO_ARRAY(newpw_sz)	   Digest size, new pw length */
+		/* STRING(newpw)		   Digest buffer, new pw */
+	};
+	int ret;
+
+	if (!is_tpmv2)
+		return TPM_LIB_ERROR;
+
+	/*
+	 * Fill the command structure starting from the first buffer:
+	 *     - the old password (if any)
+	 *     - size of the new password
+	 *     - new password
+	 */
+	ret = pack_byte_string(command_v2, sizeof(command_v2), "sws",
+			       offset, oldpw, oldpw_sz,
+			       offset + oldpw_sz, newpw_sz,
+			       offset + oldpw_sz + 2, newpw, newpw_sz);
+	offset += oldpw_sz + 2 + newpw_sz;
+	if (ret)
+		return TPM_LIB_ERROR;
+
+	return tpm_sendrecv_command(command_v2, NULL, NULL);
+}
+
 uint32_t tpm_physical_enable(void)
 {
 	const uint8_t command[10] = {
-- 
2.14.1

[U-Boot] [PATCH v2 18/19] tpm: add PCR authentication commands support

[Miquel Raynal]

Add support for the TPM2_PCR_SetAuthPolicy and
TPM2_PCR_SetAuthValue commands.

Change the command file and the help accordingly.

Note: These commands could not be tested because the TPMs available
do not support them, however they could be useful for someone else.
The user is warned by the command help.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/tpm.c     |  49 +++++++++++++++++++++++++++
 include/tpm.h |  29 ++++++++++++++++
 lib/tpm.c     | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 184 insertions(+)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index 132cc7b051..3b523de3ab 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -349,6 +349,43 @@ static int do_tpm_pcr_event(cmd_tbl_t *cmdtp, int flag,
 	return report_return_code(rc);
 }
 
+static int do_tpm_pcr_setauthpolicy(cmd_tbl_t *cmdtp, int flag,
+				    int argc, char * const argv[])
+{
+	u32 index = simple_strtoul(argv[1], NULL, 0);
+	char *key = argv[2];
+	const char *pw = (argc < 4) ? NULL : argv[3];
+	const ssize_t pw_sz = pw ? strlen(pw) : 0;
+
+	if (strlen(key) != TPM2_DIGEST_LENGTH)
+		return -EINVAL;
+
+	if (argc < 3 || argc > 4)
+		return CMD_RET_USAGE;
+
+	return report_return_code(tpm2_pcr_setauthpolicy(pw, pw_sz, index,
+							 key));
+}
+
+static int do_tpm_pcr_setauthvalue(cmd_tbl_t *cmdtp, int flag,
+				   int argc, char * const argv[])
+{
+	u32 index = simple_strtoul(argv[1], NULL, 0);
+	char *key = argv[2];
+	const ssize_t key_sz = strlen(key);
+	const char *pw = (argc < 4) ? NULL : argv[3];
+	const ssize_t pw_sz = pw ? strlen(pw) : 0;
+
+	if (strlen(key) != TPM2_DIGEST_LENGTH)
+		return -EINVAL;
+
+	if (argc < 3 || argc > 4)
+		return CMD_RET_USAGE;
+
+	return report_return_code(tpm2_pcr_setauthvalue(pw, pw_sz, index,
+							key, key_sz));
+}
+
 static int do_tpm_pcr_extend(cmd_tbl_t *cmdtp, int flag,
 			     int argc, char * const argv[])
 {
@@ -1003,6 +1040,10 @@ static cmd_tbl_t tpm_commands[] = {
 			 do_tpm_nv_write_value, "", ""),
 	U_BOOT_CMD_MKENT(pcr_event, 0, 1,
 			 do_tpm_pcr_event, "", ""),
+	U_BOOT_CMD_MKENT(pcr_setauthpolicy, 0, 1,
+			 do_tpm_pcr_setauthpolicy, "", ""),
+	U_BOOT_CMD_MKENT(pcr_setauthvalue, 0, 1,
+			 do_tpm_pcr_setauthvalue, "", ""),
 	U_BOOT_CMD_MKENT(pcr_extend, 0, 1,
 			 do_tpm_pcr_extend, "", ""),
 	U_BOOT_CMD_MKENT(pcr_read, 0, 1,
@@ -1150,6 +1191,14 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
 "    - Read <count> bytes of the public endorsement key to memory\n"
 "      address <addr>\n"
 "Integrity Collection and Reporting Commands:\n"
+"  pcr_setauthpolicy <index> <key> <pw>\n"
+"    - Change the <key> to access <index> PCR. <pw> is for the platform\n"
+"      hierarchy and may be empty.\n"
+"      /!\\WARNING: untested function, use at your own risks !\n"
+"  pcr_setauthvalue <index> <key> <pw>\n"
+"    - Change the <key> to access <index> PCR. <pw> is for the platform\n"
+"      hierarchy and may be empty.\n"
+"      /!\\WARNING: untested function, use at your own risks !\n"
 "  pcr_event <index> <digest_in> <digest_out>\n"
 "    - Add a new measurement to a PCR.  Update PCR <index> with\n"
 "      <digest_in>. It must be a 20 byte digest for TPMv1 or a SHA256\n"
diff --git a/include/tpm.h b/include/tpm.h
index cc63f06634..93ec521e74 100644
--- a/include/tpm.h
+++ b/include/tpm.h
@@ -61,11 +61,13 @@ enum tpm2_command_codes {
 	TPM2_CC_CLEAR		= 0x0126,
 	TPM2_CC_CLEARCONTROL	= 0x0127,
 	TPM2_CC_HIERCHANGEAUTH	= 0x0129,
+	TPM2_CC_PCR_SETAUTHPOL	= 0x012C,
 	TPM2_CC_DAM_RESET	= 0x0139,
 	TPM2_CC_DAM_PARAMETERS	= 0x013A,
 	TPM2_CC_GET_CAPABILITY	= 0x017A,
 	TPM2_CC_PCR_READ	= 0x017E,
 	TPM2_CC_PCR_EXTEND	= 0x0182,
+	TPM2_CC_PCR_SETAUTHVAL	= 0x0183,
 };
 
 enum tpm2_return_codes {
@@ -559,6 +561,33 @@ uint32_t tpm_nv_write_value(uint32_t index, const void *data, uint32_t length);
  */
 int tpm_pcr_event(u32 index, const void *in_digest, void *out_digest);
 
+/**
+ * Issue a TPM_PCR_SetAuthPolicy command.
+ *
+ * @param pw		Platform password
+ * @param pw_sz		Length of the password
+ * @param index		Index of the PCR
+ * @param digest	New key to access the PCR
+ *
+ * @return return code of the operation
+ */
+int tpm2_pcr_setauthpolicy(const char *pw, const ssize_t pw_sz, u32 index,
+			   const char *key);
+
+/**
+ * Issue a TPM_PCR_SetAuthValue command.
+ *
+ * @param pw		Platform password
+ * @param pw_sz		Length of the password
+ * @param index		Index of the PCR
+ * @param digest	New key to access the PCR
+ * @param key_sz	Length of the new key
+ *
+ * @return return code of the operation
+ */
+int tpm2_pcr_setauthvalue(const char *pw, const ssize_t pw_sz, u32 index,
+			  const char *key, const ssize_t key_sz);
+
 /**
  * Issue a TPM_PCR_Extend command.
  *
diff --git a/lib/tpm.c b/lib/tpm.c
index c42aa2dbc1..a10de09b3e 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -527,6 +527,112 @@ int tpm_pcr_event(u32 index, const void *in_digest, void *out_digest)
 	return 0;
 }
 
+int tpm2_pcr_setauthpolicy(const char *pw, const ssize_t pw_sz, u32 index,
+			   const char *key)
+{
+	u8 command_v2[COMMAND_BUFFER_SIZE] = {
+		U16_TO_ARRAY(TPM2_ST_SESSIONS),	/* TAG */
+		U32_TO_ARRAY(35 + pw_sz + TPM2_DIGEST_LENGTH), /* Length */
+		U32_TO_ARRAY(TPM2_CC_PCR_SETAUTHPOL), /* Command code */
+
+		/* HANDLE */
+		U32_TO_ARRAY(TPM2_RH_PLATFORM),	/* TPM resource handle */
+
+		/* AUTH_SESSION */
+		U32_TO_ARRAY(9 + pw_sz),	/* Authorization size */
+		U32_TO_ARRAY(TPM2_RS_PW),	/* session handle */
+		U16_TO_ARRAY(0),		/* Size of <nonce> */
+						/* <nonce> (if any) */
+		0,				/* Attributes: Cont/Excl/Rst */
+		U16_TO_ARRAY(pw_sz)		/* Size of <hmac/password> */
+		/* STRING(pw)			   <hmac/password> (if any) */
+
+		/* TPM2B_AUTH (TPM2B_DIGEST) */
+		/* U16_TO_ARRAY(TPM2_DIGEST_LENGTH) Digest size length */
+		/* STRING(key)			   Digest buffer (PCR key) */
+
+		/* TPMI_ALG_HASH */
+		/* U16_TO_ARRAY(TPM2_ALG_SHA256)   Algorithm of the hash */
+
+		/* TPMI_DH_PCR */
+		/* U32_TO_ARRAY(index),		   PCR Index */
+	};
+	unsigned int offset = 27;
+	int ret;
+
+	if (!is_tpmv2)
+		return TPM_LIB_ERROR;
+
+	/*
+	 * Fill the command structure starting from the first buffer:
+	 *     - the password (if any)
+	 *     - the PCR key length
+	 *     - the PCR key
+	 *     - the hash algorithm
+	 *     - the PCR index
+	 */
+	ret = pack_byte_string(command_v2, sizeof(command_v2), "swswd",
+			       offset, pw, pw_sz,
+			       offset + pw_sz, TPM2_DIGEST_LENGTH,
+			       offset + pw_sz + 2, key, TPM2_DIGEST_LENGTH,
+			       offset + pw_sz + 2 + TPM2_DIGEST_LENGTH,
+			       TPM2_ALG_SHA256,
+			       offset + pw_sz + 4 + TPM2_DIGEST_LENGTH, index);
+	offset += pw_sz + 2 + TPM2_DIGEST_LENGTH + 2 + 4;
+	if (ret)
+		return TPM_LIB_ERROR;
+
+	return tpm_sendrecv_command(command_v2, NULL, NULL);
+}
+
+int tpm2_pcr_setauthvalue(const char *pw, const ssize_t pw_sz, u32 index,
+			  const char *key, const ssize_t key_sz)
+{
+	u8 command_v2[COMMAND_BUFFER_SIZE] = {
+		U16_TO_ARRAY(TPM2_ST_SESSIONS),	/* TAG */
+		U32_TO_ARRAY(33 + pw_sz + TPM2_DIGEST_LENGTH), /* Length */
+		U32_TO_ARRAY(TPM2_CC_PCR_SETAUTHVAL), /* Command code */
+
+		/* HANDLE */
+		U32_TO_ARRAY(index),		/* Handle (PCR Index) */
+
+		/* AUTH_SESSION */
+		U32_TO_ARRAY(9 + pw_sz),	/* Authorization size */
+		U32_TO_ARRAY(TPM2_RS_PW),	/* session handle */
+		U16_TO_ARRAY(0),		/* Size of <nonce> */
+						/* <nonce> (if any) */
+		0,				/* Attributes: Cont/Excl/Rst */
+		U16_TO_ARRAY(pw_sz),		/* Size of <hmac/password> */
+		/* STRING(pw)			   <hmac/password> (if any) */
+
+		/* TPM2B_DIGEST */
+		/* U16_TO_ARRAY(key_sz)		   Key length */
+		/* STRING(key)			   Key */
+	};
+	unsigned int offset = 27;
+	int ret;
+
+	if (!is_tpmv2)
+		return TPM_LIB_ERROR;
+
+	/*
+	 * Fill the command structure starting from the first buffer:
+	 *     - the password (if any)
+	 *     - the number of digests, 1 in our case
+	 *     - the algorithm, sha256 in our case
+	 *     - the digest (64 bytes)
+	 */
+	ret = pack_byte_string(command_v2, sizeof(command_v2), "sws",
+			       offset, pw, pw_sz,
+			       offset + pw_sz, key_sz,
+			       offset + pw_sz + 2, key, key_sz);
+	offset += pw_sz + 2 + key_sz;
+	if (ret)
+		return TPM_LIB_ERROR;
+
+	return tpm_sendrecv_command(command_v2, NULL, NULL);
+}
+
 int tpm2_pcr_extend(u32 index, const uint8_t *digest)
 {
 	u8 command_v2[COMMAND_BUFFER_SIZE] = {
-- 
2.14.1

[U-Boot] [PATCH v2 19/19] test/py: add TPMv2.0 test suite

[Miquel Raynal]

Add tests for the TPMv2.0 commands.
These commands need a physical TPM to run properly, there is no sandbox
support yet.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 test/py/tests/test_tpm2.py | 254 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 254 insertions(+)
 create mode 100644 test/py/tests/test_tpm2.py

diff --git a/test/py/tests/test_tpm2.py b/test/py/tests/test_tpm2.py
new file mode 100644
index 0000000000..eb2bc93cd3
--- /dev/null
+++ b/test/py/tests/test_tpm2.py
@@ -0,0 +1,254 @@
+# Copyright (c) 2018, Bootlin
+#
+# SPDX-License-Identifier: GPL-2.0
+
+import os.path
+import pytest
+import u_boot_utils
+import re
+import time
+
+"""
+Test the TPMv2 related commands. You must have a working hardware setup in order
+to do these tests.
+
+Notes:
+* These tests will prove the password mechanism. The TPM chip must be cleared of
+any password.
+* Commands like pcr_setauthpolicy and pcr_resetauthpolicy are not implemented
+here because they would fail the tests in most cases (TPMs do not implement them
+and return an error).
+"""
+
+updates = 0
+
+def force_init(u_boot_console):
+    """When a test fails, U-Boot is reset. Because TPM stack must be initialized
+    after each reboot, we must ensure these lines are always executed before
+    trying any command or they will fail with no reason.
+    """
+    output = u_boot_console.run_command('tpm init TPM2')
+    if not 'TPM error: -16' in output:
+        u_boot_console.run_command('echo --- start of init ---')
+        u_boot_console.run_command('tpm startup TPM2_SU_CLEAR')
+        u_boot_console.run_command('tpm self_test_full')
+        u_boot_console.run_command('tpm force_clear TPM2_RH_LOCKOUT')
+        output = u_boot_console.run_command('echo $?')
+        if not output.endswith('0'):
+            u_boot_console.run_command('tpm force_clear TPM2_RH_PLATFORM')
+        u_boot_console.run_command('echo --- end of init ---')
+
+ at pytest.mark.buildconfigspec('tpm')
+ at pytest.mark.buildconfigspec('tpm_tis_spi')
+ at pytest.mark.buildconfigspec('cmd_tpm')
+def test_tpm2_init(u_boot_console):
+    """Init the software stack to use TPMv2 commands."""
+
+    output = u_boot_console.run_command('tpm init TPM2')
+    assert output.endswith('TPM complies to the v2.x specification')
+
+ at pytest.mark.buildconfigspec('tpm')
+ at pytest.mark.buildconfigspec('tpm_tis_spi')
+ at pytest.mark.buildconfigspec('cmd_tpm')
+def test_tpm2_startup(u_boot_console):
+    """Execute a TPM2_Startup command.
+
+    Initiate the TPM internal state machine.
+    """
+
+    u_boot_console.run_command('tpm startup TPM2_SU_CLEAR')
+    output = u_boot_console.run_command('echo $?')
+    assert output.endswith('0')
+
+ at pytest.mark.buildconfigspec('tpm')
+ at pytest.mark.buildconfigspec('tpm_tis_spi')
+ at pytest.mark.buildconfigspec('cmd_tpm')
+def test_tpm2_self_test_full(u_boot_console):
+    """Execute a TPM2_SelfTest (full) command.
+
+    Ask the TPM to perform all self tests to also enable full capabilities.
+    """
+
+    u_boot_console.run_command('tpm self_test_full')
+    output = u_boot_console.run_command('echo $?')
+    assert output.endswith('0')
+
+ at pytest.mark.buildconfigspec('tpm')
+ at pytest.mark.buildconfigspec('tpm_tis_spi')
+ at pytest.mark.buildconfigspec('cmd_tpm')
+def test_tpm2_continue_self_test(u_boot_console):
+    """Execute a TPM2_SelfTest (continued) command.
+
+    Ask the TPM to finish its self tests (alternative to the full test) in order
+    to enter a fully operational state.
+    """
+
+    u_boot_console.run_command('tpm continue_self_test')
+    output = u_boot_console.run_command('echo $?')
+    assert output.endswith('0')
+
+ at pytest.mark.buildconfigspec('tpm')
+ at pytest.mark.buildconfigspec('tpm_tis_spi')
+ at pytest.mark.buildconfigspec('cmd_tpm')
+def test_tpm2_clear(u_boot_console):
+    """Execute a TPM2_Clear command.
+
+    Ask the TPM to reset entirely its internal state (including internal
+    configuration, passwords, counters and DAM parameters). This is half of the
+    TAKE_OWNERSHIP command from TPMv1.
+
+    Use the LOCKOUT hierarchy for this. The LOCKOUT/PLATFORM hierarchies must
+    not have a password set, otherwise this test will fail. ENDORSEMENT and
+    PLATFORM hierarchies are also available.
+    """
+
+    u_boot_console.run_command('tpm force_clear TPM2_RH_LOCKOUT')
+    output = u_boot_console.run_command('echo $?')
+    assert output.endswith('0')
+
+    u_boot_console.run_command('tpm force_clear TPM2_RH_PLATFORM')
+    output = u_boot_console.run_command('echo $?')
+    assert output.endswith('0')
+
+ at pytest.mark.buildconfigspec('tpm')
+ at pytest.mark.buildconfigspec('tpm_tis_spi')
+ at pytest.mark.buildconfigspec('cmd_tpm')
+def test_tpm2_change_auth(u_boot_console):
+    """Execute a TPM2_HierarchyChangeAuth command.
+
+    Ask the TPM to change the owner, ie. set a new password: 'unicorn'
+
+    Use the LOCKOUT hierarchy for this. ENDORSEMENT and PLATFORM hierarchies are
+    also available.
+    """
+
+    force_init(u_boot_console)
+
+    u_boot_console.run_command('tpm change_auth TPM2_RH_LOCKOUT unicorn')
+    output = u_boot_console.run_command('echo $?')
+    assert output.endswith('0')
+
+    u_boot_console.run_command('tpm force_clear TPM2_RH_LOCKOUT')
+    output = u_boot_console.run_command('echo $?')
+    u_boot_console.run_command('tpm force_clear TPM2_RH_PLATFORM')
+    assert not output.endswith('0')
+
+ at pytest.mark.buildconfigspec('tpm')
+ at pytest.mark.buildconfigspec('tpm_tis_spi')
+ at pytest.mark.buildconfigspec('cmd_tpm')
+def test_tpm2_get_capability(u_boot_console):
+    """Execute a TPM_GetCapability command.
+
+    Display one capability. In our test case, let's display the default DAM
+    lockout counter that should be 0 since the CLEAR:
+    - TPM_CAP_TPM_PROPERTIES = 0x6
+    - TPM_PT_LOCKOUT_COUNTER (1st parameter) = PTR_VAR + 14
+
+    There is no expected default values because it would depend on the chip
+    used. We can still save them in order to check they have changed later.
+    """
+
+    force_init(u_boot_console)
+    ram = u_boot_utils.find_ram_base(u_boot_console)
+
+    read_cap = u_boot_console.run_command('tpm get_capability 0x6 0x20e 0x%x 1' % ram)
+    output = u_boot_console.run_command('echo $?')
+    assert output.endswith('0')
+    assert 'Property 0x0000020e: 0x00000000' in read_cap
+
+ at pytest.mark.buildconfigspec('tpm')
+ at pytest.mark.buildconfigspec('tpm_tis_spi')
+ at pytest.mark.buildconfigspec('cmd_tpm')
+def test_tpm2_dam_set_parameters(u_boot_console):
+    """Execute a TPM2_DictionaryAttackParameters command.
+
+    Change Dictionary Attack Mitigation (DAM) parameters. Ask the TPM to change:
+    - Max number of failed authentication before lockout: 3
+    - Time before the failure counter is automatically decremented: 10 sec
+    - Time after a lockout failure before it can be attempted again: 0 sec
+
+    For an unknown reason, the DAM parameters must be changed before changing
+    the authentication, otherwise the lockout will be engaged after the first
+    failed authentication attempt.
+    """
+
+    force_init(u_boot_console)
+    ram = u_boot_utils.find_ram_base(u_boot_console)
+
+    # Set the DAM parameters to known values
+    u_boot_console.run_command('tpm dam_set_parameters 3 10 0')
+    output = u_boot_console.run_command('echo $?')
+    assert output.endswith('0')
+
+    # Check the values have been saved
+    read_cap = u_boot_console.run_command('tpm get_capability 0x6 0x20f 0x%x 3' % ram)
+    output = u_boot_console.run_command('echo $?')
+    assert output.endswith('0')
+    assert 'Property 0x0000020f: 0x00000003' in read_cap
+    assert 'Property 0x00000210: 0x0000000a' in read_cap
+    assert 'Property 0x00000211: 0x00000000' in read_cap
+
+ at pytest.mark.buildconfigspec('tpm')
+ at pytest.mark.buildconfigspec('tpm_tis_spi')
+ at pytest.mark.buildconfigspec('cmd_tpm')
+def test_tpm2_read(u_boot_console):
+    """Execute a TPM2_PCR_Read command.
+
+    Perform a PCR read of the 0th PCR. Must be zero.
+    """
+
+    force_init(u_boot_console)
+    ram = u_boot_utils.find_ram_base(u_boot_console) + 1024
+
+    read_pcr = u_boot_console.run_command('tpm pcr_read 0 0x%x' % ram)
+    output = u_boot_console.run_command('echo $?')
+    assert output.endswith('0')
+
+    # Save the number of PCR updates
+    str = re.findall(r'known updates: \d+', read_pcr)[0]
+    global updates
+    updates = int(re.findall(r'\d+', str)[0])
+
+    # Check the output value
+    assert 'Named PCR 0 content' in read_pcr
+    assert '''
+ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00\r
+ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00''' in read_pcr
+
+ at pytest.mark.buildconfigspec('tpm')
+ at pytest.mark.buildconfigspec('tpm_tis_spi')
+ at pytest.mark.buildconfigspec('cmd_tpm')
+def test_tpm2_extend(u_boot_console):
+    """Execute a TPM2_PCR_Extend command.
+
+    Perform a PCR extension with a known hash in memory (zeroed since the board
+    must have been rebooted).
+
+    No authentication mechanism is used here, not protecting against packet
+    replay, yet.
+    """
+
+    force_init(u_boot_console)
+    ram = u_boot_utils.find_ram_base(u_boot_console) + 1024
+
+    u_boot_console.run_command('tpm pcr_extend 0 0x%x' % ram)
+    output = u_boot_console.run_command('echo $?')
+    assert output.endswith('0')
+
+    read_pcr = u_boot_console.run_command('tpm pcr_read 0 0x%x' % ram)
+    output = u_boot_console.run_command('echo $?')
+    assert output.endswith('0')
+    assert 'f5 a5 fd 42 d1 6a 20 30 27 98 ef 6e d3 09 97 9b' in read_pcr
+    assert '43 00 3d 23 20 d9 f0 e8 ea 98 31 a9 27 59 fb 4b' in read_pcr
+
+    str = re.findall(r'known updates: \d+', read_pcr)[0]
+    new_updates = int(re.findall(r'\d+', str)[0])
+    assert (updates + 1) == new_updates
+
+ at pytest.mark.buildconfigspec('tpm')
+ at pytest.mark.buildconfigspec('tpm_tis_spi')
+ at pytest.mark.buildconfigspec('cmd_tpm')
+def test_tpm2_cleanup(u_boot_console):
+    """Ensure the TPM is cleared from password or test related configuration."""
+
+    force_init(u_boot_console)
-- 
2.14.1

[U-Boot] [PATCH v2 12/19] tpm: rename the _extend() function to be _pcr_event()

[Reinhard Pfau]

Hi,

Am 2018-03-29 09:43, schrieb Miquel Raynal:
> The function currently called _extend() actually does what the
> specification defines as a _pcr_event(). Rename the function
> accordingly before implementing the actual _extend() command.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---

The TPM 1.2 spec calls this function "TPM_Extend"!
So renaming this func will be misleading for users of TPM1.2
devices.

Since TPM1.2 and TPM2 seems to be really different it might be an
idea to create a separate command ("tpm2"?) for TPM2 devices...
(seperate lib, too?)

For this renaming (as user of TPM1.2 devices): NAK

Greetings,
Reinhard Pfau
Guntermann & Drunck GmbH

[U-Boot] [PATCH v2 12/19] tpm: rename the _extend() function to be _pcr_event()

[Miquel Raynal]

Hi Reinhard,

On Thu, 29 Mar 2018 11:44:28 +0200, Reinhard Pfau
<reinhard.pfau@gdsys.cc> wrote:

> Hi,
> 
> Am 2018-03-29 09:43, schrieb Miquel Raynal:
> > The function currently called _extend() actually does what the
> > specification defines as a _pcr_event(). Rename the function
> > accordingly before implementing the actual _extend() command.  
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
> > ---  
> 
> The TPM 1.2 spec calls this function "TPM_Extend"!
> So renaming this func will be misleading for users of TPM1.2
> devices.
> 
> Since TPM1.2 and TPM2 seems to be really different it might be an
> idea to create a separate command ("tpm2"?) for TPM2 devices...
> (seperate lib, too?)

I used that trick for other commands (prefixing tpm1_ and tpm2_), I
will do the same here to avoid confusion. Thanks for pointing it.

> 
> For this renaming (as user of TPM1.2 devices): NAK
> 
> Greetings,
> Reinhard Pfau
> Guntermann & Drunck GmbH
> 
> 
> 



-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[U-Boot] [PATCH v2 01/19] tpm: add Revision ID field in the chip structure

[Simon Glass]

On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> TPM are shipped with a few read-only register from which we can retrieve
> for instance:
> - vendor ID
> - product ID
> - revision ID
>
> Product and vendor ID share the same register and are already referenced
> in the tpm_chip structure. Add the revision ID entry which is missing.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/tpm/tpm_tis.h | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 02/19] tpm: rename tpm_tis_infineon in tpm_tis_infineon_i2c

[Simon Glass]

On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> As the chips driven by tpm_tis_infineon.c are only I2C chips, rename the
> driver with the _i2c suffix to prepare the venue of its _spi cousin.
>
> Also change the driver name in the U_BOOT_DRIVER structure.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/tpm/Kconfig                                        | 4 ++--
>  drivers/tpm/Makefile                                       | 2 +-
>  drivers/tpm/{tpm_tis_infineon.c => tpm_tis_infineon_i2c.c} | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
>  rename drivers/tpm/{tpm_tis_infineon.c => tpm_tis_infineon_i2c.c} (99%)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 03/19] tpm: add support for TPMv2 SPI modules

[Simon Glass]

Hi Miquel,

On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Add the tpm_tis_spi driver that should support any TPMv2 compliant (SPI)
> module.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/tpm/Kconfig       |   9 +
>  drivers/tpm/Makefile      |   1 +
>  drivers/tpm/tpm_tis.h     |   3 +
>  drivers/tpm/tpm_tis_spi.c | 656 ++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 669 insertions(+)
>  create mode 100644 drivers/tpm/tpm_tis_spi.c

I think this came up in another context. Would it make sense to create
a common interface to i2c and SPI and then have a common driver?

Regards,
Simon

[U-Boot] [PATCH v2 04/19] tpm: fix indentation in command list before adding more

[Simon Glass]

On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Prepare the addition of more commands by first indenting correctly the
> current list.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  cmd/tpm.c | 40 ++++++++++++++++++++--------------------
>  1 file changed, 20 insertions(+), 20 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 05/19] tpm: prepare support for TPMv2 commands

[Simon Glass]

Hi Miquel,

On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Later choice between v1 and v2 compliant functions will be handled by a
> global value in lib/tpm.c that will be accessible through set/get
> accessors from lib/cmd.c.
>
> This global value is set during the initialization phase if the TPM2
> handle is given to the init command.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  cmd/tpm.c     | 37 +++++++++++++++++++++-----
>  include/tpm.h | 20 ++++++++++++++
>  lib/tpm.c     | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 131 insertions(+), 9 deletions(-)
>
> diff --git a/cmd/tpm.c b/cmd/tpm.c
> index f456396d75..1d32028b64 100644
> --- a/cmd/tpm.c
> +++ b/cmd/tpm.c
> @@ -89,12 +89,16 @@ static void *parse_byte_string(char *bytes, uint8_t *data, size_t *count_ptr)
>   */
>  static int report_return_code(int return_code)
>  {
> -       if (return_code) {
> -               printf("Error: %d\n", return_code);
> -               return CMD_RET_FAILURE;
> -       } else {
> +       if (!return_code)
>                 return CMD_RET_SUCCESS;
> -       }
> +
> +       if (return_code == -EOPNOTSUPP)
> +               printf("TPMv%d error: unavailable command with this spec\n",
> +                      tpm_get_specification());
> +       else
> +               printf("TPM error: %d\n", return_code);
> +
> +       return CMD_RET_FAILURE;
>  }
>
>  /**
> @@ -427,6 +431,24 @@ static int do_tpm_get_capability(cmd_tbl_t *cmdtp, int flag,
>         return report_return_code(rc);
>  }
>
> +static int do_tpm_init(cmd_tbl_t *cmdtp, int flag,
> +                      int argc, char * const argv[])
> +{
> +       if (argc > 2)
> +               return CMD_RET_USAGE;
> +
> +       if (argc == 2) {
> +               if (!strcasecmp("TPM1", argv[1]))
> +                       tpm_set_specification(1);
> +               else if (!strcasecmp("TPM2", argv[1]))
> +                       tpm_set_specification(2);
> +               else
> +                       return CMD_RET_USAGE;
> +       }
> +

I don't like the idea of setting a global before calling tpm_init().
Can we instead make it an arg to tpm_init()?

Also, instead of 1 and 2, can you create an enum?

> +       return report_return_code(tpm_init());
> +}
> +
>  #define TPM_COMMAND_NO_ARG(cmd)                                \
>  static int do_##cmd(cmd_tbl_t *cmdtp, int flag,                \
>                 int argc, char * const argv[])          \
> @@ -436,7 +458,6 @@ static int do_##cmd(cmd_tbl_t *cmdtp, int flag,             \
>         return report_return_code(cmd());               \
>  }
>
> -TPM_COMMAND_NO_ARG(tpm_init)
>  TPM_COMMAND_NO_ARG(tpm_self_test_full)
>  TPM_COMMAND_NO_ARG(tpm_continue_self_test)
>  TPM_COMMAND_NO_ARG(tpm_force_clear)
> @@ -902,8 +923,10 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
>  "    - Issue TPM command <cmd> with arguments <args...>.\n"
>  "Admin Startup and State Commands:\n"
>  "  info - Show information about the TPM\n"
> -"  init\n"
> +"  init [<type>]\n"
>  "    - Put TPM into a state where it waits for 'startup' command.\n"
> +"      <type> is one of TPM1 (default) or TPM2. This choice impacts the way\n"
> +"      other functions will behave.\n"
>  "  startup mode\n"
>  "    - Issue TPM_Starup command.  <mode> is one of TPM_ST_CLEAR,\n"
>  "      TPM_ST_STATE, and TPM_ST_DEACTIVATED.\n"
> diff --git a/include/tpm.h b/include/tpm.h
> index 760d94865c..0ec3428ea4 100644
> --- a/include/tpm.h
> +++ b/include/tpm.h
> @@ -30,6 +30,11 @@ enum tpm_startup_type {
>         TPM_ST_DEACTIVATED      = 0x0003,
>  };
>
> +enum tpm2_startup_types {

Please add a comment as to what this is for.

> +       TPM2_SU_CLEAR   = 0x0000,
> +       TPM2_SU_STATE   = 0x0001,
> +};
> +
>  enum tpm_physical_presence {
>         TPM_PHYSICAL_PRESENCE_HW_DISABLE        = 0x0200,
>         TPM_PHYSICAL_PRESENCE_CMD_DISABLE       = 0x0100,
> @@ -417,6 +422,21 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size,
>   */
>  int tpm_init(void);
>
> +/**
> + * Assign a value to the global is_nfcv2 boolean to discriminate in the lib
> + * between the available specifications.
> + *
> + * @version: 1 or 2, depending on the supported specification
> + */
> +int tpm_set_specification(int version);
> +
> +/**
> + * Return the current value of the specification.
> + *
> + * @return: 1 or 2, depending on the supported specification
> + */
> +int tpm_get_specification(void);
> +
>  /**
>   * Issue a TPM_Startup command.
>   *
> diff --git a/lib/tpm.c b/lib/tpm.c
> index 99556b1cf6..38b76b4961 100644
> --- a/lib/tpm.c
> +++ b/lib/tpm.c
> @@ -15,6 +15,9 @@
>  /* Internal error of TPM command library */
>  #define TPM_LIB_ERROR  ((uint32_t)~0u)
>
> +/* Global boolean to discriminate TPMv2.x from TPMv1.x functions */
> +static bool is_tpmv2;

Can this go in the TPM uclass as uclass-private data? See struct
uclass member 'priv'.

> +
>  /* Useful constants */
>  enum {
>         COMMAND_BUFFER_SIZE             = 256,
> @@ -262,6 +265,26 @@ static uint32_t tpm_sendrecv_command(const void *command,
>         return tpm_return_code(response);
>  }
>
> +int tpm_set_specification(int version)
> +{
> +       if (version == 1) {
> +               debug("TPM complies to the v1.x specification\n");
> +               is_tpmv2 = false;
> +       } else if (version == 2) {
> +               debug("TPM complies to the v2.x specification\n");
> +               is_tpmv2 = true;
> +       } else {
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +int tpm_get_specification(void)
> +{
> +       return is_tpmv2 ? 2 : 1;
> +}
> +
>  int tpm_init(void)
>  {
>         int err;
> @@ -338,6 +361,9 @@ uint32_t tpm_nv_define_space(uint32_t index, uint32_t perm, uint32_t size)
>         const size_t size_offset = 77;
>         uint8_t buf[COMMAND_BUFFER_SIZE];
>
> +       if (is_tpmv2)
> +               return -EOPNOTSUPP;
> +
>         if (pack_byte_string(buf, sizeof(buf), "sddd",
>                                 0, command, sizeof(command),
>                                 index_offset, index,
> @@ -362,6 +388,9 @@ uint32_t tpm_nv_read_value(uint32_t index, void *data, uint32_t count)
>         uint32_t data_size;
>         uint32_t err;
>
> +       if (is_tpmv2)
> +               return -EOPNOTSUPP;

This return code should be mentioned in the header file for these functions.

[...]

Regards,
Simon

[U-Boot] [PATCH v2 07/19] tpm: add possible traces to analyze buffers returned by the TPM

[Simon Glass]

On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> When debugging, it is welcome to get more information about what the TPM
> returns. Add the possibility to print the packets received to show their
> exact content.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  lib/tpm.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

You might consider using the new logging support for this? See log.h

- Simon

[U-Boot] [PATCH v2 08/19] tpm: handle different buffer sizes

[Simon Glass]

On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Usual buffer sizes for TPMv1 and TPMv2 are different. Change TPMv1
> buffer size definition for that and declare another size for TPMv2
> buffers.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  cmd/tpm.c     |  5 +++--
>  include/tpm.h |  2 ++
>  lib/tpm.c     | 60 +++++++++++++++++++++++++++++------------------------------
>  3 files changed, 34 insertions(+), 33 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Suggest TPM1_DIGEST_LEN which is a bit shorter.

- Simon

[U-Boot] [PATCH v2 09/19] tpm: add TPM2_Startup command support

[Simon Glass]

Hi Miquel,

On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Add support for the TPM2_Startup command.
>
> Change the command file and the help accordingly.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  cmd/tpm.c     |  9 +++++++--
>  include/tpm.h | 26 +++++++++++++++++++++++++-
>  lib/tpm.c     | 35 +++++++++++++++++++++++++----------
>  3 files changed, 57 insertions(+), 13 deletions(-)
>
> diff --git a/cmd/tpm.c b/cmd/tpm.c
> index 3e2bb3b118..fc9ef9d4a3 100644
> --- a/cmd/tpm.c
> +++ b/cmd/tpm.c
> @@ -255,6 +255,10 @@ static int do_tpm_startup(cmd_tbl_t *cmdtp, int flag,
>                 mode = TPM_ST_STATE;
>         } else if (!strcasecmp("TPM_ST_DEACTIVATED", argv[1])) {
>                 mode = TPM_ST_DEACTIVATED;
> +       } else if (!strcasecmp("TPM2_SU_CLEAR", argv[1])) {
> +               mode = TPM2_SU_CLEAR;
> +       } else if (!strcasecmp("TPM2_SU_STATE", argv[1])) {
> +               mode = TPM2_SU_STATE;
>         } else {
>                 printf("Couldn't recognize mode string: %s\n", argv[1]);
>                 return CMD_RET_FAILURE;
> @@ -929,8 +933,9 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
>  "      <type> is one of TPM1 (default) or TPM2. This choice impacts the way\n"
>  "      other functions will behave.\n"
>  "  startup mode\n"
> -"    - Issue TPM_Starup command.  <mode> is one of TPM_ST_CLEAR,\n"
> -"      TPM_ST_STATE, and TPM_ST_DEACTIVATED.\n"
> +"    - Issue TPM_Startup command. <mode> is one of:\n"
> +"          * TPM_ST_CLEAR, TPM_ST_STATE, TPM_ST_DEACTIVATED for TPMv1.x.\n"
> +"          * TPM2_SU_CLEAR, TPM2_SU_STATE, for TPMv2.x.\n"
>  "Admin Testing Commands:\n"
>  "  self_test_full\n"
>  "    - Test all of the TPM capabilities.\n"
> diff --git a/include/tpm.h b/include/tpm.h
> index 1a60ef5b36..ba71bac885 100644
> --- a/include/tpm.h
> +++ b/include/tpm.h
> @@ -26,6 +26,11 @@ enum tpm_duration {
>         TPM_DURATION_COUNT,
>  };
>
> +enum tpm2_structures {

Please add comments for these.

> +       TPM2_ST_NO_SESSIONS     = 0x8001,
> +       TPM2_ST_SESSIONS        = 0x8002,
> +};
> +
>  enum tpm_startup_type {
>         TPM_ST_CLEAR            = 0x0001,
>         TPM_ST_STATE            = 0x0002,
> @@ -37,6 +42,25 @@ enum tpm2_startup_types {
>         TPM2_SU_STATE   = 0x0001,
>  };
>
> +enum tpm2_command_codes {
> +       TPM2_CC_STARTUP         = 0x0144,
> +       TPM2_CC_SELF_TEST       = 0x0143,
> +       TPM2_CC_GET_CAPABILITY  = 0x017A,
> +       TPM2_CC_PCR_READ        = 0x017E,
> +       TPM2_CC_PCR_EXTEND      = 0x0182,
> +};
> +
> +enum tpm2_return_codes {
> +       TPM2_RC_SUCCESS         = 0x0000,
> +       TPM2_RC_HASH            = 0x0083, /* RC_FMT1 */
> +       TPM2_RC_HANDLE          = 0x008B,
> +       TPM2_RC_INITIALIZE      = 0x0100, /* RC_VER1 */
> +       TPM2_RC_DISABLED        = 0x0120,
> +       TPM2_RC_TESTING         = 0x090A, /* RC_WARN */
> +       TPM2_RC_REFERENCE_H0    = 0x0910,
> +       TPM2_RC_LOCKOUT         = 0x0921,
> +};
> +
>  enum tpm_physical_presence {
>         TPM_PHYSICAL_PRESENCE_HW_DISABLE        = 0x0200,
>         TPM_PHYSICAL_PRESENCE_CMD_DISABLE       = 0x0100,
> @@ -445,7 +469,7 @@ int tpm_get_specification(void);
>   * @param mode         TPM startup mode
>   * @return return code of the operation
>   */
> -uint32_t tpm_startup(enum tpm_startup_type mode);
> +int tpm_startup(enum tpm_startup_type mode);

How come the change to an int? It's fine, I just want to understand it.

>
>  /**
>   * Issue a TPM_SelfTestFull command.
> diff --git a/lib/tpm.c b/lib/tpm.c
> index c0fbba86ff..0c57ef8dc7 100644
> --- a/lib/tpm.c
> +++ b/lib/tpm.c
> @@ -310,20 +310,35 @@ int tpm_init(void)
>         return tpm_open(dev);
>  }
>
> -uint32_t tpm_startup(enum tpm_startup_type mode)
> +int tpm_startup(enum tpm_startup_type mode)
>  {
> -       const uint8_t command[12] = {
> -               0x0, 0xc1, 0x0, 0x0, 0x0, 0xc, 0x0, 0x0, 0x0, 0x99, 0x0, 0x0,
> +       const u8 command_v1[12] = {
> +               U16_TO_ARRAY(0xc1),
> +               U32_TO_ARRAY(12),
> +               U32_TO_ARRAY(0x99),
> +               U16_TO_ARRAY(mode),
>         };
> -       const size_t mode_offset = 10;
> -       uint8_t buf[COMMAND_BUFFER_SIZE];
> +       const u8 command_v2[12] = {
> +               U16_TO_ARRAY(TPM2_ST_NO_SESSIONS),
> +               U32_TO_ARRAY(12),
> +               U32_TO_ARRAY(TPM2_CC_STARTUP),
> +               U16_TO_ARRAY(mode),
> +       };
> +       int ret;
> +
> +       if (!is_tpmv2)
> +               return tpm_sendrecv_command(command_v1, NULL, NULL);
> +
> +       ret = tpm_sendrecv_command(command_v2, NULL, NULL);

This doesn't seem better (than the pack_byte thing) to me. What is the benefit?

>
> -       if (pack_byte_string(buf, sizeof(buf), "sw",
> -                               0, command, sizeof(command),
> -                               mode_offset, mode))
> -               return TPM_LIB_ERROR;
> +       /*
> +        * Note TPMv2: STARTUP command will return RC_SUCCESS the first time,
> +        * but will return RC_INITIALIZE otherwise.
> +        */
> +       if (ret && ret != TPM2_RC_INITIALIZE)
> +               return ret;
>
> -       return tpm_sendrecv_command(buf, NULL, NULL);
> +       return 0;
>  }
>
>  uint32_t tpm_self_test_full(void)
> --
> 2.14.1
>

Regards,
Simon

[U-Boot] [PATCH v2 10/19] tpm: add TPM2_SelfTest command support

[Simon Glass]

Hi Miquel,

On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Add support for the TPM2_Selftest command.
>
> Change the command file and the help accordingly.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  include/tpm.h |  9 +++++++--
>  lib/tpm.c     | 36 ++++++++++++++++++++++++++++--------
>  2 files changed, 35 insertions(+), 10 deletions(-)
>
> diff --git a/include/tpm.h b/include/tpm.h
> index ba71bac885..38d7cb899d 100644
> --- a/include/tpm.h
> +++ b/include/tpm.h
> @@ -31,6 +31,11 @@ enum tpm2_structures {
>         TPM2_ST_SESSIONS        = 0x8002,
>  };
>
> +enum tpm2_yes_no {
> +       TPMI_YES                = 1,
> +       TPMI_NO                 = 0,
> +};
> +
>  enum tpm_startup_type {
>         TPM_ST_CLEAR            = 0x0001,
>         TPM_ST_STATE            = 0x0002,
> @@ -476,14 +481,14 @@ int tpm_startup(enum tpm_startup_type mode);
>   *
>   * @return return code of the operation
>   */
> -uint32_t tpm_self_test_full(void);
> +int tpm_self_test_full(void);
>
>  /**
>   * Issue a TPM_ContinueSelfTest command.
>   *
>   * @return return code of the operation
>   */
> -uint32_t tpm_continue_self_test(void);
> +int tpm_continue_self_test(void);
>
>  /**
>   * Issue a TPM_NV_DefineSpace command.  The implementation is limited
> diff --git a/lib/tpm.c b/lib/tpm.c
> index 0c57ef8dc7..3cc2888267 100644
> --- a/lib/tpm.c
> +++ b/lib/tpm.c
> @@ -341,20 +341,40 @@ int tpm_startup(enum tpm_startup_type mode)
>         return 0;
>  }
>
> -uint32_t tpm_self_test_full(void)
> +int tpm_self_test_full(void)
>  {
> -       const uint8_t command[10] = {
> -               0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x50,
> +       const u8 command_v1[10] = {
> +               U16_TO_ARRAY(0xc1),

Here I can see some benefit to your macros because the data is better
structured. But why not use the pack_byte_string() function?

> +               U32_TO_ARRAY(10),
> +               U32_TO_ARRAY(0x50),
>         };
> -       return tpm_sendrecv_command(command, NULL, NULL);
> +       const u8 command_v2[12] = {
> +               U16_TO_ARRAY(TPM2_ST_NO_SESSIONS),
> +               U32_TO_ARRAY(11),
> +               U32_TO_ARRAY(TPM2_CC_SELF_TEST),
> +               TPMI_YES,
> +       };
> +
> +       return tpm_sendrecv_command(is_tpmv2 ? command_v2 : command_v1, NULL,
> +                                   NULL);
>  }
>
> -uint32_t tpm_continue_self_test(void)
> +int tpm_continue_self_test(void)
>  {
> -       const uint8_t command[10] = {
> -               0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x53,
> +       const u8 command_v1[10] = {
> +               U16_TO_ARRAY(0xc1),
> +               U32_TO_ARRAY(10),
> +               U32_TO_ARRAY(0x53),
>         };
> -       return tpm_sendrecv_command(command, NULL, NULL);
> +       const u8 command_v2[12] = {
> +               U16_TO_ARRAY(TPM2_ST_NO_SESSIONS),
> +               U32_TO_ARRAY(11),
> +               U32_TO_ARRAY(TPM2_CC_SELF_TEST),
> +               TPMI_NO,
> +       };
> +
> +       return tpm_sendrecv_command(is_tpmv2 ? command_v2 : command_v1, NULL,
> +                                   NULL);
>  }
>
>  uint32_t tpm_nv_define_space(uint32_t index, uint32_t perm, uint32_t size)
> --
> 2.14.1
>
Regards,
Simon

[U-Boot] [PATCH v2 11/19] tpm: add TPM2_Clear command support

[Simon Glass]

Hi Miquel,

On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Add support for the TPM2_Clear command.
>
> Change the command file and the help accordingly.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  cmd/tpm.c      | 29 ++++++++++++++++++++++++++---
>  cmd/tpm_test.c |  6 +++---
>  include/tpm.h  | 21 +++++++++++++++++----
>  lib/tpm.c      | 42 ++++++++++++++++++++++++++++++++++++++----
>  4 files changed, 84 insertions(+), 14 deletions(-)
>
> diff --git a/cmd/tpm.c b/cmd/tpm.c
> index fc9ef9d4a3..32921e1a70 100644
> --- a/cmd/tpm.c
> +++ b/cmd/tpm.c
> @@ -454,6 +454,29 @@ static int do_tpm_init(cmd_tbl_t *cmdtp, int flag,
>         return report_return_code(tpm_init());
>  }
>
> +static int do_tpm_force_clear(cmd_tbl_t *cmdtp, int flag,
> +                             int argc, char * const argv[])
> +{
> +       u32 handle = 0;
> +       const char *pw = (argc < 3) ? NULL : argv[2];
> +       const ssize_t pw_sz = pw ? strlen(pw) : 0;
> +
> +       if (argc < 2 || argc > 3)
> +               return CMD_RET_USAGE;
> +
> +       if (pw_sz > TPM2_DIGEST_LENGTH)
> +               return -EINVAL;
> +
> +       if (!strcasecmp("TPM2_RH_LOCKOUT", argv[1]))
> +               handle = TPM2_RH_LOCKOUT;
> +       else if (!strcasecmp("TPM2_RH_PLATFORM", argv[1]))
> +               handle = TPM2_RH_PLATFORM;
> +       else
> +               return CMD_RET_USAGE;
> +
> +       return report_return_code(tpm_force_clear(handle, pw, pw_sz));
> +}
> +
>  #define TPM_COMMAND_NO_ARG(cmd)                                \
>  static int do_##cmd(cmd_tbl_t *cmdtp, int flag,                \
>                 int argc, char * const argv[])          \
> @@ -465,7 +488,6 @@ static int do_##cmd(cmd_tbl_t *cmdtp, int flag,             \
>
>  TPM_COMMAND_NO_ARG(tpm_self_test_full)
>  TPM_COMMAND_NO_ARG(tpm_continue_self_test)
> -TPM_COMMAND_NO_ARG(tpm_force_clear)
>  TPM_COMMAND_NO_ARG(tpm_physical_enable)
>  TPM_COMMAND_NO_ARG(tpm_physical_disable)
>
> @@ -951,8 +973,9 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
>  "  physical_set_deactivated 0|1\n"
>  "    - Set deactivated flag.\n"
>  "Admin Ownership Commands:\n"
> -"  force_clear\n"
> -"    - Issue TPM_ForceClear command.\n"
> +"  force_clear [<type>]\n"
> +"    - Issue TPM_[Force]Clear command, with <type> one of (TPMv2 only):\n"
> +"          * TPM2_RH_LOCKOUT, TPM2_RH_PLATFORM.\n"
>  "  tsc_physical_presence flags\n"
>  "    - Set TPM device's Physical Presence flags to <flags>.\n"
>  "The Capability Commands:\n"
> diff --git a/cmd/tpm_test.c b/cmd/tpm_test.c
> index 37ad2ff33d..da40dbc423 100644
> --- a/cmd/tpm_test.c
> +++ b/cmd/tpm_test.c
> @@ -176,7 +176,7 @@ static int test_fast_enable(void)
>         TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL));
>         printf("\tdisable is %d, deactivated is %d\n", disable, deactivated);
>         for (i = 0; i < 2; i++) {
> -               TPM_CHECK(tpm_force_clear());
> +               TPM_CHECK(tpm_force_clear(0, NULL, 0));
>                 TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL));
>                 printf("\tdisable is %d, deactivated is %d\n", disable,
>                        deactivated);
> @@ -458,7 +458,7 @@ static int test_write_limit(void)
>         TPM_CHECK(TlclStartupIfNeeded());
>         TPM_CHECK(tpm_self_test_full());
>         TPM_CHECK(tpm_tsc_physical_presence(PRESENCE));
> -       TPM_CHECK(tpm_force_clear());
> +       TPM_CHECK(tpm_force_clear(0, NULL, 0));
>         TPM_CHECK(tpm_physical_enable());
>         TPM_CHECK(tpm_physical_set_deactivated(0));
>
> @@ -477,7 +477,7 @@ static int test_write_limit(void)
>         }
>
>         /* Reset write count */
> -       TPM_CHECK(tpm_force_clear());
> +       TPM_CHECK(tpm_force_clear(0, NULL, 0));
>         TPM_CHECK(tpm_physical_enable());
>         TPM_CHECK(tpm_physical_set_deactivated(0));
>
> diff --git a/include/tpm.h b/include/tpm.h
> index 38d7cb899d..2f17166662 100644
> --- a/include/tpm.h
> +++ b/include/tpm.h
> @@ -43,13 +43,23 @@ enum tpm_startup_type {
>  };
>
>  enum tpm2_startup_types {
> -       TPM2_SU_CLEAR   = 0x0000,
> -       TPM2_SU_STATE   = 0x0001,
> +       TPM2_SU_CLEAR           = 0x0000,
> +       TPM2_SU_STATE           = 0x0001,
> +};
> +
> +enum tpm2_handles {

Please add comment to enum

> +       TPM2_RH_OWNER           = 0x40000001,
> +       TPM2_RS_PW              = 0x40000009,
> +       TPM2_RH_LOCKOUT         = 0x4000000A,
> +       TPM2_RH_ENDORSEMENT     = 0x4000000B,
> +       TPM2_RH_PLATFORM        = 0x4000000C,
>  };
>
>  enum tpm2_command_codes {
>         TPM2_CC_STARTUP         = 0x0144,
>         TPM2_CC_SELF_TEST       = 0x0143,
> +       TPM2_CC_CLEAR           = 0x0126,
> +       TPM2_CC_CLEARCONTROL    = 0x0127,
>         TPM2_CC_GET_CAPABILITY  = 0x017A,
>         TPM2_CC_PCR_READ        = 0x017E,
>         TPM2_CC_PCR_EXTEND      = 0x0182,
> @@ -567,11 +577,14 @@ uint32_t tpm_tsc_physical_presence(uint16_t presence);
>  uint32_t tpm_read_pubek(void *data, size_t count);
>
>  /**
> - * Issue a TPM_ForceClear command.
> + * Issue a TPM_ForceClear or a TPM2_Clear command.
>   *
> + * @param handle       Handle
> + * @param pw           Password
> + * @param pw_sz                Length of the password
>   * @return return code of the operation
>   */
> -uint32_t tpm_force_clear(void);
> +int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz);
>
>  /**
>   * Issue a TPM_PhysicalEnable command.
> diff --git a/lib/tpm.c b/lib/tpm.c
> index 3cc2888267..9a46ac09e6 100644
> --- a/lib/tpm.c
> +++ b/lib/tpm.c
> @@ -608,13 +608,47 @@ uint32_t tpm_read_pubek(void *data, size_t count)
>         return 0;
>  }
>
> -uint32_t tpm_force_clear(void)
> +int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz)
>  {
> -       const uint8_t command[10] = {
> -               0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x5d,
> +       const u8 command_v1[10] = {
> +               U16_TO_ARRAY(0xc1),
> +               U32_TO_ARRAY(10),
> +               U32_TO_ARRAY(0x5d),
>         };
> +       u8 command_v2[COMMAND_BUFFER_SIZE] = {
> +               U16_TO_ARRAY(TPM2_ST_SESSIONS), /* TAG */
> +               U32_TO_ARRAY(27 + pw_sz),       /* Length */
> +               U32_TO_ARRAY(TPM2_CC_CLEAR),    /* Command code */

Here we have both v1 and v2 commands. Perhaps we should have a Kconfig
option to enable v2 support? Or do you think it is not a big addition
in terms of code side?

One way to do this would be to have an inline function which can tell
if you are using v2:

static inline bool tpm_v2(void) {
   return IS_ENABLED(CONFIG_TPM_V2) && further things...
}

static inline bool tpm_v1(void) {
   return IS_ENABLED(CONFIG_TPM_V1) && further things...
}

>
> -       return tpm_sendrecv_command(command, NULL, NULL);
> +               /* HANDLE */
> +               U32_TO_ARRAY(handle),           /* TPM resource handle */

If we really need these, perhaps tpm_u32() is a better name that
U32_TO_ARRAY() ?

> +
> +               /* AUTH_SESSION */
> +               U32_TO_ARRAY(9 + pw_sz),        /* Authorization size */
> +               U32_TO_ARRAY(TPM2_RS_PW),       /* Session handle */
> +               U16_TO_ARRAY(0),                /* Size of <nonce> */
> +                                               /* <nonce> (if any) */
> +               0,                              /* Attributes: Cont/Excl/Rst */
> +               U16_TO_ARRAY(pw_sz),            /* Size of <hmac/password> */
> +               /* STRING(pw)                      <hmac/password> (if any) */
> +       };
> +       unsigned int offset = 27;
> +       int ret;
> +
> +       if (!is_tpmv2)
> +               tpm_sendrecv_command(command_v1, NULL, NULL);
> +
> +       /*
> +        * Fill the command structure starting from the first buffer:
> +        *     - the password (if any)
> +        */
> +       ret = pack_byte_string(command_v2, sizeof(command_v2), "s",
> +                              offset, pw, pw_sz);
> +       offset += pw_sz;
> +       if (ret)
> +               return TPM_LIB_ERROR;
> +
> +       return tpm_sendrecv_command(command_v2, NULL, NULL);
>  }
>
>  uint32_t tpm_physical_enable(void)
> --
> 2.14.1
>

Regards,
Simon

[U-Boot] [PATCH v2 18/19] tpm: add PCR authentication commands support

[Simon Glass]

Hi Miquel,

On 29 March 2018 at 15:44, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Add support for the TPM2_PCR_SetAuthPolicy and
> TPM2_PCR_SetAuthValue commands.
>
> Change the command file and the help accordingly.
>
> Note: These commands could not be tested because the TPMs available
> do not support them, however they could be useful for someone else.
> The user is warned by the command help.

You should be able to test them by adding sandbox support.

>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  cmd/tpm.c     |  49 +++++++++++++++++++++++++++
>  include/tpm.h |  29 ++++++++++++++++
>  lib/tpm.c     | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 184 insertions(+)
>

Regards,
Simon

[U-Boot] [PATCH v2 06/19] tpm: add macros for TPMv2 commands

[Simon Glass]

Hi Miquel,

On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> TPM commands are much easier to handle with these macros that will
> transform words or integers into byte string. This way, there is no need
> to call pack_byte_string() while all variable length in a command are
> known (and at must 4 bytes, which is a lot of them).
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  lib/tpm.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/lib/tpm.c b/lib/tpm.c
> index 38b76b4961..2b329145be 100644
> --- a/lib/tpm.c
> +++ b/lib/tpm.c
> @@ -15,6 +15,12 @@
>  /* Internal error of TPM command library */
>  #define TPM_LIB_ERROR  ((uint32_t)~0u)
>
> +/* To make strings of commands more easily */
> +#define __MSB(x) ((x) >> 8)
> +#define __LSB(x) ((x) & 0xFF)
> +#define U16_TO_ARRAY(x) __MSB(x), __LSB(x)
> +#define U32_TO_ARRAY(x) U16_TO_ARRAY((x) >> 16), U16_TO_ARRAY((x) & 0xFFFF)
> +
>  /* Global boolean to discriminate TPMv2.x from TPMv1.x functions */
>  static bool is_tpmv2;
>
> --
> 2.14.1
>

See my later comments about these macros.

Regards,
Simon

[U-Boot] [PATCH v2 10/19] tpm: add TPM2_SelfTest command support

[Miquel Raynal]

Hi Simon,

I am back on that topic, let me answer some of your questions before
addressing them in a next version.

On Fri, 30 Mar 2018 06:42:25 +0800, Simon Glass
<sjg@chromium.org> wrote:

> Hi Miquel,
> 
> On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > Add support for the TPM2_Selftest command.
> >
> > Change the command file and the help accordingly.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  include/tpm.h |  9 +++++++--
> >  lib/tpm.c     | 36 ++++++++++++++++++++++++++++--------
> >  2 files changed, 35 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/tpm.h b/include/tpm.h
> > index ba71bac885..38d7cb899d 100644
> > --- a/include/tpm.h
> > +++ b/include/tpm.h
> > @@ -31,6 +31,11 @@ enum tpm2_structures {
> >         TPM2_ST_SESSIONS        = 0x8002,
> >  };
> >
> > +enum tpm2_yes_no {
> > +       TPMI_YES                = 1,
> > +       TPMI_NO                 = 0,
> > +};
> > +
> >  enum tpm_startup_type {
> >         TPM_ST_CLEAR            = 0x0001,
> >         TPM_ST_STATE            = 0x0002,
> > @@ -476,14 +481,14 @@ int tpm_startup(enum tpm_startup_type mode);
> >   *
> >   * @return return code of the operation
> >   */
> > -uint32_t tpm_self_test_full(void);
> > +int tpm_self_test_full(void);
> >
> >  /**
> >   * Issue a TPM_ContinueSelfTest command.
> >   *
> >   * @return return code of the operation
> >   */
> > -uint32_t tpm_continue_self_test(void);
> > +int tpm_continue_self_test(void);
> >
> >  /**
> >   * Issue a TPM_NV_DefineSpace command.  The implementation is limited
> > diff --git a/lib/tpm.c b/lib/tpm.c
> > index 0c57ef8dc7..3cc2888267 100644
> > --- a/lib/tpm.c
> > +++ b/lib/tpm.c
> > @@ -341,20 +341,40 @@ int tpm_startup(enum tpm_startup_type mode)
> >         return 0;
> >  }
> >
> > -uint32_t tpm_self_test_full(void)
> > +int tpm_self_test_full(void)
> >  {
> > -       const uint8_t command[10] = {
> > -               0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x50,
> > +       const u8 command_v1[10] = {
> > +               U16_TO_ARRAY(0xc1),  
> 
> Here I can see some benefit to your macros because the data is better
> structured. But why not use the pack_byte_string() function?

TPM buffers (1.0 and 2.0) are, most of the time, filled with data of
known length (1, 2 or 4 bytes). You can see that most of the time,
TPMv1 simple commands were just filling a buffer of bytes. I don't like
very much the fact that the user should split the data in byte array so
I introduced these macros that do it for me in a cleaner way. When you
get an hexadecimal value (like it was the case before) it is easy to
split a "0xabcd" in "0xab, 0xcd", but I prefer to use variables or
simply defines sometimes and writing "TPM2_ST_NO_SESSIONS" as
"TPM2_ST_NO_SESSIONS >> 8, TPM2_ST_NO_SESSIONS & 0xFF" is not readable
at all.

Now why did I not use the existing pack_byte_string() function. Well,
at first I did and realized it was a pain to have a decent
incrementation of the offsets, mostly when commands tend to be longer
and longer. Having such lists of offset/variable/length while you could
define them statically on the stack -which also saves some computing
time- is quickly annoying and hard to review. From my point of view this
function is a real asset when it comes to 'unknown'/'big' variable
length (like a password, an hmac, an user input, etc) but should be
avoided when not necessary: typically to fill a buffer with defined
values.

I am of course very open on the topic, this is my feeling but I don't
have that much experience and would carefully listen to yours.

Thank you,
Miquèl

> 
> > +               U32_TO_ARRAY(10),
> > +               U32_TO_ARRAY(0x50),
> >         };
> > -       return tpm_sendrecv_command(command, NULL, NULL);
> > +       const u8 command_v2[12] = {
> > +               U16_TO_ARRAY(TPM2_ST_NO_SESSIONS),
> > +               U32_TO_ARRAY(11),
> > +               U32_TO_ARRAY(TPM2_CC_SELF_TEST),
> > +               TPMI_YES,
> > +       };
> > +
> > +       return tpm_sendrecv_command(is_tpmv2 ? command_v2 : command_v1, NULL,
> > +                                   NULL);
> >  }
> >

-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[U-Boot] [PATCH v2 03/19] tpm: add support for TPMv2 SPI modules

[Miquel Raynal]

Hi Simon,

On Fri, 30 Mar 2018 06:41:52 +0800, Simon Glass <sjg@chromium.org>
wrote:

> Hi Miquel,
> 
> On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > Add the tpm_tis_spi driver that should support any TPMv2 compliant (SPI)
> > module.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/tpm/Kconfig       |   9 +
> >  drivers/tpm/Makefile      |   1 +
> >  drivers/tpm/tpm_tis.h     |   3 +
> >  drivers/tpm/tpm_tis_spi.c | 656 ++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 669 insertions(+)
> >  create mode 100644 drivers/tpm/tpm_tis_spi.c  
> 
> I think this came up in another context. Would it make sense to create
> a common interface to i2c and SPI and then have a common driver?

I hesitated to do it (even started to write down some common code), and
finally there was not so much of it, I was not sure if it would bring
something more than obfuscation so I chose not to add an extra layer as
I had currently only one SPI chip and no I2C chip to check the
architecture. Maybe the question should be asked again when someone
will want I2C support.

Regards,
Miquèl

[U-Boot] [PATCH v2 11/19] tpm: add TPM2_Clear command support

[Miquel Raynal]

Hi Simon,

On Fri, 30 Mar 2018 06:42:32 +0800, Simon Glass <sjg@chromium.org>
wrote:

> Hi Miquel,
> 
> On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > Add support for the TPM2_Clear command.
> >
> > Change the command file and the help accordingly.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  cmd/tpm.c      | 29 ++++++++++++++++++++++++++---
> >  cmd/tpm_test.c |  6 +++---
> >  include/tpm.h  | 21 +++++++++++++++++----
> >  lib/tpm.c      | 42 ++++++++++++++++++++++++++++++++++++++----
> >  4 files changed, 84 insertions(+), 14 deletions(-)
> >
> > diff --git a/cmd/tpm.c b/cmd/tpm.c
> > index fc9ef9d4a3..32921e1a70 100644
> > --- a/cmd/tpm.c
> > +++ b/cmd/tpm.c
> > @@ -454,6 +454,29 @@ static int do_tpm_init(cmd_tbl_t *cmdtp, int flag,
> >         return report_return_code(tpm_init());
> >  }
> >
> > +static int do_tpm_force_clear(cmd_tbl_t *cmdtp, int flag,
> > +                             int argc, char * const argv[])
> > +{
> > +       u32 handle = 0;
> > +       const char *pw = (argc < 3) ? NULL : argv[2];
> > +       const ssize_t pw_sz = pw ? strlen(pw) : 0;
> > +
> > +       if (argc < 2 || argc > 3)
> > +               return CMD_RET_USAGE;
> > +
> > +       if (pw_sz > TPM2_DIGEST_LENGTH)
> > +               return -EINVAL;
> > +
> > +       if (!strcasecmp("TPM2_RH_LOCKOUT", argv[1]))
> > +               handle = TPM2_RH_LOCKOUT;
> > +       else if (!strcasecmp("TPM2_RH_PLATFORM", argv[1]))
> > +               handle = TPM2_RH_PLATFORM;
> > +       else
> > +               return CMD_RET_USAGE;
> > +
> > +       return report_return_code(tpm_force_clear(handle, pw, pw_sz));
> > +}
> > +
> >  #define TPM_COMMAND_NO_ARG(cmd)                                \
> >  static int do_##cmd(cmd_tbl_t *cmdtp, int flag,                \
> >                 int argc, char * const argv[])          \
> > @@ -465,7 +488,6 @@ static int do_##cmd(cmd_tbl_t *cmdtp, int flag,             \
> >
> >  TPM_COMMAND_NO_ARG(tpm_self_test_full)
> >  TPM_COMMAND_NO_ARG(tpm_continue_self_test)
> > -TPM_COMMAND_NO_ARG(tpm_force_clear)
> >  TPM_COMMAND_NO_ARG(tpm_physical_enable)
> >  TPM_COMMAND_NO_ARG(tpm_physical_disable)
> >
> > @@ -951,8 +973,9 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
> >  "  physical_set_deactivated 0|1\n"
> >  "    - Set deactivated flag.\n"
> >  "Admin Ownership Commands:\n"
> > -"  force_clear\n"
> > -"    - Issue TPM_ForceClear command.\n"
> > +"  force_clear [<type>]\n"
> > +"    - Issue TPM_[Force]Clear command, with <type> one of (TPMv2 only):\n"
> > +"          * TPM2_RH_LOCKOUT, TPM2_RH_PLATFORM.\n"
> >  "  tsc_physical_presence flags\n"
> >  "    - Set TPM device's Physical Presence flags to <flags>.\n"
> >  "The Capability Commands:\n"
> > diff --git a/cmd/tpm_test.c b/cmd/tpm_test.c
> > index 37ad2ff33d..da40dbc423 100644
> > --- a/cmd/tpm_test.c
> > +++ b/cmd/tpm_test.c
> > @@ -176,7 +176,7 @@ static int test_fast_enable(void)
> >         TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL));
> >         printf("\tdisable is %d, deactivated is %d\n", disable, deactivated);
> >         for (i = 0; i < 2; i++) {
> > -               TPM_CHECK(tpm_force_clear());
> > +               TPM_CHECK(tpm_force_clear(0, NULL, 0));
> >                 TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL));
> >                 printf("\tdisable is %d, deactivated is %d\n", disable,
> >                        deactivated);
> > @@ -458,7 +458,7 @@ static int test_write_limit(void)
> >         TPM_CHECK(TlclStartupIfNeeded());
> >         TPM_CHECK(tpm_self_test_full());
> >         TPM_CHECK(tpm_tsc_physical_presence(PRESENCE));
> > -       TPM_CHECK(tpm_force_clear());
> > +       TPM_CHECK(tpm_force_clear(0, NULL, 0));
> >         TPM_CHECK(tpm_physical_enable());
> >         TPM_CHECK(tpm_physical_set_deactivated(0));
> >
> > @@ -477,7 +477,7 @@ static int test_write_limit(void)
> >         }
> >
> >         /* Reset write count */
> > -       TPM_CHECK(tpm_force_clear());
> > +       TPM_CHECK(tpm_force_clear(0, NULL, 0));
> >         TPM_CHECK(tpm_physical_enable());
> >         TPM_CHECK(tpm_physical_set_deactivated(0));
> >
> > diff --git a/include/tpm.h b/include/tpm.h
> > index 38d7cb899d..2f17166662 100644
> > --- a/include/tpm.h
> > +++ b/include/tpm.h
> > @@ -43,13 +43,23 @@ enum tpm_startup_type {
> >  };
> >
> >  enum tpm2_startup_types {
> > -       TPM2_SU_CLEAR   = 0x0000,
> > -       TPM2_SU_STATE   = 0x0001,
> > +       TPM2_SU_CLEAR           = 0x0000,
> > +       TPM2_SU_STATE           = 0x0001,
> > +};
> > +
> > +enum tpm2_handles {  
> 
> Please add comment to enum

Fine, I will document all of them. Just for you to know, these are
values extracted from the (publicly available) specification, I did
not change any of them.

> 
> > +       TPM2_RH_OWNER           = 0x40000001,
> > +       TPM2_RS_PW              = 0x40000009,
> > +       TPM2_RH_LOCKOUT         = 0x4000000A,
> > +       TPM2_RH_ENDORSEMENT     = 0x4000000B,
> > +       TPM2_RH_PLATFORM        = 0x4000000C,
> >  };
> >
> >  enum tpm2_command_codes {
> >         TPM2_CC_STARTUP         = 0x0144,
> >         TPM2_CC_SELF_TEST       = 0x0143,
> > +       TPM2_CC_CLEAR           = 0x0126,
> > +       TPM2_CC_CLEARCONTROL    = 0x0127,
> >         TPM2_CC_GET_CAPABILITY  = 0x017A,
> >         TPM2_CC_PCR_READ        = 0x017E,
> >         TPM2_CC_PCR_EXTEND      = 0x0182,
> > @@ -567,11 +577,14 @@ uint32_t tpm_tsc_physical_presence(uint16_t presence);
> >  uint32_t tpm_read_pubek(void *data, size_t count);
> >
> >  /**
> > - * Issue a TPM_ForceClear command.
> > + * Issue a TPM_ForceClear or a TPM2_Clear command.
> >   *
> > + * @param handle       Handle
> > + * @param pw           Password
> > + * @param pw_sz                Length of the password
> >   * @return return code of the operation
> >   */
> > -uint32_t tpm_force_clear(void);
> > +int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz);
> >
> >  /**
> >   * Issue a TPM_PhysicalEnable command.
> > diff --git a/lib/tpm.c b/lib/tpm.c
> > index 3cc2888267..9a46ac09e6 100644
> > --- a/lib/tpm.c
> > +++ b/lib/tpm.c
> > @@ -608,13 +608,47 @@ uint32_t tpm_read_pubek(void *data, size_t count)
> >         return 0;
> >  }
> >
> > -uint32_t tpm_force_clear(void)
> > +int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz)
> >  {
> > -       const uint8_t command[10] = {
> > -               0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x5d,
> > +       const u8 command_v1[10] = {
> > +               U16_TO_ARRAY(0xc1),
> > +               U32_TO_ARRAY(10),
> > +               U32_TO_ARRAY(0x5d),
> >         };
> > +       u8 command_v2[COMMAND_BUFFER_SIZE] = {
> > +               U16_TO_ARRAY(TPM2_ST_SESSIONS), /* TAG */
> > +               U32_TO_ARRAY(27 + pw_sz),       /* Length */
> > +               U32_TO_ARRAY(TPM2_CC_CLEAR),    /* Command code */  
> 
> Here we have both v1 and v2 commands. Perhaps we should have a Kconfig
> option to enable v2 support? Or do you think it is not a big addition
> in terms of code side?

It is a big addition in terms of code side.

> 
> One way to do this would be to have an inline function which can tell
> if you are using v2:
> 
> static inline bool tpm_v2(void) {
>    return IS_ENABLED(CONFIG_TPM_V2) && further things...
> }
> 
> static inline bool tpm_v1(void) {
>    return IS_ENABLED(CONFIG_TPM_V1) && further things...
> }

I like this way of choosing one specification or the other, I could
make them mutually exclusive. It would prevent the need for a global
variable (or an additional field in uclass).

Would it be fine to actually rename the file (with a 'tpm1' suffix) and
create a new one specific for TPMv2 commands ? Only one file would be
compiled/linked depending on the configuration, avoiding the possibility
to call a v1 command from a v2 chip and vice versa.

At first I thought a lot of code would be shared but I don't think we
would add so much by having one function per revision now.

> 
> >
> > -       return tpm_sendrecv_command(command, NULL, NULL);
> > +               /* HANDLE */
> > +               U32_TO_ARRAY(handle),           /* TPM resource handle */  
> 
> If we really need these, perhaps tpm_u32() is a better name that
> U32_TO_ARRAY() ?
> 
> > +
> > +               /* AUTH_SESSION */
> > +               U32_TO_ARRAY(9 + pw_sz),        /* Authorization size */
> > +               U32_TO_ARRAY(TPM2_RS_PW),       /* Session handle */
> > +               U16_TO_ARRAY(0),                /* Size of <nonce> */
> > +                                               /* <nonce> (if any) */
> > +               0,                              /* Attributes: Cont/Excl/Rst */
> > +               U16_TO_ARRAY(pw_sz),            /* Size of <hmac/password> */
> > +               /* STRING(pw)                      <hmac/password> (if any) */
> > +       };
> > +       unsigned int offset = 27;
> > +       int ret;
> > +
> > +       if (!is_tpmv2)
> > +               tpm_sendrecv_command(command_v1, NULL, NULL);
> > +
> > +       /*
> > +        * Fill the command structure starting from the first buffer:
> > +        *     - the password (if any)
> > +        */
> > +       ret = pack_byte_string(command_v2, sizeof(command_v2), "s",
> > +                              offset, pw, pw_sz);
> > +       offset += pw_sz;
> > +       if (ret)
> > +               return TPM_LIB_ERROR;
> > +
> > +       return tpm_sendrecv_command(command_v2, NULL, NULL);
> >  }
> >
> >  uint32_t tpm_physical_enable(void)
> > --
> > 2.14.1
> >  
> 
> Regards,
> Simon

Thanks,
Miquèl


-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[U-Boot] [PATCH v2 10/19] tpm: add TPM2_SelfTest command support

[Simon Glass]

Hi Miquel,

On 24 April 2018 at 06:53, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Hi Simon,
>
> I am back on that topic, let me answer some of your questions before
> addressing them in a next version.
>
> On Fri, 30 Mar 2018 06:42:25 +0800, Simon Glass
> <sjg@chromium.org> wrote:
>
>> Hi Miquel,
>>
>> On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>> > Add support for the TPM2_Selftest command.
>> >
>> > Change the command file and the help accordingly.
>> >
>> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>> > ---
>> >  include/tpm.h |  9 +++++++--
>> >  lib/tpm.c     | 36 ++++++++++++++++++++++++++++--------
>> >  2 files changed, 35 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/include/tpm.h b/include/tpm.h
>> > index ba71bac885..38d7cb899d 100644
>> > --- a/include/tpm.h
>> > +++ b/include/tpm.h
>> > @@ -31,6 +31,11 @@ enum tpm2_structures {
>> >         TPM2_ST_SESSIONS        = 0x8002,
>> >  };
>> >
>> > +enum tpm2_yes_no {
>> > +       TPMI_YES                = 1,
>> > +       TPMI_NO                 = 0,
>> > +};
>> > +
>> >  enum tpm_startup_type {
>> >         TPM_ST_CLEAR            = 0x0001,
>> >         TPM_ST_STATE            = 0x0002,
>> > @@ -476,14 +481,14 @@ int tpm_startup(enum tpm_startup_type mode);
>> >   *
>> >   * @return return code of the operation
>> >   */
>> > -uint32_t tpm_self_test_full(void);
>> > +int tpm_self_test_full(void);
>> >
>> >  /**
>> >   * Issue a TPM_ContinueSelfTest command.
>> >   *
>> >   * @return return code of the operation
>> >   */
>> > -uint32_t tpm_continue_self_test(void);
>> > +int tpm_continue_self_test(void);
>> >
>> >  /**
>> >   * Issue a TPM_NV_DefineSpace command.  The implementation is limited
>> > diff --git a/lib/tpm.c b/lib/tpm.c
>> > index 0c57ef8dc7..3cc2888267 100644
>> > --- a/lib/tpm.c
>> > +++ b/lib/tpm.c
>> > @@ -341,20 +341,40 @@ int tpm_startup(enum tpm_startup_type mode)
>> >         return 0;
>> >  }
>> >
>> > -uint32_t tpm_self_test_full(void)
>> > +int tpm_self_test_full(void)
>> >  {
>> > -       const uint8_t command[10] = {
>> > -               0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x50,
>> > +       const u8 command_v1[10] = {
>> > +               U16_TO_ARRAY(0xc1),
>>
>> Here I can see some benefit to your macros because the data is better
>> structured. But why not use the pack_byte_string() function?
>
> TPM buffers (1.0 and 2.0) are, most of the time, filled with data of
> known length (1, 2 or 4 bytes). You can see that most of the time,
> TPMv1 simple commands were just filling a buffer of bytes. I don't like
> very much the fact that the user should split the data in byte array so
> I introduced these macros that do it for me in a cleaner way. When you
> get an hexadecimal value (like it was the case before) it is easy to
> split a "0xabcd" in "0xab, 0xcd", but I prefer to use variables or
> simply defines sometimes and writing "TPM2_ST_NO_SESSIONS" as
> "TPM2_ST_NO_SESSIONS >> 8, TPM2_ST_NO_SESSIONS & 0xFF" is not readable
> at all.
>
> Now why did I not use the existing pack_byte_string() function. Well,
> at first I did and realized it was a pain to have a decent
> incrementation of the offsets, mostly when commands tend to be longer
> and longer. Having such lists of offset/variable/length while you could
> define them statically on the stack -which also saves some computing
> time- is quickly annoying and hard to review. From my point of view this
> function is a real asset when it comes to 'unknown'/'big' variable
> length (like a password, an hmac, an user input, etc) but should be
> avoided when not necessary: typically to fill a buffer with defined
> values.
>
> I am of course very open on the topic, this is my feeling but I don't
> have that much experience and would carefully listen to yours.

I don't think this is an unreasonable point of view. Either works.

But if you are changing the approach to use static data, should you
convert the existing code to the new scheme?

Regards,
Simon

[U-Boot] [PATCH v2 11/19] tpm: add TPM2_Clear command support

[Simon Glass]

Hi Miquel,

On 24 April 2018 at 07:17, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Hi Simon,
>
> On Fri, 30 Mar 2018 06:42:32 +0800, Simon Glass <sjg@chromium.org>
> wrote:
>
>> Hi Miquel,
>>
>> On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>> > Add support for the TPM2_Clear command.
>> >
>> > Change the command file and the help accordingly.
>> >
>> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>> > ---
>> >  cmd/tpm.c      | 29 ++++++++++++++++++++++++++---
>> >  cmd/tpm_test.c |  6 +++---
>> >  include/tpm.h  | 21 +++++++++++++++++----
>> >  lib/tpm.c      | 42 ++++++++++++++++++++++++++++++++++++++----
>> >  4 files changed, 84 insertions(+), 14 deletions(-)
>> >
>> > diff --git a/cmd/tpm.c b/cmd/tpm.c
>> > index fc9ef9d4a3..32921e1a70 100644
>> > --- a/cmd/tpm.c
>> > +++ b/cmd/tpm.c
>> > @@ -454,6 +454,29 @@ static int do_tpm_init(cmd_tbl_t *cmdtp, int flag,
>> >         return report_return_code(tpm_init());
>> >  }
>> >
>> > +static int do_tpm_force_clear(cmd_tbl_t *cmdtp, int flag,
>> > +                             int argc, char * const argv[])
>> > +{
>> > +       u32 handle = 0;
>> > +       const char *pw = (argc < 3) ? NULL : argv[2];
>> > +       const ssize_t pw_sz = pw ? strlen(pw) : 0;
>> > +
>> > +       if (argc < 2 || argc > 3)
>> > +               return CMD_RET_USAGE;
>> > +
>> > +       if (pw_sz > TPM2_DIGEST_LENGTH)
>> > +               return -EINVAL;
>> > +
>> > +       if (!strcasecmp("TPM2_RH_LOCKOUT", argv[1]))
>> > +               handle = TPM2_RH_LOCKOUT;
>> > +       else if (!strcasecmp("TPM2_RH_PLATFORM", argv[1]))
>> > +               handle = TPM2_RH_PLATFORM;
>> > +       else
>> > +               return CMD_RET_USAGE;
>> > +
>> > +       return report_return_code(tpm_force_clear(handle, pw, pw_sz));
>> > +}
>> > +
>> >  #define TPM_COMMAND_NO_ARG(cmd)                                \
>> >  static int do_##cmd(cmd_tbl_t *cmdtp, int flag,                \
>> >                 int argc, char * const argv[])          \
>> > @@ -465,7 +488,6 @@ static int do_##cmd(cmd_tbl_t *cmdtp, int flag,             \
>> >
>> >  TPM_COMMAND_NO_ARG(tpm_self_test_full)
>> >  TPM_COMMAND_NO_ARG(tpm_continue_self_test)
>> > -TPM_COMMAND_NO_ARG(tpm_force_clear)
>> >  TPM_COMMAND_NO_ARG(tpm_physical_enable)
>> >  TPM_COMMAND_NO_ARG(tpm_physical_disable)
>> >
>> > @@ -951,8 +973,9 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
>> >  "  physical_set_deactivated 0|1\n"
>> >  "    - Set deactivated flag.\n"
>> >  "Admin Ownership Commands:\n"
>> > -"  force_clear\n"
>> > -"    - Issue TPM_ForceClear command.\n"
>> > +"  force_clear [<type>]\n"
>> > +"    - Issue TPM_[Force]Clear command, with <type> one of (TPMv2 only):\n"
>> > +"          * TPM2_RH_LOCKOUT, TPM2_RH_PLATFORM.\n"
>> >  "  tsc_physical_presence flags\n"
>> >  "    - Set TPM device's Physical Presence flags to <flags>.\n"
>> >  "The Capability Commands:\n"
>> > diff --git a/cmd/tpm_test.c b/cmd/tpm_test.c
>> > index 37ad2ff33d..da40dbc423 100644
>> > --- a/cmd/tpm_test.c
>> > +++ b/cmd/tpm_test.c
>> > @@ -176,7 +176,7 @@ static int test_fast_enable(void)
>> >         TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL));
>> >         printf("\tdisable is %d, deactivated is %d\n", disable, deactivated);
>> >         for (i = 0; i < 2; i++) {
>> > -               TPM_CHECK(tpm_force_clear());
>> > +               TPM_CHECK(tpm_force_clear(0, NULL, 0));
>> >                 TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL));
>> >                 printf("\tdisable is %d, deactivated is %d\n", disable,
>> >                        deactivated);
>> > @@ -458,7 +458,7 @@ static int test_write_limit(void)
>> >         TPM_CHECK(TlclStartupIfNeeded());
>> >         TPM_CHECK(tpm_self_test_full());
>> >         TPM_CHECK(tpm_tsc_physical_presence(PRESENCE));
>> > -       TPM_CHECK(tpm_force_clear());
>> > +       TPM_CHECK(tpm_force_clear(0, NULL, 0));
>> >         TPM_CHECK(tpm_physical_enable());
>> >         TPM_CHECK(tpm_physical_set_deactivated(0));
>> >
>> > @@ -477,7 +477,7 @@ static int test_write_limit(void)
>> >         }
>> >
>> >         /* Reset write count */
>> > -       TPM_CHECK(tpm_force_clear());
>> > +       TPM_CHECK(tpm_force_clear(0, NULL, 0));
>> >         TPM_CHECK(tpm_physical_enable());
>> >         TPM_CHECK(tpm_physical_set_deactivated(0));
>> >
>> > diff --git a/include/tpm.h b/include/tpm.h
>> > index 38d7cb899d..2f17166662 100644
>> > --- a/include/tpm.h
>> > +++ b/include/tpm.h
>> > @@ -43,13 +43,23 @@ enum tpm_startup_type {
>> >  };
>> >
>> >  enum tpm2_startup_types {
>> > -       TPM2_SU_CLEAR   = 0x0000,
>> > -       TPM2_SU_STATE   = 0x0001,
>> > +       TPM2_SU_CLEAR           = 0x0000,
>> > +       TPM2_SU_STATE           = 0x0001,
>> > +};
>> > +
>> > +enum tpm2_handles {
>>
>> Please add comment to enum
>
> Fine, I will document all of them. Just for you to know, these are
> values extracted from the (publicly available) specification, I did
> not change any of them.
>
>>
>> > +       TPM2_RH_OWNER           = 0x40000001,
>> > +       TPM2_RS_PW              = 0x40000009,
>> > +       TPM2_RH_LOCKOUT         = 0x4000000A,
>> > +       TPM2_RH_ENDORSEMENT     = 0x4000000B,
>> > +       TPM2_RH_PLATFORM        = 0x4000000C,
>> >  };
>> >
>> >  enum tpm2_command_codes {
>> >         TPM2_CC_STARTUP         = 0x0144,
>> >         TPM2_CC_SELF_TEST       = 0x0143,
>> > +       TPM2_CC_CLEAR           = 0x0126,
>> > +       TPM2_CC_CLEARCONTROL    = 0x0127,
>> >         TPM2_CC_GET_CAPABILITY  = 0x017A,
>> >         TPM2_CC_PCR_READ        = 0x017E,
>> >         TPM2_CC_PCR_EXTEND      = 0x0182,
>> > @@ -567,11 +577,14 @@ uint32_t tpm_tsc_physical_presence(uint16_t presence);
>> >  uint32_t tpm_read_pubek(void *data, size_t count);
>> >
>> >  /**
>> > - * Issue a TPM_ForceClear command.
>> > + * Issue a TPM_ForceClear or a TPM2_Clear command.
>> >   *
>> > + * @param handle       Handle
>> > + * @param pw           Password
>> > + * @param pw_sz                Length of the password
>> >   * @return return code of the operation
>> >   */
>> > -uint32_t tpm_force_clear(void);
>> > +int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz);
>> >
>> >  /**
>> >   * Issue a TPM_PhysicalEnable command.
>> > diff --git a/lib/tpm.c b/lib/tpm.c
>> > index 3cc2888267..9a46ac09e6 100644
>> > --- a/lib/tpm.c
>> > +++ b/lib/tpm.c
>> > @@ -608,13 +608,47 @@ uint32_t tpm_read_pubek(void *data, size_t count)
>> >         return 0;
>> >  }
>> >
>> > -uint32_t tpm_force_clear(void)
>> > +int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz)
>> >  {
>> > -       const uint8_t command[10] = {
>> > -               0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x5d,
>> > +       const u8 command_v1[10] = {
>> > +               U16_TO_ARRAY(0xc1),
>> > +               U32_TO_ARRAY(10),
>> > +               U32_TO_ARRAY(0x5d),
>> >         };
>> > +       u8 command_v2[COMMAND_BUFFER_SIZE] = {
>> > +               U16_TO_ARRAY(TPM2_ST_SESSIONS), /* TAG */
>> > +               U32_TO_ARRAY(27 + pw_sz),       /* Length */
>> > +               U32_TO_ARRAY(TPM2_CC_CLEAR),    /* Command code */
>>
>> Here we have both v1 and v2 commands. Perhaps we should have a Kconfig
>> option to enable v2 support? Or do you think it is not a big addition
>> in terms of code side?
>
> It is a big addition in terms of code side.
>
>>
>> One way to do this would be to have an inline function which can tell
>> if you are using v2:
>>
>> static inline bool tpm_v2(void) {
>>    return IS_ENABLED(CONFIG_TPM_V2) && further things...
>> }
>>
>> static inline bool tpm_v1(void) {
>>    return IS_ENABLED(CONFIG_TPM_V1) && further things...
>> }
>
> I like this way of choosing one specification or the other, I could
> make them mutually exclusive. It would prevent the need for a global
> variable (or an additional field in uclass).
>
> Would it be fine to actually rename the file (with a 'tpm1' suffix) and
> create a new one specific for TPMv2 commands ? Only one file would be
> compiled/linked depending on the configuration, avoiding the possibility
> to call a v1 command from a v2 chip and vice versa.
>
> At first I thought a lot of code would be shared but I don't think we
> would add so much by having one function per revision now.

Well you could have two separate files, if there really is no sharing
of commands. But if there are common commands, you should have a
'common' file to implement them, rather than duplicating code.

With the above static inline functions we can support:

- v1 only (no code-size increment)
- v2 only (minimal code size for what will be a common case)
- v1 + v2 (e.g. for sandbox testing)

Regards,
Simon

[U-Boot] [PATCH v2 11/19] tpm: add TPM2_Clear command support

[Miquel Raynal]

Hi Simon,

On Thu, 26 Apr 2018 08:40:27 -0600, Simon Glass <sjg@chromium.org>
wrote:

> Hi Miquel,
> 
> On 24 April 2018 at 07:17, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > Hi Simon,
> >
> > On Fri, 30 Mar 2018 06:42:32 +0800, Simon Glass <sjg@chromium.org>
> > wrote:
> >  
> >> Hi Miquel,
> >>
> >> On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> >> > Add support for the TPM2_Clear command.
> >> >
> >> > Change the command file and the help accordingly.
> >> >
> >> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> >> > ---
> >> >  cmd/tpm.c      | 29 ++++++++++++++++++++++++++---
> >> >  cmd/tpm_test.c |  6 +++---
> >> >  include/tpm.h  | 21 +++++++++++++++++----
> >> >  lib/tpm.c      | 42 ++++++++++++++++++++++++++++++++++++++----
> >> >  4 files changed, 84 insertions(+), 14 deletions(-)
> >> >
> >> > diff --git a/cmd/tpm.c b/cmd/tpm.c
> >> > index fc9ef9d4a3..32921e1a70 100644
> >> > --- a/cmd/tpm.c
> >> > +++ b/cmd/tpm.c
> >> > @@ -454,6 +454,29 @@ static int do_tpm_init(cmd_tbl_t *cmdtp, int flag,
> >> >         return report_return_code(tpm_init());
> >> >  }
> >> >
> >> > +static int do_tpm_force_clear(cmd_tbl_t *cmdtp, int flag,
> >> > +                             int argc, char * const argv[])
> >> > +{
> >> > +       u32 handle = 0;
> >> > +       const char *pw = (argc < 3) ? NULL : argv[2];
> >> > +       const ssize_t pw_sz = pw ? strlen(pw) : 0;
> >> > +
> >> > +       if (argc < 2 || argc > 3)
> >> > +               return CMD_RET_USAGE;
> >> > +
> >> > +       if (pw_sz > TPM2_DIGEST_LENGTH)
> >> > +               return -EINVAL;
> >> > +
> >> > +       if (!strcasecmp("TPM2_RH_LOCKOUT", argv[1]))
> >> > +               handle = TPM2_RH_LOCKOUT;
> >> > +       else if (!strcasecmp("TPM2_RH_PLATFORM", argv[1]))
> >> > +               handle = TPM2_RH_PLATFORM;
> >> > +       else
> >> > +               return CMD_RET_USAGE;
> >> > +
> >> > +       return report_return_code(tpm_force_clear(handle, pw, pw_sz));
> >> > +}
> >> > +
> >> >  #define TPM_COMMAND_NO_ARG(cmd)                                \
> >> >  static int do_##cmd(cmd_tbl_t *cmdtp, int flag,                \
> >> >                 int argc, char * const argv[])          \
> >> > @@ -465,7 +488,6 @@ static int do_##cmd(cmd_tbl_t *cmdtp, int flag,             \
> >> >
> >> >  TPM_COMMAND_NO_ARG(tpm_self_test_full)
> >> >  TPM_COMMAND_NO_ARG(tpm_continue_self_test)
> >> > -TPM_COMMAND_NO_ARG(tpm_force_clear)
> >> >  TPM_COMMAND_NO_ARG(tpm_physical_enable)
> >> >  TPM_COMMAND_NO_ARG(tpm_physical_disable)
> >> >
> >> > @@ -951,8 +973,9 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
> >> >  "  physical_set_deactivated 0|1\n"
> >> >  "    - Set deactivated flag.\n"
> >> >  "Admin Ownership Commands:\n"
> >> > -"  force_clear\n"
> >> > -"    - Issue TPM_ForceClear command.\n"
> >> > +"  force_clear [<type>]\n"
> >> > +"    - Issue TPM_[Force]Clear command, with <type> one of (TPMv2 only):\n"
> >> > +"          * TPM2_RH_LOCKOUT, TPM2_RH_PLATFORM.\n"
> >> >  "  tsc_physical_presence flags\n"
> >> >  "    - Set TPM device's Physical Presence flags to <flags>.\n"
> >> >  "The Capability Commands:\n"
> >> > diff --git a/cmd/tpm_test.c b/cmd/tpm_test.c
> >> > index 37ad2ff33d..da40dbc423 100644
> >> > --- a/cmd/tpm_test.c
> >> > +++ b/cmd/tpm_test.c
> >> > @@ -176,7 +176,7 @@ static int test_fast_enable(void)
> >> >         TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL));
> >> >         printf("\tdisable is %d, deactivated is %d\n", disable, deactivated);
> >> >         for (i = 0; i < 2; i++) {
> >> > -               TPM_CHECK(tpm_force_clear());
> >> > +               TPM_CHECK(tpm_force_clear(0, NULL, 0));
> >> >                 TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL));
> >> >                 printf("\tdisable is %d, deactivated is %d\n", disable,
> >> >                        deactivated);
> >> > @@ -458,7 +458,7 @@ static int test_write_limit(void)
> >> >         TPM_CHECK(TlclStartupIfNeeded());
> >> >         TPM_CHECK(tpm_self_test_full());
> >> >         TPM_CHECK(tpm_tsc_physical_presence(PRESENCE));
> >> > -       TPM_CHECK(tpm_force_clear());
> >> > +       TPM_CHECK(tpm_force_clear(0, NULL, 0));
> >> >         TPM_CHECK(tpm_physical_enable());
> >> >         TPM_CHECK(tpm_physical_set_deactivated(0));
> >> >
> >> > @@ -477,7 +477,7 @@ static int test_write_limit(void)
> >> >         }
> >> >
> >> >         /* Reset write count */
> >> > -       TPM_CHECK(tpm_force_clear());
> >> > +       TPM_CHECK(tpm_force_clear(0, NULL, 0));
> >> >         TPM_CHECK(tpm_physical_enable());
> >> >         TPM_CHECK(tpm_physical_set_deactivated(0));
> >> >
> >> > diff --git a/include/tpm.h b/include/tpm.h
> >> > index 38d7cb899d..2f17166662 100644
> >> > --- a/include/tpm.h
> >> > +++ b/include/tpm.h
> >> > @@ -43,13 +43,23 @@ enum tpm_startup_type {
> >> >  };
> >> >
> >> >  enum tpm2_startup_types {
> >> > -       TPM2_SU_CLEAR   = 0x0000,
> >> > -       TPM2_SU_STATE   = 0x0001,
> >> > +       TPM2_SU_CLEAR           = 0x0000,
> >> > +       TPM2_SU_STATE           = 0x0001,
> >> > +};
> >> > +
> >> > +enum tpm2_handles {  
> >>
> >> Please add comment to enum  
> >
> > Fine, I will document all of them. Just for you to know, these are
> > values extracted from the (publicly available) specification, I did
> > not change any of them.
> >  
> >>  
> >> > +       TPM2_RH_OWNER           = 0x40000001,
> >> > +       TPM2_RS_PW              = 0x40000009,
> >> > +       TPM2_RH_LOCKOUT         = 0x4000000A,
> >> > +       TPM2_RH_ENDORSEMENT     = 0x4000000B,
> >> > +       TPM2_RH_PLATFORM        = 0x4000000C,
> >> >  };
> >> >
> >> >  enum tpm2_command_codes {
> >> >         TPM2_CC_STARTUP         = 0x0144,
> >> >         TPM2_CC_SELF_TEST       = 0x0143,
> >> > +       TPM2_CC_CLEAR           = 0x0126,
> >> > +       TPM2_CC_CLEARCONTROL    = 0x0127,
> >> >         TPM2_CC_GET_CAPABILITY  = 0x017A,
> >> >         TPM2_CC_PCR_READ        = 0x017E,
> >> >         TPM2_CC_PCR_EXTEND      = 0x0182,
> >> > @@ -567,11 +577,14 @@ uint32_t tpm_tsc_physical_presence(uint16_t presence);
> >> >  uint32_t tpm_read_pubek(void *data, size_t count);
> >> >
> >> >  /**
> >> > - * Issue a TPM_ForceClear command.
> >> > + * Issue a TPM_ForceClear or a TPM2_Clear command.
> >> >   *
> >> > + * @param handle       Handle
> >> > + * @param pw           Password
> >> > + * @param pw_sz                Length of the password
> >> >   * @return return code of the operation
> >> >   */
> >> > -uint32_t tpm_force_clear(void);
> >> > +int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz);
> >> >
> >> >  /**
> >> >   * Issue a TPM_PhysicalEnable command.
> >> > diff --git a/lib/tpm.c b/lib/tpm.c
> >> > index 3cc2888267..9a46ac09e6 100644
> >> > --- a/lib/tpm.c
> >> > +++ b/lib/tpm.c
> >> > @@ -608,13 +608,47 @@ uint32_t tpm_read_pubek(void *data, size_t count)
> >> >         return 0;
> >> >  }
> >> >
> >> > -uint32_t tpm_force_clear(void)
> >> > +int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz)
> >> >  {
> >> > -       const uint8_t command[10] = {
> >> > -               0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x5d,
> >> > +       const u8 command_v1[10] = {
> >> > +               U16_TO_ARRAY(0xc1),
> >> > +               U32_TO_ARRAY(10),
> >> > +               U32_TO_ARRAY(0x5d),
> >> >         };
> >> > +       u8 command_v2[COMMAND_BUFFER_SIZE] = {
> >> > +               U16_TO_ARRAY(TPM2_ST_SESSIONS), /* TAG */
> >> > +               U32_TO_ARRAY(27 + pw_sz),       /* Length */
> >> > +               U32_TO_ARRAY(TPM2_CC_CLEAR),    /* Command code */  
> >>
> >> Here we have both v1 and v2 commands. Perhaps we should have a Kconfig
> >> option to enable v2 support? Or do you think it is not a big addition
> >> in terms of code side?  
> >
> > It is a big addition in terms of code side.
> >  
> >>
> >> One way to do this would be to have an inline function which can tell
> >> if you are using v2:
> >>
> >> static inline bool tpm_v2(void) {
> >>    return IS_ENABLED(CONFIG_TPM_V2) && further things...
> >> }
> >>
> >> static inline bool tpm_v1(void) {
> >>    return IS_ENABLED(CONFIG_TPM_V1) && further things...
> >> }  
> >
> > I like this way of choosing one specification or the other, I could
> > make them mutually exclusive. It would prevent the need for a global
> > variable (or an additional field in uclass).
> >
> > Would it be fine to actually rename the file (with a 'tpm1' suffix) and
> > create a new one specific for TPMv2 commands ? Only one file would be
> > compiled/linked depending on the configuration, avoiding the possibility
> > to call a v1 command from a v2 chip and vice versa.
> >
> > At first I thought a lot of code would be shared but I don't think we
> > would add so much by having one function per revision now.  
> 
> Well you could have two separate files, if there really is no sharing
> of commands. But if there are common commands, you should have a
> 'common' file to implement them, rather than duplicating code.
> 
> With the above static inline functions we can support:
> 
> - v1 only (no code-size increment)
> - v2 only (minimal code size for what will be a common case)
> - v1 + v2 (e.g. for sandbox testing)

I am changing the whole architecture as you suggested.

Now v1 and v2 are chosen with a Kconfig menu, common code is in a
tpm-common.c file, and commands/library functions for each
specification is in tpm-v1.c and tpm-v2.c (with all the needed headers.
This way TPMv1 code is absolutely untouched while it allows TPMv2
support to be introduced independently.

You'll tell me how you find this alternative, I will soon send a v3.

Otherwise, as suggested in a first answer, I changed U32_TO_array
> 
> Regards,
> Simon



-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[U-Boot] [PATCH v2 09/19] tpm: add TPM2_Startup command support

[Miquel Raynal]

Hi Simon,

> >  enum tpm_physical_presence {
> >         TPM_PHYSICAL_PRESENCE_HW_DISABLE        = 0x0200,
> >         TPM_PHYSICAL_PRESENCE_CMD_DISABLE       = 0x0100,
> > @@ -445,7 +469,7 @@ int tpm_get_specification(void);
> >   * @param mode         TPM startup mode
> >   * @return return code of the operation
> >   */
> > -uint32_t tpm_startup(enum tpm_startup_type mode);
> > +int tpm_startup(enum tpm_startup_type mode);  
> 
> How come the change to an int? It's fine, I just want to understand it.
> 

Good catch, that is a mistake and has no interest. In the new version I
moved all the functions to be aligned by returning an u32.

Thanks,
Miquèl

-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[U-Boot] [PATCH v2 07/19] tpm: add possible traces to analyze buffers returned by the TPM

[Miquel Raynal]

Hi Simon,

On Fri, 30 Mar 2018 06:42:10 +0800, Simon Glass <sjg@chromium.org>
wrote:

> On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > When debugging, it is welcome to get more information about what the TPM
> > returns. Add the possibility to print the packets received to show their
> > exact content.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  lib/tpm.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)  
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> You might consider using the new logging support for this? See log.h

Ok, I used log() instead of debug().

Thanks,
Miquèl

> 
> - Simon



-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[U-Boot] [PATCH v2 10/19] tpm: add TPM2_SelfTest command support

[Miquel Raynal]

Hi Simon,

On Thu, 26 Apr 2018 08:40:24 -0600, Simon Glass <sjg@chromium.org>
wrote:

> Hi Miquel,
> 
> On 24 April 2018 at 06:53, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > Hi Simon,
> >
> > I am back on that topic, let me answer some of your questions before
> > addressing them in a next version.
> >
> > On Fri, 30 Mar 2018 06:42:25 +0800, Simon Glass
> > <sjg@chromium.org> wrote:
> >  
> >> Hi Miquel,
> >>
> >> On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> >> > Add support for the TPM2_Selftest command.
> >> >
> >> > Change the command file and the help accordingly.
> >> >
> >> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> >> > ---
> >> >  include/tpm.h |  9 +++++++--
> >> >  lib/tpm.c     | 36 ++++++++++++++++++++++++++++--------
> >> >  2 files changed, 35 insertions(+), 10 deletions(-)
> >> >
> >> > diff --git a/include/tpm.h b/include/tpm.h
> >> > index ba71bac885..38d7cb899d 100644
> >> > --- a/include/tpm.h
> >> > +++ b/include/tpm.h
> >> > @@ -31,6 +31,11 @@ enum tpm2_structures {
> >> >         TPM2_ST_SESSIONS        = 0x8002,
> >> >  };
> >> >
> >> > +enum tpm2_yes_no {
> >> > +       TPMI_YES                = 1,
> >> > +       TPMI_NO                 = 0,
> >> > +};
> >> > +
> >> >  enum tpm_startup_type {
> >> >         TPM_ST_CLEAR            = 0x0001,
> >> >         TPM_ST_STATE            = 0x0002,
> >> > @@ -476,14 +481,14 @@ int tpm_startup(enum tpm_startup_type mode);
> >> >   *
> >> >   * @return return code of the operation
> >> >   */
> >> > -uint32_t tpm_self_test_full(void);
> >> > +int tpm_self_test_full(void);
> >> >
> >> >  /**
> >> >   * Issue a TPM_ContinueSelfTest command.
> >> >   *
> >> >   * @return return code of the operation
> >> >   */
> >> > -uint32_t tpm_continue_self_test(void);
> >> > +int tpm_continue_self_test(void);
> >> >
> >> >  /**
> >> >   * Issue a TPM_NV_DefineSpace command.  The implementation is limited
> >> > diff --git a/lib/tpm.c b/lib/tpm.c
> >> > index 0c57ef8dc7..3cc2888267 100644
> >> > --- a/lib/tpm.c
> >> > +++ b/lib/tpm.c
> >> > @@ -341,20 +341,40 @@ int tpm_startup(enum tpm_startup_type mode)
> >> >         return 0;
> >> >  }
> >> >
> >> > -uint32_t tpm_self_test_full(void)
> >> > +int tpm_self_test_full(void)
> >> >  {
> >> > -       const uint8_t command[10] = {
> >> > -               0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x50,
> >> > +       const u8 command_v1[10] = {
> >> > +               U16_TO_ARRAY(0xc1),  
> >>
> >> Here I can see some benefit to your macros because the data is better
> >> structured. But why not use the pack_byte_string() function?  
> >
> > TPM buffers (1.0 and 2.0) are, most of the time, filled with data of
> > known length (1, 2 or 4 bytes). You can see that most of the time,
> > TPMv1 simple commands were just filling a buffer of bytes. I don't like
> > very much the fact that the user should split the data in byte array so
> > I introduced these macros that do it for me in a cleaner way. When you
> > get an hexadecimal value (like it was the case before) it is easy to
> > split a "0xabcd" in "0xab, 0xcd", but I prefer to use variables or
> > simply defines sometimes and writing "TPM2_ST_NO_SESSIONS" as
> > "TPM2_ST_NO_SESSIONS >> 8, TPM2_ST_NO_SESSIONS & 0xFF" is not readable
> > at all.
> >
> > Now why did I not use the existing pack_byte_string() function. Well,
> > at first I did and realized it was a pain to have a decent
> > incrementation of the offsets, mostly when commands tend to be longer
> > and longer. Having such lists of offset/variable/length while you could
> > define them statically on the stack -which also saves some computing
> > time- is quickly annoying and hard to review. From my point of view this
> > function is a real asset when it comes to 'unknown'/'big' variable
> > length (like a password, an hmac, an user input, etc) but should be
> > avoided when not necessary: typically to fill a buffer with defined
> > values.
> >
> > I am of course very open on the topic, this is my feeling but I don't
> > have that much experience and would carefully listen to yours.  
> 
> I don't think this is an unreasonable point of view. Either works.
> 
> But if you are changing the approach to use static data, should you
> convert the existing code to the new scheme?

I don't think this is relevant anymore as files have been split. You'll
tell me what you think with the next version (need to do some testing
and I'll be done with it).

Regards,
Miquèl

> 
> Regards,
> Simon



-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[U-Boot] [PATCH v2 11/19] tpm: add TPM2_Clear command support

[Simon Glass]

Hi Miquel,

On 27 April 2018 at 07:39, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Hi Simon,
>
> On Thu, 26 Apr 2018 08:40:27 -0600, Simon Glass <sjg@chromium.org>
> wrote:
>
>> Hi Miquel,
>>
>> On 24 April 2018 at 07:17, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>> > Hi Simon,
>> >
>> > On Fri, 30 Mar 2018 06:42:32 +0800, Simon Glass <sjg@chromium.org>
>> > wrote:
>> >
>> >> Hi Miquel,
>> >>
>> >> On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>> >> > Add support for the TPM2_Clear command.
>> >> >
>> >> > Change the command file and the help accordingly.
>> >> >
>> >> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>> >> > ---
>> >> >  cmd/tpm.c      | 29 ++++++++++++++++++++++++++---
>> >> >  cmd/tpm_test.c |  6 +++---
>> >> >  include/tpm.h  | 21 +++++++++++++++++----
>> >> >  lib/tpm.c      | 42 ++++++++++++++++++++++++++++++++++++++----
>> >> >  4 files changed, 84 insertions(+), 14 deletions(-)
>> >> >
>> >> > diff --git a/cmd/tpm.c b/cmd/tpm.c
>> >> > index fc9ef9d4a3..32921e1a70 100644
>> >> > --- a/cmd/tpm.c
>> >> > +++ b/cmd/tpm.c
>> >> > @@ -454,6 +454,29 @@ static int do_tpm_init(cmd_tbl_t *cmdtp, int flag,
>> >> >         return report_return_code(tpm_init());
>> >> >  }
>> >> >
>> >> > +static int do_tpm_force_clear(cmd_tbl_t *cmdtp, int flag,
>> >> > +                             int argc, char * const argv[])
>> >> > +{
>> >> > +       u32 handle = 0;
>> >> > +       const char *pw = (argc < 3) ? NULL : argv[2];
>> >> > +       const ssize_t pw_sz = pw ? strlen(pw) : 0;
>> >> > +
>> >> > +       if (argc < 2 || argc > 3)
>> >> > +               return CMD_RET_USAGE;
>> >> > +
>> >> > +       if (pw_sz > TPM2_DIGEST_LENGTH)
>> >> > +               return -EINVAL;
>> >> > +
>> >> > +       if (!strcasecmp("TPM2_RH_LOCKOUT", argv[1]))
>> >> > +               handle = TPM2_RH_LOCKOUT;
>> >> > +       else if (!strcasecmp("TPM2_RH_PLATFORM", argv[1]))
>> >> > +               handle = TPM2_RH_PLATFORM;
>> >> > +       else
>> >> > +               return CMD_RET_USAGE;
>> >> > +
>> >> > +       return report_return_code(tpm_force_clear(handle, pw, pw_sz));
>> >> > +}
>> >> > +
>> >> >  #define TPM_COMMAND_NO_ARG(cmd)                                \
>> >> >  static int do_##cmd(cmd_tbl_t *cmdtp, int flag,                \
>> >> >                 int argc, char * const argv[])          \
>> >> > @@ -465,7 +488,6 @@ static int do_##cmd(cmd_tbl_t *cmdtp, int flag,             \
>> >> >
>> >> >  TPM_COMMAND_NO_ARG(tpm_self_test_full)
>> >> >  TPM_COMMAND_NO_ARG(tpm_continue_self_test)
>> >> > -TPM_COMMAND_NO_ARG(tpm_force_clear)
>> >> >  TPM_COMMAND_NO_ARG(tpm_physical_enable)
>> >> >  TPM_COMMAND_NO_ARG(tpm_physical_disable)
>> >> >
>> >> > @@ -951,8 +973,9 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
>> >> >  "  physical_set_deactivated 0|1\n"
>> >> >  "    - Set deactivated flag.\n"
>> >> >  "Admin Ownership Commands:\n"
>> >> > -"  force_clear\n"
>> >> > -"    - Issue TPM_ForceClear command.\n"
>> >> > +"  force_clear [<type>]\n"
>> >> > +"    - Issue TPM_[Force]Clear command, with <type> one of (TPMv2 only):\n"
>> >> > +"          * TPM2_RH_LOCKOUT, TPM2_RH_PLATFORM.\n"
>> >> >  "  tsc_physical_presence flags\n"
>> >> >  "    - Set TPM device's Physical Presence flags to <flags>.\n"
>> >> >  "The Capability Commands:\n"
>> >> > diff --git a/cmd/tpm_test.c b/cmd/tpm_test.c
>> >> > index 37ad2ff33d..da40dbc423 100644
>> >> > --- a/cmd/tpm_test.c
>> >> > +++ b/cmd/tpm_test.c
>> >> > @@ -176,7 +176,7 @@ static int test_fast_enable(void)
>> >> >         TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL));
>> >> >         printf("\tdisable is %d, deactivated is %d\n", disable, deactivated);
>> >> >         for (i = 0; i < 2; i++) {
>> >> > -               TPM_CHECK(tpm_force_clear());
>> >> > +               TPM_CHECK(tpm_force_clear(0, NULL, 0));
>> >> >                 TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL));
>> >> >                 printf("\tdisable is %d, deactivated is %d\n", disable,
>> >> >                        deactivated);
>> >> > @@ -458,7 +458,7 @@ static int test_write_limit(void)
>> >> >         TPM_CHECK(TlclStartupIfNeeded());
>> >> >         TPM_CHECK(tpm_self_test_full());
>> >> >         TPM_CHECK(tpm_tsc_physical_presence(PRESENCE));
>> >> > -       TPM_CHECK(tpm_force_clear());
>> >> > +       TPM_CHECK(tpm_force_clear(0, NULL, 0));
>> >> >         TPM_CHECK(tpm_physical_enable());
>> >> >         TPM_CHECK(tpm_physical_set_deactivated(0));
>> >> >
>> >> > @@ -477,7 +477,7 @@ static int test_write_limit(void)
>> >> >         }
>> >> >
>> >> >         /* Reset write count */
>> >> > -       TPM_CHECK(tpm_force_clear());
>> >> > +       TPM_CHECK(tpm_force_clear(0, NULL, 0));
>> >> >         TPM_CHECK(tpm_physical_enable());
>> >> >         TPM_CHECK(tpm_physical_set_deactivated(0));
>> >> >
>> >> > diff --git a/include/tpm.h b/include/tpm.h
>> >> > index 38d7cb899d..2f17166662 100644
>> >> > --- a/include/tpm.h
>> >> > +++ b/include/tpm.h
>> >> > @@ -43,13 +43,23 @@ enum tpm_startup_type {
>> >> >  };
>> >> >
>> >> >  enum tpm2_startup_types {
>> >> > -       TPM2_SU_CLEAR   = 0x0000,
>> >> > -       TPM2_SU_STATE   = 0x0001,
>> >> > +       TPM2_SU_CLEAR           = 0x0000,
>> >> > +       TPM2_SU_STATE           = 0x0001,
>> >> > +};
>> >> > +
>> >> > +enum tpm2_handles {
>> >>
>> >> Please add comment to enum
>> >
>> > Fine, I will document all of them. Just for you to know, these are
>> > values extracted from the (publicly available) specification, I did
>> > not change any of them.
>> >
>> >>
>> >> > +       TPM2_RH_OWNER           = 0x40000001,
>> >> > +       TPM2_RS_PW              = 0x40000009,
>> >> > +       TPM2_RH_LOCKOUT         = 0x4000000A,
>> >> > +       TPM2_RH_ENDORSEMENT     = 0x4000000B,
>> >> > +       TPM2_RH_PLATFORM        = 0x4000000C,
>> >> >  };
>> >> >
>> >> >  enum tpm2_command_codes {
>> >> >         TPM2_CC_STARTUP         = 0x0144,
>> >> >         TPM2_CC_SELF_TEST       = 0x0143,
>> >> > +       TPM2_CC_CLEAR           = 0x0126,
>> >> > +       TPM2_CC_CLEARCONTROL    = 0x0127,
>> >> >         TPM2_CC_GET_CAPABILITY  = 0x017A,
>> >> >         TPM2_CC_PCR_READ        = 0x017E,
>> >> >         TPM2_CC_PCR_EXTEND      = 0x0182,
>> >> > @@ -567,11 +577,14 @@ uint32_t tpm_tsc_physical_presence(uint16_t presence);
>> >> >  uint32_t tpm_read_pubek(void *data, size_t count);
>> >> >
>> >> >  /**
>> >> > - * Issue a TPM_ForceClear command.
>> >> > + * Issue a TPM_ForceClear or a TPM2_Clear command.
>> >> >   *
>> >> > + * @param handle       Handle
>> >> > + * @param pw           Password
>> >> > + * @param pw_sz                Length of the password
>> >> >   * @return return code of the operation
>> >> >   */
>> >> > -uint32_t tpm_force_clear(void);
>> >> > +int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz);
>> >> >
>> >> >  /**
>> >> >   * Issue a TPM_PhysicalEnable command.
>> >> > diff --git a/lib/tpm.c b/lib/tpm.c
>> >> > index 3cc2888267..9a46ac09e6 100644
>> >> > --- a/lib/tpm.c
>> >> > +++ b/lib/tpm.c
>> >> > @@ -608,13 +608,47 @@ uint32_t tpm_read_pubek(void *data, size_t count)
>> >> >         return 0;
>> >> >  }
>> >> >
>> >> > -uint32_t tpm_force_clear(void)
>> >> > +int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz)
>> >> >  {
>> >> > -       const uint8_t command[10] = {
>> >> > -               0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x5d,
>> >> > +       const u8 command_v1[10] = {
>> >> > +               U16_TO_ARRAY(0xc1),
>> >> > +               U32_TO_ARRAY(10),
>> >> > +               U32_TO_ARRAY(0x5d),
>> >> >         };
>> >> > +       u8 command_v2[COMMAND_BUFFER_SIZE] = {
>> >> > +               U16_TO_ARRAY(TPM2_ST_SESSIONS), /* TAG */
>> >> > +               U32_TO_ARRAY(27 + pw_sz),       /* Length */
>> >> > +               U32_TO_ARRAY(TPM2_CC_CLEAR),    /* Command code */
>> >>
>> >> Here we have both v1 and v2 commands. Perhaps we should have a Kconfig
>> >> option to enable v2 support? Or do you think it is not a big addition
>> >> in terms of code side?
>> >
>> > It is a big addition in terms of code side.
>> >
>> >>
>> >> One way to do this would be to have an inline function which can tell
>> >> if you are using v2:
>> >>
>> >> static inline bool tpm_v2(void) {
>> >>    return IS_ENABLED(CONFIG_TPM_V2) && further things...
>> >> }
>> >>
>> >> static inline bool tpm_v1(void) {
>> >>    return IS_ENABLED(CONFIG_TPM_V1) && further things...
>> >> }
>> >
>> > I like this way of choosing one specification or the other, I could
>> > make them mutually exclusive. It would prevent the need for a global
>> > variable (or an additional field in uclass).
>> >
>> > Would it be fine to actually rename the file (with a 'tpm1' suffix) and
>> > create a new one specific for TPMv2 commands ? Only one file would be
>> > compiled/linked depending on the configuration, avoiding the possibility
>> > to call a v1 command from a v2 chip and vice versa.
>> >
>> > At first I thought a lot of code would be shared but I don't think we
>> > would add so much by having one function per revision now.
>>
>> Well you could have two separate files, if there really is no sharing
>> of commands. But if there are common commands, you should have a
>> 'common' file to implement them, rather than duplicating code.
>>
>> With the above static inline functions we can support:
>>
>> - v1 only (no code-size increment)
>> - v2 only (minimal code size for what will be a common case)
>> - v1 + v2 (e.g. for sandbox testing)
>
> I am changing the whole architecture as you suggested.
>
> Now v1 and v2 are chosen with a Kconfig menu, common code is in a
> tpm-common.c file, and commands/library functions for each
> specification is in tpm-v1.c and tpm-v2.c (with all the needed headers.
> This way TPMv1 code is absolutely untouched while it allows TPMv2
> support to be introduced independently.
>
> You'll tell me how you find this alternative, I will soon send a v3.
>
> Otherwise, as suggested in a first answer, I changed U32_TO_array

I think it works OK as you have it now, with the rename.

Regards,
Simon