devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] add tpm i2c ptp driver
@ 2019-12-02 13:33 amirmizi6
  2019-12-02 13:33 ` [PATCH v2 1/5] char: tpm: Make implementation of read16 read32 write32 optional amirmizi6
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: amirmizi6 @ 2019-12-02 13:33 UTC (permalink / raw)
  To: Eyal.Cohen, jarkko.sakkinen, oshrialkoby85, alexander.steffen,
	robh+dt, mark.rutland, peterhuewe, jgg, arnd, gregkh
  Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
	tmaimon77, gcwilson, kgoldman, ayna, Dan.Morav, oren.tanami,
	shmulik.hager, amir.mizinski, Amir Mizinski

From: Amir Mizinski <amirmizi6@gmail.com>

This patch set adds support for TPM devices that implement the I2C.
Interface defined by TCG PTP specification:
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client_Platform_TPM_Profile_PTP_2.0_r1.03_v22.pdf

The driver was tested on Raspberry-Pie 3, using Nuvoton NPCT75X TPM.

Interupts are not implemented yet, preparing it for the next patch.
This patch is based on initial work by oshri Alkoby, Alexander Steffen and Christophe Ricard

Addressed comments from:
 - Jarkko Sakkinen: https://patchwork.kernel.org/patch/11236257/
 - Rob Herring: https://patchwork.kernel.org/patch/11236253/

Changes since version 1:
 - Commit 2/5:
  - Fixed and extended commit description.
  - Fixed an issue regarding handeling max retries.
 - Commit 4/5:
  - Converted "tpm_tis_i2c.txt" to "tpm-tis-i2c.yaml".
  - Renamed "tpm_tis-i2c" to "tpm-tis-i2c".
  - Removed interrupts properties.
 - Commit 5/5: Replaced "tpm_tis-i2c" with "tpm-tis-i2c" in "tpm_tis_i2c.c".

Amir Mizinski (5):
  char: tpm: Make implementation of read16 read32 write32 optional
  char: tpm: Add check_data handle to tpm_tis_phy_ops in order to check
    data integrity
  char: tpm: rewrite "tpm_tis_req_canceled()"
  dt-bindings: tpm: Add YAML schema for TPM TIS I2C options
  char: tpm: add tpm_tis_i2c driver

 .../bindings/security/tpm/tpm-tis-i2c.yaml         |  38 +++
 drivers/char/tpm/Kconfig                           |  12 +
 drivers/char/tpm/Makefile                          |   1 +
 drivers/char/tpm/tpm_tis_core.c                    | 113 +++++----
 drivers/char/tpm/tpm_tis_core.h                    |  41 +++-
 drivers/char/tpm/tpm_tis_i2c.c                     | 272 +++++++++++++++++++++
 drivers/char/tpm/tpm_tis_spi.c                     |  41 ----
 7 files changed, 425 insertions(+), 93 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
 create mode 100644 drivers/char/tpm/tpm_tis_i2c.c

-- 
2.7.4


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

* [PATCH v2 1/5] char: tpm: Make implementation of read16 read32 write32 optional
  2019-12-02 13:33 [PATCH v2 0/5] add tpm i2c ptp driver amirmizi6
@ 2019-12-02 13:33 ` amirmizi6
  2019-12-02 13:33 ` [PATCH v2 2/5] char: tpm: Add check_data handle to tpm_tis_phy_ops in order to check data integrity amirmizi6
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: amirmizi6 @ 2019-12-02 13:33 UTC (permalink / raw)
  To: Eyal.Cohen, jarkko.sakkinen, oshrialkoby85, alexander.steffen,
	robh+dt, mark.rutland, peterhuewe, jgg, arnd, gregkh
  Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
	tmaimon77, gcwilson, kgoldman, ayna, Dan.Morav, oren.tanami,
	shmulik.hager, amir.mizinski, Amir Mizinski

From: Amir Mizinski <amirmizi6@gmail.com>

Only tpm_tis has a faster way to access multiple bytes at once, every other
driver will just fall back to read_bytes/write_bytes. Therefore, move this
common code out of tpm_tis_spi into tpm_tis_core, so that it is
automatically used when low-level drivers do not implement the specialized
methods.

This commit is based on previous work by Alexander Steffen.

Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
 drivers/char/tpm/tpm_tis_core.h | 38 +++++++++++++++++++++++++++++++++++---
 drivers/char/tpm/tpm_tis_spi.c  | 41 -----------------------------------------
 2 files changed, 35 insertions(+), 44 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 7337819..d06c65b 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -122,13 +122,35 @@ static inline int tpm_tis_read8(struct tpm_tis_data *data, u32 addr, u8 *result)
 static inline int tpm_tis_read16(struct tpm_tis_data *data, u32 addr,
 				 u16 *result)
 {
-	return data->phy_ops->read16(data, addr, result);
+	__le16 result_le;
+	int rc;
+
+	if (data->phy_ops->read16)
+		return data->phy_ops->read16(data, addr, result);
+
+	rc = data->phy_ops->read_bytes(data, addr, sizeof(u16),
+				       (u8 *)&result_le);
+	if (!rc)
+		*result = le16_to_cpu(result_le);
+
+	return rc;
 }
 
 static inline int tpm_tis_read32(struct tpm_tis_data *data, u32 addr,
 				 u32 *result)
 {
-	return data->phy_ops->read32(data, addr, result);
+	__le32 result_le;
+	int rc;
+
+	if (data->phy_ops->read32)
+		return data->phy_ops->read32(data, addr, result);
+
+	rc = data->phy_ops->read_bytes(data, addr, sizeof(u32),
+				       (u8 *)&result_le);
+	if (!rc)
+		*result = le32_to_cpu(result_le);
+
+	return rc;
 }
 
 static inline int tpm_tis_write_bytes(struct tpm_tis_data *data, u32 addr,
@@ -145,7 +167,17 @@ static inline int tpm_tis_write8(struct tpm_tis_data *data, u32 addr, u8 value)
 static inline int tpm_tis_write32(struct tpm_tis_data *data, u32 addr,
 				  u32 value)
 {
-	return data->phy_ops->write32(data, addr, value);
+	__le32 value_le;
+	int rc;
+
+	if (data->phy_ops->write32)
+		return data->phy_ops->write32(data, addr, value);
+
+	value_le = cpu_to_le32(value);
+	rc = data->phy_ops->write_bytes(data, addr, sizeof(u32),
+					(u8 *)&value_le);
+
+	return rc;
 }
 
 static inline bool is_bsw(void)
diff --git a/drivers/char/tpm/tpm_tis_spi.c b/drivers/char/tpm/tpm_tis_spi.c
index 19513e6..da82924 100644
--- a/drivers/char/tpm/tpm_tis_spi.c
+++ b/drivers/char/tpm/tpm_tis_spi.c
@@ -146,50 +146,9 @@ static int tpm_tis_spi_write_bytes(struct tpm_tis_data *data, u32 addr,
 	return tpm_tis_spi_transfer(data, addr, len, NULL, value);
 }
 
-static int tpm_tis_spi_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
-{
-	__le16 result_le;
-	int rc;
-
-	rc = data->phy_ops->read_bytes(data, addr, sizeof(u16),
-				       (u8 *)&result_le);
-	if (!rc)
-		*result = le16_to_cpu(result_le);
-
-	return rc;
-}
-
-static int tpm_tis_spi_read32(struct tpm_tis_data *data, u32 addr, u32 *result)
-{
-	__le32 result_le;
-	int rc;
-
-	rc = data->phy_ops->read_bytes(data, addr, sizeof(u32),
-				       (u8 *)&result_le);
-	if (!rc)
-		*result = le32_to_cpu(result_le);
-
-	return rc;
-}
-
-static int tpm_tis_spi_write32(struct tpm_tis_data *data, u32 addr, u32 value)
-{
-	__le32 value_le;
-	int rc;
-
-	value_le = cpu_to_le32(value);
-	rc = data->phy_ops->write_bytes(data, addr, sizeof(u32),
-					(u8 *)&value_le);
-
-	return rc;
-}
-
 static const struct tpm_tis_phy_ops tpm_spi_phy_ops = {
 	.read_bytes = tpm_tis_spi_read_bytes,
 	.write_bytes = tpm_tis_spi_write_bytes,
-	.read16 = tpm_tis_spi_read16,
-	.read32 = tpm_tis_spi_read32,
-	.write32 = tpm_tis_spi_write32,
 };
 
 static int tpm_tis_spi_probe(struct spi_device *dev)
-- 
2.7.4


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

* [PATCH v2 2/5] char: tpm: Add check_data handle to tpm_tis_phy_ops in order to check data integrity
  2019-12-02 13:33 [PATCH v2 0/5] add tpm i2c ptp driver amirmizi6
  2019-12-02 13:33 ` [PATCH v2 1/5] char: tpm: Make implementation of read16 read32 write32 optional amirmizi6
