Stable Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v4] tpm: ibmvtpm: retry on H_CLOSED in tpm_ibmvtpm_send()
@ 2020-03-20  3:27 George Wilson
  2020-03-20 14:12 ` Jarkko Sakkinen
  2020-03-20 17:59 ` Sasha Levin
  0 siblings, 2 replies; 4+ messages in thread
From: George Wilson @ 2020-03-20  3:27 UTC (permalink / raw)
  To: linux-integrity
  Cc: Alexey Kardashevskiy, Stefan Berger, Jarkko Sakkinen, Nayna Jain,
	Jason Gunthorpe, linux-kernel, George Wilson, Linh Pham, stable

tpm_ibmvtpm_send() can fail during PowerVM Live Partition Mobility resume
with an H_CLOSED return from ibmvtpm_send_crq().  The PAPR says, 'The
"partner partition suspended" transport event disables the associated CRQ
such that any H_SEND_CRQ hcall() to the associated CRQ returns H_Closed
until the CRQ has been explicitly enabled using the H_ENABLE_CRQ hcall.'
This patch adds a check in tpm_ibmvtpm_send() for an H_CLOSED return from
ibmvtpm_send_crq() and in that case calls tpm_ibmvtpm_resume() and
retries the ibmvtpm_send_crq() once.

Reported-by: Linh Pham <phaml@us.ibm.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: George Wilson <gcwilson@linux.ibm.com>
Tested-by: Linh Pham <phaml@us.ibm.com>
Fixes: 132f76294744 ("drivers/char/tpm: Add new device driver to support IBM vTPM")
Cc: stable@vger.kernel.org
---
 drivers/char/tpm/tpm_ibmvtpm.c | 136 ++++++++++++++++++---------------
 1 file changed, 73 insertions(+), 63 deletions(-)

diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 78cc52690177..e82013d587b4 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright (C) 2012 IBM Corporation
+ * Copyright (C) 2012-2020 IBM Corporation
  *
  * Author: Ashley Lai <ashleydlai@gmail.com>
  *
@@ -133,6 +133,64 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	return len;
 }
 
+/**
+ * ibmvtpm_crq_send_init - Send a CRQ initialize message
+ * @ibmvtpm:	vtpm device struct
+ *
+ * Return:
+ *	0 on success.
+ *	Non-zero on failure.
+ */
+static int ibmvtpm_crq_send_init(struct ibmvtpm_dev *ibmvtpm)
+{
+	int rc;
+
+	rc = ibmvtpm_send_crq_word(ibmvtpm->vdev, INIT_CRQ_CMD);
+	if (rc != H_SUCCESS)
+		dev_err(ibmvtpm->dev,
+			"%s failed rc=%d\n", __func__, rc);
+
+	return rc;
+}
+
+/**
+ * tpm_ibmvtpm_resume - Resume from suspend
+ *
+ * @dev:	device struct
+ *
+ * Return: Always 0.
+ */
+static int tpm_ibmvtpm_resume(struct device *dev)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+	struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
+	int rc = 0;
+
+	do {
+		if (rc)
+			msleep(100);
+		rc = plpar_hcall_norets(H_ENABLE_CRQ,
+					ibmvtpm->vdev->unit_address);
+	} while (rc == H_IN_PROGRESS || rc == H_BUSY || H_IS_LONG_BUSY(rc));
+
+	if (rc) {
+		dev_err(dev, "Error enabling ibmvtpm rc=%d\n", rc);
+		return rc;
+	}
+
+	rc = vio_enable_interrupts(ibmvtpm->vdev);
+	if (rc) {
+		dev_err(dev, "Error vio_enable_interrupts rc=%d\n", rc);
+		return rc;
+	}
+
+	rc = ibmvtpm_crq_send_init(ibmvtpm);
+	if (rc)
+		dev_err(dev, "Error send_init rc=%d\n", rc);
+
+	return rc;
+}
+
 /**
  * tpm_ibmvtpm_send() - Send a TPM command
  * @chip:	tpm chip struct
@@ -146,6 +204,7 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
 {
 	struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
+	bool retry = true;
 	int rc, sig;
 
 	if (!ibmvtpm->rtce_buf) {
@@ -179,18 +238,27 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
 	 */
 	ibmvtpm->tpm_processing_cmd = true;
 
