[PATCH] tpm: Enable CLKRUN protocol for Braswell systems

[PATCH] tpm: Enable CLKRUN protocol for Braswell systems

[Azhar Shaikh]

To overcome a hardware limitation on Intel Braswell systems,
disable CLKRUN protocol during TPM transactions and re-enable
once the transaction is completed.

Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
---
 drivers/char/tpm/tpm.h     | 20 +++++++++++
 drivers/char/tpm/tpm_tis.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4b4c8dee3096..98032a22317e 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -36,6 +36,10 @@
 #include <linux/highmem.h>
 #include <crypto/hash_info.h>
 
+#ifdef CONFIG_X86
+#include <asm/intel-family.h>
+#endif
+
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */
 	TPM_BUFSIZE = 4096,
@@ -436,6 +440,22 @@ struct tpm_buf {
 	u8 *data;
 };
 
+#define INTEL_LEGACY_BLK_BASE_ADDR	0xFED08000
+#define LPC_CNTRL_REG_OFFSET		0x84
+#define LPC_CLKRUN_EN			(1 << 2)
+
+#ifdef CONFIG_X86
+static inline bool is_bsw(void)
+{
+	return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0);
+}
+#else
+static inline bool is_bsw(void)
+{
+	return false;
+}
+#endif
+
 static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
 {
 	struct tpm_input_header *head;
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index c7e1384f1b08..25ead56382c0 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -89,13 +89,70 @@ static inline int is_itpm(struct acpi_device *dev)
 }
 #endif
 
+/**
+ * disable_lpc_clk_run() - clear LPC CLKRUN_EN i.e. clocks will be free running
+ */
+static void disable_lpc_clk_run(void)
+{
+	u32 clkrun_val;
+	void __iomem *ilb_base_addr = NULL;
+
+	ilb_base_addr = (void __iomem *)
+		kmap_atomic_pfn(INTEL_LEGACY_BLK_BASE_ADDR >> PAGE_SHIFT);
+
+	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	/* Disable LPC CLKRUN# */
+	clkrun_val &= ~LPC_CLKRUN_EN;
+	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	kunmap_atomic(ilb_base_addr);
+	/*
+	 * Write any random value on port 0x80 which is on LPC, to make
+	 * sure LPC clock is running before sending any TPM command.
+	 */
+	outb(0x80, 0xCC);
+}
+
+/**
+ * enable_lpc_clk_run() - set LPC CLKRUN_EN i.e. clocks can be turned off
+ */
+static void enable_lpc_clk_run(void)
+{
+	u32 clkrun_val;
+	void __iomem *ilb_base_addr = NULL;
+
+	ilb_base_addr = (void __iomem *)
+		kmap_atomic_pfn(INTEL_LEGACY_BLK_BASE_ADDR >> PAGE_SHIFT);
+
+	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	/* Enable LPC CLKRUN# */
+	clkrun_val |= LPC_CLKRUN_EN;
+	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	kunmap_atomic(ilb_base_addr);
+	/*
+	 * Write any random value on port 0x80 which is on LPC, to make
+	 * sure LPC clock is running before sending any TPM command.
+	 */
+	outb(0x80, 0xCC);
+}
+
 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);
 
+	if (is_bsw())
+		disable_lpc_clk_run();
+
 	while (len--)
 		*result++ = ioread8(phy->iobase + addr);
+
+	if (is_bsw())
+		enable_lpc_clk_run();
+
 	return 0;
 }
 
@@ -104,8 +161,15 @@ static int tpm_tcg_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	if (is_bsw())
+		disable_lpc_clk_run();
+
 	while (len--)
 		iowrite8(*value++, phy->iobase + addr);
+
+	if (is_bsw())
+		enable_lpc_clk_run();
+
 	return 0;
 }
 
@@ -113,7 +177,14 @@ static int tpm_tcg_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	if (is_bsw())
+		disable_lpc_clk_run();
+
 	*result = ioread16(phy->iobase + addr);
+
+	if (is_bsw())
+		enable_lpc_clk_run();
+
 	return 0;
 }
 
@@ -121,7 +192,14 @@ static int tpm_tcg_read32(struct tpm_tis_data *data, u32 addr, u32 *result)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	if (is_bsw())
+		disable_lpc_clk_run();
+
 	*result = ioread32(phy->iobase + addr);
+
+	if (is_bsw())
+		enable_lpc_clk_run();
+
 	return 0;
 }
 
@@ -129,7 +207,14 @@ static int tpm_tcg_write32(struct tpm_tis_data *data, u32 addr, u32 value)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	if (is_bsw())
+		disable_lpc_clk_run();
+
 	iowrite32(value, phy->iobase + addr);
+
+	if (is_bsw())
+		enable_lpc_clk_run();
+
 	return 0;
 }
 
-- 
1.9.1

Re: [PATCH] tpm: Enable CLKRUN protocol for Braswell systems

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1595 bytes --]

Hi Azhar,

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v4.12-rc3]
[cannot apply to next-20170601]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Azhar-Shaikh/tpm-Enable-CLKRUN-protocol-for-Braswell-systems/20170602-053032
config: frv-allyesconfig (attached as .config)
compiler: frv-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=frv 

All errors (new ones prefixed by >>):

   drivers/char/tpm/tpm_tis.c: In function 'disable_lpc_clk_run':
>> drivers/char/tpm/tpm_tis.c:101:3: error: implicit declaration of function 'kmap_atomic_pfn' [-Werror=implicit-function-declaration]
      kmap_atomic_pfn(INTEL_LEGACY_BLK_BASE_ADDR >> PAGE_SHIFT);
      ^~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/kmap_atomic_pfn +101 drivers/char/tpm/tpm_tis.c

    95	static void disable_lpc_clk_run(void)
    96	{
    97		u32 clkrun_val;
    98		void __iomem *ilb_base_addr = NULL;
    99	
   100		ilb_base_addr = (void __iomem *)
 > 101			kmap_atomic_pfn(INTEL_LEGACY_BLK_BASE_ADDR >> PAGE_SHIFT);
   102	
   103		clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
   104	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48487 bytes --]

[PATCH v2] tpm: Enable CLKRUN protocol for Braswell systems

[Azhar Shaikh]

To overcome a hardware limitation on Intel Braswell systems,
disable CLKRUN protocol during TPM transactions and re-enable
once the transaction is completed.

Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
---
Changes from v1:
- Add CONFIG_X86 around disable_lpc_clk_run () and enable_lpc_clk_run() to avoid
- build breakage on architectures which do not implement kmap_atomic_pfn()

 drivers/char/tpm/tpm.h     | 20 ++++++++++
 drivers/char/tpm/tpm_tis.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 114 insertions(+)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4b4c8dee3096..98032a22317e 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -36,6 +36,10 @@
 #include <linux/highmem.h>
 #include <crypto/hash_info.h>
 
+#ifdef CONFIG_X86
+#include <asm/intel-family.h>
+#endif
+
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */
 	TPM_BUFSIZE = 4096,
@@ -436,6 +440,22 @@ struct tpm_buf {
 	u8 *data;
 };
 
+#define INTEL_LEGACY_BLK_BASE_ADDR	0xFED08000
+#define LPC_CNTRL_REG_OFFSET		0x84
+#define LPC_CLKRUN_EN			(1 << 2)
+
+#ifdef CONFIG_X86
+static inline bool is_bsw(void)
+{
+	return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0);
+}
+#else
+static inline bool is_bsw(void)
+{
+	return false;
+}
+#endif
+
 static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
 {
 	struct tpm_input_header *head;
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index c7e1384f1b08..0c1496340a18 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -89,13 +89,79 @@ static inline int is_itpm(struct acpi_device *dev)
 }
 #endif
 
+#ifdef CONFIG_X86
+/**
+ * disable_lpc_clk_run() - clear LPC CLKRUN_EN i.e. clocks will be free running
+ */
+static void disable_lpc_clk_run(void)
+{
+	u32 clkrun_val;
+	void __iomem *ilb_base_addr = NULL;
+
+	ilb_base_addr = (void __iomem *)
+		kmap_atomic_pfn(INTEL_LEGACY_BLK_BASE_ADDR >> PAGE_SHIFT);
+
+	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	/* Disable LPC CLKRUN# */
+	clkrun_val &= ~LPC_CLKRUN_EN;
+	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	kunmap_atomic(ilb_base_addr);
+	/*
+	 * Write any random value on port 0x80 which is on LPC, to make
+	 * sure LPC clock is running before sending any TPM command.
+	 */
+	outb(0x80, 0xCC);
+}
+
+/**
+ * enable_lpc_clk_run() - set LPC CLKRUN_EN i.e. clocks can be turned off
+ */
+static void enable_lpc_clk_run(void)
+{
+	u32 clkrun_val;
+	void __iomem *ilb_base_addr = NULL;
+
+	ilb_base_addr = (void __iomem *)
+		kmap_atomic_pfn(INTEL_LEGACY_BLK_BASE_ADDR >> PAGE_SHIFT);
+
+	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	/* Enable LPC CLKRUN# */
+	clkrun_val |= LPC_CLKRUN_EN;
+	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	kunmap_atomic(ilb_base_addr);
+	/*
+	 * Write any random value on port 0x80 which is on LPC, to make
+	 * sure LPC clock is running before sending any TPM command.
+	 */
+	outb(0x80, 0xCC);
+}
+#else
+static void disable_lpc_clk_run(void)
+{
+}
+static void enable_lpc_clk_run(void)
+{
+}
+#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);
 
+	if (is_bsw())
+		disable_lpc_clk_run();
+
 	while (len--)
 		*result++ = ioread8(phy->iobase + addr);
+
+	if (is_bsw())
+		enable_lpc_clk_run();
+
 	return 0;
 }
 
@@ -104,8 +170,15 @@ static int tpm_tcg_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	if (is_bsw())
+		disable_lpc_clk_run();
+
 	while (len--)
 		iowrite8(*value++, phy->iobase + addr);
+
+	if (is_bsw())
+		enable_lpc_clk_run();
+
 	return 0;
 }
 
@@ -113,7 +186,14 @@ static int tpm_tcg_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	if (is_bsw())
+		disable_lpc_clk_run();
+
 	*result = ioread16(phy->iobase + addr);
+
+	if (is_bsw())
+		enable_lpc_clk_run();
+
 	return 0;
 }
 
@@ -121,7 +201,14 @@ static int tpm_tcg_read32(struct tpm_tis_data *data, u32 addr, u32 *result)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	if (is_bsw())
+		disable_lpc_clk_run();
+
 	*result = ioread32(phy->iobase + addr);
+
+	if (is_bsw())
+		enable_lpc_clk_run();
+
 	return 0;
 }
 
@@ -129,7 +216,14 @@ static int tpm_tcg_write32(struct tpm_tis_data *data, u32 addr, u32 value)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	if (is_bsw())
+		disable_lpc_clk_run();
+
 	iowrite32(value, phy->iobase + addr);
+
+	if (is_bsw())
+		enable_lpc_clk_run();
+
 	return 0;
 }
 
-- 
1.9.1

Re: [PATCH v2] tpm: Enable CLKRUN protocol for Braswell systems

[Jarkko Sakkinen]

On Thu, Jun 01, 2017 at 07:04:04PM -0700, Azhar Shaikh wrote:
> To overcome a hardware limitation on Intel Braswell systems,
> disable CLKRUN protocol during TPM transactions and re-enable
> once the transaction is completed.
> 
> Signed-off-by: Azhar Shaikh <azhar.shaikh-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> Changes from v1:
> - Add CONFIG_X86 around disable_lpc_clk_run () and enable_lpc_clk_run() to avoid
> - build breakage on architectures which do not implement kmap_atomic_pfn()
> 
>  drivers/char/tpm/tpm.h     | 20 ++++++++++
>  drivers/char/tpm/tpm_tis.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 114 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 4b4c8dee3096..98032a22317e 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -36,6 +36,10 @@
>  #include <linux/highmem.h>
>  #include <crypto/hash_info.h>
>  
> +#ifdef CONFIG_X86
> +#include <asm/intel-family.h>
> +#endif

#ifdef's are not necessary here.

> +
>  enum tpm_const {
>  	TPM_MINOR = 224,	/* officially assigned */
>  	TPM_BUFSIZE = 4096,
> @@ -436,6 +440,22 @@ struct tpm_buf {
>  	u8 *data;
>  };
>  
> +#define INTEL_LEGACY_BLK_BASE_ADDR	0xFED08000
> +#define LPC_CNTRL_REG_OFFSET		0x84
> +#define LPC_CLKRUN_EN			(1 << 2)
> +
> +#ifdef CONFIG_X86
> +static inline bool is_bsw(void)
> +{
> +	return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0);
> +}
> +#else
> +static inline bool is_bsw(void)
> +{
> +	return false;
> +}
> +#endif

Move these to tpm_tis.c right before disable_lpc_clk_run().

> +
>  static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
>  {
>  	struct tpm_input_header *head;
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index c7e1384f1b08..0c1496340a18 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -89,13 +89,79 @@ static inline int is_itpm(struct acpi_device *dev)
>  }
>  #endif
>  
> +#ifdef CONFIG_X86
> +/**
> + * disable_lpc_clk_run() - clear LPC CLKRUN_EN i.e. clocks will be free running
> + */
> +static void disable_lpc_clk_run(void)
> +{
> +	u32 clkrun_val;
> +	void __iomem *ilb_base_addr = NULL;
> +
> +	ilb_base_addr = (void __iomem *)
> +		kmap_atomic_pfn(INTEL_LEGACY_BLK_BASE_ADDR >> PAGE_SHIFT);
> +
> +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> +
> +	/* Disable LPC CLKRUN# */
> +	clkrun_val &= ~LPC_CLKRUN_EN;
> +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> +
> +	kunmap_atomic(ilb_base_addr);
> +	/*
> +	 * Write any random value on port 0x80 which is on LPC, to make
> +	 * sure LPC clock is running before sending any TPM command.
> +	 */
> +	outb(0x80, 0xCC);
> +}

