From: Amir Mizinski <[email protected]>
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.
Interrupts 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
Changes since version 1:
-"char:tpm:Add check_data handle to tpm_tis_phy_ops in order to check data integrity"
- Fixed and extended commit description.
- Fixed an issue regarding handling max retries.
-"dt-bindings: tpm: Add YAML schema for TPM TIS I2C options":
-Converted "tpm_tis_i2c.txt" to "tpm-tis-i2c.yaml".
- Renamed "tpm_tis-i2c" to "tpm-tis-i2c".
- Removed interrupts properties.
-"char: tpm: add tpm_tis_i2c driver"
- Replaced "tpm_tis-i2c" with "tpm-tis-i2c" in "tpm_tis_i2c.c".
Addressed comments from:
- Jarkko Sakkinen: https://patchwork.kernel.org/patch/11236257/
- Rob Herring: https://patchwork.kernel.org/patch/11236253/
Changes since version 2:
- Added 2 new commits with improvements suggested by Benoit Houyere.
-"Fix expected bit handling and send all bytes in one shot without last byte in exception"
-"Handle an exeption for TPM Firmware Update mode."
- Updated patch to latest v5.5
-"dt-bindings: tpm: Add YAML schema for TPM TIS I2C options"
- Added "interrupts" and "crc-checksum" to properties.
- Updated binding description and commit info.
-"char: tpm: add tpm_tis_i2c driver" (suggested by Benoit Houyere)
- Added repeat I2C frame after NACK.
- Checksum I2C feature activation in DTS file configuration.
Addressed comments from:
- Rob Herring: https://lore.kernel.org/patchwork/patch/1161287/
Changes since version 3:
- Updated patch to latest v5.6
- Updated commits headlines and development credit format by Jarkko Sakkinen suggestion
-"tpm: tpm_tis: Make implementation of read16 read32 write32 optional"
- Updated commit description.
-"dt-bindings: tpm: Add YAML schema for TPM TIS I2C options"
- Fixed 'make dt_binding_check' errors on YAML file.
- Removed interrupts from required and examples since there is no use for them in current patch.
Addressed comments from:
- Jarkko Sakkinen: https://lore.kernel.org/patchwork/patch/1192101/
- Rob Herring: https://lore.kernel.org/patchwork/patch/1192099/
Amir Mizinski (7):
tpm: tpm_tis: Make implementation of read16 read32 write32 optional
tpm: tpm_tis: Add check_data handle to tpm_tis_phy_ops in order
to check data integrity
tpm: tpm_tis: rewrite "tpm_tis_req_canceled()"
tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot
without last byte in exception
tpm: Handle an exception for TPM Firmware Update mode.
dt-bindings: tpm: Add YAML schema for TPM TIS I2C options
tpm: tpm_tis: add tpm_tis_i2c driver
.../bindings/security/tpm/tpm-tis-i2c.yaml | 43 +++
drivers/char/tpm/Kconfig | 12 +
drivers/char/tpm/Makefile | 1 +
drivers/char/tpm/tpm2-cmd.c | 4 +
drivers/char/tpm/tpm_tis_core.c | 182 ++++++-------
drivers/char/tpm/tpm_tis_core.h | 41 ++-
drivers/char/tpm/tpm_tis_i2c.c | 292 +++++++++++++++++++++
drivers/char/tpm/tpm_tis_spi_main.c | 41 ---
include/linux/tpm.h | 1 +
9 files changed, 482 insertions(+), 135 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
===========================================================================================
The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.
From: Amir Mizinski <[email protected]>
An extra precaution for TPM Firmware Update Mode.
For example if TPM power was cut while in Firmware update, platform
should ignore selftest failure and skip TPM initialization sequence.
This improvment was suggested by Benoit Houyere.
Signed-off-by: Amir Mizinski <[email protected]>
---
drivers/char/tpm/tpm2-cmd.c | 4 ++++
include/linux/tpm.h | 1 +
2 files changed, 5 insertions(+)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 7603295..b77e394 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -727,6 +727,10 @@ int tpm2_auto_startup(struct tpm_chip *chip)
goto out;
rc = tpm2_do_selftest(chip);
+
+ if ((rc == TPM2_RC_UPGRADE) || (rc == TPM2_RC_COMMAND_CODE))
+ return 0;
+
if (rc && rc != TPM2_RC_INITIALIZE)
goto out;
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 03e9b18..5a2e031 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -199,6 +199,7 @@ enum tpm2_return_codes {
TPM2_RC_INITIALIZE = 0x0100, /* RC_VER1 */
TPM2_RC_FAILURE = 0x0101,
TPM2_RC_DISABLED = 0x0120,
+ TPM2_RC_UPGRADE = 0x012D,
TPM2_RC_COMMAND_CODE = 0x0143,
TPM2_RC_TESTING = 0x090A, /* RC_WARN */
TPM2_RC_REFERENCE_H0 = 0x0910,
--
2.7.4
===========================================================================================
The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.
From: Amir Mizinski <[email protected]>
Added a YAML schema to support tpm tis i2c realted dt-bindings for the I2c
PTP based physical layer.
This patch adds the documentation for corresponding device tree bindings of
I2C based Physical TPM.
Refer to the 'I2C Interface Definition' section in
'TCG PC Client PlatformTPMProfile(PTP) Specification' publication
for specification.
Signed-off-by: Amir Mizinski <[email protected]>
---
.../bindings/security/tpm/tpm-tis-i2c.yaml | 46 ++++++++++++++++++++++
1 file changed, 46 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..516743e
--- /dev/null
+++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
@@ -0,0 +1,46 @@
+# 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 <[email protected]>
+
+description:
+ Device Tree Bindings for I2C based Trusted Platform Module(TPM).
+
+properties:
+ compatible:
+ contains:
+ const: tcg,tpm-tis-i2c
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ crc-checksum:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ CRC checksum enable.
+
+required:
+ - compatible
+ - reg
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ tpm-tis: tpm-tis-i2c@2e {
+ compatible = "tcg,tpm-tis-i2c";
+ reg = <0x2e>;
+ crc-checksum;
+ };
+ };
--
2.7.4
===========================================================================================
The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.
From: Amir Mizinski <[email protected]>
Implements the functionality needed to communicate with an I2C TPM
according to the TCG TPM I2C Interface Specification.
Signed-off-by: Amir Mizinski <[email protected]>
---
drivers/char/tpm/Kconfig | 12 ++
drivers/char/tpm/Makefile | 1 +
drivers/char/tpm/tpm_tis_i2c.c | 292 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 305 insertions(+)
create mode 100644 drivers/char/tpm/tpm_tis_i2c.c
diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index aacdeed..b482bbf 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -74,6 +74,18 @@ config TCG_TIS_SPI_CR50
If you have a H1 secure module running Cr50 firmware on SPI bus,
say Yes and it will be accessible from within Linux.
+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 9567e51..97999cf 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi.o
tpm_tis_spi-y := tpm_tis_spi_main.o
tpm_tis_spi-$(CONFIG_TCG_TIS_SPI_CR50) += tpm_tis_spi_cr50.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..c10ec7e
--- /dev/null
+++ b/drivers/char/tpm/tpm_tis_i2c.c
@@ -0,0 +1,292 @@
+// 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 http://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
+
+//#define I2C_IS_TPM2 1
+
+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 = 0;
+ int i = 0;
+ u8 reg = address_to_register(addr);
+ struct i2c_msg msgs[] = {
+ {
+ .addr = phy->i2c_client->addr,
+ .len = sizeof(reg),
+ .buf = ®,
+ },
+ {
+ .addr = phy->i2c_client->addr,
+ .len = len,
+ .buf = result,
+ .flags = I2C_M_RD,
+ },
+ };
+
+ do {
+ ret = i2c_transfer(phy->i2c_client->adapter, msgs,
+ ARRAY_SIZE(msgs));
+ usleep_range(250, 300); // wait default GUARD_TIME of 250µs
+
+ } while (ret < 0 && i++ < TPM_RETRY);
+
+ if (ret < 0)
+ return ret;
+
+
+ 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 = 0;
+ int i = 0;
+
+ 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,
+ },
+ };
+
+ do {
+ ret = i2c_transfer(phy->i2c_client->adapter,
+ msgs, ARRAY_SIZE(msgs));
+ // wait default GUARD_TIME of 250µs
+ usleep_range(250, 300);
+ } while (ret < 0 && i++ < TPM_RETRY);
+ }
+ } else {
+ u8 reg = address_to_register(addr);
+
+ struct i2c_msg msgs[] = {
+ {
+ .addr = phy->i2c_client->addr,
+ .len = sizeof(reg),
+ .buf = ®,
+ },
+ {
+ .addr = phy->i2c_client->addr,
+ .len = len,
+ .buf = (u8 *)value,
+ .flags = I2C_M_NOSTART,
+ },
+ };
+ do {
+ ret = i2c_transfer(phy->i2c_client->adapter, msgs,
+ ARRAY_SIZE(msgs));
+ // wait default GUARD_TIME of 250µs
+ usleep_range(250, 300);
+ } while (ret < 0 && i++ < TPM_RETRY);
+ }
+
+ if (ret < 0)
+ return ret;
+
+
+ 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;
+ int CRC_Checksum = 0;
+ const u8 loc_init = 0;
+ struct device_node *np;
+
+ 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;
+
+ // set CRC checksum calculation enable
+ np = dev->dev.of_node;
+ if (of_property_read_bool(np, "crc-checksum"))
+ CRC_Checksum = 1;
+
+ rc = csum_state_store(&phy->priv, CRC_Checksum);
+ 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
===========================================================================================
The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.
From: Amir Mizinski <[email protected]>
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/received 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 received 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.
Co-developed-by: Christophe Richard <[email protected]>
Signed-off-by: Christophe Richard <[email protected]>
Signed-off-by: Amir Mizinski <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 102 +++++++++++++++++++++++++---------------
drivers/char/tpm/tpm_tis_core.h | 3 ++
2 files changed, 67 insertions(+), 38 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 27c6ca0..6c4f232 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,59 @@ 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 +474,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
===========================================================================================
The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.
From: Amir Mizinski <[email protected]>
Today, actual implementation for send massage is not correct. We check and
loop only on TPM_STS.stsValid bit and next we single check TPM_STS.expect
bit value.
TPM_STS.expected bit shall be checked in the same time of
TPM_STS.stsValid, and should be repeated until timeout_A.
To aquire that, "wait_for_tpm_stat" function is modified to
"wait_for_tpm_stat_result". this function read regulary status register
and check bit defined by "mask" to reach value defined in "mask_result"
(that way a bit in mask can be checked if reached 1 or 0).
Respectively, to send message as defined in
TCG_DesignPrinciples_TPM2p0Driver_vp24_pubrev.pdf, all bytes should be
sent in one shot instead of sending last byte in exception.
This improvment was suggested by Benoit Houyere.
Signed-off-by: Amir Mizinski <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 72 ++++++++++++++++-------------------------
1 file changed, 28 insertions(+), 44 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 18b9dc4..c8f4cf8 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -44,9 +44,10 @@ static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
return false;
}
-static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
- unsigned long timeout, wait_queue_head_t *queue,
- bool check_cancel)
+static int wait_for_tpm_stat_result(struct tpm_chip *chip, u8 mask,
+ u8 mask_result, unsigned long timeout,
+ wait_queue_head_t *queue,
+ bool check_cancel)
{
unsigned long stop;
long rc;
@@ -55,7 +56,7 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
/* check current status */
status = chip->ops->status(chip);
- if ((status & mask) == mask)
+ if ((status & mask) == mask_result)
return 0;
stop = jiffies + timeout;
@@ -83,7 +84,7 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
usleep_range(TPM_TIMEOUT_USECS_MIN,
TPM_TIMEOUT_USECS_MAX);
status = chip->ops->status(chip);
- if ((status & mask) == mask)
+ if ((status & mask) == mask_result)
return 0;
} while (time_before(jiffies, stop));
}
@@ -290,10 +291,11 @@ static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count)
int size = 0, burstcnt, rc;
while (size < count) {
- rc = wait_for_tpm_stat(chip,
- TPM_STS_DATA_AVAIL | TPM_STS_VALID,
- chip->timeout_c,
- &priv->read_queue, true);
+ rc = wait_for_tpm_stat_result(chip,
+ TPM_STS_DATA_AVAIL | TPM_STS_VALID,
+ TPM_STS_DATA_AVAIL | TPM_STS_VALID,
+ chip->timeout_c,
+ &priv->read_queue, true);
if (rc < 0)
return rc;
burstcnt = get_burstcount(chip);
@@ -348,8 +350,9 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
goto out;
}
- if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
- &priv->int_queue, false) < 0) {
+ if (wait_for_tpm_stat_result(chip, TPM_STS_VALID,
+ TPM_STS_VALID, chip->timeout_c,
+ &priv->int_queue, false) < 0) {
size = -ETIME;
goto out;
}
@@ -385,61 +388,40 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
int rc, status, burstcnt;
size_t count = 0;
- bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
status = tpm_tis_status(chip);
if ((status & TPM_STS_COMMAND_READY) == 0) {
tpm_tis_ready(chip);
- if (wait_for_tpm_stat
- (chip, TPM_STS_COMMAND_READY, chip->timeout_b,
- &priv->int_queue, false) < 0) {
+ if (wait_for_tpm_stat_result(chip, TPM_STS_COMMAND_READY,
+ TPM_STS_COMMAND_READY,
+ chip->timeout_b,
+ &priv->int_queue, false) < 0) {
rc = -ETIME;
goto out_err;
}
}
- while (count < len - 1) {
+ while (count < len) {
burstcnt = get_burstcount(chip);
if (burstcnt < 0) {
dev_err(&chip->dev, "Unable to read burstcount\n");
rc = burstcnt;
goto out_err;
}
- burstcnt = min_t(int, burstcnt, len - count - 1);
+ burstcnt = min_t(int, burstcnt, len - count);
rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
burstcnt, buf + count);
if (rc < 0)
goto out_err;
count += burstcnt;
-
- if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
- &priv->int_queue, false) < 0) {
- rc = -ETIME;
- goto out_err;
- }
- status = tpm_tis_status(chip);
- if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
- rc = -EIO;
- goto out_err;
- }
}
-
- /* write last byte */
- rc = tpm_tis_write8(priv, TPM_DATA_FIFO(priv->locality), buf[count]);
- if (rc < 0)
- goto out_err;
-
- if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
- &priv->int_queue, false) < 0) {
+ if (wait_for_tpm_stat_result(chip, TPM_STS_VALID | TPM_STS_DATA_EXPECT,
+ TPM_STS_VALID, chip->timeout_c,
+ &priv->int_queue, false) < 0) {
rc = -ETIME;
goto out_err;
}
- status = tpm_tis_status(chip);
- if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) {
- rc = -EIO;
- goto out_err;
- }
return 0;
@@ -496,9 +478,11 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
dur = tpm_calc_ordinal_duration(chip, ordinal);
- if (wait_for_tpm_stat
- (chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID, dur,
- &priv->read_queue, false) < 0) {
+ if (wait_for_tpm_stat_result(chip,
+ TPM_STS_DATA_AVAIL | TPM_STS_VALID,
+ TPM_STS_DATA_AVAIL | TPM_STS_VALID,
+ dur,
+ &priv->read_queue, false) < 0) {
rc = -ETIME;
goto out_err;
}
--
2.7.4
===========================================================================================
The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.
On Tue, Mar 31, 2020 at 02:32:04PM +0300, [email protected] wrote:
> From: Amir Mizinski <[email protected]>
>
> Today, actual implementation for send massage is not correct. We check and
> loop only on TPM_STS.stsValid bit and next we single check TPM_STS.expect
> bit value.
> TPM_STS.expected bit shall be checked in the same time of
> TPM_STS.stsValid, and should be repeated until timeout_A.
> To aquire that, "wait_for_tpm_stat" function is modified to
> "wait_for_tpm_stat_result". this function read regulary status register
> and check bit defined by "mask" to reach value defined in "mask_result"
> (that way a bit in mask can be checked if reached 1 or 0).
>
> Respectively, to send message as defined in
> TCG_DesignPrinciples_TPM2p0Driver_vp24_pubrev.pdf, all bytes should be
> sent in one shot instead of sending last byte in exception.
>
> This improvment was suggested by Benoit Houyere.
Use suggested-by tag.
Also if something is not correct, please provide a fixes tag.
You are speaking now in theoretical level, which we don't really
care that much. Is this causing you real issues? If the answer is
yes, please report them. If the answer is no, we don't need this.
/Jarkko
>
> Signed-off-by: Amir Mizinski <[email protected]>
> ---
> drivers/char/tpm/tpm_tis_core.c | 72 ++++++++++++++++-------------------------
> 1 file changed, 28 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 18b9dc4..c8f4cf8 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -44,9 +44,10 @@ static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
> return false;
> }
>
> -static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
> - unsigned long timeout, wait_queue_head_t *queue,
> - bool check_cancel)
> +static int wait_for_tpm_stat_result(struct tpm_chip *chip, u8 mask,
> + u8 mask_result, unsigned long timeout,
> + wait_queue_head_t *queue,
> + bool check_cancel)
> {
> unsigned long stop;
> long rc;
> @@ -55,7 +56,7 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
>
> /* check current status */
> status = chip->ops->status(chip);
> - if ((status & mask) == mask)
> + if ((status & mask) == mask_result)
> return 0;
>
> stop = jiffies + timeout;
> @@ -83,7 +84,7 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
> usleep_range(TPM_TIMEOUT_USECS_MIN,
> TPM_TIMEOUT_USECS_MAX);
> status = chip->ops->status(chip);
> - if ((status & mask) == mask)
> + if ((status & mask) == mask_result)
> return 0;
> } while (time_before(jiffies, stop));
> }
> @@ -290,10 +291,11 @@ static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count)
> int size = 0, burstcnt, rc;
>
> while (size < count) {
> - rc = wait_for_tpm_stat(chip,
> - TPM_STS_DATA_AVAIL | TPM_STS_VALID,
> - chip->timeout_c,
> - &priv->read_queue, true);
> + rc = wait_for_tpm_stat_result(chip,
> + TPM_STS_DATA_AVAIL | TPM_STS_VALID,
> + TPM_STS_DATA_AVAIL | TPM_STS_VALID,
> + chip->timeout_c,
> + &priv->read_queue, true);
> if (rc < 0)
> return rc;
> burstcnt = get_burstcount(chip);
> @@ -348,8 +350,9 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> goto out;
> }
>
> - if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
> - &priv->int_queue, false) < 0) {
> + if (wait_for_tpm_stat_result(chip, TPM_STS_VALID,
> + TPM_STS_VALID, chip->timeout_c,
> + &priv->int_queue, false) < 0) {
> size = -ETIME;
> goto out;
> }
> @@ -385,61 +388,40 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
> int rc, status, burstcnt;
> size_t count = 0;
> - bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
>
> status = tpm_tis_status(chip);
> if ((status & TPM_STS_COMMAND_READY) == 0) {
> tpm_tis_ready(chip);
> - if (wait_for_tpm_stat
> - (chip, TPM_STS_COMMAND_READY, chip->timeout_b,
> - &priv->int_queue, false) < 0) {
> + if (wait_for_tpm_stat_result(chip, TPM_STS_COMMAND_READY,
> + TPM_STS_COMMAND_READY,
> + chip->timeout_b,
> + &priv->int_queue, false) < 0) {
> rc = -ETIME;
> goto out_err;
> }
> }
>
> - while (count < len - 1) {
> + while (count < len) {
> burstcnt = get_burstcount(chip);
> if (burstcnt < 0) {
> dev_err(&chip->dev, "Unable to read burstcount\n");
> rc = burstcnt;
> goto out_err;
> }
> - burstcnt = min_t(int, burstcnt, len - count - 1);
> + burstcnt = min_t(int, burstcnt, len - count);
> rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
> burstcnt, buf + count);
> if (rc < 0)
> goto out_err;
>
> count += burstcnt;
> -
> - if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
> - &priv->int_queue, false) < 0) {
> - rc = -ETIME;
> - goto out_err;
> - }
> - status = tpm_tis_status(chip);
> - if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
> - rc = -EIO;
> - goto out_err;
> - }
> }
> -
> - /* write last byte */
> - rc = tpm_tis_write8(priv, TPM_DATA_FIFO(priv->locality), buf[count]);
> - if (rc < 0)
> - goto out_err;
> -
> - if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
> - &priv->int_queue, false) < 0) {
> + if (wait_for_tpm_stat_result(chip, TPM_STS_VALID | TPM_STS_DATA_EXPECT,
> + TPM_STS_VALID, chip->timeout_c,
> + &priv->int_queue, false) < 0) {
> rc = -ETIME;
> goto out_err;
> }
> - status = tpm_tis_status(chip);
> - if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) {
> - rc = -EIO;
> - goto out_err;
> - }
>
> return 0;
>
> @@ -496,9 +478,11 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
> ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
>
> dur = tpm_calc_ordinal_duration(chip, ordinal);
> - if (wait_for_tpm_stat
> - (chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID, dur,
> - &priv->read_queue, false) < 0) {
> + if (wait_for_tpm_stat_result(chip,
> + TPM_STS_DATA_AVAIL | TPM_STS_VALID,
> + TPM_STS_DATA_AVAIL | TPM_STS_VALID,
> + dur,
> + &priv->read_queue, false) < 0) {
> rc = -ETIME;
> goto out_err;
> }
> --
> 2.7.4
>
>
>
> ===========================================================================================
> The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.
On Tue, Mar 31, 2020 at 02:32:07PM +0300, [email protected] wrote:
> From: Amir Mizinski <[email protected]>
>
> Implements the functionality needed to communicate with an I2C TPM
> according to the TCG TPM I2C Interface Specification.
>
> Signed-off-by: Amir Mizinski <[email protected]>
> ---
> drivers/char/tpm/Kconfig | 12 ++
> drivers/char/tpm/Makefile | 1 +
> drivers/char/tpm/tpm_tis_i2c.c | 292 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 305 insertions(+)
> create mode 100644 drivers/char/tpm/tpm_tis_i2c.c
>
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index aacdeed..b482bbf 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -74,6 +74,18 @@ config TCG_TIS_SPI_CR50
> If you have a H1 secure module running Cr50 firmware on SPI bus,
> say Yes and it will be accessible from within Linux.
>
> +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 9567e51..97999cf 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi.o
> tpm_tis_spi-y := tpm_tis_spi_main.o
> tpm_tis_spi-$(CONFIG_TCG_TIS_SPI_CR50) += tpm_tis_spi_cr50.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..c10ec7e
> --- /dev/null
> +++ b/drivers/char/tpm/tpm_tis_i2c.c
> @@ -0,0 +1,292 @@
> +// 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 http://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
> +
> +//#define I2C_IS_TPM2 1
> +
> +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 = 0;
> + int i = 0;
> + u8 reg = address_to_register(addr);
> + struct i2c_msg msgs[] = {
> + {
> + .addr = phy->i2c_client->addr,
> + .len = sizeof(reg),
> + .buf = ®,
> + },
> + {
> + .addr = phy->i2c_client->addr,
> + .len = len,
> + .buf = result,
> + .flags = I2C_M_RD,
> + },
> + };
> +
> + do {
> + ret = i2c_transfer(phy->i2c_client->adapter, msgs,
> + ARRAY_SIZE(msgs));
> + usleep_range(250, 300); // wait default GUARD_TIME of 250?s
> +
> + } while (ret < 0 && i++ < TPM_RETRY);
> +
> + if (ret < 0)
> + return ret;
> +
> +
> + 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 = 0;
> + int i = 0;
> +
> + 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,
> + },
> + };
> +
> + do {
> + ret = i2c_transfer(phy->i2c_client->adapter,
> + msgs, ARRAY_SIZE(msgs));
> + // wait default GUARD_TIME of 250?s
> + usleep_range(250, 300);
> + } while (ret < 0 && i++ < TPM_RETRY);
> + }
> + } else {
> + u8 reg = address_to_register(addr);
> +
> + struct i2c_msg msgs[] = {
> + {
> + .addr = phy->i2c_client->addr,
> + .len = sizeof(reg),
> + .buf = ®,
> + },
> + {
> + .addr = phy->i2c_client->addr,
> + .len = len,
> + .buf = (u8 *)value,
> + .flags = I2C_M_NOSTART,
> + },
> + };
> + do {
> + ret = i2c_transfer(phy->i2c_client->adapter, msgs,
> + ARRAY_SIZE(msgs));
> + // wait default GUARD_TIME of 250?s
> + usleep_range(250, 300);
> + } while (ret < 0 && i++ < TPM_RETRY);
> + }
> +
> + if (ret < 0)
> + return ret;
> +
> +
> + 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;
> + int CRC_Checksum = 0;
> + const u8 loc_init = 0;
> + struct device_node *np;
> +
> + 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;
> +
> + // set CRC checksum calculation enable
> + np = dev->dev.of_node;
> + if (of_property_read_bool(np, "crc-checksum"))
> + CRC_Checksum = 1;
> +
> + rc = csum_state_store(&phy->priv, CRC_Checksum);
> + 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");
Just put "TPM Driver" here.
/Jarkko
Hi--
On 3/31/20 4:32 AM, [email protected] wrote:
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index aacdeed..b482bbf 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -74,6 +74,18 @@ config TCG_TIS_SPI_CR50
> If you have a H1 secure module running Cr50 firmware on SPI bus,
> say Yes and it will be accessible from within Linux.
>
> +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.
> +
Please do as Documenatation/process/coding-style.rst says:
"Lines under a ``config`` definition
are indented with one tab, while help text is indented an additional two
spaces."
> config TCG_TIS_I2C_ATMEL
> tristate "TPM Interface Specification 1.2 Interface (I2C - Atmel)"
> depends on I2C
thanks.
--
~Randy
On Tue, 31 Mar 2020 14:32:06 +0300, <[email protected]> wrote:
>
> From: Amir Mizinski <[email protected]>
>
> Added a YAML schema to support tpm tis i2c realted dt-bindings for the I2c
> PTP based physical layer.
>
> This patch adds the documentation for corresponding device tree bindings of
> I2C based Physical TPM.
> Refer to the 'I2C Interface Definition' section in
> 'TCG PC Client PlatformTPMProfile(PTP) Specification' publication
> for specification.
>
> Signed-off-by: Amir Mizinski <[email protected]>
> ---
> .../bindings/security/tpm/tpm-tis-i2c.yaml | 46 ++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml: while parsing a block collection
in "<unicode string>", line 36, column 3
did not find expected '-' indicator
in "<unicode string>", line 46, column 4
Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.example.dts' failed
make[1]: *** [Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
warning: no schema found in file: Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml: ignoring, error parsing file
Makefile:1262: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1264698
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
> On Tue, Mar 31, 2020 at 02:32:04PM +0300, [email protected] wrote:
> > From: Amir Mizinski <[email protected]>
> >
> > Today, actual implementation for send massage is not correct. We check
> > and loop only on TPM_STS.stsValid bit and next we single check
> > TPM_STS.expect bit value.
> > TPM_STS.expected bit shall be checked in the same time of
> > TPM_STS.stsValid, and should be repeated until timeout_A.
> > To aquire that, "wait_for_tpm_stat" function is modified to
> > "wait_for_tpm_stat_result". this function read regulary status
> > register and check bit defined by "mask" to reach value defined in "mask_result"
> > (that way a bit in mask can be checked if reached 1 or 0).
> >
> > Respectively, to send message as defined in
> > TCG_DesignPrinciples_TPM2p0Driver_vp24_pubrev.pdf, all bytes should be
> > sent in one shot instead of sending last byte in exception.
> >
> > This improvment was suggested by Benoit Houyere.
>Use suggested-by tag.
>Also if something is not correct, please provide a fixes tag.
> You are speaking now in theoretical level, which we don't really care that much. Is this causing you real issues? If the answer is yes, please report them. If the > >answer is no, we don't need this.
> /Jarkko
I2C TPM specification introduce CRC calculation on TPM command bytes. CRC calculation take place from last byte acquired to TPM_STS.expected bit reset (=0) .It introduces latency and actual incorrect implementation becomes visible now under I2C on the contrary before that's all.
The case where TPM keeps TPM_STS.expected bit set with TPM_STS.stsValid set after last byte reception is possible and is not an issue. It's not theoretical level, it's practical level now.
On Tue, Mar 31, 2020 at 02:32:02PM +0300, [email protected] wrote:
> From: Amir Mizinski <[email protected]>
>
> 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/received 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 received 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.
>
> Co-developed-by: Christophe Richard <[email protected]>
> Signed-off-by: Christophe Richard <[email protected]>
> Signed-off-by: Amir Mizinski <[email protected]>
The email is malformed.
So.. How did Christophe participate on writing this patch? I haven't
seen him shouting anything about the subject and still his SOB is
there.
/Jarkko
On Tue, Mar 31, 2020 at 09:34:28PM +0000, Benoit HOUYERE wrote:
>
> > On Tue, Mar 31, 2020 at 02:32:04PM +0300, [email protected] wrote:
> > > From: Amir Mizinski <[email protected]>
> > >
> > > Today, actual implementation for send massage is not correct. We check
> > > and loop only on TPM_STS.stsValid bit and next we single check
> > > TPM_STS.expect bit value.
> > > TPM_STS.expected bit shall be checked in the same time of
> > > TPM_STS.stsValid, and should be repeated until timeout_A.
> > > To aquire that, "wait_for_tpm_stat" function is modified to
> > > "wait_for_tpm_stat_result". this function read regulary status
> > > register and check bit defined by "mask" to reach value defined in "mask_result"
> > > (that way a bit in mask can be checked if reached 1 or 0).
> > >
> > > Respectively, to send message as defined in
> > > TCG_DesignPrinciples_TPM2p0Driver_vp24_pubrev.pdf, all bytes should be
> > > sent in one shot instead of sending last byte in exception.
> > >
> > > This improvment was suggested by Benoit Houyere.
>
> >Use suggested-by tag.
>
> >Also if something is not correct, please provide a fixes tag.
>
> > You are speaking now in theoretical level, which we don't really care that much. Is this causing you real issues? If the answer is yes, please report them. If the > >answer is no, we don't need this.
>
> > /Jarkko
>
> I2C TPM specification introduce CRC calculation on TPM command bytes.
> CRC calculation take place from last byte acquired to
> TPM_STS.expected bit reset (=0) .It introduces latency and actual
> incorrect implementation becomes visible now under I2C on the contrary
> before that's all. The case where TPM keeps TPM_STS.expected bit set
> with TPM_STS.stsValid set after last byte reception is possible and is
> not an issue. It's not theoretical level, it's practical level now.
Thank you, think I got it. This means that it does not need a fixes tag
because it does not break any hardware that it currently supported.
I'd suggest refining the commit message. Not only it is somewhat loosely
writte peace of text but also has typos like "massage".
/Jarkko
>>On Tue, Mar 31, 2020 at 02:32:02PM +0300, [email protected] wrote:
>> From: Amir Mizinski <[email protected]>
>>
>> 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/received 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 received 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.
>>
>> Co-developed-by: Christophe Richard <[email protected]>
>> Signed-off-by: Christophe Richard <[email protected]>
>> Signed-off-by: Amir Mizinski <[email protected]>
>The email is malformed.
>So.. How did Christophe participate on writing this patch? I haven't seen him shouting anything about the subject and still his SOB is there.
>/Jarkko
Christophe sent patch to support I2C TCG TPM driver tpm_tis_i2c (https://patchwork.kernel.org/patch/8628681/) in the same time that tpm_tis_spi. This function was named tpm_tis_i2c_check_data.
Best Regards,
On Wed, Apr 01, 2020 at 12:19:39PM +0300, Amir Mizinski wrote:
> this commit is based on Christophe work and was submitted in 2016:
> https://patchwork.kernel.org/patch/8628661/
> I followed you comments from the previous version on commit 1/7:
> https://lore.kernel.org/patchwork/patch/1192101/
> and fixed it in this one too. should i write it differently?
> thank you,
> Amir Mizinski
No then it's correct thanks. Just wanted to verify.
Please configure your email client:
https://www.kernel.org/doc/html/v4.10/process/email-clients.html
/Jarkko