[PATCH] tpm: Re-enable TPM chip boostrapping non-tpm_tis TPM drivers

[PATCH] tpm: Re-enable TPM chip boostrapping non-tpm_tis TPM drivers

[Jarkko Sakkinen]

TPM chip bootstrapping was removed from tpm_chip_register(), and it
was relocated to tpm_tis_core. This breaks all drivers which are not
based on tpm_tis because the chip will not get properly initialized.

Take the corrective steps:
1. Rename tpm_chip_startup() as tpm_chip_bootstrap() and make it one-shot.
2. Call tpm_chip_bootstrap() in tpm_chip_register(), which reverts the
   things  as tehy used to be.

Cc: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Reported-by: Pengfei Xu <pengfei.xu@intel.com>
Link: https://lore.kernel.org/all/ZEjqhwHWBnxcaRV5@xpf.sh.intel.com/
Fixes: 548eb516ec0f ("tpm, tpm_tis: startup chip before testing for interrupts")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 drivers/char/tpm/tpm-chip.c     | 22 +++++++++++++++++++---
 drivers/char/tpm/tpm.h          |  2 +-
 drivers/char/tpm/tpm_tis_core.c |  2 +-
 include/linux/tpm.h             | 13 +++++++------
 4 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 6fdfa65a00c3..b8a484b4ff7a 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -606,13 +606,19 @@ static int tpm_get_pcr_allocation(struct tpm_chip *chip)
 }
 
 /*
- * tpm_chip_startup() - performs auto startup and allocates the PCRs
+ * tpm_chip_bootstrap() - Boostrap TPM chip after power on
  * @chip: TPM chip to use.
+ *
+ * Initialize TPM chip after power on. This a one-shot function: subsequent
+ * calls will have no effect.
  */
-int tpm_chip_startup(struct tpm_chip *chip)
+int tpm_chip_bootstrap(struct tpm_chip *chip)
 {
 	int rc;
 
+	if (chip->flags & TPM_CHIP_FLAG_BOOTSTRAP)
+		return 0;
+
 	rc = tpm_chip_start(chip);
 	if (rc)
 		return rc;
@@ -625,9 +631,15 @@ int tpm_chip_startup(struct tpm_chip *chip)
 stop:
 	tpm_chip_stop(chip);
 
+	/*
+	 * Unconditionally set, as driver initialization should cease, when the
+	 * boostrapping process fails.
+	 */
+	chip->flags |= TPM_CHIP_FLAG_BOOTSTRAP;
+
 	return rc;
 }
