* [PATCH v2 1/5] char: tpm: Make implementation of read16 read32 write32 optional
2019-12-02 13:33 [PATCH v2 0/5] add tpm i2c ptp driver amirmizi6
@ 2019-12-02 13:33 ` amirmizi6
2019-12-02 13:33 ` [PATCH v2 2/5] char: tpm: Add check_data handle to tpm_tis_phy_ops in order to check data integrity amirmizi6
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: amirmizi6 @ 2019-12-02 13:33 UTC (permalink / raw)
To: Eyal.Cohen, jarkko.sakkinen, oshrialkoby85, alexander.steffen,
robh+dt, mark.rutland, peterhuewe, jgg, arnd, gregkh
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, ayna, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski, Amir Mizinski
From: Amir Mizinski <amirmizi6@gmail.com>
Only tpm_tis has a faster way to access multiple bytes at once, every other
driver will just fall back to read_bytes/write_bytes. Therefore, move this
common code out of tpm_tis_spi into tpm_tis_core, so that it is
automatically used when low-level drivers do not implement the specialized
methods.
This commit is based on previous work by Alexander Steffen.
Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
drivers/char/tpm/tpm_tis_core.h | 38 +++++++++++++++++++++++++++++++++++---
drivers/char/tpm/tpm_tis_spi.c | 41 -----------------------------------------
2 files changed, 35 insertions(+), 44 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 7337819..d06c65b 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -122,13 +122,35 @@ static inline int tpm_tis_read8(struct tpm_tis_data *data, u32 addr, u8 *result)
static inline int tpm_tis_read16(struct tpm_tis_data *data, u32 addr,
u16 *result)
{
- return data->phy_ops->read16(data, addr, result);
+ __le16 result_le;
+ int rc;
+
+ if (data->phy_ops->read16)
+ return data->phy_ops->read16(data, addr, result);
+
+ rc = data->phy_ops->read_bytes(data, addr, sizeof(u16),
+ (u8 *)&result_le);
+ if (!rc)
+ *result = le16_to_cpu(result_le);
+
+ return rc;
}
static inline int tpm_tis_read32(struct tpm_tis_data *data, u32 addr,
u32 *result)
{
- return data->phy_ops->read32(data, addr, result);
+ __le32 result_le;
+ int rc;
+
+ if (data->phy_ops->read32)
+ return data->phy_ops->read32(data, addr, result);
+
+ rc = data->phy_ops->read_bytes(data, addr, sizeof(u32),
+ (u8 *)&result_le);
+ if (!rc)
+ *result = le32_to_cpu(result_le);
+
+ return rc;
}
static inline int tpm_tis_write_bytes(struct tpm_tis_data *data, u32 addr,
@@ -145,7 +167,17 @@ static inline int tpm_tis_write8(struct tpm_tis_data *data, u32 addr, u8 value)
static inline int tpm_tis_write32(struct tpm_tis_data *data, u32 addr,
u32 value)
{
- return data->phy_ops->write32(data, addr, value);
+ __le32 value_le;
+ int rc;
+
+ if (data->phy_ops->write32)
+ return data->phy_ops->write32(data, addr, value);
+
+ value_le = cpu_to_le32(value);
+ rc = data->phy_ops->write_bytes(data, addr, sizeof(u32),
+ (u8 *)&value_le);
+
+ return rc;
}
static inline bool is_bsw(void)
diff --git a/drivers/char/tpm/tpm_tis_spi.c b/drivers/char/tpm/tpm_tis_spi.c
index 19513e6..da82924 100644
--- a/drivers/char/tpm/tpm_tis_spi.c
+++ b/drivers/char/tpm/tpm_tis_spi.c
@@ -146,50 +146,9 @@ static int tpm_tis_spi_write_bytes(struct tpm_tis_data *data, u32 addr,
return tpm_tis_spi_transfer(data, addr, len, NULL, value);
}
-static int tpm_tis_spi_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
-{
- __le16 result_le;
- int rc;
-
- rc = data->phy_ops->read_bytes(data, addr, sizeof(u16),
- (u8 *)&result_le);
- if (!rc)
- *result = le16_to_cpu(result_le);
-
- return rc;
-}
-
-static int tpm_tis_spi_read32(struct tpm_tis_data *data, u32 addr, u32 *result)
-{
- __le32 result_le;
- int rc;
-
- rc = data->phy_ops->read_bytes(data, addr, sizeof(u32),
- (u8 *)&result_le);
- if (!rc)
- *result = le32_to_cpu(result_le);
-
- return rc;
-}
-
-static int tpm_tis_spi_write32(struct tpm_tis_data *data, u32 addr, u32 value)
-{
- __le32 value_le;
- int rc;
-
- value_le = cpu_to_le32(value);
- rc = data->phy_ops->write_bytes(data, addr, sizeof(u32),
- (u8 *)&value_le);
-
- return rc;
-}
-
static const struct tpm_tis_phy_ops tpm_spi_phy_ops = {
.read_bytes = tpm_tis_spi_read_bytes,
.write_bytes = tpm_tis_spi_write_bytes,
- .read16 = tpm_tis_spi_read16,
- .read32 = tpm_tis_spi_read32,
- .write32 = tpm_tis_spi_write32,
};
static int tpm_tis_spi_probe(struct spi_device *dev)
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 2/5] char: tpm: Add check_data handle to tpm_tis_phy_ops in order to check data integrity
2019-12-02 13:33 [PATCH v2 0/5] add tpm i2c ptp driver amirmizi6
2019-12-02 13:33 ` [PATCH v2 1/5] char: tpm: Make implementation of read16 read32 write32 optional amirmizi6
@ 2019-12-02 13:33 ` amirmizi6
2019-12-02 13:33 ` [PATCH v2 3/5] char: tpm: rewrite "tpm_tis_req_canceled()" amirmizi6
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: amirmizi6 @ 2019-12-02 13:33 UTC (permalink / raw)
To: Eyal.Cohen, jarkko.sakkinen, oshrialkoby85, alexander.steffen,
robh+dt, mark.rutland, peterhuewe, jgg, arnd, gregkh
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, ayna, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski, Amir Mizinski
From: Amir Mizinski <amirmizi6@gmail.com>
In order to compute the crc over the data sent in lower layer (I2C for instance),
tpm_tis_check_data() calls an operation (if available) to check data integrity.
If data integrity cannot be verified, a retry attempt to save the sent/recieved data is implemented.
The current steps are done when sending a command:
1. Host writes to TPM_STS.commandReady.
2. Host writes command.
3. Host checks that TPM received data is valid.
4. If data is currupted go to step 1.
When receiving data:
1. Host checks that TPM_STS.dataAvail is set.
2. Host saves recieved data.
3. Host checks that received data is correct.
4. If data is currupted Host writes to TPM_STS.responseRetry and go to step 1.
This commit is based on previous work by Christophe Richard.
Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
drivers/char/tpm/tpm_tis_core.c | 101 +++++++++++++++++++++++++---------------
drivers/char/tpm/tpm_tis_core.h | 3 ++
2 files changed, 66 insertions(+), 38 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index c3181ea..43784fd 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -242,6 +242,15 @@ static u8 tpm_tis_status(struct tpm_chip *chip)
return status;
}
+static bool tpm_tis_check_data(struct tpm_chip *chip, const u8 *buf, size_t len)
+{
+ struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+
+ if (priv->phy_ops->check_data)
+ return priv->phy_ops->check_data(priv, buf, len);
+ return true;
+}
+
static void tpm_tis_ready(struct tpm_chip *chip)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
@@ -308,47 +317,58 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
int size = 0;
- int status;
+ int status, i;
u32 expected;
+ bool check_data = false;
- if (count < TPM_HEADER_SIZE) {
- size = -EIO;
- goto out;
- }
+ for (i = 0; i < TPM_RETRY; i++) {
+ if (count < TPM_HEADER_SIZE) {
+ size = -EIO;
+ goto out;
+ }
- size = recv_data(chip, buf, TPM_HEADER_SIZE);
- /* read first 10 bytes, including tag, paramsize, and result */
- if (size < TPM_HEADER_SIZE) {
- dev_err(&chip->dev, "Unable to read header\n");
- goto out;
- }
+ size = recv_data(chip, buf, TPM_HEADER_SIZE);
+ /* read first 10 bytes, including tag, paramsize, and result */
+ if (size < TPM_HEADER_SIZE) {
+ dev_err(&chip->dev, "Unable to read header\n");
+ goto out;
+ }
- expected = be32_to_cpu(*(__be32 *) (buf + 2));
- if (expected > count || expected < TPM_HEADER_SIZE) {
- size = -EIO;
- goto out;
- }
+ expected = be32_to_cpu(*(__be32 *) (buf + 2));
+ if (expected > count || expected < TPM_HEADER_SIZE) {
+ size = -EIO;
+ goto out;
+ }
- size += recv_data(chip, &buf[TPM_HEADER_SIZE],
- expected - TPM_HEADER_SIZE);
- if (size < expected) {
- dev_err(&chip->dev, "Unable to read remainder of result\n");
- size = -ETIME;
- goto out;
- }
+ size += recv_data(chip, &buf[TPM_HEADER_SIZE],
+ expected - TPM_HEADER_SIZE);
+ if (size < expected) {
+ dev_err(&chip->dev, "Unable to read remainder of result\n");
+ size = -ETIME;
+ goto out;
+ }
- if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
- &priv->int_queue, false) < 0) {
- size = -ETIME;
- goto out;
+ if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
+ &priv->int_queue, false) < 0) {
+ size = -ETIME;
+ goto out;
+ }
+
+ status = tpm_tis_status(chip);
+ if (status & TPM_STS_DATA_AVAIL) { /* retry? */
+ dev_err(&chip->dev, "Error left over data\n");
+ size = -EIO;
+ goto out;
+ }
+
+ check_data = tpm_tis_check_data(chip, buf, size);
+ if (!check_data)
+ tpm_tis_write8(priv, TPM_STS(priv->locality),
+ TPM_STS_RESPONSE_RETRY);
+ else break;
}
- status = tpm_tis_status(chip);
- if (status & TPM_STS_DATA_AVAIL) { /* retry? */
- dev_err(&chip->dev, "Error left over data\n");
+ if (!check_data)
size = -EIO;
- goto out;
- }
-
out:
tpm_tis_ready(chip);
return size;
@@ -453,14 +473,19 @@ static void disable_interrupts(struct tpm_chip *chip)
static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
- int rc;
+ int rc, i;
u32 ordinal;
unsigned long dur;
+ bool data_valid = false;
- rc = tpm_tis_send_data(chip, buf, len);
- if (rc < 0)
- return rc;
-
+ for (i = 0; i < TPM_RETRY && !data_valid; i++) {
+ rc = tpm_tis_send_data(chip, buf, len);
+ if (rc < 0)
+ return rc;
+ data_valid = tpm_tis_check_data(chip, buf, len);
+ }
+ if (!data_valid)
+ return -EIO;
/* go and do it */
rc = tpm_tis_write8(priv, TPM_STS(priv->locality), TPM_STS_GO);
if (rc < 0)
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index d06c65b..486c2e9 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -34,6 +34,7 @@ enum tis_status {
TPM_STS_GO = 0x20,
TPM_STS_DATA_AVAIL = 0x10,
TPM_STS_DATA_EXPECT = 0x08,
+ TPM_STS_RESPONSE_RETRY = 0x02,
};
enum tis_int_flags {
@@ -106,6 +107,8 @@ struct tpm_tis_phy_ops {
int (*read16)(struct tpm_tis_data *data, u32 addr, u16 *result);
int (*read32)(struct tpm_tis_data *data, u32 addr, u32 *result);
int (*write32)(struct tpm_tis_data *data, u32 addr, u32 src);
+ bool (*check_data)(struct tpm_tis_data *data, const u8 *buf,
+ size_t len);
};
static inline int tpm_tis_read_bytes(struct tpm_tis_data *data, u32 addr,
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 3/5] char: tpm: rewrite "tpm_tis_req_canceled()"
2019-12-02 13:33 [PATCH v2 0/5] add tpm i2c ptp driver amirmizi6
2019-12-02 13:33 ` [PATCH v2 1/5] char: tpm: Make implementation of read16 read32 write32 optional amirmizi6
2019-12-02 13:33 ` [PATCH v2 2/5] char: tpm: Add check_data handle to tpm_tis_phy_ops in order to check data integrity amirmizi6
@ 2019-12-02 13:33 ` amirmizi6
2019-12-02 13:33 ` [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options amirmizi6
2019-12-02 13:33 ` [PATCH v2 5/5] char: tpm: add tpm_tis_i2c driver amirmizi6
4 siblings, 0 replies; 9+ messages in thread
From: amirmizi6 @ 2019-12-02 13:33 UTC (permalink / raw)
To: Eyal.Cohen, jarkko.sakkinen, oshrialkoby85, alexander.steffen,
robh+dt, mark.rutland, peterhuewe, jgg, arnd, gregkh
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, ayna, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski, Amir Mizinski
From: Amir Mizinski <amirmizi6@gmail.com>
Using this function while read/write data resulted in aborted operation.
After investigating according to TCG TPM Profile (PTP) Specifications,
i found cancel should happen only if TPM_STS.commandReady bit is lit
and couldn't find a case when the current condition is valid.
Also only cmdReady bit need to be compared instead of the full lower status register byte.
Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
drivers/char/tpm/tpm_tis_core.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 43784fd..924489c 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -631,17 +631,7 @@ static int probe_itpm(struct tpm_chip *chip)
static bool tpm_tis_req_canceled(struct tpm_chip *chip, u8 status)
{
- struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
-
- switch (priv->manufacturer_id) {
- case TPM_VID_WINBOND:
- return ((status == TPM_STS_VALID) ||
- (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY)));
- case TPM_VID_STM:
- return (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY));
- default:
- return (status == TPM_STS_COMMAND_READY);
- }
+ return ((status & TPM_STS_COMMAND_READY) == TPM_STS_COMMAND_READY);
}
static irqreturn_t tis_int_handler(int dummy, void *dev_id)
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options
2019-12-02 13:33 [PATCH v2 0/5] add tpm i2c ptp driver amirmizi6
` (2 preceding siblings ...)
2019-12-02 13:33 ` [PATCH v2 3/5] char: tpm: rewrite "tpm_tis_req_canceled()" amirmizi6
@ 2019-12-02 13:33 ` amirmizi6
2019-12-13 22:36 ` Rob Herring
2019-12-02 13:33 ` [PATCH v2 5/5] char: tpm: add tpm_tis_i2c driver amirmizi6
4 siblings, 1 reply; 9+ messages in thread
From: amirmizi6 @ 2019-12-02 13:33 UTC (permalink / raw)
To: Eyal.Cohen, jarkko.sakkinen, oshrialkoby85, alexander.steffen,
robh+dt, mark.rutland, peterhuewe, jgg, arnd, gregkh
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, ayna, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski, Amir Mizinski
From: Amir Mizinski <amirmizi6@gmail.com>
Added a YAML schema to support tpm tis i2c realted dt-bindings for the I2c PTP based physical layer.
Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
.../bindings/security/tpm/tpm-tis-i2c.yaml | 38 ++++++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
new file mode 100644
index 0000000..bb18917
--- /dev/null
+++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/security/tpm/tpm-tis-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: I2C PTP based TPM Device Tree Bindings
+
+maintainers:
+ - Amir Mizinski <amirmizi6@gmail.com>
+
+description:
+ The TCG defines hardware protocol, registers and interface (based
+ on the TPM Interface Specification) for accessing TPM devices
+ implemented with an I2C interface.
+ Refer to the 'I2C Interface Definition' section in 'TCG PC Client
+ PlatformTPMProfile(PTP) Specification' publication for specification.
+ Designed for Raspberry Pie 3 Board with Nuvoton's NPCT75X (2.0)
+
+properties:
+ compatible:
+ contains:
+ const: tcg,tpm-tis-i2c
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+
+examples:
+ - |
+ tpm-tis-i2c: tpm-tis-i2c@2e {
+ compatible = "tcg,tpm-tis-i2c";
+ reg = <0x2e>;
+ };
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options
2019-12-02 13:33 ` [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options amirmizi6
@ 2019-12-13 22:36 ` Rob Herring
2019-12-16 13:53 ` Amir Mizinski
0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2019-12-13 22:36 UTC (permalink / raw)
To: amirmizi6
Cc: Eyal.Cohen, jarkko.sakkinen, oshrialkoby85, alexander.steffen,
mark.rutland, peterhuewe, jgg, arnd, gregkh, devicetree,
linux-kernel, linux-integrity, oshri.alkoby, tmaimon77, gcwilson,
kgoldman, ayna, Dan.Morav, oren.tanami, shmulik.hager,
amir.mizinski
On Mon, Dec 02, 2019 at 03:33:31PM +0200, amirmizi6@gmail.com wrote:
> From: Amir Mizinski <amirmizi6@gmail.com>
>
> Added a YAML schema to support tpm tis i2c realted dt-bindings for the I2c PTP based physical layer.
Wrap your commmit message. And TPM, TIS?, and I2C should be capitalized.
>
> Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
> ---
> .../bindings/security/tpm/tpm-tis-i2c.yaml | 38 ++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
Please read my comments on v1 (The first v1 from 11/10, not the 2nd v1
you sent).
Rob
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options
2019-12-13 22:36 ` Rob Herring
@ 2019-12-16 13:53 ` Amir Mizinski
2019-12-16 14:17 ` Rob Herring
0 siblings, 1 reply; 9+ messages in thread
From: Amir Mizinski @ 2019-12-16 13:53 UTC (permalink / raw)
To: Rob Herring
Cc: Eyal.Cohen, Jarkko Sakkinen, Oshri Alkobi, Alexander Steffen,
Mark Rutland, peterhuewe, jgg, Arnd Bergmann, Greg KH,
devicetree, Linux Kernel Mailing List, linux-integrity,
IS20 Oshri Alkoby, Tomer Maimon, gcwilson, kgoldman, ayna,
IS30 Dan Morav, oren.tanami, shmulik.hager, amir.mizinski
On Sat, Dec 14, 2019 at 12:36 AM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Dec 02, 2019 at 03:33:31PM +0200, amirmizi6@gmail.com wrote:
> > From: Amir Mizinski <amirmizi6@gmail.com>
> >
> > Added a YAML schema to support tpm tis i2c realted dt-bindings for the I2c PTP based physical layer.
>
> Wrap your commmit message. And TPM, TIS?, and I2C should be capitalized.
Thanks, ill fix that.
>
> >
> > Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
> > ---
> > .../bindings/security/tpm/tpm-tis-i2c.yaml | 38 ++++++++++++++++++++++
> > 1 file changed, 38 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>
> Please read my comments on v1 (The first v1 from 11/10, not the 2nd v1
> you sent).
I sent a follow up comment regarding this:
https://patchwork.kernel.org/patch/11236253/
(2nd v1 was sent by mistake. sorry about that)
>
> Rob
Amir Mizinski
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options
2019-12-16 13:53 ` Amir Mizinski
@ 2019-12-16 14:17 ` Rob Herring
0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2019-12-16 14:17 UTC (permalink / raw)
To: Amir Mizinski
Cc: Eyal.Cohen, Jarkko Sakkinen, Oshri Alkobi, Alexander Steffen,
Mark Rutland, Peter Huewe, Jason Gunthorpe, Arnd Bergmann,
Greg KH, devicetree, Linux Kernel Mailing List, linux-integrity,
IS20 Oshri Alkoby, Tomer Maimon, gcwilson, kgoldman, ayna,
IS30 Dan Morav, oren.tanami, shmulik.hager, amir.mizinski
On Mon, Dec 16, 2019 at 7:53 AM Amir Mizinski <amirmizi6@gmail.com> wrote:
>
> On Sat, Dec 14, 2019 at 12:36 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Mon, Dec 02, 2019 at 03:33:31PM +0200, amirmizi6@gmail.com wrote:
> > > From: Amir Mizinski <amirmizi6@gmail.com>
> > >
> > > Added a YAML schema to support tpm tis i2c realted dt-bindings for the I2c PTP based physical layer.
> >
> > Wrap your commmit message. And TPM, TIS?, and I2C should be capitalized.
>
> Thanks, ill fix that.
>
> >
> > >
> > > Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
> > > ---
> > > .../bindings/security/tpm/tpm-tis-i2c.yaml | 38 ++++++++++++++++++++++
> > > 1 file changed, 38 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> >
> > Please read my comments on v1 (The first v1 from 11/10, not the 2nd v1
> > you sent).
>
> I sent a follow up comment regarding this:
> https://patchwork.kernel.org/patch/11236253/
> (2nd v1 was sent by mistake. sorry about that)
Sorry I missed your reply. However, you didn't address these comments:
> There's a bigger issue that the h/w here is more than just an I2C
> protocol. The chip may have multiple power supplies, clocks, reset
> lines, etc. HID over I2C seems like a similar case. Does the spec define
> *all* of that? If not, you need chip specific compatibles. You can keep
> this as a fallback though.
To rephrase this, a protocol does not fully describe the h/w and DT
should describe the h/w.
Also, you should include the interrupt whether you use it in the
driver currently or not. Again, it's about describing the h/w, not
what a driver happens to use ATM.
Rob
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 5/5] char: tpm: add tpm_tis_i2c driver
2019-12-02 13:33 [PATCH v2 0/5] add tpm i2c ptp driver amirmizi6
` (3 preceding siblings ...)
2019-12-02 13:33 ` [PATCH v2 4/5] dt-bindings: tpm: Add YAML schema for TPM TIS I2C options amirmizi6
@ 2019-12-02 13:33 ` amirmizi6
4 siblings, 0 replies; 9+ messages in thread
From: amirmizi6 @ 2019-12-02 13:33 UTC (permalink / raw)
To: Eyal.Cohen, jarkko.sakkinen, oshrialkoby85, alexander.steffen,
robh+dt, mark.rutland, peterhuewe, jgg, arnd, gregkh
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, ayna, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski, Amir Mizinski
From: Amir Mizinski <amirmizi6@gmail.com>
Implements the functionality needed to communicate with an I2C TPM
according to the TCG TPM I2C Interface Specification.
Limitations:
* No IRQ support
* No support for updating GUARD_TIME
Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
drivers/char/tpm/Kconfig | 12 ++
drivers/char/tpm/Makefile | 1 +
drivers/char/tpm/tpm_tis_i2c.c | 272 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 285 insertions(+)
create mode 100644 drivers/char/tpm/tpm_tis_i2c.c
diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 88a3c06..b731a80 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -67,6 +67,18 @@ config TCG_TIS_SPI
within Linux. To compile this driver as a module, choose M here;
the module will be called tpm_tis_spi.
+config TCG_TIS_I2C
+ tristate "TPM I2C Interface Specification"
+ depends on I2C
+ depends on CRC_CCITT
+ select TCG_TIS_CORE
+ ---help---
+ If you have a TPM security chip which is connected to a regular
+ I2C master (i.e. most embedded platforms) that is compliant with the
+ TCG TPM I2C Interface Specification say Yes and it will be accessible from
+ within Linux. To compile this driver as a module, choose M here;
+ the module will be called tpm_tis_i2c.
+
config TCG_TIS_I2C_ATMEL
tristate "TPM Interface Specification 1.2 Interface (I2C - Atmel)"
depends on I2C
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index a01c4ca..15a0010 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -22,6 +22,7 @@ tpm-$(CONFIG_OF) += eventlog/of.o
obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
obj-$(CONFIG_TCG_TIS) += tpm_tis.o
obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi.o
+obj-$(CONFIG_TCG_TIS_I2C) += tpm_tis_i2c.o
obj-$(CONFIG_TCG_TIS_I2C_ATMEL) += tpm_i2c_atmel.o
obj-$(CONFIG_TCG_TIS_I2C_INFINEON) += tpm_i2c_infineon.o
obj-$(CONFIG_TCG_TIS_I2C_NUVOTON) += tpm_i2c_nuvoton.o
diff --git a/drivers/char/tpm/tpm_tis_i2c.c b/drivers/char/tpm/tpm_tis_i2c.c
new file mode 100644
index 0000000..477bb55
--- /dev/null
+++ b/drivers/char/tpm/tpm_tis_i2c.c
@@ -0,0 +1,272 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2014-2019 Nuvoton Technology corporation
+ *
+ * TPM TIS I2C
+ *
+ * TPM TIS I2C Device Driver Interface for devices that implement the TPM I2C
+ * Interface defined by TCG PC Client Platform TPM Profile (PTP) Specification
+ * Revision 01.03 v22 at www.trustedcomputinggroup.org
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/wait.h>
+#include <linux/acpi.h>
+#include <linux/freezer.h>
+#include <linux/crc-ccitt.h>
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/gpio.h>
+#include <linux/of_irq.h>
+#include <linux/of_gpio.h>
+#include <linux/tpm.h>
+#include "tpm.h"
+#include "tpm_tis_core.h"
+
+#define TPM_LOC_SEL 0x04
+#define TPM_I2C_INTERFACE_CAPABILITY 0x30
+#define TPM_I2C_DEVICE_ADDRESS 0x38
+#define TPM_DATA_CSUM_ENABLE 0x40
+#define TPM_DATA_CSUM 0x44
+#define TPM_I2C_DID_VID 0x48
+#define TPM_I2C_RID 0x4C
+
+struct tpm_tis_i2c_phy {
+ struct tpm_tis_data priv;
+ struct i2c_client *i2c_client;
+ bool data_csum;
+ u8 *iobuf;
+};
+
+static inline struct tpm_tis_i2c_phy *to_tpm_tis_i2c_phy(struct tpm_tis_data *data)
+{
+ return container_of(data, struct tpm_tis_i2c_phy, priv);
+}
+
+static u8 address_to_register(u32 addr)
+{
+ addr &= 0xFFF;
+
+ switch (addr) {
+ // adapt register addresses that have changed compared to
+ // older TIS versions
+ case TPM_ACCESS(0):
+ return 0x04;
+ case TPM_LOC_SEL:
+ return 0x00;
+ case TPM_DID_VID(0):
+ return 0x48;
+ case TPM_RID(0):
+ return 0x4C;
+ default:
+ return addr;
+ }
+}
+
+static int tpm_tis_i2c_read_bytes(struct tpm_tis_data *data, u32 addr,
+ u16 len, u8 *result)
+{
+ struct tpm_tis_i2c_phy *phy = to_tpm_tis_i2c_phy(data);
+ int ret;
+ u8 reg = address_to_register(addr);
+ struct i2c_msg msgs[] = {
+ {
+ .addr = phy->i2c_client->addr,
+ .len = sizeof(reg),
+ .buf = ®,
+ },
+ {
+ .addr = phy->i2c_client->addr,
+ .len = len,
+ .buf = result,
+ .flags = I2C_M_RD,
+ },
+ };
+
+ ret = i2c_transfer(phy->i2c_client->adapter, msgs, ARRAY_SIZE(msgs));
+
+ if (ret < 0)
+ return ret;
+
+ usleep_range(250, 300); // wait default GUARD_TIME of 250µs
+
+ return 0;
+}
+
+static int tpm_tis_i2c_write_bytes(struct tpm_tis_data *data, u32 addr,
+ u16 len, const u8 *value)
+{
+ struct tpm_tis_i2c_phy *phy = to_tpm_tis_i2c_phy(data);
+ int ret;
+
+ if (phy->iobuf) {
+ if (len > TPM_BUFSIZE - 1)
+ return -EIO;
+
+ phy->iobuf[0] = address_to_register(addr);
+ memcpy(phy->iobuf + 1, value, len);
+
+ {
+ struct i2c_msg msgs[] = {
+ {
+ .addr = phy->i2c_client->addr,
+ .len = len + 1,
+ .buf = phy->iobuf,
+ },
+ };
+
+ ret = i2c_transfer(phy->i2c_client->adapter, msgs,
+ ARRAY_SIZE(msgs));
+ }
+ } else {
+ u8 reg = address_to_register(addr);
+
+ struct i2c_msg msgs[] = {
+ {
+ .addr = phy->i2c_client->addr,
+ .len = sizeof(reg),
+ .buf = ®,
+ },
+ {
+ .addr = phy->i2c_client->addr,
+ .len = len,
+ .buf = (u8 *)value,
+ .flags = I2C_M_NOSTART,
+ },
+ };
+
+ ret = i2c_transfer(phy->i2c_client->adapter, msgs,
+ ARRAY_SIZE(msgs));
+ }
+
+ if (ret < 0)
+ return ret;
+
+ usleep_range(250, 300); // wait default GUARD_TIME of 250µs
+
+ return 0;
+}
+
+static bool tpm_tis_i2c_check_data(struct tpm_tis_data *data,
+ const u8 *buf, size_t len)
+{
+ struct tpm_tis_i2c_phy *phy = to_tpm_tis_i2c_phy(data);
+ u16 crc, crc_tpm;
+ int rc;
+
+ if (phy->data_csum) {
+ crc = crc_ccitt(0x0000, buf, len);
+ rc = tpm_tis_read16(data, TPM_DATA_CSUM, &crc_tpm);
+ if (rc < 0)
+ return false;
+
+ crc_tpm = be16_to_cpu(crc_tpm);
+ return crc == crc_tpm;
+ }
+
+ return true;
+}
+
+static SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, tpm_tis_resume);
+
+static int csum_state_store(struct tpm_tis_data *data, u8 new_state)
+{
+ struct tpm_tis_i2c_phy *phy = to_tpm_tis_i2c_phy(data);
+ u8 cur_state;
+ int rc;
+
+ rc = tpm_tis_i2c_write_bytes(&phy->priv, TPM_DATA_CSUM_ENABLE,
+ 1, &new_state);
+ if (rc < 0)
+ return rc;
+
+ rc = tpm_tis_i2c_read_bytes(&phy->priv, TPM_DATA_CSUM_ENABLE,
+ 1, &cur_state);
+ if (rc < 0)
+ return rc;
+
+ if (new_state == cur_state)
+ phy->data_csum = (bool)new_state;
+
+ return rc;
+}
+
+static const struct tpm_tis_phy_ops tpm_i2c_phy_ops = {
+ .read_bytes = tpm_tis_i2c_read_bytes,
+ .write_bytes = tpm_tis_i2c_write_bytes,
+ .check_data = tpm_tis_i2c_check_data,
+};
+
+static int tpm_tis_i2c_probe(struct i2c_client *dev,
+ const struct i2c_device_id *id)
+{
+ struct tpm_tis_i2c_phy *phy;
+ int rc;
+ const u8 loc_init = 0;
+
+ phy = devm_kzalloc(&dev->dev, sizeof(struct tpm_tis_i2c_phy),
+ GFP_KERNEL);
+ if (!phy)
+ return -ENOMEM;
+
+ phy->i2c_client = dev;
+
+ if (!i2c_check_functionality(dev->adapter, I2C_FUNC_NOSTART)) {
+ phy->iobuf = devm_kmalloc(&dev->dev, TPM_BUFSIZE, GFP_KERNEL);
+ if (!phy->iobuf)
+ return -ENOMEM;
+ }
+
+ // select locality 0 (the driver will access only via locality 0)
+ rc = tpm_tis_i2c_write_bytes(&phy->priv, TPM_LOC_SEL, 1, &loc_init);
+ if (rc < 0)
+ return rc;
+
+ // enables the data checksum calculation
+ rc = csum_state_store(&phy->priv, 0x01);
+ if (rc < 0)
+ return rc;
+
+ return tpm_tis_core_init(&dev->dev, &phy->priv, -1, &tpm_i2c_phy_ops,
+ NULL);
+}
+
+static const struct i2c_device_id tpm_tis_i2c_id[] = {
+ {"tpm_tis_i2c", 0},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, tpm_tis_i2c_id);
+
+static const struct of_device_id of_tis_i2c_match[] = {
+ { .compatible = "tcg,tpm-tis-i2c", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, of_tis_i2c_match);
+
+static const struct acpi_device_id acpi_tis_i2c_match[] = {
+ {"SMO0768", 0},
+ {}
+};
+MODULE_DEVICE_TABLE(acpi, acpi_tis_i2c_match);
+
+static struct i2c_driver tpm_tis_i2c_driver = {
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = "tpm_tis_i2c",
+ .pm = &tpm_tis_pm,
+ .of_match_table = of_match_ptr(of_tis_i2c_match),
+ .acpi_match_table = ACPI_PTR(acpi_tis_i2c_match),
+ },
+ .probe = tpm_tis_i2c_probe,
+ .id_table = tpm_tis_i2c_id,
+};
+
+module_i2c_driver(tpm_tis_i2c_driver);
+
+MODULE_DESCRIPTION("TPM Driver for native I2C access");
+MODULE_LICENSE("GPL");
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread