All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6 v4] TPM cleanups and MMIO driver
@ 2021-11-03 15:09 Ilias Apalodimas
  2021-11-03 15:09 ` [PATCH 1/6 v4] tpm2: Introduce TIS tpm core Ilias Apalodimas
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Ilias Apalodimas @ 2021-11-03 15:09 UTC (permalink / raw)
  To: u-boot
  Cc: trini, Ilias Apalodimas, Rick Chen, Sean Anderson, Simon Glass,
	Heinrich Schuchardt, Masahisa Kojima

Hi! 

This is the followup series on the TPM cleanup and driver additions.

The major change since v3 [1] is that I implemented Simon's request and
converted an existing driver using the API.  We now have two consumers of
the API, MMIO and SPI TPMs.  It's worth noting that using the API reduces
the code duplication in the SPI TPM driver a lot.  

I've tested the SPI TPM on an RPI4 with [2]. As far as I can tell everything
seems to be working fine, including the EFI TCG2 protocol.  The MMIO one was
tested with QEMU and SWTPM [3] and I've added documentation on how to reproduce
that.

There was also a discussion on v2 [4] regarding the MMIO accesses and if we
should convert those to a uclass.  But the MMIO functions are just
calling io(read|write),  so after considering it for a while,  I couldn't
find any reasonable abstraction that would justify another uclass.

[1] https://lore.kernel.org/u-boot/20210708082310.87540-1-ilias.apalodimas@linaro.org/
[2] https://buyzero.de/en/products/letstrust-hardware-tpm-trusted-platform-module
[3] https://github.com/stefanberger/swtpm
[4] https://lore.kernel.org/u-boot/CAPnjgZ1U6VgeOcTuy-G=nbYFTNnu_8MqGf-o6LF6ivk=TwE4iQ@mail.gmail.com/

Changes since v3:
- Coverted SPI TPM to use the API as well
- moved some log_info to log_debug
- Added documentation on how to run QEMU and enabled TPM by default on arm qemu
  builds
Changes since v2:
- Add myself as a maintainer on TPM drivers
Changes since v1:
- split off the tis core code into a different file

Ilias Apalodimas (6):
  tpm2: Introduce TIS tpm core
  tpm2: Add a TPMv2 MMIO TIS driver
  tpm: Use the new API on tpm2 spi driver
  configs: Enable tpmv2 mmio on qemu for arm/arm64
  doc: qemu: Add instructions for swtpm usage
  MAINTAINERS: Add entry for TPM drivers

 MAINTAINERS                      |   5 +
 configs/qemu_arm64_defconfig     |   2 +
 configs/qemu_arm_defconfig       |   2 +
 doc/board/emulation/qemu-arm.rst |  25 ++
 drivers/tpm/Kconfig              |   9 +
 drivers/tpm/Makefile             |   3 +-
 drivers/tpm/tpm2_tis_core.c      | 523 +++++++++++++++++++++++++++++++
 drivers/tpm/tpm2_tis_mmio.c      | 152 +++++++++
 drivers/tpm/tpm2_tis_spi.c       | 440 ++------------------------
 drivers/tpm/tpm_tis.h            |  39 +++
 include/tpm-v2.h                 |   1 +
 11 files changed, 791 insertions(+), 410 deletions(-)
 create mode 100644 drivers/tpm/tpm2_tis_core.c
 create mode 100644 drivers/tpm/tpm2_tis_mmio.c

-- 
2.33.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/6 v4] tpm2: Introduce TIS tpm core
  2021-11-03 15:09 [PATCH 0/6 v4] TPM cleanups and MMIO driver Ilias Apalodimas
@ 2021-11-03 15:09 ` Ilias Apalodimas
  2021-11-05  2:02   ` Simon Glass
  2021-11-03 15:09 ` [PATCH 2/6 v4] tpm2: Add a TPMv2 MMIO TIS driver Ilias Apalodimas
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Ilias Apalodimas @ 2021-11-03 15:09 UTC (permalink / raw)
  To: u-boot
  Cc: trini, Ilias Apalodimas, Rick Chen, Sean Anderson, Simon Glass,
	Heinrich Schuchardt, Masahisa Kojima

There's a lot of code duplication in U-Boot right now.  All the TPM TIS
compatible drivers we have at the moment have their own copy of a TIS
implementation.

So let's create a common layer which implements the core TIS functions.
Any driver added from now own, which is compatible with the TIS spec, will
only have to provide the underlying bus communication mechanisms.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 drivers/tpm/tpm2_tis_core.c | 523 ++++++++++++++++++++++++++++++++++++
 drivers/tpm/tpm_tis.h       |  39 +++
 include/tpm-v2.h            |   1 +
 3 files changed, 563 insertions(+)
 create mode 100644 drivers/tpm/tpm2_tis_core.c