You said that this code does not work compared to a version that does
only static ioremap.

I compared this to the other version. One of the major differences is
that outb() is done before releasing the mapping. Don't know or
understand what difference that would make but it is a semantic
difference.

Another observation is that should you have wmb() before outb() to make
sure that all the write operations are complete?

> +
> +/**
> + * enable_lpc_clk_run() - set LPC CLKRUN_EN i.e. clocks can be turned off
> + */
> +static void enable_lpc_clk_run(void)
> +{
> +	u32 clkrun_val;
> +	void __iomem *ilb_base_addr = NULL;
> +
> +	ilb_base_addr = (void __iomem *)
> +		kmap_atomic_pfn(INTEL_LEGACY_BLK_BASE_ADDR >> PAGE_SHIFT);
> +
> +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> +
> +	/* Enable LPC CLKRUN# */
> +	clkrun_val |= LPC_CLKRUN_EN;
> +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> +
> +	kunmap_atomic(ilb_base_addr);
> +	/*
> +	 * Write any random value on port 0x80 which is on LPC, to make
> +	 * sure LPC clock is running before sending any TPM command.
> +	 */
> +	outb(0x80, 0xCC);
> +}
> +#else
> +static void disable_lpc_clk_run(void)
> +{
> +}
> +static void enable_lpc_clk_run(void)
> +{
> +}
> +#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);
>  
> +	if (is_bsw())
> +		disable_lpc_clk_run();

Do is_bsw() instead inside so that this can be unconditionally called.

> +
>  	while (len--)
>  		*result++ = ioread8(phy->iobase + addr);
> +
> +	if (is_bsw())
> +		enable_lpc_clk_run();
> +
>  	return 0;
>  }
>  
> @@ -104,8 +170,15 @@ static int tpm_tcg_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	if (is_bsw())
> +		disable_lpc_clk_run();
> +
>  	while (len--)
>  		iowrite8(*value++, phy->iobase + addr);
> +
> +	if (is_bsw())
> +		enable_lpc_clk_run();
> +
>  	return 0;
>  }
>  
> @@ -113,7 +186,14 @@ static int tpm_tcg_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	if (is_bsw())
> +		disable_lpc_clk_run();
> +
>  	*result = ioread16(phy->iobase + addr);
> +
> +	if (is_bsw())
> +		enable_lpc_clk_run();
> +
>  	return 0;
>  }
>  
> @@ -121,7 +201,14 @@ static int tpm_tcg_read32(struct tpm_tis_data *data, u32 addr, u32 *result)
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	if (is_bsw())
> +		disable_lpc_clk_run();
> +
>  	*result = ioread32(phy->iobase + addr);
> +
> +	if (is_bsw())
> +		enable_lpc_clk_run();
> +
>  	return 0;
>  }
>  
> @@ -129,7 +216,14 @@ static int tpm_tcg_write32(struct tpm_tis_data *data, u32 addr, u32 value)
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	if (is_bsw())
> +		disable_lpc_clk_run();
> +
>  	iowrite32(value, phy->iobase + addr);
> +
> +	if (is_bsw())
> +		enable_lpc_clk_run();
> +
>  	return 0;
>  }
>  
> -- 
> 1.9.1
> 

/Jarkko

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

RE: [PATCH v2] tpm: Enable CLKRUN protocol for Braswell systems

[Shaikh, Azhar]

> -----Original Message-----
> From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> Sent: Monday, June 5, 2017 6:32 AM
> To: Shaikh, Azhar <azhar.shaikh@intel.com>
> Cc: jgunthorpe@obsidianresearch.com; tpmdd-devel@lists.sourceforge.net;
> linux-kernel@vger.kernel.org; linux-security-module@vger.kernel.org
> Subject: Re: [PATCH v2] tpm: Enable CLKRUN protocol for Braswell systems
> 
> On Thu, Jun 01, 2017 at 07:04:04PM -0700, Azhar Shaikh wrote:
> > To overcome a hardware limitation on Intel Braswell systems, disable
> > CLKRUN protocol during TPM transactions and re-enable once the
> > transaction is completed.
> >
> > Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
> > ---
> > Changes from v1:
> > - Add CONFIG_X86 around disable_lpc_clk_run () and
> > enable_lpc_clk_run() to avoid
> > - build breakage on architectures which do not implement
> > kmap_atomic_pfn()
> >
> >  drivers/char/tpm/tpm.h     | 20 ++++++++++
> >  drivers/char/tpm/tpm_tis.c | 94
> > ++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 114 insertions(+)
> >
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index
> > 4b4c8dee3096..98032a22317e 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -36,6 +36,10 @@
> >  #include <linux/highmem.h>
> >  #include <crypto/hash_info.h>
> >
> > +#ifdef CONFIG_X86
> > +#include <asm/intel-family.h>
> > +#endif
> 
> #ifdef's are not necessary here.

If the #ifdef's are not present, on non-Intel architectures, intel-family.h file is not found during compile time.

> 
> > +
> >  enum tpm_const {
> >  	TPM_MINOR = 224,	/* officially assigned */
> >  	TPM_BUFSIZE = 4096,
> > @@ -436,6 +440,22 @@ struct tpm_buf {
> >  	u8 *data;
> >  };
> >
> > +#define INTEL_LEGACY_BLK_BASE_ADDR	0xFED08000
> > +#define LPC_CNTRL_REG_OFFSET		0x84
> > +#define LPC_CLKRUN_EN			(1 << 2)
> > +
> > +#ifdef CONFIG_X86
> > +static inline bool is_bsw(void)
> > +{
> > +	return ((boot_cpu_data.x86_model ==
> INTEL_FAM6_ATOM_AIRMONT) ? 1 :
> > +0); } #else static inline bool is_bsw(void) {
> > +	return false;
> > +}
> > +#endif
> 
> Move these to tpm_tis.c right before disable_lpc_clk_run().

Ok, will do.

> 
> > +
> >  static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32
> > ordinal)  {
> >  	struct tpm_input_header *head;
> > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> > index c7e1384f1b08..0c1496340a18 100644
> > --- a/drivers/char/tpm/tpm_tis.c
> > +++ b/drivers/char/tpm/tpm_tis.c
> > @@ -89,13 +89,79 @@ static inline int is_itpm(struct acpi_device *dev)
> > }  #endif
> >
> > +#ifdef CONFIG_X86
> > +/**
> > + * disable_lpc_clk_run() - clear LPC CLKRUN_EN i.e. clocks will be
> > +free running  */ static void disable_lpc_clk_run(void) {
> > +	u32 clkrun_val;
> > +	void __iomem *ilb_base_addr = NULL;
> > +
> > +	ilb_base_addr = (void __iomem *)
> > +		kmap_atomic_pfn(INTEL_LEGACY_BLK_BASE_ADDR >>
> PAGE_SHIFT);
> > +
> > +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> > +
> > +	/* Disable LPC CLKRUN# */
> > +	clkrun_val &= ~LPC_CLKRUN_EN;
> > +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> > +
> > +	kunmap_atomic(ilb_base_addr);
> > +	/*
> > +	 * Write any random value on port 0x80 which is on LPC, to make
> > +	 * sure LPC clock is running before sending any TPM command.
> > +	 */
> > +	outb(0x80, 0xCC);
> > +}
> 
> You said that this code does not work compared to a version that does only
> static ioremap.
> 
> I compared this to the other version. One of the major differences is that
> outb() is done before releasing the mapping. Don't know or understand what
> difference that would make but it is a semantic difference.
> 
I tried unmapping after outb() and wmb(), still code does not work.

> Another observation is that should you have wmb() before outb() to make
> sure that all the write operations are complete?
> 
I think you meant to say add wmb() 'after' outb() and not before.

> > +
> > +/**
> > + * enable_lpc_clk_run() - set LPC CLKRUN_EN i.e. clocks can be turned
> > +off  */ static void enable_lpc_clk_run(void) {
> > +	u32 clkrun_val;
> > +	void __iomem *ilb_base_addr = NULL;
> > +
> > +	ilb_base_addr = (void __iomem *)
> > +		kmap_atomic_pfn(INTEL_LEGACY_BLK_BASE_ADDR >>
> PAGE_SHIFT);
> > +
> > +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> > +
> > +	/* Enable LPC CLKRUN# */
> > +	clkrun_val |= LPC_CLKRUN_EN;
> > +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> > +
> > +	kunmap_atomic(ilb_base_addr);
> > +	/*
> > +	 * Write any random value on port 0x80 which is on LPC, to make
> > +	 * sure LPC clock is running before sending any TPM command.
> > +	 */
> > +	outb(0x80, 0xCC);
> > +}
> > +#else
> > +static void disable_lpc_clk_run(void) { } static void
> > +enable_lpc_clk_run(void) { } #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);
> >
> > +	if (is_bsw())
> > +		disable_lpc_clk_run();
> 
> Do is_bsw() instead inside so that this can be unconditionally called.
> 

Ok, will do.

> > +
> >  	while (len--)
> >  		*result++ = ioread8(phy->iobase + addr);
> > +
> > +	if (is_bsw())
> > +		enable_lpc_clk_run();
> > +
> >  	return 0;
> >  }
> >
> > @@ -104,8 +170,15 @@ static int tpm_tcg_write_bytes(struct
> > tpm_tis_data *data, u32 addr, u16 len,  {
> >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> >
> > +	if (is_bsw())
> > +		disable_lpc_clk_run();
> > +
> >  	while (len--)
> >  		iowrite8(*value++, phy->iobase + addr);
> > +
> > +	if (is_bsw())
> > +		enable_lpc_clk_run();
> > +
> >  	return 0;
> >  }
> >
> > @@ -113,7 +186,14 @@ static int tpm_tcg_read16(struct tpm_tis_data
> > *data, u32 addr, u16 *result)  {
> >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> >
> > +	if (is_bsw())
> > +		disable_lpc_clk_run();
> > +
> >  	*result = ioread16(phy->iobase + addr);
> > +
> > +	if (is_bsw())
> > +		enable_lpc_clk_run();
> > +
> >  	return 0;
> >  }
> >
> > @@ -121,7 +201,14 @@ static int tpm_tcg_read32(struct tpm_tis_data
> > *data, u32 addr, u32 *result)  {
> >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> >
> > +	if (is_bsw())
> > +		disable_lpc_clk_run();
> > +
> >  	*result = ioread32(phy->iobase + addr);
> > +
> > +	if (is_bsw())
> > +		enable_lpc_clk_run();
> > +
> >  	return 0;
> >  }
> >
> > @@ -129,7 +216,14 @@ static int tpm_tcg_write32(struct tpm_tis_data
> > *data, u32 addr, u32 value)  {
> >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> >
> > +	if (is_bsw())
> > +		disable_lpc_clk_run();
> > +
> >  	iowrite32(value, phy->iobase + addr);
> > +
> > +	if (is_bsw())
> > +		enable_lpc_clk_run();
> > +
> >  	return 0;
> >  }
> >
> > --
> > 1.9.1
> >
> 
> /Jarkko

Re: [PATCH v2] tpm: Enable CLKRUN protocol for Braswell systems

[Jarkko Sakkinen]

On Mon, Jun 05, 2017 at 06:42:55PM +0000, Shaikh, Azhar wrote:
> 
> 
> > -----Original Message-----
> > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > Sent: Monday, June 5, 2017 6:32 AM
> > To: Shaikh, Azhar <azhar.shaikh@intel.com>
> > Cc: jgunthorpe@obsidianresearch.com; tpmdd-devel@lists.sourceforge.net;
> > linux-kernel@vger.kernel.org; linux-security-module@vger.kernel.org
> > Subject: Re: [PATCH v2] tpm: Enable CLKRUN protocol for Braswell systems
> > 
> > On Thu, Jun 01, 2017 at 07:04:04PM -0700, Azhar Shaikh wrote:
> > > To overcome a hardware limitation on Intel Braswell systems, disable
> > > CLKRUN protocol during TPM transactions and re-enable once the
> > > transaction is completed.
> > >
> > > Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
> > > ---
> > > Changes from v1:
> > > - Add CONFIG_X86 around disable_lpc_clk_run () and
> > > enable_lpc_clk_run() to avoid
> > > - build breakage on architectures which do not implement
> > > kmap_atomic_pfn()
> > >
> > >  drivers/char/tpm/tpm.h     | 20 ++++++++++
> > >  drivers/char/tpm/tpm_tis.c | 94
> > > ++++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 114 insertions(+)
> > >
> > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index
> > > 4b4c8dee3096..98032a22317e 100644
> > > --- a/drivers/char/tpm/tpm.h
> > > +++ b/drivers/char/tpm/tpm.h
> > > @@ -36,6 +36,10 @@
> > >  #include <linux/highmem.h>
> > >  #include <crypto/hash_info.h>
> > >
> > > +#ifdef CONFIG_X86
> > > +#include <asm/intel-family.h>
> > > +#endif
> > 
> > #ifdef's are not necessary here.
> 
> If the #ifdef's are not present, on non-Intel architectures, intel-family.h file is not found during compile time.

Oops, my bad. You are absolutely right!

> 
> > 
> > > +
> > >  enum tpm_const {
> > >  	TPM_MINOR = 224,	/* officially assigned */
> > >  	TPM_BUFSIZE = 4096,
> > > @@ -436,6 +440,22 @@ struct tpm_buf {
> > >  	u8 *data;
> > >  };
> > >
> > > +#define INTEL_LEGACY_BLK_BASE_ADDR	0xFED08000
> > > +#define LPC_CNTRL_REG_OFFSET		0x84
> > > +#define LPC_CLKRUN_EN			(1 << 2)
> > > +
> > > +#ifdef CONFIG_X86
> > > +static inline bool is_bsw(void)
> > > +{
> > > +	return ((boot_cpu_data.x86_model ==
> > INTEL_FAM6_ATOM_AIRMONT) ? 1 :
> > > +0); } #else static inline bool is_bsw(void) {
> > > +	return false;
> > > +}
> > > +#endif
> > 
> > Move these to tpm_tis.c right before disable_lpc_clk_run().
> 
> Ok, will do.
> 
> > 
> > > +
> > >  static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32
> > > ordinal)  {
> > >  	struct tpm_input_header *head;
> > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> > > index c7e1384f1b08..0c1496340a18 100644
> > > --- a/drivers/char/tpm/tpm_tis.c
> > > +++ b/drivers/char/tpm/tpm_tis.c
> > > @@ -89,13 +89,79 @@ static inline int is_itpm(struct acpi_device *dev)
> > > }  #endif
> > >
> > > +#ifdef CONFIG_X86
> > > +/**
> > > + * disable_lpc_clk_run() - clear LPC CLKRUN_EN i.e. clocks will be
> > > +free running  */ static void disable_lpc_clk_run(void) {
> > > +	u32 clkrun_val;
> > > +	void __iomem *ilb_base_addr = NULL;
> > > +
> > > +	ilb_base_addr = (void __iomem *)
> > > +		kmap_atomic_pfn(INTEL_LEGACY_BLK_BASE_ADDR >>
> > PAGE_SHIFT);
> > > +
> > > +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> > > +
> > > +	/* Disable LPC CLKRUN# */
> > > +	clkrun_val &= ~LPC_CLKRUN_EN;
> > > +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> > > +
> > > +	kunmap_atomic(ilb_base_addr);
> > > +	/*
> > > +	 * Write any random value on port 0x80 which is on LPC, to make
> > > +	 * sure LPC clock is running before sending any TPM command.
> > > +	 */
> > > +	outb(0x80, 0xCC);
> > > +}
> > 
> > You said that this code does not work compared to a version that does only
> > static ioremap.
> > 
> > I compared this to the other version. One of the major differences is that
> > outb() is done before releasing the mapping. Don't know or understand what
> > difference that would make but it is a semantic difference.
> > 
> I tried unmapping after outb() and wmb(), still code does not work.

Send ioremap() version (or any version that actually works).

> > Another observation is that should you have wmb() before outb() to make
> > sure that all the write operations are complete?
> > 
> I think you meant to say add wmb() 'after' outb() and not before.

Yes, that is correct.

> 
> > > +
> > > +/**
> > > + * enable_lpc_clk_run() - set LPC CLKRUN_EN i.e. clocks can be turned
> > > +off  */ static void enable_lpc_clk_run(void) {
> > > +	u32 clkrun_val;
> > > +	void __iomem *ilb_base_addr = NULL;
> > > +
> > > +	ilb_base_addr = (void __iomem *)
> > > +		kmap_atomic_pfn(INTEL_LEGACY_BLK_BASE_ADDR >>
> > PAGE_SHIFT);
> > > +
> > > +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> > > +
> > > +	/* Enable LPC CLKRUN# */
> > > +	clkrun_val |= LPC_CLKRUN_EN;
> > > +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> > > +
> > > +	kunmap_atomic(ilb_base_addr);
> > > +	/*
> > > +	 * Write any random value on port 0x80 which is on LPC, to make
> > > +	 * sure LPC clock is running before sending any TPM command.
> > > +	 */
> > > +	outb(0x80, 0xCC);
> > > +}
> > > +#else
> > > +static void disable_lpc_clk_run(void) { } static void
> > > +enable_lpc_clk_run(void) { } #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);
> > >
> > > +	if (is_bsw())
> > > +		disable_lpc_clk_run();
> > 
> > Do is_bsw() instead inside so that this can be unconditionally called.
> > 
> 
> Ok, will do.
> 
> > > +
> > >  	while (len--)
> > >  		*result++ = ioread8(phy->iobase + addr);
> > > +
> > > +	if (is_bsw())
> > > +		enable_lpc_clk_run();
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -104,8 +170,15 @@ static int tpm_tcg_write_bytes(struct
> > > tpm_tis_data *data, u32 addr, u16 len,  {
> > >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> > >
> > > +	if (is_bsw())
> > > +		disable_lpc_clk_run();
> > > +
> > >  	while (len--)
> > >  		iowrite8(*value++, phy->iobase + addr);
> > > +
> > > +	if (is_bsw())
> > > +		enable_lpc_clk_run();
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -113,7 +186,14 @@ static int tpm_tcg_read16(struct tpm_tis_data
> > > *data, u32 addr, u16 *result)  {
> > >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> > >
> > > +	if (is_bsw())
> > > +		disable_lpc_clk_run();
> > > +
> > >  	*result = ioread16(phy->iobase + addr);
> > > +
> > > +	if (is_bsw())
> > > +		enable_lpc_clk_run();
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -121,7 +201,14 @@ static int tpm_tcg_read32(struct tpm_tis_data
> > > *data, u32 addr, u32 *result)  {
> > >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> > >
> > > +	if (is_bsw())
> > > +		disable_lpc_clk_run();
> > > +
> > >  	*result = ioread32(phy->iobase + addr);
> > > +
> > > +	if (is_bsw())
> > > +		enable_lpc_clk_run();
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -129,7 +216,14 @@ static int tpm_tcg_write32(struct tpm_tis_data
> > > *data, u32 addr, u32 value)  {
> > >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> > >
> > > +	if (is_bsw())
> > > +		disable_lpc_clk_run();
> > > +
> > >  	iowrite32(value, phy->iobase + addr);
> > > +
> > > +	if (is_bsw())
> > > +		enable_lpc_clk_run();
> > > +
> > >  	return 0;
> > >  }
> > >
> > > --
> > > 1.9.1
> > >
> > 
> > /Jarkko

/Jarkko

[PATCH v3] tpm: Enable CLKRUN protocol for Braswell systems

[Azhar Shaikh]

To overcome a hardware limitation on Intel Braswell systems,
disable CLKRUN protocol during TPM transactions and re-enable
once the transaction is completed.

Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
---
Changes from v1:
- Add CONFIG_X86 around disable_lpc_clk_run () and enable_lpc_clk_run() to avoid
- build breakage on architectures which do not implement kmap_atomic_pfn()

Changes from v2:
- Use ioremap()/iounmap() instead of kmap_atomic_pfn()/kunmap_atomic()
- Move is_bsw() and all macros from tpm.h to tpm_tis.c file.
- Add the is_bsw() check in disable_lpc_clk_run() and enable_lpc_clk_run()
- instead of adding it in each read/write API.

 drivers/char/tpm/tpm.h     |   4 ++
 drivers/char/tpm/tpm_tis.c | 103 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4b4c8dee3096..6b769fbc4407 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -36,6 +36,10 @@
 #include <linux/highmem.h>
 #include <crypto/hash_info.h>
 
+#ifdef CONFIG_X86
+#include <asm/intel-family.h>
+#endif
+
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */
 	TPM_BUFSIZE = 4096,
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index c7e1384f1b08..6e32b4c7c70d 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -89,13 +89,89 @@ static inline int is_itpm(struct acpi_device *dev)
 }
 #endif
 
+#ifdef CONFIG_X86
+static inline bool is_bsw(void)
+{
+	return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0);
+}
+#else
+static inline bool is_bsw(void)
+{
+	return false;
+}
+#endif
+
+#define INTEL_LEGACY_BLK_BASE_ADDR      0xFED08000
+#define ILB_REMAP_SIZE			0x100
+#define LPC_CNTRL_REG_OFFSET            0x84
+#define LPC_CLKRUN_EN                   (1 << 2)
+
+void __iomem *ilb_base_addr;
+
+/**
+ * disable_lpc_clk_run() - clear LPC CLKRUN_EN i.e. clocks will be free running
+ */
+static void disable_lpc_clk_run(void)
+{
+	u32 clkrun_val;
+
+	if (!is_bsw())
+		return;
+
+	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	/* Disable LPC CLKRUN# */
+	clkrun_val &= ~LPC_CLKRUN_EN;
+	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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(0x80, 0xCC);
+
+	/* Make sure the above write is completed */
+	wmb();
+}
+
+/**
+ * enable_lpc_clk_run() - set LPC CLKRUN_EN i.e. clocks can be turned off
+ */
+static void enable_lpc_clk_run(void)
+{
+	u32 clkrun_val;
+
+	if (!is_bsw())
+		return;
+
+	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	/* Enable LPC CLKRUN# */
+	clkrun_val |= LPC_CLKRUN_EN;
+	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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(0x80, 0xCC);
+
+	/* Make sure the above write is completed */
+	wmb();
+}
+
 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);
 
+	disable_lpc_clk_run();
+
 	while (len--)
 		*result++ = ioread8(phy->iobase + addr);
+
+	enable_lpc_clk_run();
+
 	return 0;
 }
 
@@ -104,8 +180,13 @@ static int tpm_tcg_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	disable_lpc_clk_run();
+
 	while (len--)
 		iowrite8(*value++, phy->iobase + addr);
+
+	enable_lpc_clk_run();
+
 	return 0;
 }
 
@@ -113,7 +194,12 @@ static int tpm_tcg_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	disable_lpc_clk_run();
+
 	*result = ioread16(phy->iobase + addr);
+
+	enable_lpc_clk_run();
+
 	return 0;
 }
 
@@ -121,7 +207,12 @@ static int tpm_tcg_read32(struct tpm_tis_data *data, u32 addr, u32 *result)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	disable_lpc_clk_run();
+
 	*result = ioread32(phy->iobase + addr);
+
+	enable_lpc_clk_run();
+
 	return 0;
 }
 
@@ -129,7 +220,12 @@ static int tpm_tcg_write32(struct tpm_tis_data *data, u32 addr, u32 value)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	disable_lpc_clk_run();
+
 	iowrite32(value, phy->iobase + addr);
+
+	enable_lpc_clk_run();
+
 	return 0;
 }
 
@@ -191,6 +287,10 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
 		acpi_dev_handle = ACPI_HANDLE(&pnp_dev->dev);
 	}
 
+	if (is_bsw())
+		ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
+					ILB_REMAP_SIZE);
+
 	return tpm_tis_init(&pnp_dev->dev, &tpm_info, acpi_dev_handle);
 }
 
@@ -214,6 +314,9 @@ static void tpm_tis_pnp_remove(struct pnp_dev *dev)
 
 	tpm_chip_unregister(chip);
 	tpm_tis_remove(chip);
+
+	if (is_bsw())
+		iounmap(ilb_base_addr);
 }
 
 static struct pnp_driver tis_pnp_driver = {
-- 
1.9.1

Re: [PATCH v3] tpm: Enable CLKRUN protocol for Braswell systems

[Alan Cox]

> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -89,13 +89,89 @@ static inline int is_itpm(struct acpi_device *dev)
>  }
>  #endif
>  
> +#ifdef CONFIG_X86
> +static inline bool is_bsw(void)
> +{
> +	return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0);
> +}
> +#else
> +static inline bool is_bsw(void)
> +{
> +	return false;
> +}
> +#endif

This isn't the only bit that is x86 specific

> +
> +#define INTEL_LEGACY_BLK_BASE_ADDR      0xFED08000
> +#define ILB_REMAP_SIZE			0x100
> +#define LPC_CNTRL_REG_OFFSET            0x84
> +#define LPC_CLKRUN_EN                   (1 << 2)
> +
> +void __iomem *ilb_base_addr;
> +
> +/**
> + * disable_lpc_clk_run() - clear LPC CLKRUN_EN i.e. clocks will be free running
> + */
> +static void disable_lpc_clk_run(void)
> +{
> +	u32 clkrun_val;
> +
> +	if (!is_bsw())
> +		return;
> +
> +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> +
> +	/* Disable LPC CLKRUN# */
> +	clkrun_val &= ~LPC_CLKRUN_EN;
> +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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(0x80, 0xCC);
> +
> +	/* Make sure the above write is completed */
> +	wmb();

Why the wmb(). It doesn't do what the comment says! Also this code is x86
specific


> +}
> +
> +/**
> + * enable_lpc_clk_run() - set LPC CLKRUN_EN i.e. clocks can be turned off
> + */
> +static void enable_lpc_clk_run(void)
> +{
> +	u32 clkrun_val;
> +
> +	if (!is_bsw())
> +		return;
> +
> +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> +
> +	/* Enable LPC CLKRUN# */
> +	clkrun_val |= LPC_CLKRUN_EN;
> +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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(0x80, 0xCC);
> +
> +	/* Make sure the above write is completed */
> +	wmb();
> +}

Same

> +
>  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);
>  
> +	disable_lpc_clk_run();
> +
>  	while (len--)
>  		*result++ = ioread8(phy->iobase + addr);
> +
> +	enable_lpc_clk_run();
> +
>  	return 0;
>  }

So what you actually want to do is fold all the errata crap into an x86
specific chunk and just define disable/enable_lpc_clk_run() as null
functions on everything else.

I'd pick better names too - if other platforms need a hook here it won't
I imagine be about LPC. Possibly you want names like 

	platform_begin_tpm_xfer(data);
	platform_end_tpm_xfer(data);