-EXPORT_SYMBOL_GPL(tpm_chip_startup);
+EXPORT_SYMBOL_GPL(tpm_chip_bootstrap);
 
 /*
  * tpm_chip_register() - create a character device for the TPM chip
@@ -644,6 +656,10 @@ int tpm_chip_register(struct tpm_chip *chip)
 {
 	int rc;
 
+	rc = tpm_chip_bootstrap(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 88d3bd76e076..f6c99b3f0045 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -263,7 +263,7 @@ static inline void tpm_msleep(unsigned int delay_msec)
 		     delay_msec * 1000);
 };
 
-int tpm_chip_startup(struct tpm_chip *chip);
+int tpm_chip_bootstrap(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 c2421162cf34..02945d53fcef 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -1139,7 +1139,7 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 	init_waitqueue_head(&priv->read_queue);
 	init_waitqueue_head(&priv->int_queue);
 
-	rc = tpm_chip_startup(chip);
+	rc = tpm_chip_bootstrap(chip);
 	if (rc)
 		goto out_err;
 
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 4dc97b9f65fb..50bcdee19b12 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -274,13 +274,14 @@ enum tpm2_cc_attrs {
 #define TPM_VID_ATML     0x1114
 
 enum tpm_chip_flags {
-	TPM_CHIP_FLAG_TPM2		= BIT(1),
-	TPM_CHIP_FLAG_IRQ		= BIT(2),
-	TPM_CHIP_FLAG_VIRTUAL		= BIT(3),
-	TPM_CHIP_FLAG_HAVE_TIMEOUTS	= BIT(4),
-	TPM_CHIP_FLAG_ALWAYS_POWERED	= BIT(5),
+	TPM_CHIP_FLAG_TPM2			= BIT(1),
+	TPM_CHIP_FLAG_IRQ			= BIT(2),
+	TPM_CHIP_FLAG_VIRTUAL			= BIT(3),
+	TPM_CHIP_FLAG_HAVE_TIMEOUTS		= BIT(4),
+	TPM_CHIP_FLAG_ALWAYS_POWERED		= BIT(5),
 	TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED	= BIT(6),
-	TPM_CHIP_FLAG_FIRMWARE_UPGRADE	= BIT(7),
+	TPM_CHIP_FLAG_FIRMWARE_UPGRADE		= BIT(7),
+	TPM_CHIP_FLAG_BOOTSTRAP			= BIT(8),
 };
 
 #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
-- 
2.39.2

Re: [PATCH] tpm: Re-enable TPM chip boostrapping non-tpm_tis TPM drivers

[Jarkko Sakkinen]

On Thu, 2023-04-27 at 09:58 +0800, Pengfei Xu wrote:
> On 2023-04-26 at 21:58:33 +0300, Jarkko Sakkinen wrote:
> > TPM chip bootstrapping was removed from tpm_chip_register(), and it
> > was relocated to tpm_tis_core. This breaks all drivers which are not
> > based on tpm_tis because the chip will not get properly initialized.
> > 
> > Take the corrective steps:
> > 1. Rename tpm_chip_startup() as tpm_chip_bootstrap() and make it one-shot.
> > 2. Call tpm_chip_bootstrap() in tpm_chip_register(), which reverts the
> >    things  as tehy used to be.
>            ^ two space and one typo "they"
> 	   Thanks for your fixed patch in short time!
> 
>   And I tested this patch, it could not be reproduced in 155s, and the patch
>   fixed this issue.
>   Fixed dmesg is in attached.
> 
> Tested-by: Pengfei Xu <pengfei.xu@intel.com>

Thanks a lot! I'll take immediate action to send PR to Linus.

Lino: no worries. It was expected that such a large refactorization
could break a thing or two. You did a good job in any case.

BR, Jarkko

Re: [PATCH] tpm: Re-enable TPM chip boostrapping non-tpm_tis TPM drivers

[Jarkko Sakkinen]

On Thu, 2023-04-27 at 13:52 +0300, Jarkko Sakkinen wrote:
> On Thu, 2023-04-27 at 09:58 +0800, Pengfei Xu wrote:
> > On 2023-04-26 at 21:58:33 +0300, Jarkko Sakkinen wrote:
> > > TPM chip bootstrapping was removed from tpm_chip_register(), and it
> > > was relocated to tpm_tis_core. This breaks all drivers which are not
> > > based on tpm_tis because the chip will not get properly initialized.
> > > 
> > > Take the corrective steps:
> > > 1. Rename tpm_chip_startup() as tpm_chip_bootstrap() and make it one-shot.
> > > 2. Call tpm_chip_bootstrap() in tpm_chip_register(), which reverts the
> > >    things  as tehy used to be.
> >            ^ two space and one typo "they"
> > 	   Thanks for your fixed patch in short time!
> > 
> >   And I tested this patch, it could not be reproduced in 155s, and the patch
> >   fixed this issue.
> >   Fixed dmesg is in attached.
> > 
> > Tested-by: Pengfei Xu <pengfei.xu@intel.com>
> 
> Thanks a lot! I'll take immediate action to send PR to Linus.
> 
> Lino: no worries. It was expected that such a large refactorization
> could break a thing or two. You did a good job in any case.

Done: https://lkml.org/lkml/2023/4/27/297

BR, Jarkko

Re: [PATCH] tpm: Re-enable TPM chip boostrapping non-tpm_tis TPM drivers

[Lino Sanfilippo]

Hi Jarkko,

On 27.04.23 12:52, Jarkko Sakkinen wrote:
> ATTENTION: This e-mail is from an external sender. Please check attachments and links before opening e.g. with mouseover.
> 
> 
> On Thu, 2023-04-27 at 09:58 +0800, Pengfei Xu wrote:
>> On 2023-04-26 at 21:58:33 +0300, Jarkko Sakkinen wrote:
>>> TPM chip bootstrapping was removed from tpm_chip_register(), and it
>>> was relocated to tpm_tis_core. This breaks all drivers which are not
>>> based on tpm_tis because the chip will not get properly initialized.
>>>
>>> Take the corrective steps:
>>> 1. Rename tpm_chip_startup() as tpm_chip_bootstrap() and make it one-shot.
>>> 2. Call tpm_chip_bootstrap() in tpm_chip_register(), which reverts the
>>>    things  as tehy used to be.
>>            ^ two space and one typo "they"
>>          Thanks for your fixed patch in short time!
>>
>>   And I tested this patch, it could not be reproduced in 155s, and the patch
>>   fixed this issue.
>>   Fixed dmesg is in attached.
>>
>> Tested-by: Pengfei Xu <pengfei.xu@intel.com>
> 
> Thanks a lot! I'll take immediate action to send PR to Linus.
> 
> Lino: no worries. It was expected that such a large refactorization
> could break a thing or two. You did a good job in any case.
> 
> BR, Jarkko

I guess you are right, there will probably be more issues showing up
in the next time. I will try my best to help fixing them.
Thank you very much (also for fixing this issue)!

Best regards,
Lino

Re: [PATCH] tpm: Re-enable TPM chip boostrapping non-tpm_tis TPM drivers

[Jarkko Sakkinen]

On Thu Apr 27, 2023 at 6:33 PM EEST, Lino Sanfilippo wrote:
>
> Hi Jarkko,
>
> On 27.04.23 12:52, Jarkko Sakkinen wrote:
> > ATTENTION: This e-mail is from an external sender. Please check attachments and links before opening e.g. with mouseover.
> > 
> > 
> > On Thu, 2023-04-27 at 09:58 +0800, Pengfei Xu wrote:
> >> On 2023-04-26 at 21:58:33 +0300, Jarkko Sakkinen wrote:
> >>> TPM chip bootstrapping was removed from tpm_chip_register(), and it
> >>> was relocated to tpm_tis_core. This breaks all drivers which are not
> >>> based on tpm_tis because the chip will not get properly initialized.
> >>>
> >>> Take the corrective steps:
> >>> 1. Rename tpm_chip_startup() as tpm_chip_bootstrap() and make it one-shot.
> >>> 2. Call tpm_chip_bootstrap() in tpm_chip_register(), which reverts the
> >>>    things  as tehy used to be.
> >>            ^ two space and one typo "they"
> >>          Thanks for your fixed patch in short time!
> >>
> >>   And I tested this patch, it could not be reproduced in 155s, and the patch
> >>   fixed this issue.
> >>   Fixed dmesg is in attached.
> >>
> >> Tested-by: Pengfei Xu <pengfei.xu@intel.com>
> > 
> > Thanks a lot! I'll take immediate action to send PR to Linus.
> > 
> > Lino: no worries. It was expected that such a large refactorization
> > could break a thing or two. You did a good job in any case.
> > 
> > BR, Jarkko
>
> I guess you are right, there will probably be more issues showing up
> in the next time. I will try my best to help fixing them.
> Thank you very much (also for fixing this issue)!

Yeah, no worries, that is my responsibility in the end to take care of
whatever I accept :-) And yeah, this was complicated change to do, I
think you did your best and that is good enough as far as I'm concerned.

BR, Jarkko