LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/3] v4.16 tpmdd backports
@ 2018-04-25 10:44 Jarkko Sakkinen
  2018-04-25 10:44 ` [PATCH 1/3] tpm: cmd_ready command can be issued only after granting locality Jarkko Sakkinen
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2018-04-25 10:44 UTC (permalink / raw)
  To: stable
  Cc: Jarkko Sakkinen, Azhar Shaikh, Jason Gunthorpe,
	open list:TPM DEVICE DRIVER, open list, Tomas Winkler

"tpm: add retry logic" caused merge conflicts so I picked couple of
other fixes in order to get it apply cleanly.

James Bottomley (1):
  tpm: add retry logic

Tomas Winkler (1):
  tpm: cmd_ready command can be issued only after granting locality

Winkler, Tomas (1):
  tpm: tpm-interface: fix tpm_transmit/_cmd kdoc

 drivers/char/tpm/tpm-interface.c | 131 ++++++++++++++++++++++++-------
 drivers/char/tpm/tpm.h           |   1 +
 drivers/char/tpm/tpm_crb.c       | 108 +++++++++++++++++--------
 drivers/char/tpm/tpm_tis_core.c  |   4 +-
 include/linux/tpm.h              |   2 +-
 5 files changed, 183 insertions(+), 63 deletions(-)

-- 
2.17.0

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

* [PATCH 1/3] tpm: cmd_ready command can be issued only after granting locality
  2018-04-25 10:44 [PATCH 0/3] v4.16 tpmdd backports Jarkko Sakkinen
@ 2018-04-25 10:44 ` Jarkko Sakkinen
  2018-04-25 10:44 ` [PATCH 2/3] tpm: tpm-interface: fix tpm_transmit/_cmd kdoc Jarkko Sakkinen
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2018-04-25 10:44 UTC (permalink / raw)
  To: stable
  Cc: Tomas Winkler, Jarkko Sakkinen, Peter Huewe, Jarkko Sakkinen,
	Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman, Azhar Shaikh,
	open list:TPM DEVICE DRIVER, open list

From: Tomas Winkler <tomas.winkler@intel.com>

commit 888d867df4417deffc33927e6fc2c6925736fe92 upstream

The correct sequence is to first request locality and only after
that perform cmd_ready handshake, otherwise the hardware will drop
the subsequent message as from the device point of view the cmd_ready
handshake wasn't performed. Symmetrically locality has to be relinquished
only after going idle handshake has completed, this requires that
go_idle has to poll for the completion and as well locality
relinquish has to poll for completion so it is not overridden
in back to back commands flow.

Two wrapper functions are added (request_locality relinquish_locality)
to simplify the error handling.

The issue is only visible on devices that support multiple localities.

Fixes: 877c57d0d0ca ("tpm_crb: request and relinquish locality 0")
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c |  54 ++++++++++++----
 drivers/char/tpm/tpm_crb.c       | 108 +++++++++++++++++++++----------
 drivers/char/tpm/tpm_tis_core.c  |   4 +-
 include/linux/tpm.h              |   2 +-
 4 files changed, 120 insertions(+), 48 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 248c04090dea..0091eccdaf8d 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -369,6 +369,36 @@ static int tpm_validate_command(struct tpm_chip *chip,
 	return -EINVAL;
 }
 
+static int tpm_request_locality(struct tpm_chip *chip)
+{
+	int rc;
+
+	if (!chip->ops->request_locality)
+		return 0;
+
+	rc = chip->ops->request_locality(chip, 0);
+	if (rc < 0)
+		return rc;
+
+	chip->locality = rc;
+
+	return 0;
+}
+
+static void tpm_relinquish_locality(struct tpm_chip *chip)
+{
+	int rc;
+
+	if (!chip->ops->relinquish_locality)
+		return;
+
+	rc = chip->ops->relinquish_locality(chip, chip->locality);
+	if (rc)
+		dev_err(&chip->dev, "%s: : error %d\n", __func__, rc);
+
+	chip->locality = -1;
+}
+
 /**
  * tmp_transmit - Internal kernel interface to transmit TPM commands.
  *
@@ -422,8 +452,6 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	if (!(flags & TPM_TRANSMIT_UNLOCKED))
 		mutex_lock(&chip->tpm_mutex);
 
-	if (chip->dev.parent)
-		pm_runtime_get_sync(chip->dev.parent);
 
 	if (chip->ops->clk_enable != NULL)
 		chip->ops->clk_enable(chip, true);
@@ -431,14 +459,15 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	/* Store the decision as chip->locality will be changed. */
 	need_locality = chip->locality == -1;
 
-	if (!(flags & TPM_TRANSMIT_RAW) &&
-	    need_locality && chip->ops->request_locality)  {
-		rc = chip->ops->request_locality(chip, 0);
+	if (!(flags & TPM_TRANSMIT_RAW) && need_locality) {
+		rc = tpm_request_locality(chip);
 		if (rc < 0)
 			goto out_no_locality;
-		chip->locality = rc;
 	}
 
+	if (chip->dev.parent)
+		pm_runtime_get_sync(chip->dev.parent);
+
 	rc = tpm2_prepare_space(chip, space, ordinal, buf);
 	if (rc)
 		goto out;
@@ -499,17 +528,16 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
 
 out:
-	if (need_locality && chip->ops->relinquish_locality) {
-		chip->ops->relinquish_locality(chip, chip->locality);
-		chip->locality = -1;
-	}
+	if (chip->dev.parent)
+		pm_runtime_put_sync(chip->dev.parent);
+
+	if (need_locality)
+		tpm_relinquish_locality(chip);
+
 out_no_locality:
 	if (chip->ops->clk_enable != NULL)
 		chip->ops->clk_enable(chip, false);
 
-	if (chip->dev.parent)
-		pm_runtime_put_sync(chip->dev.parent);
-
 	if (!(flags & TPM_TRANSMIT_UNLOCKED))
 		mutex_unlock(&chip->tpm_mutex);
 	return rc ? rc : len;
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 7b3c2a8aa9de..497edd9848cd 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -112,6 +112,25 @@ struct tpm2_crb_smc {
 	u32 smc_func_id;
 };
 
+static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
+				unsigned long timeout)
+{
+	ktime_t start;
+	ktime_t stop;
+
+	start = ktime_get();
+	stop = ktime_add(start, ms_to_ktime(timeout));
+
+	do {
+		if ((ioread32(reg) & mask) == value)
+			return true;
+
+		usleep_range(50, 100);
+	} while (ktime_before(ktime_get(), stop));
+
+	return ((ioread32(reg) & mask) == value);
+}
+
 /**
  * crb_go_idle - request tpm crb device to go the idle state
  *
@@ -128,7 +147,7 @@ struct tpm2_crb_smc {
  *
  * Return: 0 always
  */
-static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
+static int crb_go_idle(struct device *dev, struct crb_priv *priv)
 {
 	if ((priv->sm == ACPI_TPM2_START_METHOD) ||
 	    (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD) ||
@@ -136,30 +155,17 @@ static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
 		return 0;
 
 	iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->regs_t->ctrl_req);
-	/* we don't really care when this settles */
 
+	if (!crb_wait_for_reg_32(&priv->regs_t->ctrl_req,
+				 CRB_CTRL_REQ_GO_IDLE/* mask */,
+				 0, /* value */
+				 TPM2_TIMEOUT_C)) {
+		dev_warn(dev, "goIdle timed out\n");
+		return -ETIME;
+	}
 	return 0;
 }
 
-static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
-				unsigned long timeout)
-{
-	ktime_t start;
-	ktime_t stop;
-
-	start = ktime_get();
-	stop = ktime_add(start, ms_to_ktime(timeout));
-
-	do {
-		if ((ioread32(reg) & mask) == value)
-			return true;
-
-		usleep_range(50, 100);
-	} while (ktime_before(ktime_get(), stop));
-
-	return false;
-}
-
 /**
  * crb_cmd_ready - request tpm crb device to enter ready state
  *
@@ -175,8 +181,7 @@ static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
  *
  * Return: 0 on success -ETIME on timeout;
  */
-static int __maybe_unused crb_cmd_ready(struct device *dev,
-					struct crb_priv *priv)
+static int crb_cmd_ready(struct device *dev, struct crb_priv *priv)
 {
 	if ((priv->sm == ACPI_TPM2_START_METHOD) ||
 	    (priv->sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD) ||
@@ -195,11 +200,11 @@ static int __maybe_unused crb_cmd_ready(struct device *dev,
 	return 0;
 }
 
-static int crb_request_locality(struct tpm_chip *chip, int loc)
+static int __crb_request_locality(struct device *dev,
+				  struct crb_priv *priv, int loc)
 {
-	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
 	u32 value = CRB_LOC_STATE_LOC_ASSIGNED |
-		CRB_LOC_STATE_TPM_REG_VALID_STS;
+		    CRB_LOC_STATE_TPM_REG_VALID_STS;
 
 	if (!priv->regs_h)
 		return 0;
@@ -207,21 +212,45 @@ static int crb_request_locality(struct tpm_chip *chip, int loc)
 	iowrite32(CRB_LOC_CTRL_REQUEST_ACCESS, &priv->regs_h->loc_ctrl);
 	if (!crb_wait_for_reg_32(&priv->regs_h->loc_state, value, value,
 				 TPM2_TIMEOUT_C)) {
-		dev_warn(&chip->dev, "TPM_LOC_STATE_x.requestAccess timed out\n");
+		dev_warn(dev, "TPM_LOC_STATE_x.requestAccess timed out\n");
 		return -ETIME;
 	}
 
 	return 0;
 }
 
-static void crb_relinquish_locality(struct tpm_chip *chip, int loc)
+static int crb_request_locality(struct tpm_chip *chip, int loc)
 {
 	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
 
+	return __crb_request_locality(&chip->dev, priv, loc);
+}
+
+static int __crb_relinquish_locality(struct device *dev,
+				     struct crb_priv *priv, int loc)
+{
+	u32 mask = CRB_LOC_STATE_LOC_ASSIGNED |
+		   CRB_LOC_STATE_TPM_REG_VALID_STS;
+	u32 value = CRB_LOC_STATE_TPM_REG_VALID_STS;
+
 	if (!priv->regs_h)
-		return;
+		return 0;
 
 	iowrite32(CRB_LOC_CTRL_RELINQUISH, &priv->regs_h->loc_ctrl);
+	if (!crb_wait_for_reg_32(&priv->regs_h->loc_state, mask, value,
+				 TPM2_TIMEOUT_C)) {
+		dev_warn(dev, "TPM_LOC_STATE_x.requestAccess timed out\n");
+		return -ETIME;
+	}
+
+	return 0;
+}
+
+static int crb_relinquish_locality(struct tpm_chip *chip, int loc)
+{
+	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
+
+	return __crb_relinquish_locality(&chip->dev, priv, loc);
 }
 
 static u8 crb_status(struct tpm_chip *chip)
@@ -475,6 +504,10 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 			dev_warn(dev, FW_BUG "Bad ACPI memory layout");
 	}
 
+	ret = __crb_request_locality(dev, priv, 0);
+	if (ret)
+		return ret;
+
 	priv->regs_t = crb_map_res(dev, priv, &io_res, buf->control_address,
 				   sizeof(struct crb_regs_tail));
 	if (IS_ERR(priv->regs_t))
@@ -531,6 +564,8 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 
 	crb_go_idle(dev, priv);
 
+	__crb_relinquish_locality(dev, priv, 0);
+
 	return ret;
 }
 
@@ -588,10 +623,14 @@ static int crb_acpi_add(struct acpi_device *device)
 	chip->acpi_dev_handle = device->handle;
 	chip->flags = TPM_CHIP_FLAG_TPM2;
 
-	rc  = crb_cmd_ready(dev, priv);
+	rc = __crb_request_locality(dev, priv, 0);
 	if (rc)
 		return rc;
 
+	rc  = crb_cmd_ready(dev, priv);
+	if (rc)
+		goto out;
+
 	pm_runtime_get_noresume(dev);
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
@@ -601,12 +640,15 @@ static int crb_acpi_add(struct acpi_device *device)
 		crb_go_idle(dev, priv);
 		pm_runtime_put_noidle(dev);
 		pm_runtime_disable(dev);
-		return rc;
+		goto out;
 	}
 
-	pm_runtime_put(dev);
+	pm_runtime_put_sync(dev);
 
-	return 0;
+out:
+	__crb_relinquish_locality(dev, priv, 0);
+
+	return rc;
 }
 
 static int crb_acpi_remove(struct acpi_device *device)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index da074e3db19b..5a1f47b43947 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -143,11 +143,13 @@ static bool check_locality(struct tpm_chip *chip, int l)
 	return false;
 }
 
-static void release_locality(struct tpm_chip *chip, int l)
+static int release_locality(struct tpm_chip *chip, int l)
 {
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
 
 	tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY);
+
+	return 0;
 }
 
 static int request_locality(struct tpm_chip *chip, int l)
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index bcdd3790e94d..06639fb6ab85 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -44,7 +44,7 @@ struct tpm_class_ops {
 	bool (*update_timeouts)(struct tpm_chip *chip,
 				unsigned long *timeout_cap);
 	int (*request_locality)(struct tpm_chip *chip, int loc);
-	void (*relinquish_locality)(struct tpm_chip *chip, int loc);
+	int (*relinquish_locality)(struct tpm_chip *chip, int loc);
 	void (*clk_enable)(struct tpm_chip *chip, bool value);
 };
 
-- 
2.17.0

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

* [PATCH 2/3] tpm: tpm-interface: fix tpm_transmit/_cmd kdoc
  2018-04-25 10:44 [PATCH 0/3] v4.16 tpmdd backports Jarkko Sakkinen
  2018-04-25 10:44 ` [PATCH 1/3] tpm: cmd_ready command can be issued only after granting locality Jarkko Sakkinen
@ 2018-04-25 10:44 ` Jarkko Sakkinen
  2018-04-25 10:44 ` [PATCH 3/3] tpm: add retry logic Jarkko Sakkinen
  2018-04-25 11:06 ` [PATCH 0/3] v4.16 tpmdd backports Greg KH
  3 siblings, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2018-04-25 10:44 UTC (permalink / raw)
  To: stable
  Cc: Winkler, Tomas, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe,
	Arnd Bergmann, Greg Kroah-Hartman, open list:TPM DEVICE DRIVER,
	open list

From: "Winkler, Tomas" <tomas.winkler@intel.com>

commit 65520d46a4adbf7f23bbb6d9b1773513f7bc7821 upstream

Fix tmp_ -> tpm_ typo and add reference to 'space' parameter
in kdoc for tpm_transmit and tpm_transmit_cmd functions.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 0091eccdaf8d..f71352cf7552 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -400,9 +400,10 @@ static void tpm_relinquish_locality(struct tpm_chip *chip)
 }
 
 /**
- * tmp_transmit - Internal kernel interface to transmit TPM commands.
+ * tpm_transmit - Internal kernel interface to transmit TPM commands.
  *
  * @chip: TPM chip to use
+ * @space: tpm space
  * @buf: TPM command buffer
  * @bufsiz: length of the TPM command buffer
  * @flags: tpm transmit flags - bitmap
@@ -544,10 +545,11 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 }
 
 /**
- * tmp_transmit_cmd - send a tpm command to the device
+ * tpm_transmit_cmd - send a tpm command to the device
  *    The function extracts tpm out header return code
  *
  * @chip: TPM chip to use
+ * @space: tpm space
  * @buf: TPM command buffer
  * @bufsiz: length of the buffer
  * @min_rsp_body_length: minimum expected length of response body
-- 
2.17.0

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

* [PATCH 3/3] tpm: add retry logic
  2018-04-25 10:44 [PATCH 0/3] v4.16 tpmdd backports Jarkko Sakkinen
  2018-04-25 10:44 ` [PATCH 1/3] tpm: cmd_ready command can be issued only after granting locality Jarkko Sakkinen
  2018-04-25 10:44 ` [PATCH 2/3] tpm: tpm-interface: fix tpm_transmit/_cmd kdoc Jarkko Sakkinen
@ 2018-04-25 10:44 ` Jarkko Sakkinen
  2018-04-25 11:06 ` [PATCH 0/3] v4.16 tpmdd backports Greg KH
  3 siblings, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2018-04-25 10:44 UTC (permalink / raw)
  To: stable
  Cc: James Bottomley, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe,
	Arnd Bergmann, Greg Kroah-Hartman, open list:TPM DEVICE DRIVER,
	open list

From: James Bottomley <James.Bottomley@HansenPartnership.com>

commit e2fb992d82c626c43ed0566e07c410e56a087af3 upstream

TPM2 can return TPM2_RC_RETRY to any command and when it does we get
unexpected failures inside the kernel that surprise users (this is
mostly observed in the trusted key handling code).  The UEFI 2.6 spec
has advice on how to handle this:

    The firmware SHALL not return TPM2_RC_RETRY prior to the completion
    of the call to ExitBootServices().

    Implementer’s Note: the implementation of this function should check
    the return value in the TPM response and, if it is TPM2_RC_RETRY,
    resend the command. The implementation may abort if a sufficient
    number of retries has been done.

So we follow that advice in our tpm_transmit() code using
TPM2_DURATION_SHORT as the initial wait duration and
TPM2_DURATION_LONG as the maximum wait time.  This should fix all the
in-kernel use cases and also means that user space TSS implementations
don't have to have their own retry handling.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: stable@vger.kernel.org
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 75 +++++++++++++++++++++++++-------
 drivers/char/tpm/tpm.h           |  1 +
 2 files changed, 61 insertions(+), 15 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index f71352cf7552..255db6fe15c8 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -399,21 +399,10 @@ static void tpm_relinquish_locality(struct tpm_chip *chip)
 	chip->locality = -1;
 }
 
-/**
- * tpm_transmit - Internal kernel interface to transmit TPM commands.
- *
- * @chip: TPM chip to use
- * @space: tpm space
- * @buf: TPM command buffer
- * @bufsiz: length of the TPM command buffer
- * @flags: tpm transmit flags - bitmap
- *
- * Return:
- *     0 when the operation is successful.
- *     A negative number for system errors (errno).
- */
-ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
-		     u8 *buf, size_t bufsiz, unsigned int flags)
+static ssize_t tpm_try_transmit(struct tpm_chip *chip,
+				struct tpm_space *space,
+				u8 *buf, size_t bufsiz,
+				unsigned int flags)
 {
 	struct tpm_output_header *header = (void *)buf;
 	int rc;
@@ -544,6 +533,62 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	return rc ? rc : len;
 }
 
+/**
+ * tpm_transmit - Internal kernel interface to transmit TPM commands.
+ *
+ * @chip: TPM chip to use
+ * @space: tpm space
+ * @buf: TPM command buffer
+ * @bufsiz: length of the TPM command buffer
+ * @flags: tpm transmit flags - bitmap
+ *
+ * A wrapper around tpm_try_transmit that handles TPM2_RC_RETRY
+ * returns from the TPM and retransmits the command after a delay up
+ * to a maximum wait of TPM2_DURATION_LONG.
+ *
+ * Note: TPM1 never returns TPM2_RC_RETRY so the retry logic is TPM2
+ * only
+ *
+ * Return:
+ *     the length of the return when the operation is successful.
+ *     A negative number for system errors (errno).
+ */
+ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
+		     u8 *buf, size_t bufsiz, unsigned int flags)
+{
+	struct tpm_output_header *header = (struct tpm_output_header *)buf;
+	/* space for header and handles */
+	u8 save[TPM_HEADER_SIZE + 3*sizeof(u32)];
+	unsigned int delay_msec = TPM2_DURATION_SHORT;
+	u32 rc = 0;
+	ssize_t ret;
+	const size_t save_size = min(space ? sizeof(save) : TPM_HEADER_SIZE,
+				     bufsiz);
+
+	/*
+	 * Subtlety here: if we have a space, the handles will be
+	 * transformed, so when we restore the header we also have to
+	 * restore the handles.
+	 */
+	memcpy(save, buf, save_size);
+
+	for (;;) {
+		ret = tpm_try_transmit(chip, space, buf, bufsiz, flags);
+		if (ret < 0)
+			break;
+		rc = be32_to_cpu(header->return_code);
+		if (rc != TPM2_RC_RETRY)
+			break;
+		delay_msec *= 2;
+		if (delay_msec > TPM2_DURATION_LONG) {
+			dev_err(&chip->dev, "TPM is in retry loop\n");
+			break;
+		}
+		tpm_msleep(delay_msec);
+		memcpy(buf, save, save_size);
+	}
+	return ret;
+}
 /**
  * tpm_transmit_cmd - send a tpm command to the device
  *    The function extracts tpm out header return code
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f895fba4e20d..d73f3fb81b42 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -108,6 +108,7 @@ enum tpm2_return_codes {
 	TPM2_RC_COMMAND_CODE    = 0x0143,
 	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
 	TPM2_RC_REFERENCE_H0	= 0x0910,
+	TPM2_RC_RETRY		= 0x0922,
 };
 
 enum tpm2_algorithms {
-- 
2.17.0

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

* Re: [PATCH 0/3] v4.16 tpmdd backports
  2018-04-25 10:44 [PATCH 0/3] v4.16 tpmdd backports Jarkko Sakkinen
                   ` (2 preceding siblings ...)
  2018-04-25 10:44 ` [PATCH 3/3] tpm: add retry logic Jarkko Sakkinen
@ 2018-04-25 11:06 ` Greg KH
  2018-04-25 17:53   ` James Bottomley
  3 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2018-04-25 11:06 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: stable, Azhar Shaikh, Jason Gunthorpe,
	open list:TPM DEVICE DRIVER, open list, Tomas Winkler

On Wed, Apr 25, 2018 at 01:44:20PM +0300, Jarkko Sakkinen wrote:
> "tpm: add retry logic" caused merge conflicts so I picked couple of
> other fixes in order to get it apply cleanly.

Are these only needed in 4.16.y?  Nothing earlier?

thanks for the backports, I'll queue them up after this next 4.16.y
kernel is released at the end of this week.

greg k-h

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

* Re: [PATCH 0/3] v4.16 tpmdd backports
  2018-04-25 11:06 ` [PATCH 0/3] v4.16 tpmdd backports Greg KH