>  
> @@ -104,8 +180,13 @@ static int tpm_tcg_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	disable_lpc_clk_run();
> +
>  	while (len--)
>  		iowrite8(*value++, phy->iobase + addr);
> +
> +	enable_lpc_clk_run();
> +
>  	return 0;
>  }
>  
> @@ -113,7 +194,12 @@ static int tpm_tcg_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	disable_lpc_clk_run();
> +
>  	*result = ioread16(phy->iobase + addr);
> +
> +	enable_lpc_clk_run();
> +
>  	return 0;
>  }
>  
> @@ -121,7 +207,12 @@ static int tpm_tcg_read32(struct tpm_tis_data *data, u32 addr, u32 *result)
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	disable_lpc_clk_run();
> +
>  	*result = ioread32(phy->iobase + addr);
> +
> +	enable_lpc_clk_run();
> +
>  	return 0;
>  }
>  
> @@ -129,7 +220,12 @@ static int tpm_tcg_write32(struct tpm_tis_data *data, u32 addr, u32 value)
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	disable_lpc_clk_run();
> +
>  	iowrite32(value, phy->iobase + addr);
> +
> +	enable_lpc_clk_run();
> +
>  	return 0;
>  }
>  
> @@ -191,6 +287,10 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
>  		acpi_dev_handle = ACPI_HANDLE(&pnp_dev->dev);
>  	}
>  
> +	if (is_bsw())
> +		ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
> +					ILB_REMAP_SIZE);
> +

This suggests to me that the bsw stuff wants to wrap the standard methods
because it's weird and ugly having random magic hardware globals in what
should be standard code.

>  	return tpm_tis_init(&pnp_dev->dev, &tpm_info, acpi_dev_handle);
>  }
>  
> @@ -214,6 +314,9 @@ static void tpm_tis_pnp_remove(struct pnp_dev *dev)
>  
>  	tpm_chip_unregister(chip);
>  	tpm_tis_remove(chip);
> +
> +	if (is_bsw())
> +		iounmap(ilb_base_addr);
>  }
>  
>  static struct pnp_driver tis_pnp_driver = {

RE: [PATCH v3] tpm: Enable CLKRUN protocol for Braswell systems

[Shaikh, Azhar]

> -----Original Message-----
> From: Alan Cox [mailto:gnomes@lxorguk.ukuu.org.uk]
> Sent: Wednesday, June 7, 2017 2:45 PM
> To: Shaikh, Azhar <azhar.shaikh@intel.com>
> Cc: jarkko.sakkinen@linux.intel.com; jgunthorpe@obsidianresearch.com;
> tpmdd-devel@lists.sourceforge.net; linux-kernel@vger.kernel.org; linux-
> security-module@vger.kernel.org
> Subject: Re: [PATCH v3] tpm: Enable CLKRUN protocol for Braswell systems
> 
> > +++ b/drivers/char/tpm/tpm_tis.c
> > @@ -89,13 +89,89 @@ static inline int is_itpm(struct acpi_device *dev)
> > }  #endif
> >
> > +#ifdef CONFIG_X86
> > +static inline bool is_bsw(void)
> > +{
> > +	return ((boot_cpu_data.x86_model ==
> INTEL_FAM6_ATOM_AIRMONT) ? 1 :
> > +0); } #else static inline bool is_bsw(void) {
> > +	return false;
> > +}
> > +#endif
> 
> This isn't the only bit that is x86 specific
> 
> > +
> > +#define INTEL_LEGACY_BLK_BASE_ADDR      0xFED08000
> > +#define ILB_REMAP_SIZE			0x100
> > +#define LPC_CNTRL_REG_OFFSET            0x84
> > +#define LPC_CLKRUN_EN                   (1 << 2)
> > +
> > +void __iomem *ilb_base_addr;
> > +
> > +/**
> > + * disable_lpc_clk_run() - clear LPC CLKRUN_EN i.e. clocks will be
> > +free running  */ static void disable_lpc_clk_run(void) {
> > +	u32 clkrun_val;
> > +
> > +	if (!is_bsw())
> > +		return;
> > +
> > +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> > +
> > +	/* Disable LPC CLKRUN# */
> > +	clkrun_val &= ~LPC_CLKRUN_EN;
> > +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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(0x80, 0xCC);
> > +
> > +	/* Make sure the above write is completed */
> > +	wmb();
> 
> Why the wmb(). It doesn't do what the comment says! Also this code is x86
> specific
> 
> 

Memory barrier to enforce the order so that the outb() is completed, which ensures that the LPC clocks are running before sending any TPM command.

> > +}
> > +
> > +/**
> > + * enable_lpc_clk_run() - set LPC CLKRUN_EN i.e. clocks can be turned
> > +off  */ static void enable_lpc_clk_run(void) {
> > +	u32 clkrun_val;
> > +
> > +	if (!is_bsw())
> > +		return;
> > +
> > +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> > +
> > +	/* Enable LPC CLKRUN# */
> > +	clkrun_val |= LPC_CLKRUN_EN;
> > +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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(0x80, 0xCC);
> > +
> > +	/* Make sure the above write is completed */
> > +	wmb();
> > +}
> 
> Same
> 
> > +
> >  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);
> >
> > +	disable_lpc_clk_run();
> > +
> >  	while (len--)
> >  		*result++ = ioread8(phy->iobase + addr);
> > +
> > +	enable_lpc_clk_run();
> > +
> >  	return 0;
> >  }
> 
> So what you actually want to do is fold all the errata crap into an x86 specific
> chunk and just define disable/enable_lpc_clk_run() as null functions on
> everything else.
> 

Ok, will do.

> I'd pick better names too - if other platforms need a hook here it won't I
> imagine be about LPC. Possibly you want names like
> 
> 	platform_begin_tpm_xfer(data);
> 	platform_end_tpm_xfer(data);
> 

How about these? Since most of the functions in this driver begin with 'tpm_'
disable_lpc_clk_run()	- >	tpm_start_xfer()
enable_lpc_clk_run()	->	tpm_end_xfer()

> >
> > @@ -104,8 +180,13 @@ static int tpm_tcg_write_bytes(struct
> > tpm_tis_data *data, u32 addr, u16 len,  {
> >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> >
> > +	disable_lpc_clk_run();
> > +
> >  	while (len--)
> >  		iowrite8(*value++, phy->iobase + addr);
> > +
> > +	enable_lpc_clk_run();
> > +
> >  	return 0;
> >  }
> >
> > @@ -113,7 +194,12 @@ static int tpm_tcg_read16(struct tpm_tis_data
> > *data, u32 addr, u16 *result)  {
> >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> >
> > +	disable_lpc_clk_run();
> > +
> >  	*result = ioread16(phy->iobase + addr);
> > +
> > +	enable_lpc_clk_run();
> > +
> >  	return 0;
> >  }
> >
> > @@ -121,7 +207,12 @@ static int tpm_tcg_read32(struct tpm_tis_data
> > *data, u32 addr, u32 *result)  {
> >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> >
> > +	disable_lpc_clk_run();
> > +
> >  	*result = ioread32(phy->iobase + addr);
> > +
> > +	enable_lpc_clk_run();
> > +
> >  	return 0;
> >  }
> >
> > @@ -129,7 +220,12 @@ static int tpm_tcg_write32(struct tpm_tis_data
> > *data, u32 addr, u32 value)  {
> >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> >
> > +	disable_lpc_clk_run();
> > +
> >  	iowrite32(value, phy->iobase + addr);
> > +
> > +	enable_lpc_clk_run();
> > +
> >  	return 0;
> >  }
> >
> > @@ -191,6 +287,10 @@ static int tpm_tis_pnp_init(struct pnp_dev
> *pnp_dev,
> >  		acpi_dev_handle = ACPI_HANDLE(&pnp_dev->dev);
> >  	}
> >
> > +	if (is_bsw())
> > +		ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
> > +					ILB_REMAP_SIZE);
> > +
> 
> This suggests to me that the bsw stuff wants to wrap the standard methods
> because it's weird and ugly having random magic hardware globals in what
> should be standard code.
> 
> >  	return tpm_tis_init(&pnp_dev->dev, &tpm_info, acpi_dev_handle);
> }
> >
> > @@ -214,6 +314,9 @@ static void tpm_tis_pnp_remove(struct pnp_dev
> > *dev)
> >
> >  	tpm_chip_unregister(chip);
> >  	tpm_tis_remove(chip);
> > +
> > +	if (is_bsw())
> > +		iounmap(ilb_base_addr);
> >  }
> >
> >  static struct pnp_driver tis_pnp_driver = {

Re: [PATCH v3] tpm: Enable CLKRUN protocol for Braswell systems

[Jarkko Sakkinen]

On Thu, Jun 08, 2017 at 01:11:43AM +0000, Shaikh, Azhar wrote:
> 
> 
> > -----Original Message-----
> > From: Alan Cox [mailto:gnomes@lxorguk.ukuu.org.uk]
> > Sent: Wednesday, June 7, 2017 2:45 PM
> > To: Shaikh, Azhar <azhar.shaikh@intel.com>
> > Cc: jarkko.sakkinen@linux.intel.com; jgunthorpe@obsidianresearch.com;
> > tpmdd-devel@lists.sourceforge.net; linux-kernel@vger.kernel.org; linux-
> > security-module@vger.kernel.org
> > Subject: Re: [PATCH v3] tpm: Enable CLKRUN protocol for Braswell systems
> > 
> > > +++ b/drivers/char/tpm/tpm_tis.c
> > > @@ -89,13 +89,89 @@ static inline int is_itpm(struct acpi_device *dev)
> > > }  #endif
> > >
> > > +#ifdef CONFIG_X86
> > > +static inline bool is_bsw(void)
> > > +{
> > > +	return ((boot_cpu_data.x86_model ==
> > INTEL_FAM6_ATOM_AIRMONT) ? 1 :
> > > +0); } #else static inline bool is_bsw(void) {
> > > +	return false;
> > > +}
> > > +#endif
> > 
> > This isn't the only bit that is x86 specific
> > 
> > > +
> > > +#define INTEL_LEGACY_BLK_BASE_ADDR      0xFED08000
> > > +#define ILB_REMAP_SIZE			0x100
> > > +#define LPC_CNTRL_REG_OFFSET            0x84
> > > +#define LPC_CLKRUN_EN                   (1 << 2)
> > > +
> > > +void __iomem *ilb_base_addr;
> > > +
> > > +/**
> > > + * disable_lpc_clk_run() - clear LPC CLKRUN_EN i.e. clocks will be
> > > +free running  */ static void disable_lpc_clk_run(void) {
> > > +	u32 clkrun_val;
> > > +
> > > +	if (!is_bsw())
> > > +		return;
> > > +
> > > +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> > > +
> > > +	/* Disable LPC CLKRUN# */
> > > +	clkrun_val &= ~LPC_CLKRUN_EN;
> > > +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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(0x80, 0xCC);
> > > +
> > > +	/* Make sure the above write is completed */
> > > +	wmb();
> > 
> > Why the wmb(). It doesn't do what the comment says! Also this code is x86
> > specific
> > 
> > 
> 
> Memory barrier to enforce the order so that the outb() is completed, which ensures that the LPC clocks are running before sending any TPM command.
> 
> > > +}
> > > +
> > > +/**
> > > + * enable_lpc_clk_run() - set LPC CLKRUN_EN i.e. clocks can be turned
> > > +off  */ static void enable_lpc_clk_run(void) {
> > > +	u32 clkrun_val;
> > > +
> > > +	if (!is_bsw())
> > > +		return;
> > > +
> > > +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> > > +
> > > +	/* Enable LPC CLKRUN# */
> > > +	clkrun_val |= LPC_CLKRUN_EN;
> > > +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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(0x80, 0xCC);
> > > +
> > > +	/* Make sure the above write is completed */
> > > +	wmb();
> > > +}
> > 
> > Same
> > 
> > > +
> > >  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);
> > >
> > > +	disable_lpc_clk_run();
> > > +
> > >  	while (len--)
> > >  		*result++ = ioread8(phy->iobase + addr);
> > > +
> > > +	enable_lpc_clk_run();
> > > +
> > >  	return 0;
> > >  }
> > 
> > So what you actually want to do is fold all the errata crap into an x86 specific
> > chunk and just define disable/enable_lpc_clk_run() as null functions on
> > everything else.
> > 
> 
> Ok, will do.
> 
> > I'd pick better names too - if other platforms need a hook here it won't I
> > imagine be about LPC. Possibly you want names like
> > 
> > 	platform_begin_tpm_xfer(data);
> > 	platform_end_tpm_xfer(data);
> > 
> 
> How about these? Since most of the functions in this driver begin with 'tpm_'
> disable_lpc_clk_run()	- >	tpm_start_xfer()
> enable_lpc_clk_run()	->	tpm_end_xfer()

tpm_platform_begin_xfer() would be the best alternative as it highlights
platform quirk better.

/Jarkko

Re: [PATCH v3] tpm: Enable CLKRUN protocol for Braswell systems

[Alan Cox]

> > > +	outb(0x80, 0xCC);
> > > +
> > > +	/* Make sure the above write is completed */
> > > +	wmb();  
> > 
> > Why the wmb(). It doesn't do what the comment says! Also this code is x86
> > specific
> > 
> >   
> 
> Memory barrier to enforce the order so that the outb() is completed, which ensures that the LPC clocks are running before sending any TPM command.

wmb() doesn't do that. It merely ensures that the write has been posted
to the fabric. If as I suspect your LPC bus implements outb() as a
non-posted write you don't need the wmb(). If it doesn't then you need to
issue whatever access is needed to the fabric to ensure the post
completed (eg for PCI if you do an MMIO write you must do an MMIO read
from the same devfn).

Secondly outb(0x80, 0xCC) doesn't write 0xCC to port 0x80. It writes 0x80
to port 0xCC !

Alan

Re: [PATCH v3] tpm: Enable CLKRUN protocol for Braswell systems