diff --git a/drivers/tpm/tpm2_tis_core.c b/drivers/tpm/tpm2_tis_core.c
new file mode 100644
index 000000000000..7d7050151622
--- /dev/null
+++ b/drivers/tpm/tpm2_tis_core.c
@@ -0,0 +1,523 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020, Linaro Limited
+ *
+ * Based on the Linux TIS core interface and U-Boot original SPI TPM driver
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <tpm-v2.h>
+#include <linux/delay.h>
+#include <linux/unaligned/be_byteshift.h>
+#include "tpm_tis.h"
+
+/**
+ * tpm_tis_get_desc - Get the TPM description
+ *
+ * @udev: udevice
+ * @buf:  buffer to fill data
+ * @size: buffer size
+ *
+ * @Return: Number of characters written (or would have been written) in buffer
+ */
+int tpm_tis_get_desc(struct udevice *udev, char *buf, int size)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+
+	if (size < 80)
+		return -ENOSPC;
+
+	return snprintf(buf, size,
+			"%s v2.0: VendorID 0x%04x, DeviceID 0x%04x, RevisionID 0x%02x [%s]",
+			udev->name, chip->vend_dev & 0xFFFF,
+			chip->vend_dev >> 16, chip->rid,
+			(chip->is_open ? "open" : "closed"));
+}
+
+/**
+ * tpm_tis_check_locality - Check the current TPM locality
+ *
+ * @udev: udevice
+ * @loc:  locality
+ *
+ * Return: True if the tested locality matches
+ */
+static bool tpm_tis_check_locality(struct udevice *udev, int loc)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+	struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
+	u8 locality;
+
+	phy_ops->read_bytes(udev, TPM_ACCESS(loc), 1, &locality);
+	if ((locality & (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID |
+	    TPM_ACCESS_REQUEST_USE)) ==
+	    (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) {
+		chip->locality = loc;
+		return true;
+	}
+
+	return false;
+}
+
+/**
+ * tpm_tis_request_locality - Request a locality from the TPM
+ *
+ * @udev: udev
+ * @loc:  requested locality
+ *
+ * Return: 0 on success -1 on failure
+ */
+int tpm_tis_request_locality(struct udevice *udev, int loc)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+	struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
+	u8 buf = TPM_ACCESS_REQUEST_USE;
+	unsigned long start, stop;
+
+	if (tpm_tis_check_locality(udev, loc))
+		return 0;
+
+	phy_ops->write_bytes(udev, TPM_ACCESS(loc), 1, &buf);
+	start = get_timer(0);
+	stop = chip->timeout_a;
+	do {
+		if (tpm_tis_check_locality(udev, loc))
+			return 0;
+		mdelay(TPM_TIMEOUT_MS);
+	} while (get_timer(start) < stop);
+
+	return -1;
+}
+
+/**
+ * tpm_tis_status - Check the current device status
+ *
+ * @udev:   udevice
+ * @status: return value of status
+ *
+ * Return: 0 on success, negative on failure
+ */
+static int tpm_tis_status(struct udevice *udev, u8 *status)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+	struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
+
+	if (chip->locality < 0)
+		return -EINVAL;
+
+	phy_ops->read_bytes(udev, TPM_STS(chip->locality), 1, status);
+
+	if ((*status & TPM_STS_READ_ZERO)) {
+		log_err("TPM returned invalid status\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/**
+ * tpm_tis_release_locality - Release the requested locality
+ *
+ * @udev: udevice
+ * @loc:  requested locality
+ *
+ * Return: 0 on success, negative on failure
+ */
+int tpm_tis_release_locality(struct udevice *udev, int loc)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+	struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
+	u8 buf = TPM_ACCESS_ACTIVE_LOCALITY;
+	int ret;
+
+	if (chip->locality < 0)
+		return 0;
+
+	ret = phy_ops->write_bytes(udev, TPM_ACCESS(loc), 1, &buf);
+	chip->locality = -1;
+
+	return ret;
+}
+
+/**
+ * tpm_tis_wait_for_stat - Wait for TPM to become ready
+ *
+ * @udev:    udev
+ * @mask:    mask to match
+ * @timeout: timeout for retries
+ * @status:  current status
+ *
+ * Return: 0 on success, negative on failure
+ */
+static int tpm_tis_wait_for_stat(struct udevice *udev, 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_status(udev, status);
+		if (ret)
+			return ret;
+
+		if ((*status & mask) == mask)
+			return 0;
+	} while (get_timer(start) < stop);
+
+	return -ETIMEDOUT;
+}
+
+/**
+ * tpm_tis_get_burstcount - Get the burstcount for the data FIFO
+ *
+ * @udev:       udevice
+ * @burstcount: current burstcount
+ *
+ * Return: 0 on success, negative on failure
+ */
+static int tpm_tis_get_burstcount(struct udevice *udev, size_t *burstcount)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+	struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
+	unsigned long start, stop;
+	u32 burst;
+
+	if (chip->locality < 0)
+		return -EINVAL;
+
+	/* wait for burstcount */
+	start = get_timer(0);
+	/*
+	 * This is the TPMv2 defined timeout. Change this in case you want to
+	 * make the driver compatile to TPMv1
+	 */
+	stop = chip->timeout_a;
+	do {
+		phy_ops->read32(udev, TPM_STS(chip->locality), &burst);
+		*burstcount = (burst >> 8) & 0xFFFF;
+		if (*burstcount)
+			return 0;
+
+		mdelay(TPM_TIMEOUT_MS);
+	} while (get_timer(start) < stop);
+
+	return -ETIMEDOUT;
+}
+
+/**
+ * tpm_tis_ready - Cancel pending comands and get the device on a ready state
+ *
+ * @udev: udevcie
+ *
+ * Return: 0 on success, negative on failure
+ */
+static int tpm_tis_ready(struct udevice *udev)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+	struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
+	u8 data = TPM_STS_COMMAND_READY;
+
+	/* This will cancel any pending commands */
+	return phy_ops->write_bytes(udev, TPM_STS(chip->locality), 1, &data);
+}
+
+/**
+ * tpm_tis_send - send data to the device
+ *
+ * @udev: udevice
+ * @buf:  buffer to send
+ * @len:  size of the buffer
+ *
+ * Return: number of bytes sent or negative on failure
+ */
+int tpm_tis_send(struct udevice *udev, const u8 *buf, size_t len)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+	struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
+	size_t burstcnt, wr_size, sent = 0;
+	u8 data = TPM_STS_GO;
+	u8 status;
+	int ret;
+
+	if (!chip)
+		return -ENODEV;
+
+	ret = tpm_tis_request_locality(udev, 0);
+	if (ret < 0)
+		return -EBUSY;
+
+	ret = tpm_tis_status(udev, &status);
+	if (ret)
+		goto release_locality;
+
+	if (!(status & TPM_STS_COMMAND_READY)) {
+		ret = tpm_tis_ready(udev);
+		if (ret) {
+			log_err("Can't cancel previous TPM operation\n");
+			goto release_locality;
+		}
+		ret = tpm_tis_wait_for_stat(udev, TPM_STS_COMMAND_READY,
+					    chip->timeout_b, &status);
+		if (ret) {
+			log_err("TPM not ready\n");
+			goto release_locality;
+		}
+	}
+
+	while (len > 0) {
+		ret = tpm_tis_get_burstcount(udev, &burstcnt);
+		if (ret)
+			goto release_locality;
+
+		wr_size = min(len, burstcnt);
+		ret = phy_ops->write_bytes(udev, TPM_DATA_FIFO(chip->locality),
+					   wr_size, buf + sent);
+		if (ret < 0)
+			goto release_locality;
+
+		ret = tpm_tis_wait_for_stat(udev, TPM_STS_VALID,
+					    chip->timeout_c, &status);
+		if (ret)
+			goto release_locality;
+
+		sent += wr_size;
+		len -= wr_size;
+		/* make sure the TPM expects more data */
+		if (len && !(status & TPM_STS_DATA_EXPECT)) {
+			ret = -EIO;
+			goto release_locality;
+		}
+	}
+
+	/*
+	 * Make a final check ensuring everything is ok and the TPM expects no
+	 * more data
+	 */
+	ret = tpm_tis_wait_for_stat(udev, TPM_STS_VALID, chip->timeout_c,
+				    &status);
+	if (ret)
+		goto release_locality;
+
+	if (status & TPM_STS_DATA_EXPECT) {
+		ret = -EIO;
+		goto release_locality;
+	}
+
+	ret = phy_ops->write_bytes(udev, TPM_STS(chip->locality), 1, &data);
+	if (ret)
+		goto release_locality;
+
+	return sent;
+
+release_locality:
+	tpm_tis_ready(udev);
+	tpm_tis_release_locality(udev, chip->locality);
+
+	return ret;
+}
+
+/**
+ * tpm_tis_recv_data - Receive data from a device. Wrapper for tpm_tis_recv
+ *
+ * @udev: udevice
+ * @buf:  buffer to copy data
+ * @size: buffer size
+ *
+ * Return: bytes read or negative on failure
+ */
+static int tpm_tis_recv_data(struct udevice *udev, u8 *buf, size_t count)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+	struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
+	int size = 0, len, ret;
+	size_t burstcnt;
+	u8 status;
+
+	while (size < count &&
+	       tpm_tis_wait_for_stat(udev, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
+				     chip->timeout_c, &status) == 0) {
+		ret = tpm_tis_get_burstcount(udev, &burstcnt);
+		if (ret)
+			return ret;
+
+		len = min_t(int, burstcnt, count - size);
+		ret = phy_ops->read_bytes(udev, TPM_DATA_FIFO(chip->locality),
+					  len, buf + size);
+		if (ret < 0)
+			return ret;
+
+		size += len;
+	}
+
+	return size;
+}
+
+/**
+ * tpm_tis_recv - Receive data from a device
+ *
+ * @udev: udevice
+ * @buf:  buffer to copy data
+ * @size: buffer size
+ *
+ * Return: bytes read or negative on failure
+ */
+int tpm_tis_recv(struct udevice *udev, u8 *buf, size_t count)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+	int size, expected;
+
+	if (count < TPM_HEADER_SIZE)
+		return -E2BIG;
+
+	size = tpm_tis_recv_data(udev, buf, TPM_HEADER_SIZE);
+	if (size < TPM_HEADER_SIZE) {
+		log_err("TPM error, unable to read header\n");
+		goto out;
+	}
+
+	expected = get_unaligned_be32(buf + TPM_CMD_COUNT_OFFSET);
+	if (expected > count) {
+		size = -EIO;
+		log_warning("Too much data: %d > %zu\n", expected, count);
+		goto out;
+	}
+
+	size += tpm_tis_recv_data(udev, &buf[TPM_HEADER_SIZE],
+				   expected - TPM_HEADER_SIZE);
+	if (size < expected) {
+		log(LOGC_NONE, LOGL_ERR,
+		    "TPM error, unable to read remaining bytes of result\n");
+		size = -EIO;
+		goto out;
+	}
+
+out:
+	tpm_tis_ready(udev);
+	/* acquired in tpm_tis_send */
+	tpm_tis_release_locality(udev, chip->locality);
+
+	return size;
+}
+
+/** tpm_tis_cleanup - Get the device in ready state and release locality
+ *
+ * @udev: udevice
+ *
+ * Return: always 0
+ */
+int tpm_tis_cleanup(struct udevice *udev)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+
+	tpm_tis_ready(udev);
+	tpm_tis_release_locality(udev, chip->locality);
+
+	return 0;
+}
+
+/**
+ * tpm_tis_open - Open the device and request locality 0
+ *
+ * @udev: udevice
+ *
+ * Return: 0 on success, negative on failure
+ */
+int tpm_tis_open(struct udevice *udev)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+	int ret;
+
+	if (chip->is_open)
+		return -EBUSY;
+
+	ret = tpm_tis_request_locality(udev, 0);
+	if (!ret)
+		chip->is_open = 1;
+
+	return ret;
+}
+
+/**
+ * tpm_tis_ops_register - register the PHY ops for the device
+ *
+ * @udev: udevice
+ * @ops: bus ops for the device
+ */
+void tpm_tis_ops_register(struct udevice *udev, struct tpm_tis_phy_ops *ops)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+
+	chip->phy_ops = ops;
+}
+
+static bool tis_check_ops(struct tpm_tis_phy_ops *phy_ops)
+{
+	if (!phy_ops || !phy_ops->read_bytes || !phy_ops->write_bytes ||
+	    !phy_ops->read32 || !phy_ops->write32)
+		return false;
+
+	return true;
+}
+
+/**
+ * tpm_tis_init - inititalize the device
+ *
+ * @udev: udevice
+ *
+ * Return: 0 on success, negative on failure
+ */
+int tpm_tis_init(struct udevice *udev)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+	struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
+	int ret;
+	u32 tmp;
+
+	if (!tis_check_ops(phy_ops)) {
+		log_err("Driver bug. No bus ops defined\n");
+		return -1;
+	}
+	ret = tpm_tis_request_locality(udev, 0);
+	if (ret)
+		return ret;
+
+	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;
+
+	/* Disable interrupts */
+	phy_ops->read32(udev, TPM_INT_ENABLE(chip->locality), &tmp);
+	tmp |= TPM_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT |
+	       TPM_INTF_DATA_AVAIL_INT | TPM_INTF_STS_VALID_INT;
+	tmp &= ~TPM_GLOBAL_INT_ENABLE;
+	phy_ops->write32(udev, TPM_INT_ENABLE(chip->locality), tmp);
+
+	phy_ops->read_bytes(udev, TPM_RID(chip->locality), 1, &chip->rid);
+	phy_ops->read32(udev, TPM_DID_VID(chip->locality), &chip->vend_dev);
+
+	return tpm_tis_release_locality(udev, chip->locality);
+}
+
+/**
+ * tpm_tis_close - Close the device and release locality
+ *
+ * @udev: udevice
+ *
+ * Return: 0 on success, negative on failure
+ */
+int tpm_tis_close(struct udevice *udev)
+{
+	struct tpm_chip *chip = dev_get_priv(udev);
+	int ret = 0;
+
+	if (chip->is_open) {
+		ret = tpm_tis_release_locality(udev, chip->locality);
+		chip->is_open = 0;
+	}
+
+	return ret;
+}
diff --git a/drivers/tpm/tpm_tis.h b/drivers/tpm/tpm_tis.h
index 2a160fe05c9a..28dd7f200329 100644
--- a/drivers/tpm/tpm_tis.h
+++ b/drivers/tpm/tpm_tis.h
@@ -21,6 +21,36 @@
 #include <linux/compiler.h>
 #include <linux/types.h>
 
