[PATCH v4 0/5] Use tpm_transmit_cmd() consistently across kernel call sites

[PATCH v4 0/5] Use tpm_transmit_cmd() consistently across kernel call sites

[Jarkko Sakkinen]

These commits update the subsystem consistently to use
tpm_transmit_cmd() throughout the subsystem the exception being
tpm_write() where it makes sense to use the raw interface because higher
level command handling is delegated to user space. The point is to make
semantics and error handling across the kernel call sites.

v4: Make interrupt generation internal to tpm_tis as it is not really
used for anything useful anywhere else and it does not really belong to
the upper TPM driver.

v3: Removed patch that renames tpm_pcr_read_dev() to tpm1_pcr_read()
because it is not mandatory for this patch set.

v2: Added commit that drops externs from all function declarations in
tpm.h as suggested by Jason Gunthorpe.

Jarkko Sakkinen (4):
  tpm: remove unnecessary externs from tpm.h
  tpm: replace tpm_gen_interrupt() with tpm_tis_gen_interrupt()
  tpm: use tpm_transmit_cmd() in tpm2_probe()
  tpm: use tpm_pcr_read_dev() in tpm_do_selftest()

Jason Gunthorpe (1):
  tpm/st33zp24: Remove useless tpm_gen_interrupt

 drivers/char/tpm/st33zp24/st33zp24.c |  2 --
 drivers/char/tpm/tpm-interface.c     | 22 ++--------------------
 drivers/char/tpm/tpm.h               | 34 ++++++++++++++++------------------
 drivers/char/tpm/tpm2-cmd.c          | 21 +--------------------
 drivers/char/tpm/tpm_tis_core.c      | 19 +++++++++++++++----
 5 files changed, 34 insertions(+), 64 deletions(-)

-- 
2.7.4

[PATCH v4 1/5] tpm/st33zp24: Remove useless tpm_gen_interrupt

[Jarkko Sakkinen]

From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

This function should only be called as part of an IRQ probing protocol
and st33 does not have any code to detect that the IRQ it tries to
generate was not generated and disable the IRQ.

Since st33 is primarily a DT binding driver it should not be doing
IRQ probing anyhow, so let us just delete this useless call.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/st33zp24/st33zp24.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
index c2ee304..6f060c7 100644
--- a/drivers/char/tpm/st33zp24/st33zp24.c
+++ b/drivers/char/tpm/st33zp24/st33zp24.c
@@ -589,8 +589,6 @@ int st33zp24_probe(void *phy_id, const struct st33zp24_phy_ops *ops,
 		chip->flags |= TPM_CHIP_FLAG_IRQ;
 
 		disable_irq_nosync(tpm_dev->irq);
-
-		tpm_gen_interrupt(chip);
 	}
 
 	return tpm_chip_register(chip);
-- 
2.7.4

[PATCH v4 2/5] tpm: remove unnecessary externs from tpm.h

[Jarkko Sakkinen]

Removed unnecessary externs from tpm.h.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm.h | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 3e32d5b..8b864dd 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -482,26 +482,26 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
 		     size_t bufsiz);
 ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd, int len,
 			 const char *desc);
-extern int tpm_get_timeouts(struct tpm_chip *);
-extern void tpm_gen_interrupt(struct tpm_chip *);
+int tpm_get_timeouts(struct tpm_chip *chip);
+void tpm_gen_interrupt(struct tpm_chip *chip);
 int tpm1_auto_startup(struct tpm_chip *chip);
-extern int tpm_do_selftest(struct tpm_chip *);
-extern unsigned long tpm_calc_ordinal_duration(struct tpm_chip *, u32);
-extern int tpm_pm_suspend(struct device *);
-extern int tpm_pm_resume(struct device *);
-extern int wait_for_tpm_stat(struct tpm_chip *, u8, unsigned long,
-			     wait_queue_head_t *, bool);
+int tpm_do_selftest(struct tpm_chip *chip);
+unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
+int tpm_pm_suspend(struct device *dev);
+int tpm_pm_resume(struct device *dev);
+int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
+		      wait_queue_head_t *queue, bool check_cancel);
 
 struct tpm_chip *tpm_chip_find_get(int chip_num);
 __must_check int tpm_try_get_ops(struct tpm_chip *chip);
 void tpm_put_ops(struct tpm_chip *chip);
 
-extern struct tpm_chip *tpm_chip_alloc(struct device *dev,
-				       const struct tpm_class_ops *ops);
-extern struct tpm_chip *tpmm_chip_alloc(struct device *pdev,
-				       const struct tpm_class_ops *ops);
-extern int tpm_chip_register(struct tpm_chip *chip);
-extern void tpm_chip_unregister(struct tpm_chip *chip);
+struct tpm_chip *tpm_chip_alloc(struct device *dev,
+				const struct tpm_class_ops *ops);
+struct tpm_chip *tpmm_chip_alloc(struct device *pdev,
+				 const struct tpm_class_ops *ops);
+int tpm_chip_register(struct tpm_chip *chip);
+void tpm_chip_unregister(struct tpm_chip *chip);
 
 void tpm_sysfs_add_device(struct tpm_chip *chip);
 