[Jason Gunthorpe]

On Thu, Jun 08, 2017 at 07:22:59PM +0100, Alan Cox wrote:
> > > > +	outb(0x80, 0xCC);
> > > > +
> > > > +	/* Make sure the above write is completed */
> > > > +	wmb();  
> > > 
> > > Why the wmb(). It doesn't do what the comment says! Also this code is x86
> > > specific
> > > 
> > >   
> > 
> > Memory barrier to enforce the order so that the outb() is
> > completed, which ensures that the LPC clocks are running before
> > sending any TPM command.
>
> wmb() doesn't do that. It merely ensures that the write has been posted
> to the fabric. If as I suspect your LPC bus implements outb() as a
> non-posted write you don't need the wmb().

I think the point here is to bootstrap the sleeping LPC bus clock
before a TPM command is issued, presumably because the auto-wakeup circuit
is busted or something.

For that purpose all that should be required is strong ordering of the
outb relative to the other TPM commands at the LPC interface FIFO. I
also think the wmb is not needed because outb is already defined to be
strongly in order with respect to writel/readl ?

Jason

Re: [PATCH v3] tpm: Enable CLKRUN protocol for Braswell systems

[Alan Cox]

> For that purpose all that should be required is strong ordering of the
> outb relative to the other TPM commands at the LPC interface FIFO. I
> also think the wmb is not needed because outb is already defined to be
> strongly in order with respect to writel/readl ?

That's my assumption but given this is all some kind of 'it's broken'
fixup I thought best to ask. Assuming there is nothing else magical going
on then yes it should be deleted.

Alan

RE: [PATCH v3] tpm: Enable CLKRUN protocol for Braswell systems

[Shaikh, Azhar]

> -----Original Message-----
> From: Alan Cox [mailto:gnomes@lxorguk.ukuu.org.uk]
> Sent: Thursday, June 8, 2017 11:23 AM
> To: Shaikh, Azhar <azhar.shaikh@intel.com>
> Cc: jarkko.sakkinen@linux.intel.com; jgunthorpe@obsidianresearch.com;
> tpmdd-devel@lists.sourceforge.net; linux-kernel@vger.kernel.org; linux-
> security-module@vger.kernel.org
> Subject: Re: [PATCH v3] tpm: Enable CLKRUN protocol for Braswell systems
> 
> > > > +	outb(0x80, 0xCC);
> > > > +
> > > > +	/* Make sure the above write is completed */
> > > > +	wmb();
> > >
> > > Why the wmb(). It doesn't do what the comment says! Also this code
> > > is x86 specific
> > >
> > >
> >
> > Memory barrier to enforce the order so that the outb() is completed,
> which ensures that the LPC clocks are running before sending any TPM
> command.
> 
> wmb() doesn't do that. It merely ensures that the write has been posted to
> the fabric. If as I suspect your LPC bus implements outb() as a non-posted
> write you don't need the wmb(). If it doesn't then you need to issue
> whatever access is needed to the fabric to ensure the post completed (eg for
> PCI if you do an MMIO write you must do an MMIO read from the same
> devfn).
> 
> Secondly outb(0x80, 0xCC) doesn't write 0xCC to port 0x80. It writes 0x80 to
> port 0xCC !
> 

Oops my bad! I got that reversed. Will change it.

> Alan

RE: [PATCH v3] tpm: Enable CLKRUN protocol for Braswell systems

[Shaikh, Azhar]

> -----Original Message-----
> From: Alan Cox [mailto:gnomes@lxorguk.ukuu.org.uk]
> Sent: Thursday, June 8, 2017 11:50 AM
> To: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Cc: Shaikh, Azhar <azhar.shaikh@intel.com>;
> jarkko.sakkinen@linux.intel.com; tpmdd-devel@lists.sourceforge.net; linux-
> kernel@vger.kernel.org; linux-security-module@vger.kernel.org
> Subject: Re: [PATCH v3] tpm: Enable CLKRUN protocol for Braswell systems
> 
> > For that purpose all that should be required is strong ordering of the
> > outb relative to the other TPM commands at the LPC interface FIFO. I
> > also think the wmb is not needed because outb is already defined to be
> > strongly in order with respect to writel/readl ?
> 
> That's my assumption but given this is all some kind of 'it's broken'
> fixup I thought best to ask. Assuming there is nothing else magical going on
> then yes it should be deleted.
> 

As Jason mentioned, outb is already define to be strongly ordered, then wmb is not needed.
I will delete it.

> Alan

[PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems

[Azhar Shaikh]

To overcome a hardware limitation on Intel Braswell systems,
disable CLKRUN protocol during TPM transactions and re-enable
once the transaction is completed.

Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
---
Changes from v1:
- Add CONFIG_X86 around disable_lpc_clk_run () and enable_lpc_clk_run() to avoid
- build breakage on architectures which do not implement kmap_atomic_pfn()

Changes from v2:
- Use ioremap()/iounmap() instead of kmap_atomic_pfn()/kunmap_atomic()
- Move is_bsw() and all macros from tpm.h to tpm_tis.c file.
- Add the is_bsw() check in disable_lpc_clk_run() and enable_lpc_clk_run()
- instead of adding it in each read/write API.

Changes from v3:
- Move the code under #ifdef CONFIG_X86 and create stub functions for everything else
- Rename the functions disable_lpc_clk_run() -> tpm_platform_begin_xfer() and
- enable_lpc_clk_run() -> tpm_platform_end_xfer()
- Remove wmb()

 drivers/char/tpm/tpm.h     |   4 ++
 drivers/char/tpm/tpm_tis.c | 112 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 116 insertions(+)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4b4c8dee3096..6b769fbc4407 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -36,6 +36,10 @@
 #include <linux/highmem.h>
 #include <crypto/hash_info.h>
 
+#ifdef CONFIG_X86
+#include <asm/intel-family.h>
+#endif
+
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */
 	TPM_BUFSIZE = 4096,
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index c7e1384f1b08..ce6f1a8b0510 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -89,13 +89,93 @@ static inline int is_itpm(struct acpi_device *dev)
 }
 #endif
 
