diff mbox series

[v11,13/14] tpm, tpm_tis: startup chip before testing for interrupts

Message ID 20221124135538.31020-14-LinoSanfilippo@gmx.de
State New
Headers
Series TPM IRQ fixes |

Commit Message

Lino Sanfilippo Nov. 24, 2022, 1:55 p.m. UTC 
  From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

In tpm_tis_gen_interrupt() a request for a property value is sent to the
TPM to test if interrupts are generated. However after a power cycle the
TPM responds with TPM_RC_INITIALIZE which indicates that the TPM is not
yet properly initialized.
Fix this by first starting the TPM up before the request is sent. For this
the startup implementation is removed from tpm_chip_register() and put
into the new function tpm_chip_startup() which is called before the
interrupts are tested.

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
---
 drivers/char/tpm/tpm-chip.c     | 38 +++++++++++++++++++++------------
 drivers/char/tpm/tpm.h          |  1 +
 drivers/char/tpm/tpm_tis_core.c |  5 +++++
 3 files changed, 30 insertions(+), 14 deletions(-)

Comments

Lino Sanfilippo Nov. 28, 2022, 12:53 p.m. UTC | #1 
On 24.11.22 14:55, Lino Sanfilippo wrote:
> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
>
> In tpm_tis_gen_interrupt() a request for a property value is sent to the
> TPM to test if interrupts are generated. However after a power cycle the
> TPM responds with TPM_RC_INITIALIZE which indicates that the TPM is not
> yet properly initialized.
> Fix this by first starting the TPM up before the request is sent. For this
> the startup implementation is removed from tpm_chip_register() and put
> into the new function tpm_chip_startup() which is called before the
> interrupts are tested.
>
> Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
> ---
>  drivers/char/tpm/tpm-chip.c     | 38 +++++++++++++++++++++------------
>  drivers/char/tpm/tpm.h          |  1 +
>  drivers/char/tpm/tpm_tis_core.c |  5 +++++
>  3 files changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 783d65fc71f0..370aa1f529f2 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -543,6 +543,30 @@ static int tpm_get_pcr_allocation(struct tpm_chip *chip)
>  	return rc;
>  }
>
> +/*
> + * tpm_chip_startup() - performs auto startup and allocates the PCRs
> + * @chip: TPM chip to use.
> + */
> +int tpm_chip_startup(struct tpm_chip *chip)
> +{
> +	int rc;
> +
> +	rc = tpm_chip_start(chip);
> +	if (rc)
> +		return rc;
> +
> +	rc = tpm_auto_startup(chip);
> +	if (rc)
> +		goto stop;
> +
> +	rc = tpm_get_pcr_allocation(chip);
> +stop:
> +	tpm_chip_stop(chip);
> +
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(tpm_chip_startup);
> +
>  /*
>   * tpm_chip_register() - create a character device for the TPM chip
>   * @chip: TPM chip to use.
> @@ -558,20 +582,6 @@ int tpm_chip_register(struct tpm_chip *chip)
>  {
>  	int rc;
>
> -	rc = tpm_chip_start(chip);
> -	if (rc)
> -		return rc;
> -	rc = tpm_auto_startup(chip);
> -	if (rc) {
> -		tpm_chip_stop(chip);
> -		return rc;
> -	}
> -
> -	rc = tpm_get_pcr_allocation(chip);
> -	tpm_chip_stop(chip);
> -	if (rc)
> -		return rc;
> -
>  	tpm_sysfs_add_device(chip);
>
>  	tpm_bios_log_setup(chip);
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 24ee4e1cc452..919bb0b88b12 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -190,6 +190,7 @@ static inline void tpm_msleep(unsigned int delay_msec)
>  		     delay_msec * 1000);
>  };
>
> +int tpm_chip_startup(struct tpm_chip *chip);
>  int tpm_chip_start(struct tpm_chip *chip);
>  void tpm_chip_stop(struct tpm_chip *chip);
>  struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index ddaf362e62c1..94a2bfb244b3 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -1129,6 +1129,11 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
>  	/* INTERRUPT Setup */
>  	init_waitqueue_head(&priv->read_queue);
>  	init_waitqueue_head(&priv->int_queue);
> +
> +	rc = tpm_chip_startup(chip);
> +	if (rc)
> +		goto out_err;
> +
>  	if (irq != -1) {
>  		/*
>  		 * Before doing irq testing issue a command to the TPM in polling mode

Jarko, thanks for the review so far. What about this patch, are there any concerns from your side?

Regards,
Lino
Jarkko Sakkinen Dec. 4, 2022, 5:01 p.m. UTC | #2 
On Thu, Nov 24, 2022 at 02:55:37PM +0100, Lino Sanfilippo wrote:
> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
> 
> In tpm_tis_gen_interrupt() a request for a property value is sent to the
> TPM to test if interrupts are generated. However after a power cycle the
> TPM responds with TPM_RC_INITIALIZE which indicates that the TPM is not
> yet properly initialized.
> Fix this by first starting the TPM up before the request is sent. For this
> the startup implementation is removed from tpm_chip_register() and put
> into the new function tpm_chip_startup() which is called before the
> interrupts are tested.
> 
> Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
> ---
>  drivers/char/tpm/tpm-chip.c     | 38 +++++++++++++++++++++------------
>  drivers/char/tpm/tpm.h          |  1 +
>  drivers/char/tpm/tpm_tis_core.c |  5 +++++
>  3 files changed, 30 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 783d65fc71f0..370aa1f529f2 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -543,6 +543,30 @@ static int tpm_get_pcr_allocation(struct tpm_chip *chip)
>  	return rc;
>  }
>  
> +/*
> + * tpm_chip_startup() - performs auto startup and allocates the PCRs
> + * @chip: TPM chip to use.
> + */
> +int tpm_chip_startup(struct tpm_chip *chip)
> +{
> +	int rc;
> +
> +	rc = tpm_chip_start(chip);
> +	if (rc)
> +		return rc;
> +
> +	rc = tpm_auto_startup(chip);
> +	if (rc)
> +		goto stop;
> +
> +	rc = tpm_get_pcr_allocation(chip);
> +stop:
> +	tpm_chip_stop(chip);
> +
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(tpm_chip_startup);
> +
>  /*
>   * tpm_chip_register() - create a character device for the TPM chip
>   * @chip: TPM chip to use.
> @@ -558,20 +582,6 @@ int tpm_chip_register(struct tpm_chip *chip)
>  {
>  	int rc;
>  
> -	rc = tpm_chip_start(chip);
> -	if (rc)
> -		return rc;
> -	rc = tpm_auto_startup(chip);
> -	if (rc) {
> -		tpm_chip_stop(chip);
> -		return rc;
> -	}
> -
> -	rc = tpm_get_pcr_allocation(chip);
> -	tpm_chip_stop(chip);
> -	if (rc)
> -		return rc;
> -
>  	tpm_sysfs_add_device(chip);
>  
>  	tpm_bios_log_setup(chip);
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 24ee4e1cc452..919bb0b88b12 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -190,6 +190,7 @@ static inline void tpm_msleep(unsigned int delay_msec)
>  		     delay_msec * 1000);
>  };
>  
> +int tpm_chip_startup(struct tpm_chip *chip);
>  int tpm_chip_start(struct tpm_chip *chip);
>  void tpm_chip_stop(struct tpm_chip *chip);
>  struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index ddaf362e62c1..94a2bfb244b3 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -1129,6 +1129,11 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
>  	/* INTERRUPT Setup */
>  	init_waitqueue_head(&priv->read_queue);
>  	init_waitqueue_head(&priv->int_queue);
> +
> +	rc = tpm_chip_startup(chip);
> +	if (rc)
> +		goto out_err;
> +
>  	if (irq != -1) {
>  		/*
>  		 * Before doing irq testing issue a command to the TPM in polling mode
> -- 
> 2.36.1
> 

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

BR, Jarkko
Jarkko Sakkinen Dec. 4, 2022, 5:02 p.m. UTC | #3 
On Mon, Nov 28, 2022 at 01:53:35PM +0100, Lino Sanfilippo wrote:
> 
> On 24.11.22 14:55, Lino Sanfilippo wrote:
> > From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
> >
> > In tpm_tis_gen_interrupt() a request for a property value is sent to the
> > TPM to test if interrupts are generated. However after a power cycle the
> > TPM responds with TPM_RC_INITIALIZE which indicates that the TPM is not
> > yet properly initialized.
> > Fix this by first starting the TPM up before the request is sent. For this
> > the startup implementation is removed from tpm_chip_register() and put
> > into the new function tpm_chip_startup() which is called before the
> > interrupts are tested.
> >
> > Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
> > ---
> >  drivers/char/tpm/tpm-chip.c     | 38 +++++++++++++++++++++------------
> >  drivers/char/tpm/tpm.h          |  1 +
> >  drivers/char/tpm/tpm_tis_core.c |  5 +++++
> >  3 files changed, 30 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index 783d65fc71f0..370aa1f529f2 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -543,6 +543,30 @@ static int tpm_get_pcr_allocation(struct tpm_chip *chip)
> >  	return rc;
> >  }
> >
> > +/*
> > + * tpm_chip_startup() - performs auto startup and allocates the PCRs
> > + * @chip: TPM chip to use.
> > + */
> > +int tpm_chip_startup(struct tpm_chip *chip)
> > +{
> > +	int rc;
> > +
> > +	rc = tpm_chip_start(chip);
> > +	if (rc)
> > +		return rc;
> > +
> > +	rc = tpm_auto_startup(chip);
> > +	if (rc)
> > +		goto stop;
> > +
> > +	rc = tpm_get_pcr_allocation(chip);
> > +stop:
> > +	tpm_chip_stop(chip);
> > +
> > +	return rc;
> > +}
> > +EXPORT_SYMBOL_GPL(tpm_chip_startup);
> > +
> >  /*
> >   * tpm_chip_register() - create a character device for the TPM chip
> >   * @chip: TPM chip to use.
> > @@ -558,20 +582,6 @@ int tpm_chip_register(struct tpm_chip *chip)
> >  {
> >  	int rc;
> >
> > -	rc = tpm_chip_start(chip);
> > -	if (rc)
> > -		return rc;
> > -	rc = tpm_auto_startup(chip);
> > -	if (rc) {
> > -		tpm_chip_stop(chip);
> > -		return rc;
> > -	}
> > -
> > -	rc = tpm_get_pcr_allocation(chip);
> > -	tpm_chip_stop(chip);
> > -	if (rc)
> > -		return rc;
> > -
> >  	tpm_sysfs_add_device(chip);
> >
> >  	tpm_bios_log_setup(chip);
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > index 24ee4e1cc452..919bb0b88b12 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -190,6 +190,7 @@ static inline void tpm_msleep(unsigned int delay_msec)
> >  		     delay_msec * 1000);
> >  };
> >
> > +int tpm_chip_startup(struct tpm_chip *chip);
> >  int tpm_chip_start(struct tpm_chip *chip);
> >  void tpm_chip_stop(struct tpm_chip *chip);
> >  struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
> > diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> > index ddaf362e62c1..94a2bfb244b3 100644
> > --- a/drivers/char/tpm/tpm_tis_core.c
> > +++ b/drivers/char/tpm/tpm_tis_core.c
> > @@ -1129,6 +1129,11 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
> >  	/* INTERRUPT Setup */
> >  	init_waitqueue_head(&priv->read_queue);
> >  	init_waitqueue_head(&priv->int_queue);
> > +
> > +	rc = tpm_chip_startup(chip);
> > +	if (rc)
> > +		goto out_err;
> > +
> >  	if (irq != -1) {
> >  		/*
> >  		 * Before doing irq testing issue a command to the TPM in polling mode
> 
> Jarko, thanks for the review so far. What about this patch, are there any concerns from your side?

No concerns on this one.

BR, Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 783d65fc71f0..370aa1f529f2 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -543,6 +543,30 @@  static int tpm_get_pcr_allocation(struct tpm_chip *chip)
 	return rc;
 }
 
+/*
+ * tpm_chip_startup() - performs auto startup and allocates the PCRs
+ * @chip: TPM chip to use.
+ */
+int tpm_chip_startup(struct tpm_chip *chip)
+{
+	int rc;
+
+	rc = tpm_chip_start(chip);
+	if (rc)
+		return rc;
+
+	rc = tpm_auto_startup(chip);
+	if (rc)
+		goto stop;
+
+	rc = tpm_get_pcr_allocation(chip);
+stop:
+	tpm_chip_stop(chip);
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(tpm_chip_startup);
+
 /*
  * tpm_chip_register() - create a character device for the TPM chip
  * @chip: TPM chip to use.
@@ -558,20 +582,6 @@  int tpm_chip_register(struct tpm_chip *chip)
 {
 	int rc;
 
-	rc = tpm_chip_start(chip);
-	if (rc)
-		return rc;
-	rc = tpm_auto_startup(chip);
-	if (rc) {
-		tpm_chip_stop(chip);
-		return rc;
-	}
-
-	rc = tpm_get_pcr_allocation(chip);
-	tpm_chip_stop(chip);
-	if (rc)
-		return rc;
-
 	tpm_sysfs_add_device(chip);
 
 	tpm_bios_log_setup(chip);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 24ee4e1cc452..919bb0b88b12 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -190,6 +190,7 @@  static inline void tpm_msleep(unsigned int delay_msec)
 		     delay_msec * 1000);
 };
 
+int tpm_chip_startup(struct tpm_chip *chip);
 int tpm_chip_start(struct tpm_chip *chip);
 void tpm_chip_stop(struct tpm_chip *chip);
 struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index ddaf362e62c1..94a2bfb244b3 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -1129,6 +1129,11 @@  int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 	/* INTERRUPT Setup */
 	init_waitqueue_head(&priv->read_queue);
 	init_waitqueue_head(&priv->int_queue);
+
+	rc = tpm_chip_startup(chip);
+	if (rc)
+		goto out_err;
+
 	if (irq != -1) {
 		/*
 		 * Before doing irq testing issue a command to the TPM in polling mode