diff mbox series

[v2] tpm: Add flag to use default cancellation policy

Message ID	20221107171423.51019-1-eajames@linux.ibm.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Eddie James <eajames@linux.ibm.com> To: linux-integrity@vger.kernel.org Cc: linux-kernel@vger.kernel.org, peterhuewe@gmx.de, jarkko@kernel.org, jgg@ziepe.ca, joel@jms.id.au, Eddie James <eajames@linux.ibm.com> Subject: [PATCH v2] tpm: Add flag to use default cancellation policy Date: Mon, 7 Nov 2022 11:14:23 -0600 Message-Id: <20221107171423.51019-1-eajames@linux.ibm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	[v2] tpm: Add flag to use default cancellation policy \| [v2] tpm: Add flag to use default cancellation policy

Commit Message

Eddie James Nov. 7, 2022, 5:14 p.m. UTC

  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 <eajames@linux.ibm.com>
---
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(-)

Comments

Joel Stanley Nov. 14, 2022, 3:17 a.m. UTC | #1

On Mon, 7 Nov 2022 at 17:14, Eddie James <eajames@linux.ibm.com> wrote:
>
> 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 <eajames@linux.ibm.com>

Tested-by: Joel Stanley <joel@jms.id.au>
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.


> ---
> 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(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 757623bacfd5..3f98e587b3e8 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -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)
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index 66a5a13cd1df..b68479e0de10 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -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 {
> diff --git a/drivers/char/tpm/tpm_tis_i2c.c b/drivers/char/tpm/tpm_tis_i2c.c
> index 45f388127f4b..91451ee1ef8d 100644
> --- a/drivers/char/tpm/tpm_tis_i2c.c
> +++ b/drivers/char/tpm/tpm_tis_i2c.c
> @@ -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 Nov. 27, 2022, 3:47 p.m. UTC | #2

On Mon, Nov 07, 2022 at 11:14:23AM -0600, Eddie James wrote:
> 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 <eajames@linux.ibm.com>
> ---
> 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(-)
> 
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 757623bacfd5..3f98e587b3e8 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -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)
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index 66a5a13cd1df..b68479e0de10 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -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 {
> diff --git a/drivers/char/tpm/tpm_tis_i2c.c b/drivers/char/tpm/tpm_tis_i2c.c
> index 45f388127f4b..91451ee1ef8d 100644
> --- a/drivers/char/tpm/tpm_tis_i2c.c
> +++ b/drivers/char/tpm/tpm_tis_i2c.c
> @@ -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
> 

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 757623bacfd5..3f98e587b3e8 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -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)
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 66a5a13cd1df..b68479e0de10 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -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 {
diff --git a/drivers/char/tpm/tpm_tis_i2c.c b/drivers/char/tpm/tpm_tis_i2c.c
index 45f388127f4b..91451ee1ef8d 100644
--- a/drivers/char/tpm/tpm_tis_i2c.c
+++ b/drivers/char/tpm/tpm_tis_i2c.c
@@ -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 */