@@ -528,8 +528,8 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
 			u32 *value, const char *desc);
 
 int tpm2_auto_startup(struct tpm_chip *chip);
-extern void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
-extern unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *, u32);
-extern int tpm2_gen_interrupt(struct tpm_chip *chip);
-extern int tpm2_probe(struct tpm_chip *chip);
+void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
+unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
+int tpm2_gen_interrupt(struct tpm_chip *chip);
+int tpm2_probe(struct tpm_chip *chip);
 #endif
-- 
2.7.4

[PATCH v4 3/5] tpm: replace tpm_gen_interrupt() with tpm_tis_gen_interrupt()

[Jarkko Sakkinen]

Since tpm_gen_interrupt() is only used in tpm_tis_core.c this commit
replaces it with an internal tpm_tis_gen_interrupt(). The semantics
also changed in a way that on a system error the driver initialization
is failed.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 15 ---------------
 drivers/char/tpm/tpm.h           |  2 --
 drivers/char/tpm/tpm2-cmd.c      | 17 -----------------
 drivers/char/tpm/tpm_tis_core.c  | 19 +++++++++++++++----
 4 files changed, 15 insertions(+), 38 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1abe2d7..f43cc3f 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -459,21 +459,6 @@ ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
 	return rc;
 }
 
-void tpm_gen_interrupt(struct tpm_chip *chip)
-{
-	struct	tpm_cmd_t tpm_cmd;
-	ssize_t rc;
-
-	tpm_cmd.header.in = tpm_getcap_header;
-	tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
-	tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
-	tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
-
-	rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
-			      "attempting to determine the timeouts");
-}
-EXPORT_SYMBOL_GPL(tpm_gen_interrupt);
-
 #define TPM_ORD_STARTUP cpu_to_be32(153)
 #define TPM_ST_CLEAR cpu_to_be16(1)
 #define TPM_ST_STATE cpu_to_be16(2)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 8b864dd..6e002c4 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -483,7 +483,6 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
 ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd, int len,
 			 const char *desc);
 int tpm_get_timeouts(struct tpm_chip *chip);
-void tpm_gen_interrupt(struct tpm_chip *chip);
 int tpm1_auto_startup(struct tpm_chip *chip);
 int tpm_do_selftest(struct tpm_chip *chip);
 unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
@@ -530,6 +529,5 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
 int tpm2_auto_startup(struct tpm_chip *chip);
 void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
 unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
-int tpm2_gen_interrupt(struct tpm_chip *chip);
 int tpm2_probe(struct tpm_chip *chip);
 #endif
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 08c7e23..1c393ba 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -895,23 +895,6 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
 }
 
 /**
- * tpm2_gen_interrupt() - generate an interrupt
- * @chip: TPM chip to use
- *
- * 0 is returned when the operation is successful. If a negative number is
- * returned it remarks a POSIX error code. If a positive number is returned
- * it remarks a TPM error.
- */
-int tpm2_gen_interrupt(struct tpm_chip *chip)
-{
-	u32 dummy;
-
-	return tpm2_get_tpm_pt(chip, 0x100, &dummy,
-			       "attempting to generate an interrupt");
-}
-EXPORT_SYMBOL_GPL(tpm2_gen_interrupt);
-
-/**
  * tpm2_probe() - probe TPM 2.0
  * @chip: TPM chip to use
  *
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index d66f51b..f22caf8 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -526,6 +526,18 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static int tpm_tis_gen_interrupt(struct tpm_chip *chip)
+{
+	const char *desc = "attempting to generate an interrupt";
+	u32 cap2;
+	cap_t cap;
+
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		return tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
+	else
+		return tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc);
+}
+
 /* Register the IRQ and issue a command that will cause an interrupt. If an
  * irq is seen then leave the chip setup for IRQ operation, otherwise reverse
  * everything and leave in polling mode. Returns 0 on success.
@@ -575,10 +587,9 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
 	/* Generate an interrupt by having the core call through to
 	 * tpm_tis_send
 	 */
