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 exception 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/
Changes since version 4:
-"tpm: tpm_tis: Make implementation of read16 read32 write32 optional"
-Added a "Reviewed-by" tag:
-"tpm: tpm_tis: Add check_data handle to tpm_tis_phy_ops in order to check data integrity"
-Fixed credit typos.
-"tpm: tpm_tis: rewrite "tpm_tis_req_canceled()""
-Added fixes tag and removed changes for STM.
-"tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception"
-Fixed typos, edited description to be clearer, and added a "Suggested-by" tag.
-"tpm: Handle an exception for TPM Firmware Update mode."
-Added a "Suggested-by" tag.
-"dt-bindings: tpm: Add YAML schema for TPM TIS I2C options"
-Fixed 'make dt_binding_check' errors.
-"tpm: tpm_tis: add tpm_tis_i2c driver"
-Added tested-by tag by Eddie James.
-Fixed indent in Kconfig file.
-Fixed 'MODULE_DESCRIPTION'.
Addressed comments from:
- Jarkko Sakkinen: https://patchwork.kernel.org/patch/11467645/
https://patchwork.kernel.org/patch/11467655/
https://patchwork.kernel.org/patch/11467643/
https://patchwork.kernel.org/patch/11467659/
https://patchwork.kernel.org/patch/11467651/
- Rob Herring: https://patchwork.kernel.org/patch/11467653/
- Randy Dunlap: https://patchwork.kernel.org/patch/11467651/
- Eddie James: https://lore.kernel.org/patchwork/patch/1192104/
Changes since version 5:
-"tpm: tpm_tis: Add check_data handle to tpm_tis_phy_ops"
-Updated short description and fixed long description to be more clear.
Addressed comments from:
- Jarkko Sakkinen: https://lkml.org/lkml/2020/4/6/748
Changes since version 6:
-"tpm: tpm_tis: Make implementation of read16, read32 and write32 optional"
-Fixed short description.
-fixed long description proofreading issues.
-"tpm: tpm_tis: Add check_data handle to tpm_tis_phy_ops"
-Fixed long description by Jarkko comments and proofreading issues.
-Replaced "check_data" with verify_data_integrity".
-New line before return statement.
-"tpm: tpm_tis: rewrite "tpm_tis_req_canceled()"
-Fixed line over 80 characters.
-fixed long description proofreading issues.
-" tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot"
-fixed long description proofreading issues.
-"dt-bindings: tpm: Add YAML schema for TPM TIS I2C option"
-Replaced "tpm-tis-i2c@2e" with "tpm_tis@2e".
-Fixed CRC_Checksum description.
-"tpm: tpm_tis: add tpm_tis_i2c driver"
-Replaced "depends on CRC_CCIT" with "select CRC_CCIT".
-Added tested-by tag by Joel Stanley.
-Fixed checkpatch.pl warnings.
Addressed comments from:
- Jarkko Sakkinen:
https://lore.kernel.org/patchwork/patch/1221336/
https://lore.kernel.org/patchwork/patch/1221337/
https://lore.kernel.org/patchwork/patch/1221339/
- Joel Stanley:
https://lore.kernel.org/patchwork/patch/1220543/
- Rob Herring:
https://lore.kernel.org/patchwork/patch/1221334/
Changes since version 7:
- Added a new commit with improvements suggested by Benoit Houyere.
-"tpm: tpm_tis: verify TPM_STS register is valid after locality request"
-"tpm: tpm_tis: Rewrite "tpm_tis_req_canceled()""
-Fixed Hash for Fixes tag.
-"tpm: Add YAML schema for TPM TIS I2C options"
-Added a compatible string specific to the nuvoton npct75x chip.
-"tpm: tpm_tis: add tpm_tis_i2c driver"
-added a compatible string according to yaml file.
Addressed comments from:
- Jarkko Sakkinen:
https://lore.kernel.org/patchwork/patch/1231524/
- Rob Herring:
https://lore.kernel.org/patchwork/patch/1231526/
Changes since version 8:
- "tpm: tpm_tis: Make implementation of read16, read32 and write32 optional"
-Fixed a compile error conflicting CR50
- "tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception"
-Moved commit backwards from 4/8 to 2/8 for a better flow with new data integrity check design
- "tpm: tpm_tis: Add retry in case of protocol failure or data integrity (on I2C only) failure."
-Renamed from "tpm: tpm_tis: Add check_data handle to tpm_tis_phy_ops"
-Redesign and added a retry for additional error cases.
- "tpm: Add YAML schema for TPM TIS I2C options"
-fixed Dual-license new binding
-removed "oneOf"
-fixed tpm_tis@2e to tpm@2e
Addressed comments from:
- Jarkko Sakkinen:
https://lore.kernel.org/patchwork/patch/1240728/
https://lore.kernel.org/patchwork/patch/1240736/
- Rob Herring:
https://lore.kernel.org/patchwork/patch/1240733/
Amir Mizinski (8):
tpm: tpm_tis: Make implementation of read16, read32 and write32
optional
tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot
without last byte in exception
tpm: tpm_tis: Add retry in case of protocol failure or data integrity
(on I2C only) failure.
tpm: tpm_tis: Rewrite "tpm_tis_req_canceled()"
tpm: Handle an exception for TPM Firmware Update mode.
tpm: tpm_tis: verify TPM_STS register is valid after locality request
tpm: Add YAML schema for TPM TIS I2C options
tpm: tpm_tis: add tpm_tis_i2c driver
.../bindings/security/tpm/tpm-tis-i2c.yaml | 50 ++++
drivers/char/tpm/Kconfig | 12 +
drivers/char/tpm/Makefile | 1 +
drivers/char/tpm/tpm2-cmd.c | 4 +
drivers/char/tpm/tpm_tis_core.c | 189 ++++++-------
drivers/char/tpm/tpm_tis_core.h | 41 ++-
drivers/char/tpm/tpm_tis_i2c.c | 292 +++++++++++++++++++++
drivers/char/tpm/tpm_tis_spi.h | 4 -
drivers/char/tpm/tpm_tis_spi_cr50.c | 3 -
drivers/char/tpm/tpm_tis_spi_main.c | 41 ---
include/linux/tpm.h | 1 +
11 files changed, 498 insertions(+), 140 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
From: Amir Mizinski <[email protected]>
Added a YAML schema to support tpm tis i2c related 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 | 50 ++++++++++++++++++++++
1 file changed, 50 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..68b13d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%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:
+ items:
+ - enum:
+ # Nuvoton's Trusted Platform Module (TPM) (NPCT75x)
+ - nuvoton,npct75x
+ - const: tcg,tpm-tis-i2c
+
+ reg:
+ maxItems: 1
+
+ interrupt:
+ maxItems: 1
+
+ crc-checksum:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Set this flag to enable CRC checksum.
+
+required:
+ - compatible
+ - reg
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ tpm@2e {
+ compatible = "nuvoton,npct75x", "tcg,tpm-tis-i2c";
+ reg = <0x2e>;
+ crc-checksum;
+ };
+ };
+...
--
2.7.4
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]>
Tested-by: Eddie James <[email protected]>
Tested-by: Joel Stanley <[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..2116d94 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
+ select 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..4c9bad0
--- /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_verify_data_integrity(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,
+ .verify_data_integrity = tpm_tis_i2c_verify_data_integrity,
+};
+
+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 = "nuvoton,npct75x", },
+ { .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");
+MODULE_LICENSE("GPL");
--
2.7.4
From: Amir Mizinski <[email protected]>
Only tpm_tis can use memory-mapped I/O, which is truly mapped into
the kernel's memory space. Therefore, using ioread16/ioread32/iowrite32
turns into a straightforward pointer dereference.
Every other driver requires more complicated operations to read more than
one byte at a time and will just fall back to read_bytes/write_bytes.
Therefore, move this common code out of tpm_tis_spi and into tpm_tis_core
so that it is used automatically when low-level drivers do not implement
the specialized methods.
Co-developed-by: Alexander Steffen <[email protected]>
Signed-off-by: Alexander Steffen <[email protected]>
Signed-off-by: Amir Mizinski <[email protected]>
---
drivers/char/tpm/tpm_tis_core.h | 38 +++++++++++++++++++++++++++++++---
drivers/char/tpm/tpm_tis_spi.h | 4 ----
drivers/char/tpm/tpm_tis_spi_cr50.c | 3 ---
drivers/char/tpm/tpm_tis_spi_main.c | 41 -------------------------------------
4 files changed, 35 insertions(+), 51 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.h b/drivers/char/tpm/tpm_tis_spi.h
index bba7397..d0f66f6 100644
--- a/drivers/char/tpm/tpm_tis_spi.h
+++ b/drivers/char/tpm/tpm_tis_spi.h
@@ -31,10 +31,6 @@ extern int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy,
extern int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
u8 *in, const u8 *out);
-extern int tpm_tis_spi_read16(struct tpm_tis_data *data, u32 addr, u16 *result);
-extern int tpm_tis_spi_read32(struct tpm_tis_data *data, u32 addr, u32 *result);
-extern int tpm_tis_spi_write32(struct tpm_tis_data *data, u32 addr, u32 value);
-
#ifdef CONFIG_TCG_TIS_SPI_CR50
extern int cr50_spi_probe(struct spi_device *spi);
#else
diff --git a/drivers/char/tpm/tpm_tis_spi_cr50.c b/drivers/char/tpm/tpm_tis_spi_cr50.c
index 37d72e8..f339d20 100644
--- a/drivers/char/tpm/tpm_tis_spi_cr50.c
+++ b/drivers/char/tpm/tpm_tis_spi_cr50.c
@@ -215,9 +215,6 @@ static int tpm_tis_spi_cr50_write_bytes(struct tpm_tis_data *data, u32 addr,
static const struct tpm_tis_phy_ops tpm_spi_cr50_phy_ops = {
.read_bytes = tpm_tis_spi_cr50_read_bytes,
.write_bytes = tpm_tis_spi_cr50_write_bytes,
- .read16 = tpm_tis_spi_read16,
- .read32 = tpm_tis_spi_read32,
- .write32 = tpm_tis_spi_write32,
};
static void cr50_print_fw_version(struct tpm_tis_data *data)
diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c
index d1754fd..95fef9d 100644
--- a/drivers/char/tpm/tpm_tis_spi_main.c
+++ b/drivers/char/tpm/tpm_tis_spi_main.c
@@ -152,44 +152,6 @@ static int tpm_tis_spi_write_bytes(struct tpm_tis_data *data, u32 addr,
return tpm_tis_spi_transfer(data, addr, len, NULL, value);
}
-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;
-}
-
-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;
-}
-
-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;
-}
-
int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy,
int irq, const struct tpm_tis_phy_ops *phy_ops)
{
@@ -205,9 +167,6 @@ int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy,
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
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.
Suggested-by: Benoit Houyere <[email protected]>
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..6e42946 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
From: Amir Mizinski <[email protected]>
Incorrect implementation of send message was detected. We polled only
TPM_STS.stsValid bit and then we single-checked the TPM_STS.expect bit
value.
TPM_STS.expected bit should be checked at the same time as
TPM_STS.stsValid bit, and this should be repeated until timeout_A.
To detect a TPM_STS.expected bit reset, the "wait_for_tpm_stat" function is
modified to "wait_for_tpm_stat_result". This function regularly reads the
status register and check the bits defined by "mask" to reach the value
defined in "mask_result".
This correct implementation is required for using the new CRC calculation
on I2C TPM command bytes or I2C TPM answer bytes. TPM_STS.expected bit is
reset after all bytes are acquired and the CRC result is inserted in the
dedicated register. It introduces a normal latency for TPM_STS.expected
bit reset.
Respectively, to send a message, as defined in
TCG_DesignPrinciples_TPM2p0Driver_vp24_pubrev.pdf, all bytes should be
sent in one shot instead of sending the last byte separately.
Suggested-by: Benoit Houyere <[email protected]>
Signed-off-by: Amir Mizinski <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 74 +++++++++++++++++------------------------
1 file changed, 30 insertions(+), 44 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 27c6ca0..c725b68 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));
}
@@ -281,10 +282,13 @@ 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);
@@ -337,8 +341,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;
}
@@ -364,61 +369,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_a,
+ &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;
@@ -470,9 +454,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
From: Amir Mizinski <[email protected]>
The FIFO protocol described in the TCG PC Client Device Driver Design
Principles for TPM 2.0 advises retrying sending a command or receiving
a response using the FIFO protocol in case of any error in the protocol.
Add a retry mechanism on any protocol error. In addition, in case of a data
integrity issue in the I2C bus protocol, check after sending a command
completion or receiving a response from the TPM.
Co-developed-by: Christophe Ricard <[email protected]>
Signed-off-by: Christophe Ricard <[email protected]>
Signed-off-by: Amir Mizinski <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 107 ++++++++++++++++++++++++----------------
drivers/char/tpm/tpm_tis_core.h | 3 ++
2 files changed, 67 insertions(+), 43 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index c725b68..97eae72 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -312,7 +312,7 @@ 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;
if (count < TPM_HEADER_SIZE) {
@@ -320,40 +320,53 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
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;
- }
+ for (i = 0; i < TPM_RETRY; i++) {
+ 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 retry;
+ }
- 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 retry;
+ }
- 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 retry;
+ }
- 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;
- }
- 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;
- }
+ if (wait_for_tpm_stat_result(chip, TPM_STS_VALID,
+ TPM_STS_VALID, chip->timeout_c,
+ &priv->int_queue, false) < 0) {
+ size = -ETIME;
+ goto retry;
+ }
+ status = tpm_tis_status(chip);
+ if (status & TPM_STS_DATA_AVAIL) { /* retry? */
+ dev_err(&chip->dev, "Error left over data\n");
+ size = -EIO;
+ goto retry;
+ }
+
+ if (priv->phy_ops->verify_data_integrity)
+ if (!priv->phy_ops->verify_data_integrity(priv, buf,
+ size))
+ size = -EIO;
+retry:
+ if (size <= 0)
+ tpm_tis_write8(priv, TPM_STS(priv->locality),
+ TPM_STS_RESPONSE_RETRY);
+ else
+ goto out;
+ }
out:
tpm_tis_ready(chip);
return size;
@@ -378,7 +391,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
chip->timeout_b,
&priv->int_queue, false) < 0) {
rc = -ETIME;
- goto out_err;
+ return rc;
}
}
@@ -387,13 +400,13 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
if (burstcnt < 0) {
dev_err(&chip->dev, "Unable to read burstcount\n");
rc = burstcnt;
- goto out_err;
+ return rc;
}
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;
+ return rc;
count += burstcnt;
}
@@ -401,14 +414,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
TPM_STS_VALID, chip->timeout_a,
&priv->int_queue, false) < 0) {
rc = -ETIME;
- goto out_err;
+ return rc;
}
return 0;
-
-out_err:
- tpm_tis_ready(chip);
- return rc;
}
static void disable_interrupts(struct tpm_chip *chip)
@@ -437,13 +446,25 @@ 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;
- rc = tpm_tis_send_data(chip, buf, len);
- if (rc < 0)
- return rc;
+ for (i = 0; i < TPM_RETRY; i++) {
+ rc = tpm_tis_send_data(chip, buf, len);
+ if (rc < 0)
+ continue;
+ if (priv->phy_ops->verify_data_integrity) {
+ if (!priv->phy_ops->verify_data_integrity(priv, buf,
+ len)){
+ rc = -EIO;
+ continue;
+ }
+ }
+ break;
+ }
+ if (i == TPM_RETRY)
+ goto out_err;
/* go and do it */
rc = tpm_tis_write8(priv, TPM_STS(priv->locality), TPM_STS_GO);
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index d06c65b..cd97c01 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 (*verify_data_integrity)(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
From: Amir Mizinski <[email protected]>
Issue could result when the TPM does not update TPM_STS register after
a locality request (TPM_STS Initial value = 0xFF) and a TPM_STS register
read occurs (tpm_tis_status(chip)).
Checking the next condition("if ((status & TPM_STS_COMMAND_READY) == 0)"),
the status will be at 0xFF and will be considered, wrongly, in "Ready"
state (by checking only one bit). However, at this moment the TPM is, in
fact, in "Idle" state and remains in "Idle" state because
"tpm_tis_ready(chip);" was not executed.
Suggested-by: Benoit Houyere <[email protected]>
Signed-off-by: Amir Mizinski <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 39b28a0..d9c9ff1 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -222,8 +222,14 @@ static int request_locality(struct tpm_chip *chip, int l)
} else {
/* wait for burstcount */
do {
- if (check_locality(chip, l))
+ if (check_locality(chip, l)) {
+ if (wait_for_tpm_stat_result(chip, TPM_STS_GO,
+ 0, chip->timeout_c,
+ &priv->int_queue,
+ false) < 0)
+ return -ETIME;
return l;
+ }
tpm_msleep(TPM_TIMEOUT);
} while (time_before(jiffies, stop));
}
--
2.7.4
On Tue, 26 May 2020 17:16:57 +0300, [email protected] wrote:
> From: Amir Mizinski <[email protected]>
>
> Added a YAML schema to support tpm tis i2c related 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 | 50 ++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>
Reviewed-by: Rob Herring <[email protected]>
Plese, write the short summary as
tpm: Make read{16, 32}() and write32() in tpm_tis_phy_ops optional
On Tue, May 26, 2020 at 05:16:51PM +0300, [email protected] wrote:
> From: Amir Mizinski <[email protected]>
>
> Only tpm_tis can use memory-mapped I/O, which is truly mapped into
> the kernel's memory space. Therefore, using ioread16/ioread32/iowrite32
> turns into a straightforward pointer dereference.
> Every other driver requires more complicated operations to read more than
> one byte at a time and will just fall back to read_bytes/write_bytes.
> Therefore, move this common code out of tpm_tis_spi and into tpm_tis_core
> so that it is used automatically when low-level drivers do not implement
> the specialized methods.
>
> Co-developed-by: Alexander Steffen <[email protected]>
> Signed-off-by: Alexander Steffen <[email protected]>
> Signed-off-by: Amir Mizinski <[email protected]>
> ---
> drivers/char/tpm/tpm_tis_core.h | 38 +++++++++++++++++++++++++++++++---
> drivers/char/tpm/tpm_tis_spi.h | 4 ----
> drivers/char/tpm/tpm_tis_spi_cr50.c | 3 ---
> drivers/char/tpm/tpm_tis_spi_main.c | 41 -------------------------------------
> 4 files changed, 35 insertions(+), 51 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.h b/drivers/char/tpm/tpm_tis_spi.h
> index bba7397..d0f66f6 100644
> --- a/drivers/char/tpm/tpm_tis_spi.h
> +++ b/drivers/char/tpm/tpm_tis_spi.h
> @@ -31,10 +31,6 @@ extern int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy,
> extern int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
> u8 *in, const u8 *out);
>
> -extern int tpm_tis_spi_read16(struct tpm_tis_data *data, u32 addr, u16 *result);
> -extern int tpm_tis_spi_read32(struct tpm_tis_data *data, u32 addr, u32 *result);
> -extern int tpm_tis_spi_write32(struct tpm_tis_data *data, u32 addr, u32 value);
> -
> #ifdef CONFIG_TCG_TIS_SPI_CR50
> extern int cr50_spi_probe(struct spi_device *spi);
> #else
> diff --git a/drivers/char/tpm/tpm_tis_spi_cr50.c b/drivers/char/tpm/tpm_tis_spi_cr50.c
> index 37d72e8..f339d20 100644
> --- a/drivers/char/tpm/tpm_tis_spi_cr50.c
> +++ b/drivers/char/tpm/tpm_tis_spi_cr50.c
> @@ -215,9 +215,6 @@ static int tpm_tis_spi_cr50_write_bytes(struct tpm_tis_data *data, u32 addr,
> static const struct tpm_tis_phy_ops tpm_spi_cr50_phy_ops = {
> .read_bytes = tpm_tis_spi_cr50_read_bytes,
> .write_bytes = tpm_tis_spi_cr50_write_bytes,
> - .read16 = tpm_tis_spi_read16,
> - .read32 = tpm_tis_spi_read32,
> - .write32 = tpm_tis_spi_write32,
> };
>
> static void cr50_print_fw_version(struct tpm_tis_data *data)
> diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c
> index d1754fd..95fef9d 100644
> --- a/drivers/char/tpm/tpm_tis_spi_main.c
> +++ b/drivers/char/tpm/tpm_tis_spi_main.c
> @@ -152,44 +152,6 @@ static int tpm_tis_spi_write_bytes(struct tpm_tis_data *data, u32 addr,
> return tpm_tis_spi_transfer(data, addr, len, NULL, value);
> }
>
> -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;
> -}
> -
> -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;
> -}
> -
> -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;
> -}
> -
> int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy,
> int irq, const struct tpm_tis_phy_ops *phy_ops)
> {
> @@ -205,9 +167,6 @@ int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy,
> 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
Other than that looks good.
/Jarkko
>
On Tue, May 26, 2020 at 05:16:52PM +0300, [email protected] wrote:
> From: Amir Mizinski <[email protected]>
>
> Incorrect implementation of send message was detected. We polled only
> TPM_STS.stsValid bit and then we single-checked the TPM_STS.expect bit
> value.
> TPM_STS.expected bit should be checked at the same time as
> TPM_STS.stsValid bit, and this should be repeated until timeout_A.
I don't understand what the first paragraph is trying to say. It does
not conclude to anything. Please write instead soemthing that explains
what is going on.
> To detect a TPM_STS.expected bit reset, the "wait_for_tpm_stat" function is
> modified to "wait_for_tpm_stat_result". This function regularly reads the
> status register and check the bits defined by "mask" to reach the value
> defined in "mask_result".
Please remove this and explain instead how are you are changing the
existing function.
> This correct implementation is required for using the new CRC calculation
> on I2C TPM command bytes or I2C TPM answer bytes. TPM_STS.expected bit is
> reset after all bytes are acquired and the CRC result is inserted in the
> dedicated register. It introduces a normal latency for TPM_STS.expected
> bit reset.
>
> Respectively, to send a message, as defined in
> TCG_DesignPrinciples_TPM2p0Driver_vp24_pubrev.pdf, all bytes should be
> sent in one shot instead of sending the last byte separately.
>
> Suggested-by: Benoit Houyere <[email protected]>
> Signed-off-by: Amir Mizinski <[email protected]>
> ---
> drivers/char/tpm/tpm_tis_core.c | 74 +++++++++++++++++------------------------
> 1 file changed, 30 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 27c6ca0..c725b68 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)
Please do not change the function name.
/Jarkko