+struct tpm_tis_phy_ops {
+	int (*read_bytes)(struct udevice *udev, u32 addr, u16 len,
+			  u8 *result);
+	int (*write_bytes)(struct udevice *udev, u32 addr, u16 len,
+			   const u8 *value);
+	int (*read32)(struct udevice *udev, u32 addr, u32 *result);
+	int (*write32)(struct udevice *udev, u32 addr, u32 src);
+};
+
+enum tis_int_flags {
+	TPM_GLOBAL_INT_ENABLE = 0x80000000,
+	TPM_INTF_BURST_COUNT_STATIC = 0x100,
+	TPM_INTF_CMD_READY_INT = 0x080,
+	TPM_INTF_INT_EDGE_FALLING = 0x040,
+	TPM_INTF_INT_EDGE_RISING = 0x020,
+	TPM_INTF_INT_LEVEL_LOW = 0x010,
+	TPM_INTF_INT_LEVEL_HIGH = 0x008,
+	TPM_INTF_LOCALITY_CHANGE_INT = 0x004,
+	TPM_INTF_STS_VALID_INT = 0x002,
+	TPM_INTF_DATA_AVAIL_INT = 0x001,
+};
+
+#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 TPM_INTF_CAPS(l)                (0x0014 | ((l) << 12))
+
 enum tpm_timeout {
 	TPM_TIMEOUT_MS			= 5,
 	TIS_SHORT_TIMEOUT_MS		= 750,
@@ -43,6 +73,7 @@ struct tpm_chip {
 	u8 rid;
 	unsigned long timeout_a, timeout_b, timeout_c, timeout_d;  /* msec */
 	ulong chip_type;
+	struct tpm_tis_phy_ops *phy_ops;
 };
 
 struct tpm_input_header {
@@ -130,4 +161,12 @@ enum tis_status {
 };
 #endif
 
+int tpm_tis_open(struct udevice *udev);
+int tpm_tis_close(struct udevice *udev);
+int tpm_tis_cleanup(struct udevice *udev);
+int tpm_tis_send(struct udevice *udev, const u8 *buf, size_t len);
+int tpm_tis_recv(struct udevice *udev, u8 *buf, size_t count);
+int tpm_tis_get_desc(struct udevice *udev, char *buf, int size);
+int tpm_tis_init(struct udevice *udev);
+void tpm_tis_ops_register(struct udevice *udev, struct tpm_tis_phy_ops *ops);
 #endif
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index 13b3db67c60f..e6b68769f3ff 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -396,6 +396,7 @@ enum {
 	TPM_STS_DATA_EXPECT		= 1 << 3,
 	TPM_STS_SELF_TEST_DONE		= 1 << 2,
 	TPM_STS_RESPONSE_RETRY		= 1 << 1,
+	TPM_STS_READ_ZERO               = 0x23
 };
 
 enum {
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/6 v4] tpm2: Add a TPMv2 MMIO TIS driver
  2021-11-03 15:09 [PATCH 0/6 v4] TPM cleanups and MMIO driver Ilias Apalodimas
  2021-11-03 15:09 ` [PATCH 1/6 v4] tpm2: Introduce TIS tpm core Ilias Apalodimas
@ 2021-11-03 15:09 ` Ilias Apalodimas
  2021-11-05  2:02   ` Simon Glass
  2021-11-03 15:09 ` [PATCH 3/6 v4] tpm: Use the new API on tpm2 spi driver Ilias Apalodimas
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Ilias Apalodimas @ 2021-11-03 15:09 UTC (permalink / raw)
  To: u-boot
  Cc: trini, Ilias Apalodimas, Rick Chen, Sean Anderson, Simon Glass,
	Heinrich Schuchardt, Masahisa Kojima

Add support for devices that expose a TPMv2 though MMIO.
Apart from those devices, we can use the driver in our QEMU setups and
test TPM related code which is difficult to achieve using the sandbox
driver (e.g test the EFI TCG2 protocol).

It's worth noting that a previous patch added TPMv2 TIS core functions,
which the current driver is consuming.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 drivers/tpm/Kconfig         |   9 +++
 drivers/tpm/Makefile        |   1 +
 drivers/tpm/tpm2_tis_mmio.c | 152 ++++++++++++++++++++++++++++++++++++
 3 files changed, 162 insertions(+)
 create mode 100644 drivers/tpm/tpm2_tis_mmio.c

diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig
index 9eebab5cfd90..406ee8716e1e 100644
--- a/drivers/tpm/Kconfig
+++ b/drivers/tpm/Kconfig
@@ -161,6 +161,15 @@ config TPM2_FTPM_TEE
 	help
 	  This driver supports firmware TPM running in TEE.
 
+config TPM2_MMIO
+	bool "MMIO based TPM2 Interface"
+	depends on TPM_V2
+	help
+	  This driver supports firmware TPM2.0 MMIO interface.
+	  The usual TPM operations and the 'tpm' command can be used to talk
+	  to the device using the standard TPM Interface Specification (TIS)
+	  protocol.
+
 endif # TPM_V2
 
 endmenu
diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
index c65be5267002..494aa5a46d30 100644
--- a/drivers/tpm/Makefile
+++ b/drivers/tpm/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_$(SPL_TPL_)TPM2_CR50_I2C) += cr50_i2c.o
 obj-$(CONFIG_TPM2_TIS_SANDBOX) += tpm2_tis_sandbox.o sandbox_common.o
 obj-$(CONFIG_TPM2_TIS_SPI) += tpm2_tis_spi.o
 obj-$(CONFIG_TPM2_FTPM_TEE) += tpm2_ftpm_tee.o
+obj-$(CONFIG_TPM2_MMIO) += tpm2_tis_core.o tpm2_tis_mmio.o
diff --git a/drivers/tpm/tpm2_tis_mmio.c b/drivers/tpm/tpm2_tis_mmio.c
new file mode 100644
index 000000000000..3bd0b0871a83
--- /dev/null
+++ b/drivers/tpm/tpm2_tis_mmio.c
@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * driver for mmio TCG/TIS TPM (trusted platform module).
+ *
+ * Specifications at www.trustedcomputinggroup.org
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <log.h>
+#include <tpm-v2.h>
+#include <linux/bitops.h>
+#include <linux/compiler.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/types.h>
+#include <linux/io.h>
+#include <linux/unaligned/be_byteshift.h>
+#include "tpm_tis.h"
+#include "tpm_internal.h"
+
+/**
+ * struct tpm_tis_chip_data - Information about an MMIO TPM
+ * @pcr_count:          Number of PCR per bank
+ * @pcr_select_min:	Minimum size in bytes of the pcrSelect array
+ * @iobase:		Base address
+ */
+struct tpm_tis_chip_data {
+	unsigned int pcr_count;
+	unsigned int pcr_select_min;
+	void __iomem *iobase;
+};
+
+static int mmio_read_bytes(struct udevice *udev, u32 addr, u16 len,
+			   u8 *result)
+{
+	struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
+
+	while (len--)
+		*result++ = ioread8(drv_data->iobase + addr);
+	return 0;
+}
+
+static int mmio_write_bytes(struct udevice *udev, u32 addr, u16 len,
+			    const u8 *value)
+{
+	struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
+
+	while (len--)
+		iowrite8(*value++, drv_data->iobase + addr);
+	return 0;
+}
+
+static int mmio_read32(struct udevice *udev, u32 addr, u32 *result)
+{
+	struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
+
+	*result = ioread32(drv_data->iobase + addr);
+	return 0;
+}
+
+static int mmio_write32(struct udevice *udev, u32 addr, u32 value)
+{
+	struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
+
+	iowrite32(value, drv_data->iobase + addr);
+	return 0;
+}
+
+static struct tpm_tis_phy_ops phy_ops = {
+	.read_bytes = mmio_read_bytes,
+	.write_bytes = mmio_write_bytes,
+	.read32 = mmio_read32,
+	.write32 = mmio_write32,
+};
+
+static int tpm_tis_probe(struct udevice *udev)
+{
+	struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
+	struct tpm_chip_priv *priv = dev_get_uclass_priv(udev);
+	int ret = 0;
+	fdt_addr_t ioaddr;
+	u64 sz;
+
+	ioaddr = dev_read_addr(udev);
+	if (ioaddr == FDT_ADDR_T_NONE)
+		return log_msg_ret("ioaddr", -EINVAL);
+
+	ret = dev_read_u64(udev, "reg", &sz);
+	if (ret)
+		return -EINVAL;
+
+	drv_data->iobase = ioremap(ioaddr, sz);
+	log_debug("Remapped TPM2 base: 0x%llx size: 0x%llx\n", ioaddr, sz);
+	tpm_tis_ops_register(udev, &phy_ops);
+	ret = tpm_tis_init(udev);
+	if (ret)
+		goto iounmap;
+
+	priv->pcr_count = drv_data->pcr_count;
+	priv->pcr_select_min = drv_data->pcr_select_min;
+	/*
+	 * Although the driver probably works with a TPMv1 our Kconfig
+	 * limits the driver to TPMv2 only
+	 */
+	priv->version = TPM_V2;
+
+	return ret;
+iounmap:
+	iounmap(drv_data->iobase);
+	return -EINVAL;
+}
+
+static int tpm_tis_remove(struct udevice *udev)
+{
+	struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
+
+	iounmap(drv_data->iobase);
+	return tpm_tis_cleanup(udev);
+}
+
+static const struct tpm_ops tpm_tis_ops = {
+	.open		= tpm_tis_open,
+	.close		= tpm_tis_close,
+	.get_desc	= tpm_tis_get_desc,
+	.send		= tpm_tis_send,
+	.recv		= tpm_tis_recv,
+	.cleanup	= tpm_tis_cleanup,
+};
+
+static const struct tpm_tis_chip_data tpm_tis_std_chip_data = {
+	.pcr_count = 24,
+	.pcr_select_min = 3,
+};
+
+static const struct udevice_id tpm_tis_ids[] = {
+	{
+		.compatible = "tcg,tpm-tis-mmio",
+		.data = (ulong)&tpm_tis_std_chip_data,
+	},
+	{ }
+};
+
+U_BOOT_DRIVER(tpm_tis_mmio) = {
+	.name   = "tpm_tis_mmio",
+	.id     = UCLASS_TPM,
+	.of_match = tpm_tis_ids,
+	.ops    = &tpm_tis_ops,
+	.probe	= tpm_tis_probe,
+	.remove	= tpm_tis_remove,
+	.priv_auto	= sizeof(struct tpm_chip),
+};
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/6 v4] tpm: Use the new API on tpm2 spi driver
  2021-11-03 15:09 [PATCH 0/6 v4] TPM cleanups and MMIO driver Ilias Apalodimas
  2021-11-03 15:09 ` [PATCH 1/6 v4] tpm2: Introduce TIS tpm core Ilias Apalodimas
  2021-11-03 15:09 ` [PATCH 2/6 v4] tpm2: Add a TPMv2 MMIO TIS driver Ilias Apalodimas
