* [PATCH v14 1/7] tpm: Make read{16, 32}() and write32() in tpm_tis_phy_ops optional
2021-09-13 14:43 [PATCH v14 0/7] Add tpm i2c ptp driver amirmizi6
@ 2021-09-13 14:43 ` amirmizi6
2021-09-13 21:06 ` Jarkko Sakkinen
2021-09-13 14:43 ` [PATCH v14 2/7] tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception amirmizi6
` (5 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: amirmizi6 @ 2021-09-13 14:43 UTC (permalink / raw)
To: Eyal.Cohen, jarkko, oshrialkoby85, alexander.steffen, robh+dt,
mark.rutland, peterhuewe, jgg, arnd, gregkh, benoit.houyere,
eajames, joel
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski, Amir Mizinski, Alexander Steffen
From: Amir Mizinski <amirmizi6@gmail.com>
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.
Some drivers, such as tpm_tis_spi, require more complicated operations to
read more than one byte at a time and, as a result, will revert to
read_bytes/write_bytes.
Therefore, re-implement tpm_tis_{read, write}_{16, 32}, so that they check
if implementations for {read, write}_{16, 32} in tpm_tis_phys_ops exist;
if they do not exist, then revert to {read, write}_bytes().
Co-developed-by: Alexander Steffen <Alexander.Steffen@infineon.com>
Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
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 b2a3c6c..e8fad0e 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -124,13 +124,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,
@@ -147,7 +169,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 ea759af..9af6f84 100644
--- a/drivers/char/tpm/tpm_tis_spi_cr50.c
+++ b/drivers/char/tpm/tpm_tis_spi_cr50.c
@@ -220,9 +220,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 54584b4..942900c 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v14 1/7] tpm: Make read{16, 32}() and write32() in tpm_tis_phy_ops optional
2021-09-13 14:43 ` [PATCH v14 1/7] tpm: Make read{16, 32}() and write32() in tpm_tis_phy_ops optional amirmizi6
@ 2021-09-13 21:06 ` Jarkko Sakkinen
0 siblings, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2021-09-13 21:06 UTC (permalink / raw)
To: amirmizi6, Eyal.Cohen, oshrialkoby85, alexander.steffen, robh+dt,
mark.rutland, peterhuewe, jgg, arnd, gregkh, benoit.houyere,
eajames, joel
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski
On Mon, 2021-09-13 at 17:43 +0300, amirmizi6@gmail.com wrote:
> From: Amir Mizinski <amirmizi6@gmail.com>
>
> 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.
> Some drivers, such as tpm_tis_spi, require more complicated operations to
> read more than one byte at a time and, as a result, will revert to
> read_bytes/write_bytes.
> Therefore, re-implement tpm_tis_{read, write}_{16, 32}, so that they check
> if implementations for {read, write}_{16, 32} in tpm_tis_phys_ops exist;
> if they do not exist, then revert to {read, write}_bytes().
> static int tpm_tis_spi_probe(struct spi_device *dev)
This is otherwise good but lacks explanation why this makes sense, other than
somewhat obvious benefit of having less calbacks, which BTW should be stated
explicitly in the commit message too.
You must answer what is your by doing this change, e.g. what you are unable
to do, if the callbacks are removed (just an example).
I get the change, but motivation to do it needs to be there.
/Jarkko
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v14 2/7] tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception
2021-09-13 14:43 [PATCH v14 0/7] Add tpm i2c ptp driver amirmizi6
2021-09-13 14:43 ` [PATCH v14 1/7] tpm: Make read{16, 32}() and write32() in tpm_tis_phy_ops optional amirmizi6
@ 2021-09-13 14:43 ` amirmizi6
2021-09-13 21:10 ` Jarkko Sakkinen
2021-09-13 14:43 ` [PATCH v14 3/7] tpm: tpm_tis: Rewrite "tpm_tis_req_canceled()" amirmizi6
` (4 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: amirmizi6 @ 2021-09-13 14:43 UTC (permalink / raw)
To: Eyal.Cohen, jarkko, oshrialkoby85, alexander.steffen, robh+dt,
mark.rutland, peterhuewe, jgg, arnd, gregkh, benoit.houyere,
eajames, joel
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski, Amir Mizinski
From: Amir Mizinski <amirmizi6@gmail.com>
Detected an incorrect implementation of the send command.
Currently, the driver polls the TPM_STS.stsValid field until TRUE; then it
reads TPM_STS register again to verify only that TPM_STS.expect field is
FALSE (i.e., it ignores TPM_STS.stsValid).
Since TPM_STS.stsValid represents the TPM_STS.expect validity, both fields
fields should be checked in the same TPM_STS register read value.
This fix modifies the signature of 'wait_for_tpm_stat()', adding an
additional "mask_result" parameter to its call and renaming it to
'tpm_tis_wait_for_stat()' for better alignment with other naming.
'tpm_tis_wait_for_stat()' is now polling the TPM_STS with a mask and waits
for the value in mask_result. The fix adds the ability to check if certain
TPM_STS bits have been cleared.
This change is also aligned to verifying the CRC on I2C TPM. The CRC
verification should be done after the TPM_STS.expect field is cleared
(TPM received all expected command bytes and set the calculated CRC value
in the register).
In addition, the send command was changed to comply with
TCG_DesignPrinciples_TPM2p0Driver_vp24_pubrev.pdf as follows:
- send all command bytes in one loop
- remove special handling of the last byte
Suggested-by: Benoit Houyere <benoit.houyere@st.com>
Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
drivers/char/tpm/tpm_tis_core.c | 70 +++++++++++++++--------------------------
1 file changed, 26 insertions(+), 44 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 69579ef..b34e59b 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -44,9 +44,9 @@ 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 tpm_tis_wait_for_stat(struct tpm_chip *chip, u8 mask, u8 stat,
+ unsigned long timeout,
+ wait_queue_head_t *queue, bool check_cancel)
{
unsigned long stop;
long rc;
@@ -55,7 +55,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) == stat)
return 0;
stop = jiffies + timeout;
@@ -83,7 +83,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) == stat)
return 0;
} while (time_before(jiffies, stop));
}
@@ -259,10 +259,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 = tpm_tis_wait_for_stat(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);
@@ -315,8 +316,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 (tpm_tis_wait_for_stat(chip, TPM_STS_VALID, TPM_STS_VALID,
+ chip->timeout_c, &priv->int_queue,
+ false) < 0) {
size = -ETIME;
goto out;
}
@@ -342,61 +344,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 (tpm_tis_wait_for_stat(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 (tpm_tis_wait_for_stat(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;
@@ -451,9 +432,10 @@ 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 (tpm_tis_wait_for_stat(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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v14 2/7] tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception
2021-09-13 14:43 ` [PATCH v14 2/7] tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception amirmizi6
@ 2021-09-13 21:10 ` Jarkko Sakkinen
0 siblings, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2021-09-13 21:10 UTC (permalink / raw)
To: amirmizi6, Eyal.Cohen, oshrialkoby85, alexander.steffen, robh+dt,
mark.rutland, peterhuewe, jgg, arnd, gregkh, benoit.houyere,
eajames, joel
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski
On Mon, 2021-09-13 at 17:43 +0300, amirmizi6@gmail.com wrote:
> From: Amir Mizinski <amirmizi6@gmail.com>
>
> Detected an incorrect implementation of the send command.
> Currently, the driver polls the TPM_STS.stsValid field until TRUE; then it
> reads TPM_STS register again to verify only that TPM_STS.expect field is
> FALSE (i.e., it ignores TPM_STS.stsValid).
> Since TPM_STS.stsValid represents the TPM_STS.expect validity, both fields
> fields should be checked in the same TPM_STS register read value.
>
> This fix modifies the signature of 'wait_for_tpm_stat()', adding an
> additional "mask_result" parameter to its call and renaming it to
> 'tpm_tis_wait_for_stat()' for better alignment with other naming.
> 'tpm_tis_wait_for_stat()' is now polling the TPM_STS with a mask and waits
> for the value in mask_result. The fix adds the ability to check if certain
> TPM_STS bits have been cleared.
Use imprative form, e.g. "Modify the signature...".
>
> This change is also aligned to verifying the CRC on I2C TPM. The CRC
> verification should be done after the TPM_STS.expect field is cleared
> (TPM received all expected command bytes and set the calculated CRC value
> in the register).
What does it mean when you "align to verifying"?
> In addition, the send command was changed to comply with
> TCG_DesignPrinciples_TPM2p0Driver_vp24_pubrev.pdf as follows:
> - send all command bytes in one loop
> - remove special handling of the last byte
>
> Suggested-by: Benoit Houyere <benoit.houyere@st.com>
> Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
I don't think the rename is important enough to be done, and it
definitely should not be melded into another patch.
/Jarkko
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v14 3/7] tpm: tpm_tis: Rewrite "tpm_tis_req_canceled()"
2021-09-13 14:43 [PATCH v14 0/7] Add tpm i2c ptp driver amirmizi6
2021-09-13 14:43 ` [PATCH v14 1/7] tpm: Make read{16, 32}() and write32() in tpm_tis_phy_ops optional amirmizi6
2021-09-13 14:43 ` [PATCH v14 2/7] tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception amirmizi6
@ 2021-09-13 14:43 ` amirmizi6
2021-09-13 14:43 ` [PATCH v14 4/7] tpm: Handle an exception for TPM Firmware Update mode amirmizi6
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: amirmizi6 @ 2021-09-13 14:43 UTC (permalink / raw)
To: Eyal.Cohen, jarkko, oshrialkoby85, alexander.steffen, robh+dt,
mark.rutland, peterhuewe, jgg, arnd, gregkh, benoit.houyere,
eajames, joel
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski, Amir Mizinski
From: Amir Mizinski <amirmizi6@gmail.com>
tpm_tis_req_canceled() function is used to check if the caller requested
to abort the current operation. It was found that in some cases
tpm_tis_req_canceled() wrongly returned true.
Since a cancel request sets the TPM_STS.commandReady field to TRUE, the
tpm_tis_req_canceled() function should check only the TPM_STS.commandReady
field value.
The case for TPM_VID_WINBOND is wrong and was therefore removed.
Also, the default comparison is wrong. Only cmdReady bit needs to be
compared instead of the full lower status register byte.
Fixes: 1f866057291f (tpm: Fix cancellation of TPM commands (polling mode))
Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
drivers/char/tpm/tpm_tis_core.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index b34e59b..4145758 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -650,13 +650,11 @@ 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);
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v14 4/7] tpm: Handle an exception for TPM Firmware Update mode.
2021-09-13 14:43 [PATCH v14 0/7] Add tpm i2c ptp driver amirmizi6
` (2 preceding siblings ...)
2021-09-13 14:43 ` [PATCH v14 3/7] tpm: tpm_tis: Rewrite "tpm_tis_req_canceled()" amirmizi6
@ 2021-09-13 14:43 ` amirmizi6
2021-09-13 14:43 ` [PATCH v14 5/7] tpm: tpm_tis: verify TPM_STS register is valid after locality request amirmizi6
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: amirmizi6 @ 2021-09-13 14:43 UTC (permalink / raw)
To: Eyal.Cohen, jarkko, oshrialkoby85, alexander.steffen, robh+dt,
mark.rutland, peterhuewe, jgg, arnd, gregkh, benoit.houyere,
eajames, joel
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski, Amir Mizinski
From: Amir Mizinski <amirmizi6@gmail.com>
Add a condition to enable communication with the TPM while the TPM is in
firmware update mode.
In such a case if power was cut during the TPM firmware update, the driver
should ignore the "selftest" command return code (TPM2_RC_UPGRADE or
TPM2_RC_COMMAND_CODE) and skip the rest of the TPM initialization
sequence.
Suggested-by: Benoit Houyere <benoit.houyere@st.com>
Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
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 a25815a..c2b541d 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -729,6 +729,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 aa11fe3..c5bf934 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -207,6 +207,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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v14 5/7] tpm: tpm_tis: verify TPM_STS register is valid after locality request
2021-09-13 14:43 [PATCH v14 0/7] Add tpm i2c ptp driver amirmizi6
` (3 preceding siblings ...)
2021-09-13 14:43 ` [PATCH v14 4/7] tpm: Handle an exception for TPM Firmware Update mode amirmizi6
@ 2021-09-13 14:43 ` amirmizi6
2021-09-13 14:43 ` [PATCH v14 6/7] tpm: tpm_tis: add tpm_tis_i2c driver amirmizi6
2021-09-13 14:43 ` [PATCH v14 7/7] \tpm: Add YAML schema for TPM TIS I2C options amirmizi6
6 siblings, 0 replies; 10+ messages in thread
From: amirmizi6 @ 2021-09-13 14:43 UTC (permalink / raw)
To: Eyal.Cohen, jarkko, oshrialkoby85, alexander.steffen, robh+dt,
mark.rutland, peterhuewe, jgg, arnd, gregkh, benoit.houyere,
eajames, joel
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski, Amir Mizinski
From: Amir Mizinski <amirmizi6@gmail.com>
This issue could occur 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 in the tpm_tis_status(chip) function call.
When a call to tpm_tis_send_data() function is made after a
request_locality() call, the condition
("if ((status & TPM_STS_COMMAND_READY) == 0)") is checked. At this moment
if the status value is 0xFF, then it is 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.
Waiting for the condition TPM_STS.tpmGo == 0, will ensure that the TPM
status register has the correct value.
Suggested-by: Benoit Houyere <benoit.houyere@st.com>
Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
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 4145758..d527c43 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -177,8 +177,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 (tpm_tis_wait_for_stat(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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v14 6/7] tpm: tpm_tis: add tpm_tis_i2c driver
2021-09-13 14:43 [PATCH v14 0/7] Add tpm i2c ptp driver amirmizi6
` (4 preceding siblings ...)
2021-09-13 14:43 ` [PATCH v14 5/7] tpm: tpm_tis: verify TPM_STS register is valid after locality request amirmizi6
@ 2021-09-13 14:43 ` amirmizi6
2021-09-13 14:43 ` [PATCH v14 7/7] \tpm: Add YAML schema for TPM TIS I2C options amirmizi6
6 siblings, 0 replies; 10+ messages in thread
From: amirmizi6 @ 2021-09-13 14:43 UTC (permalink / raw)
To: Eyal.Cohen, jarkko, oshrialkoby85, alexander.steffen, robh+dt,
mark.rutland, peterhuewe, jgg, arnd, gregkh, benoit.houyere,
eajames, joel
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski, Amir Mizinski
From: Amir Mizinski <amirmizi6@gmail.com>
I2C support for TPM devices on the TIS interface is currently available
only as a proprietary driver implementation for each TPM vendor.
This patch implements the infrastructure for a TCG-compliant TPM TIS I2C
driver with the functionality required to communicate with a TPM device
over I2C according to the "TCG PC Client PTP Interface Specification"
and "Device Driver Design Principles for TPM 2.0"
References:
[1] "TCG PC Client Platform TPM Profile(PTP) Interface Specification for
TPM 2.0" Version 01.03 Revision 22 at:
https://trustedcomputinggroup.org/resource/pc-client-platform-tpm-profile-ptp-specification/
[2] "TCG PC Client Device Driver Design Principles for TPM 2.0"
Version 1.0 Revision 27 at:
https://trustedcomputinggroup.org/resource/tcg-pc-client-device-driver-design-principles-for-tpm-2-0/
Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
Tested-by: Eddie James <eajames@linux.ibm.com>
Tested-by: Joel Stanley <joel@jms.id.au>
---
drivers/char/tpm/Kconfig | 12 +++
drivers/char/tpm/Makefile | 1 +
drivers/char/tpm/tpm_tis_i2c.c | 227 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 240 insertions(+)
create mode 100644 drivers/char/tpm/tpm_tis_i2c.c
diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 4308f9c..1395968 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -86,6 +86,18 @@ config TCG_TIS_SYNQUACER
To compile this driver as a module, choose M here;
the module will be called tpm_tis_synquacer.
+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, compliant with the TCG TPM PTP
+ (I2C interface) specification and connected to an I2C bus master,
+ 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_CR50
tristate "TPM Interface Specification 2.0 Interface (I2C - CR50)"
depends on I2C
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 66d39ea..0222b1d 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -29,6 +29,7 @@ tpm_tis_spi-$(CONFIG_TCG_TIS_SPI_CR50) += tpm_tis_spi_cr50.o
obj-$(CONFIG_TCG_TIS_I2C_CR50) += tpm_tis_i2c_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..b57c485
--- /dev/null
+++ b/drivers/char/tpm/tpm_tis_i2c.c
@@ -0,0 +1,227 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2014-2021 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
+ * version 01.05 r14" and "TCG PC Client Device Driver Design Principles
+ * version 1.0 r27" for TPM 2.0 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/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_I2C_DID_VID 0x48
+#define TPM_I2C_RID 0x4C
+
+struct tpm_tis_i2c_phy {
+ struct tpm_tis_data priv;
+ struct i2c_client *i2c_client;
+ 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 SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, tpm_tis_resume);
+
+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,
+};
+
+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;
+
+ 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 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),
+ },
+ .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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v14 7/7] \tpm: Add YAML schema for TPM TIS I2C options
2021-09-13 14:43 [PATCH v14 0/7] Add tpm i2c ptp driver amirmizi6
` (5 preceding siblings ...)
2021-09-13 14:43 ` [PATCH v14 6/7] tpm: tpm_tis: add tpm_tis_i2c driver amirmizi6
@ 2021-09-13 14:43 ` amirmizi6
6 siblings, 0 replies; 10+ messages in thread
From: amirmizi6 @ 2021-09-13 14:43 UTC (permalink / raw)
To: Eyal.Cohen, jarkko, oshrialkoby85, alexander.steffen, robh+dt,
mark.rutland, peterhuewe, jgg, arnd, gregkh, benoit.houyere,
eajames, joel
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski, Amir Mizinski
From: Amir Mizinski <amirmizi6@gmail.com>
Add 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 <amirmizi6@gmail.com>
---
.../bindings/security/tpm/tpm-tis-i2c.yaml | 52 ++++++++++++++++++++++
1 file changed, 52 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..217ba8e
--- /dev/null
+++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
@@ -0,0 +1,52 @@
+# 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 <amirmizi6@gmail.com>
+
+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
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ tpm@2e {
+ compatible = "nuvoton,npct75x", "tcg,tpm-tis-i2c";
+ reg = <0x2e>;
+ crc-checksum;
+ };
+ };
+...
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread