* [PATCH 1/4] tpm_crb: fix crb_req_canceled behavior
2016-09-02 19:34 [PATCH 0/4] Small fixes and cleanups for tpm_crb Jarkko Sakkinen
@ 2016-09-02 19:34 ` Jarkko Sakkinen
2016-09-02 19:34 ` [PATCH 2/4] tpm_crb: remove wmb()'s Jarkko Sakkinen
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2016-09-02 19:34 UTC (permalink / raw)
To: Peter Huewe
Cc: linux-security-module, Stefan Berger, Jarkko Sakkinen, stable,
Marcel Selhorst, Jason Gunthorpe,
moderated list:TPM DEVICE DRIVER, open list
The req_canceled() callback is used by tpm_transmit() periodically to
check whether the request has been canceled while it is receiving a
response from the TPM.
The TPM_CRB_CTRL_CANCEL register was cleared already in the crb_cancel
callback, which has two consequences:
* Cancel might not happen.
* req_canceled() always returns zero.
A better place to clear the register is when starting to send a new
command. The behavior of TPM_CRB_CTRL_CANCEL is described in the
section 5.5.3.6 of the PTP specification.
CC: stable@vger.kernel.org
Fixes: 30fc8d138e91 ("tpm: TPM 2.0 CRB Interface")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm_crb.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 018c3825..1801f38 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -142,6 +142,11 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
struct crb_priv *priv = dev_get_drvdata(&chip->dev);
int rc = 0;
+ /* Zero the cancel register so that the next command will not get
+ * canceled.
+ */
+ iowrite32(0, &priv->cca->cancel);
+
if (len > ioread32(&priv->cca->cmd_size)) {
dev_err(&chip->dev,
"invalid command count value %x %zx\n",
@@ -175,8 +180,6 @@ static void crb_cancel(struct tpm_chip *chip)
if ((priv->flags & CRB_FL_ACPI_START) && crb_do_acpi_start(chip))
dev_err(&chip->dev, "ACPI Start failed\n");
-
- iowrite32(0, &priv->cca->cancel);
}
static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/4] tpm_crb: remove wmb()'s
2016-09-02 19:34 [PATCH 0/4] Small fixes and cleanups for tpm_crb Jarkko Sakkinen
2016-09-02 19:34 ` [PATCH 1/4] tpm_crb: fix crb_req_canceled behavior Jarkko Sakkinen
@ 2016-09-02 19:34 ` Jarkko Sakkinen
2016-09-02 19:34 ` [PATCH 3/4] tpm_crb: refine the naming of constants Jarkko Sakkinen
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2016-09-02 19:34 UTC (permalink / raw)
To: Peter Huewe
Cc: linux-security-module, Stefan Berger, Jarkko Sakkinen,
Marcel Selhorst, Jason Gunthorpe,
moderated list:TPM DEVICE DRIVER, open list
wmb()'s are not needed as iowrite32() is used.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm_crb.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 1801f38..358c475 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -175,9 +175,6 @@ static void crb_cancel(struct tpm_chip *chip)
iowrite32(cpu_to_le32(CRB_CANCEL_INVOKE), &priv->cca->cancel);
- /* Make sure that cmd is populated before issuing cancel. */
- wmb();
-
if ((priv->flags & CRB_FL_ACPI_START) && crb_do_acpi_start(chip))
dev_err(&chip->dev, "ACPI Start failed\n");
}
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/4] tpm_crb: refine the naming of constants
2016-09-02 19:34 [PATCH 0/4] Small fixes and cleanups for tpm_crb Jarkko Sakkinen
2016-09-02 19:34 ` [PATCH 1/4] tpm_crb: fix crb_req_canceled behavior Jarkko Sakkinen
2016-09-02 19:34 ` [PATCH 2/4] tpm_crb: remove wmb()'s Jarkko Sakkinen
@ 2016-09-02 19:34 ` Jarkko Sakkinen
2016-09-02 19:34 ` [PATCH 4/4] tpm_crb: fix incorrect values of cmdReady and goIdle bits Jarkko Sakkinen
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2016-09-02 19:34 UTC (permalink / raw)
To: Peter Huewe
Cc: linux-security-module, Stefan Berger, Jarkko Sakkinen,
Marcel Selhorst, Jason Gunthorpe,
moderated list:TPM DEVICE DRIVER, open list
Renamed CRB protocol specific constants to match the TCG PC Client
Platform TPM Profile (PTP) Specification and driver status constants
to be explicit that they are driver specific.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm_crb.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 358c475..c8b0d91 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -34,14 +34,14 @@ enum crb_defaults {
CRB_ACPI_START_INDEX = 1,
};
-enum crb_ca_request {
- CRB_CA_REQ_GO_IDLE = BIT(0),
- CRB_CA_REQ_CMD_READY = BIT(1),
+enum crb_ctrl_req {
+ CRB_CTRL_REQ_GO_IDLE = BIT(0),
+ CRB_CTRL_REQ_CMD_READY = BIT(1),
};
-enum crb_ca_status {
- CRB_CA_STS_ERROR = BIT(0),
- CRB_CA_STS_TPM_IDLE = BIT(1),
+enum crb_ctrl_sts {
+ CRB_CTRL_STS_ERROR = BIT(0),
+ CRB_CTRL_STS_TPM_IDLE = BIT(1),
};
enum crb_start {
@@ -67,7 +67,7 @@ struct crb_control_area {
} __packed;
enum crb_status {
- CRB_STS_COMPLETE = BIT(0),
+ CRB_DRV_STS_COMPLETE = BIT(0),
};
enum crb_flags {
@@ -92,7 +92,7 @@ static u8 crb_status(struct tpm_chip *chip)
if ((ioread32(&priv->cca->start) & CRB_START_INVOKE) !=
CRB_START_INVOKE)
- sts |= CRB_STS_COMPLETE;
+ sts |= CRB_DRV_STS_COMPLETE;
return sts;
}
@@ -106,7 +106,7 @@ static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count)
if (count < 6)
return -EIO;
- if (ioread32(&priv->cca->sts) & CRB_CA_STS_ERROR)
+ if (ioread32(&priv->cca->sts) & CRB_CTRL_STS_ERROR)
return -EIO;
memcpy_fromio(buf, priv->rsp, 6);
@@ -194,8 +194,8 @@ static const struct tpm_class_ops tpm_crb = {
.send = crb_send,
.cancel = crb_cancel,
.req_canceled = crb_req_canceled,
- .req_complete_mask = CRB_STS_COMPLETE,
- .req_complete_val = CRB_STS_COMPLETE,
+ .req_complete_mask = CRB_DRV_STS_COMPLETE,
+ .req_complete_val = CRB_DRV_STS_COMPLETE,
};
static int crb_init(struct acpi_device *device, struct crb_priv *priv)
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/4] tpm_crb: fix incorrect values of cmdReady and goIdle bits
2016-09-02 19:34 [PATCH 0/4] Small fixes and cleanups for tpm_crb Jarkko Sakkinen
` (2 preceding siblings ...)
2016-09-02 19:34 ` [PATCH 3/4] tpm_crb: refine the naming of constants Jarkko Sakkinen
@ 2016-09-02 19:34 ` Jarkko Sakkinen
2016-09-02 19:41 ` Jarkko Sakkinen
2016-09-03 6:27 ` [PATCH 0/4] Small fixes and cleanups for tpm_crb Jarkko Sakkinen
2016-09-08 10:27 ` Jarkko Sakkinen
5 siblings, 1 reply; 10+ messages in thread
From: Jarkko Sakkinen @ 2016-09-02 19:34 UTC (permalink / raw)
To: Peter Huewe
Cc: linux-security-module, Stefan Berger, Jarkko Sakkinen,
Marcel Selhorst, Jason Gunthorpe,
moderated list:TPM DEVICE DRIVER, open list
CRB_CTRL_CMD_READY and CRB_CTRL_GO_IDLE have incorrect values.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm_crb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index c8b0d91..7f602dc 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -35,8 +35,8 @@ enum crb_defaults {
};
enum crb_ctrl_req {
- CRB_CTRL_REQ_GO_IDLE = BIT(0),
- CRB_CTRL_REQ_CMD_READY = BIT(1),
+ CRB_CTRL_REQ_CMD_READY = BIT(0),
+ CRB_CTRL_REQ_GO_IDLE = BIT(1),
};
enum crb_ctrl_sts {
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 0/4] Small fixes and cleanups for tpm_crb
2016-09-02 19:34 [PATCH 0/4] Small fixes and cleanups for tpm_crb Jarkko Sakkinen
` (3 preceding siblings ...)
2016-09-02 19:34 ` [PATCH 4/4] tpm_crb: fix incorrect values of cmdReady and goIdle bits Jarkko Sakkinen
@ 2016-09-03 6:27 ` Jarkko Sakkinen
2016-09-03 6:41 ` Jarkko Sakkinen
2016-09-08 10:27 ` Jarkko Sakkinen
5 siblings, 1 reply; 10+ messages in thread
From: Jarkko Sakkinen @ 2016-09-03 6:27 UTC (permalink / raw)
To: Peter Huewe
Cc: linux-security-module, Stefan Berger, Jason Gunthorpe, open list,
moderated list:TPM DEVICE DRIVER
On Fri, Sep 02, 2016 at 10:34:16PM +0300, Jarkko Sakkinen wrote:
> A set of small fixes and clean ups for tpm_crb.
>
> Jarkko Sakkinen (4):
> tpm_crb: fix crb_req_canceled behavior
> tpm_crb: remove wmb()'s
> tpm_crb: refine the naming of constants
> tpm_crb: fix incorrect values of cmdReady and goIdle bits
>
> drivers/char/tpm/tpm_crb.c | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
I guess these should be no-brainers?
/Jarkko
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/4] Small fixes and cleanups for tpm_crb
2016-09-03 6:27 ` [PATCH 0/4] Small fixes and cleanups for tpm_crb Jarkko Sakkinen
@ 2016-09-03 6:41 ` Jarkko Sakkinen
0 siblings, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2016-09-03 6:41 UTC (permalink / raw)
To: Peter Huewe
Cc: linux-security-module, Stefan Berger, Jason Gunthorpe, open list,
moderated list:TPM DEVICE DRIVER
On Sat, Sep 03, 2016 at 09:27:54AM +0300, Jarkko Sakkinen wrote:
> On Fri, Sep 02, 2016 at 10:34:16PM +0300, Jarkko Sakkinen wrote:
> > A set of small fixes and clean ups for tpm_crb.
> >
> > Jarkko Sakkinen (4):
> > tpm_crb: fix crb_req_canceled behavior
> > tpm_crb: remove wmb()'s
> > tpm_crb: refine the naming of constants
> > tpm_crb: fix incorrect values of cmdReady and goIdle bits
> >
> > drivers/char/tpm/tpm_crb.c | 32 ++++++++++++++++----------------
> > 1 file changed, 16 insertions(+), 16 deletions(-)
>
> I guess these should be no-brainers?
Actually I'm picking these patches to my runtime PM patch set.
Still reviews are always welcome of course.
/Jarkko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/4] Small fixes and cleanups for tpm_crb
2016-09-02 19:34 [PATCH 0/4] Small fixes and cleanups for tpm_crb Jarkko Sakkinen
` (4 preceding siblings ...)
2016-09-03 6:27 ` [PATCH 0/4] Small fixes and cleanups for tpm_crb Jarkko Sakkinen
@ 2016-09-08 10:27 ` Jarkko Sakkinen
2016-09-08 11:35 ` Jarkko Sakkinen
5 siblings, 1 reply; 10+ messages in thread
From: Jarkko Sakkinen @ 2016-09-08 10:27 UTC (permalink / raw)
To: Peter Huewe, tomas.winkler
Cc: linux-security-module, Stefan Berger, Jason Gunthorpe, open list,
moderated list:TPM DEVICE DRIVER
On Fri, Sep 02, 2016 at 10:34:16PM +0300, Jarkko Sakkinen wrote:
> A set of small fixes and clean ups for tpm_crb.
I will apply these fixes tomorrow because they are no brainers
basically. Tomas, I guess you can gives these tested-by since
you've tested runtime PM code on top of these?
> Jarkko Sakkinen (4):
> tpm_crb: fix crb_req_canceled behavior
> tpm_crb: remove wmb()'s
> tpm_crb: refine the naming of constants
> tpm_crb: fix incorrect values of cmdReady and goIdle bits
>
> drivers/char/tpm/tpm_crb.c | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> --
> 2.7.4
/Jarkko
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/4] Small fixes and cleanups for tpm_crb
2016-09-08 10:27 ` Jarkko Sakkinen
@ 2016-09-08 11:35 ` Jarkko Sakkinen
0 siblings, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2016-09-08 11:35 UTC (permalink / raw)
To: Peter Huewe, tomas.winkler
Cc: linux-security-module, Stefan Berger, Jason Gunthorpe, open list,
moderated list:TPM DEVICE DRIVER
On Thu, Sep 08, 2016 at 01:27:59PM +0300, Jarkko Sakkinen wrote:
> On Fri, Sep 02, 2016 at 10:34:16PM +0300, Jarkko Sakkinen wrote:
> > A set of small fixes and clean ups for tpm_crb.
>
> I will apply these fixes tomorrow because they are no brainers
> basically. Tomas, I guess you can gives these tested-by since
> you've tested runtime PM code on top of these?
I've applied these to my master branch.
/Jarkko
> > Jarkko Sakkinen (4):
> > tpm_crb: fix crb_req_canceled behavior
> > tpm_crb: remove wmb()'s
> > tpm_crb: refine the naming of constants
> > tpm_crb: fix incorrect values of cmdReady and goIdle bits
> >
> > drivers/char/tpm/tpm_crb.c | 32 ++++++++++++++++----------------
> > 1 file changed, 16 insertions(+), 16 deletions(-)
> >
> > --
> > 2.7.4
>
> /Jarkko
^ permalink raw reply [flat|nested] 10+ messages in thread