@ 2021-11-03 15:09 ` Ilias Apalodimas
  2021-11-05  2:02   ` Simon Glass
  2021-11-03 15:09 ` [PATCH 4/6 v4] configs: Enable tpmv2 mmio on qemu for arm/arm64 Ilias Apalodimas
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Ilias Apalodimas @ 2021-11-03 15:09 UTC (permalink / raw)
  To: u-boot
  Cc: trini, Ilias Apalodimas, Rick Chen, Sean Anderson, Simon Glass,
	Heinrich Schuchardt, Masahisa Kojima

Convert our SPI TPM driver and use the newly added API

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 drivers/tpm/Makefile       |   2 +-
 drivers/tpm/tpm2_tis_spi.c | 440 +++----------------------------------
 2 files changed, 32 insertions(+), 410 deletions(-)

diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
index 494aa5a46d30..51725230c780 100644
--- a/drivers/tpm/Makefile
+++ b/drivers/tpm/Makefile
@@ -12,6 +12,6 @@ obj-$(CONFIG_TPM_ST33ZP24_SPI) += tpm_tis_st33zp24_spi.o
 
 obj-$(CONFIG_$(SPL_TPL_)TPM2_CR50_I2C) += cr50_i2c.o
 obj-$(CONFIG_TPM2_TIS_SANDBOX) += tpm2_tis_sandbox.o sandbox_common.o
-obj-$(CONFIG_TPM2_TIS_SPI) += tpm2_tis_spi.o
+obj-$(CONFIG_TPM2_TIS_SPI) += tpm2_tis_core.o tpm2_tis_spi.o
 obj-$(CONFIG_TPM2_FTPM_TEE) += tpm2_ftpm_tee.o
 obj-$(CONFIG_TPM2_MMIO) += tpm2_tis_core.o tpm2_tis_mmio.o
diff --git a/drivers/tpm/tpm2_tis_spi.c b/drivers/tpm/tpm2_tis_spi.c
index 1d24d32d867e..547d163200ea 100644
--- a/drivers/tpm/tpm2_tis_spi.c
+++ b/drivers/tpm/tpm2_tis_spi.c
@@ -165,7 +165,7 @@ release_bus:
 	return ret;
 }
 
-static int tpm_tis_spi_read(struct udevice *dev, u16 addr, u8 *in, u16 len)
+static int tpm_tis_spi_read(struct udevice *dev, u32 addr, u16 len, u8 *in)
 {
 	return tpm_tis_spi_xfer(dev, addr, NULL, in, len);
 }
@@ -175,382 +175,24 @@ static 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));
+	ret = tpm_tis_spi_read(dev, addr, sizeof(u32), (u8 *)&result_le);
 	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 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) {
-		log(LOGC_NONE, LOGL_ERR, "%s: Failed to get locality: %d\n",
-		    __func__, ret);
-		return ret;
-	}
-
-	ret = tpm_tis_spi_write(dev, TPM_ACCESS(loc), &buf, 1);
-	if (ret) {
-		log(LOGC_NONE, LOGL_ERR, "%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) {
-			log(LOGC_NONE, LOGL_ERR,
-			    "%s: Failed to get locality: %d\n", __func__, ret);
-			return ret;
-		}
-
-		mdelay(TPM_TIMEOUT_MS);
-	} while (get_timer(start) < stop);
-
-	log(LOGC_NONE, LOGL_ERR, "%s: Timeout getting locality: %d\n", __func__,
-	    ret);
-
-	return ret;
-}
-
-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;
-}
-
-static u8 tpm_tis_spi_valid_status(struct udevice *dev, u8 *status)
-{
-	struct tpm_chip *chip = dev_get_priv(dev);
-
-	return tpm_tis_spi_wait_for_stat(dev, TPM_STS_VALID,
-		chip->timeout_c, status);
-}
-
-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;
-}
-
-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) {
-		log(LOGC_NONE, LOGL_ERR, "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) {
-		log(LOGC_NONE, LOGL_ERR,
-		    "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 at startup */
-		ret = tpm_tis_spi_cancel(dev);
-		if (ret) {
-			log(LOGC_NONE, LOGL_ERR,
-			    "%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)) {
-			log(LOGC_NONE, LOGL_ERR,
-			    "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_valid_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_valid_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)
+static int tpm_tis_spi_write(struct udevice *dev, u32 addr, u16 len, const u8 *out)
 {
-	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;
+	return tpm_tis_spi_xfer(dev, addr, out, NULL, len);
 }
 
-static int tpm_tis_get_desc(struct udevice *dev, char *buf, int size)
+static int tpm_tis_spi_write32(struct udevice *dev, u32 addr, u32 value)
 {
-	struct tpm_chip *chip = dev_get_priv(dev);
-
-	if (size < 80)
-		return -ENOSPC;
+	__le32 value_le = cpu_to_le32(value);
 
-	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"));
+	return tpm_tis_spi_write(dev, addr, sizeof(value), (u8 *)&value_le);
 }
 
 static int tpm_tis_wait_init(struct udevice *dev, int loc)
@@ -565,7 +207,7 @@ static int tpm_tis_wait_init(struct udevice *dev, int loc)
 	do {
 		mdelay(TPM_TIMEOUT_MS);
 
-		ret = tpm_tis_spi_read(dev, TPM_ACCESS(loc), &status, 1);
+		ret = tpm_tis_spi_read(dev, TPM_ACCESS(loc), 1, &status);
 		if (ret)
 			break;
 
@@ -576,6 +218,13 @@ static int tpm_tis_wait_init(struct udevice *dev, int loc)
 	return -EIO;
 }
 
+static struct tpm_tis_phy_ops phy_ops = {
+	.read_bytes = tpm_tis_spi_read,
+	.write_bytes = tpm_tis_spi_write,
+	.read32 = tpm_tis_spi_read32,
+	.write32 = tpm_tis_spi_write32,
+};
+
 static int tpm_tis_spi_probe(struct udevice *dev)
 {
 	struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(dev);
@@ -611,65 +260,38 @@ init:
 	/* Ensure a minimum amount of time elapsed since reset of the TPM */
 	mdelay(drv_data->time_before_first_cmd_ms);
 
-	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;
-	priv->pcr_count = drv_data->pcr_count;
-	priv->pcr_select_min = drv_data->pcr_select_min;
-
 	ret = tpm_tis_wait_init(dev, chip->locality);
 	if (ret) {
 		log(LOGC_DM, LOGL_ERR, "%s: no device found\n", __func__);
 		return ret;
 	}
 
-	ret = tpm_tis_spi_request_locality(dev, chip->locality);
-	if (ret) {
-		log(LOGC_NONE, LOGL_ERR, "%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) {
-		log(LOGC_NONE, LOGL_ERR,
-		    "%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) {
-		log(LOGC_NONE, LOGL_ERR, "%s: could not retrieve RevisionID\n",
-		    __func__);
-		return ret;
-	}
+	tpm_tis_ops_register(dev, &phy_ops);
+	ret = tpm_tis_init(dev);
+	if (ret)
+		goto err;
 
-	log(LOGC_NONE, LOGL_ERR,
-	    "SPI TPMv2.0 found (vid:%04x, did:%04x, rid:%02x)\n",
-	    chip->vend_dev & 0xFFFF, chip->vend_dev >> 16, chip->rid);
+	priv->pcr_count = drv_data->pcr_count;
+	priv->pcr_select_min = drv_data->pcr_select_min;
+	priv->version = TPM_V2;
 
 	return 0;
+err:
+	return -EINVAL;
 }
 
-static int tpm_tis_spi_remove(struct udevice *dev)
+static int tpm_tis_spi_remove(struct udevice *udev)
 {
-	struct tpm_chip *chip = dev_get_priv(dev);
-
-	tpm_tis_spi_release_locality(dev, chip->locality, true);
-
-	return 0;
+	return tpm_tis_cleanup(udev);
 }
 
 static const struct tpm_ops tpm_tis_spi_ops = {
-	.open		= tpm_tis_spi_open,
-	.close		= tpm_tis_spi_close,
+	.open		= tpm_tis_open,
+	.close		= tpm_tis_close,
 	.get_desc	= tpm_tis_get_desc,
-	.send		= tpm_tis_spi_send,
-	.recv		= tpm_tis_spi_recv,
-	.cleanup	= tpm_tis_spi_cleanup,
+	.send		= tpm_tis_send,
+	.recv		= tpm_tis_recv,
+	.cleanup	= tpm_tis_cleanup,
 };
 
 static const struct tpm_tis_chip_data tpm_tis_std_chip_data = {
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/6 v4] configs: Enable tpmv2 mmio on qemu for arm/arm64
  2021-11-03 15:09 [PATCH 0/6 v4] TPM cleanups and MMIO driver Ilias Apalodimas
                   ` (2 preceding siblings ...)
  2021-11-03 15:09 ` [PATCH 3/6 v4] tpm: Use the new API on tpm2 spi driver Ilias Apalodimas
@ 2021-11-03 15:09 ` Ilias Apalodimas
  2021-11-05  2:02   ` Simon Glass
  2021-11-03 15:09 ` [PATCH 5/6 v4] doc: qemu: Add instructions for swtpm usage Ilias Apalodimas
  2021-11-03 15:09 ` [PATCH 6/6 v4] MAINTAINERS: Add entry for TPM drivers Ilias Apalodimas
  5 siblings, 1 reply; 17+ messages in thread
From: Ilias Apalodimas @ 2021-11-03 15:09 UTC (permalink / raw)
  To: u-boot
  Cc: trini, Ilias Apalodimas, Rick Chen, Sean Anderson, Simon Glass,
	Heinrich Schuchardt, Masahisa Kojima

A previous commit is adding an MMIO TPMv2 driver.  Include in the default
qemu arm configs, since we plan on using them on EFI testing

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 configs/qemu_arm64_defconfig | 2 ++
 configs/qemu_arm_defconfig   | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig
index 003717efde28..83d7ae54de4d 100644
--- a/configs/qemu_arm64_defconfig
+++ b/configs/qemu_arm64_defconfig
@@ -49,6 +49,8 @@ CONFIG_SCSI=y
 CONFIG_DM_SCSI=y
 CONFIG_SYSRESET=y
 CONFIG_SYSRESET_PSCI=y
+CONFIG_TPM2_MMIO=y
 CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_PCI=y
+CONFIG_TPM=y
diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig
index 27b0e49f6f89..ab5574847e89 100644
--- a/configs/qemu_arm_defconfig
+++ b/configs/qemu_arm_defconfig
@@ -51,6 +51,8 @@ CONFIG_SCSI=y
 CONFIG_DM_SCSI=y
 CONFIG_SYSRESET=y
 CONFIG_SYSRESET_PSCI=y
+CONFIG_TPM2_MMIO=y
 CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_PCI=y
+CONFIG_TPM=y
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/6 v4] doc: qemu: Add instructions for swtpm usage
  2021-11-03 15:09 [PATCH 0/6 v4] TPM cleanups and MMIO driver Ilias Apalodimas
                   ` (3 preceding siblings ...)
  2021-11-03 15:09 ` [PATCH 4/6 v4] configs: Enable tpmv2 mmio on qemu for arm/arm64 Ilias Apalodimas
@ 2021-11-03 15:09 ` Ilias Apalodimas
  2021-11-05  2:02   ` Simon Glass
  2021-11-03 15:09 ` [PATCH 6/6 v4] MAINTAINERS: Add entry for TPM drivers Ilias Apalodimas
  5 siblings, 1 reply; 17+ messages in thread
From: Ilias Apalodimas @ 2021-11-03 15:09 UTC (permalink / raw)
  To: u-boot
  Cc: trini, Ilias Apalodimas, Rick Chen, Sean Anderson, Simon Glass,
	Heinrich Schuchardt, Masahisa Kojima

A previous patch added support for an mmio based TPM.
Add an example in QEMU on it's usage

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 doc/board/emulation/qemu-arm.rst | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/doc/board/emulation/qemu-arm.rst b/doc/board/emulation/qemu-arm.rst
index 8d7fda10f15e..584ef0a7e150 100644
--- a/doc/board/emulation/qemu-arm.rst
+++ b/doc/board/emulation/qemu-arm.rst
@@ -81,6 +81,31 @@ can be enabled with the following command line parameters:
 
 These have been tested in QEMU 2.9.0 but should work in at least 2.5.0 as well.
 
