No objections from my side. Please merge.
Regards, Azhar Shaikh >-----Original Message----- >From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] >Sent: Tuesday, March 13, 2018 8:24 AM >To: linux-kernel@vger.kernel.org >Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>; >sta...@vger.kernel.org; Shaikh, Azhar <azhar.sha...@intel.com>; Jarkko >Sakkinen <jarkko.sakki...@linux.intel.com> >Subject: [PATCH 4.14 020/140] tpm: Keep CLKRUN enabled throughout the >duration of transmit_cmd() > >4.14-stable review patch. If anyone has any objections, please let me know. > >------------------ > >From: Azhar Shaikh <azhar.sha...@intel.com> > >commit b3e958ce4c585bf666de249dc794971ebc62d2d3 upstream. > >Commit 5e572cab92f0bb5 ("tpm: Enable CLKRUN protocol for Braswell >systems") disabled CLKRUN protocol during TPM transactions and re-enabled >once the transaction is completed. But there were still some corner cases >observed where, reading of TPM header failed for savestate command while >going to suspend, which resulted in suspend failure. >To fix this issue keep the CLKRUN protocol disabled for the entire duration of >a single TPM command and not disabling and re-enabling again for every TPM >transaction. For the other TPM accesses outside TPM command flow, add a >higher level of disabling and re-enabling the CLKRUN protocol, instead of >doing for every TPM transaction. > >Fixes: 5e572cab92f0bb5 ("tpm: Enable CLKRUN protocol for Braswell >systems") >Signed-off-by: Azhar Shaikh <azhar.sha...@intel.com> >Reviewed-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com> >Tested-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com> >Signed-off-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com> >Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org> > >--- > drivers/char/tpm/tpm-interface.c | 6 ++ > drivers/char/tpm/tpm_tis.c | 92 +++------------------------------ > drivers/char/tpm/tpm_tis_core.c | 108 >+++++++++++++++++++++++++++++++++++---- > drivers/char/tpm/tpm_tis_core.h | 4 + > include/linux/tpm.h | 1 > 5 files changed, 119 insertions(+), 92 deletions(-) > >--- a/drivers/char/tpm/tpm-interface.c >+++ b/drivers/char/tpm/tpm-interface.c >@@ -413,6 +413,9 @@ ssize_t tpm_transmit(struct tpm_chip *ch > if (chip->dev.parent) > pm_runtime_get_sync(chip->dev.parent); > >+ if (chip->ops->clk_enable != NULL) >+ chip->ops->clk_enable(chip, true); >+ > /* Store the decision as chip->locality will be changed. */ > need_locality = chip->locality == -1; > >@@ -489,6 +492,9 @@ out: > 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); > >--- a/drivers/char/tpm/tpm_tis.c >+++ b/drivers/char/tpm/tpm_tis.c >@@ -132,79 +132,17 @@ static int check_acpi_tpm2(struct device } #endif > >-#ifdef CONFIG_X86 >-#define LPC_CNTRL_OFFSET 0x84 >-#define LPC_CLKRUN_EN (1 << 2) >- >-/** >- * tpm_platform_begin_xfer() - clear LPC CLKRUN_EN i.e. clocks will be >running >- */ >-static void tpm_platform_begin_xfer(struct tpm_tis_data *data) -{ >- u32 clkrun_val; >- >- if (!is_bsw()) >- return; >- >- clkrun_val = ioread32(data->ilb_base_addr + LPC_CNTRL_OFFSET); >- >- /* Disable LPC CLKRUN# */ >- clkrun_val &= ~LPC_CLKRUN_EN; >- iowrite32(clkrun_val, data->ilb_base_addr + LPC_CNTRL_OFFSET); >- >- /* >- * Write any random value on port 0x80 which is on LPC, to make >- * sure LPC clock is running before sending any TPM command. >- */ >- outb(0xCC, 0x80); >- >-} >- >-/** >- * tpm_platform_end_xfer() - set LPC CLKRUN_EN i.e. clocks can be turned >off >- */ >-static void tpm_platform_end_xfer(struct tpm_tis_data *data) -{ >- u32 clkrun_val; >- >- if (!is_bsw()) >- return; >- >- clkrun_val = ioread32(data->ilb_base_addr + LPC_CNTRL_OFFSET); >- >- /* Enable LPC CLKRUN# */ >- clkrun_val |= LPC_CLKRUN_EN; >- iowrite32(clkrun_val, data->ilb_base_addr + LPC_CNTRL_OFFSET); >- >- /* >- * Write any random value on port 0x80 which is on LPC, to make >- * sure LPC clock is running before sending any TPM command. >- */ >- outb(0xCC, 0x80); >- >-} >-#else >-static void tpm_platform_begin_xfer(struct tpm_tis_data *data) -{ -} >- >-static void tpm_platform_end_xfer(struct tpm_tis_data *data) -{ -} -#endif >- > static int tpm_tcg_read_bytes(struct tpm_tis_data *data, u32 addr, u16 len, > u8 *result) > { > struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data); > >- tpm_platform_begin_xfer(data); >+ if (is_bsw() && !(data->flags & TPM_TIS_CLK_ENABLE)) >+ WARN(1, "CLKRUN not enabled!\n"); > > while (len--) > *result++ = ioread8(phy->iobase + addr); > >- tpm_platform_end_xfer(data); >- > return 0; > } > >@@ -213,13 +151,12 @@ static int tpm_tcg_write_bytes(struct tp { > struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data); > >- tpm_platform_begin_xfer(data); >+ if (is_bsw() && !(data->flags & TPM_TIS_CLK_ENABLE)) >+ WARN(1, "CLKRUN not enabled!\n"); > > while (len--) > iowrite8(*value++, phy->iobase + addr); > >- tpm_platform_end_xfer(data); >- > return 0; > } > >@@ -227,12 +164,11 @@ static int tpm_tcg_read16(struct tpm_tis { > struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data); > >- tpm_platform_begin_xfer(data); >+ if (is_bsw() && !(data->flags & TPM_TIS_CLK_ENABLE)) >+ WARN(1, "CLKRUN not enabled!\n"); > > *result = ioread16(phy->iobase + addr); > >- tpm_platform_end_xfer(data); >- > return 0; > } > >@@ -240,12 +176,11 @@ static int tpm_tcg_read32(struct tpm_tis { > struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data); > >- tpm_platform_begin_xfer(data); >+ if (is_bsw() && !(data->flags & TPM_TIS_CLK_ENABLE)) >+ WARN(1, "CLKRUN not enabled!\n"); > > *result = ioread32(phy->iobase + addr); > >- tpm_platform_end_xfer(data); >- > return 0; > } > >@@ -253,12 +188,11 @@ static int tpm_tcg_write32(struct tpm_ti { > struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data); > >- tpm_platform_begin_xfer(data); >+ if (is_bsw() && !(data->flags & TPM_TIS_CLK_ENABLE)) >+ WARN(1, "CLKRUN not enabled!\n"); > > iowrite32(value, phy->iobase + addr); > >- tpm_platform_end_xfer(data); >- > return 0; > } > >@@ -340,9 +274,6 @@ static void tpm_tis_pnp_remove(struct pn > > tpm_chip_unregister(chip); > tpm_tis_remove(chip); >- if (is_bsw()) >- iounmap(priv->ilb_base_addr); >- > } > > static struct pnp_driver tis_pnp_driver = { @@ -394,9 +325,6 @@ static int >tpm_tis_plat_remove(struct pl > tpm_chip_unregister(chip); > tpm_tis_remove(chip); > >- if (is_bsw()) >- iounmap(priv->ilb_base_addr); >- > return 0; > } > >--- a/drivers/char/tpm/tpm_tis_core.c >+++ b/drivers/char/tpm/tpm_tis_core.c >@@ -31,6 +31,8 @@ > #include "tpm.h" > #include "tpm_tis_core.h" > >+static void tpm_tis_clkrun_enable(struct tpm_chip *chip, bool value); >+ > /* Before we attempt to access the TPM we must see that the valid bit is set. > * The specification says that this bit is 0 at reset and remains 0 until the > * 'TPM has gone through its self test and initialization and has established >@@ -422,19 +424,28 @@ static bool tpm_tis_update_timeouts(stru > int i, rc; > u32 did_vid; > >+ if (chip->ops->clk_enable != NULL) >+ chip->ops->clk_enable(chip, true); >+ > rc = tpm_tis_read32(priv, TPM_DID_VID(0), &did_vid); > if (rc < 0) >- return rc; >+ goto out; > > for (i = 0; i != ARRAY_SIZE(vendor_timeout_overrides); i++) { > if (vendor_timeout_overrides[i].did_vid != did_vid) > continue; > memcpy(timeout_cap, >vendor_timeout_overrides[i].timeout_us, > sizeof(vendor_timeout_overrides[i].timeout_us)); >- return true; >+ rc = true; > } > >- return false; >+ rc = false; >+ >+out: >+ if (chip->ops->clk_enable != NULL) >+ chip->ops->clk_enable(chip, false); >+ >+ return rc; > } > > /* >@@ -654,14 +665,74 @@ void tpm_tis_remove(struct tpm_chip *chi > u32 interrupt; > int rc; > >+ tpm_tis_clkrun_enable(chip, true); >+ > rc = tpm_tis_read32(priv, reg, &interrupt); > if (rc < 0) > interrupt = 0; > > tpm_tis_write32(priv, reg, ~TPM_GLOBAL_INT_ENABLE & interrupt); >+ >+ tpm_tis_clkrun_enable(chip, false); >+ >+ if (priv->ilb_base_addr) >+ iounmap(priv->ilb_base_addr); > } > EXPORT_SYMBOL_GPL(tpm_tis_remove); > >+/** >+ * tpm_tis_clkrun_enable() - Keep clkrun protocol disabled for entire >duration >+ * of a single TPM command >+ * @chip: TPM chip to use >+ * @value: 1 - Disable CLKRUN protocol, so that clocks are free running >+ * 0 - Enable CLKRUN protocol >+ * Call this function directly in tpm_tis_remove() in error or driver >+removal >+ * path, since the chip->ops is set to NULL in tpm_chip_unregister(). >+ */ >+static void tpm_tis_clkrun_enable(struct tpm_chip *chip, bool value) { >+ struct tpm_tis_data *data = dev_get_drvdata(&chip->dev); >+ u32 clkrun_val; >+ >+ if (!IS_ENABLED(CONFIG_X86) || !is_bsw()) >+ return; >+ >+ if (value) { >+ data->flags |= TPM_TIS_CLK_ENABLE; >+ data->clkrun_enabled++; >+ if (data->clkrun_enabled > 1) >+ return; >+ clkrun_val = ioread32(data->ilb_base_addr + >LPC_CNTRL_OFFSET); >+ >+ /* Disable LPC CLKRUN# */ >+ clkrun_val &= ~LPC_CLKRUN_EN; >+ iowrite32(clkrun_val, data->ilb_base_addr + >LPC_CNTRL_OFFSET); >+ >+ /* >+ * Write any random value on port 0x80 which is on LPC, to >make >+ * sure LPC clock is running before sending any TPM >command. >+ */ >+ outb(0xCC, 0x80); >+ } else { >+ data->clkrun_enabled--; >+ if (data->clkrun_enabled) >+ return; >+ >+ clkrun_val = ioread32(data->ilb_base_addr + >LPC_CNTRL_OFFSET); >+ >+ /* Enable LPC CLKRUN# */ >+ clkrun_val |= LPC_CLKRUN_EN; >+ iowrite32(clkrun_val, data->ilb_base_addr + >LPC_CNTRL_OFFSET); >+ >+ /* >+ * Write any random value on port 0x80 which is on LPC, to >make >+ * sure LPC clock is running before sending any TPM >command. >+ */ >+ outb(0xCC, 0x80); >+ data->flags &= ~TPM_TIS_CLK_ENABLE; >+ } >+} >+ > static const struct tpm_class_ops tpm_tis = { > .flags = TPM_OPS_AUTO_STARTUP, > .status = tpm_tis_status, >@@ -674,6 +745,7 @@ static const struct tpm_class_ops tpm_ti > .req_canceled = tpm_tis_req_canceled, > .request_locality = request_locality, > .relinquish_locality = release_locality, >+ .clk_enable = tpm_tis_clkrun_enable, > }; > > int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, >@@ -708,6 +780,9 @@ int tpm_tis_core_init(struct device *dev > return -ENOMEM; > } > >+ if (chip->ops->clk_enable != NULL) >+ chip->ops->clk_enable(chip, true); >+ > if (wait_startup(chip, 0) != 0) { > rc = -ENODEV; > goto out_err; >@@ -799,14 +874,18 @@ int tpm_tis_core_init(struct device *dev > } > > rc = tpm_chip_register(chip); >- if (rc && is_bsw()) >- iounmap(priv->ilb_base_addr); >+ if (rc) >+ goto out_err; > >- return rc; >+ if (chip->ops->clk_enable != NULL) >+ chip->ops->clk_enable(chip, false); >+ >+ return 0; > out_err: >+ if ((chip->ops != NULL) && (chip->ops->clk_enable != NULL)) >+ chip->ops->clk_enable(chip, false); >+ > tpm_tis_remove(chip); >- if (is_bsw()) >- iounmap(priv->ilb_base_addr); > > return rc; > } >@@ -819,22 +898,31 @@ static void tpm_tis_reenable_interrupts( > u32 intmask; > int rc; > >+ if (chip->ops->clk_enable != NULL) >+ chip->ops->clk_enable(chip, true); >+ > /* reenable interrupts that device may have lost or > * BIOS/firmware may have disabled > */ > rc = tpm_tis_write8(priv, TPM_INT_VECTOR(priv->locality), priv->irq); > if (rc < 0) >- return; >+ goto out; > > rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), >&intmask); > if (rc < 0) >- return; >+ goto out; > > intmask |= TPM_INTF_CMD_READY_INT > | TPM_INTF_LOCALITY_CHANGE_INT | >TPM_INTF_DATA_AVAIL_INT > | TPM_INTF_STS_VALID_INT | TPM_GLOBAL_INT_ENABLE; > > tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask); >+ >+out: >+ if (chip->ops->clk_enable != NULL) >+ chip->ops->clk_enable(chip, false); >+ >+ return; > } > > int tpm_tis_resume(struct device *dev) >--- a/drivers/char/tpm/tpm_tis_core.h >+++ b/drivers/char/tpm/tpm_tis_core.h >@@ -79,11 +79,14 @@ enum tis_defaults { > #define TPM_DID_VID(l) (0x0F00 | ((l) << 12)) > #define TPM_RID(l) (0x0F04 | ((l) << 12)) > >+#define LPC_CNTRL_OFFSET 0x84 >+#define LPC_CLKRUN_EN (1 << 2) > #define INTEL_LEGACY_BLK_BASE_ADDR 0xFED08000 > #define ILB_REMAP_SIZE 0x100 > > enum tpm_tis_flags { > TPM_TIS_ITPM_WORKAROUND = BIT(0), >+ TPM_TIS_CLK_ENABLE = BIT(1), > }; > > struct tpm_tis_data { >@@ -93,6 +96,7 @@ struct tpm_tis_data { > bool irq_tested; > unsigned int flags; > void __iomem *ilb_base_addr; >+ u16 clkrun_enabled; > wait_queue_head_t int_queue; > wait_queue_head_t read_queue; > const struct tpm_tis_phy_ops *phy_ops; >--- a/include/linux/tpm.h >+++ b/include/linux/tpm.h >@@ -50,6 +50,7 @@ struct tpm_class_ops { > unsigned long *timeout_cap); > int (*request_locality)(struct tpm_chip *chip, int loc); > void (*relinquish_locality)(struct tpm_chip *chip, int loc); >+ void (*clk_enable)(struct tpm_chip *chip, bool value); > }; > > #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE) >
