LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2] tpm: fix suspend/resume paths for TPM 2.0
@ 2015-01-29  5:43 Jarkko Sakkinen
  2015-01-29 18:43 ` Scot Doyle
  0 siblings, 1 reply; 3+ messages in thread
From: Jarkko Sakkinen @ 2015-01-29  5:43 UTC (permalink / raw)
  To: Peter Huewe, Ashley Lai, Marcel Selhorst
  Cc: tpmdd-devel, linux-kernel, josh, christophe.ricard,
	jason.gunthorpe, stefanb, linux-api, trousers-tech, jmorris,
	Jarkko Sakkinen

Fixed suspend/resume paths for TPM 2.0 and consolidated all the
associated code to the tpm_pm_suspend() and tpm_pm_resume()
functions. Resume path should be handled by the firmware, i.e.
Startup(CLEAR) for hibernate and Startup(STATE) for suspend.

There might be some non-PC embedded devices in the future where
Startup() is not the handled by the FW but fixing the code for
those IMHO should be postponed until there is hardware available
to test the fixes although extra Startup in the driver code is
essentially a NOP.

Added Shutdown(CLEAR) to the remove paths of TIS and CRB drivers.
Changed tpm2_shutdown() to a void function because there isn't
much you can do except print an error message if this fails with
a system error.

Reported-by: Peter Hüwe <PeterHuewe@gmx.de>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c |  6 ++++--
 drivers/char/tpm/tpm.h           |  2 +-
 drivers/char/tpm/tpm2-cmd.c      | 19 +++++++++++--------
 drivers/char/tpm/tpm_crb.c       | 20 +++++---------------
 drivers/char/tpm/tpm_tis.c       | 26 +++++++++++++-------------
 5 files changed, 34 insertions(+), 39 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index bf53a37..e85d341 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -901,8 +901,10 @@ int tpm_pm_suspend(struct device *dev)
 	if (chip == NULL)
 		return -ENODEV;
 
-	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		return tpm2_shutdown(chip, TPM2_SU_CLEAR);
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		tpm2_shutdown(chip, TPM2_SU_STATE);
+		return 0;
+	}
 
 	/* for buggy tpm, flush pcrs with extend to selected dummy */
 	if (tpm_suspend_pcr) {
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 7b0727c..a2ce379 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -432,7 +432,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
 			u32 *value, const char *desc);
 
 extern int tpm2_startup(struct tpm_chip *chip, u16 startup_type);
-extern int tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
+extern void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
 extern unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *, u32);
 extern int tpm2_do_selftest(struct tpm_chip *chip);
 extern int tpm2_gen_interrupt(struct tpm_chip *chip, bool quiet);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 1abe650..f2f38a5 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -456,20 +456,23 @@ static const struct tpm_input_header tpm2_shutdown_header = {
  * @chip:		TPM chip to use.
  * @shutdown_type	shutdown type. The value is either
  *			TPM_SU_CLEAR or TPM_SU_STATE.
- *
- * 0 is returned when the operation is successful. If a negative number is
- * returned it remarks a POSIX error code. If a positive number is returned
- * it remarks a TPM error.
  */
-int tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
+void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
 {
 	struct tpm2_cmd cmd;
+	int rc;
 
 	cmd.header.in = tpm2_shutdown_header;
-
 	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
-	return tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
-				"stopping the TPM");
+
+	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), "stopping the TPM");
+
+	/* In places where shutdown command is sent there's no much we can do
+	 * except print the error code on a system failure.
+	 */
+	if (rc < 0)
+		dev_warn(chip->pdev, "transmit returned %d while stopping the TPM",
+			 rc);
 }
 EXPORT_SYMBOL_GPL(tpm2_shutdown);
 
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 3dd23cf..b26ceee 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -95,21 +95,7 @@ struct crb_priv {
 	u8 __iomem *rsp;
 };
 
-#ifdef CONFIG_PM_SLEEP
-static int crb_resume(struct device *dev)
-{
-	int rc;
-	struct tpm_chip *chip = dev_get_drvdata(dev);
-
-	rc = tpm2_shutdown(chip, TPM2_SU_STATE);
-	if (!rc)
-		rc = tpm2_do_selftest(chip);
-
-	return rc;
-}
-#endif
-
-static SIMPLE_DEV_PM_OPS(crb_pm, tpm_pm_suspend, crb_resume);
+static SIMPLE_DEV_PM_OPS(crb_pm, tpm_pm_suspend, tpm_pm_resume);
 
 static u8 crb_status(struct tpm_chip *chip)
 {
@@ -326,6 +312,10 @@ static int crb_acpi_remove(struct acpi_device *device)
 	struct tpm_chip *chip = dev_get_drvdata(dev);
 
 	tpm_chip_unregister(chip);
+
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		tpm2_shutdown(chip, TPM2_SU_CLEAR);
+
 	return 0;
 }
 
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 6725bef..e12b3ab 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -588,6 +588,9 @@ MODULE_PARM_DESC(interrupts, "Enable interrupts");
 
 static void tpm_tis_remove(struct tpm_chip *chip)
 {
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		tpm2_shutdown(chip, TPM2_SU_CLEAR);
+
 	iowrite32(~TPM_GLOBAL_INT_ENABLE &
 		  ioread32(chip->vendor.iobase +
 			   TPM_INT_ENABLE(chip->vendor.
@@ -865,25 +868,22 @@ static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
 static int tpm_tis_resume(struct device *dev)
 {
 	struct tpm_chip *chip = dev_get_drvdata(dev);
-	int ret = 0;
+	int ret;
 
 	if (chip->vendor.irq)
 		tpm_tis_reenable_interrupts(chip);
 
-	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
-		/* NOP if firmware properly does this. */
-		tpm2_startup(chip, TPM2_SU_STATE);
+	ret = tpm_pm_resume(dev);
+	if (ret)
+		return ret;
 
-		ret = tpm2_shutdown(chip, TPM2_SU_STATE);
-		if (!ret)
-			ret = tpm2_do_selftest(chip);
-	} else {
-		ret = tpm_pm_resume(dev);
-		if (!ret)
-			tpm_do_selftest(chip);
-	}
+	/* TPM 1.2 requires self-test on resume. This function actually returns
+	 * an error code but for unknown reason it isn't handled.
+	 */
+	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+		tpm_do_selftest(chip);
 
-	return ret;
+	return 0;
 }
 #endif
 
-- 
2.1.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] tpm: fix suspend/resume paths for TPM 2.0
  2015-01-29  5:43 [PATCH v2] tpm: fix suspend/resume paths for TPM 2.0 Jarkko Sakkinen
@ 2015-01-29 18:43 ` Scot Doyle
  2015-02-02 19:20   ` Jarkko Sakkinen
  0 siblings, 1 reply; 3+ messages in thread
From: Scot Doyle @ 2015-01-29 18:43 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Peter Huewe, Ashley Lai, tpmdd-devel, linux-kernel, josh,
	christophe.ricard, jason.gunthorpe, stefanb, linux-api,
	trousers-tech

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1386 bytes --]

On Thu, 29 Jan 2015, Jarkko Sakkinen wrote:
> Fixed suspend/resume paths for TPM 2.0 and consolidated all the
> associated code to the tpm_pm_suspend() and tpm_pm_resume()
> functions. Resume path should be handled by the firmware, i.e.
> Startup(CLEAR) for hibernate and Startup(STATE) for suspend.
> 
> There might be some non-PC embedded devices in the future where
> Startup() is not the handled by the FW but fixing the code for
> those IMHO should be postponed until there is hardware available
> to test the fixes although extra Startup in the driver code is
> essentially a NOP.
> 
> Added Shutdown(CLEAR) to the remove paths of TIS and CRB drivers.
> Changed tpm2_shutdown() to a void function because there isn't
> much you can do except print an error message if this fails with
> a system error.
> 
> Reported-by: Peter Hüwe <PeterHuewe@gmx.de>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  drivers/char/tpm/tpm-interface.c |  6 ++++--
>  drivers/char/tpm/tpm.h           |  2 +-
>  drivers/char/tpm/tpm2-cmd.c      | 19 +++++++++++--------
>  drivers/char/tpm/tpm_crb.c       | 20 +++++---------------
>  drivers/char/tpm/tpm_tis.c       | 26 +++++++++++++-------------
>  5 files changed, 34 insertions(+), 39 deletions(-)

Resume still functions on TPM 1.2 chip, with and without CONFIG_TCG_CRB.

Tested-by: Scot Doyle <lkml14@scotdoyle.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] tpm: fix suspend/resume paths for TPM 2.0
  2015-01-29 18:43 ` Scot Doyle
