tpm: Add flag to use default cancellation policy

PATCH In review Eddie James 3 participants 3 messages Nov 7, 2022 – Nov 27, 2022

Subsystems (2)

Keys-trusted TPM device driver

lore

Share & tools

lore

Eddie James

author

2022-11-07 17:14 UTC #0 lore raw

PATCH v2 1/1 tpm: Add flag to use default cancellation policy

The check for cancelled request depends on the VID of the chip, but some chips share VID which shouldn't share their cancellation behavior. This is the case for the Nuvoton NPCT75X, which should use the default cancellation check, not the Winbond one.
To avoid changing the existing behavior, add a new flag to indicate that the chip should use the default cancellation check and set it for the I2C TPM2 TIS driver.

Signed-off-by Eddie James <[email protected]>

---
Changes since v1:
- Update switch statement with default: break;

 drivers/char/tpm/tpm_tis_core.c | 20 ++++++++++++--------
 drivers/char/tpm/tpm_tis_core.h |  1 +
 drivers/char/tpm/tpm_tis_i2c.c  |  1 +
 3 files changed, 14 insertions(+), 8 deletions(-)

drivers/char/tpm/tpm_tis_core.c +12 -8

index 757623bacfd5..3f98e587b3e8 100644
@@ -682,15 +682,19 @@ 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);
+	if (!test_bit(TPM_TIS_DEFAULT_CANCELLATION, &priv->flags)) {
+		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:
+			break;
+		}
 	}
+
+	return status == TPM_STS_COMMAND_READY;
 }
 
 static irqreturn_t tis_int_handler(int dummy, void *dev_id)

drivers/char/tpm/tpm_tis_core.h +1 -0

index 66a5a13cd1df..b68479e0de10 100644
@@ -86,6 +86,7 @@ enum tis_defaults {
 enum tpm_tis_flags {
 	TPM_TIS_ITPM_WORKAROUND		= BIT(0),
 	TPM_TIS_INVALID_STATUS		= BIT(1),
+	TPM_TIS_DEFAULT_CANCELLATION	= BIT(2),
 };
 
 struct tpm_tis_data {

drivers/char/tpm/tpm_tis_i2c.c +1 -0

index 45f388127f4b..91451ee1ef8d 100644
@@ -329,6 +329,7 @@ static int tpm_tis_i2c_probe(struct i2c_client *dev,
 	if (!phy->io_buf)
 		return -ENOMEM;
 
+	set_bit(TPM_TIS_DEFAULT_CANCELLATION, &phy->priv.flags);
 	phy->i2c_client = dev;
 
 	/* must precede all communication with the tpm */

Signature · 1 line

2.31.1

Joel Stanley

Tested-by on v2 1/1

2022-11-14 03:18 UTC #1 lore raw

Quoted 10 lines · Eddie James

The check for cancelled request depends on the VID of the chip, but some chips share VID which shouldn't share their cancellation behavior. This is the case for the Nuvoton NPCT75X, which should use the default cancellation check, not the Winbond one.
To avoid changing the existing behavior, add a new flag to indicate that the chip should use the default cancellation check and set it for the I2C TPM2 TIS driver.
Signed-off-by Eddie James <[email protected]>

Tested-by Joel Stanley <[email protected]>

Fixes bbc23a07b072 ("tpm: Add tpm_tis_i2c backend for tpm_tis_core")

With this patch, and "tpm: tis_i2c: Fix sanity check interrupt enable mask", and the extra compatible strings the driver correctly probes on boot with the Nuvoton part.

Quoted 68 lines

---
Changes since v1:
- Update switch statement with default: break;

 drivers/char/tpm/tpm_tis_core.c | 20 ++++++++++++--------
 drivers/char/tpm/tpm_tis_core.h |  1 +
 drivers/char/tpm/tpm_tis_i2c.c  |  1 +
 3 files changed, 14 insertions(+), 8 deletions(-)

drivers/char/tpm/tpm_tis_core.c +12 -8

index 757623bacfd5..3f98e587b3e8 100644
@@ -682,15 +682,19 @@ 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);
+       if (!test_bit(TPM_TIS_DEFAULT_CANCELLATION, &priv->flags)) {
+               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:
+                       break;
+               }
        }
+
+       return status == TPM_STS_COMMAND_READY;
 }
 
 static irqreturn_t tis_int_handler(int dummy, void *dev_id)

drivers/char/tpm/tpm_tis_core.h +1 -0

index 66a5a13cd1df..b68479e0de10 100644
@@ -86,6 +86,7 @@ enum tis_defaults {
 enum tpm_tis_flags {
        TPM_TIS_ITPM_WORKAROUND         = BIT(0),
        TPM_TIS_INVALID_STATUS          = BIT(1),
+       TPM_TIS_DEFAULT_CANCELLATION    = BIT(2),
 };
 
 struct tpm_tis_data {

drivers/char/tpm/tpm_tis_i2c.c +1 -1

index 45f388127f4b..91451ee1ef8d 100644
@@ -329,6 +329,7 @@ static int tpm_tis_i2c_probe(struct i2c_client *dev,
        if (!phy->io_buf)
                return -ENOMEM;
 
+       set_bit(TPM_TIS_DEFAULT_CANCELLATION, &phy->priv.flags);
        phy->i2c_client = dev;
 
        /* must precede all communication with the tpm */
--

2.31.1

Jarkko Sakkinen

maintainer Reviewed-by on v2 1/1

2022-11-27 15:47 UTC #2 lore raw

In reply to Eddie James

Quoted 77 lines · Eddie James

Signed-off-by Eddie James <[email protected]>

---
Changes since v1:
- Update switch statement with default: break;

 drivers/char/tpm/tpm_tis_core.c | 20 ++++++++++++--------
 drivers/char/tpm/tpm_tis_core.h |  1 +
 drivers/char/tpm/tpm_tis_i2c.c  |  1 +
 3 files changed, 14 insertions(+), 8 deletions(-)

drivers/char/tpm/tpm_tis_core.c +12 -8

index 757623bacfd5..3f98e587b3e8 100644
@@ -682,15 +682,19 @@ 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);
+	if (!test_bit(TPM_TIS_DEFAULT_CANCELLATION, &priv->flags)) {
+		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:
+			break;
+		}
 	}
+
+	return status == TPM_STS_COMMAND_READY;
 }
 
 static irqreturn_t tis_int_handler(int dummy, void *dev_id)

drivers/char/tpm/tpm_tis_core.h +1 -0

index 66a5a13cd1df..b68479e0de10 100644
@@ -86,6 +86,7 @@ enum tis_defaults {
 enum tpm_tis_flags {
 	TPM_TIS_ITPM_WORKAROUND		= BIT(0),
 	TPM_TIS_INVALID_STATUS		= BIT(1),
+	TPM_TIS_DEFAULT_CANCELLATION	= BIT(2),
 };
 
 struct tpm_tis_data {

drivers/char/tpm/tpm_tis_i2c.c +1 -0

index 45f388127f4b..91451ee1ef8d 100644
@@ -329,6 +329,7 @@ static int tpm_tis_i2c_probe(struct i2c_client *dev,
 	if (!phy->io_buf)
 		return -ENOMEM;
 
+	set_bit(TPM_TIS_DEFAULT_CANCELLATION, &phy->priv.flags);
 	phy->i2c_client = dev;
 
 	/* must precede all communication with the tpm */

Signature · 1 line

2.31.1

Reviewed-by Jarkko Sakkinen <[email protected]>

BR, Jarkko