@ 2018-04-25 17:53   ` James Bottomley
  2018-04-26 17:49     ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: James Bottomley @ 2018-04-25 17:53 UTC (permalink / raw)
  To: Greg KH, Jarkko Sakkinen
  Cc: stable, Azhar Shaikh, Jason Gunthorpe,
	open list:TPM DEVICE DRIVER, open list, Tomas Winkler

On Wed, 2018-04-25 at 13:06 +0200, Greg KH wrote:
> On Wed, Apr 25, 2018 at 01:44:20PM +0300, Jarkko Sakkinen wrote:
> > "tpm: add retry logic" caused merge conflicts so I picked couple of
> > other fixes in order to get it apply cleanly.
> 
> Are these only needed in 4.16.y?  Nothing earlier?

The retry one (tpm: add retry logic) could go back as far as you can,
but the bug it causes is rarely seen: mostly it's a failure of the
kernel trusted keys due to a tpm retry being interpreted as a fatal
error.  The number of users we have for kernel trusted keys seems to be
pretty small ...

I'd say if the backport works as is, go for it, but if we get a patch
apply failure, it's probably not worth trying to work out how to modify
the patch again until someone actually complains about the problem.

James

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

* Re: [PATCH 0/3] v4.16 tpmdd backports
  2018-04-25 17:53   ` James Bottomley
@ 2018-04-26 17:49     ` Greg KH
  2018-05-01 11:55       ` Jarkko Sakkinen
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2018-04-26 17:49 UTC (permalink / raw)
  To: James Bottomley
  Cc: Jarkko Sakkinen, stable, Azhar Shaikh, Jason Gunthorpe,
	open list:TPM DEVICE DRIVER, open list, Tomas Winkler

On Wed, Apr 25, 2018 at 10:53:32AM -0700, James Bottomley wrote:
> On Wed, 2018-04-25 at 13:06 +0200, Greg KH wrote:
> > On Wed, Apr 25, 2018 at 01:44:20PM +0300, Jarkko Sakkinen wrote:
> > > "tpm: add retry logic" caused merge conflicts so I picked couple of
> > > other fixes in order to get it apply cleanly.
> > 
> > Are these only needed in 4.16.y?  Nothing earlier?
> 
> The retry one (tpm: add retry logic) could go back as far as you can,
> but the bug it causes is rarely seen: mostly it's a failure of the
> kernel trusted keys due to a tpm retry being interpreted as a fatal
> error.  The number of users we have for kernel trusted keys seems to be
> pretty small ...
> 
> I'd say if the backport works as is, go for it, but if we get a patch
> apply failure, it's probably not worth trying to work out how to modify
> the patch again until someone actually complains about the problem.