@ 2019-12-02 13:33 ` amirmizi6
  2019-12-02 13:33 ` [PATCH v2 3/5] char: tpm: rewrite "tpm_tis_req_canceled()" amirmizi6
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: amirmizi6 @ 2019-12-02 13:33 UTC (permalink / raw)
  To: Eyal.Cohen, jarkko.sakkinen, oshrialkoby85, alexander.steffen,
	robh+dt, mark.rutland, peterhuewe, jgg, arnd, gregkh
  Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
	tmaimon77, gcwilson, kgoldman, ayna, Dan.Morav, oren.tanami,
	shmulik.hager, amir.mizinski, Amir Mizinski

From: Amir Mizinski <amirmizi6@gmail.com>

In order to compute the crc over the data sent in lower layer (I2C for instance),
tpm_tis_check_data() calls an operation (if available) to check data integrity.
If data integrity cannot be verified, a retry attempt to save the sent/recieved data is implemented.

The current steps are done when sending a command:
    1. Host writes to TPM_STS.commandReady.
    2. Host writes command.
    3. Host checks that TPM received data is valid.
    4. If data is currupted go to step 1.

When receiving data:
    1. Host checks that TPM_STS.dataAvail is set.
    2. Host saves recieved data.
    3. Host checks that received data is correct.
    4. If data is currupted Host writes to TPM_STS.responseRetry and go to step 1.

