Add tpm i2c ptp driver
Subsystems (3)
v17 cover Add tpm i2c ptp driver
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/
Changes since version 9:
- "tpm: Make read{16, 32}() and write32() in tpm_tis_phy_ops optional"
-Fixed short description
- "tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception" -Canceled wait_for_tpm_stat() function renaming.
-Fixed long description
- "tpm: Add YAML schema for TPM TIS I2C options"
-Added a reviewed-by tag.
Addressed comments from:
- Jarkko Sakkinen:
https://lore.kernel.org/patchwork/patch/1247163/ https://lore.kernel.org/patchwork/patch/1247164/ - Rob Herring: https://lore.kernel.org/patchwork/patch/1247161/
Changes since version 10:
- "tpm: Make read{16, 32}() and write32() in tpm_tis_phy_ops optional"
-Added a Reviewed-by and Tested-by tags
- "tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception" -Renamed "mask_result" parameter with "stat"
- "tpm: tpm_tis: Add retry in case of protocol failure or data integrity (on I2C only) failure." -Edited long description.
-Modified tpm_tis_recv() to __tpm_tis_recv() and Introduced a new tpm_tis_recv() function
Addressed comments from:
- Jarkko Sakkinen:
https://lore.kernel.org/patchwork/patch/1252428/ https://lore.kernel.org/patchwork/patch/1252422/ https://lore.kernel.org/patchwork/patch/1252424/
Changes since version 11:
- "tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception" -Added a "Reviewed-by" tag
-Renamed 'wait_for_tpm_stat()' function with 'tpm_tis_wait_for_stat()'
- "tpm: tpm_tis: Add retry in case of protocol failure."
-Removed data integrity check and created a new commit for it.
-Edited short and long description.
- "tpm: tpm_tis: Add verify_data_integrity handle to tpm_tis_phy_ops"
-This is a new commit.
Addressed comments from:
- Jarkko Sakkinen:
https://lore.kernel.org/patchwork/patch/1258107/ https://lore.kernel.org/patchwork/patch/1258110/
Changes since version 12:
- Moved "tpm: Add YAML schema for TPM TIS I2C options" to end of patch.
- Removed two commits to be resubmited on later patch:
- "tpm: tpm_tis: Add retry in case of protocol failure."
- "tpm: tpm_tis: Add verify_data_integrity handle to tpm_tis_phy_ops"
- "tpm: tpm_tis: add tpm_tis_i2c driver"
- Removed verify data integrity (Checksum) functuality from i2c driver.
- Edited Long Description.
- Updated header comment for tpm_tis_i2c.c
Addressed comments from:
- Jarkko Sakkinen:
https://lore.kernel.org/patchwork/patch/1263805/ https://lore.kernel.org/patchwork/patch/1263813/
Changes since version 13:
- Edited description of commits 1-6 by Jarkko comments.
- "tpm: Add YAML schema for TPM TIS I2C options"
- Fixed YAML compilation error of missing "additionalProperties" field
Addressed comments from:
- Jarkko Sakkinen:
https://lkml.org/lkml/2021/8/26/546 https://lkml.org/lkml/2021/8/26/548 https://lkml.org/lkml/2021/8/26/550 https://lkml.org/lkml/2021/8/26/551 https://lkml.org/lkml/2021/8/26/552 https://lkml.org/lkml/2021/8/26/553 https://lkml.org/lkml/2021/8/26/555 - Rob Herring: https://lkml.org/lkml/2021/8/26/427
Changes since version 14:
- "tpm: Make read{16, 32}() and write32() in tpm_tis_phy_ops optional"
-Removed commit.
- "tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception" -Fixed description and restored "wait_for_tom_stat" to its original name
- "tpm: tpm_tis: add tpm_tis_i2c driver"
-Added read{16, 32}() and write32() functions in i2c driver.
Addressed comments from:
- Jarkko Sakkinen:
https://lkml.org/lkml/2021/9/13/2235
https://lkml.org/lkml/2021/9/13/2241Changes since version 15:
- tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception -Fixed and added description
-Added a fixed tag
-Changed stat parameter name to result.
- tpm: Add YAML schema for TPM TIS I2C options
-Added Reviewd-by tag
Addressed comments from:
- Jarkko Sakkinen:
https://www.spinics.net/lists/linux-integrity/msg19686.html - Rob Herring: https://www.spinics.net/lists/linux-integrity/msg19686.html
Changes since version 16:
- tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception -removed modifing of handeling the last byte on send command for future addition. -fixed minor description issues and fixes tag format.
-tpm: tpm_tis: Rewrite "tpm_tis_req_canceled()"
-fixed fixes tag format.
Addressed comments from:
- Jarkko Sakkinen:
https://www.spinics.net/lists/kernel/msg4120640.html https://www.spinics.net/lists/kernel/msg4120641.html
Amir Mizinski (6):
tpm_tis: Fix expected bit handling and send all bytes in one shot
without last byte in exception
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: tpm_tis: Add tpm_tis_i2c driver
tpm: Add YAML schema for TPM TIS I2C options
.../bindings/security/tpm/tpm-tis-i2c.yaml | 52 ++++ drivers/char/tpm/Kconfig | 12 + drivers/char/tpm/Makefile | 1 + drivers/char/tpm/tpm2-cmd.c | 4 + drivers/char/tpm/tpm_tis_core.c | 73 +++--- drivers/char/tpm/tpm_tis_i2c.c | 268 +++++++++++++++++++++ include/linux/tpm.h | 1 + 7 files changed, 373 insertions(+), 38 deletions(-) create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml create mode 100644 drivers/char/tpm/tpm_tis_i2c.c
Signature · 1 line
2.7.4
PATCH v17 1/6 tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception
From: Amir Mizinski <[email protected]>
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, a check of only one of these fields is wrong. Fix this condition so that both fields are checked in the same TPM_STS register read.
Modify the signature of wait_for_tpm_stat(), adding an additional "result" parameter to its call.wait_for_tpm_stat() is now polling the TPM_STS with a mask and waits for the value in result. This modification adds the ability to check if certain TPM_STS bits have been cleared.
For example, use the new parameter to check in status that TPM_STS_VALID is set and also that TPM_STS_EXPECT is zeroed. This prevents a racy check.
--- drivers/char/tpm/tpm_tis_core.c | 59 ++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 33 deletions(-)
drivers/char/tpm/tpm_tis_core.c +26 -33
@@ -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 wait_for_tpm_stat (struct tpm_chip *chip,u8 mask,u8 result, +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) == result)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) == result)return 0 ; }while (time_before (jiffies, stop)); }
@@ -260,9 +260,10 @@ static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count)while (size < count) { rc =wait_for_tpm_stat (chip, -TPM_STS_DATA_AVAIL |TPM_STS_VALID , - chip->timeout_c, - &priv->read_queue, true); +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 (wait_for_tpm_stat (chip,TPM_STS_VALID ,TPM_STS_VALID , + chip->timeout_c, &priv->int_queue, + false) <0 ) { size = -ETIME ; goto out; }
@@ -342,14 +344,14 @@ 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 (chip,TPM_STS_COMMAND_READY , +TPM_STS_COMMAND_READY , + chip->timeout_b, &priv->int_queue, + false) <0 ) { rc = -ETIME ; goto out_err; }
@@ -369,34 +371,24 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len) goto out_err; count += burstcnt; - -if (wait_for_tpm_stat (chip,TPM_STS_VALID , chip->timeout_c, - &priv->int_queue, false) <0 ) { +if (wait_for_tpm_stat (chip,TPM_STS_VALID |TPM_STS_DATA_EXPECT , +TPM_STS_VALID |TPM_STS_DATA_EXPECT , 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; - } }/* 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 (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 +443,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 (wait_for_tpm_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; }
Signature · 1 line
2.7.4
Jarkko Sakkinen
maintainerRe: [PATCH v17 1/6] tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception
Quoted 18 lines · On Sun, 2021-10-24 at 19:48 +0300, [email protected] wrote:
From: Amir Mizinski <[email protected]>
Currently, the driver polls the TPM_STS.stsValid field until TRUE; then it reads
TPM_STSregister 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, a check of only one of these fields is wrong. Fix this condition so that both fields are checked in the sameTPM_STSregister read.Modify the signature of
wait_for_tpm_stat(), adding an additional "result" parameter to its call.wait_for_tpm_stat()is now polling theTPM_STSwith a mask and waits for the value in result. This modification adds the ability to check if certainTPM_STSbits have been cleared.
For example, use the new parameter to check in status thatTPM_STS_VALIDis set and also thatTPM_STS_EXPECTis zeroed. This prevents a racy check.
Quoted 1 line
Fixes 27084efee0c3 ("tpm: driver for next generation TPM chips")
Where does this failure occur in practice? If nowhere, this can be dropped, and the patch can be considered as a feature. Defining it as a fix makes only sense, if it needs to be backported to stable kernels. This requires something to be actually broken.
The commit message does not contain a real bug report. It is just referring to the specification, which is not a workload.
Quoted 14 lines
Suggested-by Benoit Houyere <[email protected]>Signed-off-by Amir Mizinski <[email protected]>---
drivers/char/tpm/tpm_tis_core.c | 59 ++++++++++++++++++-----------------------1 file changed, 26 insertions(+), 33 deletions(-)drivers/char/tpm/tpm_tis_core.c +0
-0@@ -44,9 +44,9 @@ static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask, return false; }
drivers/char/tpm/tpm_tis_core.c static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask, · line 49
Quoted 6 lines
drivers/char/tpm/tpm_tis_core.c +3
-3-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 (struct tpm_chip *chip,u8 mask,u8 result, + unsigned long timeout, + wait_queue_head_t *queue, bool check_cancel)
I would consider renaming this as, given that you are changing the signature anyway:
tpm_tis_wait_for_stat()
This would be more consistent with the other naming, and make e.g. grepping kernel tree easier.
How did you end up to the name "result"? I have hard time deriving from that name the actual semantics. E.g. "expected" would already a way more sane name.
/Jarkko
PATCH v17 2/6 tpm: tpm_tis: Rewrite "tpm_tis_req_canceled()"
In reply to [email protected]From: Amir Mizinski <[email protected]>
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.
--- drivers/char/tpm/tpm_tis_core.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/char/tpm/tpm_tis_core.c +2 -4
@@ -661,13 +661,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 ); } }
Signature · 1 line
2.7.4
Jarkko Sakkinen
maintainerRe: [PATCH v17 2/6] tpm: tpm_tis: Rewrite "tpm_tis_req_canceled()"
Quoted 10 lines · On Sun, 2021-10-24 at 19:48 +0300, [email protected] wrote:
From: Amir Mizinski <[email protected]>
tpm_tis_req_canceled()function is used to check if the caller requested to abort the current operation. It was found that in some casestpm_tis_req_canceled()wrongly returned true.
Since a cancel request sets the TPM_STS.commandReady field to TRUE, thetpm_tis_req_canceled()function should check only the TPM_STS.commandReady field value.
The case forTPM_VID_WINBONDis wrong and was therefore removed.
Quoted 5 lines
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 <[email protected]>
Also here, I'm not sure about the fixes tag (same reasoning as for 1/6).
/Jarkko
PATCH v17 3/6 tpm: Handle an exception for TPM Firmware Update mode.
In reply to [email protected]From: Amir Mizinski <[email protected]>
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.
--- drivers/char/tpm/tpm2-cmd.c | 4 ++++ include/linux/tpm.h | 1 + 2 files changed, 5 insertions(+)
drivers/char/tpm/tpm2-cmd.c +4 -0
@@ -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;
include/linux/tpm.h +1 -0
@@ -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,
Signature · 1 line
2.7.4
PATCH v17 4/6 tpm: tpm_tis: Verify TPM_STS register is valid after locality request
In reply to [email protected]From: Amir Mizinski <[email protected]>
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.
--- drivers/char/tpm/tpm_tis_core.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/char/tpm/tpm_tis_core.c +7 -1
@@ -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 (wait_for_tpm_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)); }
Signature · 1 line
2.7.4
Jarkko Sakkinen
maintainerRe: [PATCH v17 4/6] tpm: tpm_tis: Verify TPM_STS register is valid after locality request
Quoted 3 lines · On Sun, 2021-10-24 at 19:48 +0300, [email protected] wrote:
From: Amir Mizinski <[email protected]>
This issue could occur when the TPM does not update
TPM_STSregister after
What is "this issue"?
/Jarkko
PATCH v17 5/6 tpm: tpm_tis: Add tpm_tis_i2c driver
In reply to [email protected]From: Amir Mizinski <[email protected]>
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/
--- drivers/char/tpm/Kconfig | 12 ++ drivers/char/tpm/Makefile | 1 + drivers/char/tpm/tpm_tis_i2c.c | 268 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 281 insertions(+) create mode 100644 drivers/char/tpm/tpm_tis_i2c.c
drivers/char/tpm/Kconfig +12 -0
@@ -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
drivers/char/tpm/Makefile +1 -0
@@ -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
drivers/char/tpm/tpm_tis_i2c.c new file +268 -0
268 lines not on this page · raw message
Signature · 1 line
2.7.4
Re: [PATCH v17 5/6] tpm: tpm_tis: Add tpm_tis_i2c driver
Quoted 8 lines · On 10/24/21 9:48 AM, [email protected] wrote:
drivers/char/tpm/Kconfig +0
-0@@ -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.
drivers/char/tpm/Kconfig config TCG_TIS_SYNQUACER · line 98
Quoted 10 lines
drivers/char/tpm/Kconfig +10
-0+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;
choose M here;
[drop one space]
Quoted 1 line
drivers/char/tpm/Kconfig +1
-0+ the module will be called tpm_tis_i2c.
Signature · 1 line
~Randy
PATCH v17 6/6 tpm: Add YAML schema for TPM TIS I2C options
In reply to [email protected]From: Amir Mizinski <[email protected]>
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.
--- .../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
Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml new file +52 -0
@@ -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 <[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 + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + tpm@2e { + compatible = "nuvoton,npct75x", "tcg,tpm-tis-i2c"; + reg = <0x2e>; + crc-checksum; + }; + }; +...
Signature · 1 line
2.7.4