@ 2015-02-02 19:20   ` Jarkko Sakkinen
  0 siblings, 0 replies; 3+ messages in thread
From: Jarkko Sakkinen @ 2015-02-02 19:20 UTC (permalink / raw)
  To: Scot Doyle
  Cc: Peter Huewe, Ashley Lai, tpmdd-devel, linux-kernel, josh,
	christophe.ricard, jason.gunthorpe, stefanb, linux-api,
	trousers-tech

Are we good with this?

/Jarkko

On Thu, Jan 29, 2015 at 06:43:12PM +0000, Scot Doyle wrote:
> On Thu, 29 Jan 2015, Jarkko Sakkinen wrote:
> > Fixed suspend/resume paths for TPM 2.0 and consolidated all the
> > associated code to the tpm_pm_suspend() and tpm_pm_resume()
> > functions. Resume path should be handled by the firmware, i.e.
> > Startup(CLEAR) for hibernate and Startup(STATE) for suspend.
> > 
> > There might be some non-PC embedded devices in the future where
> > Startup() is not the handled by the FW but fixing the code for
> > those IMHO should be postponed until there is hardware available
> > to test the fixes although extra Startup in the driver code is
> > essentially a NOP.
> > 
> > Added Shutdown(CLEAR) to the remove paths of TIS and CRB drivers.
> > Changed tpm2_shutdown() to a void function because there isn't
> > much you can do except print an error message if this fails with
> > a system error.
> > 
> > Reported-by: Peter Hüwe <PeterHuewe@gmx.de>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> >  drivers/char/tpm/tpm-interface.c |  6 ++++--
> >  drivers/char/tpm/tpm.h           |  2 +-
> >  drivers/char/tpm/tpm2-cmd.c      | 19 +++++++++++--------
> >  drivers/char/tpm/tpm_crb.c       | 20 +++++---------------
> >  drivers/char/tpm/tpm_tis.c       | 26 +++++++++++++-------------
> >  5 files changed, 34 insertions(+), 39 deletions(-)
> 
> Resume still functions on TPM 1.2 chip, with and without CONFIG_TCG_CRB.
> 
> Tested-by: Scot Doyle <lkml14@scotdoyle.com>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-02-02 19:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-29  5:43 [PATCH v2] tpm: fix suspend/resume paths for TPM 2.0 Jarkko Sakkinen
2015-01-29 18:43 ` Scot Doyle
2015-02-02 19:20   ` Jarkko Sakkinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).