+Enabling TPMv2 support
+----------------------
+
+To emulate a TPM the swtpm package may be used. It can be built from the
+following repositories:
+
+     https://github.com/stefanberger/swtpm.git
+
+Swtpm provides a socket for the TPM emulation which can be consumed by QEMU.
+
+In a first console invoke swtpm with::
+
+     swtpm socket --tpmstate dir=/tmp/mytpm1   \
+     --ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock --log level=20
+
+In a second console invoke qemu-system-aarch64 with::
+
+     -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
+     -tpmdev emulator,id=tpm0,chardev=chrtpm \
+     -device tpm-tis-device,tpmdev=tpm0
+
+Enable the TPM on U-Boot's command line with::
+
+    tpm2 startup TPM2_SU_CLEAR
+
 Debug UART
 ----------
 
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/6 v4] MAINTAINERS: Add entry for TPM drivers
  2021-11-03 15:09 [PATCH 0/6 v4] TPM cleanups and MMIO driver Ilias Apalodimas
                   ` (4 preceding siblings ...)
  2021-11-03 15:09 ` [PATCH 5/6 v4] doc: qemu: Add instructions for swtpm usage Ilias Apalodimas
@ 2021-11-03 15:09 ` Ilias Apalodimas
  5 siblings, 0 replies; 17+ messages in thread
From: Ilias Apalodimas @ 2021-11-03 15:09 UTC (permalink / raw)
  To: u-boot
  Cc: trini, Ilias Apalodimas, Heinrich Schuchardt, Simon Glass,
	Rick Chen, Sean Anderson, Masahisa Kojima

TPM drivers have currently no maintainers.  Add myself since I contributed
the TIS implementation.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 MAINTAINERS | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9d8cba902800..f02901c55de5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1183,6 +1183,11 @@ F:	configs/am65x_hs_evm_a53_defconfig
 F:	configs/j721e_hs_evm_r5_defconfig
 F:	configs/j721e_hs_evm_a72_defconfig
 
+TPM DRIVERS
+M:	Ilias Apalodimas <ilias.apalodimas@linaro.org>
+S:	Maintained
+F:	drivers/tpm/
+
 TQ GROUP
 #M:	Martin Krause <martin.krause@tq-systems.de>
 S:	Orphaned (Since 2016-02)
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/6 v4] tpm2: Introduce TIS tpm core
  2021-11-03 15:09 ` [PATCH 1/6 v4] tpm2: Introduce TIS tpm core Ilias Apalodimas
@ 2021-11-05  2:02   ` Simon Glass
  2021-11-05  7:01     ` Ilias Apalodimas
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2021-11-05  2:02 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: u-boot, trini, Rick Chen, Sean Anderson, Heinrich Schuchardt,
	Masahisa Kojima

Hi Ilias,

On Wed, 3 Nov 2021 at 09:09, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> There's a lot of code duplication in U-Boot right now.  All the TPM TIS
> compatible drivers we have at the moment have their own copy of a TIS
> implementation.
>
> So let's create a common layer which implements the core TIS functions.
> Any driver added from now own, which is compatible with the TIS spec, will
> only have to provide the underlying bus communication mechanisms.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  drivers/tpm/tpm2_tis_core.c | 523 ++++++++++++++++++++++++++++++++++++
>  drivers/tpm/tpm_tis.h       |  39 +++
>  include/tpm-v2.h            |   1 +
>  3 files changed, 563 insertions(+)
>  create mode 100644 drivers/tpm/tpm2_tis_core.c
>
> diff --git a/drivers/tpm/tpm2_tis_core.c b/drivers/tpm/tpm2_tis_core.c
> new file mode 100644
> index 000000000000..7d7050151622
> --- /dev/null
> +++ b/drivers/tpm/tpm2_tis_core.c
> @@ -0,0 +1,523 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020, Linaro Limited
> + *
> + * Based on the Linux TIS core interface and U-Boot original SPI TPM driver
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <tpm-v2.h>
> +#include <linux/delay.h>
> +#include <linux/unaligned/be_byteshift.h>
> +#include "tpm_tis.h"
> +
> +/**
> + * tpm_tis_get_desc - Get the TPM description
> + *
> + * @udev: udevice

TPM device

> + * @buf:  buffer to fill data
> + * @size: buffer size
> + *
> + * @Return: Number of characters written (or would have been written) in buffer
> + */
> +int tpm_tis_get_desc(struct udevice *udev, char *buf, int size)

Please use dev throughout, to be consistent with how driver model is used

> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +
> +       if (size < 80)
> +               return -ENOSPC;
> +
> +       return snprintf(buf, size,
> +                       "%s v2.0: VendorID 0x%04x, DeviceID 0x%04x, RevisionID 0x%02x [%s]",
> +                       udev->name, chip->vend_dev & 0xFFFF,
> +                       chip->vend_dev >> 16, chip->rid,
> +                       (chip->is_open ? "open" : "closed"));
> +}
> +
> +/**
> + * tpm_tis_check_locality - Check the current TPM locality
> + *
> + * @udev: udevice
> + * @loc:  locality
> + *
> + * Return: True if the tested locality matches
> + */
> +static bool tpm_tis_check_locality(struct udevice *udev, int loc)
> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +       struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
> +       u8 locality;
> +
> +       phy_ops->read_bytes(udev, TPM_ACCESS(loc), 1, &locality);
> +       if ((locality & (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID |
> +           TPM_ACCESS_REQUEST_USE)) ==
> +           (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) {
> +               chip->locality = loc;
> +               return true;
> +       }
> +
> +       return false;
> +}
> +
> +/**
> + * tpm_tis_request_locality - Request a locality from the TPM
> + *
> + * @udev: udev
> + * @loc:  requested locality
> + *
> + * Return: 0 on success -1 on failure
> + */
> +int tpm_tis_request_locality(struct udevice *udev, int loc)
> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +       struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
> +       u8 buf = TPM_ACCESS_REQUEST_USE;
> +       unsigned long start, stop;
> +
> +       if (tpm_tis_check_locality(udev, loc))
> +               return 0;
> +
> +       phy_ops->write_bytes(udev, TPM_ACCESS(loc), 1, &buf);
> +       start = get_timer(0);
> +       stop = chip->timeout_a;
> +       do {
> +               if (tpm_tis_check_locality(udev, loc))
> +                       return 0;
> +               mdelay(TPM_TIMEOUT_MS);
> +       } while (get_timer(start) < stop);
> +
> +       return -1;
> +}
> +
> +/**
> + * tpm_tis_status - Check the current device status
> + *
> + * @udev:   udevice
> + * @status: return value of status
> + *
> + * Return: 0 on success, negative on failure
> + */
> +static int tpm_tis_status(struct udevice *udev, u8 *status)
> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +       struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
> +
> +       if (chip->locality < 0)
> +               return -EINVAL;
> +
> +       phy_ops->read_bytes(udev, TPM_STS(chip->locality), 1, status);
> +
> +       if ((*status & TPM_STS_READ_ZERO)) {
> +               log_err("TPM returned invalid status\n");
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * tpm_tis_release_locality - Release the requested locality
> + *
> + * @udev: udevice
> + * @loc:  requested locality
> + *
> + * Return: 0 on success, negative on failure
> + */
> +int tpm_tis_release_locality(struct udevice *udev, int loc)
> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +       struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
> +       u8 buf = TPM_ACCESS_ACTIVE_LOCALITY;
> +       int ret;
> +
> +       if (chip->locality < 0)
> +               return 0;
> +
> +       ret = phy_ops->write_bytes(udev, TPM_ACCESS(loc), 1, &buf);
> +       chip->locality = -1;
> +
> +       return ret;
> +}
> +
> +/**
> + * tpm_tis_wait_for_stat - Wait for TPM to become ready
> + *
> + * @udev:    udev
> + * @mask:    mask to match
> + * @timeout: timeout for retries
> + * @status:  current status
> + *
> + * Return: 0 on success, negative on failure
> + */
> +static int tpm_tis_wait_for_stat(struct udevice *udev, 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_status(udev, status);
> +               if (ret)
> +                       return ret;
> +
> +               if ((*status & mask) == mask)
> +                       return 0;
> +       } while (get_timer(start) < stop);
> +
> +       return -ETIMEDOUT;
> +}
> +
> +/**
> + * tpm_tis_get_burstcount - Get the burstcount for the data FIFO
> + *
> + * @udev:       udevice
> + * @burstcount: current burstcount
> + *
> + * Return: 0 on success, negative on failure
> + */
> +static int tpm_tis_get_burstcount(struct udevice *udev, size_t *burstcount)
> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +       struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
> +       unsigned long start, stop;
> +       u32 burst;
> +
> +       if (chip->locality < 0)
> +               return -EINVAL;
> +
> +       /* wait for burstcount */
> +       start = get_timer(0);
> +       /*
> +        * This is the TPMv2 defined timeout. Change this in case you want to
> +        * make the driver compatile to TPMv1
> +        */
> +       stop = chip->timeout_a;
> +       do {
> +               phy_ops->read32(udev, TPM_STS(chip->locality), &burst);
> +               *burstcount = (burst >> 8) & 0xFFFF;
> +               if (*burstcount)
> +                       return 0;
> +
> +               mdelay(TPM_TIMEOUT_MS);
> +       } while (get_timer(start) < stop);
> +
> +       return -ETIMEDOUT;
> +}
> +
> +/**
> + * tpm_tis_ready - Cancel pending comands and get the device on a ready state
> + *
> + * @udev: udevcie
> + *
> + * Return: 0 on success, negative on failure
> + */
> +static int tpm_tis_ready(struct udevice *udev)
> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +       struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
> +       u8 data = TPM_STS_COMMAND_READY;
> +
> +       /* This will cancel any pending commands */
> +       return phy_ops->write_bytes(udev, TPM_STS(chip->locality), 1, &data);
> +}
> +
> +/**
> + * tpm_tis_send - send data to the device
> + *
> + * @udev: udevice
> + * @buf:  buffer to send
> + * @len:  size of the buffer
> + *
> + * Return: number of bytes sent or negative on failure
> + */
> +int tpm_tis_send(struct udevice *udev, const u8 *buf, size_t len)
> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +       struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
> +       size_t burstcnt, wr_size, sent = 0;
> +       u8 data = TPM_STS_GO;
> +       u8 status;
> +       int ret;
> +
> +       if (!chip)
> +               return -ENODEV;
> +
> +       ret = tpm_tis_request_locality(udev, 0);
> +       if (ret < 0)
> +               return -EBUSY;
> +
> +       ret = tpm_tis_status(udev, &status);
> +       if (ret)
> +               goto release_locality;
> +
> +       if (!(status & TPM_STS_COMMAND_READY)) {
> +               ret = tpm_tis_ready(udev);
> +               if (ret) {
> +                       log_err("Can't cancel previous TPM operation\n");
> +                       goto release_locality;
> +               }
> +               ret = tpm_tis_wait_for_stat(udev, TPM_STS_COMMAND_READY,
> +                                           chip->timeout_b, &status);
> +               if (ret) {
> +                       log_err("TPM not ready\n");
> +                       goto release_locality;
> +               }
> +       }
> +
> +       while (len > 0) {
> +               ret = tpm_tis_get_burstcount(udev, &burstcnt);
> +               if (ret)
> +                       goto release_locality;
> +
> +               wr_size = min(len, burstcnt);
> +               ret = phy_ops->write_bytes(udev, TPM_DATA_FIFO(chip->locality),
> +                                          wr_size, buf + sent);
> +               if (ret < 0)
> +                       goto release_locality;
> +
> +               ret = tpm_tis_wait_for_stat(udev, TPM_STS_VALID,
> +                                           chip->timeout_c, &status);
> +               if (ret)
> +                       goto release_locality;
> +
> +               sent += wr_size;
> +               len -= wr_size;
> +               /* make sure the TPM expects more data */
> +               if (len && !(status & TPM_STS_DATA_EXPECT)) {
> +                       ret = -EIO;
> +                       goto release_locality;
> +               }
> +       }
> +
> +       /*
> +        * Make a final check ensuring everything is ok and the TPM expects no
> +        * more data
> +        */
> +       ret = tpm_tis_wait_for_stat(udev, TPM_STS_VALID, chip->timeout_c,
> +                                   &status);
> +       if (ret)
> +               goto release_locality;
> +
> +       if (status & TPM_STS_DATA_EXPECT) {
> +               ret = -EIO;
> +               goto release_locality;
> +       }
> +
> +       ret = phy_ops->write_bytes(udev, TPM_STS(chip->locality), 1, &data);
> +       if (ret)
> +               goto release_locality;
> +
> +       return sent;
> +
> +release_locality:
> +       tpm_tis_ready(udev);
> +       tpm_tis_release_locality(udev, chip->locality);
> +
> +       return ret;
> +}
> +
> +/**
> + * tpm_tis_recv_data - Receive data from a device. Wrapper for tpm_tis_recv
> + *
> + * @udev: udevice
> + * @buf:  buffer to copy data
> + * @size: buffer size
> + *
> + * Return: bytes read or negative on failure
> + */
> +static int tpm_tis_recv_data(struct udevice *udev, u8 *buf, size_t count)
> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +       struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
> +       int size = 0, len, ret;
> +       size_t burstcnt;
> +       u8 status;
> +
> +       while (size < count &&
> +              tpm_tis_wait_for_stat(udev, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
> +                                    chip->timeout_c, &status) == 0) {
> +               ret = tpm_tis_get_burstcount(udev, &burstcnt);
> +               if (ret)
> +                       return ret;
> +
> +               len = min_t(int, burstcnt, count - size);
> +               ret = phy_ops->read_bytes(udev, TPM_DATA_FIFO(chip->locality),
> +                                         len, buf + size);
> +               if (ret < 0)
> +                       return ret;
> +
> +               size += len;
> +       }
> +
> +       return size;
> +}
> +
> +/**
> + * tpm_tis_recv - Receive data from a device
> + *
> + * @udev: udevice
> + * @buf:  buffer to copy data
> + * @size: buffer size
> + *
> + * Return: bytes read or negative on failure
> + */
> +int tpm_tis_recv(struct udevice *udev, u8 *buf, size_t count)
> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +       int size, expected;
> +
> +       if (count < TPM_HEADER_SIZE)
> +               return -E2BIG;
> +
> +       size = tpm_tis_recv_data(udev, buf, TPM_HEADER_SIZE);
> +       if (size < TPM_HEADER_SIZE) {
> +               log_err("TPM error, unable to read header\n");
> +               goto out;
> +       }
> +
> +       expected = get_unaligned_be32(buf + TPM_CMD_COUNT_OFFSET);
> +       if (expected > count) {
> +               size = -EIO;
> +               log_warning("Too much data: %d > %zu\n", expected, count);
> +               goto out;
> +       }
> +
> +       size += tpm_tis_recv_data(udev, &buf[TPM_HEADER_SIZE],
> +                                  expected - TPM_HEADER_SIZE);
> +       if (size < expected) {
> +               log(LOGC_NONE, LOGL_ERR,
> +                   "TPM error, unable to read remaining bytes of result\n");
> +               size = -EIO;
> +               goto out;
> +       }
> +
> +out:
> +       tpm_tis_ready(udev);
> +       /* acquired in tpm_tis_send */
> +       tpm_tis_release_locality(udev, chip->locality);
> +
> +       return size;
> +}
> +
> +/** tpm_tis_cleanup - Get the device in ready state and release locality
> + *
> + * @udev: udevice
> + *
> + * Return: always 0
> + */
> +int tpm_tis_cleanup(struct udevice *udev)
> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +
> +       tpm_tis_ready(udev);
> +       tpm_tis_release_locality(udev, chip->locality);
> +
> +       return 0;
> +}
> +
> +/**
> + * tpm_tis_open - Open the device and request locality 0
> + *
> + * @udev: udevice
> + *
> + * Return: 0 on success, negative on failure
> + */
> +int tpm_tis_open(struct udevice *udev)
> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +       int ret;
> +
> +       if (chip->is_open)
> +               return -EBUSY;
> +
> +       ret = tpm_tis_request_locality(udev, 0);
> +       if (!ret)
> +               chip->is_open = 1;
> +
> +       return ret;
> +}
> +
> +/**
> + * tpm_tis_ops_register - register the PHY ops for the device
> + *
> + * @udev: udevice
> + * @ops: bus ops for the device
> + */
> +void tpm_tis_ops_register(struct udevice *udev, struct tpm_tis_phy_ops *ops)
> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +
> +       chip->phy_ops = ops;
> +}
> +
> +static bool tis_check_ops(struct tpm_tis_phy_ops *phy_ops)
> +{
> +       if (!phy_ops || !phy_ops->read_bytes || !phy_ops->write_bytes ||
> +           !phy_ops->read32 || !phy_ops->write32)
> +               return false;
> +
> +       return true;
> +}
> +
> +/**
> + * tpm_tis_init - inititalize the device
> + *
> + * @udev: udevice
> + *
> + * Return: 0 on success, negative on failure
> + */
> +int tpm_tis_init(struct udevice *udev)
> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +       struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
> +       int ret;
> +       u32 tmp;
> +
> +       if (!tis_check_ops(phy_ops)) {
> +               log_err("Driver bug. No bus ops defined\n");
> +               return -1;
> +       }
> +       ret = tpm_tis_request_locality(udev, 0);
> +       if (ret)
> +               return ret;
> +
> +       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;
> +
> +       /* Disable interrupts */
> +       phy_ops->read32(udev, TPM_INT_ENABLE(chip->locality), &tmp);
> +       tmp |= TPM_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT |
> +              TPM_INTF_DATA_AVAIL_INT | TPM_INTF_STS_VALID_INT;
> +       tmp &= ~TPM_GLOBAL_INT_ENABLE;
> +       phy_ops->write32(udev, TPM_INT_ENABLE(chip->locality), tmp);
> +
> +       phy_ops->read_bytes(udev, TPM_RID(chip->locality), 1, &chip->rid);
> +       phy_ops->read32(udev, TPM_DID_VID(chip->locality), &chip->vend_dev);
> +
> +       return tpm_tis_release_locality(udev, chip->locality);
> +}
> +
> +/**
> + * tpm_tis_close - Close the device and release locality
> + *
> + * @udev: udevice
> + *
> + * Return: 0 on success, negative on failure
> + */
> +int tpm_tis_close(struct udevice *udev)
> +{
> +       struct tpm_chip *chip = dev_get_priv(udev);
> +       int ret = 0;
> +
> +       if (chip->is_open) {
> +               ret = tpm_tis_release_locality(udev, chip->locality);
> +               chip->is_open = 0;
> +       }
> +
> +       return ret;
> +}
> diff --git a/drivers/tpm/tpm_tis.h b/drivers/tpm/tpm_tis.h
> index 2a160fe05c9a..28dd7f200329 100644
> --- a/drivers/tpm/tpm_tis.h
> +++ b/drivers/tpm/tpm_tis.h
> @@ -21,6 +21,36 @@
>  #include <linux/compiler.h>
>  #include <linux/types.h>
>
> +struct tpm_tis_phy_ops {
> +       int (*read_bytes)(struct udevice *udev, u32 addr, u16 len,
> +                         u8 *result);
> +       int (*write_bytes)(struct udevice *udev, u32 addr, u16 len,
> +                          const u8 *value);
> +       int (*read32)(struct udevice *udev, u32 addr, u32 *result);
> +       int (*write32)(struct udevice *udev, u32 addr, u32 src);
> +};
> +
> +enum tis_int_flags {
> +       TPM_GLOBAL_INT_ENABLE = 0x80000000,
> +       TPM_INTF_BURST_COUNT_STATIC = 0x100,
> +       TPM_INTF_CMD_READY_INT = 0x080,
> +       TPM_INTF_INT_EDGE_FALLING = 0x040,
> +       TPM_INTF_INT_EDGE_RISING = 0x020,
> +       TPM_INTF_INT_LEVEL_LOW = 0x010,
> +       TPM_INTF_INT_LEVEL_HIGH = 0x008,
> +       TPM_INTF_LOCALITY_CHANGE_INT = 0x004,
> +       TPM_INTF_STS_VALID_INT = 0x002,
> +       TPM_INTF_DATA_AVAIL_INT = 0x001,
> +};
> +
> +#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 TPM_INTF_CAPS(l)                (0x0014 | ((l) << 12))

lower-case hex