+again:
 	rc = ibmvtpm_send_crq(ibmvtpm->vdev,
 			IBMVTPM_VALID_CMD, VTPM_TPM_COMMAND,
 			count, ibmvtpm->rtce_dma_handle);
 	if (rc != H_SUCCESS) {
+		/*
+		 * H_CLOSED can be returned after LPM resume.  Call
+		 * tpm_ibmvtpm_resume() to re-enable the CRQ then retry
+		 * ibmvtpm_send_crq() once before failing.
+		 */
+		if (rc == H_CLOSED && retry) {
+			tpm_ibmvtpm_resume(ibmvtpm->dev);
+			retry = false;
+			goto again;
+		}
 		dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc);
-		rc = 0;
 		ibmvtpm->tpm_processing_cmd = false;
-	} else
-		rc = 0;
+	}
 
 	spin_unlock(&ibmvtpm->rtce_lock);
-	return rc;
+	return 0;
 }
 
 static void tpm_ibmvtpm_cancel(struct tpm_chip *chip)
@@ -268,26 +336,6 @@ static int ibmvtpm_crq_send_init_complete(struct ibmvtpm_dev *ibmvtpm)
 	return rc;
 }
 
-/**
- * ibmvtpm_crq_send_init - Send a CRQ initialize message
- * @ibmvtpm:	vtpm device struct
- *
- * Return:
- *	0 on success.
- *	Non-zero on failure.
- */
-static int ibmvtpm_crq_send_init(struct ibmvtpm_dev *ibmvtpm)
-{
-	int rc;
-
-	rc = ibmvtpm_send_crq_word(ibmvtpm->vdev, INIT_CRQ_CMD);
-	if (rc != H_SUCCESS)
-		dev_err(ibmvtpm->dev,
-			"ibmvtpm_crq_send_init failed rc=%d\n", rc);
-
-	return rc;
-}
-
 /**
  * tpm_ibmvtpm_remove - ibm vtpm remove entry point
  * @vdev:	vio device struct
@@ -400,44 +448,6 @@ static int ibmvtpm_reset_crq(struct ibmvtpm_dev *ibmvtpm)
 				  ibmvtpm->crq_dma_handle, CRQ_RES_BUF_SIZE);
 }
 
-/**
- * tpm_ibmvtpm_resume - Resume from suspend
- *
- * @dev:	device struct
- *
- * Return: Always 0.
- */
-static int tpm_ibmvtpm_resume(struct device *dev)
-{
-	struct tpm_chip *chip = dev_get_drvdata(dev);
-	struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
-	int rc = 0;
-
-	do {
-		if (rc)
-			msleep(100);
-		rc = plpar_hcall_norets(H_ENABLE_CRQ,
-					ibmvtpm->vdev->unit_address);
-	} while (rc == H_IN_PROGRESS || rc == H_BUSY || H_IS_LONG_BUSY(rc));
-
-	if (rc) {
-		dev_err(dev, "Error enabling ibmvtpm rc=%d\n", rc);
-		return rc;
-	}
-
-	rc = vio_enable_interrupts(ibmvtpm->vdev);
-	if (rc) {
-		dev_err(dev, "Error vio_enable_interrupts rc=%d\n", rc);
-		return rc;
-	}
-
-	rc = ibmvtpm_crq_send_init(ibmvtpm);
-	if (rc)
-		dev_err(dev, "Error send_init rc=%d\n", rc);
-
-	return rc;
-}
-
 static bool tpm_ibmvtpm_req_canceled(struct tpm_chip *chip, u8 status)
 {
 	return (status == 0);
-- 
2.24.1


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

* Re: [PATCH v4] tpm: ibmvtpm: retry on H_CLOSED in tpm_ibmvtpm_send()
  2020-03-20  3:27 [PATCH v4] tpm: ibmvtpm: retry on H_CLOSED in tpm_ibmvtpm_send() George Wilson
@ 2020-03-20 14:12 ` Jarkko Sakkinen
  2020-03-20 17:59 ` Sasha Levin
  1 sibling, 0 replies; 4+ messages in thread
From: Jarkko Sakkinen @ 2020-03-20 14:12 UTC (permalink / raw)
  To: George Wilson
  Cc: linux-integrity, Alexey Kardashevskiy, Stefan Berger, Nayna Jain,
	Jason Gunthorpe, linux-kernel, Linh Pham, stable

On Thu, Mar 19, 2020 at 11:27:58PM -0400, George Wilson wrote:
> tpm_ibmvtpm_send() can fail during PowerVM Live Partition Mobility resume
> with an H_CLOSED return from ibmvtpm_send_crq().  The PAPR says, 'The
> "partner partition suspended" transport event disables the associated CRQ
> such that any H_SEND_CRQ hcall() to the associated CRQ returns H_Closed
> until the CRQ has been explicitly enabled using the H_ENABLE_CRQ hcall.'
> This patch adds a check in tpm_ibmvtpm_send() for an H_CLOSED return from
> ibmvtpm_send_crq() and in that case calls tpm_ibmvtpm_resume() and
> retries the ibmvtpm_send_crq() once.
> 
> Reported-by: Linh Pham <phaml@us.ibm.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> Signed-off-by: George Wilson <gcwilson@linux.ibm.com>
> Tested-by: Linh Pham <phaml@us.ibm.com>
> Fixes: 132f76294744 ("drivers/char/tpm: Add new device driver to support IBM vTPM")
> Cc: stable@vger.kernel.org

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Thanks. Applied now.

/Jarkko

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

* Re: [PATCH v4] tpm: ibmvtpm: retry on H_CLOSED in tpm_ibmvtpm_send()
  2020-03-20  3:27 [PATCH v4] tpm: ibmvtpm: retry on H_CLOSED in tpm_ibmvtpm_send() George Wilson
  2020-03-20 14:12 ` Jarkko Sakkinen
@ 2020-03-20 17:59 ` Sasha Levin
  2020-03-25 15:45   ` George Wilson
  1 sibling, 1 reply; 4+ messages in thread
From: Sasha Levin @ 2020-03-20 17:59 UTC (permalink / raw)
  To: Sasha Levin, George Wilson, linux-integrity
  Cc: Alexey Kardashevskiy, stable, stable

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: 132f76294744 ("drivers/char/tpm: Add new device driver to support IBM vTPM").

The bot has tested the following trees: v5.5.10, v5.4.26, v4.19.111, v4.14.173, v4.9.216, v4.4.216.

v5.5.10: Build OK!
v5.4.26: Build OK!
v4.19.111: Build OK!
v4.14.173: Build OK!
v4.9.216: Failed to apply! Possible dependencies:
    02ae1382882f ("tpm: redefine read_log() to handle ACPI/OF at runtime")
    2528a64664f8 ("tpm: define a generic open() method for ascii & bios measurements")
    402149c6470d ("tpm: vtpm_proxy: Suppress error logging when in closed state")
    4d23cc323cdb ("tpm: add securityfs support for TPM 2.0 firmware event log")
    745b361e989a ("tpm: infrastructure for TPM spaces")
    748935eeb72c ("tpm: have event log use the tpm_chip")
    7518a21a9da3 ("tpm: drop tpm1_chip_register(/unregister)")
    b1a9b7b602c5 ("tpm: replace symbolic permission with octal for securityfs files")
    cd9b7631a888 ("tpm: replace dynamically allocated bios_dir with a static array")
    f5595f5baa30 ("tpm: Unify the send callback behaviour")

v4.4.216: Failed to apply! Possible dependencies:
    02ae1382882f ("tpm: redefine read_log() to handle ACPI/OF at runtime")
    036bb38ffb3e ("tpm_tis: Ensure interrupts are disabled when the driver starts")
    23d06ff700f5 ("tpm: drop tpm_atmel specific fields from tpm_vendor_specific")
    25112048cd59 ("tpm: rework tpm_get_timeouts()")
    402149c6470d ("tpm: vtpm_proxy: Suppress error logging when in closed state")
    41a5e1cf1fe1 ("tpm/tpm_tis: Split tpm_tis driver into a core and TCG TIS compliant phy")
    4d627e672bd0 ("tpm_tis: Do not fall back to a hardcoded address for TPM2")
    4eea703caaac ("tpm: drop 'iobase' from struct tpm_vendor_specific")
    51dd43dff74b ("tpm_tis: Use devm_ioremap_resource")
    55a889c2cb13 ("tpm_crb: Use the common ACPI definition of struct acpi_tpm2")
    56671c893e0e ("tpm: drop 'locality' from struct tpm_vendor_specific")
    570a36097f30 ("tpm: drop 'irq' from struct tpm_vendor_specific")
    57dacc2b4ce5 ("tpm: tpm_tis: Share common data between phys")
    745b361e989a ("tpm: infrastructure for TPM spaces")
    7ab4032fa579 ("tpm_tis: Get rid of the duplicate IRQ probing code")
    d30b8e4f68ef ("tpm: cleanup tpm_tis_remove()")
    d4956524f1b0 ("tpm: drop manufacturer_id from struct tpm_vendor_specific")
    e3837e74a06d ("tpm_tis: Refactor the interrupt setup")
    ee1779840d09 ("tpm: drop 'base' from struct tpm_vendor_specific")
    ef7b81dc7864 ("tpm_tis: Disable interrupt auto probing on a per-device basis")
    f5595f5baa30 ("tpm: Unify the send callback behaviour")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha

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

* Re: [PATCH v4] tpm: ibmvtpm: retry on H_CLOSED in tpm_ibmvtpm_send()
  2020-03-20 17:59 ` Sasha Levin
@ 2020-03-25 15:45   ` George Wilson
  0 siblings, 0 replies; 4+ messages in thread
From: George Wilson @ 2020-03-25 15:45 UTC (permalink / raw)
  To: Sasha Levin; +Cc: linux-integrity, Alexey Kardashevskiy, stable

On Fri, Mar 20, 2020 at 05:59:44PM +0000, Sasha Levin wrote:
> Hi
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag
> fixing commit: 132f76294744 ("drivers/char/tpm: Add new device driver to support IBM vTPM").
> 
> The bot has tested the following trees: v5.5.10, v5.4.26, v4.19.111, v4.14.173, v4.9.216, v4.4.216.
> 
> v5.5.10: Build OK!
> v5.4.26: Build OK!
> v4.19.111: Build OK!
> v4.14.173: Build OK!
> v4.9.216: Failed to apply! Possible dependencies:
>     02ae1382882f ("tpm: redefine read_log() to handle ACPI/OF at runtime")
>     2528a64664f8 ("tpm: define a generic open() method for ascii & bios measurements")
>     402149c6470d ("tpm: vtpm_proxy: Suppress error logging when in closed state")
>     4d23cc323cdb ("tpm: add securityfs support for TPM 2.0 firmware event log")
>     745b361e989a ("tpm: infrastructure for TPM spaces")
>     748935eeb72c ("tpm: have event log use the tpm_chip")
>     7518a21a9da3 ("tpm: drop tpm1_chip_register(/unregister)")
>     b1a9b7b602c5 ("tpm: replace symbolic permission with octal for securityfs files")
>     cd9b7631a888 ("tpm: replace dynamically allocated bios_dir with a static array")
>     f5595f5baa30 ("tpm: Unify the send callback behaviour")
> 
> v4.4.216: Failed to apply! Possible dependencies:
>     02ae1382882f ("tpm: redefine read_log() to handle ACPI/OF at runtime")
>     036bb38ffb3e ("tpm_tis: Ensure interrupts are disabled when the driver starts")
>     23d06ff700f5 ("tpm: drop tpm_atmel specific fields from tpm_vendor_specific")
>     25112048cd59 ("tpm: rework tpm_get_timeouts()")
>     402149c6470d ("tpm: vtpm_proxy: Suppress error logging when in closed state")
>     41a5e1cf1fe1 ("tpm/tpm_tis: Split tpm_tis driver into a core and TCG TIS compliant phy")
>     4d627e672bd0 ("tpm_tis: Do not fall back to a hardcoded address for TPM2")
>     4eea703caaac ("tpm: drop 'iobase' from struct tpm_vendor_specific")
>     51dd43dff74b ("tpm_tis: Use devm_ioremap_resource")
>     55a889c2cb13 ("tpm_crb: Use the common ACPI definition of struct acpi_tpm2")
>     56671c893e0e ("tpm: drop 'locality' from struct tpm_vendor_specific")
>     570a36097f30 ("tpm: drop 'irq' from struct tpm_vendor_specific")
>     57dacc2b4ce5 ("tpm: tpm_tis: Share common data between phys")
>     745b361e989a ("tpm: infrastructure for TPM spaces")
>     7ab4032fa579 ("tpm_tis: Get rid of the duplicate IRQ probing code")
>     d30b8e4f68ef ("tpm: cleanup tpm_tis_remove()")
>     d4956524f1b0 ("tpm: drop manufacturer_id from struct tpm_vendor_specific")
>     e3837e74a06d ("tpm_tis: Refactor the interrupt setup")
>     ee1779840d09 ("tpm: drop 'base' from struct tpm_vendor_specific")
>     ef7b81dc7864 ("tpm_tis: Disable interrupt auto probing on a per-device basis")
>     f5595f5baa30 ("tpm: Unify the send callback behaviour")
> 
> 
> NOTE: The patch will not be queued to stable trees until it is upstream.
> 
> How should we proceed with this patch?

Hi Sasha,

I've backported the patch to both 4.9.y and 4.4.y.

> 
> -- 
> Thanks
> Sasha

-- 
George Wilson
IBM Linux Technology Center
Security Development

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-20  3:27 [PATCH v4] tpm: ibmvtpm: retry on H_CLOSED in tpm_ibmvtpm_send() George Wilson
2020-03-20 14:12 ` Jarkko Sakkinen
2020-03-20 17:59 ` Sasha Levin
2020-03-25 15:45   ` George Wilson

Stable Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://lore.kernel.org/stable \
		stable@vger.kernel.org
	public-inbox-index stable

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git