tpm_tis: convert to use locality callbacks

RFC PATCH Superseded Jerry Snitselaar 3 participants 5 messages Mar 25, 2017 – Mar 27, 2017 TPM device driver

Share & tools

Jerry Snitselaar

author

2017-03-25 20:05 UTC #0 lore raw

cover tpm_tis: convert to use locality callbacks

This is an attempt to convert tpm_tis over to using the locality callbacks added by Jarkko's tpm_crb patch. Requires the patch currently on tpmdd/locality which has moved the need_locality assignment inside the mutex in tpm_transmit.

This is pretty much the same as Jarkko's earlier patch (I think), with the addition of my changes to probe_itpm and tpm_tis_core_init. If this looks good Jarkko, if you want to split it out and have my changes go on top of your patch, or fold my changes into your patch that is fine.

Tested on TPM2.0 based tpm_tis device. Will test on TPM1.2 device this afternoon when I dig out the laptop and switch it over to the discrete tpm.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Jerry Snitselaar

author

2017-03-25 20:05 UTC #1 lore raw

RFC PATCH 1/1 tpm_tis: convert to using locality callbacks

This patch converts tpm_tis to use of the new tpm class ops request_locality, and relinquish_locality.

With the move to using the callbacks, release_locality is changed so that we now release the locality even if there is no request pending.

This required some changes to the tpm_tis_core_init code path to make sure locality is requested when needed:

- tpm2_probe code path will end up calling request/release through callbacks, so request_locality prior to tpm2_probe not needed.

- probe_itpm makes calls to tpm_tis_send_data which no longer calls request_locality, so add request_locality prior to tpm_tis_send_data calls. Also drop release_locality call in middleof probe_itpm, and keep locality until release_locality called at end of probe_itpm.

Cc Peter Huewe <[email protected]>

Cc Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/[email protected]>

Cc Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

Cc Marcel Selhorst <[email protected]>

Signed-off-by Jerry Snitselaar <[email protected]>

---
 drivers/char/tpm/tpm_tis_core.c | 35 +++++++++--------------------------
 1 file changed, 9 insertions(+), 26 deletions(-)

drivers/char/tpm/tpm_tis_core.c +9 ~~-26~~

index f31fc831c8f9..4483f6f25e17 100644
@@ -75,21 +75,11 @@ static bool check_locality(struct tpm_chip *chip, int l)
 	return false;
 }
 
-static void release_locality(struct tpm_chip *chip, int l, int force)
+static void release_locality(struct tpm_chip *chip, int l)
 {
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
-	int rc;
-	u8 access;
-
-	rc = tpm_tis_read8(priv, TPM_ACCESS(l), &access);
-	if (rc < 0)
-		return;
-
-	if (force || (access &
-		      (TPM_ACCESS_REQUEST_PENDING | TPM_ACCESS_VALID)) ==
-	    (TPM_ACCESS_REQUEST_PENDING | TPM_ACCESS_VALID))
-		tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY);
 
+	tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY);
 }
 
 static int request_locality(struct tpm_chip *chip, int l)

@@ -254,7 +244,6 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 
 out:
 	tpm_tis_ready(chip);
-	release_locality(chip, priv->locality, 0);
 	return size;
 }

@@ -270,9 +259,6 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 	size_t count = 0;
 	bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
 
-	if (request_locality(chip, 0) < 0)
-		return -EBUSY;
-
 	status = tpm_tis_status(chip);
 	if ((status & TPM_STS_COMMAND_READY) == 0) {
 		tpm_tis_ready(chip);

@@ -331,7 +317,6 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 
 out_err:
 	tpm_tis_ready(chip);
-	release_locality(chip, priv->locality, 0);
 	return rc;
 }

@@ -392,7 +377,6 @@ static int tpm_tis_send_main(struct tpm_chip *chip, u8 *buf, size_t len)
 	return len;
 out_err:
 	tpm_tis_ready(chip);
-	release_locality(chip, priv->locality, 0);
 	return rc;
 }

@@ -479,12 +463,14 @@ static int probe_itpm(struct tpm_chip *chip)
 	if (vendor != TPM_VID_INTEL)
 		return 0;
 
+	if (request_locality(chip, 0) != 0)
+		return -EBUSY;
+
 	rc = tpm_tis_send_data(chip, cmd_getticks, len);
 	if (rc == 0)
 		goto out;
 
 	tpm_tis_ready(chip);
-	release_locality(chip, priv->locality, 0);
 
 	priv->flags |= TPM_TIS_ITPM_WORKAROUND;

@@ -498,7 +484,7 @@ static int probe_itpm(struct tpm_chip *chip)
 
 out:
 	tpm_tis_ready(chip);
-	release_locality(chip, priv->locality, 0);
+	release_locality(chip, priv->locality);
 
 	return rc;
 }