This commit is based on previous work by Christophe Richard.

Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
 drivers/char/tpm/tpm_tis_core.c | 101 +++++++++++++++++++++++++---------------
 drivers/char/tpm/tpm_tis_core.h |   3 ++
 2 files changed, 66 insertions(+), 38 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index c3181ea..43784fd 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -242,6 +242,15 @@ static u8 tpm_tis_status(struct tpm_chip *chip)
 	return status;
 }
 
+static bool tpm_tis_check_data(struct tpm_chip *chip, const u8 *buf, size_t len)
+{
+	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+
+	if (priv->phy_ops->check_data)
+		return priv->phy_ops->check_data(priv, buf, len);
+	return true;
+}
+
 static void tpm_tis_ready(struct tpm_chip *chip)
 {
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
@@ -308,47 +317,58 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 {
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
 	int size = 0;
-	int status;
+	int status, i;
 	u32 expected;
+	bool check_data = false;
 
-	if (count < TPM_HEADER_SIZE) {
-		size = -EIO;
-		goto out;
-	}
+	for (i = 0; i < TPM_RETRY; i++) {
+		if (count < TPM_HEADER_SIZE) {
+			size = -EIO;
+			goto out;
+		}
 
-	size = recv_data(chip, buf, TPM_HEADER_SIZE);
-	/* read first 10 bytes, including tag, paramsize, and result */
-	if (size < TPM_HEADER_SIZE) {
-		dev_err(&chip->dev, "Unable to read header\n");
-		goto out;
-	}
+		size = recv_data(chip, buf, TPM_HEADER_SIZE);
+		/* read first 10 bytes, including tag, paramsize, and result */
+		if (size < TPM_HEADER_SIZE) {
+			dev_err(&chip->dev, "Unable to read header\n");
+			goto out;
+		}
 
-	expected = be32_to_cpu(*(__be32 *) (buf + 2));
-	if (expected > count || expected < TPM_HEADER_SIZE) {
-		size = -EIO;
-		goto out;
-	}
+		expected = be32_to_cpu(*(__be32 *) (buf + 2));
+		if (expected > count || expected < TPM_HEADER_SIZE) {
+			size = -EIO;
+			goto out;
+		}
 
-	size += recv_data(chip, &buf[TPM_HEADER_SIZE],
-			  expected - TPM_HEADER_SIZE);
-	if (size < expected) {
-		dev_err(&chip->dev, "Unable to read remainder of result\n");
-		size = -ETIME;
-		goto out;
-	}
+		size += recv_data(chip, &buf[TPM_HEADER_SIZE],
+				  expected - TPM_HEADER_SIZE);
+		if (size < expected) {
+			dev_err(&chip->dev, "Unable to read remainder of result\n");
+			size = -ETIME;
+			goto out;
+		}
 
-	if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
-				&priv->int_queue, false) < 0) {
-		size = -ETIME;
-		goto out;
+		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
+				      &priv->int_queue, false) < 0) {
+			size = -ETIME;
+			goto out;
+		}
+
+		status = tpm_tis_status(chip);
+		if (status & TPM_STS_DATA_AVAIL) {	/* retry? */
+			dev_err(&chip->dev, "Error left over data\n");
+			size = -EIO;
+			goto out;
+		}
+
+		check_data = tpm_tis_check_data(chip, buf, size);
+		if (!check_data)
+			tpm_tis_write8(priv, TPM_STS(priv->locality),
+				       TPM_STS_RESPONSE_RETRY);
+		else break;
 	}
-	status = tpm_tis_status(chip);
-	if (status & TPM_STS_DATA_AVAIL) {	/* retry? */
-		dev_err(&chip->dev, "Error left over data\n");
+	if (!check_data)
 		size = -EIO;
-		goto out;
-	}
-
 out:
 	tpm_tis_ready(chip);
 	return size;
@@ -453,14 +473,19 @@ static void disable_interrupts(struct tpm_chip *chip)
 static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
 {
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
-	int rc;
+	int rc, i;
 	u32 ordinal;
 	unsigned long dur;
+	bool data_valid = false;
 
-	rc = tpm_tis_send_data(chip, buf, len);
-	if (rc < 0)
-		return rc;
-
+	for (i = 0; i < TPM_RETRY && !data_valid; i++) {
+		rc = tpm_tis_send_data(chip, buf, len);
+		if (rc < 0)
+			return rc;
+		data_valid = tpm_tis_check_data(chip, buf, len);
+	}
+	if (!data_valid)
+		return -EIO;
 	/* go and do it */
 	rc = tpm_tis_write8(priv, TPM_STS(priv->locality), TPM_STS_GO);
 	if (rc < 0)
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index d06c65b..486c2e9 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -34,6 +34,7 @@ enum tis_status {
 	TPM_STS_GO = 0x20,
 	TPM_STS_DATA_AVAIL = 0x10,
 	TPM_STS_DATA_EXPECT = 0x08,
+	TPM_STS_RESPONSE_RETRY = 0x02,
 };
 
 enum tis_int_flags {
@@ -106,6 +107,8 @@ struct tpm_tis_phy_ops {
 	int (*read16)(struct tpm_tis_data *data, u32 addr, u16 *result);
 	int (*read32)(struct tpm_tis_data *data, u32 addr, u32 *result);
 	int (*write32)(struct tpm_tis_data *data, u32 addr, u32 src);
+	bool (*check_data)(struct tpm_tis_data *data, const u8 *buf,
+			   size_t len);
 };
 
 static inline int tpm_tis_read_bytes(struct tpm_tis_data *data, u32 addr,
-- 
2.7.4


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

* [PATCH v2 3/5] char: tpm: rewrite "tpm_tis_req_canceled()"
  2019-12-02 13:33 [PATCH v2 0/5] add tpm i2c ptp driver amirmizi6
  2019-12-02 13:33 ` [PATCH v2 1/5] char: tpm: Make implementation of read16 read32 write32 optional amirmizi6
  2019-12-02 13:33 ` [PATCH v2 2/5] char: tpm: Add check_data handle to tpm_tis_phy_ops in order to check data integrity amirmizi6
@ 2019-12-02 13:33 ` amirmizi6
  2019-12-02 13:33 ` [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options amirmizi6
  2019-12-02 13:33 ` [PATCH v2 5/5] char: tpm: add tpm_tis_i2c driver amirmizi6
  4 siblings, 0 replies; 9+ messages in thread
From: amirmizi6 @ 2019-12-02 13:33 UTC (permalink / raw)
  To: Eyal.Cohen, jarkko.sakkinen, oshrialkoby85, alexander.steffen,
	robh+dt, mark.rutland, peterhuewe, jgg, arnd, gregkh
  Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
	tmaimon77, gcwilson, kgoldman, ayna, Dan.Morav, oren.tanami,
	shmulik.hager, amir.mizinski, Amir Mizinski

From: Amir Mizinski <amirmizi6@gmail.com>

Using this function while read/write data resulted in aborted operation.
After investigating according to TCG TPM Profile (PTP) Specifications,
i found cancel should happen only if TPM_STS.commandReady bit is lit
and couldn't find a case when the current condition is valid.
Also only cmdReady bit need to be compared instead of the full lower status register byte.

Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
 drivers/char/tpm/tpm_tis_core.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 43784fd..924489c 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -631,17 +631,7 @@ static int probe_itpm(struct tpm_chip *chip)
 
 static bool tpm_tis_req_canceled(struct tpm_chip *chip, u8 status)
 {
-	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
-
-	switch (priv->manufacturer_id) {
-	case TPM_VID_WINBOND:
-		return ((status == TPM_STS_VALID) ||
-			(status == (TPM_STS_VALID | TPM_STS_COMMAND_READY)));
-	case TPM_VID_STM:
-		return (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY));
-	default:
-		return (status == TPM_STS_COMMAND_READY);
-	}
+	return ((status & TPM_STS_COMMAND_READY) == TPM_STS_COMMAND_READY);
 }
 
 static irqreturn_t tis_int_handler(int dummy, void *dev_id)
-- 
2.7.4


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

* [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options
  2019-12-02 13:33 [PATCH v2 0/5] add tpm i2c ptp driver amirmizi6
                   ` (2 preceding siblings ...)
  2019-12-02 13:33 ` [PATCH v2 3/5] char: tpm: rewrite "tpm_tis_req_canceled()" amirmizi6
@ 2019-12-02 13:33 ` amirmizi6
  2019-12-13 22:36   ` Rob Herring
  2019-12-02 13:33 ` [PATCH v2 5/5] char: tpm: add tpm_tis_i2c driver amirmizi6
  4 siblings, 1 reply; 9+ messages in thread
From: amirmizi6 @ 2019-12-02 13:33 UTC (permalink / raw)
  To: Eyal.Cohen, jarkko.sakkinen, oshrialkoby85, alexander.steffen,
	robh+dt, mark.rutland, peterhuewe, jgg, arnd, gregkh
  Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
	tmaimon77, gcwilson, kgoldman, ayna, Dan.Morav, oren.tanami,
	shmulik.hager, amir.mizinski, Amir Mizinski

From: Amir Mizinski <amirmizi6@gmail.com>

Added a YAML schema to support tpm tis i2c realted dt-bindings for the I2c PTP based physical layer.

Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
 .../bindings/security/tpm/tpm-tis-i2c.yaml         | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml

diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
new file mode 100644
index 0000000..bb18917
--- /dev/null
+++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/security/tpm/tpm-tis-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: I2C PTP based TPM Device Tree Bindings
+
+maintainers:
+  - Amir Mizinski <amirmizi6@gmail.com>
+
+description:
+  The TCG defines hardware protocol, registers and interface (based
+  on the TPM Interface Specification) for accessing TPM devices
+  implemented with an I2C interface.
+  Refer to the 'I2C Interface Definition' section in 'TCG PC Client
+  PlatformTPMProfile(PTP) Specification' publication for specification.
+  Designed for Raspberry Pie 3 Board with Nuvoton's NPCT75X (2.0)
+  
+properties:
+  compatible:
+    contains:
+      const: tcg,tpm-tis-i2c
+      
+  reg:
+    maxItems: 1
+    
+required:
+  - compatible
+  - reg  
+    
+    
+examples:
+  - |
+    tpm-tis-i2c: tpm-tis-i2c@2e {
+       compatible = "tcg,tpm-tis-i2c";
+       reg = <0x2e>;
+    };
-- 
2.7.4


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

* [PATCH v2 5/5] char: tpm: add tpm_tis_i2c driver
  2019-12-02 13:33 [PATCH v2 0/5] add tpm i2c ptp driver amirmizi6
                   ` (3 preceding siblings ...)
  2019-12-02 13:33 ` [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options amirmizi6
@ 2019-12-02 13:33 ` amirmizi6
  4 siblings, 0 replies; 9+ messages in thread
From: amirmizi6 @ 2019-12-02 13:33 UTC (permalink / raw)
  To: Eyal.Cohen, jarkko.sakkinen, oshrialkoby85, alexander.steffen,
	robh+dt, mark.rutland, peterhuewe, jgg, arnd, gregkh
  Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
	tmaimon77, gcwilson, kgoldman, ayna, Dan.Morav, oren.tanami,
	shmulik.hager, amir.mizinski, Amir Mizinski

From: Amir Mizinski <amirmizi6@gmail.com>

Implements the functionality needed to communicate with an I2C TPM
according to the TCG TPM I2C Interface Specification.

Limitations:
* No IRQ support
* No support for updating GUARD_TIME 
Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
 drivers/char/tpm/Kconfig       |  12 ++
 drivers/char/tpm/Makefile      |   1 +
 drivers/char/tpm/tpm_tis_i2c.c | 272 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 285 insertions(+)
 create mode 100644 drivers/char/tpm/tpm_tis_i2c.c

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 88a3c06..b731a80 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -67,6 +67,18 @@ config TCG_TIS_SPI
 	  within Linux. To compile this driver as a module, choose  M here;
 	  the module will be called tpm_tis_spi.
 
+config TCG_TIS_I2C
+	tristate "TPM I2C Interface Specification"
+	depends on I2C
+        depends on CRC_CCITT
+	select TCG_TIS_CORE
+	---help---
+	  If you have a TPM security chip which is connected to a regular
+	  I2C master (i.e. most embedded platforms) that is compliant with the
+	  TCG TPM I2C Interface Specification say Yes and it will be accessible from
+	  within Linux. To compile this driver as a module, choose  M here;
+	  the module will be called tpm_tis_i2c.
+
 config TCG_TIS_I2C_ATMEL
 	tristate "TPM Interface Specification 1.2 Interface (I2C - Atmel)"
 	depends on I2C
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index a01c4ca..15a0010 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -22,6 +22,7 @@ tpm-$(CONFIG_OF) += eventlog/of.o
 obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
 obj-$(CONFIG_TCG_TIS) += tpm_tis.o
 obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi.o
+obj-$(CONFIG_TCG_TIS_I2C) += tpm_tis_i2c.o
 obj-$(CONFIG_TCG_TIS_I2C_ATMEL) += tpm_i2c_atmel.o
 obj-$(CONFIG_TCG_TIS_I2C_INFINEON) += tpm_i2c_infineon.o
 obj-$(CONFIG_TCG_TIS_I2C_NUVOTON) += tpm_i2c_nuvoton.o
diff --git a/drivers/char/tpm/tpm_tis_i2c.c b/drivers/char/tpm/tpm_tis_i2c.c
new file mode 100644
index 0000000..477bb55
--- /dev/null
+++ b/drivers/char/tpm/tpm_tis_i2c.c
@@ -0,0 +1,272 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2014-2019 Nuvoton Technology corporation
+ *
+ * TPM TIS I2C
+ *
+ * TPM TIS I2C Device Driver Interface for devices that implement the TPM I2C
+ * Interface defined by TCG PC Client Platform TPM Profile (PTP) Specification
+ * Revision 01.03 v22 at www.trustedcomputinggroup.org
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/wait.h>
+#include <linux/acpi.h>
+#include <linux/freezer.h>
+#include <linux/crc-ccitt.h>
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/gpio.h>
+#include <linux/of_irq.h>
+#include <linux/of_gpio.h>
+#include <linux/tpm.h>
+#include "tpm.h"
+#include "tpm_tis_core.h"
+
+#define TPM_LOC_SEL                    0x04
+#define TPM_I2C_INTERFACE_CAPABILITY   0x30
+#define TPM_I2C_DEVICE_ADDRESS         0x38
+#define TPM_DATA_CSUM_ENABLE           0x40
+#define TPM_DATA_CSUM                  0x44
+#define TPM_I2C_DID_VID                        0x48
+#define TPM_I2C_RID                    0x4C
+
+struct tpm_tis_i2c_phy {
+	struct tpm_tis_data priv;
+	struct i2c_client *i2c_client;
+	bool data_csum;
+	u8 *iobuf;
+};
+
+static inline struct tpm_tis_i2c_phy *to_tpm_tis_i2c_phy(struct tpm_tis_data *data)
+{
+	return container_of(data, struct tpm_tis_i2c_phy, priv);
+}
+
+static u8 address_to_register(u32 addr)
+{
+	addr &= 0xFFF;
+
+	switch (addr) {
+		// adapt register addresses that have changed compared to
+		// older TIS versions
+	case TPM_ACCESS(0):
+		return 0x04;
+	case TPM_LOC_SEL:
+		return 0x00;
+	case TPM_DID_VID(0):
+		return 0x48;
+	case TPM_RID(0):
+		return 0x4C;
+	default:
+		return addr;
+	}
+}
+
+static int tpm_tis_i2c_read_bytes(struct tpm_tis_data *data, u32 addr,
+				  u16 len, u8 *result)
+{
+	struct tpm_tis_i2c_phy *phy = to_tpm_tis_i2c_phy(data);
+	int ret;
+	u8 reg = address_to_register(addr);
+	struct i2c_msg msgs[] = {
+		{
+			.addr = phy->i2c_client->addr,
+			.len = sizeof(reg),
+			.buf = &reg,
+		},
+		{
+			.addr = phy->i2c_client->addr,
+			.len = len,
+			.buf = result,
+			.flags = I2C_M_RD,
+		},
+	};
+
+	ret = i2c_transfer(phy->i2c_client->adapter, msgs, ARRAY_SIZE(msgs));
+
+	if (ret < 0)
+		return ret;
+
+	usleep_range(250, 300); // wait default GUARD_TIME of 250µs
+
+	return 0;
+}
+
+static int tpm_tis_i2c_write_bytes(struct tpm_tis_data *data, u32 addr,
+				   u16 len, const u8 *value)
+{
+	struct tpm_tis_i2c_phy *phy = to_tpm_tis_i2c_phy(data);
+	int ret;
+
+	if (phy->iobuf) {
+		if (len > TPM_BUFSIZE - 1)
+			return -EIO;
+
+		phy->iobuf[0] = address_to_register(addr);
+		memcpy(phy->iobuf + 1, value, len);
+
+		{
+			struct i2c_msg msgs[] = {
+				{
+					.addr = phy->i2c_client->addr,
+					.len = len + 1,
+					.buf = phy->iobuf,
+				},
+			};
+
+			ret = i2c_transfer(phy->i2c_client->adapter, msgs,
+					   ARRAY_SIZE(msgs));
+		}
+	} else {
+		u8 reg = address_to_register(addr);
+
+		struct i2c_msg msgs[] = {
+			{
+				.addr = phy->i2c_client->addr,
+				.len = sizeof(reg),
+				.buf = &reg,
+			},
+			{
+				.addr = phy->i2c_client->addr,
+				.len = len,
+				.buf = (u8 *)value,
+				.flags = I2C_M_NOSTART,
+			},
+		};
+
+		ret = i2c_transfer(phy->i2c_client->adapter, msgs,
+				   ARRAY_SIZE(msgs));
+	}
+
+	if (ret < 0)
+		return ret;
+
+	usleep_range(250, 300); // wait default GUARD_TIME of 250µs
+
+	return 0;
+}
+
+static bool tpm_tis_i2c_check_data(struct tpm_tis_data *data,
+				   const u8 *buf, size_t len)
+{
+	struct tpm_tis_i2c_phy *phy = to_tpm_tis_i2c_phy(data);
+	u16 crc, crc_tpm;
+	int rc;
+
+	if (phy->data_csum) {
+		crc = crc_ccitt(0x0000, buf, len);
+		rc = tpm_tis_read16(data, TPM_DATA_CSUM, &crc_tpm);
+		if (rc < 0)
+			return false;
+
+		crc_tpm = be16_to_cpu(crc_tpm);
+		return crc == crc_tpm;
+	}
+
+	return true;
+}
+
+static SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, tpm_tis_resume);
+
+static int csum_state_store(struct tpm_tis_data *data, u8 new_state)
+{
+	struct tpm_tis_i2c_phy *phy = to_tpm_tis_i2c_phy(data);
+	u8 cur_state;
+	int rc;
+
+	rc = tpm_tis_i2c_write_bytes(&phy->priv, TPM_DATA_CSUM_ENABLE,
+				     1, &new_state);
+	if (rc < 0)
+		return rc;
+
+	rc = tpm_tis_i2c_read_bytes(&phy->priv, TPM_DATA_CSUM_ENABLE,
+				    1, &cur_state);
+	if (rc < 0)
+		return rc;
+
+	if (new_state == cur_state)
+		phy->data_csum = (bool)new_state;
+
+	return rc;
+}
+
+static const struct tpm_tis_phy_ops tpm_i2c_phy_ops = {
+	.read_bytes = tpm_tis_i2c_read_bytes,
+	.write_bytes = tpm_tis_i2c_write_bytes,
+	.check_data = tpm_tis_i2c_check_data,
+};
+
+static int tpm_tis_i2c_probe(struct i2c_client *dev,
+			     const struct i2c_device_id *id)
+{
+	struct tpm_tis_i2c_phy *phy;
+	int rc;
+	const u8 loc_init = 0;
+
+	phy = devm_kzalloc(&dev->dev, sizeof(struct tpm_tis_i2c_phy),
+			   GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	phy->i2c_client = dev;
+
+	if (!i2c_check_functionality(dev->adapter, I2C_FUNC_NOSTART)) {
+		phy->iobuf = devm_kmalloc(&dev->dev, TPM_BUFSIZE, GFP_KERNEL);
+		if (!phy->iobuf)
+			return -ENOMEM;
+	}
+
+	// select locality 0 (the driver will access only via locality 0)
+	rc = tpm_tis_i2c_write_bytes(&phy->priv, TPM_LOC_SEL, 1, &loc_init);
+	if (rc < 0)
+		return rc;
+
+	// enables the data checksum calculation
+	rc = csum_state_store(&phy->priv, 0x01);
+	if (rc < 0)
+		return rc;
+
+	return tpm_tis_core_init(&dev->dev, &phy->priv, -1, &tpm_i2c_phy_ops,
+					NULL);
+}
+
+static const struct i2c_device_id tpm_tis_i2c_id[] = {
+	{"tpm_tis_i2c", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, tpm_tis_i2c_id);
+
+static const struct of_device_id of_tis_i2c_match[] = {
+	{ .compatible = "tcg,tpm-tis-i2c", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, of_tis_i2c_match);
+
+static const struct acpi_device_id acpi_tis_i2c_match[] = {
+	{"SMO0768", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, acpi_tis_i2c_match);
+
+static struct i2c_driver tpm_tis_i2c_driver = {
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = "tpm_tis_i2c",
+		.pm = &tpm_tis_pm,
+		.of_match_table = of_match_ptr(of_tis_i2c_match),
+		.acpi_match_table = ACPI_PTR(acpi_tis_i2c_match),
+	},
+	.probe = tpm_tis_i2c_probe,
+	.id_table = tpm_tis_i2c_id,
+};
+
+module_i2c_driver(tpm_tis_i2c_driver);
+
+MODULE_DESCRIPTION("TPM Driver for native I2C access");
+MODULE_LICENSE("GPL");
-- 
2.7.4


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

* Re: [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options
  2019-12-02 13:33 ` [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options amirmizi6
@ 2019-12-13 22:36   ` Rob Herring
  2019-12-16 13:53     ` Amir Mizinski
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2019-12-13 22:36 UTC (permalink / raw)
  To: amirmizi6
  Cc: Eyal.Cohen, jarkko.sakkinen, oshrialkoby85, alexander.steffen,
	mark.rutland, peterhuewe, jgg, arnd, gregkh, devicetree,
	linux-kernel, linux-integrity, oshri.alkoby, tmaimon77, gcwilson,
	kgoldman, ayna, Dan.Morav, oren.tanami, shmulik.hager,
	amir.mizinski

On Mon, Dec 02, 2019 at 03:33:31PM +0200, amirmizi6@gmail.com wrote:
> From: Amir Mizinski <amirmizi6@gmail.com>
> 
> Added a YAML schema to support tpm tis i2c realted dt-bindings for the I2c PTP based physical layer.

Wrap your commmit message. And TPM, TIS?, and I2C should be capitalized.

> 
> Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
> ---
>  .../bindings/security/tpm/tpm-tis-i2c.yaml         | 38 ++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml

Please read my comments on v1 (The first v1 from 11/10, not the 2nd v1 
you sent).

Rob

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

* Re: [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options
  2019-12-13 22:36   ` Rob Herring
@ 2019-12-16 13:53     ` Amir Mizinski
  2019-12-16 14:17       ` Rob Herring
  0 siblings, 1 reply; 9+ messages in thread
From: Amir Mizinski @ 2019-12-16 13:53 UTC (permalink / raw)
  To: Rob Herring
  Cc: Eyal.Cohen, Jarkko Sakkinen, Oshri Alkobi, Alexander Steffen,
	Mark Rutland, peterhuewe, jgg, Arnd Bergmann, Greg KH,
	devicetree, Linux Kernel Mailing List, linux-integrity,
	IS20 Oshri Alkoby, Tomer Maimon, gcwilson, kgoldman, ayna,
	IS30 Dan Morav, oren.tanami, shmulik.hager, amir.mizinski

On Sat, Dec 14, 2019 at 12:36 AM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Dec 02, 2019 at 03:33:31PM +0200, amirmizi6@gmail.com wrote:
> > From: Amir Mizinski <amirmizi6@gmail.com>
> >
> > Added a YAML schema to support tpm tis i2c realted dt-bindings for the I2c PTP based physical layer.
>
> Wrap your commmit message. And TPM, TIS?, and I2C should be capitalized.

Thanks,  ill fix that.

>
> >
> > Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
> > ---
> >  .../bindings/security/tpm/tpm-tis-i2c.yaml         | 38 ++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>
> Please read my comments on v1 (The first v1 from 11/10, not the 2nd v1
> you sent).

I sent a follow up comment regarding this:
https://patchwork.kernel.org/patch/11236253/
(2nd v1 was sent by mistake. sorry about that)

>
> Rob

Amir Mizinski

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

* Re: [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options
  2019-12-16 13:53     ` Amir Mizinski
@ 2019-12-16 14:17       ` Rob Herring
  0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2019-12-16 14:17 UTC (permalink / raw)
  To: Amir Mizinski
  Cc: Eyal.Cohen, Jarkko Sakkinen, Oshri Alkobi, Alexander Steffen,
	Mark Rutland, Peter Huewe, Jason Gunthorpe, Arnd Bergmann,
	Greg KH, devicetree, Linux Kernel Mailing List, linux-integrity,
	IS20 Oshri Alkoby, Tomer Maimon, gcwilson, kgoldman, ayna,
	IS30 Dan Morav, oren.tanami, shmulik.hager, amir.mizinski

On Mon, Dec 16, 2019 at 7:53 AM Amir Mizinski <amirmizi6@gmail.com> wrote:
>
> On Sat, Dec 14, 2019 at 12:36 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Mon, Dec 02, 2019 at 03:33:31PM +0200, amirmizi6@gmail.com wrote:
> > > From: Amir Mizinski <amirmizi6@gmail.com>
> > >
> > > Added a YAML schema to support tpm tis i2c realted dt-bindings for the I2c PTP based physical layer.
> >
> > Wrap your commmit message. And TPM, TIS?, and I2C should be capitalized.
>
> Thanks,  ill fix that.
>
> >
> > >
> > > Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
> > > ---
> > >  .../bindings/security/tpm/tpm-tis-i2c.yaml         | 38 ++++++++++++++++++++++
> > >  1 file changed, 38 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> >
> > Please read my comments on v1 (The first v1 from 11/10, not the 2nd v1
> > you sent).
>
> I sent a follow up comment regarding this:
> https://patchwork.kernel.org/patch/11236253/
> (2nd v1 was sent by mistake. sorry about that)

Sorry I missed your reply. However, you didn't address these comments:

> There's a bigger issue that the h/w here is more than just an I2C
> protocol. The chip may have multiple power supplies, clocks, reset
> lines, etc. HID over I2C seems like a similar case. Does the spec define
> *all* of that? If not, you need chip specific compatibles. You can keep
> this as a fallback though.

To rephrase this, a protocol does not fully describe the h/w and DT
should describe the h/w.

Also, you should include the interrupt whether you use it in the
driver currently or not. Again, it's about describing the h/w, not
what a driver happens to use ATM.

Rob

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

end of thread, other threads:[~2019-12-16 14:17 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-02 13:33 [PATCH v2 0/5] add tpm i2c ptp driver amirmizi6
2019-12-02 13:33 ` [PATCH v2 1/5] char: tpm: Make implementation of read16 read32 write32 optional amirmizi6
2019-12-02 13:33 ` [PATCH v2 2/5] char: tpm: Add check_data handle to tpm_tis_phy_ops in order to check data integrity amirmizi6
2019-12-02 13:33 ` [PATCH v2 3/5] char: tpm: rewrite "tpm_tis_req_canceled()" amirmizi6
2019-12-02 13:33 ` [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options amirmizi6
2019-12-13 22:36   ` Rob Herring
2019-12-16 13:53     ` Amir Mizinski
2019-12-16 14:17       ` Rob Herring
2019-12-02 13:33 ` [PATCH v2 5/5] char: tpm: add tpm_tis_i2c driver amirmizi6

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).