> +
>  enum tpm_timeout {
>         TPM_TIMEOUT_MS                  = 5,
>         TIS_SHORT_TIMEOUT_MS            = 750,
> @@ -43,6 +73,7 @@ struct tpm_chip {
>         u8 rid;
>         unsigned long timeout_a, timeout_b, timeout_c, timeout_d;  /* msec */
>         ulong chip_type;
> +       struct tpm_tis_phy_ops *phy_ops;

comment?

>  };
>
>  struct tpm_input_header {
> @@ -130,4 +161,12 @@ enum tis_status {
>  };
>  #endif
>
> +int tpm_tis_open(struct udevice *udev);

Please add comments

> +int tpm_tis_close(struct udevice *udev);
> +int tpm_tis_cleanup(struct udevice *udev);
> +int tpm_tis_send(struct udevice *udev, const u8 *buf, size_t len);
> +int tpm_tis_recv(struct udevice *udev, u8 *buf, size_t count);
> +int tpm_tis_get_desc(struct udevice *udev, char *buf, int size);
> +int tpm_tis_init(struct udevice *udev);
> +void tpm_tis_ops_register(struct udevice *udev, struct tpm_tis_phy_ops *ops);
>  #endif
> diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> index 13b3db67c60f..e6b68769f3ff 100644
> --- a/include/tpm-v2.h
> +++ b/include/tpm-v2.h
> @@ -396,6 +396,7 @@ enum {
>         TPM_STS_DATA_EXPECT             = 1 << 3,
>         TPM_STS_SELF_TEST_DONE          = 1 << 2,
>         TPM_STS_RESPONSE_RETRY          = 1 << 1,
> +       TPM_STS_READ_ZERO               = 0x23
>  };
>
>  enum {
> --
> 2.33.1
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/6 v4] tpm2: Add a TPMv2 MMIO TIS driver
  2021-11-03 15:09 ` [PATCH 2/6 v4] tpm2: Add a TPMv2 MMIO TIS driver Ilias Apalodimas
@ 2021-11-05  2:02   ` Simon Glass
  2021-11-05  8:17     ` Ilias Apalodimas
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2021-11-05  2:02 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: u-boot, trini, Rick Chen, Sean Anderson, Heinrich Schuchardt,
	Masahisa Kojima

Hi Ilias,

On Wed, 3 Nov 2021 at 09:09, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Add support for devices that expose a TPMv2 though MMIO.
> Apart from those devices, we can use the driver in our QEMU setups and
> test TPM related code which is difficult to achieve using the sandbox
> driver (e.g test the EFI TCG2 protocol).
>
> It's worth noting that a previous patch added TPMv2 TIS core functions,
> which the current driver is consuming.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  drivers/tpm/Kconfig         |   9 +++
>  drivers/tpm/Makefile        |   1 +
>  drivers/tpm/tpm2_tis_mmio.c | 152 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 162 insertions(+)
>  create mode 100644 drivers/tpm/tpm2_tis_mmio.c
>
> diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig
> index 9eebab5cfd90..406ee8716e1e 100644
> --- a/drivers/tpm/Kconfig
> +++ b/drivers/tpm/Kconfig
> @@ -161,6 +161,15 @@ config TPM2_FTPM_TEE
>         help
>           This driver supports firmware TPM running in TEE.
>
> +config TPM2_MMIO
> +       bool "MMIO based TPM2 Interface"
> +       depends on TPM_V2
> +       help
> +         This driver supports firmware TPM2.0 MMIO interface.
> +         The usual TPM operations and the 'tpm' command can be used to talk
> +         to the device using the standard TPM Interface Specification (TIS)
> +         protocol.
> +
>  endif # TPM_V2
>
>  endmenu
> diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
> index c65be5267002..494aa5a46d30 100644
> --- a/drivers/tpm/Makefile
> +++ b/drivers/tpm/Makefile
> @@ -14,3 +14,4 @@ obj-$(CONFIG_$(SPL_TPL_)TPM2_CR50_I2C) += cr50_i2c.o
>  obj-$(CONFIG_TPM2_TIS_SANDBOX) += tpm2_tis_sandbox.o sandbox_common.o
>  obj-$(CONFIG_TPM2_TIS_SPI) += tpm2_tis_spi.o
>  obj-$(CONFIG_TPM2_FTPM_TEE) += tpm2_ftpm_tee.o
> +obj-$(CONFIG_TPM2_MMIO) += tpm2_tis_core.o tpm2_tis_mmio.o
> diff --git a/drivers/tpm/tpm2_tis_mmio.c b/drivers/tpm/tpm2_tis_mmio.c
> new file mode 100644
> index 000000000000..3bd0b0871a83
> --- /dev/null
> +++ b/drivers/tpm/tpm2_tis_mmio.c
> @@ -0,0 +1,152 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * driver for mmio TCG/TIS TPM (trusted platform module).
> + *
> + * Specifications at www.trustedcomputinggroup.org
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <log.h>
> +#include <tpm-v2.h>
> +#include <linux/bitops.h>
> +#include <linux/compiler.h>
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/types.h>
> +#include <linux/io.h>
> +#include <linux/unaligned/be_byteshift.h>
> +#include "tpm_tis.h"
> +#include "tpm_internal.h"
> +
> +/**
> + * struct tpm_tis_chip_data - Information about an MMIO TPM
> + * @pcr_count:          Number of PCR per bank
> + * @pcr_select_min:    Minimum size in bytes of the pcrSelect array
> + * @iobase:            Base address
> + */
> +struct tpm_tis_chip_data {
> +       unsigned int pcr_count;
> +       unsigned int pcr_select_min;
> +       void __iomem *iobase;
> +};
> +
> +static int mmio_read_bytes(struct udevice *udev, u32 addr, u16 len,

dev again

> +                          u8 *result)
> +{
> +       struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
> +
> +       while (len--)
> +               *result++ = ioread8(drv_data->iobase + addr);
> +       return 0;
> +}
> +
> +static int mmio_write_bytes(struct udevice *udev, u32 addr, u16 len,
> +                           const u8 *value)
> +{
> +       struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
> +
> +       while (len--)
> +               iowrite8(*value++, drv_data->iobase + addr);

So should this use regmap?

> +       return 0;
> +}
[..]

Regards,
Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/6 v4] tpm: Use the new API on tpm2 spi driver
  2021-11-03 15:09 ` [PATCH 3/6 v4] tpm: Use the new API on tpm2 spi driver Ilias Apalodimas
@ 2021-11-05  2:02   ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2021-11-05  2:02 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: u-boot, trini, Rick Chen, Sean Anderson, Heinrich Schuchardt,
	Masahisa Kojima

On Wed, 3 Nov 2021 at 09:09, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Convert our SPI TPM driver and use the newly added API
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  drivers/tpm/Makefile       |   2 +-
>  drivers/tpm/tpm2_tis_spi.c | 440 +++----------------------------------
>  2 files changed, 32 insertions(+), 410 deletions(-)
>

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/6 v4] configs: Enable tpmv2 mmio on qemu for arm/arm64
  2021-11-03 15:09 ` [PATCH 4/6 v4] configs: Enable tpmv2 mmio on qemu for arm/arm64 Ilias Apalodimas
@ 2021-11-05  2:02   ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2021-11-05  2:02 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: u-boot, trini, Rick Chen, Sean Anderson, Heinrich Schuchardt,
	Masahisa Kojima

On Wed, 3 Nov 2021 at 09:09, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> A previous commit is adding an MMIO TPMv2 driver.  Include in the default
> qemu arm configs, since we plan on using them on EFI testing
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  configs/qemu_arm64_defconfig | 2 ++
>  configs/qemu_arm_defconfig   | 2 ++
>  2 files changed, 4 insertions(+)

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/6 v4] doc: qemu: Add instructions for swtpm usage
  2021-11-03 15:09 ` [PATCH 5/6 v4] doc: qemu: Add instructions for swtpm usage Ilias Apalodimas
@ 2021-11-05  2:02   ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2021-11-05  2:02 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: u-boot, trini, Rick Chen, Sean Anderson, Heinrich Schuchardt,
	Masahisa Kojima

On Wed, 3 Nov 2021 at 09:09, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> A previous patch added support for an mmio based TPM.
> Add an example in QEMU on it's usage
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  doc/board/emulation/qemu-arm.rst | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>

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

Any particular QEMU version? We should enable this in CI.

> diff --git a/doc/board/emulation/qemu-arm.rst b/doc/board/emulation/qemu-arm.rst
> index 8d7fda10f15e..584ef0a7e150 100644
> --- a/doc/board/emulation/qemu-arm.rst
> +++ b/doc/board/emulation/qemu-arm.rst
> @@ -81,6 +81,31 @@ can be enabled with the following command line parameters:
>
>  These have been tested in QEMU 2.9.0 but should work in at least 2.5.0 as well.
>
> +Enabling TPMv2 support
> +----------------------
> +
> +To emulate a TPM the swtpm package may be used. It can be built from the
> +following repositories:
> +
> +     https://github.com/stefanberger/swtpm.git
> +
> +Swtpm provides a socket for the TPM emulation which can be consumed by QEMU.
> +
> +In a first console invoke swtpm with::
> +
> +     swtpm socket --tpmstate dir=/tmp/mytpm1   \
> +     --ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock --log level=20
> +
> +In a second console invoke qemu-system-aarch64 with::
> +
> +     -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
> +     -tpmdev emulator,id=tpm0,chardev=chrtpm \
> +     -device tpm-tis-device,tpmdev=tpm0
> +
> +Enable the TPM on U-Boot's command line with::
> +
> +    tpm2 startup TPM2_SU_CLEAR
> +
>  Debug UART
>  ----------
>
> --
> 2.33.1
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/6 v4] tpm2: Introduce TIS tpm core
  2021-11-05  2:02   ` Simon Glass
@ 2021-11-05  7:01     ` Ilias Apalodimas
  2021-11-05 16:12       ` Simon Glass
  0 siblings, 1 reply; 17+ messages in thread
From: Ilias Apalodimas @ 2021-11-05  7:01 UTC (permalink / raw)
  To: Simon Glass
  Cc: u-boot, trini, Rick Chen, Sean Anderson, Heinrich Schuchardt,
	Masahisa Kojima

Hi Simon,

[...]
> >
> > +int tpm_tis_open(struct udevice *udev);
>
> Please add comments

There are comments for all of those in drivers/tpm/tpm2_tis_core.c,
isn't that enough?

Thanks
/Ilias
>
> > +int tpm_tis_close(struct udevice *udev);
> > +int tpm_tis_cleanup(struct udevice *udev);
> > +int tpm_tis_send(struct udevice *udev, const u8 *buf, size_t len);
> > +int tpm_tis_recv(struct udevice *udev, u8 *buf, size_t count);
> > +int tpm_tis_get_desc(struct udevice *udev, char *buf, int size);
> > +int tpm_tis_init(struct udevice *udev);
> > +void tpm_tis_ops_register(struct udevice *udev, struct tpm_tis_phy_ops *ops);
> >  #endif
> > diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> > index 13b3db67c60f..e6b68769f3ff 100644
> > --- a/include/tpm-v2.h
> > +++ b/include/tpm-v2.h
> > @@ -396,6 +396,7 @@ enum {
> >         TPM_STS_DATA_EXPECT             = 1 << 3,
> >         TPM_STS_SELF_TEST_DONE          = 1 << 2,
> >         TPM_STS_RESPONSE_RETRY          = 1 << 1,
> > +       TPM_STS_READ_ZERO               = 0x23
> >  };
> >
> >  enum {
> > --
> > 2.33.1
> >
>
> Regards,
> Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/6 v4] tpm2: Add a TPMv2 MMIO TIS driver
  2021-11-05  2:02   ` Simon Glass