@@ -672,7 +658,7 @@ void tpm_tis_remove(struct tpm_chip *chip)
 		interrupt = 0;
 
 	tpm_tis_write32(priv, reg, ~TPM_GLOBAL_INT_ENABLE & interrupt);
-	release_locality(chip, priv->locality, 1);
+	release_locality(chip, priv->locality);
 }
 EXPORT_SYMBOL_GPL(tpm_tis_remove);

@@ -686,6 +672,8 @@ static const struct tpm_class_ops tpm_tis = {
 	.req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_canceled = tpm_tis_req_canceled,
+	.request_locality = request_locality,
+	.relinquish_locality = release_locality,
 };
 
 int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,

@@ -728,11 +716,6 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 	intmask &= ~TPM_GLOBAL_INT_ENABLE;
 	tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
 
-	if (request_locality(chip, 0) != 0) {
-		rc = -ENODEV;
-		goto out_err;
-	}
-
 	rc = tpm2_probe(chip);
 	if (rc)
 		goto out_err;

Signature · 6 lines

2.11.0.258.ge05806da9


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Jason Gunthorpe

2017-03-26 20:36 UTC #2 lore raw

Re: [RFC PATCH 1/1] tpm_tis: convert to using locality callbacks

drivers/char/tpm/tpm_tis_core.c void tpm_tis_remove(struct tpm_chip *chip) · line 661

Quoted 6 lines · Jerry Snitselaar

drivers/char/tpm/tpm_tis_core.c +1 -1

@@ -672,7 +658,7 @@ void tpm_tis_remove(struct tpm_chip *chip)
 		interrupt = 0;
 
 	tpm_tis_write32(priv, reg, ~TPM_GLOBAL_INT_ENABLE & interrupt);
-	release_locality(chip, priv->locality, 1);
+	release_locality(chip, priv->locality);

Why is this done during remove? The tpm core should now keep things so that there is not a requested locality except during command so execution we should not get here with a requested locality..

Jason

Jerry Snitselaar

author

2017-03-26 22:40 UTC #3 lore raw

Re: [RFC PATCH 1/1] tpm_tis: convert to using locality callbacks

----- Original Message -----

Quoted 20 lines

From: "Jason Gunthorpe" <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
To: "Jerry Snitselaar" <[email protected]>
Cc: [email protected], [email protected], "Peter Huewe" <[email protected]>, "Jarkko Sakkinen" <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/[email protected]>, "Marcel Selhorst" <[email protected]>
Sent: Sunday, March 26, 2017 1:36:28 PM
Subject: Re: [RFC PATCH 1/1] tpm_tis: convert to using locality callbacks
drivers/char/tpm/tpm_tis_core.c void tpm_tis_remove(struct tpm_chip *chip) · line 661
Quoted 6 lines · Jerry Snitselaar
drivers/char/tpm/tpm_tis_core.c +1 -1
@@ -672,7 +658,7 @@ void tpm_tis_remove(struct tpm_chip *chip)
 		interrupt = 0;
 
 	tpm_tis_write32(priv, reg, ~TPM_GLOBAL_INT_ENABLE & interrupt);
-	release_locality(chip, priv->locality, 1);
+	release_locality(chip, priv->locality);
Why is this done during remove? The tpm core should now keep things so that there is not a requested locality except during command so execution we should not get here with a requested locality..

Quoted 2 lines

Jason

You're right, this call should be dropped. With release_locality always releasing now it shouldn't have a locality when going into remove. I'll drop this in v2.

Thanks,
Jerry

Jarkko Sakkinen

2017-03-27 05:28 UTC #4 lore raw

Re: [RFC PATCH 1/1] tpm_tis: convert to using locality callbacks

In reply to Jerry Snitselaar

Quoted 17 lines · Jerry Snitselaar

This patch converts tpm_tis to use of the new tpm class ops request_locality, and relinquish_locality.
With the move to using the callbacks, release_locality is changed so that we now release the locality even if there is no request pending.
This required some changes to the tpm_tis_core_init code path to make sure locality is requested when needed:
- tpm2_probe code path will end up calling request/release through callbacks, so request_locality prior to tpm2_probe not needed.
- probe_itpm makes calls to tpm_tis_send_data which no longer calls request_locality, so add request_locality prior to tpm_tis_send_data calls. Also drop release_locality call in middleof probe_itpm, and keep locality until release_locality called at end of probe_itpm.

Quoted 8 lines

Cc Peter Huewe <[email protected]>
Cc Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/[email protected]>
Cc Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Cc Marcel Selhorst <[email protected]>
Signed-off-by Jerry Snitselaar <[email protected]>
---
 drivers/char/tpm/tpm_tis_core.c | 35 +++++++++--------------------------
 1 file changed, 9 insertions(+), 26 deletions(-)

LGTM except what Jason said earlier.

/Jarkko