* [PATCH 0/2] tpm: fix locality and power saving handling @ 2018-01-28 7:50 ` Tomas Winkler 0 siblings, 0 replies; 24+ messages in thread From: Tomas Winkler @ 2018-01-28 7:50 UTC (permalink / raw) To: Jarkko Sakkinen, Jason Gunthorpe Cc: Alexander Usyskin, linux-integrity, linux-security-module, linux-kernel, Tomas Winkler Devices that support locality has failed to transmit due to reserved order of locality request and cmdReady/goIdle handshake. The cmdReady/goIdle should be performed on the requested locality. The first patch corrects the locality and power-save order, adds required polling for completion of goIdle and locality relinquish. The second patch removes cmdRead/goIdle handshake from runtime_pm as it's rather a part of TPM2 protocol then true device power management, and on device it's no longer optional. Tomas Winkler (2): tpm: cmd_ready command can be issued only after granting locality tpm: separate cmd_ready/go_idle from runtime_pm drivers/char/tpm/tpm-interface.c | 21 ++++-- drivers/char/tpm/tpm_crb.c | 157 +++++++++++++++++++++++---------------- drivers/char/tpm/tpm_tis_core.c | 4 +- include/linux/tpm.h | 4 +- 4 files changed, 115 insertions(+), 71 deletions(-) -- 2.14.3 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 0/2] tpm: fix locality and power saving handling @ 2018-01-28 7:50 ` Tomas Winkler 0 siblings, 0 replies; 24+ messages in thread From: Tomas Winkler @ 2018-01-28 7:50 UTC (permalink / raw) To: linux-security-module Devices that support locality has failed to transmit due to reserved order of locality request and cmdReady/goIdle handshake. The cmdReady/goIdle should be performed on the requested locality. The first patch corrects the locality and power-save order, adds required polling for completion of goIdle and locality relinquish. The second patch removes cmdRead/goIdle handshake from runtime_pm as it's rather a part of TPM2 protocol then true device power management, and on device it's no longer optional. Tomas Winkler (2): tpm: cmd_ready command can be issued only after granting locality tpm: separate cmd_ready/go_idle from runtime_pm drivers/char/tpm/tpm-interface.c | 21 ++++-- drivers/char/tpm/tpm_crb.c | 157 +++++++++++++++++++++++---------------- drivers/char/tpm/tpm_tis_core.c | 4 +- include/linux/tpm.h | 4 +- 4 files changed, 115 insertions(+), 71 deletions(-) -- 2.14.3 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality 2018-01-28 7:50 ` Tomas Winkler @ 2018-01-28 7:51 ` Tomas Winkler -1 siblings, 0 replies; 24+ messages in thread From: Tomas Winkler @ 2018-01-28 7:51 UTC (permalink / raw) To: Jarkko Sakkinen, Jason Gunthorpe Cc: Alexander Usyskin, linux-integrity, linux-security-module, linux-kernel, Tomas Winkler 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 overrriden in back to back commands flow. The issue is only visible on devices that support multiple localities. Signed-off-by: Tomas Winkler <tomas.winkler@intel.com> --- V2: poll for locality relinquish completion drivers/char/tpm/tpm-interface.c | 14 ++--- drivers/char/tpm/tpm_crb.c | 108 +++++++++++++++++++++++++++------------ drivers/char/tpm/tpm_tis_core.c | 4 +- include/linux/tpm.h | 2 +- 4 files changed, 87 insertions(+), 41 deletions(-) diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index 76df4fbcf089..9fb3d406b078 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -422,8 +422,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); @@ -439,6 +437,9 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space, 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 +500,18 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space, rc = tpm2_commit_space(chip, space, ordinal, buf, &len); out: + if (chip->dev.parent) + pm_runtime_put_sync(chip->dev.parent); + if (need_locality && chip->ops->relinquish_locality) { - chip->ops->relinquish_locality(chip, chip->locality); + rc = chip->ops->relinquish_locality(chip, chip->locality); chip->locality = -1; } + 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 183a5f54d875..a22b12adbdfd 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.14.3 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality @ 2018-01-28 7:51 ` Tomas Winkler 0 siblings, 0 replies; 24+ messages in thread From: Tomas Winkler @ 2018-01-28 7:51 UTC (permalink / raw) To: linux-security-module 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 overrriden in back to back commands flow. The issue is only visible on devices that support multiple localities. Signed-off-by: Tomas Winkler <tomas.winkler@intel.com> --- V2: poll for locality relinquish completion drivers/char/tpm/tpm-interface.c | 14 ++--- drivers/char/tpm/tpm_crb.c | 108 +++++++++++++++++++++++++++------------ drivers/char/tpm/tpm_tis_core.c | 4 +- include/linux/tpm.h | 2 +- 4 files changed, 87 insertions(+), 41 deletions(-) diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index 76df4fbcf089..9fb3d406b078 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -422,8 +422,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); @@ -439,6 +437,9 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space, 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 +500,18 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space, rc = tpm2_commit_space(chip, space, ordinal, buf, &len); out: + if (chip->dev.parent) + pm_runtime_put_sync(chip->dev.parent); + if (need_locality && chip->ops->relinquish_locality) { - chip->ops->relinquish_locality(chip, chip->locality); + rc = chip->ops->relinquish_locality(chip, chip->locality); chip->locality = -1; } + 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 183a5f54d875..a22b12adbdfd 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.14.3 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality 2018-01-28 7:51 ` Tomas Winkler @ 2018-01-28 20:15 ` Jason Gunthorpe -1 siblings, 0 replies; 24+ messages in thread From: Jason Gunthorpe @ 2018-01-28 20:15 UTC (permalink / raw) To: Tomas Winkler Cc: Jarkko Sakkinen, Alexander Usyskin, linux-integrity, linux-security-module, linux-kernel On Sun, Jan 28, 2018 at 09:51:00AM +0200, Tomas Winkler wrote: > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index bcdd3790e94d..06639fb6ab85 100644 > +++ 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); This seems wrong.. What is the core code supposed to do if relinquish fails? Just returning an error code from transmit doesn't really do anything helpful from a broad subsytem perspective. I think if a driver can fail reliquish then it needs some kind of strategy to recover. Suggest trying the reliquish again on every next request until success, otherwise fail request locality, potentially permanently. Jason ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality @ 2018-01-28 20:15 ` Jason Gunthorpe 0 siblings, 0 replies; 24+ messages in thread From: Jason Gunthorpe @ 2018-01-28 20:15 UTC (permalink / raw) To: linux-security-module On Sun, Jan 28, 2018 at 09:51:00AM +0200, Tomas Winkler wrote: > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index bcdd3790e94d..06639fb6ab85 100644 > +++ 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); This seems wrong.. What is the core code supposed to do if relinquish fails? Just returning an error code from transmit doesn't really do anything helpful from a broad subsytem perspective. I think if a driver can fail reliquish then it needs some kind of strategy to recover. Suggest trying the reliquish again on every next request until success, otherwise fail request locality, potentially permanently. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality 2018-01-28 20:15 ` Jason Gunthorpe @ 2018-01-28 21:17 ` Winkler, Tomas -1 siblings, 0 replies; 24+ messages in thread From: Winkler, Tomas @ 2018-01-28 21:17 UTC (permalink / raw) To: Jason Gunthorpe Cc: Jarkko Sakkinen, Usyskin, Alexander, linux-integrity, linux-security-module, linux-kernel > > On Sun, Jan 28, 2018 at 09:51:00AM +0200, Tomas Winkler wrote: > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h index > > bcdd3790e94d..06639fb6ab85 100644 > > +++ 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); > > This seems wrong.. What is the core code supposed to do if relinquish fails? Not much just propage the error to the caller and leave the policy decision to it. > Just returning an error code from transmit doesn't really do anything helpful > from a broad subsytem perspective. Yes, you are right, but I'm not sure even if the subsystem is broad enough to understand the system setup, or in another direction specific enough to behave upon hw limitations. > > I think if a driver can fail reliquish then it needs some kind of strategy to > recover. Maybe some driver can and some not, but if it doesn't succeed it should return an error. > > Suggest trying the reliquish again on every next request until success, > otherwise fail request locality, potentially permanently. This is something I rather prevent because it leaves the HW in kind of undefined state ( and we should probably work on that a bit more later). As far as I've debugged the flow now, the driver just fails, and the error goes up user space caller or the internal flow is stopped. A user can reboot the system or whatever it helps in his/her particular setup. Make sense? Anyhow I will dig to it more how fatal is that relinquish failure. Thanks Tomas ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality @ 2018-01-28 21:17 ` Winkler, Tomas 0 siblings, 0 replies; 24+ messages in thread From: Winkler, Tomas @ 2018-01-28 21:17 UTC (permalink / raw) To: linux-security-module > > On Sun, Jan 28, 2018 at 09:51:00AM +0200, Tomas Winkler wrote: > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h index > > bcdd3790e94d..06639fb6ab85 100644 > > +++ 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); > > This seems wrong.. What is the core code supposed to do if relinquish fails? Not much just propage the error to the caller and leave the policy decision to it. > Just returning an error code from transmit doesn't really do anything helpful > from a broad subsytem perspective. Yes, you are right, but I'm not sure even if the subsystem is broad enough to understand the system setup, or in another direction specific enough to behave upon hw limitations. > > I think if a driver can fail reliquish then it needs some kind of strategy to > recover. Maybe some driver can and some not, but if it doesn't succeed it should return an error. > > Suggest trying the reliquish again on every next request until success, > otherwise fail request locality, potentially permanently. This is something I rather prevent because it leaves the HW in kind of undefined state ( and we should probably work on that a bit more later). As far as I've debugged the flow now, the driver just fails, and the error goes up user space caller or the internal flow is stopped. A user can reboot the system or whatever it helps in his/her particular setup. Make sense? Anyhow I will dig to it more how fatal is that relinquish failure. Thanks Tomas -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality 2018-01-28 21:17 ` Winkler, Tomas @ 2018-01-29 17:57 ` Jason Gunthorpe -1 siblings, 0 replies; 24+ messages in thread From: Jason Gunthorpe @ 2018-01-29 17:57 UTC (permalink / raw) To: Winkler, Tomas Cc: Jarkko Sakkinen, Usyskin, Alexander, linux-integrity, linux-security-module, linux-kernel On Sun, Jan 28, 2018 at 09:17:53PM +0000, Winkler, Tomas wrote: > > I think if a driver can fail reliquish then it needs some kind of strategy to > > recover. > Maybe some driver can and some not, but if it doesn't succeed it > should return an error. But you can't just leave the driver in some inconsistent state.. Every time I've audited something to do with 'add error codes to destroy/free/release' I find driver design issues.. > > Suggest trying the reliquish again on every next request until success, > > otherwise fail request locality, potentially permanently. > > This is something I rather prevent because it leaves the HW in kind of undefined state > ( and we should probably work on that a bit more later). > As far as I've debugged the flow now, the driver just fails, and the error goes up > user space caller or the internal flow is stopped. But tranmist_command will be called again - then what does the driver do? The driver needs an answer for that.. If you don't want to retry then I'd rather see request_locality permanently fail then adding a return code to release. Jason ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality @ 2018-01-29 17:57 ` Jason Gunthorpe 0 siblings, 0 replies; 24+ messages in thread From: Jason Gunthorpe @ 2018-01-29 17:57 UTC (permalink / raw) To: linux-security-module On Sun, Jan 28, 2018 at 09:17:53PM +0000, Winkler, Tomas wrote: > > I think if a driver can fail reliquish then it needs some kind of strategy to > > recover. > Maybe some driver can and some not, but if it doesn't succeed it > should return an error. But you can't just leave the driver in some inconsistent state.. Every time I've audited something to do with 'add error codes to destroy/free/release' I find driver design issues.. > > Suggest trying the reliquish again on every next request until success, > > otherwise fail request locality, potentially permanently. > > This is something I rather prevent because it leaves the HW in kind of undefined state > ( and we should probably work on that a bit more later). > As far as I've debugged the flow now, the driver just fails, and the error goes up > user space caller or the internal flow is stopped. But tranmist_command will be called again - then what does the driver do? The driver needs an answer for that.. If you don't want to retry then I'd rather see request_locality permanently fail then adding a return code to release. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality 2018-01-29 17:57 ` Jason Gunthorpe @ 2018-01-29 19:40 ` Winkler, Tomas -1 siblings, 0 replies; 24+ messages in thread From: Winkler, Tomas @ 2018-01-29 19:40 UTC (permalink / raw) To: Jason Gunthorpe Cc: Jarkko Sakkinen, Usyskin, Alexander, linux-integrity, linux-security-module, linux-kernel > On Sun, Jan 28, 2018 at 09:17:53PM +0000, Winkler, Tomas wrote: > > > > I think if a driver can fail reliquish then it needs some kind of > > > strategy to recover. > > > Maybe some driver can and some not, but if it doesn't succeed it > > should return an error. > > But you can't just leave the driver in some inconsistent state.. > > Every time I've audited something to do with 'add error codes to > destroy/free/release' I find driver design issues.. I'm sure of it, but from this particular point the driver itself is stateless, it's just reading HW state via registers. It's not going through driver state changes. > > > Suggest trying the reliquish again on every next request until > > > success, otherwise fail request locality, potentially permanently. > > > > This is something I rather prevent because it leaves the HW in kind of > > undefined state ( and we should probably work on that a bit more later). > > As far as I've debugged the flow now, the driver just fails, and the > > error goes up user space caller or the internal flow is stopped. > > But tranmist_command will be called again - then what does the driver do? > The driver needs an answer for that.. It will just fail again > > If you don't want to retry then I'd rather see request_locality permanently > fail then adding a return code to release. What do you mean exactly mean by permanently fail, My current assumption is that it will fail permanently because the HW is not responsive Or indicate error on any subsequent command, unless the hw recover somehow. Currently I'm not aware of any possibility to reset the device except rebooting the system. Thanks Tomas ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality @ 2018-01-29 19:40 ` Winkler, Tomas 0 siblings, 0 replies; 24+ messages in thread From: Winkler, Tomas @ 2018-01-29 19:40 UTC (permalink / raw) To: linux-security-module > On Sun, Jan 28, 2018 at 09:17:53PM +0000, Winkler, Tomas wrote: > > > > I think if a driver can fail reliquish then it needs some kind of > > > strategy to recover. > > > Maybe some driver can and some not, but if it doesn't succeed it > > should return an error. > > But you can't just leave the driver in some inconsistent state.. > > Every time I've audited something to do with 'add error codes to > destroy/free/release' I find driver design issues.. I'm sure of it, but from this particular point the driver itself is stateless, it's just reading HW state via registers. It's not going through driver state changes. > > > Suggest trying the reliquish again on every next request until > > > success, otherwise fail request locality, potentially permanently. > > > > This is something I rather prevent because it leaves the HW in kind of > > undefined state ( and we should probably work on that a bit more later). > > As far as I've debugged the flow now, the driver just fails, and the > > error goes up user space caller or the internal flow is stopped. > > But tranmist_command will be called again - then what does the driver do? > The driver needs an answer for that.. It will just fail again > > If you don't want to retry then I'd rather see request_locality permanently > fail then adding a return code to release. What do you mean exactly mean by permanently fail, My current assumption is that it will fail permanently because the HW is not responsive Or indicate error on any subsequent command, unless the hw recover somehow. Currently I'm not aware of any possibility to reset the device except rebooting the system. Thanks Tomas -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality 2018-01-28 21:17 ` Winkler, Tomas @ 2018-02-06 20:02 ` Jarkko Sakkinen -1 siblings, 0 replies; 24+ messages in thread From: Jarkko Sakkinen @ 2018-02-06 20:02 UTC (permalink / raw) To: Winkler, Tomas Cc: Jason Gunthorpe, Usyskin, Alexander, linux-integrity, linux-security-module, linux-kernel On Sun, Jan 28, 2018 at 09:17:53PM +0000, Winkler, Tomas wrote: > > > > > On Sun, Jan 28, 2018 at 09:51:00AM +0200, Tomas Winkler wrote: > > > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h index > > > bcdd3790e94d..06639fb6ab85 100644 > > > +++ 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); > > > > This seems wrong.. What is the core code supposed to do if relinquish fails? > > Not much just propage the error to the caller and leave the policy > decision to it. Your patch set must either cover this or keep it as void. A better idea is to print an error to klog. /Jarkko ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality @ 2018-02-06 20:02 ` Jarkko Sakkinen 0 siblings, 0 replies; 24+ messages in thread From: Jarkko Sakkinen @ 2018-02-06 20:02 UTC (permalink / raw) To: linux-security-module On Sun, Jan 28, 2018 at 09:17:53PM +0000, Winkler, Tomas wrote: > > > > > On Sun, Jan 28, 2018 at 09:51:00AM +0200, Tomas Winkler wrote: > > > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h index > > > bcdd3790e94d..06639fb6ab85 100644 > > > +++ 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); > > > > This seems wrong.. What is the core code supposed to do if relinquish fails? > > Not much just propage the error to the caller and leave the policy > decision to it. Your patch set must either cover this or keep it as void. A better idea is to print an error to klog. /Jarkko -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality 2018-02-06 20:02 ` Jarkko Sakkinen @ 2018-02-06 21:26 ` Winkler, Tomas -1 siblings, 0 replies; 24+ messages in thread From: Winkler, Tomas @ 2018-02-06 21:26 UTC (permalink / raw) To: Jarkko Sakkinen Cc: Jason Gunthorpe, Usyskin, Alexander, linux-integrity, linux-security-module, linux-kernel > > On Sun, Jan 28, 2018 at 09:17:53PM +0000, Winkler, Tomas wrote: > > > > > > > > On Sun, Jan 28, 2018 at 09:51:00AM +0200, Tomas Winkler wrote: > > > > > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h index > > > > bcdd3790e94d..06639fb6ab85 100644 > > > > +++ 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); > > > > > > This seems wrong.. What is the core code supposed to do if relinquish > fails? > > > > Not much just propage the error to the caller and leave the policy > > decision to it. > > Your patch set must either cover this or keep it as void. How the code is covering other failures in the transmit functions, how is this one different from for example request_locality failure? Why we should not propage this error up? > > A better idea is to print an error to klog. We can do that in addition. Thanks Tomas ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality @ 2018-02-06 21:26 ` Winkler, Tomas 0 siblings, 0 replies; 24+ messages in thread From: Winkler, Tomas @ 2018-02-06 21:26 UTC (permalink / raw) To: linux-security-module > > On Sun, Jan 28, 2018 at 09:17:53PM +0000, Winkler, Tomas wrote: > > > > > > > > On Sun, Jan 28, 2018 at 09:51:00AM +0200, Tomas Winkler wrote: > > > > > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h index > > > > bcdd3790e94d..06639fb6ab85 100644 > > > > +++ 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); > > > > > > This seems wrong.. What is the core code supposed to do if relinquish > fails? > > > > Not much just propage the error to the caller and leave the policy > > decision to it. > > Your patch set must either cover this or keep it as void. How the code is covering other failures in the transmit functions, how is this one different from for example request_locality failure? Why we should not propage this error up? > > A better idea is to print an error to klog. We can do that in addition. Thanks Tomas -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality 2018-02-06 21:26 ` Winkler, Tomas @ 2018-02-08 12:44 ` Jarkko Sakkinen -1 siblings, 0 replies; 24+ messages in thread From: Jarkko Sakkinen @ 2018-02-08 12:44 UTC (permalink / raw) To: Winkler, Tomas Cc: Jason Gunthorpe, Usyskin, Alexander, linux-integrity, linux-security-module, linux-kernel On Tue, Feb 06, 2018 at 09:26:15PM +0000, Winkler, Tomas wrote: > > > > On Sun, Jan 28, 2018 at 09:17:53PM +0000, Winkler, Tomas wrote: > > > > > > > > > > > On Sun, Jan 28, 2018 at 09:51:00AM +0200, Tomas Winkler wrote: > > > > > > > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h index > > > > > bcdd3790e94d..06639fb6ab85 100644 > > > > > +++ 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); > > > > > > > > This seems wrong.. What is the core code supposed to do if relinquish > > fails? > > > > > > Not much just propage the error to the caller and leave the policy > > > decision to it. > > > > Your patch set must either cover this or keep it as void. > > > How the code is covering other failures in the transmit functions, > how is this one different from for example request_locality failure? > Why we should not propage this error up? > > > > > A better idea is to print an error to klog. > We can do that in addition. I guess you are right. This can be propagated to the user space so that it knows that there is problem. To make the root more visible the klog message would make sense. /Jarkko ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality @ 2018-02-08 12:44 ` Jarkko Sakkinen 0 siblings, 0 replies; 24+ messages in thread From: Jarkko Sakkinen @ 2018-02-08 12:44 UTC (permalink / raw) To: linux-security-module On Tue, Feb 06, 2018 at 09:26:15PM +0000, Winkler, Tomas wrote: > > > > On Sun, Jan 28, 2018 at 09:17:53PM +0000, Winkler, Tomas wrote: > > > > > > > > > > > On Sun, Jan 28, 2018 at 09:51:00AM +0200, Tomas Winkler wrote: > > > > > > > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h index > > > > > bcdd3790e94d..06639fb6ab85 100644 > > > > > +++ 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); > > > > > > > > This seems wrong.. What is the core code supposed to do if relinquish > > fails? > > > > > > Not much just propage the error to the caller and leave the policy > > > decision to it. > > > > Your patch set must either cover this or keep it as void. > > > How the code is covering other failures in the transmit functions, > how is this one different from for example request_locality failure? > Why we should not propage this error up? > > > > > A better idea is to print an error to klog. > We can do that in addition. I guess you are right. This can be propagated to the user space so that it knows that there is problem. To make the root more visible the klog message would make sense. /Jarkko -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality 2018-02-08 12:44 ` Jarkko Sakkinen @ 2018-02-08 12:46 ` Winkler, Tomas -1 siblings, 0 replies; 24+ messages in thread From: Winkler, Tomas @ 2018-02-08 12:46 UTC (permalink / raw) To: Jarkko Sakkinen Cc: Jason Gunthorpe, Usyskin, Alexander, linux-integrity, linux-security-module, linux-kernel > > On Tue, Feb 06, 2018 at 09:26:15PM +0000, Winkler, Tomas wrote: > > > > > > On Sun, Jan 28, 2018 at 09:17:53PM +0000, Winkler, Tomas wrote: > > > > > > > > > > > > > > On Sun, Jan 28, 2018 at 09:51:00AM +0200, Tomas Winkler wrote: > > > > > > > > > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h index > > > > > > bcdd3790e94d..06639fb6ab85 100644 > > > > > > +++ 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); > > > > > > > > > > This seems wrong.. What is the core code supposed to do if > > > > > relinquish > > > fails? > > > > > > > > Not much just propage the error to the caller and leave the policy > > > > decision to it. > > > > > > Your patch set must either cover this or keep it as void. > > > > > > How the code is covering other failures in the transmit functions, how > > is this one different from for example request_locality failure? > > Why we should not propage this error up? > > > > > > > > A better idea is to print an error to klog. > > We can do that in addition. > > I guess you are right. This can be propagated to the user space so that it > knows that there is problem. To make the root more visible the klog > message would make sense. Thanks, will add an error message. Tomas ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality @ 2018-02-08 12:46 ` Winkler, Tomas 0 siblings, 0 replies; 24+ messages in thread From: Winkler, Tomas @ 2018-02-08 12:46 UTC (permalink / raw) To: linux-security-module > > On Tue, Feb 06, 2018 at 09:26:15PM +0000, Winkler, Tomas wrote: > > > > > > On Sun, Jan 28, 2018 at 09:17:53PM +0000, Winkler, Tomas wrote: > > > > > > > > > > > > > > On Sun, Jan 28, 2018 at 09:51:00AM +0200, Tomas Winkler wrote: > > > > > > > > > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h index > > > > > > bcdd3790e94d..06639fb6ab85 100644 > > > > > > +++ 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); > > > > > > > > > > This seems wrong.. What is the core code supposed to do if > > > > > relinquish > > > fails? > > > > > > > > Not much just propage the error to the caller and leave the policy > > > > decision to it. > > > > > > Your patch set must either cover this or keep it as void. > > > > > > How the code is covering other failures in the transmit functions, how > > is this one different from for example request_locality failure? > > Why we should not propage this error up? > > > > > > > > A better idea is to print an error to klog. > > We can do that in addition. > > I guess you are right. This can be propagated to the user space so that it > knows that there is problem. To make the root more visible the klog > message would make sense. Thanks, will add an error message. Tomas -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/2 v2] tpm: separate cmd_ready/go_idle from runtime_pm 2018-01-28 7:50 ` Tomas Winkler @ 2018-01-28 7:51 ` Tomas Winkler -1 siblings, 0 replies; 24+ messages in thread From: Tomas Winkler @ 2018-01-28 7:51 UTC (permalink / raw) To: Jarkko Sakkinen, Jason Gunthorpe Cc: Alexander Usyskin, linux-integrity, linux-security-module, linux-kernel, Tomas Winkler We cannot use go_idle cmd_ready commands via runtime_pm handles as with the introduction of localities this is no longer an optional feature, while runtime pm can be not enabled. Though cmd_ready/go_idle provides power saving feature, it's also part of TPM2 protocol and should be called explicitly. This patch exposes cmd_read/go_idle via tpm class ops and removes runtime pm support as it is not used by any driver. Signed-off-by: Tomas Winkler <tomas.winkler@intel.com> --- V2: resent drivers/char/tpm/tpm-interface.c | 15 ++++++--- drivers/char/tpm/tpm_crb.c | 69 +++++++++++++++++----------------------- include/linux/tpm.h | 2 ++ 3 files changed, 42 insertions(+), 44 deletions(-) diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index 9fb3d406b078..0c493f885a9e 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -29,7 +29,6 @@ #include <linux/mutex.h> #include <linux/spinlock.h> #include <linux/freezer.h> -#include <linux/pm_runtime.h> #include <linux/tpm_eventlog.h> #include "tpm.h" @@ -437,8 +436,11 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space, chip->locality = rc; } - if (chip->dev.parent) - pm_runtime_get_sync(chip->dev.parent); + if (chip->ops->cmd_ready) { + rc = chip->ops->cmd_ready(chip); + if (rc) + goto out; + } rc = tpm2_prepare_space(chip, space, ordinal, buf); if (rc) @@ -500,8 +502,11 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space, rc = tpm2_commit_space(chip, space, ordinal, buf, &len); out: - if (chip->dev.parent) - pm_runtime_put_sync(chip->dev.parent); + if (chip->ops->go_idle) { + rc = chip->ops->go_idle(chip); + if (rc) + goto out; + } if (need_locality && chip->ops->relinquish_locality) { rc = chip->ops->relinquish_locality(chip, chip->locality); diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c index 497edd9848cd..acfcdc6f31af 100644 --- a/drivers/char/tpm/tpm_crb.c +++ b/drivers/char/tpm/tpm_crb.c @@ -132,7 +132,7 @@ static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value, } /** - * crb_go_idle - request tpm crb device to go the idle state + * __crb_go_idle - request tpm crb device to go the idle state * * @dev: crb device * @priv: crb private data @@ -147,7 +147,7 @@ static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value, * * Return: 0 always */ -static int 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) || @@ -166,8 +166,16 @@ static int crb_go_idle(struct device *dev, struct crb_priv *priv) return 0; } +static int crb_go_idle(struct tpm_chip *chip) +{ + struct device *dev = &chip->dev; + struct crb_priv *priv = dev_get_drvdata(dev); + + return __crb_go_idle(dev, priv); +} + /** - * crb_cmd_ready - request tpm crb device to enter ready state + * __crb_cmd_ready - request tpm crb device to enter ready state * * @dev: crb device * @priv: crb private data @@ -181,7 +189,7 @@ static int crb_go_idle(struct device *dev, struct crb_priv *priv) * * Return: 0 on success -ETIME on timeout; */ -static int 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) || @@ -200,6 +208,14 @@ static int crb_cmd_ready(struct device *dev, struct crb_priv *priv) return 0; } +static int crb_cmd_ready(struct tpm_chip *chip) +{ + struct device *dev = &chip->dev; + struct crb_priv *priv = dev_get_drvdata(dev); + + return __crb_cmd_ready(dev, priv); +} + static int __crb_request_locality(struct device *dev, struct crb_priv *priv, int loc) { @@ -401,6 +417,8 @@ static const struct tpm_class_ops tpm_crb = { .send = crb_send, .cancel = crb_cancel, .req_canceled = crb_req_canceled, + .go_idle = crb_go_idle, + .cmd_ready = crb_cmd_ready, .request_locality = crb_request_locality, .relinquish_locality = crb_relinquish_locality, .req_complete_mask = CRB_DRV_STS_COMPLETE, @@ -517,7 +535,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv, * PTT HW bug w/a: wake up the device to access * possibly not retained registers. */ - ret = crb_cmd_ready(dev, priv); + ret = __crb_cmd_ready(dev, priv); if (ret) return ret; @@ -562,7 +580,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv, if (!ret) priv->cmd_size = cmd_size; - crb_go_idle(dev, priv); + __crb_go_idle(dev, priv); __crb_relinquish_locality(dev, priv, 0); @@ -627,23 +645,13 @@ static int crb_acpi_add(struct acpi_device *device) if (rc) return rc; - rc = crb_cmd_ready(dev, priv); + rc = __crb_cmd_ready(dev, priv); if (rc) goto out; - pm_runtime_get_noresume(dev); - pm_runtime_set_active(dev); - pm_runtime_enable(dev); - rc = tpm_chip_register(chip); - if (rc) { - crb_go_idle(dev, priv); - pm_runtime_put_noidle(dev); - pm_runtime_disable(dev); - goto out; - } - pm_runtime_put_sync(dev); + __crb_go_idle(dev, priv); out: __crb_relinquish_locality(dev, priv, 0); @@ -658,43 +666,27 @@ static int crb_acpi_remove(struct acpi_device *device) tpm_chip_unregister(chip); - pm_runtime_disable(dev); - return 0; } -static int __maybe_unused crb_pm_runtime_suspend(struct device *dev) -{ - struct tpm_chip *chip = dev_get_drvdata(dev); - struct crb_priv *priv = dev_get_drvdata(&chip->dev); - - return crb_go_idle(dev, priv); -} - -static int __maybe_unused crb_pm_runtime_resume(struct device *dev) -{ - struct tpm_chip *chip = dev_get_drvdata(dev); - struct crb_priv *priv = dev_get_drvdata(&chip->dev); - - return crb_cmd_ready(dev, priv); -} - static int __maybe_unused crb_pm_suspend(struct device *dev) { + struct tpm_chip *chip = dev_get_drvdata(dev); int ret; ret = tpm_pm_suspend(dev); if (ret) return ret; - return crb_pm_runtime_suspend(dev); + return crb_go_idle(chip); } static int __maybe_unused crb_pm_resume(struct device *dev) { + struct tpm_chip *chip = dev_get_drvdata(dev); int ret; - ret = crb_pm_runtime_resume(dev); + ret = crb_cmd_ready(chip); if (ret) return ret; @@ -703,7 +695,6 @@ static int __maybe_unused crb_pm_resume(struct device *dev) static const struct dev_pm_ops crb_pm = { SET_SYSTEM_SLEEP_PM_OPS(crb_pm_suspend, crb_pm_resume) - SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend, crb_pm_runtime_resume, NULL) }; static const struct acpi_device_id crb_device_ids[] = { diff --git a/include/linux/tpm.h b/include/linux/tpm.h index 06639fb6ab85..8eb5e5ebe136 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -43,6 +43,8 @@ struct tpm_class_ops { u8 (*status) (struct tpm_chip *chip); bool (*update_timeouts)(struct tpm_chip *chip, unsigned long *timeout_cap); + int (*go_idle)(struct tpm_chip *chip); + int (*cmd_ready)(struct tpm_chip *chip); int (*request_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.14.3 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/2 v2] tpm: separate cmd_ready/go_idle from runtime_pm @ 2018-01-28 7:51 ` Tomas Winkler 0 siblings, 0 replies; 24+ messages in thread From: Tomas Winkler @ 2018-01-28 7:51 UTC (permalink / raw) To: linux-security-module We cannot use go_idle cmd_ready commands via runtime_pm handles as with the introduction of localities this is no longer an optional feature, while runtime pm can be not enabled. Though cmd_ready/go_idle provides power saving feature, it's also part of TPM2 protocol and should be called explicitly. This patch exposes cmd_read/go_idle via tpm class ops and removes runtime pm support as it is not used by any driver. Signed-off-by: Tomas Winkler <tomas.winkler@intel.com> --- V2: resent drivers/char/tpm/tpm-interface.c | 15 ++++++--- drivers/char/tpm/tpm_crb.c | 69 +++++++++++++++++----------------------- include/linux/tpm.h | 2 ++ 3 files changed, 42 insertions(+), 44 deletions(-) diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index 9fb3d406b078..0c493f885a9e 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -29,7 +29,6 @@ #include <linux/mutex.h> #include <linux/spinlock.h> #include <linux/freezer.h> -#include <linux/pm_runtime.h> #include <linux/tpm_eventlog.h> #include "tpm.h" @@ -437,8 +436,11 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space, chip->locality = rc; } - if (chip->dev.parent) - pm_runtime_get_sync(chip->dev.parent); + if (chip->ops->cmd_ready) { + rc = chip->ops->cmd_ready(chip); + if (rc) + goto out; + } rc = tpm2_prepare_space(chip, space, ordinal, buf); if (rc) @@ -500,8 +502,11 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space, rc = tpm2_commit_space(chip, space, ordinal, buf, &len); out: - if (chip->dev.parent) - pm_runtime_put_sync(chip->dev.parent); + if (chip->ops->go_idle) { + rc = chip->ops->go_idle(chip); + if (rc) + goto out; + } if (need_locality && chip->ops->relinquish_locality) { rc = chip->ops->relinquish_locality(chip, chip->locality); diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c index 497edd9848cd..acfcdc6f31af 100644 --- a/drivers/char/tpm/tpm_crb.c +++ b/drivers/char/tpm/tpm_crb.c @@ -132,7 +132,7 @@ static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value, } /** - * crb_go_idle - request tpm crb device to go the idle state + * __crb_go_idle - request tpm crb device to go the idle state * * @dev: crb device * @priv: crb private data @@ -147,7 +147,7 @@ static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value, * * Return: 0 always */ -static int 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) || @@ -166,8 +166,16 @@ static int crb_go_idle(struct device *dev, struct crb_priv *priv) return 0; } +static int crb_go_idle(struct tpm_chip *chip) +{ + struct device *dev = &chip->dev; + struct crb_priv *priv = dev_get_drvdata(dev); + + return __crb_go_idle(dev, priv); +} + /** - * crb_cmd_ready - request tpm crb device to enter ready state + * __crb_cmd_ready - request tpm crb device to enter ready state * * @dev: crb device * @priv: crb private data @@ -181,7 +189,7 @@ static int crb_go_idle(struct device *dev, struct crb_priv *priv) * * Return: 0 on success -ETIME on timeout; */ -static int 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) || @@ -200,6 +208,14 @@ static int crb_cmd_ready(struct device *dev, struct crb_priv *priv) return 0; } +static int crb_cmd_ready(struct tpm_chip *chip) +{ + struct device *dev = &chip->dev; + struct crb_priv *priv = dev_get_drvdata(dev); + + return __crb_cmd_ready(dev, priv); +} + static int __crb_request_locality(struct device *dev, struct crb_priv *priv, int loc) { @@ -401,6 +417,8 @@ static const struct tpm_class_ops tpm_crb = { .send = crb_send, .cancel = crb_cancel, .req_canceled = crb_req_canceled, + .go_idle = crb_go_idle, + .cmd_ready = crb_cmd_ready, .request_locality = crb_request_locality, .relinquish_locality = crb_relinquish_locality, .req_complete_mask = CRB_DRV_STS_COMPLETE, @@ -517,7 +535,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv, * PTT HW bug w/a: wake up the device to access * possibly not retained registers. */ - ret = crb_cmd_ready(dev, priv); + ret = __crb_cmd_ready(dev, priv); if (ret) return ret; @@ -562,7 +580,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv, if (!ret) priv->cmd_size = cmd_size; - crb_go_idle(dev, priv); + __crb_go_idle(dev, priv); __crb_relinquish_locality(dev, priv, 0); @@ -627,23 +645,13 @@ static int crb_acpi_add(struct acpi_device *device) if (rc) return rc; - rc = crb_cmd_ready(dev, priv); + rc = __crb_cmd_ready(dev, priv); if (rc) goto out; - pm_runtime_get_noresume(dev); - pm_runtime_set_active(dev); - pm_runtime_enable(dev); - rc = tpm_chip_register(chip); - if (rc) { - crb_go_idle(dev, priv); - pm_runtime_put_noidle(dev); - pm_runtime_disable(dev); - goto out; - } - pm_runtime_put_sync(dev); + __crb_go_idle(dev, priv); out: __crb_relinquish_locality(dev, priv, 0); @@ -658,43 +666,27 @@ static int crb_acpi_remove(struct acpi_device *device) tpm_chip_unregister(chip); - pm_runtime_disable(dev); - return 0; } -static int __maybe_unused crb_pm_runtime_suspend(struct device *dev) -{ - struct tpm_chip *chip = dev_get_drvdata(dev); - struct crb_priv *priv = dev_get_drvdata(&chip->dev); - - return crb_go_idle(dev, priv); -} - -static int __maybe_unused crb_pm_runtime_resume(struct device *dev) -{ - struct tpm_chip *chip = dev_get_drvdata(dev); - struct crb_priv *priv = dev_get_drvdata(&chip->dev); - - return crb_cmd_ready(dev, priv); -} - static int __maybe_unused crb_pm_suspend(struct device *dev) { + struct tpm_chip *chip = dev_get_drvdata(dev); int ret; ret = tpm_pm_suspend(dev); if (ret) return ret; - return crb_pm_runtime_suspend(dev); + return crb_go_idle(chip); } static int __maybe_unused crb_pm_resume(struct device *dev) { + struct tpm_chip *chip = dev_get_drvdata(dev); int ret; - ret = crb_pm_runtime_resume(dev); + ret = crb_cmd_ready(chip); if (ret) return ret; @@ -703,7 +695,6 @@ static int __maybe_unused crb_pm_resume(struct device *dev) static const struct dev_pm_ops crb_pm = { SET_SYSTEM_SLEEP_PM_OPS(crb_pm_suspend, crb_pm_resume) - SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend, crb_pm_runtime_resume, NULL) }; static const struct acpi_device_id crb_device_ids[] = { diff --git a/include/linux/tpm.h b/include/linux/tpm.h index 06639fb6ab85..8eb5e5ebe136 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -43,6 +43,8 @@ struct tpm_class_ops { u8 (*status) (struct tpm_chip *chip); bool (*update_timeouts)(struct tpm_chip *chip, unsigned long *timeout_cap); + int (*go_idle)(struct tpm_chip *chip); + int (*cmd_ready)(struct tpm_chip *chip); int (*request_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.14.3 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2 v2] tpm: separate cmd_ready/go_idle from runtime_pm 2018-01-28 7:51 ` Tomas Winkler @ 2018-02-06 20:04 ` Jarkko Sakkinen -1 siblings, 0 replies; 24+ messages in thread From: Jarkko Sakkinen @ 2018-02-06 20:04 UTC (permalink / raw) To: Tomas Winkler Cc: Jason Gunthorpe, Alexander Usyskin, linux-integrity, linux-security-module, linux-kernel On Sun, Jan 28, 2018 at 09:51:01AM +0200, Tomas Winkler wrote: > We cannot use go_idle cmd_ready commands via runtime_pm handles > as with the introduction of localities this is no longer an optional > feature, while runtime pm can be not enabled. > Though cmd_ready/go_idle provides power saving feature, it's also part of > TPM2 protocol and should be called explicitly. > This patch exposes cmd_read/go_idle via tpm class ops and removes > runtime pm support as it is not used by any driver. > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com> Makes sense so that only 1/2 woud need to be backported to stable (1/2 should probably have cc for stable). /Jarkko ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/2 v2] tpm: separate cmd_ready/go_idle from runtime_pm @ 2018-02-06 20:04 ` Jarkko Sakkinen 0 siblings, 0 replies; 24+ messages in thread From: Jarkko Sakkinen @ 2018-02-06 20:04 UTC (permalink / raw) To: linux-security-module On Sun, Jan 28, 2018 at 09:51:01AM +0200, Tomas Winkler wrote: > We cannot use go_idle cmd_ready commands via runtime_pm handles > as with the introduction of localities this is no longer an optional > feature, while runtime pm can be not enabled. > Though cmd_ready/go_idle provides power saving feature, it's also part of > TPM2 protocol and should be called explicitly. > This patch exposes cmd_read/go_idle via tpm class ops and removes > runtime pm support as it is not used by any driver. > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com> Makes sense so that only 1/2 woud need to be backported to stable (1/2 should probably have cc for stable). /Jarkko -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2018-02-08 12:46 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-01-28 7:50 [PATCH 0/2] tpm: fix locality and power saving handling Tomas Winkler 2018-01-28 7:50 ` Tomas Winkler 2018-01-28 7:51 ` [PATCH 1/2 v2] tpm: cmd_ready command can be issued only after granting locality Tomas Winkler 2018-01-28 7:51 ` Tomas Winkler 2018-01-28 20:15 ` Jason Gunthorpe 2018-01-28 20:15 ` Jason Gunthorpe 2018-01-28 21:17 ` Winkler, Tomas 2018-01-28 21:17 ` Winkler, Tomas 2018-01-29 17:57 ` Jason Gunthorpe 2018-01-29 17:57 ` Jason Gunthorpe 2018-01-29 19:40 ` Winkler, Tomas 2018-01-29 19:40 ` Winkler, Tomas 2018-02-06 20:02 ` Jarkko Sakkinen 2018-02-06 20:02 ` Jarkko Sakkinen 2018-02-06 21:26 ` Winkler, Tomas 2018-02-06 21:26 ` Winkler, Tomas 2018-02-08 12:44 ` Jarkko Sakkinen 2018-02-08 12:44 ` Jarkko Sakkinen 2018-02-08 12:46 ` Winkler, Tomas 2018-02-08 12:46 ` Winkler, Tomas 2018-01-28 7:51 ` [PATCH 2/2 v2] tpm: separate cmd_ready/go_idle from runtime_pm Tomas Winkler 2018-01-28 7:51 ` Tomas Winkler 2018-02-06 20:04 ` Jarkko Sakkinen 2018-02-06 20:04 ` Jarkko Sakkinen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.