Request and relinquish locality 0 so that the CRB driver is a good
citizen in a multi locality environment like TXT.
Jarkko Sakkinen (3):
tpm_crb: map locality registers
tpm_crb: encapsulate crb_wait_for_reg_32
tpm_crb: request and relinquish locality 0
drivers/char/tpm/tpm_crb.c | 152 ++++++++++++++++++++++++++++-----------------
1 file changed, 96 insertions(+), 56 deletions(-)
--
2.7.4
In order to allow to use locality 0, expand the data structure to expose
all of the CRB registers. The address is calculated from the control
area address in order to retain backwards compatibility to ACPI start
based hardware (pre-Skylake).
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
drivers/char/tpm/tpm_crb.c | 82 ++++++++++++++++++++++++++++------------------
1 file changed, 50 insertions(+), 32 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 65040d7..3d021e6 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -52,18 +52,26 @@ enum crb_cancel {
CRB_CANCEL_INVOKE = BIT(0),
};
-struct crb_control_area {
- u32 req;
- u32 sts;
- u32 cancel;
- u32 start;
- u32 int_enable;
- u32 int_sts;
- u32 cmd_size;
- u32 cmd_pa_low;
- u32 cmd_pa_high;
- u32 rsp_size;
- u64 rsp_pa;
+struct crb_regs {
+ u32 loc_state;
+ u32 reserved1;
+ u32 loc_ctrl;
+ u32 loc_sts;
+ u8 reserved2[32];
+ u64 intf_id;
+ u64 ctrl_ext;
+ /* the control area */
+ u32 ctrl_req;
+ u32 ctrl_sts;
+ u32 ctrl_cancel;
+ u32 ctrl_start;
+ u32 ctrl_int_enable;
+ u32 ctrl_int_sts;
+ u32 ctrl_cmd_size;
+ u32 ctrl_cmd_pa_low;
+ u32 ctrl_cmd_pa_high;
+ u32 ctrl_rsp_size;
+ u64 ctrl_rsp_pa;
} __packed;
enum crb_status {
@@ -78,7 +86,7 @@ enum crb_flags {
struct crb_priv {
unsigned int flags;
void __iomem *iobase;
- struct crb_control_area __iomem *cca;
+ struct crb_regs __iomem *regs;
u8 __iomem *cmd;
u8 __iomem *rsp;
u32 cmd_size;
@@ -104,7 +112,7 @@ static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
if (priv->flags & CRB_FL_ACPI_START)
return 0;
- iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
+ iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->regs->ctrl_req);
/* we don't really care when this settles */
return 0;
@@ -128,16 +136,18 @@ static int __maybe_unused crb_cmd_ready(struct device *dev,
struct crb_priv *priv)
{
ktime_t stop, start;
+ u32 req;
if (priv->flags & CRB_FL_ACPI_START)
return 0;
- iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
+ iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->regs->ctrl_req);
start = ktime_get();
stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
do {
- if (!(ioread32(&priv->cca->req) & CRB_CTRL_REQ_CMD_READY)) {
+ req = ioread32(&priv->regs->ctrl_req);
+ if (!(req & CRB_CTRL_REQ_CMD_READY)) {
dev_dbg(dev, "cmdReady in %lld usecs\n",
ktime_to_us(ktime_sub(ktime_get(), start)));
return 0;
@@ -145,7 +155,7 @@ static int __maybe_unused crb_cmd_ready(struct device *dev,
usleep_range(50, 100);
} while (ktime_before(ktime_get(), stop));
- if (ioread32(&priv->cca->req) & CRB_CTRL_REQ_CMD_READY) {
+ if (ioread32(&priv->regs->ctrl_req) & CRB_CTRL_REQ_CMD_READY) {
dev_warn(dev, "cmdReady timed out\n");
return -ETIME;
}
@@ -158,7 +168,7 @@ static u8 crb_status(struct tpm_chip *chip)
struct crb_priv *priv = dev_get_drvdata(&chip->dev);
u8 sts = 0;
- if ((ioread32(&priv->cca->start) & CRB_START_INVOKE) !=
+ if ((ioread32(&priv->regs->ctrl_start) & CRB_START_INVOKE) !=
CRB_START_INVOKE)
sts |= CRB_DRV_STS_COMPLETE;
@@ -174,7 +184,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_CTRL_STS_ERROR)
+ if (ioread32(&priv->regs->ctrl_sts) & CRB_CTRL_STS_ERROR)
return -EIO;
memcpy_fromio(buf, priv->rsp, 6);
@@ -213,7 +223,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
/* Zero the cancel register so that the next command will not get
* canceled.
*/
- iowrite32(0, &priv->cca->cancel);
+ iowrite32(0, &priv->regs->ctrl_cancel);
if (len > priv->cmd_size) {
dev_err(&chip->dev, "invalid command count value %zd %d\n",
@@ -227,7 +237,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
wmb();
if (priv->flags & CRB_FL_CRB_START)
- iowrite32(CRB_START_INVOKE, &priv->cca->start);
+ iowrite32(CRB_START_INVOKE, &priv->regs->ctrl_start);
if (priv->flags & CRB_FL_ACPI_START)
rc = crb_do_acpi_start(chip);
@@ -239,7 +249,7 @@ static void crb_cancel(struct tpm_chip *chip)
{
struct crb_priv *priv = dev_get_drvdata(&chip->dev);
- iowrite32(CRB_CANCEL_INVOKE, &priv->cca->cancel);
+ iowrite32(CRB_CANCEL_INVOKE, &priv->regs->ctrl_cancel);
if ((priv->flags & CRB_FL_ACPI_START) && crb_do_acpi_start(chip))
dev_err(&chip->dev, "ACPI Start failed\n");
@@ -248,7 +258,7 @@ static void crb_cancel(struct tpm_chip *chip)
static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
{
struct crb_priv *priv = dev_get_drvdata(&chip->dev);
- u32 cancel = ioread32(&priv->cca->cancel);
+ u32 cancel = ioread32(&priv->regs->ctrl_cancel);
return (cancel & CRB_CANCEL_INVOKE) == CRB_CANCEL_INVOKE;
}
@@ -302,6 +312,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
struct list_head resources;
struct resource io_res;
struct device *dev = &device->dev;
+ void *ctrl;
u32 pa_high, pa_low;
u64 cmd_pa;
u32 cmd_size;
@@ -325,10 +336,17 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
if (IS_ERR(priv->iobase))
return PTR_ERR(priv->iobase);
- priv->cca = crb_map_res(dev, priv, &io_res, buf->control_address,
- sizeof(struct crb_control_area));
- if (IS_ERR(priv->cca))
- return PTR_ERR(priv->cca);
+ ctrl = crb_map_res(dev, priv, &io_res, buf->control_address,
+ sizeof(struct crb_regs) -
+ offsetof(struct crb_regs, ctrl_req));
+ if (IS_ERR(ctrl))
+ return PTR_ERR(ctrl);
+
+ /* The control area always overrlaps IO memory mapped from the ACPI
+ * object with CRB start only devices. Thus, this is perfectly safe.
+ */
+ priv->regs = (void *)((unsigned long)ctrl -
+ offsetof(struct crb_regs, ctrl_req));
/*
* PTT HW bug w/a: wake up the device to access
@@ -338,10 +356,10 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
if (ret)
return ret;
- pa_high = ioread32(&priv->cca->cmd_pa_high);
- pa_low = ioread32(&priv->cca->cmd_pa_low);
+ pa_high = ioread32(&priv->regs->ctrl_cmd_pa_high);
+ pa_low = ioread32(&priv->regs->ctrl_cmd_pa_low);
cmd_pa = ((u64)pa_high << 32) | pa_low;
- cmd_size = ioread32(&priv->cca->cmd_size);
+ cmd_size = ioread32(&priv->regs->ctrl_cmd_size);
dev_dbg(dev, "cmd_hi = %X cmd_low = %X cmd_size %X\n",
pa_high, pa_low, cmd_size);
@@ -352,9 +370,9 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
goto out;
}
- memcpy_fromio(&rsp_pa, &priv->cca->rsp_pa, 8);
+ memcpy_fromio(&rsp_pa, &priv->regs->ctrl_rsp_pa, 8);
rsp_pa = le64_to_cpu(rsp_pa);
- rsp_size = ioread32(&priv->cca->rsp_size);
+ rsp_size = ioread32(&priv->regs->ctrl_rsp_size);
if (cmd_pa != rsp_pa) {
priv->rsp = crb_map_res(dev, priv, &io_res, rsp_pa, rsp_size);
--
2.7.4
In order to provide access to locality registers, this commits adds
mapping of the head of the CRB registers, which are located right
before the control area.
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
drivers/char/tpm/tpm_crb.c | 86 +++++++++++++++++++++++++++++-----------------
1 file changed, 54 insertions(+), 32 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 65040d7..c34318b 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -52,18 +52,28 @@ enum crb_cancel {
CRB_CANCEL_INVOKE = BIT(0),
};
-struct crb_control_area {
- u32 req;
- u32 sts;
- u32 cancel;
- u32 start;
- u32 int_enable;
- u32 int_sts;
- u32 cmd_size;
- u32 cmd_pa_low;
- u32 cmd_pa_high;
- u32 rsp_size;
- u64 rsp_pa;
+struct crb_regs_head {
+ u32 loc_state;
+ u32 reserved1;
+ u32 loc_ctrl;
+ u32 loc_sts;
+ u8 reserved2[32];
+ u64 intf_id;
+ u64 ctrl_ext;
+} __packed;
+
+struct crb_regs_tail {
+ u32 ctrl_req;
+ u32 ctrl_sts;
+ u32 ctrl_cancel;
+ u32 ctrl_start;
+ u32 ctrl_int_enable;
+ u32 ctrl_int_sts;
+ u32 ctrl_cmd_size;
+ u32 ctrl_cmd_pa_low;
+ u32 ctrl_cmd_pa_high;
+ u32 ctrl_rsp_size;
+ u64 ctrl_rsp_pa;
} __packed;
enum crb_status {
@@ -78,7 +88,8 @@ enum crb_flags {
struct crb_priv {
unsigned int flags;
void __iomem *iobase;
- struct crb_control_area __iomem *cca;
+ struct crb_regs_head __iomem *regs_h;
+ struct crb_regs_tail __iomem *regs_t;
u8 __iomem *cmd;
u8 __iomem *rsp;
u32 cmd_size;
@@ -104,7 +115,7 @@ static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
if (priv->flags & CRB_FL_ACPI_START)
return 0;
- iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
+ iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->regs_t->ctrl_req);
/* we don't really care when this settles */
return 0;
@@ -128,16 +139,18 @@ static int __maybe_unused crb_cmd_ready(struct device *dev,
struct crb_priv *priv)
{
ktime_t stop, start;
+ u32 req;
if (priv->flags & CRB_FL_ACPI_START)
return 0;
- iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
+ iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->regs_t->ctrl_req);
start = ktime_get();
stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
do {
- if (!(ioread32(&priv->cca->req) & CRB_CTRL_REQ_CMD_READY)) {
+ req = ioread32(&priv->regs_t->ctrl_req);
+ if (!(req & CRB_CTRL_REQ_CMD_READY)) {
dev_dbg(dev, "cmdReady in %lld usecs\n",
ktime_to_us(ktime_sub(ktime_get(), start)));
return 0;
@@ -145,7 +158,7 @@ static int __maybe_unused crb_cmd_ready(struct device *dev,
usleep_range(50, 100);
} while (ktime_before(ktime_get(), stop));
- if (ioread32(&priv->cca->req) & CRB_CTRL_REQ_CMD_READY) {
+ if (ioread32(&priv->regs_t->ctrl_req) & CRB_CTRL_REQ_CMD_READY) {
dev_warn(dev, "cmdReady timed out\n");
return -ETIME;
}
@@ -158,7 +171,7 @@ static u8 crb_status(struct tpm_chip *chip)
struct crb_priv *priv = dev_get_drvdata(&chip->dev);
u8 sts = 0;
- if ((ioread32(&priv->cca->start) & CRB_START_INVOKE) !=
+ if ((ioread32(&priv->regs_t->ctrl_start) & CRB_START_INVOKE) !=
CRB_START_INVOKE)
sts |= CRB_DRV_STS_COMPLETE;
@@ -174,7 +187,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_CTRL_STS_ERROR)
+ if (ioread32(&priv->regs_t->ctrl_sts) & CRB_CTRL_STS_ERROR)
return -EIO;
memcpy_fromio(buf, priv->rsp, 6);
@@ -213,7 +226,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
/* Zero the cancel register so that the next command will not get
* canceled.
*/
- iowrite32(0, &priv->cca->cancel);
+ iowrite32(0, &priv->regs_t->ctrl_cancel);
if (len > priv->cmd_size) {
dev_err(&chip->dev, "invalid command count value %zd %d\n",
@@ -227,7 +240,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
wmb();
if (priv->flags & CRB_FL_CRB_START)
- iowrite32(CRB_START_INVOKE, &priv->cca->start);
+ iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
if (priv->flags & CRB_FL_ACPI_START)
rc = crb_do_acpi_start(chip);
@@ -239,7 +252,7 @@ static void crb_cancel(struct tpm_chip *chip)
{
struct crb_priv *priv = dev_get_drvdata(&chip->dev);
- iowrite32(CRB_CANCEL_INVOKE, &priv->cca->cancel);
+ iowrite32(CRB_CANCEL_INVOKE, &priv->regs_t->ctrl_cancel);
if ((priv->flags & CRB_FL_ACPI_START) && crb_do_acpi_start(chip))
dev_err(&chip->dev, "ACPI Start failed\n");
@@ -248,7 +261,7 @@ static void crb_cancel(struct tpm_chip *chip)
static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
{
struct crb_priv *priv = dev_get_drvdata(&chip->dev);
- u32 cancel = ioread32(&priv->cca->cancel);
+ u32 cancel = ioread32(&priv->regs_t->ctrl_cancel);
return (cancel & CRB_CANCEL_INVOKE) == CRB_CANCEL_INVOKE;
}
@@ -290,6 +303,8 @@ static void __iomem *crb_map_res(struct device *dev, struct crb_priv *priv,
if (start != new_res.start)
return (void __iomem *) ERR_PTR(-EINVAL);
+ dev_dbg(dev, "%pr %pr", io_res, &new_res);
+
if (!resource_contains(io_res, &new_res))
return devm_ioremap_resource(dev, &new_res);
@@ -325,10 +340,17 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
if (IS_ERR(priv->iobase))
return PTR_ERR(priv->iobase);
- priv->cca = crb_map_res(dev, priv, &io_res, buf->control_address,
- sizeof(struct crb_control_area));
- if (IS_ERR(priv->cca))
- return PTR_ERR(priv->cca);
+ if (priv->flags & CRB_FL_CRB_START) {
+ priv->regs_h = crb_map_res(dev, priv, &io_res, io_res.start,
+ sizeof(struct crb_regs_head));
+ if (IS_ERR(priv->regs_h))
+ return PTR_ERR(priv->regs_h);
+ }
+
+ priv->regs_t = crb_map_res(dev, priv, &io_res, buf->control_address,
+ sizeof(struct crb_regs_tail));
+ if (IS_ERR(priv->regs_t))
+ return PTR_ERR(priv->regs_t);
/*
* PTT HW bug w/a: wake up the device to access
@@ -338,10 +360,10 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
if (ret)
return ret;
- pa_high = ioread32(&priv->cca->cmd_pa_high);
- pa_low = ioread32(&priv->cca->cmd_pa_low);
+ pa_high = ioread32(&priv->regs_t->ctrl_cmd_pa_high);
+ pa_low = ioread32(&priv->regs_t->ctrl_cmd_pa_low);
cmd_pa = ((u64)pa_high << 32) | pa_low;
- cmd_size = ioread32(&priv->cca->cmd_size);
+ cmd_size = ioread32(&priv->regs_t->ctrl_cmd_size);
dev_dbg(dev, "cmd_hi = %X cmd_low = %X cmd_size %X\n",
pa_high, pa_low, cmd_size);
@@ -352,9 +374,9 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
goto out;
}
- memcpy_fromio(&rsp_pa, &priv->cca->rsp_pa, 8);
+ memcpy_fromio(&rsp_pa, &priv->regs_t->ctrl_rsp_pa, 8);
rsp_pa = le64_to_cpu(rsp_pa);
- rsp_size = ioread32(&priv->cca->rsp_size);
+ rsp_size = ioread32(&priv->regs_t->ctrl_rsp_size);
if (cmd_pa != rsp_pa) {
priv->rsp = crb_map_res(dev, priv, &io_res, rsp_pa, rsp_size);
--
2.7.4
Encapsulated crb_wait_for_reg32() so that state changes in other CRB
registers than TPM_CRB_CTRL_REQ_x can be waited.
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
drivers/char/tpm/tpm_crb.c | 40 +++++++++++++++++++++++-----------------
1 file changed, 23 insertions(+), 17 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index c34318b..45f53c2 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -121,6 +121,25 @@ static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
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
*
@@ -138,27 +157,14 @@ static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
static int __maybe_unused crb_cmd_ready(struct device *dev,
struct crb_priv *priv)
{
- ktime_t stop, start;
- u32 req;
-
if (priv->flags & CRB_FL_ACPI_START)
return 0;
iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->regs_t->ctrl_req);
-
- start = ktime_get();
- stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
- do {
- req = ioread32(&priv->regs_t->ctrl_req);
- if (!(req & CRB_CTRL_REQ_CMD_READY)) {
- dev_dbg(dev, "cmdReady in %lld usecs\n",
- ktime_to_us(ktime_sub(ktime_get(), start)));
- return 0;
- }
- usleep_range(50, 100);
- } while (ktime_before(ktime_get(), stop));
-
- if (ioread32(&priv->regs_t->ctrl_req) & CRB_CTRL_REQ_CMD_READY) {
+ if (!crb_wait_for_reg_32(&priv->regs_t->ctrl_req,
+ CRB_CTRL_REQ_CMD_READY /* mask */,
+ 0, /* value */
+ TPM2_TIMEOUT_C)) {
dev_warn(dev, "cmdReady timed out\n");
return -ETIME;
}
--
2.7.4
Request and relinquish locality for the driver use in order to be
a better citizen in a multi locality environment like TXT. The
locality is requested and relinquished as part of going into and
waking up from idle.
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
drivers/char/tpm/tpm_crb.c | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 45f53c2..69debe1 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -34,6 +34,15 @@ enum crb_defaults {
CRB_ACPI_START_INDEX = 1,
};
+enum crb_loc_ctrl {
+ CRB_LOC_CTRL_REQUEST_ACCESS = BIT(0),
+ CRB_LOC_CTRL_RELINQUISH = BIT(1),
+};
+
+enum crb_loc_state {
+ CRB_LOC_STATE_LOC_ASSIGNED = BIT(1),
+};
+
enum crb_ctrl_req {
CRB_CTRL_REQ_CMD_READY = BIT(0),
CRB_CTRL_REQ_GO_IDLE = BIT(1),
@@ -101,12 +110,8 @@ struct crb_priv {
* @dev: crb device
* @priv: crb private data
*
- * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
- * The device should respond within TIMEOUT_C by clearing the bit.
- * Anyhow, we do not wait here as a consequent CMD_READY request
- * will be handled correctly even if idle was not completed.
- *
- * The function does nothing for devices with ACPI-start method.
+ * Put device to the idle state and relinquish locality. The function does
+ * nothing for devices with the ACPI-start method.
*
* Return: 0 always
*/
@@ -115,6 +120,7 @@ static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
if (priv->flags & CRB_FL_ACPI_START)
return 0;
+ iowrite32(CRB_LOC_CTRL_RELINQUISH, &priv->regs_h->loc_ctrl);
iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->regs_t->ctrl_req);
/* we don't really care when this settles */
@@ -146,11 +152,8 @@ static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
* @dev: crb device
* @priv: crb private data
*
- * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
- * and poll till the device acknowledge it by clearing the bit.
- * The device should respond within TIMEOUT_C.
- *
- * The function does nothing for devices with ACPI-start method
+ * Try to wake up the device and request locality. The function does nothing
+ * for devices with the ACPI-start method.
*
* Return: 0 on success -ETIME on timeout;
*/
@@ -165,7 +168,16 @@ static int __maybe_unused crb_cmd_ready(struct device *dev,
CRB_CTRL_REQ_CMD_READY /* mask */,
0, /* value */
TPM2_TIMEOUT_C)) {
- dev_warn(dev, "cmdReady timed out\n");
+ dev_warn(dev, "TPM_CRB_CTRL_REQ_x.cmdReady timed out\n");
+ return -ETIME;
+ }
+
+ iowrite32(CRB_LOC_CTRL_REQUEST_ACCESS, &priv->regs_h->loc_ctrl);
+ if (!crb_wait_for_reg_32(&priv->regs_h->loc_state,
+ CRB_LOC_STATE_LOC_ASSIGNED, /* mask */
+ CRB_LOC_STATE_LOC_ASSIGNED, /* value */
+ TPM2_TIMEOUT_C)) {
+ dev_warn(dev, "TPM_LOC_STATE_x.requestAccess timed out\n");
return -ETIME;
}
--
2.7.4
> Encapsulated crb_wait_for_reg32() so that state changes in other CRB registers
> than TPM_CRB_CTRL_REQ_x can be waited.
>
> Signed-off-by: Jarkko Sakkinen <[email protected]>
> ---
> drivers/char/tpm/tpm_crb.c | 40 +++++++++++++++++++++++-----------------
> 1 file changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c index
> c34318b..45f53c2 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -121,6 +121,25 @@ static int __maybe_unused crb_go_idle(struct device
> *dev, struct crb_priv *priv)
> return 0;
> }
>
> +static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
> + unsigned long timeout)
This is a boiler plate register polling function I would call it _poll_ rather the _wait_
> +{
> + ktime_t start;
> + ktime_t stop;
> +
> + start = ktime_get();
> + stop = ktime_add(start, ms_to_ktime(timeout));
> +
> + do {
> + if ((ioread32(reg) & mask) == value)
I prefer the register value is synced to variable, this inlining is harder to add adhoc debug prints.
Also you removed the debug print out that I know when this settled which is important for catching bugs.
> + return true;
> +
> + usleep_range(50, 100);
How do you know this is correct sleep time, I've tuned that for power gating I'm not sure you this fits also for locality.
> + } while (ktime_before(ktime_get(), stop));
> +
> + return false;
> +}
> +
> /**
> * crb_cmd_ready - request tpm crb device to enter ready state
> *
> @@ -138,27 +157,14 @@ static int __maybe_unused crb_go_idle(struct device
> *dev, struct crb_priv *priv) static int __maybe_unused crb_cmd_ready(struct
> device *dev,
> struct crb_priv *priv)
> {
> - ktime_t stop, start;
> - u32 req;
> -
> if (priv->flags & CRB_FL_ACPI_START)
> return 0;
>
> iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->regs_t->ctrl_req);
> -
> - start = ktime_get();
> - stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
> - do {
> - req = ioread32(&priv->regs_t->ctrl_req);
> - if (!(req & CRB_CTRL_REQ_CMD_READY)) {
> - dev_dbg(dev, "cmdReady in %lld usecs\n",
> - ktime_to_us(ktime_sub(ktime_get(), start)));
> - return 0;
> - }
> - usleep_range(50, 100);
> - } while (ktime_before(ktime_get(), stop));
> -
> - if (ioread32(&priv->regs_t->ctrl_req) & CRB_CTRL_REQ_CMD_READY) {
> + if (!crb_wait_for_reg_32(&priv->regs_t->ctrl_req,
> + CRB_CTRL_REQ_CMD_READY /* mask */,
> + 0, /* value */
> + TPM2_TIMEOUT_C)) {
> dev_warn(dev, "cmdReady timed out\n");
> return -ETIME;
> }
> --
> 2.7.4
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> tpmdd-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
On Tue, Oct 11, 2016 at 12:23:04PM +0300, Jarkko Sakkinen wrote:
> In order to provide access to locality registers, this commits adds
> mapping of the head of the CRB registers, which are located right
> before the control area.
>
> Signed-off-by: Jarkko Sakkinen <[email protected]>
> ---
> drivers/char/tpm/tpm_crb.c | 86 +++++++++++++++++++++++++++++-----------------
> 1 file changed, 54 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 65040d7..c34318b 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -52,18 +52,28 @@ enum crb_cancel {
> CRB_CANCEL_INVOKE = BIT(0),
> };
>
> -struct crb_control_area {
> - u32 req;
> - u32 sts;
> - u32 cancel;
> - u32 start;
> - u32 int_enable;
> - u32 int_sts;
> - u32 cmd_size;
> - u32 cmd_pa_low;
> - u32 cmd_pa_high;
> - u32 rsp_size;
> - u64 rsp_pa;
> +struct crb_regs_head {
> + u32 loc_state;
> + u32 reserved1;
> + u32 loc_ctrl;
> + u32 loc_sts;
> + u8 reserved2[32];
> + u64 intf_id;
> + u64 ctrl_ext;
> +} __packed;
> +
> +struct crb_regs_tail {
> + u32 ctrl_req;
> + u32 ctrl_sts;
> + u32 ctrl_cancel;
> + u32 ctrl_start;
> + u32 ctrl_int_enable;
> + u32 ctrl_int_sts;
> + u32 ctrl_cmd_size;
> + u32 ctrl_cmd_pa_low;
> + u32 ctrl_cmd_pa_high;
> + u32 ctrl_rsp_size;
> + u64 ctrl_rsp_pa;
> } __packed;
>
> enum crb_status {
> @@ -78,7 +88,8 @@ enum crb_flags {
> struct crb_priv {
> unsigned int flags;
> void __iomem *iobase;
> - struct crb_control_area __iomem *cca;
> + struct crb_regs_head __iomem *regs_h;
> + struct crb_regs_tail __iomem *regs_t;
> u8 __iomem *cmd;
> u8 __iomem *rsp;
> u32 cmd_size;
> @@ -104,7 +115,7 @@ static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
> if (priv->flags & CRB_FL_ACPI_START)
> return 0;
>
> - iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
> + iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->regs_t->ctrl_req);
> /* we don't really care when this settles */
>
> return 0;
> @@ -128,16 +139,18 @@ static int __maybe_unused crb_cmd_ready(struct device *dev,
> struct crb_priv *priv)
> {
> ktime_t stop, start;
> + u32 req;
>
> if (priv->flags & CRB_FL_ACPI_START)
> return 0;
>
> - iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
> + iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->regs_t->ctrl_req);
>
> start = ktime_get();
> stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
> do {
> - if (!(ioread32(&priv->cca->req) & CRB_CTRL_REQ_CMD_READY)) {
> + req = ioread32(&priv->regs_t->ctrl_req);
> + if (!(req & CRB_CTRL_REQ_CMD_READY)) {
> dev_dbg(dev, "cmdReady in %lld usecs\n",
> ktime_to_us(ktime_sub(ktime_get(), start)));
> return 0;
> @@ -145,7 +158,7 @@ static int __maybe_unused crb_cmd_ready(struct device *dev,
> usleep_range(50, 100);
> } while (ktime_before(ktime_get(), stop));
>
> - if (ioread32(&priv->cca->req) & CRB_CTRL_REQ_CMD_READY) {
> + if (ioread32(&priv->regs_t->ctrl_req) & CRB_CTRL_REQ_CMD_READY) {
> dev_warn(dev, "cmdReady timed out\n");
> return -ETIME;
> }
> @@ -158,7 +171,7 @@ static u8 crb_status(struct tpm_chip *chip)
> struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> u8 sts = 0;
>
> - if ((ioread32(&priv->cca->start) & CRB_START_INVOKE) !=
> + if ((ioread32(&priv->regs_t->ctrl_start) & CRB_START_INVOKE) !=
> CRB_START_INVOKE)
> sts |= CRB_DRV_STS_COMPLETE;
>
> @@ -174,7 +187,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_CTRL_STS_ERROR)
> + if (ioread32(&priv->regs_t->ctrl_sts) & CRB_CTRL_STS_ERROR)
> return -EIO;
>
> memcpy_fromio(buf, priv->rsp, 6);
> @@ -213,7 +226,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
> /* Zero the cancel register so that the next command will not get
> * canceled.
> */
> - iowrite32(0, &priv->cca->cancel);
> + iowrite32(0, &priv->regs_t->ctrl_cancel);
>
> if (len > priv->cmd_size) {
> dev_err(&chip->dev, "invalid command count value %zd %d\n",
> @@ -227,7 +240,7 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
> wmb();
>
> if (priv->flags & CRB_FL_CRB_START)
> - iowrite32(CRB_START_INVOKE, &priv->cca->start);
> + iowrite32(CRB_START_INVOKE, &priv->regs_t->ctrl_start);
>
> if (priv->flags & CRB_FL_ACPI_START)
> rc = crb_do_acpi_start(chip);
> @@ -239,7 +252,7 @@ static void crb_cancel(struct tpm_chip *chip)
> {
> struct crb_priv *priv = dev_get_drvdata(&chip->dev);
>
> - iowrite32(CRB_CANCEL_INVOKE, &priv->cca->cancel);
> + iowrite32(CRB_CANCEL_INVOKE, &priv->regs_t->ctrl_cancel);
>
> if ((priv->flags & CRB_FL_ACPI_START) && crb_do_acpi_start(chip))
> dev_err(&chip->dev, "ACPI Start failed\n");
> @@ -248,7 +261,7 @@ static void crb_cancel(struct tpm_chip *chip)
> static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
> {
> struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> - u32 cancel = ioread32(&priv->cca->cancel);
> + u32 cancel = ioread32(&priv->regs_t->ctrl_cancel);
>
> return (cancel & CRB_CANCEL_INVOKE) == CRB_CANCEL_INVOKE;
> }
> @@ -290,6 +303,8 @@ static void __iomem *crb_map_res(struct device *dev, struct crb_priv *priv,
> if (start != new_res.start)
> return (void __iomem *) ERR_PTR(-EINVAL);
>
> + dev_dbg(dev, "%pr %pr", io_res, &new_res);
> +
> if (!resource_contains(io_res, &new_res))
> return devm_ioremap_resource(dev, &new_res);
>
> @@ -325,10 +340,17 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
> if (IS_ERR(priv->iobase))
> return PTR_ERR(priv->iobase);
>
> - priv->cca = crb_map_res(dev, priv, &io_res, buf->control_address,
> - sizeof(struct crb_control_area));
> - if (IS_ERR(priv->cca))
> - return PTR_ERR(priv->cca);
> + if (priv->flags & CRB_FL_CRB_START) {
Oops. Should be check for CRB_FL_ACPI_START.
Because we anyway *always* set CRB_FL_CRB_START for everything
how about ripping of CRB_FL_CRB_START flag completely in order
to not cause mistakes like this in future?
/Jarkko
> + priv->regs_h = crb_map_res(dev, priv, &io_res, io_res.start,
> + sizeof(struct crb_regs_head));
> + if (IS_ERR(priv->regs_h))
> + return PTR_ERR(priv->regs_h);
> + }
> +
> + priv->regs_t = crb_map_res(dev, priv, &io_res, buf->control_address,
> + sizeof(struct crb_regs_tail));
> + if (IS_ERR(priv->regs_t))
> + return PTR_ERR(priv->regs_t);
On Tue, Oct 11, 2016 at 10:21:03AM +0000, Winkler, Tomas wrote:
> > Encapsulated crb_wait_for_reg32() so that state changes in other CRB registers
> > than TPM_CRB_CTRL_REQ_x can be waited.
> >
> > Signed-off-by: Jarkko Sakkinen <[email protected]>
> > ---
> > drivers/char/tpm/tpm_crb.c | 40 +++++++++++++++++++++++-----------------
> > 1 file changed, 23 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c index
> > c34318b..45f53c2 100644
> > --- a/drivers/char/tpm/tpm_crb.c
> > +++ b/drivers/char/tpm/tpm_crb.c
> > @@ -121,6 +121,25 @@ static int __maybe_unused crb_go_idle(struct device
> > *dev, struct crb_priv *priv)
> > return 0;
> > }
> >
> > +static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
> > + unsigned long timeout)
> This is a boiler plate register polling function I would call it _poll_ rather the _wait_
>
> > +{
> > + ktime_t start;
> > + ktime_t stop;
> > +
> > + start = ktime_get();
> > + stop = ktime_add(start, ms_to_ktime(timeout));
> > +
> > + do {
> > + if ((ioread32(reg) & mask) == value)
> I prefer the register value is synced to variable, this inlining is
> harder to add adhoc debug prints. Also you removed the debug print
> out that I know when this settled which is important for catching
> bugs.
I can add it but can you just briefly explain why the warning
is not enough?
> > + return true;
> > +
> > + usleep_range(50, 100);
> How do you know this is correct sleep time, I've tuned that for power
> gating I'm not sure you this fits also for locality.
Does it matter as long as it is less than for the timeout?
/Jarkko
On Tue, Oct 11, 2016 at 12:23:04PM +0300, Jarkko Sakkinen wrote:
> In order to provide access to locality registers, this commits adds
> mapping of the head of the CRB registers, which are located right
> before the control area.
I think you should squash this into the prior patch, no sense in
changing all these lines twice
But looks better to me.
> - priv->cca = crb_map_res(dev, priv, &io_res, buf->control_address,
> - sizeof(struct crb_control_area));
> - if (IS_ERR(priv->cca))
> - return PTR_ERR(priv->cca);
> + if (priv->flags & CRB_FL_CRB_START) {
> + priv->regs_h = crb_map_res(dev, priv, &io_res, io_res.start,
> + sizeof(struct crb_regs_head));
> + if (IS_ERR(priv->regs_h))
> + return PTR_ERR(priv->regs_h);
> + }
> +
> + priv->regs_t = crb_map_res(dev, priv, &io_res, buf->control_address,
> + sizeof(struct crb_regs_tail));
> + if (IS_ERR(priv->regs_t))
> + return PTR_ERR(priv->regs_t);
So.. The ACPI IO region starts at the head area and continues to
include the control area, as one nice sane region - except for some
older stuff that puts the control area outside the ACPI IO region?
That makes a lot more sense..
Maybe chuck in a
if (priv->flags & CRB_FL_CRB_START) {
if (buf->control_address != io_res.start + sizeof(struct
crb_regs_head))
dev_warn(dev, FIRMWARE_BUG "Bad ACPI memory layout")
Jason
On Tue, Oct 11, 2016 at 11:01:43AM -0600, Jason Gunthorpe wrote:
> On Tue, Oct 11, 2016 at 12:23:04PM +0300, Jarkko Sakkinen wrote:
> > In order to provide access to locality registers, this commits adds
> > mapping of the head of the CRB registers, which are located right
> > before the control area.
>
> I think you should squash this into the prior patch, no sense in
> changing all these lines twice
>
> But looks better to me.
So... do you suggest to squash with request/reliquish locality stuff?
Just checking because this is 1/3 :)
> > - priv->cca = crb_map_res(dev, priv, &io_res, buf->control_address,
> > - sizeof(struct crb_control_area));
> > - if (IS_ERR(priv->cca))
> > - return PTR_ERR(priv->cca);
> > + if (priv->flags & CRB_FL_CRB_START) {
> > + priv->regs_h = crb_map_res(dev, priv, &io_res, io_res.start,
> > + sizeof(struct crb_regs_head));
> > + if (IS_ERR(priv->regs_h))
> > + return PTR_ERR(priv->regs_h);
> > + }
> > +
> > + priv->regs_t = crb_map_res(dev, priv, &io_res, buf->control_address,
> > + sizeof(struct crb_regs_tail));
> > + if (IS_ERR(priv->regs_t))
> > + return PTR_ERR(priv->regs_t);
>
> So.. The ACPI IO region starts at the head area and continues to
> include the control area, as one nice sane region - except for some
> older stuff that puts the control area outside the ACPI IO region?
Yes. The old hardware triggered SMM to do a DMA transfer (those that
so called ACPI start). As a workaround for some of the hardware the
driver always sets the CRB start flag also in the control area.
That's why I also propose that we replace them with a single flag.
> That makes a lot more sense..
>
> Maybe chuck in a
>
> if (priv->flags & CRB_FL_CRB_START) {
> if (buf->control_address != io_res.start + sizeof(struct
> crb_regs_head))
> dev_warn(dev, FIRMWARE_BUG "Bad ACPI memory layout")
>
> Jason
As a sanity check this would probably make sense.
/Jarkko
On Tue, Oct 11, 2016 at 09:21:00PM +0300, Jarkko Sakkinen wrote:
> On Tue, Oct 11, 2016 at 11:01:43AM -0600, Jason Gunthorpe wrote:
> > On Tue, Oct 11, 2016 at 12:23:04PM +0300, Jarkko Sakkinen wrote:
> > > In order to provide access to locality registers, this commits adds
> > > mapping of the head of the CRB registers, which are located right
> > > before the control area.
> >
> > I think you should squash this into the prior patch, no sense in
> > changing all these lines twice
> >
> > But looks better to me.
>
> So... do you suggest to squash with request/reliquish locality stuff?
> Just checking because this is 1/3 :)
Hm, maybe my mailer went wrong, but I saw the original patch right
before this one.. Never mind
Jason
On Tue, Oct 11, 2016 at 12:29:22PM -0600, Jason Gunthorpe wrote:
> On Tue, Oct 11, 2016 at 09:21:00PM +0300, Jarkko Sakkinen wrote:
> > On Tue, Oct 11, 2016 at 11:01:43AM -0600, Jason Gunthorpe wrote:
> > > On Tue, Oct 11, 2016 at 12:23:04PM +0300, Jarkko Sakkinen wrote:
> > > > In order to provide access to locality registers, this commits adds
> > > > mapping of the head of the CRB registers, which are located right
> > > > before the control area.
> > >
> > > I think you should squash this into the prior patch, no sense in
> > > changing all these lines twice
> > >
> > > But looks better to me.
> >
> > So... do you suggest to squash with request/reliquish locality stuff?
> > Just checking because this is 1/3 :)
>
> Hm, maybe my mailer went wrong, but I saw the original patch right
> before this one.. Never mind
I did format-patch -o to the same folder where I had the first series so
I posted the RFC patches by accident. Sorry. Should always do --dry-run.
> Jason
/Jarkko