They seem to all work for 4.14.y and 4.16.y, so now queued up, thanks.

greg k-h

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

* Re: [PATCH 0/3] v4.16 tpmdd backports
  2018-04-26 17:49     ` Greg KH
@ 2018-05-01 11:55       ` Jarkko Sakkinen
  0 siblings, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2018-05-01 11:55 UTC (permalink / raw)
  To: Greg KH
  Cc: James Bottomley, stable, Azhar Shaikh, Jason Gunthorpe,
	open list:TPM DEVICE DRIVER, open list, Tomas Winkler

On Thu, Apr 26, 2018 at 07:49:24PM +0200, Greg KH wrote:
> On Wed, Apr 25, 2018 at 10:53:32AM -0700, James Bottomley wrote:
> > On Wed, 2018-04-25 at 13:06 +0200, Greg KH wrote:
> > > On Wed, Apr 25, 2018 at 01:44:20PM +0300, Jarkko Sakkinen wrote:
> > > > "tpm: add retry logic" caused merge conflicts so I picked couple of
> > > > other fixes in order to get it apply cleanly.
> > > 
> > > Are these only needed in 4.16.y?  Nothing earlier?
> > 
> > The retry one (tpm: add retry logic) could go back as far as you can,
> > but the bug it causes is rarely seen: mostly it's a failure of the
> > kernel trusted keys due to a tpm retry being interpreted as a fatal
> > error.  The number of users we have for kernel trusted keys seems to be
> > pretty small ...
> > 
> > I'd say if the backport works as is, go for it, but if we get a patch
> > apply failure, it's probably not worth trying to work out how to modify
> > the patch again until someone actually complains about the problem.
> 
> They seem to all work for 4.14.y and 4.16.y, so now queued up, thanks.
> 
> greg k-h

+1 what James said. Thank you.

/Jarkko

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

end of thread, other threads:[~2018-05-01 11:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-25 10:44 [PATCH 0/3] v4.16 tpmdd backports Jarkko Sakkinen
2018-04-25 10:44 ` [PATCH 1/3] tpm: cmd_ready command can be issued only after granting locality Jarkko Sakkinen
2018-04-25 10:44 ` [PATCH 2/3] tpm: tpm-interface: fix tpm_transmit/_cmd kdoc Jarkko Sakkinen
2018-04-25 10:44 ` [PATCH 3/3] tpm: add retry logic Jarkko Sakkinen
2018-04-25 11:06 ` [PATCH 0/3] v4.16 tpmdd backports Greg KH
2018-04-25 17:53   ` James Bottomley
2018-04-26 17:49     ` Greg KH
2018-05-01 11:55       ` 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).