+#ifdef CONFIG_X86
+#define INTEL_LEGACY_BLK_BASE_ADDR      0xFED08000
+#define ILB_REMAP_SIZE			0x100
+#define LPC_CNTRL_REG_OFFSET            0x84
+#define LPC_CLKRUN_EN                   (1 << 2)
+
+void __iomem *ilb_base_addr;
+
+static inline bool is_bsw(void)
+{
+	return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0);
+}
+
+/**
+ * tpm_platform_begin_xfer() - clear LPC CLKRUN_EN i.e. clocks will be running
+ */
+static void tpm_platform_begin_xfer(void)
+{
+	u32 clkrun_val;
+
+	if (!is_bsw())
+		return;
+
+	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	/* Disable LPC CLKRUN# */
+	clkrun_val &= ~LPC_CLKRUN_EN;
+	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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(void)
+{
+	u32 clkrun_val;
+
+	if (!is_bsw())
+		return;
+
+	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	/* Enable LPC CLKRUN# */
+	clkrun_val |= LPC_CLKRUN_EN;
+	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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 inline bool is_bsw(void)
+{
+	return false;
+}
+
+static void tpm_platform_begin_xfer(void)
+{
+}
+
+static void tpm_platform_end_xfer(void)
+{
+}
+#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();
+
 	while (len--)
 		*result++ = ioread8(phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -104,8 +184,13 @@ static int tpm_tcg_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	tpm_platform_begin_xfer();
+
 	while (len--)
 		iowrite8(*value++, phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -113,7 +198,12 @@ static int tpm_tcg_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	tpm_platform_begin_xfer();
+
 	*result = ioread16(phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -121,7 +211,12 @@ static int tpm_tcg_read32(struct tpm_tis_data *data, u32 addr, u32 *result)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	tpm_platform_begin_xfer();
+
 	*result = ioread32(phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -129,7 +224,12 @@ static int tpm_tcg_write32(struct tpm_tis_data *data, u32 addr, u32 value)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	tpm_platform_begin_xfer();
+
 	iowrite32(value, phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -191,6 +291,12 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
 		acpi_dev_handle = ACPI_HANDLE(&pnp_dev->dev);
 	}
 
+#ifdef CONFIG_X86
+	if (is_bsw())
+		ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
+					ILB_REMAP_SIZE);
+#endif
+
 	return tpm_tis_init(&pnp_dev->dev, &tpm_info, acpi_dev_handle);
 }
 
@@ -214,6 +320,12 @@ static void tpm_tis_pnp_remove(struct pnp_dev *dev)
 
 	tpm_chip_unregister(chip);
 	tpm_tis_remove(chip);
+
+#ifdef CONFIG_X86
+	if (is_bsw())
+		iounmap(ilb_base_addr);
+#endif
+
 }
 
 static struct pnp_driver tis_pnp_driver = {
-- 
1.9.1

Re: [PATCH v3] tpm: Enable CLKRUN protocol for Braswell systems

[Jarkko Sakkinen]

On Thu, Jun 08, 2017 at 12:39:20PM -0600, Jason Gunthorpe wrote:
> On Thu, Jun 08, 2017 at 07:22:59PM +0100, Alan Cox wrote:
> > > > > +	outb(0x80, 0xCC);
> > > > > +
> > > > > +	/* Make sure the above write is completed */
> > > > > +	wmb();  
> > > > 
> > > > Why the wmb(). It doesn't do what the comment says! Also this code is x86
> > > > specific
> > > > 
> > > >   
> > > 
> > > Memory barrier to enforce the order so that the outb() is
> > > completed, which ensures that the LPC clocks are running before
> > > sending any TPM command.
> >
> > wmb() doesn't do that. It merely ensures that the write has been posted
> > to the fabric. If as I suspect your LPC bus implements outb() as a
> > non-posted write you don't need the wmb().
> 
> I think the point here is to bootstrap the sleeping LPC bus clock
> before a TPM command is issued, presumably because the auto-wakeup circuit
> is busted or something.
> 
> For that purpose all that should be required is strong ordering of the
> outb relative to the other TPM commands at the LPC interface FIFO. I
> also think the wmb is not needed because outb is already defined to be
> strongly in order with respect to writel/readl ?
> 
> Jason

writel AFAIK guarantees by itself strong RW ordering.

/Jarkko

Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems

[Jarkko Sakkinen]

On Thu, Jun 08, 2017 at 04:46:33PM -0700, Azhar Shaikh wrote:
> To overcome a hardware limitation on Intel Braswell systems,
> disable CLKRUN protocol during TPM transactions and re-enable
> once the transaction is completed.
> 
> Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
> ---
> Changes from v1:
> - Add CONFIG_X86 around disable_lpc_clk_run () and enable_lpc_clk_run() to avoid
> - build breakage on architectures which do not implement kmap_atomic_pfn()
> 
> Changes from v2:
> - Use ioremap()/iounmap() instead of kmap_atomic_pfn()/kunmap_atomic()
> - Move is_bsw() and all macros from tpm.h to tpm_tis.c file.
> - Add the is_bsw() check in disable_lpc_clk_run() and enable_lpc_clk_run()
> - instead of adding it in each read/write API.
> 
> Changes from v3:
> - Move the code under #ifdef CONFIG_X86 and create stub functions for everything else
> - Rename the functions disable_lpc_clk_run() -> tpm_platform_begin_xfer() and
> - enable_lpc_clk_run() -> tpm_platform_end_xfer()
> - Remove wmb()

The wrong parameter order in outb() is not worth of mentioning in your
opinion?

Related.

I looked again the kmap version of the patch and still cannot figure
out what could be wrong. Obviously the wrong outb() cause unexpected
behavior.

Do you have chances to grab klog from kmap version with correct outb?

/Jarkko

RE: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems

[Shaikh, Azhar]

> -----Original Message-----
> From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> Sent: Saturday, June 10, 2017 4:14 AM
> To: Shaikh, Azhar <azhar.shaikh@intel.com>
> Cc: jgunthorpe@obsidianresearch.com; tpmdd-devel@lists.sourceforge.net;
> linux-kernel@vger.kernel.org; linux-security-module@vger.kernel.org
> Subject: Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems
> 
> On Thu, Jun 08, 2017 at 04:46:33PM -0700, Azhar Shaikh wrote:
> > To overcome a hardware limitation on Intel Braswell systems, disable
> > CLKRUN protocol during TPM transactions and re-enable once the
> > transaction is completed.
> >
> > Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
> > ---
> > Changes from v1:
> > - Add CONFIG_X86 around disable_lpc_clk_run () and
> > enable_lpc_clk_run() to avoid
> > - build breakage on architectures which do not implement
> > kmap_atomic_pfn()
> >
> > Changes from v2:
> > - Use ioremap()/iounmap() instead of
> kmap_atomic_pfn()/kunmap_atomic()
> > - Move is_bsw() and all macros from tpm.h to tpm_tis.c file.
> > - Add the is_bsw() check in disable_lpc_clk_run() and
> > enable_lpc_clk_run()
> > - instead of adding it in each read/write API.
> >
> > Changes from v3:
> > - Move the code under #ifdef CONFIG_X86 and create stub functions for
> > everything else
> > - Rename the functions disable_lpc_clk_run() ->
> > tpm_platform_begin_xfer() and
> > - enable_lpc_clk_run() -> tpm_platform_end_xfer()
> > - Remove wmb()
> 
> The wrong parameter order in outb() is not worth of mentioning in your
> opinion?
> 

Oh yes! It is definitely worth mentioning. I forgot to mention that.
Will definitely mention it in the changelog for the next version if any or will re send v4 with updated changelog for v3.

> Related.
> 
> I looked again the kmap version of the patch and still cannot figure out what
> could be wrong. Obviously the wrong outb() cause unexpected behavior.
> 
> Do you have chances to grab klog from kmap version with correct outb?
> 

I have not tried the kmap version with corrected outb(). Will give that a try.

> /Jarkko

Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems

[Jarkko Sakkinen]

On Sat, Jun 10, 2017 at 04:35:50PM +0000, Shaikh, Azhar wrote:
> 
> 
> > -----Original Message-----
> > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > Sent: Saturday, June 10, 2017 4:14 AM
> > To: Shaikh, Azhar <azhar.shaikh@intel.com>
> > Cc: jgunthorpe@obsidianresearch.com; tpmdd-devel@lists.sourceforge.net;
> > linux-kernel@vger.kernel.org; linux-security-module@vger.kernel.org
> > Subject: Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems
> > 
> > On Thu, Jun 08, 2017 at 04:46:33PM -0700, Azhar Shaikh wrote:
> > > To overcome a hardware limitation on Intel Braswell systems, disable
> > > CLKRUN protocol during TPM transactions and re-enable once the
> > > transaction is completed.
> > >
> > > Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
> > > ---
> > > Changes from v1:
> > > - Add CONFIG_X86 around disable_lpc_clk_run () and
> > > enable_lpc_clk_run() to avoid
> > > - build breakage on architectures which do not implement
> > > kmap_atomic_pfn()
> > >
> > > Changes from v2:
> > > - Use ioremap()/iounmap() instead of
> > kmap_atomic_pfn()/kunmap_atomic()
> > > - Move is_bsw() and all macros from tpm.h to tpm_tis.c file.
> > > - Add the is_bsw() check in disable_lpc_clk_run() and
> > > enable_lpc_clk_run()
> > > - instead of adding it in each read/write API.
> > >
> > > Changes from v3:
> > > - Move the code under #ifdef CONFIG_X86 and create stub functions for
> > > everything else
> > > - Rename the functions disable_lpc_clk_run() ->
> > > tpm_platform_begin_xfer() and
> > > - enable_lpc_clk_run() -> tpm_platform_end_xfer()
> > > - Remove wmb()
> > 
> > The wrong parameter order in outb() is not worth of mentioning in your
> > opinion?
> > 
> 
> Oh yes! It is definitely worth mentioning. I forgot to mention that.
> Will definitely mention it in the changelog for the next version if any or will re send v4 with updated changelog for v3.
> 
> > Related.
> > 
> > I looked again the kmap version of the patch and still cannot figure out what
> > could be wrong. Obviously the wrong outb() cause unexpected behavior.
> > 
> > Do you have chances to grab klog from kmap version with correct outb?
> > 
> 
> I have not tried the kmap version with corrected outb(). Will give that a try.
> 
> > /Jarkko

Thank you!

/Jarkko

RE: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems

[Shaikh, Azhar]

> -----Original Message-----
> From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> Sent: Monday, June 12, 2017 12:50 AM
> To: Shaikh, Azhar <azhar.shaikh@intel.com>
> Cc: jgunthorpe@obsidianresearch.com; tpmdd-devel@lists.sourceforge.net;
> linux-kernel@vger.kernel.org; linux-security-module@vger.kernel.org
> Subject: Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems
> 
> On Sat, Jun 10, 2017 at 04:35:50PM +0000, Shaikh, Azhar wrote:
> >
> >
> > > -----Original Message-----
> > > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > > Sent: Saturday, June 10, 2017 4:14 AM
> > > To: Shaikh, Azhar <azhar.shaikh@intel.com>
> > > Cc: jgunthorpe@obsidianresearch.com;
> > > tpmdd-devel@lists.sourceforge.net;
> > > linux-kernel@vger.kernel.org; linux-security-module@vger.kernel.org
> > > Subject: Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell
> > > systems
> > >
> > > On Thu, Jun 08, 2017 at 04:46:33PM -0700, Azhar Shaikh wrote:
> > > > To overcome a hardware limitation on Intel Braswell systems,
> > > > disable CLKRUN protocol during TPM transactions and re-enable once
> > > > the transaction is completed.
> > > >
> > > > Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
> > > > ---
> > > > Changes from v1:
> > > > - Add CONFIG_X86 around disable_lpc_clk_run () and
> > > > enable_lpc_clk_run() to avoid
> > > > - build breakage on architectures which do not implement
> > > > kmap_atomic_pfn()
> > > >
> > > > Changes from v2:
> > > > - Use ioremap()/iounmap() instead of
> > > kmap_atomic_pfn()/kunmap_atomic()
> > > > - Move is_bsw() and all macros from tpm.h to tpm_tis.c file.
> > > > - Add the is_bsw() check in disable_lpc_clk_run() and
> > > > enable_lpc_clk_run()
> > > > - instead of adding it in each read/write API.
> > > >
> > > > Changes from v3:
> > > > - Move the code under #ifdef CONFIG_X86 and create stub functions
> > > > for everything else
> > > > - Rename the functions disable_lpc_clk_run() ->
> > > > tpm_platform_begin_xfer() and
> > > > - enable_lpc_clk_run() -> tpm_platform_end_xfer()
> > > > - Remove wmb()
> > >
> > > The wrong parameter order in outb() is not worth of mentioning in
> > > your opinion?
> > >
> >
> > Oh yes! It is definitely worth mentioning. I forgot to mention that.
> > Will definitely mention it in the changelog for the next version if any or will
> re send v4 with updated changelog for v3.
> >
> > > Related.
> > >
> > > I looked again the kmap version of the patch and still cannot figure
> > > out what could be wrong. Obviously the wrong outb() cause unexpected
> behavior.
> > >
> > > Do you have chances to grab klog from kmap version with correct outb?
> > >
> >
> > I have not tried the kmap version with corrected outb(). Will give that a try.
> >
> > > /Jarkko
> 
> Thank you!
> 

Even with the corrected outb(), kmap version fails to boot. Could not get boot logs :-(

> /Jarkko

Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems

[Jarkko Sakkinen]

On Wed, Jun 14, 2017 at 12:51:25AM +0000, Shaikh, Azhar wrote:
> 
> 
> > -----Original Message-----
> > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > Sent: Monday, June 12, 2017 12:50 AM
> > To: Shaikh, Azhar <azhar.shaikh@intel.com>
> > Cc: jgunthorpe@obsidianresearch.com; tpmdd-devel@lists.sourceforge.net;
> > linux-kernel@vger.kernel.org; linux-security-module@vger.kernel.org
> > Subject: Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems
> > 
> > On Sat, Jun 10, 2017 at 04:35:50PM +0000, Shaikh, Azhar wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > > > Sent: Saturday, June 10, 2017 4:14 AM
> > > > To: Shaikh, Azhar <azhar.shaikh@intel.com>
> > > > Cc: jgunthorpe@obsidianresearch.com;
> > > > tpmdd-devel@lists.sourceforge.net;
> > > > linux-kernel@vger.kernel.org; linux-security-module@vger.kernel.org
> > > > Subject: Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell
> > > > systems
> > > >
> > > > On Thu, Jun 08, 2017 at 04:46:33PM -0700, Azhar Shaikh wrote:
> > > > > To overcome a hardware limitation on Intel Braswell systems,
> > > > > disable CLKRUN protocol during TPM transactions and re-enable once
> > > > > the transaction is completed.
> > > > >
> > > > > Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
> > > > > ---
> > > > > Changes from v1:
> > > > > - Add CONFIG_X86 around disable_lpc_clk_run () and
> > > > > enable_lpc_clk_run() to avoid
> > > > > - build breakage on architectures which do not implement
> > > > > kmap_atomic_pfn()
> > > > >
> > > > > Changes from v2:
> > > > > - Use ioremap()/iounmap() instead of
> > > > kmap_atomic_pfn()/kunmap_atomic()
> > > > > - Move is_bsw() and all macros from tpm.h to tpm_tis.c file.
> > > > > - Add the is_bsw() check in disable_lpc_clk_run() and
> > > > > enable_lpc_clk_run()
> > > > > - instead of adding it in each read/write API.
> > > > >
> > > > > Changes from v3:
> > > > > - Move the code under #ifdef CONFIG_X86 and create stub functions
> > > > > for everything else
> > > > > - Rename the functions disable_lpc_clk_run() ->
> > > > > tpm_platform_begin_xfer() and
> > > > > - enable_lpc_clk_run() -> tpm_platform_end_xfer()
> > > > > - Remove wmb()
> > > >
> > > > The wrong parameter order in outb() is not worth of mentioning in
> > > > your opinion?
> > > >
> > >
> > > Oh yes! It is definitely worth mentioning. I forgot to mention that.
> > > Will definitely mention it in the changelog for the next version if any or will
> > re send v4 with updated changelog for v3.
> > >
> > > > Related.
> > > >
> > > > I looked again the kmap version of the patch and still cannot figure
> > > > out what could be wrong. Obviously the wrong outb() cause unexpected
> > behavior.
> > > >
> > > > Do you have chances to grab klog from kmap version with correct outb?
> > > >
> > >
> > > I have not tried the kmap version with corrected outb(). Will give that a try.
> > >
> > > > /Jarkko
> > 
> > Thank you!
> > 
> 
> Even with the corrected outb(), kmap version fails to boot. Could not get boot logs :-(
> 
> > /Jarkko

Lets go with the ioremap() version for now.

/Jarkko

RE: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems

[Shaikh, Azhar]

> -----Original Message-----
> From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> Sent: Wednesday, June 14, 2017 7:46 AM
> To: Shaikh, Azhar <azhar.shaikh@intel.com>
> Cc: jgunthorpe@obsidianresearch.com; tpmdd-devel@lists.sourceforge.net;
> linux-kernel@vger.kernel.org; linux-security-module@vger.kernel.org
> Subject: Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems
> 
> On Wed, Jun 14, 2017 at 12:51:25AM +0000, Shaikh, Azhar wrote:
> >
> >
> > > -----Original Message-----
> > > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > > Sent: Monday, June 12, 2017 12:50 AM
> > > To: Shaikh, Azhar <azhar.shaikh@intel.com>
> > > Cc: jgunthorpe@obsidianresearch.com;
> > > tpmdd-devel@lists.sourceforge.net;
> > > linux-kernel@vger.kernel.org; linux-security-module@vger.kernel.org
> > > Subject: Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell
> > > systems
> > >
> > > On Sat, Jun 10, 2017 at 04:35:50PM +0000, Shaikh, Azhar wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > > > > Sent: Saturday, June 10, 2017 4:14 AM
> > > > > To: Shaikh, Azhar <azhar.shaikh@intel.com>
> > > > > Cc: jgunthorpe@obsidianresearch.com;
> > > > > tpmdd-devel@lists.sourceforge.net;
> > > > > linux-kernel@vger.kernel.org;
> > > > > linux-security-module@vger.kernel.org
> > > > > Subject: Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell
> > > > > systems
> > > > >
> > > > > On Thu, Jun 08, 2017 at 04:46:33PM -0700, Azhar Shaikh wrote:
> > > > > > To overcome a hardware limitation on Intel Braswell systems,
> > > > > > disable CLKRUN protocol during TPM transactions and re-enable
> > > > > > once the transaction is completed.
> > > > > >
> > > > > > Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
> > > > > > ---
> > > > > > Changes from v1:
> > > > > > - Add CONFIG_X86 around disable_lpc_clk_run () and
> > > > > > enable_lpc_clk_run() to avoid
> > > > > > - build breakage on architectures which do not implement
> > > > > > kmap_atomic_pfn()
> > > > > >
> > > > > > Changes from v2:
> > > > > > - Use ioremap()/iounmap() instead of
> > > > > kmap_atomic_pfn()/kunmap_atomic()
> > > > > > - Move is_bsw() and all macros from tpm.h to tpm_tis.c file.
> > > > > > - Add the is_bsw() check in disable_lpc_clk_run() and
> > > > > > enable_lpc_clk_run()
> > > > > > - instead of adding it in each read/write API.
> > > > > >
> > > > > > Changes from v3:
> > > > > > - Move the code under #ifdef CONFIG_X86 and create stub
> > > > > > functions for everything else
> > > > > > - Rename the functions disable_lpc_clk_run() ->
> > > > > > tpm_platform_begin_xfer() and
> > > > > > - enable_lpc_clk_run() -> tpm_platform_end_xfer()
> > > > > > - Remove wmb()
> > > > >
> > > > > The wrong parameter order in outb() is not worth of mentioning
> > > > > in your opinion?
> > > > >
> > > >
> > > > Oh yes! It is definitely worth mentioning. I forgot to mention that.
> > > > Will definitely mention it in the changelog for the next version
> > > > if any or will
> > > re send v4 with updated changelog for v3.
> > > >
> > > > > Related.
> > > > >
> > > > > I looked again the kmap version of the patch and still cannot
> > > > > figure out what could be wrong. Obviously the wrong outb() cause
> > > > > unexpected
> > > behavior.
> > > > >
> > > > > Do you have chances to grab klog from kmap version with correct
> outb?
> > > > >
> > > >
> > > > I have not tried the kmap version with corrected outb(). Will give that a
> try.
> > > >
> > > > > /Jarkko
> > >
> > > Thank you!
> > >
> >
> > Even with the corrected outb(), kmap version fails to boot. Could not
> > get boot logs :-(
> >
> > > /Jarkko
> 
> Lets go with the ioremap() version for now.
> 

Okay. I will re-send the same [PATCH v4] after updating the change log for v3.

> /Jarkko

[PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems

[Azhar Shaikh]

To overcome a hardware limitation on Intel Braswell systems,
disable CLKRUN protocol during TPM transactions and re-enable
once the transaction is completed.

Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
---
Changes from v1:
- Add CONFIG_X86 around disable_lpc_clk_run() and enable_lpc_clk_run() to avoid
- build breakage on architectures which do not implement kmap_atomic_pfn()

Changes from v2:
- Use ioremap()/iounmap() instead of kmap_atomic_pfn()/kunmap_atomic()
- Move is_bsw() and all macros from tpm.h to tpm_tis.c file.
- Add the is_bsw() check in disable_lpc_clk_run() and enable_lpc_clk_run()
- instead of adding it in each read/write API.

Changes from v3:
- Move the code under #ifdef CONFIG_X86 and create stub functions for everything else
- Rename the functions disable_lpc_clk_run() -> tpm_platform_begin_xfer() and
- enable_lpc_clk_run() -> tpm_platform_end_xfer()
- Remove wmb()
- Correct the parameters for outb()

 drivers/char/tpm/tpm.h     |   4 ++
 drivers/char/tpm/tpm_tis.c | 112 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 116 insertions(+)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4b4c8dee3096..6b769fbc4407 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -36,6 +36,10 @@
 #include <linux/highmem.h>
 #include <crypto/hash_info.h>
 
+#ifdef CONFIG_X86
+#include <asm/intel-family.h>
+#endif
+
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */
 	TPM_BUFSIZE = 4096,
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index c7e1384f1b08..ce6f1a8b0510 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -89,13 +89,93 @@ static inline int is_itpm(struct acpi_device *dev)
 }
 #endif
 
+#ifdef CONFIG_X86
+#define INTEL_LEGACY_BLK_BASE_ADDR      0xFED08000
+#define ILB_REMAP_SIZE			0x100
+#define LPC_CNTRL_REG_OFFSET            0x84
+#define LPC_CLKRUN_EN                   (1 << 2)
+
+void __iomem *ilb_base_addr;
+
+static inline bool is_bsw(void)
+{
+	return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0);
+}
+
+/**
+ * tpm_platform_begin_xfer() - clear LPC CLKRUN_EN i.e. clocks will be running
+ */
+static void tpm_platform_begin_xfer(void)
+{
+	u32 clkrun_val;
+
+	if (!is_bsw())
+		return;
+
+	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	/* Disable LPC CLKRUN# */
+	clkrun_val &= ~LPC_CLKRUN_EN;
+	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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(void)
+{
+	u32 clkrun_val;
+
+	if (!is_bsw())
+		return;
+
+	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	/* Enable LPC CLKRUN# */
+	clkrun_val |= LPC_CLKRUN_EN;
+	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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 inline bool is_bsw(void)
+{
+	return false;
+}
+
+static void tpm_platform_begin_xfer(void)
+{
+}
+
+static void tpm_platform_end_xfer(void)
+{
+}
+#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();
+
 	while (len--)
 		*result++ = ioread8(phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -104,8 +184,13 @@ static int tpm_tcg_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	tpm_platform_begin_xfer();
+
 	while (len--)
 		iowrite8(*value++, phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -113,7 +198,12 @@ static int tpm_tcg_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	tpm_platform_begin_xfer();
+
 	*result = ioread16(phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -121,7 +211,12 @@ static int tpm_tcg_read32(struct tpm_tis_data *data, u32 addr, u32 *result)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	tpm_platform_begin_xfer();
+
 	*result = ioread32(phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -129,7 +224,12 @@ static int tpm_tcg_write32(struct tpm_tis_data *data, u32 addr, u32 value)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	tpm_platform_begin_xfer();
+
 	iowrite32(value, phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -191,6 +291,12 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
 		acpi_dev_handle = ACPI_HANDLE(&pnp_dev->dev);
 	}
 
+#ifdef CONFIG_X86
+	if (is_bsw())
+		ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
+					ILB_REMAP_SIZE);
+#endif
+
 	return tpm_tis_init(&pnp_dev->dev, &tpm_info, acpi_dev_handle);
 }
 
@@ -214,6 +320,12 @@ static void tpm_tis_pnp_remove(struct pnp_dev *dev)
 
 	tpm_chip_unregister(chip);
 	tpm_tis_remove(chip);
+
+#ifdef CONFIG_X86
+	if (is_bsw())
+		iounmap(ilb_base_addr);
+#endif
+
 }
 
 static struct pnp_driver tis_pnp_driver = {
-- 
1.9.1

Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems

[Jarkko Sakkinen]

On Wed, 2017-06-14 at 12:39 -0700, Azhar Shaikh wrote:
> To overcome a hardware limitation on Intel Braswell systems,
> disable CLKRUN protocol during TPM transactions and re-enable
> once the transaction is completed.
> 
> Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>

Does not merge to my tree:

git://git.infradead.org/users/jjs/linux-tpmdd.git

Please fix.

/Jarkko

> ---
> Changes from v1:
> - Add CONFIG_X86 around disable_lpc_clk_run() and enable_lpc_clk_run() to avoid
> - build breakage on architectures which do not implement kmap_atomic_pfn()
> 
> Changes from v2:
> - Use ioremap()/iounmap() instead of kmap_atomic_pfn()/kunmap_atomic()
> - Move is_bsw() and all macros from tpm.h to tpm_tis.c file.
> - Add the is_bsw() check in disable_lpc_clk_run() and enable_lpc_clk_run()
> - instead of adding it in each read/write API.
> 
> Changes from v3:
> - Move the code under #ifdef CONFIG_X86 and create stub functions for everything else
> - Rename the functions disable_lpc_clk_run() -> tpm_platform_begin_xfer() and
> - enable_lpc_clk_run() -> tpm_platform_end_xfer()
> - Remove wmb()
> - Correct the parameters for outb()
> 
>  drivers/char/tpm/tpm.h     |   4 ++
>  drivers/char/tpm/tpm_tis.c | 112 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 116 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 4b4c8dee3096..6b769fbc4407 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -36,6 +36,10 @@
>  #include <linux/highmem.h>
>  #include <crypto/hash_info.h>
>  
> +#ifdef CONFIG_X86
> +#include <asm/intel-family.h>
> +#endif
> +
>  enum tpm_const {
>  	TPM_MINOR = 224,	/* officially assigned */
>  	TPM_BUFSIZE = 4096,
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index c7e1384f1b08..ce6f1a8b0510 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -89,13 +89,93 @@ static inline int is_itpm(struct acpi_device *dev)
>  }
>  #endif
>  
> +#ifdef CONFIG_X86
> +#define INTEL_LEGACY_BLK_BASE_ADDR      0xFED08000
> +#define ILB_REMAP_SIZE			0x100
> +#define LPC_CNTRL_REG_OFFSET            0x84
> +#define LPC_CLKRUN_EN                   (1 << 2)
> +
> +void __iomem *ilb_base_addr;
> +
> +static inline bool is_bsw(void)
> +{
> +	return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0);
> +}
> +
> +/**
> + * tpm_platform_begin_xfer() - clear LPC CLKRUN_EN i.e. clocks will be running
> + */
> +static void tpm_platform_begin_xfer(void)
> +{
> +	u32 clkrun_val;
> +
> +	if (!is_bsw())
> +		return;
> +
> +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> +
> +	/* Disable LPC CLKRUN# */
> +	clkrun_val &= ~LPC_CLKRUN_EN;
> +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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(void)
> +{
> +	u32 clkrun_val;
> +
> +	if (!is_bsw())
> +		return;
> +
> +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> +
> +	/* Enable LPC CLKRUN# */
> +	clkrun_val |= LPC_CLKRUN_EN;
> +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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 inline bool is_bsw(void)
> +{
> +	return false;
> +}
> +
> +static void tpm_platform_begin_xfer(void)
> +{
> +}
> +
> +static void tpm_platform_end_xfer(void)
> +{
> +}
> +#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();
> +
>  	while (len--)
>  		*result++ = ioread8(phy->iobase + addr);
> +
> +	tpm_platform_end_xfer();
> +
>  	return 0;
>  }
>  
> @@ -104,8 +184,13 @@ static int tpm_tcg_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	tpm_platform_begin_xfer();
> +
>  	while (len--)
>  		iowrite8(*value++, phy->iobase + addr);
> +
> +	tpm_platform_end_xfer();
> +
>  	return 0;
>  }
>  
> @@ -113,7 +198,12 @@ static int tpm_tcg_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	tpm_platform_begin_xfer();
> +
>  	*result = ioread16(phy->iobase + addr);
> +
> +	tpm_platform_end_xfer();
> +
>  	return 0;
>  }
>  
> @@ -121,7 +211,12 @@ static int tpm_tcg_read32(struct tpm_tis_data *data, u32 addr, u32 *result)
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	tpm_platform_begin_xfer();
> +
>  	*result = ioread32(phy->iobase + addr);
> +
> +	tpm_platform_end_xfer();
> +
>  	return 0;
>  }
>  
> @@ -129,7 +224,12 @@ static int tpm_tcg_write32(struct tpm_tis_data *data, u32 addr, u32 value)
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	tpm_platform_begin_xfer();
> +
>  	iowrite32(value, phy->iobase + addr);
> +
> +	tpm_platform_end_xfer();
> +
>  	return 0;
>  }
>  
> @@ -191,6 +291,12 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
>  		acpi_dev_handle = ACPI_HANDLE(&pnp_dev->dev);
>  	}
>  
> +#ifdef CONFIG_X86
> +	if (is_bsw())
> +		ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
> +					ILB_REMAP_SIZE);
> +#endif
> +
>  	return tpm_tis_init(&pnp_dev->dev, &tpm_info, acpi_dev_handle);
>  }
>  
> @@ -214,6 +320,12 @@ static void tpm_tis_pnp_remove(struct pnp_dev *dev)
>  
>  	tpm_chip_unregister(chip);
>  	tpm_tis_remove(chip);
> +
> +#ifdef CONFIG_X86
> +	if (is_bsw())
> +		iounmap(ilb_base_addr);
> +#endif
> +
>  }
>  
>  static struct pnp_driver tis_pnp_driver = {

Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems

[Shaikh, Azhar]

> -----Original Message-----
> From: Jarkko Sakkinen [mailto:jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org]
> Sent: Sunday, June 18, 2017 4:53 PM
> To: Shaikh, Azhar <azhar.shaikh-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>;
> jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org; tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org;
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH v4] tpm: Enable CLKRUN protocol for Braswell systems
> 
> On Wed, 2017-06-14 at 12:39 -0700, Azhar Shaikh wrote:
> > To overcome a hardware limitation on Intel Braswell systems, disable
> > CLKRUN protocol during TPM transactions and re-enable once the
> > transaction is completed.
> >
> > Signed-off-by: Azhar Shaikh <azhar.shaikh-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> Does not merge to my tree:
> 
> git://git.infradead.org/users/jjs/linux-tpmdd.git
> 
> Please fix.
> 

Sure will rebase and send again.

> /Jarkko
> 
> > ---
> > Changes from v1:
> > - Add CONFIG_X86 around disable_lpc_clk_run() and enable_lpc_clk_run()
> > to avoid
> > - build breakage on architectures which do not implement
> > kmap_atomic_pfn()
> >
> > Changes from v2:
> > - Use ioremap()/iounmap() instead of
> kmap_atomic_pfn()/kunmap_atomic()
> > - Move is_bsw() and all macros from tpm.h to tpm_tis.c file.
> > - Add the is_bsw() check in disable_lpc_clk_run() and
> > enable_lpc_clk_run()
> > - instead of adding it in each read/write API.
> >
> > Changes from v3:
> > - Move the code under #ifdef CONFIG_X86 and create stub functions for
> > everything else
> > - Rename the functions disable_lpc_clk_run() ->
> > tpm_platform_begin_xfer() and
> > - enable_lpc_clk_run() -> tpm_platform_end_xfer()
> > - Remove wmb()
> > - Correct the parameters for outb()
> >
> >  drivers/char/tpm/tpm.h     |   4 ++
> >  drivers/char/tpm/tpm_tis.c | 112
> > +++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 116 insertions(+)
> >
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index
> > 4b4c8dee3096..6b769fbc4407 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -36,6 +36,10 @@
> >  #include <linux/highmem.h>
> >  #include <crypto/hash_info.h>
> >
> > +#ifdef CONFIG_X86
> > +#include <asm/intel-family.h>
> > +#endif
> > +
> >  enum tpm_const {
> >  	TPM_MINOR = 224,	/* officially assigned */
> >  	TPM_BUFSIZE = 4096,
> > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> > index c7e1384f1b08..ce6f1a8b0510 100644
> > --- a/drivers/char/tpm/tpm_tis.c
> > +++ b/drivers/char/tpm/tpm_tis.c
> > @@ -89,13 +89,93 @@ static inline int is_itpm(struct acpi_device *dev)
> > }  #endif
> >
> > +#ifdef CONFIG_X86
> > +#define INTEL_LEGACY_BLK_BASE_ADDR      0xFED08000
> > +#define ILB_REMAP_SIZE			0x100
> > +#define LPC_CNTRL_REG_OFFSET            0x84
> > +#define LPC_CLKRUN_EN                   (1 << 2)
> > +
> > +void __iomem *ilb_base_addr;
> > +
> > +static inline bool is_bsw(void)
> > +{
> > +	return ((boot_cpu_data.x86_model ==
> INTEL_FAM6_ATOM_AIRMONT) ? 1 :
> > +0); }
> > +
> > +/**
> > + * tpm_platform_begin_xfer() - clear LPC CLKRUN_EN i.e. clocks will
> > +be running  */ static void tpm_platform_begin_xfer(void) {
> > +	u32 clkrun_val;
> > +
> > +	if (!is_bsw())
> > +		return;
> > +
> > +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> > +
> > +	/* Disable LPC CLKRUN# */
> > +	clkrun_val &= ~LPC_CLKRUN_EN;
> > +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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(void) {
> > +	u32 clkrun_val;
> > +
> > +	if (!is_bsw())
> > +		return;
> > +
> > +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> > +
> > +	/* Enable LPC CLKRUN# */
> > +	clkrun_val |= LPC_CLKRUN_EN;
> > +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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 inline bool is_bsw(void)
> > +{
> > +	return false;
> > +}
> > +
> > +static void tpm_platform_begin_xfer(void) { }
> > +
> > +static void tpm_platform_end_xfer(void) { } #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();
> > +
> >  	while (len--)
> >  		*result++ = ioread8(phy->iobase + addr);
> > +
> > +	tpm_platform_end_xfer();
> > +
> >  	return 0;
> >  }
> >
> > @@ -104,8 +184,13 @@ static int tpm_tcg_write_bytes(struct
> > tpm_tis_data *data, u32 addr, u16 len,  {
> >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> >
> > +	tpm_platform_begin_xfer();
> > +
> >  	while (len--)
> >  		iowrite8(*value++, phy->iobase + addr);
> > +
> > +	tpm_platform_end_xfer();
> > +
> >  	return 0;
> >  }
> >
> > @@ -113,7 +198,12 @@ static int tpm_tcg_read16(struct tpm_tis_data
> > *data, u32 addr, u16 *result)  {
> >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> >
> > +	tpm_platform_begin_xfer();
> > +
> >  	*result = ioread16(phy->iobase + addr);
> > +
> > +	tpm_platform_end_xfer();
> > +
> >  	return 0;
> >  }
> >
> > @@ -121,7 +211,12 @@ static int tpm_tcg_read32(struct tpm_tis_data
> > *data, u32 addr, u32 *result)  {
> >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> >
> > +	tpm_platform_begin_xfer();
> > +
> >  	*result = ioread32(phy->iobase + addr);
> > +
> > +	tpm_platform_end_xfer();
> > +
> >  	return 0;
> >  }
> >
> > @@ -129,7 +224,12 @@ static int tpm_tcg_write32(struct tpm_tis_data
> > *data, u32 addr, u32 value)  {
> >  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
> >
> > +	tpm_platform_begin_xfer();
> > +
> >  	iowrite32(value, phy->iobase + addr);
> > +
> > +	tpm_platform_end_xfer();
> > +
> >  	return 0;
> >  }
> >
> > @@ -191,6 +291,12 @@ static int tpm_tis_pnp_init(struct pnp_dev
> *pnp_dev,
> >  		acpi_dev_handle = ACPI_HANDLE(&pnp_dev->dev);
> >  	}
> >
> > +#ifdef CONFIG_X86
> > +	if (is_bsw())
> > +		ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
> > +					ILB_REMAP_SIZE);
> > +#endif
> > +
> >  	return tpm_tis_init(&pnp_dev->dev, &tpm_info, acpi_dev_handle);
> }
> >
> > @@ -214,6 +320,12 @@ static void tpm_tis_pnp_remove(struct pnp_dev
> > *dev)
> >
> >  	tpm_chip_unregister(chip);
> >  	tpm_tis_remove(chip);
> > +
> > +#ifdef CONFIG_X86
> > +	if (is_bsw())
> > +		iounmap(ilb_base_addr);
> > +#endif
> > +
> >  }
> >
> >  static struct pnp_driver tis_pnp_driver = {
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[PATCH v5] tpm: Enable CLKRUN protocol for Braswell systems

[Azhar Shaikh]

To overcome a hardware limitation on Intel Braswell systems,
disable CLKRUN protocol during TPM transactions and re-enable
once the transaction is completed.

Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
---
Changes from v1:
- Add CONFIG_X86 around disable_lpc_clk_run() and enable_lpc_clk_run() to avoid
- build breakage on architectures which do not implement kmap_atomic_pfn()

Changes from v2:
- Use ioremap()/iounmap() instead of kmap_atomic_pfn()/kunmap_atomic()
- Move is_bsw() and all macros from tpm.h to tpm_tis.c file.
- Add the is_bsw() check in disable_lpc_clk_run() and enable_lpc_clk_run()
- instead of adding it in each read/write API.

Changes from v3:
- Move the code under #ifdef CONFIG_X86 and create stub functions for everything else
- Rename the functions disable_lpc_clk_run() -> tpm_platform_begin_xfer() and
- enable_lpc_clk_run() -> tpm_platform_end_xfer()
- Remove wmb()
- Correct the parameters for outb()

Changes from v4:
- Rebased to apply on git://git.infradead.org/users/jjs/linux-tpmdd.git

 drivers/char/tpm/tpm.h     |   4 ++
 drivers/char/tpm/tpm_tis.c | 112 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 116 insertions(+)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 1df0521138d3..cdd261383dea 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -36,6 +36,10 @@
 #include <linux/highmem.h>
 #include <crypto/hash_info.h>
 
+#ifdef CONFIG_X86
+#include <asm/intel-family.h>
+#endif
+
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */
 	TPM_BUFSIZE = 4096,
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index b14d4aa97af8..506e62ca3576 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -132,13 +132,93 @@ static int check_acpi_tpm2(struct device *dev)
 }
 #endif
 
+#ifdef CONFIG_X86
+#define INTEL_LEGACY_BLK_BASE_ADDR      0xFED08000
+#define ILB_REMAP_SIZE			0x100
+#define LPC_CNTRL_REG_OFFSET            0x84
+#define LPC_CLKRUN_EN                   (1 << 2)
+
+void __iomem *ilb_base_addr;
+
+static inline bool is_bsw(void)
+{
+	return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0);
+}
+
+/**
+ * tpm_platform_begin_xfer() - clear LPC CLKRUN_EN i.e. clocks will be running
+ */
+static void tpm_platform_begin_xfer(void)
+{
+	u32 clkrun_val;
+
+	if (!is_bsw())
+		return;
+
+	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	/* Disable LPC CLKRUN# */
+	clkrun_val &= ~LPC_CLKRUN_EN;
+	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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(void)
+{
+	u32 clkrun_val;
+
+	if (!is_bsw())
+		return;
+
+	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
+
+	/* Enable LPC CLKRUN# */
+	clkrun_val |= LPC_CLKRUN_EN;
+	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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 inline bool is_bsw(void)
+{
+	return false;
+}
+
+static void tpm_platform_begin_xfer(void)
+{
+}
+
+static void tpm_platform_end_xfer(void)
+{
+}
+#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();
+
 	while (len--)
 		*result++ = ioread8(phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -147,8 +227,13 @@ static int tpm_tcg_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	tpm_platform_begin_xfer();
+
 	while (len--)
 		iowrite8(*value++, phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -156,7 +241,12 @@ static int tpm_tcg_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	tpm_platform_begin_xfer();
+
 	*result = ioread16(phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -164,7 +254,12 @@ static int tpm_tcg_read32(struct tpm_tis_data *data, u32 addr, u32 *result)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	tpm_platform_begin_xfer();
+
 	*result = ioread32(phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -172,7 +267,12 @@ static int tpm_tcg_write32(struct tpm_tis_data *data, u32 addr, u32 value)
 {
 	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
 
+	tpm_platform_begin_xfer();
+
 	iowrite32(value, phy->iobase + addr);
+
+	tpm_platform_end_xfer();
+
 	return 0;
 }
 
@@ -230,6 +330,12 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
 	else
 		tpm_info.irq = -1;
 
+#ifdef CONFIG_X86
+	if (is_bsw())
+		ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
+					ILB_REMAP_SIZE);
+#endif
+
 	return tpm_tis_init(&pnp_dev->dev, &tpm_info);
 }
 
@@ -253,6 +359,12 @@ static void tpm_tis_pnp_remove(struct pnp_dev *dev)
 
 	tpm_chip_unregister(chip);
 	tpm_tis_remove(chip);
+
+#ifdef CONFIG_X86
+	if (is_bsw())
+		iounmap(ilb_base_addr);
+#endif
+
 }
 
 static struct pnp_driver tis_pnp_driver = {
-- 
1.9.1

Re: [PATCH v5] tpm: Enable CLKRUN protocol for Braswell systems

[Jarkko Sakkinen]

On Sun, Jun 18, 2017 at 07:17:59PM -0700, Azhar Shaikh wrote:
> To overcome a hardware limitation on Intel Braswell systems,
> disable CLKRUN protocol during TPM transactions and re-enable
> once the transaction is completed.
> 
> Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> (compilation)

/Jarkko

> ---
> Changes from v1:
> - Add CONFIG_X86 around disable_lpc_clk_run() and enable_lpc_clk_run() to avoid
> - build breakage on architectures which do not implement kmap_atomic_pfn()
> 
> Changes from v2:
> - Use ioremap()/iounmap() instead of kmap_atomic_pfn()/kunmap_atomic()
> - Move is_bsw() and all macros from tpm.h to tpm_tis.c file.
> - Add the is_bsw() check in disable_lpc_clk_run() and enable_lpc_clk_run()
> - instead of adding it in each read/write API.
> 
> Changes from v3:
> - Move the code under #ifdef CONFIG_X86 and create stub functions for everything else
> - Rename the functions disable_lpc_clk_run() -> tpm_platform_begin_xfer() and
> - enable_lpc_clk_run() -> tpm_platform_end_xfer()
> - Remove wmb()
> - Correct the parameters for outb()
> 
> Changes from v4:
> - Rebased to apply on git://git.infradead.org/users/jjs/linux-tpmdd.git
> 
>  drivers/char/tpm/tpm.h     |   4 ++
>  drivers/char/tpm/tpm_tis.c | 112 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 116 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 1df0521138d3..cdd261383dea 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -36,6 +36,10 @@
>  #include <linux/highmem.h>
>  #include <crypto/hash_info.h>
>  
> +#ifdef CONFIG_X86
> +#include <asm/intel-family.h>
> +#endif
> +
>  enum tpm_const {
>  	TPM_MINOR = 224,	/* officially assigned */
>  	TPM_BUFSIZE = 4096,
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index b14d4aa97af8..506e62ca3576 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -132,13 +132,93 @@ static int check_acpi_tpm2(struct device *dev)
>  }
>  #endif
>  
> +#ifdef CONFIG_X86
> +#define INTEL_LEGACY_BLK_BASE_ADDR      0xFED08000
> +#define ILB_REMAP_SIZE			0x100
> +#define LPC_CNTRL_REG_OFFSET            0x84
> +#define LPC_CLKRUN_EN                   (1 << 2)
> +
> +void __iomem *ilb_base_addr;
> +
> +static inline bool is_bsw(void)
> +{
> +	return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0);
> +}
> +
> +/**
> + * tpm_platform_begin_xfer() - clear LPC CLKRUN_EN i.e. clocks will be running
> + */
> +static void tpm_platform_begin_xfer(void)
> +{
> +	u32 clkrun_val;
> +
> +	if (!is_bsw())
> +		return;
> +
> +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> +
> +	/* Disable LPC CLKRUN# */
> +	clkrun_val &= ~LPC_CLKRUN_EN;
> +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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(void)
> +{
> +	u32 clkrun_val;
> +
> +	if (!is_bsw())
> +		return;
> +
> +	clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET);
> +
> +	/* Enable LPC CLKRUN# */
> +	clkrun_val |= LPC_CLKRUN_EN;
> +	iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_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 inline bool is_bsw(void)
> +{
> +	return false;
> +}
> +
> +static void tpm_platform_begin_xfer(void)
> +{
> +}
> +
> +static void tpm_platform_end_xfer(void)
> +{
> +}
> +#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();
> +
>  	while (len--)
>  		*result++ = ioread8(phy->iobase + addr);
> +
> +	tpm_platform_end_xfer();
> +
>  	return 0;
>  }
>  
> @@ -147,8 +227,13 @@ static int tpm_tcg_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	tpm_platform_begin_xfer();
> +
>  	while (len--)
>  		iowrite8(*value++, phy->iobase + addr);
> +
> +	tpm_platform_end_xfer();
> +
>  	return 0;
>  }
>  
> @@ -156,7 +241,12 @@ static int tpm_tcg_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	tpm_platform_begin_xfer();
> +
>  	*result = ioread16(phy->iobase + addr);
> +
> +	tpm_platform_end_xfer();
> +
>  	return 0;
>  }
>  
> @@ -164,7 +254,12 @@ static int tpm_tcg_read32(struct tpm_tis_data *data, u32 addr, u32 *result)
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	tpm_platform_begin_xfer();
> +
>  	*result = ioread32(phy->iobase + addr);
> +
> +	tpm_platform_end_xfer();
> +
>  	return 0;
>  }
>  
> @@ -172,7 +267,12 @@ static int tpm_tcg_write32(struct tpm_tis_data *data, u32 addr, u32 value)
>  {
>  	struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);
>  
> +	tpm_platform_begin_xfer();
> +
>  	iowrite32(value, phy->iobase + addr);
> +
> +	tpm_platform_end_xfer();
> +
>  	return 0;
>  }
>  
> @@ -230,6 +330,12 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
>  	else
>  		tpm_info.irq = -1;
>  
> +#ifdef CONFIG_X86
> +	if (is_bsw())
> +		ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
> +					ILB_REMAP_SIZE);
> +#endif
> +
>  	return tpm_tis_init(&pnp_dev->dev, &tpm_info);
>  }
>  
> @@ -253,6 +359,12 @@ static void tpm_tis_pnp_remove(struct pnp_dev *dev)
>  
>  	tpm_chip_unregister(chip);
>  	tpm_tis_remove(chip);
> +
> +#ifdef CONFIG_X86
> +	if (is_bsw())
> +		iounmap(ilb_base_addr);
> +#endif
> +
>  }
>  
>  static struct pnp_driver tis_pnp_driver = {
> -- 
> 1.9.1
>