-	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		tpm2_gen_interrupt(chip);
-	else
-		tpm_gen_interrupt(chip);
+	rc = tpm_tis_gen_interrupt(chip);
+	if (rc < 0)
+		return rc;
 
 	/* tpm_tis_send will either confirm the interrupt is working or it
 	 * will call disable_irq which undoes all of the above.
-- 
2.7.4

[PATCH v4 4/5] tpm: use tpm_transmit_cmd() in tpm2_probe()

[Jarkko Sakkinen]

It is better to tpm_transmit_cmd() in tpm2_probe() in order to get
consistent command handling throughout the subsystem.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm2-cmd.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 1c393ba..e1db404 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -911,11 +911,9 @@ int tpm2_probe(struct tpm_chip *chip)
 	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(0x100);
 	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
 
-	rc = tpm_transmit(chip, (const char *) &cmd, sizeof(cmd));
+	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), NULL);
 	if (rc <  0)
 		return rc;
-	else if (rc < TPM_HEADER_SIZE)
-		return -EFAULT;
 
 	if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
 		chip->flags |= TPM_CHIP_FLAG_TPM2;
-- 
2.7.4

[PATCH v4 5/5] tpm: use tpm_pcr_read_dev() in tpm_do_selftest()

[Jarkko Sakkinen]

Instead of a  ad-hoc protocol message construction it is better to
call tpm_pcr_read_dev().

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index f43cc3f..ef5ad80 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -777,7 +777,7 @@ int tpm_do_selftest(struct tpm_chip *chip)
 	unsigned int loops;
 	unsigned int delay_msec = 100;
 	unsigned long duration;
-	struct tpm_cmd_t cmd;
+	u8 dummy[TPM_DIGEST_SIZE];
 
 	duration = tpm_calc_ordinal_duration(chip, TPM_ORD_CONTINUE_SELFTEST);
 
@@ -792,9 +792,7 @@ int tpm_do_selftest(struct tpm_chip *chip)
 
 	do {
 		/* Attempt to read a PCR value */
-		cmd.header.in = pcrread_header;
-		cmd.params.pcrread_in.pcr_idx = cpu_to_be32(0);
-		rc = tpm_transmit(chip, (u8 *) &cmd, READ_PCR_RESULT_SIZE);
+		rc = tpm_pcr_read_dev(chip, 0, dummy);
 		/* Some buggy TPMs will not respond to tpm_tis_ready() for
 		 * around 300ms while the self test is ongoing, keep trying
 		 * until the self test duration expires. */
@@ -809,7 +807,6 @@ int tpm_do_selftest(struct tpm_chip *chip)
 		if (rc < TPM_HEADER_SIZE)
 			return -EFAULT;
 
-		rc = be32_to_cpu(cmd.header.out.return_code);
 		if (rc == TPM_ERR_DISABLED || rc == TPM_ERR_DEACTIVATED) {
 			dev_info(&chip->dev,
 				 "TPM is disabled/deactivated (0x%X)\n", rc);
-- 
2.7.4

Re: [PATCH v4 3/5] tpm: replace tpm_gen_interrupt() with tpm_tis_gen_interrupt()

[Jason Gunthorpe]

On Wed, Jul 20, 2016 at 02:44:58AM +0300, Jarkko Sakkinen wrote:
> Since tpm_gen_interrupt() is only used in tpm_tis_core.c this commit
> replaces it with an internal tpm_tis_gen_interrupt(). The semantics
> also changed in a way that on a system error the driver initialization
> is failed.

All the patches look good to me now..

Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

Jason

Re: [PATCH v4 3/5] tpm: replace tpm_gen_interrupt() with tpm_tis_gen_interrupt()

[Jarkko Sakkinen]

On Wed, Jul 20, 2016 at 11:06:24AM -0600, Jason Gunthorpe wrote:
> On Wed, Jul 20, 2016 at 02:44:58AM +0300, Jarkko Sakkinen wrote:
> > Since tpm_gen_interrupt() is only used in tpm_tis_core.c this commit
> > replaces it with an internal tpm_tis_gen_interrupt(). The semantics
> > also changed in a way that on a system error the driver initialization
> > is failed.
> 
> All the patches look good to me now..
> 
> Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

Thanks.

/Jarkko

Re: [PATCH v4 3/5] tpm: replace tpm_gen_interrupt() with tpm_tis_gen_interrupt()

[Jarkko Sakkinen]

On Wed, Jul 20, 2016 at 11:54:10PM +0300, Jarkko Sakkinen wrote:
> On Wed, Jul 20, 2016 at 11:06:24AM -0600, Jason Gunthorpe wrote:
> > On Wed, Jul 20, 2016 at 02:44:58AM +0300, Jarkko Sakkinen wrote:
> > > Since tpm_gen_interrupt() is only used in tpm_tis_core.c this commit
> > > replaces it with an internal tpm_tis_gen_interrupt(). The semantics
> > > also changed in a way that on a system error the driver initialization
> > > is failed.
> > 
> > All the patches look good to me now..
> > 
> > Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> 
> Thanks.

The patches are applied to the master branch now.

/Jarkko

Re: [PATCH v4 3/5] tpm: replace tpm_gen_interrupt() with tpm_tis_gen_interrupt()

[kbuild test robot]

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

Hi,

[auto build test ERROR on next-20160722]
[cannot apply to char-misc/char-misc-testing v4.7-rc7 v4.7-rc6 v4.7-rc5 v4.7-rc7]
[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/Jarkko-Sakkinen/Use-tpm_transmit_cmd-consistently-across-kernel-call-sites/20160724-131407
config: i386-randconfig-i1-201630 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> ERROR: "tpm_getcap" [drivers/char/tpm/tpm_tis_core.ko] undefined!
>> ERROR: "tpm2_get_tpm_pt" [drivers/char/tpm/tpm_tis_core.ko] undefined!

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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 22345 bytes --]