@ 2021-11-05  8:17     ` Ilias Apalodimas
  2021-11-05  8:23       ` Ilias Apalodimas
  0 siblings, 1 reply; 17+ messages in thread
From: Ilias Apalodimas @ 2021-11-05  8:17 UTC (permalink / raw)
  To: Simon Glass
  Cc: u-boot, trini, Rick Chen, Sean Anderson, Heinrich Schuchardt,
	Masahisa Kojima

Hi Simon, 

[...]

> > +                          u8 *result)
> > +{
> > +       struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
> > +
> > +       while (len--)
> > +               *result++ = ioread8(drv_data->iobase + addr);
> > +       return 0;
> > +}
> > +
> > +static int mmio_write_bytes(struct udevice *udev, u32 addr, u16 len,
> > +                           const u8 *value)
> > +{
> > +       struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
> > +
> > +       while (len--)
> > +               iowrite8(*value++, drv_data->iobase + addr);
> 
> So should this use regmap?
> 

Isn't the point of regmap abstracting the bus access itself? Something
along the lines of 

********			**********       ***********
* SPI **  --> 		*        * -->   * SPI DM ** --> Device
********			*        *       ***********
					* REGMAP *
********			*        *
* MMIO *  -->		*        * -->   **************
********			**********		 * MMIO access*  --> Device
									 **************

Right now we have discrete drivers for the SPI and MMIO TPMs.
However using it makes sense if we want to merge parts of the SPI, MMIO and I2C
drivers in the future. That though is not what this patchset deals with. 
Let's first clean up the crud of the TIS APIs duplication  we've been
carrying over various TPM drivers and worry about consolidating the bus
accesses later.

Thanks
/Ilias

> > +       return 0;
> > +}
> [..]
> 
> Regards,
> Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/6 v4] tpm2: Add a TPMv2 MMIO TIS driver
  2021-11-05  8:17     ` Ilias Apalodimas
@ 2021-11-05  8:23       ` Ilias Apalodimas
  2021-11-05 16:12         ` Simon Glass
  0 siblings, 1 reply; 17+ messages in thread
From: Ilias Apalodimas @ 2021-11-05  8:23 UTC (permalink / raw)
  To: Simon Glass
  Cc: u-boot, trini, Rick Chen, Sean Anderson, Heinrich Schuchardt,
	Masahisa Kojima

On Fri, Nov 05, 2021 at 10:17:21AM +0200, Ilias Apalodimas wrote:
> Hi Simon, 
> 
> [...]
> 
> > > +                          u8 *result)
> > > +{
> > > +       struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
> > > +
> > > +       while (len--)
> > > +               *result++ = ioread8(drv_data->iobase + addr);
> > > +       return 0;
> > > +}
> > > +
> > > +static int mmio_write_bytes(struct udevice *udev, u32 addr, u16 len,
> > > +                           const u8 *value)
> > > +{
> > > +       struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
> > > +
> > > +       while (len--)
> > > +               iowrite8(*value++, drv_data->iobase + addr);
> > 
> > So should this use regmap?
> > 
> 
> Isn't the point of regmap abstracting the bus access itself? Something
> along the lines of 
> 
> ********			**********       ***********
> * SPI **  --> 		*        * -->   * SPI DM ** --> Device
> ********			*        *       ***********
> 					* REGMAP *
> ********			*        *
> * MMIO *  -->		*        * -->   **************
> ********			**********		 * MMIO access*  --> Device
> 									 **************
> 

Hopefully I'll get the ASCII right this time...

********          **********       ***********
* SPI **  -->     *        * -->   * SPI DM ** --> Device
********          *        *       ***********
                  * REGMAP *
********          *        *
* MMIO *  -->     *        * -->   **************
********          **********       * MMIO access*  --> Device
                                   **************
> Right now we have discrete drivers for the SPI and MMIO TPMs.
> However using it makes sense if we want to merge parts of the SPI, MMIO and I2C
> drivers in the future. That though is not what this patchset deals with. 
> Let's first clean up the crud of the TIS APIs duplication  we've been
> carrying over various TPM drivers and worry about consolidating the bus
> accesses later.
> 
> Thanks
> /Ilias
> 
> > > +       return 0;
> > > +}
> > [..]
> > 
> > Regards,
> > Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/6 v4] tpm2: Introduce TIS tpm core
  2021-11-05  7:01     ` Ilias Apalodimas
@ 2021-11-05 16:12       ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2021-11-05 16:12 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: u-boot, trini, Rick Chen, Sean Anderson, Heinrich Schuchardt,
	Masahisa Kojima

Hi Ilias,

On Fri, 5 Nov 2021 at 01:02, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Simon,
>
> [...]
> > >
> > > +int tpm_tis_open(struct udevice *udev);
> >
> > Please add comments
>
> There are comments for all of those in drivers/tpm/tpm2_tis_core.c,
> isn't that enough?

So just move them to the header file, then? If this is an API it needs
to be documented and at some point we'll want sphinx to pick it up for
htmldocs.

Regards,
Simon


>
> Thanks
> /Ilias
> >
> > > +int tpm_tis_close(struct udevice *udev);
> > > +int tpm_tis_cleanup(struct udevice *udev);
> > > +int tpm_tis_send(struct udevice *udev, const u8 *buf, size_t len);
> > > +int tpm_tis_recv(struct udevice *udev, u8 *buf, size_t count);
> > > +int tpm_tis_get_desc(struct udevice *udev, char *buf, int size);
> > > +int tpm_tis_init(struct udevice *udev);
> > > +void tpm_tis_ops_register(struct udevice *udev, struct tpm_tis_phy_ops *ops);
> > >  #endif
> > > diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> > > index 13b3db67c60f..e6b68769f3ff 100644
> > > --- a/include/tpm-v2.h
> > > +++ b/include/tpm-v2.h
> > > @@ -396,6 +396,7 @@ enum {
> > >         TPM_STS_DATA_EXPECT             = 1 << 3,
> > >         TPM_STS_SELF_TEST_DONE          = 1 << 2,
> > >         TPM_STS_RESPONSE_RETRY          = 1 << 1,
> > > +       TPM_STS_READ_ZERO               = 0x23
> > >  };
> > >
> > >  enum {
> > > --
> > > 2.33.1
> > >
> >
> > Regards,
> > Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/6 v4] tpm2: Add a TPMv2 MMIO TIS driver
  2021-11-05  8:23       ` Ilias Apalodimas
@ 2021-11-05 16:12         ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2021-11-05 16:12 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: u-boot, trini, Rick Chen, Sean Anderson, Heinrich Schuchardt,
	Masahisa Kojima

Hi Ilias,

On Fri, 5 Nov 2021 at 02:23, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> On Fri, Nov 05, 2021 at 10:17:21AM +0200, Ilias Apalodimas wrote:
> > Hi Simon,
> >
> > [...]
> >
> > > > +                          u8 *result)
> > > > +{
> > > > +       struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
> > > > +
> > > > +       while (len--)
> > > > +               *result++ = ioread8(drv_data->iobase + addr);
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static int mmio_write_bytes(struct udevice *udev, u32 addr, u16 len,
> > > > +                           const u8 *value)
> > > > +{
> > > > +       struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
> > > > +
> > > > +       while (len--)
> > > > +               iowrite8(*value++, drv_data->iobase + addr);
> > >
> > > So should this use regmap?
> > >
> >
> > Isn't the point of regmap abstracting the bus access itself? Something
> > along the lines of

It is for MMIO at present, but I suppose it could handle the bus. It
would need to know about register numbers though, and we've never
really figured out if it is a win or not.

> >
> > ********                      **********       ***********
> > * SPI **  -->                 *        * -->   * SPI DM ** --> Device
> > ********                      *        *       ***********
> >                                       * REGMAP *
> > ********                      *        *
> > * MMIO *  -->         *        * -->   **************
> > ********                      **********               * MMIO access*  --> Device
> >                                                                        **************
> >
>
> Hopefully I'll get the ASCII right this time...
>
> ********          **********       ***********
> * SPI **  -->     *        * -->   * SPI DM ** --> Device
> ********          *        *       ***********
>                   * REGMAP *
> ********          *        *
> * MMIO *  -->     *        * -->   **************
> ********          **********       * MMIO access*  --> Device
>                                    **************
> > Right now we have discrete drivers for the SPI and MMIO TPMs.
> > However using it makes sense if we want to merge parts of the SPI, MMIO and I2C
> > drivers in the future. That though is not what this patchset deals with.
> > Let's first clean up the crud of the TIS APIs duplication  we've been
> > carrying over various TPM drivers and worry about consolidating the bus
> > accesses later.

That's OK with me.

Regards,
Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-11-05 16:15 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-03 15:09 [PATCH 0/6 v4] TPM cleanups and MMIO driver Ilias Apalodimas
2021-11-03 15:09 ` [PATCH 1/6 v4] tpm2: Introduce TIS tpm core Ilias Apalodimas
2021-11-05  2:02   ` Simon Glass
2021-11-05  7:01     ` Ilias Apalodimas
2021-11-05 16:12       ` Simon Glass
2021-11-03 15:09 ` [PATCH 2/6 v4] tpm2: Add a TPMv2 MMIO TIS driver Ilias Apalodimas
2021-11-05  2:02   ` Simon Glass
2021-11-05  8:17     ` Ilias Apalodimas
2021-11-05  8:23       ` Ilias Apalodimas
2021-11-05 16:12         ` Simon Glass
2021-11-03 15:09 ` [PATCH 3/6 v4] tpm: Use the new API on tpm2 spi driver Ilias Apalodimas
2021-11-05  2:02   ` Simon Glass
2021-11-03 15:09 ` [PATCH 4/6 v4] configs: Enable tpmv2 mmio on qemu for arm/arm64 Ilias Apalodimas
2021-11-05  2:02   ` Simon Glass
2021-11-03 15:09 ` [PATCH 5/6 v4] doc: qemu: Add instructions for swtpm usage Ilias Apalodimas
2021-11-05  2:02   ` Simon Glass
2021-11-03 15:09 ` [PATCH 6/6 v4] MAINTAINERS: Add entry for TPM drivers Ilias Apalodimas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.