[PATCH v1 00/12] tpm: TPM2 support

[PATCH v1 00/12] tpm: TPM2 support

[Jarkko Sakkinen]

This patch set enables TPM2 protocol and provides drivers for FIFO and
CRB interfaces.

Jarkko Sakkinen (8):
  tpm: prepare TPM driver for adding TPM2 support
  tpm: TPM2 support for tpm_pcr_read()
  tpm: TPM2 support for tpm_do_selftest()
  tpm: added tpm2_get_tpm_pt()
  tpm: TPM2 support for tpm_pcr_extend()
  tpm: TPM2 support for tpm_get_random().
  tpm: Driver for TPM 2.0 CRB Interface
  tpm: TPM2 sysfs attributes

Will Arthur (4):
  tpm: TPM2 support for tpm_calc_ordinal_durations()
  tpm: TPM2 support for tpm_startup()
  tpm: TPM2 support for tpm_gen_interrupt().
  tpm: TPM 2.0 FIFO Interface

 drivers/char/tpm/Kconfig         |   9 +
 drivers/char/tpm/Makefile        |   3 +-
 drivers/char/tpm/tpm-chip.c      | 175 ++++++++++++++++
 drivers/char/tpm/tpm-interface.c | 160 +++++----------
 drivers/char/tpm/tpm.h           |  84 ++++++++
 drivers/char/tpm/tpm2-commands.c | 422 +++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm2-sysfs.c    | 242 ++++++++++++++++++++++
 drivers/char/tpm/tpm2.h          | 108 ++++++++++
 drivers/char/tpm/tpm_crb.c       | 332 ++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm_tis.c       |  53 ++++-
 10 files changed, 1463 insertions(+), 125 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-chip.c
 create mode 100644 drivers/char/tpm/tpm2-commands.c
 create mode 100644 drivers/char/tpm/tpm2-sysfs.c
 create mode 100644 drivers/char/tpm/tpm2.h
 create mode 100644 drivers/char/tpm/tpm_crb.c

-- 
2.1.0

[PATCH v1 01/12] tpm: prepare TPM driver for adding TPM2 support

[Jarkko Sakkinen]

* Separated allocation and registeration into two functions:
  * tpm_chip_alloc()
  * tpm_chip_register()
* Modified tpm_register_hardware() to call tpm_chip_alloc/register().
* Added tpm2 boolean flag to struct tpm_chip.
* If a chip has tpm2 flag set TCPA log is not initialized in
  tpm_chip_register().

The rationale is to introduce TPM2 support without existing drivers
getting harmed.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/Makefile        |   2 +-
 drivers/char/tpm/tpm-chip.c      | 169 +++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm-interface.c | 120 ++-------------------------
 drivers/char/tpm/tpm.h           |   9 +++
 4 files changed, 187 insertions(+), 113 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-chip.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 4d85dd6..837da04 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the kernel tpm device drivers.
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
-tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o
+tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o
 
 ifdef CONFIG_ACPI
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
new file mode 100644
index 0000000..128942b
--- /dev/null
+++ b/drivers/char/tpm/tpm-chip.c
@@ -0,0 +1,169 @@
+/*
+ * Copyright (C) 2004 IBM Corporation
+ * Copyright (C) 2014 Intel Corporation
+ *
+ * Authors:
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ * Leendert van Doorn <leendert@watson.ibm.com>
+ * Dave Safford <safford@watson.ibm.com>
+ * Reiner Sailer <sailer@watson.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * TPM chip management routines.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ */
+
+#include <linux/poll.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+#include <linux/freezer.h>
+#include "tpm.h"
+#include "tpm_eventlog.h"
+
+static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
+static LIST_HEAD(tpm_chip_list);
+static DEFINE_SPINLOCK(driver_lock);
+
+/*
+ * tpm_chip_find_get - return tpm_chip for given chip number
+ */
+struct tpm_chip *tpm_chip_find_get(int chip_num)
+{
+	struct tpm_chip *pos, *chip = NULL;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(pos, &tpm_chip_list, list) {
+		if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num)
+			continue;
+
+		if (try_module_get(pos->dev->driver->owner)) {
+			chip = pos;
+			break;
+		}
+	}
+	rcu_read_unlock();
+	return chip;
+}
+
+
+/* In case vendor provided release function, call it too.*/
+
+void tpm_dev_vendor_release(struct tpm_chip *chip)
+{
+	if (!chip)
+		return;
+
+	clear_bit(chip->dev_num, dev_mask);
+}
+EXPORT_SYMBOL_GPL(tpm_dev_vendor_release);
+
+/*
+ * Once all references to platform device are down to 0,
+ * release all allocated structures.
+ */
+static void tpm_dev_release(struct device *dev)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+
+	if (!chip)
+		return;
+
+	tpm_dev_vendor_release(chip);
+
+	chip->release(dev);
+	kfree(chip);
+}
+
+struct tpm_chip *tpm_chip_alloc(struct device *dev,
+				const struct tpm_class_ops *ops)
+{
+	struct tpm_chip *chip;
+
+	/* Driver specific per-device data */
+	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+
+	if (chip == NULL)
+		return NULL;
+
+	mutex_init(&chip->tpm_mutex);
+	INIT_LIST_HEAD(&chip->list);
+
+	chip->ops = ops;
+	chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
+
+	if (chip->dev_num >= TPM_NUM_DEVICES) {
+		dev_err(dev, "No available tpm device numbers\n");
+		kfree(chip);
+		return NULL;
+	}
+
+	set_bit(chip->dev_num, dev_mask);
+
+	scnprintf(chip->devname, sizeof(chip->devname), "%s%d", "tpm",
+		  chip->dev_num);
+
+	chip->dev = get_device(dev);
+	chip->release = dev->release;
+	dev->release = tpm_dev_release;
+	dev_set_drvdata(dev, chip);
+
+	return chip;
+}
+EXPORT_SYMBOL_GPL(tpm_chip_alloc);
+
+int tpm_chip_register(struct tpm_chip *chip)
+{
+	int rc;
+
+	rc = tpm_dev_add_device(chip);
+	if (rc)
+		return rc;
+
+	rc = tpm_sysfs_add_device(chip);
+	if (rc)
+		goto del_misc;
+
+	rc = tpm_add_ppi(&chip->dev->kobj);
+	if (rc)
+		goto del_sysfs;
+
+	if (!chip->tpm2)
+		chip->bios_dir = tpm_bios_log_setup(chip->devname);
+
+	/* Make chip available */
+	spin_lock(&driver_lock);
+	list_add_rcu(&chip->list, &tpm_chip_list);
+	spin_unlock(&driver_lock);
+
+	return 0;
+del_sysfs:
+	tpm_sysfs_del_device(chip);
+del_misc:
+	tpm_dev_del_device(chip);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(tpm_chip_register);
+
+void tpm_chip_unregister(struct tpm_chip *chip)
+{
+	spin_lock(&driver_lock);
+	list_del_rcu(&chip->list);
+	spin_unlock(&driver_lock);
+	synchronize_rcu();
+
+	tpm_dev_del_device(chip);
+	tpm_sysfs_del_device(chip);
+	tpm_remove_ppi(&chip->dev->kobj);
+
+	if (!chip->tpm2)
+		tpm_bios_log_teardown(chip->bios_dir);
+}
+EXPORT_SYMBOL_GPL(tpm_chip_unregister);
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index f638f9d..3c54570 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -47,10 +47,6 @@ module_param_named(suspend_pcr, tpm_suspend_pcr, uint, 0644);
 MODULE_PARM_DESC(suspend_pcr,
 		 "PCR to use for dummy writes to faciltate flush on suspend.");
 
-static LIST_HEAD(tpm_chip_list);
-static DEFINE_SPINLOCK(driver_lock);
-static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
-
 /*
  * Array with one entry per ordinal defining the maximum amount
  * of time the chip could take to return the result.  The ordinal
@@ -636,27 +632,6 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
 	return rc;
 }
 
-/*
- * tpm_chip_find_get - return tpm_chip for given chip number
- */
-static struct tpm_chip *tpm_chip_find_get(int chip_num)
-{
-	struct tpm_chip *pos, *chip = NULL;
-
-	rcu_read_lock();
-	list_for_each_entry_rcu(pos, &tpm_chip_list, list) {
-		if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num)
-			continue;
-
-		if (try_module_get(pos->dev->driver->owner)) {
-			chip = pos;
-			break;
-		}
-	}
-	rcu_read_unlock();
-	return chip;
-}
-
 #define TPM_ORDINAL_PCRREAD cpu_to_be32(21)
 #define READ_PCR_RESULT_SIZE 30
 static struct tpm_input_header pcrread_header = {
@@ -893,15 +868,7 @@ void tpm_remove_hardware(struct device *dev)
 		return;
 	}
 
-	spin_lock(&driver_lock);
-	list_del_rcu(&chip->list);
-	spin_unlock(&driver_lock);
-	synchronize_rcu();
-
-	tpm_dev_del_device(chip);
-	tpm_sysfs_del_device(chip);
-	tpm_remove_ppi(&dev->kobj);
-	tpm_bios_log_teardown(chip->bios_dir);
+	tpm_chip_unregister(chip);
 
 	/* write it this way to be explicit (chip->dev == dev) */
 	put_device(chip->dev);
@@ -1041,35 +1008,6 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
 }
 EXPORT_SYMBOL_GPL(tpm_get_random);
 
-/* In case vendor provided release function, call it too.*/
-
-void tpm_dev_vendor_release(struct tpm_chip *chip)
-{
-	if (!chip)
-		return;
-
-	clear_bit(chip->dev_num, dev_mask);
-}
-EXPORT_SYMBOL_GPL(tpm_dev_vendor_release);
-
-
-/*
- * Once all references to platform device are down to 0,
- * release all allocated structures.
- */
-static void tpm_dev_release(struct device *dev)
-{
-	struct tpm_chip *chip = dev_get_drvdata(dev);
-
-	if (!chip)
-		return;
-
-	tpm_dev_vendor_release(chip);
-
-	chip->release(dev);
-	kfree(chip);
-}
-
 /*
  * Called from tpm_<specific>.c probe function only for devices
  * the driver has determined it should claim.  Prior to calling
@@ -1081,61 +1019,19 @@ struct tpm_chip *tpm_register_hardware(struct device *dev,
 				       const struct tpm_class_ops *ops)
 {
 	struct tpm_chip *chip;
+	int rc;
 
-	/* Driver specific per-device data */
-	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
-
-	if (chip == NULL)
+	chip = tpm_chip_alloc(dev, ops);
+	if (!chip)
 		return NULL;
 
-	mutex_init(&chip->tpm_mutex);
-	INIT_LIST_HEAD(&chip->list);
-
-	chip->ops = ops;
-	chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
-
-	if (chip->dev_num >= TPM_NUM_DEVICES) {
-		dev_err(dev, "No available tpm device numbers\n");
-		goto out_free;
+	rc = tpm_chip_register(chip);
+	if (rc) {
+		put_device(chip->dev);
+		kfree(chip);
 	}
 
-	set_bit(chip->dev_num, dev_mask);
-
-	scnprintf(chip->devname, sizeof(chip->devname), "%s%d", "tpm",
-		  chip->dev_num);
-
-	chip->dev = get_device(dev);
-	chip->release = dev->release;
-	dev->release = tpm_dev_release;
-	dev_set_drvdata(dev, chip);
-
-	if (tpm_dev_add_device(chip))
-		goto put_device;
-
-	if (tpm_sysfs_add_device(chip))
-		goto del_misc;
-
-	if (tpm_add_ppi(&dev->kobj))
-		goto del_sysfs;
-
-	chip->bios_dir = tpm_bios_log_setup(chip->devname);
-
-	/* Make chip available */
-	spin_lock(&driver_lock);
-	list_add_rcu(&chip->list, &tpm_chip_list);
-	spin_unlock(&driver_lock);
-
 	return chip;
-
-del_sysfs:
-	tpm_sysfs_del_device(chip);
-del_misc:
-	tpm_dev_del_device(chip);
-put_device:
-	put_device(chip->dev);
-out_free:
-	kfree(chip);
-	return NULL;
 }
 EXPORT_SYMBOL_GPL(tpm_register_hardware);
 
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index d893335..cc95a52 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -109,6 +109,8 @@ struct tpm_chip {
 
 	struct dentry **bios_dir;
 
+	bool tpm2; /* is this a TPM2 chip? */
+
 	struct list_head list;
 	void (*release) (struct device *);
 };
@@ -332,6 +334,13 @@ extern int tpm_pm_resume(struct device *);
 extern int wait_for_tpm_stat(struct tpm_chip *, u8, unsigned long,
 			     wait_queue_head_t *, bool);
 
+struct tpm_chip *tpm_chip_find_get(int chip_num);
+extern void tpm_dev_vendor_release(struct tpm_chip *);
+extern struct tpm_chip *tpm_chip_alloc(struct device *dev,
+				       const struct tpm_class_ops *ops);
+extern int tpm_chip_register(struct tpm_chip *chip);
+extern void tpm_chip_unregister(struct tpm_chip *chip);
+
 int tpm_dev_add_device(struct tpm_chip *chip);
 void tpm_dev_del_device(struct tpm_chip *chip);
 int tpm_sysfs_add_device(struct tpm_chip *chip);
-- 
2.1.0

Re: [PATCH v1 01/12] tpm: prepare TPM driver for adding TPM2 support

[Jason Gunthorpe]

On Wed, Sep 24, 2014 at 12:05:51PM +0300, Jarkko Sakkinen wrote:
> * Separated allocation and registeration into two functions:
>   * tpm_chip_alloc()
>   * tpm_chip_register()

Okay, this is a nice start!

I had intended tpm-interface.c to hold these 'chip' functions and the
other cmd related functions to be moved out into tpm-cmds.c (or
tpm1-cmds?), which is more symmetrical with what you are doing with
tpm2-cmds. Does that seem reasonable?

It doesn't make sense to leave some chip related stuff in
tpm-interface mixed with cmd stuff, I'd prefer to see a clean split if
you are going to split them.

> +struct tpm_chip *tpm_chip_alloc(struct device *dev,
> +                               const struct tpm_class_ops *ops)
> +{

I want to see a clear description of what the unwind procedures are
for each step using the new API, and kdoc on all the new API functions
drivers are expected to use.

> +int tpm_chip_register(struct tpm_chip *chip)
> +{
> +	int rc;

All the common startup functions need to be done here too: get
timeouts, self test, etc.

> +	rc = tpm_dev_add_device(chip);
> +	if (rc)
> +		return rc;
> +
> +	rc = tpm_sysfs_add_device(chip);
> +	if (rc)
> +		goto del_misc;
> +
> +	rc = tpm_add_ppi(&chip->dev->kobj);
> +	if (rc)
> +		goto del_sysfs;
> +
> +	if (!chip->tpm2)
> +		chip->bios_dir = tpm_bios_log_setup(chip->devname);

What about failure of tpm_bios_log_setup?

>  /*
>   * Called from tpm_<specific>.c probe function only for devices
>   * the driver has determined it should claim.  Prior to calling
> @@ -1081,61 +1019,19 @@ struct tpm_chip *tpm_register_hardware(struct device *dev,
>  				       const struct tpm_class_ops *ops)
>  {
>  	struct tpm_chip *chip;
> +	int rc;
>  
> -	/* Driver specific per-device data */
> -	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
> -
> -	if (chip == NULL)
> +	chip = tpm_chip_alloc(dev, ops);
> +	if (!chip)
>  		return NULL;

I think tpm_chip_alloc should return an ERR_PTR..

> -	mutex_init(&chip->tpm_mutex);
> -	INIT_LIST_HEAD(&chip->list);
> -
> -	chip->ops = ops;
> -	chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
> -
> -	if (chip->dev_num >= TPM_NUM_DEVICES) {
> -		dev_err(dev, "No available tpm device numbers\n");
> -		goto out_free;
> +	rc = tpm_chip_register(chip);
> +	if (rc) {
> +		put_device(chip->dev);
> +		kfree(chip);

No, open coding this is not a good unwind, and this is not correct
anyhow, it looks like it will double free chip...

This is where things got stuck for me too, the unwind is where all the
problems are and I think the solution is to make the alloc paths
completely different between new and old.

We really want tpmm_chip_alloc for the new style path (so drivers have
no unwind) and the old style path .. I don't know what is correct
there, but the safest thing is to stay exactly the same.

Also, please split this patch into moving stuff into tmp1-cmds.c and
then patching to add functionality.

Jason

[PATCH v1 02/12] tpm: TPM2 support for tpm_calc_ordinal_durations()

[Jarkko Sakkinen]

From: Will Arthur <will.c.arthur@intel.com>

Implemented TPM2 support for tpm_calc_ordinal_duration(). TPM2 version
is encapsulated into tpm2_calc_ordinal_duration() that is called by
tpm_calc_ordinal_duration() for TPM2 chips.

[jarkko.sakkinen: implemented tpm2_calc_ordinal_duration() based on
 the Wills original patch. Updated durations table to use worst case
 estimates.]

Signed-off-by: Will Arthur <will.c.arthur@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/Makefile        |   2 +-
 drivers/char/tpm/tpm-interface.c |   7 +-
 drivers/char/tpm/tpm.h           |   1 +
 drivers/char/tpm/tpm2-commands.c | 159 +++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm2.h          |  26 +++++++
 5 files changed, 193 insertions(+), 2 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2-commands.c
 create mode 100644 drivers/char/tpm/tpm2.h

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 837da04..5ff5f3d 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the kernel tpm device drivers.
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
-tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o
+tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-commands.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o
 
 ifdef CONFIG_ACPI
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 3c54570..07a2fc5 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -309,7 +309,12 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
 {
 	int duration_idx = TPM_UNDEFINED;
 	int duration = 0;
-	u8 category = (ordinal >> 24) & 0xFF;
+	u8 category;
+
+	if (chip->tpm2)
+		return tpm2_calc_ordinal_duration(chip, ordinal);
+
+	category = (ordinal >> 24) & 0xFF;
 
 	if ((category == TPM_PROTECTED_COMMAND && ordinal < TPM_MAX_ORDINAL) ||
 	    (category == TPM_CONNECTION_COMMAND && ordinal < TSC_MAX_ORDINAL))
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index cc95a52..bda88aa 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -27,6 +27,7 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/tpm.h>
+#include "tpm2.h"
 
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */
diff --git a/drivers/char/tpm/tpm2-commands.c b/drivers/char/tpm/tpm2-commands.c
new file mode 100644
index 0000000..14b3ae7
--- /dev/null
+++ b/drivers/char/tpm/tpm2-commands.c
@@ -0,0 +1,159 @@
+/*
+ * Copyright (C) 2014 Intel Corporation
+ *
+ * Authors:
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * This file contains TPM2 protocol implementations of the commands
+ * used by the kernel internally.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include "tpm.h"
+
+/*
+ * Array with one entry per ordinal defining the maximum amount
+ * of time the chip could take to return the result. The values
+ * of the SHORT, MEDIUM, and LONG durations are retrieved from 
+ * the chip during initialization with a call to tpm_get_timeouts.
+ */
+static const u8 tpm2_ordinal_duration[TPM2_CC_LAST - TPM2_CC_FIRST + 1] = {
+	TPM_UNDEFINED,		/* 11F */
+	TPM_UNDEFINED,		/* 120 */
+	TPM_LONG,		/* 121 */
+	TPM_UNDEFINED,		/* 122 */
+	TPM_UNDEFINED,		/* 123 */
+	TPM_UNDEFINED,		/* 124 */
+	TPM_UNDEFINED,		/* 125 */
+	TPM_UNDEFINED,		/* 126 */
+	TPM_UNDEFINED,		/* 127 */
+	TPM_UNDEFINED,		/* 128 */
+	TPM_LONG,		/* 129 */
+	TPM_UNDEFINED,		/* 12a */
+	TPM_UNDEFINED,		/* 12b */
+	TPM_UNDEFINED,		/* 12c */
+	TPM_UNDEFINED,		/* 12d */
+	TPM_UNDEFINED,		/* 12e */
+	TPM_UNDEFINED,		/* 12f */
+	TPM_UNDEFINED,		/* 130 */
+	TPM_UNDEFINED,		/* 131 */
+	TPM_UNDEFINED,		/* 132 */
+	TPM_UNDEFINED,		/* 133 */
+	TPM_UNDEFINED,		/* 134 */
+	TPM_UNDEFINED,		/* 135 */
+	TPM_UNDEFINED,		/* 136 */
+	TPM_UNDEFINED,		/* 137 */
+	TPM_UNDEFINED,		/* 138 */
+	TPM_UNDEFINED,		/* 139 */
+	TPM_UNDEFINED,		/* 13a */
+	TPM_UNDEFINED,		/* 13b */
+	TPM_UNDEFINED,		/* 13c */
+	TPM_UNDEFINED,		/* 13d */
+	TPM_MEDIUM,		/* 13e */
+	TPM_UNDEFINED,		/* 13f */
+	TPM_UNDEFINED,		/* 140 */
+	TPM_UNDEFINED,		/* 141 */
+	TPM_UNDEFINED,		/* 142 */
+	TPM_LONG,		/* 143 */
+	TPM_MEDIUM,		/* 144 */
+	TPM_UNDEFINED,		/* 145 */
+	TPM_UNDEFINED,		/* 146 */
+	TPM_UNDEFINED,		/* 147 */
+	TPM_UNDEFINED,		/* 148 */
+	TPM_UNDEFINED,		/* 149 */
+	TPM_UNDEFINED,		/* 14a */
+	TPM_UNDEFINED,		/* 14b */
+	TPM_UNDEFINED,		/* 14c */
+	TPM_UNDEFINED,		/* 14d */
+	TPM_LONG,		/* 14e */
+	TPM_UNDEFINED,		/* 14f */
+	TPM_UNDEFINED,		/* 150 */
+	TPM_UNDEFINED,		/* 151 */
+	TPM_UNDEFINED,		/* 152 */
+	TPM_UNDEFINED,		/* 153 */
+	TPM_UNDEFINED,		/* 154 */
+	TPM_UNDEFINED,		/* 155 */
+	TPM_UNDEFINED,		/* 156 */
+	TPM_UNDEFINED,		/* 157 */
+	TPM_UNDEFINED,		/* 158 */
+	TPM_UNDEFINED,		/* 159 */
+	TPM_UNDEFINED,		/* 15a */
+	TPM_UNDEFINED,		/* 15b */
+	TPM_MEDIUM,		/* 15c */
+	TPM_UNDEFINED,		/* 15d */
+	TPM_UNDEFINED,		/* 15e */
+	TPM_UNDEFINED,		/* 15f */
+	TPM_UNDEFINED,		/* 160 */
+	TPM_UNDEFINED,		/* 161 */
+	TPM_UNDEFINED,		/* 162 */
+	TPM_UNDEFINED,		/* 163 */
+	TPM_UNDEFINED,		/* 164 */
+	TPM_UNDEFINED,		/* 165 */
+	TPM_UNDEFINED,		/* 166 */
+	TPM_UNDEFINED,		/* 167 */
+	TPM_UNDEFINED,		/* 168 */
+	TPM_UNDEFINED,		/* 169 */
+	TPM_UNDEFINED,		/* 16a */
+	TPM_UNDEFINED,		/* 16b */
+	TPM_UNDEFINED,		/* 16c */
+	TPM_UNDEFINED,		/* 16d */
+	TPM_UNDEFINED,		/* 16e */
+	TPM_UNDEFINED,		/* 16f */
+	TPM_UNDEFINED,		/* 170 */
+	TPM_UNDEFINED,		/* 171 */
+	TPM_UNDEFINED,		/* 172 */
+	TPM_UNDEFINED,		/* 173 */
+	TPM_UNDEFINED,		/* 174 */
+	TPM_UNDEFINED,		/* 175 */
+	TPM_UNDEFINED,		/* 176 */
+	TPM_LONG,		/* 177 */
+	TPM_UNDEFINED,		/* 178 */
+	TPM_UNDEFINED,		/* 179 */
+	TPM_MEDIUM,		/* 17a */
+	TPM_LONG,		/* 17b */
+	TPM_UNDEFINED,		/* 17c */
+	TPM_UNDEFINED,		/* 17d */
+	TPM_UNDEFINED,		/* 17e */
+	TPM_UNDEFINED,		/* 17f */
+	TPM_UNDEFINED,		/* 180 */
+	TPM_UNDEFINED,		/* 181 */
+	TPM_MEDIUM,		/* 182 */
+	TPM_UNDEFINED,		/* 183 */
+	TPM_UNDEFINED,		/* 184 */
+	TPM_MEDIUM,		/* 185 */
+	TPM_MEDIUM,		/* 186 */
+	TPM_UNDEFINED,		/* 187 */
+	TPM_UNDEFINED,		/* 188 */
+	TPM_UNDEFINED,		/* 189 */
+	TPM_UNDEFINED,		/* 18a */
+	TPM_UNDEFINED,		/* 18b */
+	TPM_UNDEFINED,		/* 18c */
+	TPM_UNDEFINED,		/* 18d */
+	TPM_UNDEFINED,		/* 18e */
+	TPM_UNDEFINED		/* 18f */
+};
+
+/*
+ * Returns max number of jiffies to wait
+ */
+unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
+{
+	int index = TPM_UNDEFINED;
+	int duration = 0;
+
+	if (ordinal >= TPM2_CC_FIRST && ordinal <= TPM2_CC_LAST)
+		index = tpm2_ordinal_duration[ordinal - TPM2_CC_FIRST];
+
+	if (index != TPM_UNDEFINED)
+		duration = chip->vendor.duration[index];
+	if (duration <= 0)
+		return 2 * 60 * HZ;
+	else
+		return duration;
+}
diff --git a/drivers/char/tpm/tpm2.h b/drivers/char/tpm/tpm2.h
new file mode 100644
index 0000000..dc0a2a2
--- /dev/null
+++ b/drivers/char/tpm/tpm2.h
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2014 Intel Corporation
+ *
+ * Authors:
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ */
+
+#ifndef __DRIVERS_CHAR_TPM2_H__
+#define __DRIVERS_CHAR_TPM2_H__
+
+struct tpm_chip;
+
+#define TPM2_CC_FIRST	0x11F
+#define TPM2_CC_LAST	0x18F
+
+unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *, u32);
+
+#endif /* __DRIVERS_CHAR_TPM2_H__ */
-- 
2.1.0

[PATCH v1 03/12] tpm: TPM2 support for tpm_pcr_read()

[Jarkko Sakkinen]

Implemented TPM2 support for tpm_pcr_read() by adding a new function
tpm2_pcr_read_dev() that is called for TPM2 chipsets.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c |  5 ++++-
 drivers/char/tpm/tpm.h           | 21 ++++++++++++++++++++
 drivers/char/tpm/tpm2-commands.c | 43 ++++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm2.h          | 18 +++++++++++++++++
 4 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 07a2fc5..4494b16 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -680,7 +680,10 @@ int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf)
 	chip = tpm_chip_find_get(chip_num);
 	if (chip == NULL)
 		return -ENODEV;
-	rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf);
+	if (chip->tpm2)
+		rc = tpm2_pcr_read_dev(chip, pcr_idx, res_buf);
+	else
+		rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf);
 	tpm_chip_put(chip);
 	return rc;
 }
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index bda88aa..10d6731 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -298,6 +298,25 @@ struct tpm_startup_in {
 	__be16	startup_type;
 } __packed;
 
+struct tpm2_pcr_read_in {
+	__be32	pcr_selects_cnt;
+	__be16	hash_alg;
+	u8	pcr_select_size;
+	u8	pcr_select[TPM2_PCR_SELECT_MIN];
+} __packed;
+
+
+struct tpm2_pcr_read_out {
+	__be32	update_cnt;
+	__be32	pcr_selects_cnt;
+	__be16	hash_alg;
+	u8	pcr_select_size;
+	u8	pcr_select[TPM2_PCR_SELECT_MIN];
+	__be32	digests_cnt;
+	__be16	digest_size;
+	u8	digest[TPM_DIGEST_SIZE];
+} __packed;
+
 typedef union {
 	struct	tpm_getcap_params_out getcap_out;
 	struct	tpm_readpubek_params_out readpubek_out;
@@ -309,6 +328,8 @@ typedef union {
 	struct	tpm_getrandom_in getrandom_in;
 	struct	tpm_getrandom_out getrandom_out;
 	struct tpm_startup_in startup_in;
+	struct	tpm2_pcr_read_in tpm2_pcrread_in;
+	struct	tpm2_pcr_read_out tpm2_pcrread_out;
 } tpm_cmd_params;
 
 struct tpm_cmd_t {
diff --git a/drivers/char/tpm/tpm2-commands.c b/drivers/char/tpm/tpm2-commands.c
index 14b3ae7..2fb553c 100644
--- a/drivers/char/tpm/tpm2-commands.c
+++ b/drivers/char/tpm/tpm2-commands.c
@@ -157,3 +157,46 @@ unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
 	else
 		return duration;
 }
+
+#define TPM2_PCR_READ_IN_SIZE \
+	(sizeof(struct tpm_input_header) + \
+	 sizeof(struct tpm2_pcr_read_in))
+
+static struct tpm_input_header tpm2_pcrread_header = {
+	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
+	.length = cpu_to_be32(TPM2_PCR_READ_IN_SIZE),
+	.ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
+};
+
+int tpm2_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
+{
+	int rc;
+	struct tpm_cmd_t cmd;
+	u8 *buf;
+	int i, j;
+
+	if (pcr_idx >= TPM2_PLATFORM_PCR)
+		return -EINVAL;
+
+	cmd.header.in = tpm2_pcrread_header;
+	cmd.params.tpm2_pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
+	cmd.params.tpm2_pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
+	cmd.params.tpm2_pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
+
+	for (i = 0; i < TPM2_PCR_SELECT_MIN; i++) {
+		j = pcr_idx - i * 8;
+
+		cmd.params.tpm2_pcrread_in.pcr_select[i] =
+			(j >= 0 && j < 8) ? 1 << j : 0;
+	}
+
+	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+			      "attempting to read a pcr value");
+
+	if (rc == 0) {
+		buf = cmd.params.tpm2_pcrread_out.digest;
+		memcpy(res_buf, buf, TPM_DIGEST_SIZE);
+	}
+
+	return rc;
+}
diff --git a/drivers/char/tpm/tpm2.h b/drivers/char/tpm/tpm2.h
index dc0a2a2..fbab49c 100644
--- a/drivers/char/tpm/tpm2.h
+++ b/drivers/char/tpm/tpm2.h
@@ -16,11 +16,29 @@
 #ifndef __DRIVERS_CHAR_TPM2_H__
 #define __DRIVERS_CHAR_TPM2_H__
 
+enum tpm2_const {
+	TPM2_PLATFORM_PCR = 24,
+	TPM2_PCR_SELECT_MIN = ((TPM2_PLATFORM_PCR + 7) / 8),
+};
+
+enum tpm2_structures {
+	TPM2_ST_NO_SESSIONS	= 0x8001,
+};
+
+enum tpm2_algorithms {
+	TPM2_ALG_SHA1		= 0x0004,
+};
+
+enum tpm2_command_codes {
+	TPM2_CC_PCR_READ	= 0x017E,
+};
+
 struct tpm_chip;
 
 #define TPM2_CC_FIRST	0x11F
 #define TPM2_CC_LAST	0x18F
 
 unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *, u32);
+int tpm2_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
 
 #endif /* __DRIVERS_CHAR_TPM2_H__ */
-- 
2.1.0

Re: [PATCH v1 03/12] tpm: TPM2 support for tpm_pcr_read()

[Jason Gunthorpe]

On Wed, Sep 24, 2014 at 12:05:53PM +0300, Jarkko Sakkinen wrote:

> +static struct tpm_input_header tpm2_pcrread_header = {

Missing const - all of these static structures in tpm2-cmds.c are missing the
const, please fix them all.

> +	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> +	.length = cpu_to_be32(TPM2_PCR_READ_IN_SIZE),
> +	.ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
> +};

BTW, I always thought this was a goofy and very expensive way to store
3 values and zero initialize. If you want to do something different in
the tpm2-cmds.c that would be great too...

Jason

Re: [PATCH v1 03/12] tpm: TPM2 support for tpm_pcr_read()

[Jarkko Sakkinen]

On Wed, Sep 24, 2014 at 10:53:20AM -0600, Jason Gunthorpe wrote:
> On Wed, Sep 24, 2014 at 12:05:53PM +0300, Jarkko Sakkinen wrote:
> 
> > +static struct tpm_input_header tpm2_pcrread_header = {
> 
> Missing const - all of these static structures in tpm2-cmds.c are missing the
> const, please fix them all.
> 
> > +	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> > +	.length = cpu_to_be32(TPM2_PCR_READ_IN_SIZE),
> > +	.ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
> > +};
> 
> BTW, I always thought this was a goofy and very expensive way to store
> 3 values and zero initialize. If you want to do something different in
> the tpm2-cmds.c that would be great too...

What do you think about the way trusted module builds messages? It's easier 
to maintain and debug than the approach used in the tpm subsystem.

That's the approach I was thinking to use when I started doing this but
didn't want to diverge before I get feedback.

> Jason

/Jarkko

Re: [PATCH v1 03/12] tpm: TPM2 support for tpm_pcr_read()

[Peter Hüwe]

Am Mittwoch, 24. September 2014, 21:43:23 schrieb Jarkko Sakkinen:
> On Wed, Sep 24, 2014 at 10:53:20AM -0600, Jason Gunthorpe wrote:
> > On Wed, Sep 24, 2014 at 12:05:53PM +0300, Jarkko Sakkinen wrote:
> > > +static struct tpm_input_header tpm2_pcrread_header = {
> > 
> > Missing const - all of these static structures in tpm2-cmds.c are missing
> > the const, please fix them all.
> > 
> > > +	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> > > +	.length = cpu_to_be32(TPM2_PCR_READ_IN_SIZE),
> > > +	.ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
> > > +};
> > 
> > BTW, I always thought this was a goofy and very expensive way to store
> > 3 values and zero initialize. If you want to do something different in
> > the tpm2-cmds.c that would be great too...
> 
> What do you think about the way trusted module builds messages? It's easier
> to maintain and debug than the approach used in the tpm subsystem.


Can you post an example (e.g. for the command stream above)

Peter

Re: [PATCH v1 03/12] tpm: TPM2 support for tpm_pcr_read()

[Jason Gunthorpe]

On Wed, Sep 24, 2014 at 10:43:23PM +0300, Jarkko Sakkinen wrote:

> What do you think about the way trusted module builds messages? It's
> easier to maintain and debug than the approach used in the tpm
> subsystem.

Do you have a source reference?

In my userspace TPM work I use a code generator to produce all the
marshal/unmarshal/AUTH directly from the TPM specification text. The
full generality of TPM RPCs is sufficiently complex that hand coding
is too error prone. The handful of ops the kernel does is not so bad
though...

Jason

[PATCH v1 04/12] tpm: TPM2 support for tpm_do_selftest()

[Jarkko Sakkinen]

Implemented tpm2_do_selftest() that runs the full (all commands)
self-test for the TPM chip. During the self-test TPM2 commands return
with the TPM error code RC_TESTING. Waiting is done by issuing PCR
read until it executes succesfully or some other error condition
than RC_TESTING happens.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c |  3 ++
 drivers/char/tpm/tpm.h           |  5 +++
 drivers/char/tpm/tpm2-commands.c | 82 ++++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm2.h          |  7 ++++
 4 files changed, 97 insertions(+)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 4494b16..069be1e 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -744,6 +744,9 @@ int tpm_do_selftest(struct tpm_chip *chip)
 	unsigned long duration;
 	struct tpm_cmd_t cmd;
 
+	if (chip->tpm2)
+		return tpm2_do_selftest(chip);
+
 	duration = tpm_calc_ordinal_duration(chip, TPM_ORD_CONTINUE_SELFTEST);
 
 	loops = jiffies_to_msecs(duration) / delay_msec;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 10d6731..7cb0206 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -298,6 +298,10 @@ struct tpm_startup_in {
 	__be16	startup_type;
 } __packed;
 
+struct tpm2_self_test_in {
+	u8	full_test;
+} __packed;
+
 struct tpm2_pcr_read_in {
 	__be32	pcr_selects_cnt;
 	__be16	hash_alg;
@@ -330,6 +334,7 @@ typedef union {
 	struct tpm_startup_in startup_in;
 	struct	tpm2_pcr_read_in tpm2_pcrread_in;
 	struct	tpm2_pcr_read_out tpm2_pcrread_out;
+	struct	tpm2_self_test_in tpm2_selftest_in;
 } tpm_cmd_params;
 
 struct tpm_cmd_t {
diff --git a/drivers/char/tpm/tpm2-commands.c b/drivers/char/tpm/tpm2-commands.c
index 2fb553c..d54a0d0 100644
--- a/drivers/char/tpm/tpm2-commands.c
+++ b/drivers/char/tpm/tpm2-commands.c
@@ -200,3 +200,85 @@ int tpm2_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
 
 	return rc;
 }
+
+static struct tpm_input_header tpm2_selftest_header = {
+	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
+	.length = cpu_to_be32(sizeof(struct tpm_input_header) +
+			      sizeof(struct tpm2_self_test_in)),
+	.ordinal = cpu_to_be32(TPM2_CC_SELF_TEST)
+};
+
+#define TPM2_SELF_TEST_IN_SIZE \
+	(sizeof(struct tpm_input_header) + sizeof(struct tpm2_self_test_in))
+
+/**
+ * tpm2_continue_selftest -- start a self test
+ * @chip: TPM chip to use
+ * @full: test all commands instead of testing only those that were not
+ *        previously tested.
+ *
+ * Returns 0 on success, < 0 on a system error and > 0 on a TPM error.
+ */
+static int tpm2_start_selftest(struct tpm_chip *chip, bool full)
+{
+	int rc;
+	struct tpm_cmd_t cmd;
+
+	cmd.header.in = tpm2_selftest_header;
+	cmd.params.tpm2_selftest_in.full_test = full;
+
+	rc = tpm_transmit_cmd(chip, &cmd, TPM2_SELF_TEST_IN_SIZE,
+			      "continue selftest");
+
+	return rc;
+}
+
+/**
+ * tpm2_do_selftest -- start a full self test and wait until it is completed.
+ *                     During the self test TPM2 commands return with the error
+ *                     code RC_TESTING. Waiting is done by issuing PCR read until
+ *                     it executes succesfully.
+ * @chip: TPM chip to use
+ *
+ * Returns 0 on success, < 0 on a system error and > 0 on a TPM error.
+ */
+int tpm2_do_selftest(struct tpm_chip *chip)
+{
+	int rc;
+	unsigned int loops;
+	unsigned int delay_msec = 100;
+	unsigned long duration;
+	struct tpm_cmd_t cmd;
+	int i;
+
+	duration = tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST);
+
+	loops = jiffies_to_msecs(duration) / delay_msec;
+
+	rc = tpm2_start_selftest(chip, true);
+	if (rc)
+		return rc;
+
+	for (i = 0; i < loops; i++) {
+		/* Attempt to read a PCR value */
+		cmd.header.in = tpm2_pcrread_header;
+		cmd.params.tpm2_pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
+		cmd.params.tpm2_pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
+		cmd.params.tpm2_pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
+		cmd.params.tpm2_pcrread_in.pcr_select[0] = 0x01;
+		cmd.params.tpm2_pcrread_in.pcr_select[1] = 0x00;
+		cmd.params.tpm2_pcrread_in.pcr_select[2] = 0x00;
+
+		rc = tpm_transmit(chip, (u8 *) &cmd, sizeof(cmd));
+		if (rc < 0)
+			break;
+
+		rc = be32_to_cpu(cmd.header.out.return_code);
+		if (rc != TPM2_RC_TESTING)
+			break;
+
+		msleep(delay_msec);
+	}
+
+	return rc;
+}
diff --git a/drivers/char/tpm/tpm2.h b/drivers/char/tpm/tpm2.h
index fbab49c..b1721f9 100644
--- a/drivers/char/tpm/tpm2.h
+++ b/drivers/char/tpm/tpm2.h
@@ -25,11 +25,17 @@ enum tpm2_structures {
 	TPM2_ST_NO_SESSIONS	= 0x8001,
 };
 
+enum tpm2_return_codes {
+	TPM2_RC_TESTING		= 0x090A,
+	TPM2_RC_DISABLED	= 0x0120,
+};
+
 enum tpm2_algorithms {
 	TPM2_ALG_SHA1		= 0x0004,
 };
 
 enum tpm2_command_codes {
+	TPM2_CC_SELF_TEST	= 0x0143,
 	TPM2_CC_PCR_READ	= 0x017E,
 };
 
@@ -40,5 +46,6 @@ struct tpm_chip;
 
 unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *, u32);
 int tpm2_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
+int tpm2_do_selftest(struct tpm_chip *chip);
 
 #endif /* __DRIVERS_CHAR_TPM2_H__ */
-- 
2.1.0

[PATCH v1 05/12] tpm: added tpm2_get_tpm_pt()

[Jarkko Sakkinen]

Added the function tpm2_get_tpm_pt() for acquiring TPM properties
(properties under under TPM_CAP_TPM_PROPERTIES capability).

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm.h           | 16 ++++++++++++++++
 drivers/char/tpm/tpm2-commands.c | 30 ++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm2.h          |  7 +++++++
 3 files changed, 53 insertions(+)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 7cb0206..3cdbf9c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -321,6 +321,20 @@ struct tpm2_pcr_read_out {
 	u8	digest[TPM_DIGEST_SIZE];
 } __packed;
 
+struct tpm2_get_tpm_pt_in {
+	__be32	cap_id;
+	__be32	property_id;
+	__be32	property_cnt;
+} __packed;
+
+struct tpm2_get_tpm_pt_out {
+	u8	more_data;
+	__be32	subcap_id;
+	__be32	property_cnt;
+	__be32	property_id;
+	__be32	value;
+} __packed;
+
 typedef union {
 	struct	tpm_getcap_params_out getcap_out;
 	struct	tpm_readpubek_params_out readpubek_out;
@@ -335,6 +349,8 @@ typedef union {
 	struct	tpm2_pcr_read_in tpm2_pcrread_in;
 	struct	tpm2_pcr_read_out tpm2_pcrread_out;
 	struct	tpm2_self_test_in tpm2_selftest_in;
+	struct	tpm2_get_tpm_pt_in tpm2_get_tpm_pt_in;
+	struct	tpm2_get_tpm_pt_out tpm2_get_tpm_pt_out;
 } tpm_cmd_params;
 
 struct tpm_cmd_t {
diff --git a/drivers/char/tpm/tpm2-commands.c b/drivers/char/tpm/tpm2-commands.c
index d54a0d0..11c031b 100644
--- a/drivers/char/tpm/tpm2-commands.c
+++ b/drivers/char/tpm/tpm2-commands.c
@@ -158,6 +158,36 @@ unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
 		return duration;
 }
 
+#define TPM2_GET_TPM_PT_IN_SIZE \
+	(sizeof(struct tpm_input_header) + \
+	 sizeof(struct tpm2_get_tpm_pt_in))
+
+static struct tpm_input_header tpm2_get_tpm_pt_header = {
+	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
+	.length = cpu_to_be32(TPM2_GET_TPM_PT_IN_SIZE),
+	.ordinal = cpu_to_be32(TPM2_CC_GET_CAPABILITY)
+};
+
+ssize_t tpm2_get_tpm_pt(struct device *dev, u32 property_id,  u32* value,
+			const char *desc)
+{
+	struct tpm_cmd_t cmd;
+	int rc;
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+
+	cmd.header.in = tpm2_get_tpm_pt_header;
+	cmd.params.tpm2_get_tpm_pt_in.cap_id =
+		cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
+	cmd.params.tpm2_get_tpm_pt_in.property_id = property_id;
+	cmd.params.tpm2_get_tpm_pt_in.property_cnt = cpu_to_be32(1);
+
+	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), desc);
+	if (!rc)
+		*value = cmd.params.tpm2_get_tpm_pt_out.value;
+
+	return rc;
+}
+
 #define TPM2_PCR_READ_IN_SIZE \
 	(sizeof(struct tpm_input_header) + \
 	 sizeof(struct tpm2_pcr_read_in))
diff --git a/drivers/char/tpm/tpm2.h b/drivers/char/tpm/tpm2.h
index b1721f9..61ab0df 100644
--- a/drivers/char/tpm/tpm2.h
+++ b/drivers/char/tpm/tpm2.h
@@ -36,15 +36,22 @@ enum tpm2_algorithms {
 
 enum tpm2_command_codes {
 	TPM2_CC_SELF_TEST	= 0x0143,
+	TPM2_CC_GET_CAPABILITY	= 0x017A,
 	TPM2_CC_PCR_READ	= 0x017E,
 };
 
+enum tpm2_capabilities {
+	TPM2_CAP_TPM_PROPERTIES = 6,
+};
+
 struct tpm_chip;
 
 #define TPM2_CC_FIRST	0x11F
 #define TPM2_CC_LAST	0x18F
 
 unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *, u32);
+ssize_t tpm2_get_tpm_pt(struct device *dev, u32 property_id,  u32* value,
+			const char *desc);
 int tpm2_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
 int tpm2_do_selftest(struct tpm_chip *chip);
 
-- 
2.1.0

[PATCH v1 06/12] tpm: TPM2 support for tpm_pcr_extend()

[Jarkko Sakkinen]

Implemented TPM2 protocol support for tpm_pcr_extend().

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c |  6 ++++++
 drivers/char/tpm/tpm.h           | 17 +++++++++++++++++
 drivers/char/tpm/tpm2-commands.c | 37 +++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm2.h          |  7 +++++++
 4 files changed, 67 insertions(+)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 069be1e..ed23ba3 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -717,6 +717,12 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
 	if (chip == NULL)
 		return -ENODEV;
 
+	if (chip->tpm2) {
+		rc = tpm2_pcr_extend(chip, pcr_idx, hash);
+		tpm_chip_put(chip);
+		return rc;
+	}
+
 	cmd.header.in = pcrextend_header;
 	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
 	memcpy(cmd.params.pcrextend_in.hash, hash, TPM_DIGEST_SIZE);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 3cdbf9c..4637342 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -321,6 +321,22 @@ struct tpm2_pcr_read_out {
 	u8	digest[TPM_DIGEST_SIZE];
 } __packed;
 
+struct tpm2_null_auth_area {
+	__be32			handle;
+	__be16			nonce_size;
+	u8			attributes;
+	__be16			auth_size;
+} __packed;
+
+struct tpm2_pcr_extend_in {
+	__be32				pcr_idx;
+	__be32				auth_area_size;
+	struct tpm2_null_auth_area	auth_area;
+	__be32				digest_cnt;
+	__be16				hash_alg;
+	u8				digest[TPM_DIGEST_SIZE];
+} __packed;
+
 struct tpm2_get_tpm_pt_in {
 	__be32	cap_id;
 	__be32	property_id;
@@ -348,6 +364,7 @@ typedef union {
 	struct tpm_startup_in startup_in;
 	struct	tpm2_pcr_read_in tpm2_pcrread_in;
 	struct	tpm2_pcr_read_out tpm2_pcrread_out;
+	struct	tpm2_pcr_extend_in tpm2_pcrextend_in;
 	struct	tpm2_self_test_in tpm2_selftest_in;
 	struct	tpm2_get_tpm_pt_in tpm2_get_tpm_pt_in;
 	struct	tpm2_get_tpm_pt_out tpm2_get_tpm_pt_out;
diff --git a/drivers/char/tpm/tpm2-commands.c b/drivers/char/tpm/tpm2-commands.c
index 11c031b..b69999e 100644
--- a/drivers/char/tpm/tpm2-commands.c
+++ b/drivers/char/tpm/tpm2-commands.c
@@ -231,6 +231,43 @@ int tpm2_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
 	return rc;
 }
 
+/**
+ * tpm_pcr_extend - extend pcr value with hash
+ * @chip_num:	tpm idx # or AN&
+ * @pcr_idx:	pcr idx to extend
+ * @hash:	hash value used to extend pcr value
+ */
+static struct tpm_input_header tpm2_pcrextend_header = {
+	.tag = cpu_to_be16(TPM2_ST_SESSIONS),
+	.length = cpu_to_be32(sizeof(struct tpm_input_header) +
+			      sizeof(struct tpm2_pcr_extend_in)),
+	.ordinal = cpu_to_be32(TPM2_CC_PCR_EXTEND)
+};
+
+int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
+{
+	struct tpm_cmd_t cmd;
+	int rc;
+
+	cmd.header.in = tpm2_pcrextend_header;
+	cmd.params.tpm2_pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
+	cmd.params.tpm2_pcrextend_in.auth_area_size =
+		cpu_to_be32(sizeof(struct tpm2_null_auth_area));
+	cmd.params.tpm2_pcrextend_in.auth_area.handle =
+		cpu_to_be32(TPM2_RS_PW);
+	cmd.params.tpm2_pcrextend_in.auth_area.nonce_size = 0;
+	cmd.params.tpm2_pcrextend_in.auth_area.attributes = 0;
+	cmd.params.tpm2_pcrextend_in.auth_area.auth_size = 0;
+	cmd.params.tpm2_pcrextend_in.digest_cnt = cpu_to_be32(1);
+	cmd.params.tpm2_pcrextend_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
+	memcpy(cmd.params.tpm2_pcrextend_in.digest, hash, TPM_DIGEST_SIZE);
+
+	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+			      "attempting extend a PCR value");
+
+	return rc;
+}
+
 static struct tpm_input_header tpm2_selftest_header = {
 	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
 	.length = cpu_to_be32(sizeof(struct tpm_input_header) +
diff --git a/drivers/char/tpm/tpm2.h b/drivers/char/tpm/tpm2.h
index 61ab0df..e4ed2e5 100644
--- a/drivers/char/tpm/tpm2.h
+++ b/drivers/char/tpm/tpm2.h
@@ -23,6 +23,7 @@ enum tpm2_const {
 
 enum tpm2_structures {
 	TPM2_ST_NO_SESSIONS	= 0x8001,
+	TPM2_ST_SESSIONS	= 0x8002,
 };
 
 enum tpm2_return_codes {
@@ -38,6 +39,11 @@ enum tpm2_command_codes {
 	TPM2_CC_SELF_TEST	= 0x0143,
 	TPM2_CC_GET_CAPABILITY	= 0x017A,
 	TPM2_CC_PCR_READ	= 0x017E,
+	TPM2_CC_PCR_EXTEND	= 0x0182,
+};
+
+enum tpm2_permanent_handles {
+	TPM2_RS_PW		= 0x40000009,
 };
 
 enum tpm2_capabilities {
@@ -53,6 +59,7 @@ unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *, u32);
 ssize_t tpm2_get_tpm_pt(struct device *dev, u32 property_id,  u32* value,
 			const char *desc);
 int tpm2_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
+int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
 int tpm2_do_selftest(struct tpm_chip *chip);
 
 #endif /* __DRIVERS_CHAR_TPM2_H__ */
-- 
2.1.0

[PATCH v1 07/12] tpm: TPM2 support for tpm_get_random().

[Jarkko Sakkinen]

Implemented TPM2 support for tpm_get_random().

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c |  6 ++++++
 drivers/char/tpm/tpm.h           | 11 ++++++++++
 drivers/char/tpm/tpm2-commands.c | 45 ++++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm2.h          |  2 ++
 4 files changed, 64 insertions(+)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index ed23ba3..3d4a664 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1002,6 +1002,12 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
 	if (chip == NULL)
 		return -ENODEV;
 
+	if (chip->tpm2) {
+		err = tpm2_get_random(chip, out, max);
+		tpm_chip_put(chip);
+		return err;
+	}
+
 	do {
 		tpm_cmd.header.in = tpm_getrandom_header;
 		tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4637342..b4c0d5d 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -351,6 +351,15 @@ struct tpm2_get_tpm_pt_out {
 	__be32	value;
 } __packed;
 
+struct tpm2_get_random_in {
+	__be16	size;
+} __packed;
+
+struct tpm2_get_random_out {
+	__be16	size;
+	u8	buffer[TPM_MAX_RNG_DATA];
+} __packed;
+
 typedef union {
 	struct	tpm_getcap_params_out getcap_out;
 	struct	tpm_readpubek_params_out readpubek_out;
@@ -368,6 +377,8 @@ typedef union {
 	struct	tpm2_self_test_in tpm2_selftest_in;
 	struct	tpm2_get_tpm_pt_in tpm2_get_tpm_pt_in;
 	struct	tpm2_get_tpm_pt_out tpm2_get_tpm_pt_out;
+	struct	tpm2_get_random_in tpm2_getrandom_in;
+	struct	tpm2_get_random_out tpm2_getrandom_out;
 } tpm_cmd_params;
 
 struct tpm_cmd_t {
diff --git a/drivers/char/tpm/tpm2-commands.c b/drivers/char/tpm/tpm2-commands.c
index b69999e..90cebb2 100644
--- a/drivers/char/tpm/tpm2-commands.c
+++ b/drivers/char/tpm/tpm2-commands.c
@@ -349,3 +349,48 @@ int tpm2_do_selftest(struct tpm_chip *chip)
 
 	return rc;
 }
+
+static struct tpm_input_header tpm2_getrandom_header = {
+	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
+	.length = cpu_to_be32(sizeof(struct tpm_input_header) +
+			      sizeof(struct tpm2_get_random_in)),
+	.ordinal = cpu_to_be32(TPM2_CC_GET_RANDOM)
+};
+
+/**
+ * tpm2_get_random() - get random bytes from the TPM RNG
+ * @chip: TPM chip to use
+ * @out: destination buffer for the random bytes
+ * @max: the max number of bytes to write to @out
+ *
+ * Returns < 0 on error and the number of bytes read on success
+ */
+int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
+{
+	struct tpm_cmd_t tpm_cmd;
+	u32 recd, num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA);
+	int err, total = 0, retries = 5;
+	u8 *dest = out;
+
+	if (!out || !num_bytes || max > TPM_MAX_RNG_DATA)
+		return -EINVAL;
+
+	do {
+		tpm_cmd.header.in = tpm2_getrandom_header;
+		tpm_cmd.params.tpm2_getrandom_in.size = cpu_to_be16(num_bytes);
+
+		err = tpm_transmit_cmd(chip, &tpm_cmd, sizeof(tpm_cmd),
+				       "attempting get random");
+		if (err)
+			break;
+
+		recd = be16_to_cpu(tpm_cmd.params.tpm2_getrandom_out.size);
+		memcpy(dest, tpm_cmd.params.tpm2_getrandom_out.buffer, recd);
+
+		dest += recd;
+		total += recd;
+		num_bytes -= recd;
+	} while (retries-- && total < max);
+
+	return total ? total : -EIO;
+}
diff --git a/drivers/char/tpm/tpm2.h b/drivers/char/tpm/tpm2.h
index e4ed2e5..6ec84bc 100644
--- a/drivers/char/tpm/tpm2.h
+++ b/drivers/char/tpm/tpm2.h
@@ -38,6 +38,7 @@ enum tpm2_algorithms {
 enum tpm2_command_codes {
 	TPM2_CC_SELF_TEST	= 0x0143,
 	TPM2_CC_GET_CAPABILITY	= 0x017A,
+	TPM2_CC_GET_RANDOM	= 0x017B,
 	TPM2_CC_PCR_READ	= 0x017E,
 	TPM2_CC_PCR_EXTEND	= 0x0182,
 };
@@ -61,5 +62,6 @@ ssize_t tpm2_get_tpm_pt(struct device *dev, u32 property_id,  u32* value,
 int tpm2_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
 int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
 int tpm2_do_selftest(struct tpm_chip *chip);
+int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
 
 #endif /* __DRIVERS_CHAR_TPM2_H__ */
-- 
2.1.0

[PATCH v1 08/12] tpm: TPM2 support for tpm_startup()

[Jarkko Sakkinen]

From: Will Arthur <will.c.arthur@intel.com>

This patch implements TPM2 support for tpm_startup().

[jarkko.sakkinen: implemented tpm2_startup() based on the Wills
 original patch.]

Signed-off-by: Will Arthur <will.c.arthur@intel.com>
---
 drivers/char/tpm/tpm-interface.c |  6 +++++-
 drivers/char/tpm/tpm.h           |  4 ++++
 drivers/char/tpm/tpm2-commands.c | 17 +++++++++++++++++
 drivers/char/tpm/tpm2.h          |  2 ++
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 3d4a664..f0c9009 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -474,6 +474,7 @@ EXPORT_SYMBOL_GPL(tpm_gen_interrupt);
 #define TPM_ST_CLEAR cpu_to_be16(1)
 #define TPM_ST_STATE cpu_to_be16(2)
 #define TPM_ST_DEACTIVATED cpu_to_be16(3)
+
 static const struct tpm_input_header tpm_startup_header = {
 	.tag = TPM_TAG_RQU_COMMAND,
 	.length = cpu_to_be32(12),
@@ -483,7 +484,10 @@ static const struct tpm_input_header tpm_startup_header = {
 static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
 {
 	struct tpm_cmd_t start_cmd;
-	start_cmd.header.in = tpm_startup_header;
+
+	if (chip->tpm2)
+		return tpm2_startup(chip, startup_type);
+
 	start_cmd.params.startup_in.startup_type = startup_type;
 	return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
 				"attempting to start the TPM");
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index b4c0d5d..5a29fb7 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -298,6 +298,10 @@ struct tpm_startup_in {
 	__be16	startup_type;
 } __packed;
 
+struct tpm2_startup_in {
+	__be16	startup_type;
+} __packed;
+
 struct tpm2_self_test_in {
 	u8	full_test;
 } __packed;
diff --git a/drivers/char/tpm/tpm2-commands.c b/drivers/char/tpm/tpm2-commands.c
index 90cebb2..c36f7fb 100644
--- a/drivers/char/tpm/tpm2-commands.c
+++ b/drivers/char/tpm/tpm2-commands.c
@@ -158,6 +158,23 @@ unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
 		return duration;
 }
 
+static const struct tpm_input_header tpm2_startup_header = {
+	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
+	.length = cpu_to_be32(12),
+	.ordinal = cpu_to_be32(TPM2_CC_STARTUP)
+};
+
+int tpm2_startup(struct tpm_chip *chip, __be16 startup_type)
+{
+	struct tpm_cmd_t cmd;
+
+	cmd.header.in = tpm2_startup_header;
+
+	cmd.params.startup_in.startup_type = startup_type;
+	return tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+				"attempting to start the TPM");
+}
+
 #define TPM2_GET_TPM_PT_IN_SIZE \
 	(sizeof(struct tpm_input_header) + \
 	 sizeof(struct tpm2_get_tpm_pt_in))
diff --git a/drivers/char/tpm/tpm2.h b/drivers/char/tpm/tpm2.h
index 6ec84bc..73a1c35 100644
--- a/drivers/char/tpm/tpm2.h
+++ b/drivers/char/tpm/tpm2.h
@@ -37,6 +37,7 @@ enum tpm2_algorithms {
 
 enum tpm2_command_codes {
 	TPM2_CC_SELF_TEST	= 0x0143,
+	TPM2_CC_STARTUP		= 0x0144,
 	TPM2_CC_GET_CAPABILITY	= 0x017A,
 	TPM2_CC_GET_RANDOM	= 0x017B,
 	TPM2_CC_PCR_READ	= 0x017E,
@@ -57,6 +58,7 @@ struct tpm_chip;
 #define TPM2_CC_LAST	0x18F
 
 unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *, u32);
+int tpm2_startup(struct tpm_chip *chip, __be16 startup_type);
 ssize_t tpm2_get_tpm_pt(struct device *dev, u32 property_id,  u32* value,
 			const char *desc);
 int tpm2_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
-- 
2.1.0

[PATCH v1 09/12] tpm: TPM2 support for tpm_gen_interrupt().

[Jarkko Sakkinen]

From: Will Arthur <will.c.arthur@intel.com>

This patch adds TPM2 support for tpm_gen_interrupt().

[jarkko.sakkinen: implemented tpm2_gen_interrupt() based on Wills
 original patch that is called from tpm_gen_interrupt() for TPM2
 chipsets.]

Signed-off-by: Will Arthur <will.c.arthur@intel.com>
---
 drivers/char/tpm/tpm-interface.c | 5 +++++
 drivers/char/tpm/tpm2-commands.c | 9 +++++++++
 drivers/char/tpm/tpm2.h          | 1 +
 3 files changed, 15 insertions(+)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index f0c9009..b1d1cc8 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -460,6 +460,11 @@ void tpm_gen_interrupt(struct tpm_chip *chip)
 	struct	tpm_cmd_t tpm_cmd;
 	ssize_t rc;
 
+	if (chip->tpm2) {
+		tpm2_gen_interrupt(chip);
+		return;
+	}
+
 	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);
diff --git a/drivers/char/tpm/tpm2-commands.c b/drivers/char/tpm/tpm2-commands.c
index c36f7fb..a21dfd5 100644
--- a/drivers/char/tpm/tpm2-commands.c
+++ b/drivers/char/tpm/tpm2-commands.c
@@ -411,3 +411,12 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
 
 	return total ? total : -EIO;
 }
+
+void tpm2_gen_interrupt(struct tpm_chip *chip)
+{
+	u32 value;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(chip->dev, TPM2_CAP_TPM_PROPERTIES, &value,
+			     "attempting to determine the timeouts");
+}
diff --git a/drivers/char/tpm/tpm2.h b/drivers/char/tpm/tpm2.h
index 73a1c35..98b8b80 100644
--- a/drivers/char/tpm/tpm2.h
+++ b/drivers/char/tpm/tpm2.h
@@ -65,5 +65,6 @@ int tpm2_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
 int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
 int tpm2_do_selftest(struct tpm_chip *chip);
 int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
+void tpm2_gen_interrupt(struct tpm_chip *chip);
 
 #endif /* __DRIVERS_CHAR_TPM2_H__ */
-- 
2.1.0

[PATCH v1 10/12] tpm: TPM 2.0 FIFO Interface

[Jarkko Sakkinen]

From: Will Arthur <will.c.arthur@intel.com>

Detect TPM 2.0 by using the extended STS (STS3) register. For TPM 2.0,
instead of calling tpm_get_timeouts(), assign duration and timeout
values defined in the TPM 2.0 PTP specification.

[jarkko.sakkinen: Added durations and timeouts handling. Rewrote
 the commit message.]

Signed-off-by: Will Arthur <will.c.arthur@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c |  3 +++
 drivers/char/tpm/tpm2.h          |  7 ++++++
 drivers/char/tpm/tpm_tis.c       | 53 ++++++++++++++++++++++++++++++++--------
 3 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index b1d1cc8..7a9c096 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -506,6 +506,9 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 	struct duration_t *duration_cap;
 	ssize_t rc;
 
+	if (chip->tpm2)
+		return 0;
+
 	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);
diff --git a/drivers/char/tpm/tpm2.h b/drivers/char/tpm/tpm2.h
index 98b8b80..ba7c053 100644
--- a/drivers/char/tpm/tpm2.h
+++ b/drivers/char/tpm/tpm2.h
@@ -19,6 +19,13 @@
 enum tpm2_const {
 	TPM2_PLATFORM_PCR = 24,
 	TPM2_PCR_SELECT_MIN = ((TPM2_PLATFORM_PCR + 7) / 8),
+	TPM2_TIMEOUT_A = 750 * 1000,
+	TPM2_TIMEOUT_B = 2000 * 1000,
+	TPM2_TIMEOUT_C = 200 * 1000,
+	TPM2_TIMEOUT_D = 30 * 1000,
+	TPM2_DURATION_SHORT = 20 * 1000,
+	TPM2_DURATION_MEDIUM = 750 * 1000,
+	TPM2_DURATION_LONG = 2000 * 1000,
 };
 
 enum tpm2_structures {
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 2c46734..d33e60f 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -44,6 +44,12 @@ enum tis_status {
 	TPM_STS_DATA_EXPECT = 0x08,
 };
 
+enum tis_status3 {
+	TPM_STS3_TPM2_FAM = 0x04,
+	TPM_STS3_RST_EST = 0x02,
+	TPM_STS3_CANCEL = 0x01,
+};
+
 enum tis_int_flags {
 	TPM_GLOBAL_INT_ENABLE = 0x80000000,
 	TPM_INTF_BURST_COUNT_STATIC = 0x100,
@@ -70,6 +76,7 @@ enum tis_defaults {
 #define	TPM_INT_STATUS(l)		(0x0010 | ((l) << 12))
 #define	TPM_INTF_CAPS(l)		(0x0014 | ((l) << 12))
 #define	TPM_STS(l)			(0x0018 | ((l) << 12))
+#define	TPM_STS3(l)			(0x001b | ((l) << 12))
 #define	TPM_DATA_FIFO(l)		(0x0024 | ((l) << 12))
 
 #define	TPM_DID_VID(l)			(0x0F00 | ((l) << 12))
@@ -534,21 +541,48 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
 	u32 vendor, intfcaps, intmask;
 	int rc, i, irq_s, irq_e, probe;
 	struct tpm_chip *chip;
+	u8 sts3;
 
-	if (!(chip = tpm_register_hardware(dev, &tpm_tis)))
+	chip = tpm_chip_alloc(dev, &tpm_tis);
+	if (!chip)
 		return -ENODEV;
 
 	chip->vendor.iobase = ioremap(start, len);
 	if (!chip->vendor.iobase) {
-		rc = -EIO;
-		goto out_err;
+		put_device(chip->dev);
+		kfree(chip);
+		return -EIO;
+	}
+
+	sts3 = ioread8(chip->vendor.iobase + TPM_STS3(1));
+	chip->tpm2 = (sts3 & (TPM_STS3_TPM2_FAM)) == TPM_STS3_TPM2_FAM;
+
+	rc = tpm_chip_register(chip);
+	if (rc) {
+		iounmap(chip->vendor.iobase);
+		put_device(chip->dev);
+		kfree(chip);
+		return -ENODEV;
 	}
 
 	/* Default timeouts */
-	chip->vendor.timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
-	chip->vendor.timeout_b = msecs_to_jiffies(TIS_LONG_TIMEOUT);
-	chip->vendor.timeout_c = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
-	chip->vendor.timeout_d = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
+	if (chip->tpm2) {
+		chip->vendor.timeout_a = usecs_to_jiffies(TPM2_TIMEOUT_A);
+		chip->vendor.timeout_b = usecs_to_jiffies(TPM2_TIMEOUT_B);
+		chip->vendor.timeout_c = usecs_to_jiffies(TPM2_TIMEOUT_C);
+		chip->vendor.timeout_d = usecs_to_jiffies(TPM2_TIMEOUT_D);
+		chip->vendor.duration[TPM_SHORT] =
+			usecs_to_jiffies(TPM2_DURATION_SHORT);
+		chip->vendor.duration[TPM_MEDIUM] =
+			usecs_to_jiffies(TPM2_DURATION_MEDIUM);
+		chip->vendor.duration[TPM_LONG] =
+			usecs_to_jiffies(TPM2_DURATION_LONG);
+	} else {
+		chip->vendor.timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
+		chip->vendor.timeout_b = msecs_to_jiffies(TIS_LONG_TIMEOUT);
+		chip->vendor.timeout_c = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
+		chip->vendor.timeout_d = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
+	}
 
 	if (wait_startup(chip, 0) != 0) {
 		rc = -ENODEV;
@@ -563,8 +597,8 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
 	vendor = ioread32(chip->vendor.iobase + TPM_DID_VID(0));
 	chip->vendor.manufacturer_id = vendor;
 
-	dev_info(dev,
-		 "1.2 TPM (device-id 0x%X, rev-id %d)\n",
+	dev_info(dev, "%s TPM (device-id 0x%X, rev-id %d)\n",
+		 chip->tpm2 ? "2.0" : "1.2",
 		 vendor >> 16, ioread8(chip->vendor.iobase + TPM_RID(0)));
 
 	if (!itpm) {
@@ -724,7 +758,6 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
 	list_add(&chip->vendor.list, &tis_chips);
 	mutex_unlock(&tis_lock);
 
-
 	return 0;
 out_err:
 	if (chip->vendor.iobase)
-- 
2.1.0

Re: [PATCH v1 10/12] tpm: TPM 2.0 FIFO Interface

[Jason Gunthorpe]

On Wed, Sep 24, 2014 at 12:06:00PM +0300, Jarkko Sakkinen wrote:
  
> -	if (!(chip = tpm_register_hardware(dev, &tpm_tis)))
> +	chip = tpm_chip_alloc(dev, &tpm_tis);
> +	if (!chip)
>  		return -ENODEV;

Please put this in a separate patch, don't co-mingle it with TPM2
support. If drivers are going to be converted, then I want to see the
new API used properly and the driver itself to be a *correct* example
of using the new API.

So you have to purge the tis_chips, fix the missing removal functions,
call unregister, fix the force probe path, etc.

>  	chip->vendor.iobase = ioremap(start, len);

Since this changes the ordering, can we devm this ioremap?

Jason

Re: [PATCH v1 10/12] tpm: TPM 2.0 FIFO Interface

[Jarkko Sakkinen]

On Wed, Sep 24, 2014 at 10:59:21AM -0600, Jason Gunthorpe wrote:
> On Wed, Sep 24, 2014 at 12:06:00PM +0300, Jarkko Sakkinen wrote:
>   
> > -	if (!(chip = tpm_register_hardware(dev, &tpm_tis)))
> > +	chip = tpm_chip_alloc(dev, &tpm_tis);
> > +	if (!chip)
> >  		return -ENODEV;
> 
> Please put this in a separate patch, don't co-mingle it with TPM2
> support. If drivers are going to be converted, then I want to see the
> new API used properly and the driver itself to be a *correct* example
> of using the new API.

Agreed.

> So you have to purge the tis_chips, fix the missing removal functions,
> call unregister, fix the force probe path, etc.
> 
> >  	chip->vendor.iobase = ioremap(start, len);
> 
> Since this changes the ordering, can we devm this ioremap?

Hmm... I was going to do that but forgot it. I'll add this to my
backlog.

> Jason
/Jarkko

[PATCH v1 11/12] tpm: Driver for TPM 2.0 CRB Interface

[Jarkko Sakkinen]

tpm_crb is a driver for TPM 2.0 CRB Interface as defined in

- http://www.trustedcomputinggroup.org/resources/tpm_20_mobile_command_response_buffer_interface_specification
- http://www.trustedcomputinggroup.org/resources/server_acpi_specification

Tested with TPM2 PTT included in 4th generation Intel processors.

Notes:

- PTT reports using only ACPI start as start method but seems to
  require CRB with ACPI start.
- cmdReady does not seem to work with PTT.
- Does not have yet interrupt based send/receive because I don't
  have hardware for that.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/Kconfig   |   9 ++
 drivers/char/tpm/Makefile  |   1 +
 drivers/char/tpm/tpm_crb.c | 332 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 342 insertions(+)
 create mode 100644 drivers/char/tpm/tpm_crb.c

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index c54cac3..10c9419 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -122,4 +122,13 @@ config TCG_XEN
 	  To compile this driver as a module, choose M here; the module
 	  will be called xen-tpmfront.
 
+config TCG_CRB
+	tristate "TPM 2.0 CRB Interface"
+	depends on X86 && ACPI
+	---help---
+	  If you have a TPM security chip that is compliant with the
+	  TCG CRB 2.0 TPM specification say Yes and it will be accessible
+	  from within Linux.  To compile this driver as a module, choose
+	  M here; the module will be called tpm_crb.
+
 endif # TCG_TPM
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 5ff5f3d..253e823 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -22,3 +22,4 @@ obj-$(CONFIG_TCG_INFINEON) += tpm_infineon.o
 obj-$(CONFIG_TCG_IBMVTPM) += tpm_ibmvtpm.o
 obj-$(CONFIG_TCG_ST33_I2C) += tpm_i2c_stm_st33.o
 obj-$(CONFIG_TCG_XEN) += xen-tpmfront.o
+obj-$(CONFIG_TCG_CRB) += tpm_crb.o
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
new file mode 100644
index 0000000..db29bd2
--- /dev/null
+++ b/drivers/char/tpm/tpm_crb.c
@@ -0,0 +1,332 @@
+/*
+ * Copyright (C) 2014 Intel Corporation
+ *
+ * Authors:
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * This device driver implements the TPM interface as defined in
+ * the TCG CRB 2.0 TPM specification.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include <linux/acpi.h>
+#include <linux/highmem.h>
+#include <linux/rculist.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include "tpm.h"
+
+#define ACPI_SIG_TPM2 "TPM2"
+
+static const u8 CRB_ACPI_START_UUID[] = {
+	/* 0000 */ 0xAB, 0x6C, 0xBF, 0x6B, 0x63, 0x54, 0x14, 0x47,
+	/* 0008 */ 0xB7, 0xCD, 0xF0, 0x20, 0x3C, 0x03, 0x68, 0xD4
+};
+
+enum crb_defaults {
+	CRB_SHORT_TIMEOUT = 1000,
+	CRB_LONG_TIMEOUT = 2000,
+	CRB_ACPI_START_REVISION_ID = 1,
+	CRB_ACPI_START_INDEX = 1,
+};
+
+enum crb_start_method {
+	CRB_SM_ACPI_START = 2,
+	CRB_SM_CRB = 7,
+	CRB_SM_CRB_WITH_ACPI_START = 8,
+};
+
+struct acpi_tpm2 {
+	struct acpi_table_header hdr;
+	u16 platform_class;
+	u16 reserved;
+	u64 control_area_pa;
+	u32 start_method;
+};
+
+enum crb_ca_request {
+	CRB_CA_REQ_GO_IDLE	= 0x01,
+	CRB_CA_REQ_CMD_READY	= 0x02,
+};
+
+enum crb_ca_status {
+	CRB_CA_STS_ERROR	= 0x01,
+	CRB_CA_STS_TPM_IDLE	= 0x02,
+};
+
+struct crb_control_area {
+	u32 req;
+	u32 sts;
+	u32 cancel;
+	u32 start;
+	u32 int_enable;
+	u32 int_sts;
+	u32 cmd_size;
+	u64 cmd_pa;
+	u32 rsp_size;
+	u64 rsp_pa;
+} __packed;
+
+enum crb_status {
+	CRB_STS_COMPLETE	= 0x01,
+};
+
+struct crb_priv {
+	struct {
+		unsigned int acpi_start : 1;
+		unsigned int crb_start	: 1;
+		unsigned int reserved	: 30;
+	} opt;
+
+	struct crb_control_area *cca;
+	unsigned long cca_pa;
+	acpi_handle dev_handle;
+};
+
+#ifdef CONFIG_PM_SLEEP
+int crb_suspend(struct device *dev)
+{
+	return 0;
+}
+
+static int crb_resume(struct device *dev)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+
+	(void) tpm_do_selftest(chip);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(crb_pm, crb_suspend, crb_resume);
+
+static u8 crb_status(struct tpm_chip *chip)
+{
+	struct crb_priv *priv = chip->vendor.priv;
+	u8 sts = 0;
+
+	if ((priv->cca->start & 1) != 1)
+		sts |= CRB_STS_COMPLETE;
+
+	return sts;
+}
+
+static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count)
+{
+	struct crb_priv *priv = chip->vendor.priv;
+	struct crb_control_area *cca;
+	unsigned int expected;
+	unsigned long offset;
+	u8 *resp;
+
+	cca = priv->cca;
+	if (cca->sts & CRB_CA_STS_ERROR)
+		return -EIO;
+
+	offset = cca->rsp_pa - priv->cca_pa;
+	resp = (u8 *) ((unsigned long) cca + offset);
+	memcpy(buf, resp, 6);
+	expected = be32_to_cpu(*(__be32 *) (buf + 2));
+
+	if (expected > count)
+		return -EIO;
+
+	memcpy(buf + 6, resp + 6, expected - 6);
+
+	return expected;
+}
+
+static int crb_do_acpi_start(struct tpm_chip *chip)
+{
+	struct crb_priv *priv = chip->vendor.priv;
+	union acpi_object *obj;
+	int rc;
+
+	obj = acpi_evaluate_dsm(priv->dev_handle,
+				CRB_ACPI_START_UUID,
+				CRB_ACPI_START_REVISION_ID,
+				CRB_ACPI_START_INDEX,
+				NULL);
+	if (!obj)
+		return -ENXIO;
+	rc = obj->integer.value == 0 ? 0 : -ENXIO;
+	ACPI_FREE(obj);
+	return rc;
+}
+
+static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
+{
+	struct crb_priv *priv = chip->vendor.priv;
+	struct crb_control_area *cca;
+	u8 *cmd;
+	int rc = 0;
+
+	cca = priv->cca;
+
+	if (len > cca->cmd_size) {
+		dev_err(chip->dev,
+			"invalid command count value %x %zx\n",
+			(unsigned int) len,
+			(size_t) cca->cmd_size);
+		return -E2BIG;
+	}
+
+	cmd = (u8 *) ((unsigned long) cca + cca->cmd_pa - priv->cca_pa);
+	memcpy(cmd, buf, len);
+	wmb();
+
+	cca->start = 1;
+	rc = crb_do_acpi_start(chip);
+	return rc;
+}
+
+static void crb_cancel(struct tpm_chip *chip)
+{
+	struct crb_priv *priv = chip->vendor.priv;
+	struct crb_control_area *cca;
+
+	cca = priv->cca;
+	cca->cancel = 1;
+	wmb();
+
+	if (crb_do_acpi_start(chip))
+		dev_err(chip->dev, "ACPI Start failed\n");
+
+	cca->cancel = 0;
+}
+
+static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
+{
+	struct crb_priv *priv = chip->vendor.priv;
+
+	return priv->cca->cancel;
+}
+
+static const struct tpm_class_ops tpm_crb = {
+	.status = crb_status,
+	.recv = crb_recv,
+	.send = crb_send,
+	.cancel = crb_cancel,
+	.req_canceled = crb_req_canceled,
+	.req_complete_mask = CRB_STS_COMPLETE,
+	.req_complete_val = CRB_STS_COMPLETE,
+};
+
+static void crb_release(void *data)
+{
+	struct tpm_chip *chip = (struct tpm_chip *) data;
+	tpm_remove_hardware(chip->dev);
+}
+
+static int crb_acpi_add(struct acpi_device *device)
+{
+	struct tpm_chip *chip;
+	struct acpi_tpm2 *buf;
+	struct crb_priv *priv;
+	struct device *dev = &device->dev;
+	acpi_status status;
+	u32 sm;
+	int rc;
+
+	chip = tpm_chip_alloc(dev, &tpm_crb);
+	if (!chip)
+		return -ENODEV;
+
+	chip->tpm2 = true;
+
+	rc = tpm_chip_register(chip);
+	if (rc) {
+		put_device(chip->dev);
+		kfree(chip);
+		return -ENODEV;
+	}
+
+	status = acpi_get_table(ACPI_SIG_TPM2, 1,
+				(struct acpi_table_header **) &buf);
+	if (ACPI_FAILURE(status)) {
+		dev_err(dev, "could not get TPM2 ACPI table\n");
+		rc = -ENODEV;
+		goto out_err;
+	}
+
+	priv = (struct crb_priv *) devm_kzalloc(dev, sizeof(struct crb_priv),
+						GFP_KERNEL);
+	if (!priv) {
+		rc = -ENODEV;
+		goto out_err;
+	}
+
+	sm = buf->start_method;
+
+	if (sm == CRB_SM_CRB || sm == CRB_SM_CRB_WITH_ACPI_START)
+		priv->opt.crb_start = 1;
+
+	if (sm == CRB_SM_ACPI_START || sm == CRB_SM_CRB_WITH_ACPI_START)
+		priv->opt.acpi_start = 1;
+
+	priv->dev_handle = device->handle;
+	priv->cca_pa = buf->control_area_pa;
+	priv->cca = (struct crb_control_area *)
+		devm_ioremap_nocache(dev, buf->control_area_pa, 0x1000);
+	if (!priv->cca) {
+		rc = -ENODEV;
+		goto out_err;
+	}
+
+	chip->vendor.priv = priv;
+
+	/* Default timeouts and durations */
+	chip->vendor.timeout_a = msecs_to_jiffies(CRB_SHORT_TIMEOUT);
+	chip->vendor.timeout_b = msecs_to_jiffies(CRB_LONG_TIMEOUT);
+	chip->vendor.timeout_c = msecs_to_jiffies(CRB_SHORT_TIMEOUT);
+	chip->vendor.timeout_d = msecs_to_jiffies(CRB_SHORT_TIMEOUT);
+
+	chip->vendor.duration[TPM_SHORT] = usecs_to_jiffies(TPM2_DURATION_SHORT);
+	chip->vendor.duration[TPM_MEDIUM] = usecs_to_jiffies(TPM2_DURATION_MEDIUM);
+	chip->vendor.duration[TPM_LONG] = usecs_to_jiffies(TPM2_DURATION_LONG);
+
+	rc = tpm_do_selftest(chip);
+	if (rc) {
+		rc = -ENODEV;
+		goto out_err;
+	}
+
+	rc = devm_add_action(dev, crb_release, chip);
+	if (rc)
+		goto out_err;
+
+	return 0;
+out_err:
+	tpm_remove_hardware(chip->dev);
+	return rc;
+}
+
+static struct acpi_device_id crb_device_ids[] = {
+	{"MSFT0101", 0},
+	{"", 0},
+};
+MODULE_DEVICE_TABLE(acpi, crb_device_ids);
+
+static struct acpi_driver crb_acpi_driver = {
+	.name = "tpm_crb",
+	.ids = crb_device_ids,
+	.ops = {
+		.add = crb_acpi_add,
+	},
+	.drv = {
+		.pm = &crb_pm,
+	},
+};
+
+module_acpi_driver(crb_acpi_driver);
+MODULE_AUTHOR("Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>");
+MODULE_DESCRIPTION("TPM2 Driver");
+MODULE_VERSION("0.1");
+MODULE_LICENSE("GPL");
-- 
2.1.0

Re: [PATCH v1 11/12] tpm: Driver for TPM 2.0 CRB Interface

[Jason Gunthorpe]

On Wed, Sep 24, 2014 at 12:06:01PM +0300, Jarkko Sakkinen wrote:

> +	offset = cca->rsp_pa - priv->cca_pa;
> +	resp = (u8 *) ((unsigned long) cca + offset);
> +	memcpy(buf, resp, 6);
> +	expected = be32_to_cpu(*(__be32 *) (buf + 2));

be32_to_cpup?

> +static void crb_release(void *data)
> +{
> +	struct tpm_chip *chip = (struct tpm_chip *) data;
> +	tpm_remove_hardware(chip->dev);
> +}

Please use a proper remove function on the device driver, not a devm
function like this. 'tpm_remove_hardware' is the wrong name for a new
API, it must be 'tpm_chip_unregister' (ie the undo of 'tpm_chip_register')

> +static int crb_acpi_add(struct acpi_device *device)
> +{
> +	struct tpm_chip *chip;
> +	struct acpi_tpm2 *buf;
> +	struct crb_priv *priv;
> +	struct device *dev = &device->dev;
> +	acpi_status status;
> +	u32 sm;
> +	int rc;
> +
> +	chip = tpm_chip_alloc(dev, &tpm_crb);
> +	if (!chip)
> +		return -ENODEV;

Lets use ERRPTR here

> +	chip->tpm2 = true;
> +
> +	rc = tpm_chip_register(chip);

This is in the wrong place, it needs to be the last call in the probe
function - the driver must be fully operational when register is
called, that is one of the bugs the new interface must be fixing.

> +	rc = tpm_do_selftest(chip);
> +	if (rc) {
> +		rc = -ENODEV;
> +		goto out_err;
> +	}

The common TPM command startup sequence should be in
tpm_chip_register(), so move this into there.

> +	rc = devm_add_action(dev, crb_release, chip);
> +	if (rc)
> +		goto out_err;
> +
> +	return 0;
> +out_err:
> +	tpm_remove_hardware(chip->dev);
> +	return rc;
> +}
> +
> +static struct acpi_device_id crb_device_ids[] = {

const? Not sure

Jason

Re: [PATCH v1 11/12] tpm: Driver for TPM 2.0 CRB Interface

[Jarkko Sakkinen]

Thanks for reviewing this. I'll fix these issues, not much to add.
Thank you.

/Jarkko

On Wed, Sep 24, 2014 at 11:05:51AM -0600, Jason Gunthorpe wrote:
> On Wed, Sep 24, 2014 at 12:06:01PM +0300, Jarkko Sakkinen wrote:
> 
> > +	offset = cca->rsp_pa - priv->cca_pa;
> > +	resp = (u8 *) ((unsigned long) cca + offset);
> > +	memcpy(buf, resp, 6);
> > +	expected = be32_to_cpu(*(__be32 *) (buf + 2));
> 
> be32_to_cpup?
> 
> > +static void crb_release(void *data)
> > +{
> > +	struct tpm_chip *chip = (struct tpm_chip *) data;
> > +	tpm_remove_hardware(chip->dev);
> > +}
> 
> Please use a proper remove function on the device driver, not a devm
> function like this. 'tpm_remove_hardware' is the wrong name for a new
> API, it must be 'tpm_chip_unregister' (ie the undo of 'tpm_chip_register')
> 
> > +static int crb_acpi_add(struct acpi_device *device)
> > +{
> > +	struct tpm_chip *chip;
> > +	struct acpi_tpm2 *buf;
> > +	struct crb_priv *priv;
> > +	struct device *dev = &device->dev;
> > +	acpi_status status;
> > +	u32 sm;
> > +	int rc;
> > +
> > +	chip = tpm_chip_alloc(dev, &tpm_crb);
> > +	if (!chip)
> > +		return -ENODEV;
> 
> Lets use ERRPTR here
> 
> > +	chip->tpm2 = true;
> > +
> > +	rc = tpm_chip_register(chip);
> 
> This is in the wrong place, it needs to be the last call in the probe
> function - the driver must be fully operational when register is
> called, that is one of the bugs the new interface must be fixing.
> 
> > +	rc = tpm_do_selftest(chip);
> > +	if (rc) {
> > +		rc = -ENODEV;
> > +		goto out_err;
> > +	}
> 
> The common TPM command startup sequence should be in
> tpm_chip_register(), so move this into there.
> 
> > +	rc = devm_add_action(dev, crb_release, chip);
> > +	if (rc)
> > +		goto out_err;
> > +
> > +	return 0;
> > +out_err:
> > +	tpm_remove_hardware(chip->dev);
> > +	return rc;
> > +}
> > +
> > +static struct acpi_device_id crb_device_ids[] = {
> 
> const? Not sure
> 
> Jason

Re: [PATCH v1 11/12] tpm: Driver for TPM 2.0 CRB Interface

[Jarkko Sakkinen]

On Wed, Sep 24, 2014 at 11:05:51AM -0600, Jason Gunthorpe wrote:
> On Wed, Sep 24, 2014 at 12:06:01PM +0300, Jarkko Sakkinen wrote:
> 
> > +	offset = cca->rsp_pa - priv->cca_pa;
> > +	resp = (u8 *) ((unsigned long) cca + offset);
> > +	memcpy(buf, resp, 6);
> > +	expected = be32_to_cpu(*(__be32 *) (buf + 2));
> 
> be32_to_cpup?
> 
> > +static void crb_release(void *data)
> > +{
> > +	struct tpm_chip *chip = (struct tpm_chip *) data;
> > +	tpm_remove_hardware(chip->dev);
> > +}
> 
> Please use a proper remove function on the device driver, not a devm
> function like this. 'tpm_remove_hardware' is the wrong name for a new
> API, it must be 'tpm_chip_unregister' (ie the undo of 'tpm_chip_register')
> 
> > +static int crb_acpi_add(struct acpi_device *device)
> > +{
> > +	struct tpm_chip *chip;
> > +	struct acpi_tpm2 *buf;
> > +	struct crb_priv *priv;
> > +	struct device *dev = &device->dev;
> > +	acpi_status status;
> > +	u32 sm;
> > +	int rc;
> > +
> > +	chip = tpm_chip_alloc(dev, &tpm_crb);
> > +	if (!chip)
> > +		return -ENODEV;
> 
> Lets use ERRPTR here
> 
> > +	chip->tpm2 = true;
> > +
> > +	rc = tpm_chip_register(chip);
> 
> This is in the wrong place, it needs to be the last call in the probe
> function - the driver must be fully operational when register is
> called, that is one of the bugs the new interface must be fixing.
> 
> > +	rc = tpm_do_selftest(chip);
> > +	if (rc) {
> > +		rc = -ENODEV;
> > +		goto out_err;
> > +	}
> 
> The common TPM command startup sequence should be in
> tpm_chip_register(), so move this into there.
> 
> > +	rc = devm_add_action(dev, crb_release, chip);
> > +	if (rc)
> > +		goto out_err;
> > +
> > +	return 0;
> > +out_err:
> > +	tpm_remove_hardware(chip->dev);
> > +	return rc;
> > +}
> > +
> > +static struct acpi_device_id crb_device_ids[] = {
> 
> const? Not sure

Cannot be because acpi_bus_register_driver() does not take a const
pointer.

> Jason

/Jarkko

[PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Jarkko Sakkinen]

Added tpm2-sysfs.c that implements sysfs attributes for a TPM2
device.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/Makefile        |   2 +-
 drivers/char/tpm/tpm-chip.c      |  10 +-
 drivers/char/tpm/tpm-interface.c |   1 +
 drivers/char/tpm/tpm2-commands.c |   2 +-
 drivers/char/tpm/tpm2-sysfs.c    | 242 +++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm2.h          |  31 +++++
 6 files changed, 284 insertions(+), 4 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2-sysfs.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 253e823..9e71c43 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the kernel tpm device drivers.
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
-tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-commands.o
+tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-commands.o tpm2-sysfs.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o
 
 ifdef CONFIG_ACPI
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 128942b..91d8213 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -127,7 +127,10 @@ int tpm_chip_register(struct tpm_chip *chip)
 	if (rc)
 		return rc;
 
-	rc = tpm_sysfs_add_device(chip);
+	if (chip->tpm2)
+		rc = tpm2_sysfs_add_device(chip);
+	else
+		rc = tpm_sysfs_add_device(chip);
 	if (rc)
 		goto del_misc;
 
@@ -160,7 +163,10 @@ void tpm_chip_unregister(struct tpm_chip *chip)
 	synchronize_rcu();
 
 	tpm_dev_del_device(chip);
-	tpm_sysfs_del_device(chip);
+	if (chip->tpm2)
+		tpm2_sysfs_del_device(chip);
+	else
+		tpm_sysfs_del_device(chip);
 	tpm_remove_ppi(&chip->dev->kobj);
 
 	if (!chip->tpm2)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 7a9c096..26195db 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -899,6 +899,7 @@ void tpm_remove_hardware(struct device *dev)
 
 	tpm_chip_unregister(chip);
 
+
 	/* write it this way to be explicit (chip->dev == dev) */
 	put_device(chip->dev);
 }
diff --git a/drivers/char/tpm/tpm2-commands.c b/drivers/char/tpm/tpm2-commands.c
index a21dfd5..6365087 100644
--- a/drivers/char/tpm/tpm2-commands.c
+++ b/drivers/char/tpm/tpm2-commands.c
@@ -195,7 +195,7 @@ ssize_t tpm2_get_tpm_pt(struct device *dev, u32 property_id,  u32* value,
 	cmd.header.in = tpm2_get_tpm_pt_header;
 	cmd.params.tpm2_get_tpm_pt_in.cap_id =
 		cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
-	cmd.params.tpm2_get_tpm_pt_in.property_id = property_id;
+	cmd.params.tpm2_get_tpm_pt_in.property_id = cpu_to_be32(property_id);
 	cmd.params.tpm2_get_tpm_pt_in.property_cnt = cpu_to_be32(1);
 
 	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), desc);
diff --git a/drivers/char/tpm/tpm2-sysfs.c b/drivers/char/tpm/tpm2-sysfs.c
new file mode 100644
index 0000000..a254b2c
--- /dev/null
+++ b/drivers/char/tpm/tpm2-sysfs.c
@@ -0,0 +1,242 @@
+/*
+ * Copyright (C) 2004 IBM Corporation
+ * Authors:
+ * Leendert van Doorn <leendert@watson.ibm.com>
+ * Dave Safford <safford@watson.ibm.com>
+ * Reiner Sailer <sailer@watson.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * Copyright (C) 2013 Obsidian Research Corp
+ * Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
+ *
+ * sysfs filesystem inspection interface to the TPM
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ */
+#include <linux/device.h>
+#include "tpm.h"
+
+static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	u8 digest[TPM_DIGEST_SIZE];
+	ssize_t rc;
+	int i, j;
+	char *str = buf;
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+
+	for (i = 0; i < TPM2_PLATFORM_PCR; i++) {
+		rc = tpm2_pcr_read_dev(chip, i, digest);
+		if (rc)
+			break;
+		str += sprintf(str, "PCR-%02d: ", i);
+		for (j = 0; j < TPM_DIGEST_SIZE; j++)
+			str += sprintf(str, "%02X ", digest[j]);
+		str += sprintf(str, "\n");
+	}
+
+	return str - buf;
+}
+static DEVICE_ATTR_RO(pcrs);
+
+static ssize_t enabled_sh_show(struct device *dev, struct device_attribute *attr,
+		     char *buf)
+{
+	struct tpm2_startup_clear value;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(dev, TPM2_PT_STARTUP_CLEAR, (u32 *) &value,
+			     "could not retrieve STARTUP_CLEAR property");
+	if (rc)
+		return 0;
+
+	rc = sprintf(buf, "%d\n", value.sh_enable);
+	return rc;
+}
+static DEVICE_ATTR_RO(enabled_sh);
+
+static ssize_t enabled_eh_show(struct device *dev, struct device_attribute *attr,
+		     char *buf)
+{
+	struct tpm2_startup_clear value;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(dev, TPM2_PT_STARTUP_CLEAR, (u32 *) &value,
+			     "could not retrieve STARTUP_CLEAR property");
+	if (rc)
+		return 0;
+
+	rc = sprintf(buf, "%d\n", value.eh_enable);
+	return rc;
+}
+static DEVICE_ATTR_RO(enabled_eh);
+
+static ssize_t owned_sh_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	struct tpm2_permanent value;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(dev, TPM2_PT_PERMANENT, (u32 *) &value,
+			     "could not retrieve PERMANENT property");
+	if (rc)
+		return 0;
+
+	rc = sprintf(buf, "%d\n", value.owner_auth_set);
+	return rc;
+}
+static DEVICE_ATTR_RO(owned_sh);
+
+static ssize_t owned_eh_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	struct tpm2_permanent value;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(dev, TPM2_PT_PERMANENT, (u32 *) &value,
+			     "could not retrieve PERMANENT property");
+	if (rc)
+		return 0;
+
+	rc = sprintf(buf, "%d\n", value.endorsement_auth_set);
+	return rc;
+}
+static DEVICE_ATTR_RO(owned_eh);
+
+static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	u32 value1;
+	u32 value2;
+	ssize_t rc;
+	char *str = buf;
+
+	rc = tpm2_get_tpm_pt(dev, TPM2_PT_MANUFACTURER, (u32 *) &value1,
+			     "could not retrieve MANUFACTURER property");
+	if (rc)
+		return 0;
+
+	str += sprintf(str, "Manufacturer: 0x%08x\n", be32_to_cpu(value1));
+	str += sprintf(str, "TCG version: 2.0\n");
+
+	rc = tpm2_get_tpm_pt(dev, TPM2_PT_FIRMWARE_VERSION_1, (u32 *) &value1,
+			     "could not retrieve FIRMWARE_VERSION_1 property");
+	if (rc)
+		return 0;
+
+	rc = tpm2_get_tpm_pt(dev, TPM2_PT_FIRMWARE_VERSION_2, (u32 *) &value2,
+			     "could not retrieve FIRMWARE_VERSION_2 property");
+	if (rc)
+		return 0;
+
+	str += sprintf(str, "Firmware version: 0x%08x 0x%08x\n", value1, value2);
+
+	return str - buf;
+}
+static DEVICE_ATTR_RO(caps);
+
+static ssize_t cancel_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+	if (chip == NULL)
+		return 0;
+
+	chip->ops->cancel(chip);
+	return count;
+}
+static DEVICE_ATTR_WO(cancel);
+
+static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+
+	if (chip->vendor.duration[TPM_LONG] == 0)
+		return 0;
+
+	return sprintf(buf, "%d %d %d [%s]\n",
+		       jiffies_to_usecs(chip->vendor.duration[TPM_SHORT]),
+		       jiffies_to_usecs(chip->vendor.duration[TPM_MEDIUM]),
+		       jiffies_to_usecs(chip->vendor.duration[TPM_LONG]),
+		       chip->vendor.duration_adjusted
+		       ? "adjusted" : "original");
+}
+static DEVICE_ATTR_RO(durations);
+
+static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d %d %d %d [%s]\n",
+		       jiffies_to_usecs(chip->vendor.timeout_a),
+		       jiffies_to_usecs(chip->vendor.timeout_b),
+		       jiffies_to_usecs(chip->vendor.timeout_c),
+		       jiffies_to_usecs(chip->vendor.timeout_d),
+		       chip->vendor.timeout_adjusted
+		       ? "adjusted" : "original");
+}
+static DEVICE_ATTR_RO(timeouts);
+
+static ssize_t version_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	char *str = buf;
+
+	str += sprintf(str, "2.0\n");
+
+	return str - buf;
+}
+
+static DEVICE_ATTR_RO(version);
+
+
+static ssize_t tpm2_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", chip->tpm2);
+}
+static DEVICE_ATTR_RO(tpm2);
+
+static struct attribute *tpm_dev_attrs[] = {
+	&dev_attr_pcrs.attr,
+	&dev_attr_enabled_sh.attr,
+	&dev_attr_enabled_eh.attr,
+	&dev_attr_owned_sh.attr,
+	&dev_attr_owned_eh.attr,
+	&dev_attr_caps.attr,
+	&dev_attr_cancel.attr,
+	&dev_attr_durations.attr,
+	&dev_attr_timeouts.attr,
+	&dev_attr_version.attr,
+	&dev_attr_tpm2.attr,
+	NULL,
+};
+
+static const struct attribute_group tpm_dev_group = {
+	.attrs = tpm_dev_attrs,
+};
+
+int tpm2_sysfs_add_device(struct tpm_chip *chip)
+{
+	int err;
+	err = sysfs_create_group(&chip->dev->kobj,
+				 &tpm_dev_group);
+
+	if (err)
+		dev_err(chip->dev,
+			"failed to create sysfs attributes, %d\n", err);
+	return err;
+}
+
+void tpm2_sysfs_del_device(struct tpm_chip *chip)
+{
+	sysfs_remove_group(&chip->dev->kobj, &tpm_dev_group);
+}
diff --git a/drivers/char/tpm/tpm2.h b/drivers/char/tpm/tpm2.h
index ba7c053..7a1502a 100644
--- a/drivers/char/tpm/tpm2.h
+++ b/drivers/char/tpm/tpm2.h
@@ -59,6 +59,34 @@ enum tpm2_capabilities {
 	TPM2_CAP_TPM_PROPERTIES = 6,
 };
 
+enum tpm2_tpm_properties {
+	TPM2_PT_MANUFACTURER		= 0x00000105,
+	TPM2_PT_FIRMWARE_VERSION_1	= 0x00000111,
+	TPM2_PT_FIRMWARE_VERSION_2	= 0x00000111,
+	TPM2_PT_PERMANENT		= 0x00000200,
+	TPM2_PT_STARTUP_CLEAR		= 0x00000201,
+};
+
+struct tpm2_permanent {
+	unsigned int owner_auth_set		: 1;
+	unsigned int endorsement_auth_set	: 1;
+	unsigned int lockout_auth_set		: 1;
+	unsigned int reserved1			: 5;
+	unsigned int disable_clear		: 1;
+	unsigned int in_lockout			: 1;
+	unsigned int tpm_generated_eps		: 1;
+	unsigned int reserved2			: 21;
+};
+
+struct tpm2_startup_clear {
+	unsigned int ph_enable			: 1;
+	unsigned int sh_enable			: 1;
+	unsigned int eh_enable			: 1;
+	unsigned int ph_enable_nv		: 1;
+	unsigned int reserved			: 27;
+	unsigned int orderly			: 1;
+};
+
 struct tpm_chip;
 
 #define TPM2_CC_FIRST	0x11F
@@ -74,4 +102,7 @@ int tpm2_do_selftest(struct tpm_chip *chip);
 int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
 void tpm2_gen_interrupt(struct tpm_chip *chip);
 
+int tpm2_sysfs_add_device(struct tpm_chip *chip);
+void tpm2_sysfs_del_device(struct tpm_chip *chip);
+
 #endif /* __DRIVERS_CHAR_TPM2_H__ */
-- 
2.1.0

Re: [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Jason Gunthorpe]

On Wed, Sep 24, 2014 at 12:06:02PM +0300, Jarkko Sakkinen wrote:
> Added tpm2-sysfs.c that implements sysfs attributes for a TPM2
> device.

You need to document in Documentation every new sysfs that is added.

The existing ones are not documented :(

> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -899,6 +899,7 @@ void tpm_remove_hardware(struct device *dev)
>  
>  	tpm_chip_unregister(chip);
>  
> +
>  	/* write it this way to be explicit (chip->dev == dev) */
>  	put_device(chip->dev);
>  }

Hunk does not belong in this patch

> diff --git a/drivers/char/tpm/tpm2-commands.c b/drivers/char/tpm/tpm2-commands.c
> index a21dfd5..6365087 100644
> +++ b/drivers/char/tpm/tpm2-commands.c
> @@ -195,7 +195,7 @@ ssize_t tpm2_get_tpm_pt(struct device *dev, u32 property_id,  u32* value,
>  	cmd.header.in = tpm2_get_tpm_pt_header;
>  	cmd.params.tpm2_get_tpm_pt_in.cap_id =
>  		cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
> -	cmd.params.tpm2_get_tpm_pt_in.property_id = property_id;
> +	cmd.params.tpm2_get_tpm_pt_in.property_id = cpu_to_be32(property_id);
>  	cmd.params.tpm2_get_tpm_pt_in.property_cnt = cpu_to_be32(1);

Hunk does not belong in this patch

> +static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
> +			 char *buf)
> +{

The pcrs file never conformed to the sysfs rules, if TPM2 is getting a
whole new file set, I wouldn't mind seeing it not include the
non-conformant ones. What do you think?

> +static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> +			 char *buf)
> +{

Ditto.. The manfacturer number should probably be its own file

> +static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
> +			      char *buf)
> +{
> +	struct tpm_chip *chip = dev_get_drvdata(dev);
> +
> +	if (chip->vendor.duration[TPM_LONG] == 0)
> +		return 0;
> +
> +	return sprintf(buf, "%d %d %d [%s]\n",
> +		       jiffies_to_usecs(chip->vendor.duration[TPM_SHORT]),
> +		       jiffies_to_usecs(chip->vendor.duration[TPM_MEDIUM]),
> +		       jiffies_to_usecs(chip->vendor.duration[TPM_LONG]),
> +		       chip->vendor.duration_adjusted
> +		       ? "adjusted" : "original");
> +}
> +static DEVICE_ATTR_RO(durations);

Seem useless since the durations are constant, drop it?

> +static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
> +			     char *buf)
> +{
> +	struct tpm_chip *chip = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%d %d %d %d [%s]\n",
> +		       jiffies_to_usecs(chip->vendor.timeout_a),
> +		       jiffies_to_usecs(chip->vendor.timeout_b),
> +		       jiffies_to_usecs(chip->vendor.timeout_c),
> +		       jiffies_to_usecs(chip->vendor.timeout_d),
> +		       chip->vendor.timeout_adjusted
> +		       ? "adjusted" : "original");
> +}
> +static DEVICE_ATTR_RO(timeouts);

Ditto

> +static ssize_t version_show(struct device *dev, struct device_attribute *attr,
> +			   char *buf)
> +{
> +	char *str = buf;
> +
> +	str += sprintf(str, "2.0\n");
> +
> +	return str - buf;
> +}
> +
> +static DEVICE_ATTR_RO(version);
> +
> +
> +static ssize_t tpm2_show(struct device *dev, struct device_attribute *attr,
> +			 char *buf)
> +{
> +	struct tpm_chip *chip = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%d\n", chip->tpm2);
> +}
> +static DEVICE_ATTR_RO(tpm2);

Two things for the same report? Drop one?

Also, I think some thought is needed for the char interface - some
kind of ioctl to enter TPM2 mode and EINVAL access until that is done?

Finally, this is in the wrong place in sysfs, I suspect it should be
attached to the char device node, not the platform device node? We
should at least investigate this...

> +struct tpm2_permanent {
> +	unsigned int owner_auth_set		: 1;
> +	unsigned int endorsement_auth_set	: 1;
> +	unsigned int lockout_auth_set		: 1;
> +	unsigned int reserved1			: 5;
> +	unsigned int disable_clear		: 1;
> +	unsigned int in_lockout			: 1;
> +	unsigned int tpm_generated_eps		: 1;
> +	unsigned int reserved2			: 21;
> +};
> +
> +struct tpm2_startup_clear {
> +	unsigned int ph_enable			: 1;
> +	unsigned int sh_enable			: 1;
> +	unsigned int eh_enable			: 1;
> +	unsigned int ph_enable_nv		: 1;
> +	unsigned int reserved			: 27;
> +	unsigned int orderly			: 1;
> +};

This idea is not portable, you cannot use bitfields to index bits in a
word like this. Please use constants defined with BIT(xx)

Next posting can you include a github link? Thanks

Jason

Re: [tpmdd-devel] [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Stefan Berger]

On 09/24/2014 01:13 PM, Jason Gunthorpe wrote:
> On Wed, Sep 24, 2014 at 12:06:02PM +0300, Jarkko Sakkinen wrote:
>> +static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
>> +			      char *buf)
>> +{
>> +	struct tpm_chip *chip = dev_get_drvdata(dev);
>> +
>> +	if (chip->vendor.duration[TPM_LONG] == 0)
>> +		return 0;
>> +
>> +	return sprintf(buf, "%d %d %d [%s]\n",
>> +		       jiffies_to_usecs(chip->vendor.duration[TPM_SHORT]),
>> +		       jiffies_to_usecs(chip->vendor.duration[TPM_MEDIUM]),
>> +		       jiffies_to_usecs(chip->vendor.duration[TPM_LONG]),
>> +		       chip->vendor.duration_adjusted
>> +		       ? "adjusted" : "original");
>> +}
>> +static DEVICE_ATTR_RO(durations);
> Seem useless since the durations are constant, drop it?

We show them for TPM 1.2 as well, so I'd keep them fo TPM2.

>
>> +static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
>> +			     char *buf)
>> +{
>> +	struct tpm_chip *chip = dev_get_drvdata(dev);
>> +
>> +	return sprintf(buf, "%d %d %d %d [%s]\n",
>> +		       jiffies_to_usecs(chip->vendor.timeout_a),
>> +		       jiffies_to_usecs(chip->vendor.timeout_b),
>> +		       jiffies_to_usecs(chip->vendor.timeout_c),
>> +		       jiffies_to_usecs(chip->vendor.timeout_d),
>> +		       chip->vendor.timeout_adjusted
>> +		       ? "adjusted" : "original");
>> +}
>> +static DEVICE_ATTR_RO(timeouts);

With all the problems we had with TPM 1.2 TPM's wrong timeouts and 
showing them in sysfs, why not show them for TPM2 as well?

Re: [tpmdd-devel] [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Jason Gunthorpe]

On Wed, Sep 24, 2014 at 01:34:11PM -0400, Stefan Berger wrote:
> On 09/24/2014 01:13 PM, Jason Gunthorpe wrote:
> >On Wed, Sep 24, 2014 at 12:06:02PM +0300, Jarkko Sakkinen wrote:
> >>+static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
> >>+			      char *buf)
> >>+{
> >>+	struct tpm_chip *chip = dev_get_drvdata(dev);
> >>+
> >>+	if (chip->vendor.duration[TPM_LONG] == 0)
> >>+		return 0;
> >>+
> >>+	return sprintf(buf, "%d %d %d [%s]\n",
> >>+		       jiffies_to_usecs(chip->vendor.duration[TPM_SHORT]),
> >>+		       jiffies_to_usecs(chip->vendor.duration[TPM_MEDIUM]),
> >>+		       jiffies_to_usecs(chip->vendor.duration[TPM_LONG]),
> >>+		       chip->vendor.duration_adjusted
> >>+		       ? "adjusted" : "original");
> >>+}
> >>+static DEVICE_ATTR_RO(durations);
> >Seem useless since the durations are constant, drop it?
> 
> We show them for TPM 1.2 as well, so I'd keep them fo TPM2.

The durations are constant and hardwired in the driver for TPM2, and
the sysfs file format does not follow the one-value-per-file
rule.

So it doesn't display anything useful. In TPM2 mode all the timeouts
are constant and known, so I'd rather see it go away.

> With all the problems we had with TPM 1.2 TPM's wrong timeouts and
> showing them in sysfs, why not show them for TPM2 as well?

We had problems with devices reporting the wrong timeout from their
cap queries - that isn't possible in TPM2.

Both of these values should live in debugfs anyhow.

It would be nice to start with a fresh set of correct sysfs files for
TPM2, since essentially everything is different about the userspace
interface we can safely make a breaking change here.

Jason

Re: [tpmdd-devel] [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Jarkko Sakkinen]

On Wed, Sep 24, 2014 at 11:59:40AM -0600, Jason Gunthorpe wrote:
> On Wed, Sep 24, 2014 at 01:34:11PM -0400, Stefan Berger wrote:
> > On 09/24/2014 01:13 PM, Jason Gunthorpe wrote:
> > >On Wed, Sep 24, 2014 at 12:06:02PM +0300, Jarkko Sakkinen wrote:
> > >>+static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
> > >>+			      char *buf)
> > >>+{
> > >>+	struct tpm_chip *chip = dev_get_drvdata(dev);
> > >>+
> > >>+	if (chip->vendor.duration[TPM_LONG] == 0)
> > >>+		return 0;
> > >>+
> > >>+	return sprintf(buf, "%d %d %d [%s]\n",
> > >>+		       jiffies_to_usecs(chip->vendor.duration[TPM_SHORT]),
> > >>+		       jiffies_to_usecs(chip->vendor.duration[TPM_MEDIUM]),
> > >>+		       jiffies_to_usecs(chip->vendor.duration[TPM_LONG]),
> > >>+		       chip->vendor.duration_adjusted
> > >>+		       ? "adjusted" : "original");
> > >>+}
> > >>+static DEVICE_ATTR_RO(durations);
> > >Seem useless since the durations are constant, drop it?
> > 
> > We show them for TPM 1.2 as well, so I'd keep them fo TPM2.
> 
> The durations are constant and hardwired in the driver for TPM2, and
> the sysfs file format does not follow the one-value-per-file
> rule.
> 
> So it doesn't display anything useful. In TPM2 mode all the timeouts
> are constant and known, so I'd rather see it go away.
> 
> > With all the problems we had with TPM 1.2 TPM's wrong timeouts and
> > showing them in sysfs, why not show them for TPM2 as well?
> 
> We had problems with devices reporting the wrong timeout from their
> cap queries - that isn't possible in TPM2.
> 
> Both of these values should live in debugfs anyhow.
> 
> It would be nice to start with a fresh set of correct sysfs files for
> TPM2, since essentially everything is different about the userspace
> interface we can safely make a breaking change here.

I'll drop timeouts and durations. It's easy to add them later if they
are needed.

> Jason

/Jarkko

Re: [tpmdd-devel] [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Peter Hüwe]

Am Mittwoch, 24. September 2014, 19:59:40 schrieb Jason Gunthorpe:
> On Wed, Sep 24, 2014 at 01:34:11PM -0400, Stefan Berger wrote:
> > On 09/24/2014 01:13 PM, Jason Gunthorpe wrote:
> > >On Wed, Sep 24, 2014 at 12:06:02PM +0300, Jarkko Sakkinen wrote:
> > >>+static ssize_t durations_show(struct device *dev, struct
> > >>device_attribute *attr, +			      char *buf)
> > >>+{
> > >>+	struct tpm_chip *chip = dev_get_drvdata(dev);
> > >>+
> > >>+	if (chip->vendor.duration[TPM_LONG] == 0)
> > >>+		return 0;
> > >>+
> > >>+	return sprintf(buf, "%d %d %d [%s]\n",
> > >>+		       jiffies_to_usecs(chip->vendor.duration[TPM_SHORT]),
> > >>+		       jiffies_to_usecs(chip->vendor.duration[TPM_MEDIUM]),
> > >>+		       jiffies_to_usecs(chip->vendor.duration[TPM_LONG]),
> > >>+		       chip->vendor.duration_adjusted
> > >>+		       ? "adjusted" : "original");
> > >>+}
> > >>+static DEVICE_ATTR_RO(durations);
> > >
> > >Seem useless since the durations are constant, drop it?
> > 
> > We show them for TPM 1.2 as well, so I'd keep them fo TPM2.
> 
> The durations are constant and hardwired in the driver for TPM2, and
> the sysfs file format does not follow the one-value-per-file
> rule.
> 
> So it doesn't display anything useful. In TPM2 mode all the timeouts
> are constant and known, so I'd rather see it go away.


If it is more or less a no-op since we have set chip-
>vendor.duration[TPM_SHORT] for other code to work, we can show the values for 
TPM2.0.

However I think we don't need any extra code to show or hide the sysfs files 
for TPM2.0.

Peter

Re: [tpmdd-devel] [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Jason Gunthorpe]

On Wed, Sep 24, 2014 at 10:39:41PM +0200, Peter Hüwe wrote:
 
> If it is more or less a no-op since we have set
> chip->vendor.duration[TPM_SHORT] for other code to work, we can show
> the values for TPM2.0.

It is not a no-op, Jarkko created a whole new file for the tpm2 sysfs
interface, so the two timeout prints are duplicated code from the tpm1
sysfs interface.

Mind you, not sure if we need separate sysfs files ...

Jason

Re: [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Peter Hüwe]

Am Mittwoch, 24. September 2014, 19:13:38 schrieb Jason Gunthorpe:
> On Wed, Sep 24, 2014 at 12:06:02PM +0300, Jarkko Sakkinen wrote:
> > Added tpm2-sysfs.c that implements sysfs attributes for a TPM2
> > device.
> 
> You need to document in Documentation every new sysfs that is added.
> 
> The existing ones are not documented :(

For 1.2 - they are.
Documentation/ABI/stable/sysfs-class-tpm
So we also now what our stable sysfs abi is :)



Peter

Re: [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Jarkko Sakkinen]

On Wed, Sep 24, 2014 at 11:13:38AM -0600, Jason Gunthorpe wrote:
> On Wed, Sep 24, 2014 at 12:06:02PM +0300, Jarkko Sakkinen wrote:
> > Added tpm2-sysfs.c that implements sysfs attributes for a TPM2
> > device.
> 
> You need to document in Documentation every new sysfs that is added.
> 
> The existing ones are not documented :(

This came up in Peters reply (you probably saw that).

https://www.kernel.org/doc/Documentation/ABI/stable/sysfs-class-tpm

> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -899,6 +899,7 @@ void tpm_remove_hardware(struct device *dev)
> >  
> >  	tpm_chip_unregister(chip);
> >  
> > +
> >  	/* write it this way to be explicit (chip->dev == dev) */
> >  	put_device(chip->dev);
> >  }
> 
> Hunk does not belong in this patch

Ack (thanks).

> > diff --git a/drivers/char/tpm/tpm2-commands.c b/drivers/char/tpm/tpm2-commands.c
> > index a21dfd5..6365087 100644
> > +++ b/drivers/char/tpm/tpm2-commands.c
> > @@ -195,7 +195,7 @@ ssize_t tpm2_get_tpm_pt(struct device *dev, u32 property_id,  u32* value,
> >  	cmd.header.in = tpm2_get_tpm_pt_header;
> >  	cmd.params.tpm2_get_tpm_pt_in.cap_id =
> >  		cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
> > -	cmd.params.tpm2_get_tpm_pt_in.property_id = property_id;
> > +	cmd.params.tpm2_get_tpm_pt_in.property_id = cpu_to_be32(property_id);
> >  	cmd.params.tpm2_get_tpm_pt_in.property_cnt = cpu_to_be32(1);
> 
> Hunk does not belong in this patch

Ack (thanks).

> > +static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
> > +			 char *buf)
> > +{
> 
> The pcrs file never conformed to the sysfs rules, if TPM2 is getting a
> whole new file set, I wouldn't mind seeing it not include the
> non-conformant ones. What do you think?

I think that it's better to put extra focus on these sysfs attributes in
first patch set because it's user space visible. What's wrong in the
current pcrs file?

> > +static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> > +			 char *buf)
> > +{
> 
> Ditto.. The manfacturer number should probably be its own file

Maybe here would make sense to have three files:

- manufacturer
- firmware_1
- firmware_2

More or less following the name of the TPM properties in the
specification.

> > +static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
> > +			      char *buf)
> > +{
> > +	struct tpm_chip *chip = dev_get_drvdata(dev);
> > +
> > +	if (chip->vendor.duration[TPM_LONG] == 0)
> > +		return 0;
> > +
> > +	return sprintf(buf, "%d %d %d [%s]\n",
> > +		       jiffies_to_usecs(chip->vendor.duration[TPM_SHORT]),
> > +		       jiffies_to_usecs(chip->vendor.duration[TPM_MEDIUM]),
> > +		       jiffies_to_usecs(chip->vendor.duration[TPM_LONG]),
> > +		       chip->vendor.duration_adjusted
> > +		       ? "adjusted" : "original");
> > +}
> > +static DEVICE_ATTR_RO(durations);
> 
> Seem useless since the durations are constant, drop it?
> 
> > +static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
> > +			     char *buf)
> > +{
> > +	struct tpm_chip *chip = dev_get_drvdata(dev);
> > +
> > +	return sprintf(buf, "%d %d %d %d [%s]\n",
> > +		       jiffies_to_usecs(chip->vendor.timeout_a),
> > +		       jiffies_to_usecs(chip->vendor.timeout_b),
> > +		       jiffies_to_usecs(chip->vendor.timeout_c),
> > +		       jiffies_to_usecs(chip->vendor.timeout_d),
> > +		       chip->vendor.timeout_adjusted
> > +		       ? "adjusted" : "original");
> > +}
> > +static DEVICE_ATTR_RO(timeouts);
> 
> Ditto
> 
> > +static ssize_t version_show(struct device *dev, struct device_attribute *attr,
> > +			   char *buf)
> > +{
> > +	char *str = buf;
> > +
> > +	str += sprintf(str, "2.0\n");
> > +
> > +	return str - buf;
> > +}
> > +
> > +static DEVICE_ATTR_RO(version);
> > +
> > +
> > +static ssize_t tpm2_show(struct device *dev, struct device_attribute *attr,
> > +			 char *buf)
> > +{
> > +	struct tpm_chip *chip = dev_get_drvdata(dev);
> > +
> > +	return sprintf(buf, "%d\n", chip->tpm2);
> > +}
> > +static DEVICE_ATTR_RO(tpm2);
> 
> Two things for the same report? Drop one?
> 
> Also, I think some thought is needed for the char interface - some
> kind of ioctl to enter TPM2 mode and EINVAL access until that is done?
> 
> Finally, this is in the wrong place in sysfs, I suspect it should be
> attached to the char device node, not the platform device node? We
> should at least investigate this...

This was forgotten. Should not be here at all. Instead we have the
variable 'version' to state specification family and level.

I did not fully understand the comment about tpm2 flag. Why driver
cannot set it when it initializes the device like with this based
on value of the STS3?

> > +struct tpm2_permanent {
> > +	unsigned int owner_auth_set		: 1;
> > +	unsigned int endorsement_auth_set	: 1;
> > +	unsigned int lockout_auth_set		: 1;
> > +	unsigned int reserved1			: 5;
> > +	unsigned int disable_clear		: 1;
> > +	unsigned int in_lockout			: 1;
> > +	unsigned int tpm_generated_eps		: 1;
> > +	unsigned int reserved2			: 21;
> > +};
> > +
> > +struct tpm2_startup_clear {
> > +	unsigned int ph_enable			: 1;
> > +	unsigned int sh_enable			: 1;
> > +	unsigned int eh_enable			: 1;
> > +	unsigned int ph_enable_nv		: 1;
> > +	unsigned int reserved			: 27;
> > +	unsigned int orderly			: 1;
> > +};
> 
> This idea is not portable, you cannot use bitfields to index bits in a
> word like this. Please use constants defined with BIT(xx)

Thanks, I'll change this.

> Next posting can you include a github link? Thanks

Yup sure. I do everything for v2 in tpm2-v1 by adding fixes on top of 
these patches. When things look good there I'll create a new branch
tpm2-v2 and prepare the next patch set.

> Jason

/Jarkko

Re: [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Jason Gunthorpe]

On Wed, Sep 24, 2014 at 10:02:34PM +0300, Jarkko Sakkinen wrote:

> > The pcrs file never conformed to the sysfs rules, if TPM2 is getting a
> > whole new file set, I wouldn't mind seeing it not include the
> > non-conformant ones. What do you think?
> 
> I think that it's better to put extra focus on these sysfs attributes in
> first patch set because it's user space visible. What's wrong in the
> current pcrs file?

Each PCR should be a distinct sysfs file, probably with a
directory. One Value Per File is the rule.

> > > +static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> > > +			 char *buf)
> > > +{
> > 
> > Ditto.. The manfacturer number should probably be its own file
> 
> Maybe here would make sense to have three files:
> 
> - manufacturer
> - firmware_1
> - firmware_2
> 
> More or less following the name of the TPM properties in the
> specification.

Probably, maybe firmware_1/2 could be combined if they are the same
logical value? (I've always expressed it as firmware_1.firwmare_2?)

> I did not fully understand the comment about tpm2 flag. Why driver
> cannot set it when it initializes the device like with this based
> on value of the STS3?

I was talking about the /dev/ char device - a random application today
will open it and send TPM1 formed messages. Those should be refused
with EINVAL for a TPM2 chip unless the application declares via IOCTL
that it will be sending TPM2 messages.

Otherwise the API contract for the /dev/ device (write TPM1 formed
messages) is broken..

Jason

Re: [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Peter Hüwe]

Am Mittwoch, 24. September 2014, 22:19:38 schrieb Jason Gunthorpe:
> On Wed, Sep 24, 2014 at 10:02:34PM +0300, Jarkko Sakkinen wrote:
> > > The pcrs file never conformed to the sysfs rules, if TPM2 is getting a
> > > whole new file set, I wouldn't mind seeing it not include the
> > > non-conformant ones. What do you think?
> > 
> > I think that it's better to put extra focus on these sysfs attributes in
> > first patch set because it's user space visible. What's wrong in the
> > current pcrs file?
> 
> Each PCR should be a distinct sysfs file, probably with a
> directory. One Value Per File is the rule.

That would be 24*2 files only for pcrs... 
Documentation/filesystems/sysfs.txt says:

"
Attributes should be ASCII text files, preferably with only one value
per file. It is noted that it may not be efficient to contain only one
value per file, so it is socially acceptable to express an array of
values of the same type. "

So it would be more or less o.k. to have it in one file like we had.


Then however:
"Mixing types, expressing multiple lines of data, and doing fancy
formatting of data is heavily frowned upon. Doing these things may get
you publicly humiliated and your code rewritten without notice."


Do we really need the PCRs as sysfs files?
I know they are handy as a dev, but does any application actually use this 
directly?


Thanks,
Peter

Re: [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Jason Gunthorpe]

On Wed, Sep 24, 2014 at 10:35:42PM +0200, Peter Hüwe wrote:
> Am Mittwoch, 24. September 2014, 22:19:38 schrieb Jason Gunthorpe:
> > On Wed, Sep 24, 2014 at 10:02:34PM +0300, Jarkko Sakkinen wrote:
> > > > The pcrs file never conformed to the sysfs rules, if TPM2 is getting a
> > > > whole new file set, I wouldn't mind seeing it not include the
> > > > non-conformant ones. What do you think?
> > > 
> > > I think that it's better to put extra focus on these sysfs attributes in
> > > first patch set because it's user space visible. What's wrong in the
> > > current pcrs file?
> > 
> > Each PCR should be a distinct sysfs file, probably with a
> > directory. One Value Per File is the rule.
> 
> That would be 24*2 files only for pcrs...

Some subsystems do just that..

$ ls /sys/class/infiniband/qib0/ports/1/sl2vl/
0  1  10  11  12  13  14  15  2  3  4  5  6  7  8  9

> Documentation/filesystems/sysfs.txt says:
> 
> "
> Attributes should be ASCII text files, preferably with only one value
> per file. It is noted that it may not be efficient to contain only one
> value per file, so it is socially acceptable to express an array of
> values of the same type. "
> 
> So it would be more or less o.k. to have it in one file like we had.
> 
> Then however:
> "Mixing types, expressing multiple lines of data, and doing fancy
> formatting of data is heavily frowned upon. Doing these things may get
> you publicly humiliated and your code rewritten without notice."

I think taken together that says an array of 128 bit PCR hex values
without new lines or other formatting would be OK. But the breakdown
and fancy formatting we do is not OK.

> Do we really need the PCRs as sysfs files?  I know they are handy as
> a dev, but does any application actually use this directly?

No idea, but using tpm2 to find out seems like a reasonable idea,
especially if the pcr meaning changes in some way with TPM2 ..

Jason

Re: [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Jarkko Sakkinen]

On Wed, Sep 24, 2014 at 02:46:27PM -0600, Jason Gunthorpe wrote:
> > That would be 24*2 files only for pcrs...
> 
> Some subsystems do just that..
> 
> $ ls /sys/class/infiniband/qib0/ports/1/sl2vl/
> 0  1  10  11  12  13  14  15  2  3  4  5  6  7  8  9

They use static structures in drivers/infiniband/hw/qib/qib_sysfs.c
and it does not looks a mess. I would prefer to create struct attribute
entries dynamically if there's clean and easy way to do that.

/Jarkko

Re: [tpmdd-devel] [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Jarkko Sakkinen]

On Fri, Sep 26, 2014 at 08:19:47PM +0300, Jarkko Sakkinen wrote:
> On Wed, Sep 24, 2014 at 02:46:27PM -0600, Jason Gunthorpe wrote:
> > > That would be 24*2 files only for pcrs...
> > 
> > Some subsystems do just that..
> > 
> > $ ls /sys/class/infiniband/qib0/ports/1/sl2vl/
> > 0  1  10  11  12  13  14  15  2  3  4  5  6  7  8  9
> 
> They use static structures in drivers/infiniband/hw/qib/qib_sysfs.c
> and it does not looks a mess. I would prefer to create struct attribute
> entries dynamically if there's clean and easy way to do that.

I gave this a shot:

https://github.com/jsakkine/linux-tpm2/commit/dffce68ce34da265a62908dec71b2d85fc16824f

I want to initialize dynamically so that it is easy to support 
TPM_PT_PCR_COUNT later.

/Jarkko

Re: [tpmdd-devel] [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Jason Gunthorpe]

On Tue, Sep 30, 2014 at 11:07:21PM +0300, Jarkko Sakkinen wrote:
> On Fri, Sep 26, 2014 at 08:19:47PM +0300, Jarkko Sakkinen wrote:
> > On Wed, Sep 24, 2014 at 02:46:27PM -0600, Jason Gunthorpe wrote:
> > > > That would be 24*2 files only for pcrs...
> > > 
> > > Some subsystems do just that..
> > > 
> > > $ ls /sys/class/infiniband/qib0/ports/1/sl2vl/
> > > 0  1  10  11  12  13  14  15  2  3  4  5  6  7  8  9
> > 
> > They use static structures in
> > drivers/infiniband/hw/qib/qib_sysfs.c and it does not looks a
> > mess. I would prefer to create struct attribute entries
> > dynamically if there's clean and easy way to do that.
> 
> I gave this a shot:
> 
> https://github.com/jsakkine/linux-tpm2/commit/dffce68ce34da265a62908dec71b2d85fc16824f
> 
> I want to initialize dynamically so that it is easy to support
> TPM_PT_PCR_COUNT later.

You can't use a static pcr_dev_attrs, this has to be allocated in the
chip structure (because of the NULL). Otherwise looks about right
(although there are more problematic core details here, like racing of the
tpm dev create with the creation of the sysfs files)

Do you have a reason to have the pcrs in sysfs? I'd be just as happy
to see them dropped or moved to debugfs for TPM2 as well.

Jason

Re: [tpmdd-devel] [PATCH v1 12/12] tpm: TPM2 sysfs attributes

[Jarkko Sakkinen]

On Tue, Sep 30, 2014 at 02:12:09PM -0600, Jason Gunthorpe wrote:
> On Tue, Sep 30, 2014 at 11:07:21PM +0300, Jarkko Sakkinen wrote:
> > On Fri, Sep 26, 2014 at 08:19:47PM +0300, Jarkko Sakkinen wrote:
> > > On Wed, Sep 24, 2014 at 02:46:27PM -0600, Jason Gunthorpe wrote:
> > > > > That would be 24*2 files only for pcrs...
> > > > 
> > > > Some subsystems do just that..
> > > > 
> > > > $ ls /sys/class/infiniband/qib0/ports/1/sl2vl/
> > > > 0  1  10  11  12  13  14  15  2  3  4  5  6  7  8  9
> > > 
> > > They use static structures in
> > > drivers/infiniband/hw/qib/qib_sysfs.c and it does not looks a
> > > mess. I would prefer to create struct attribute entries
> > > dynamically if there's clean and easy way to do that.
> > 
> > I gave this a shot:
> > 
> > https://github.com/jsakkine/linux-tpm2/commit/dffce68ce34da265a62908dec71b2d85fc16824f
> > 
> > I want to initialize dynamically so that it is easy to support
> > TPM_PT_PCR_COUNT later.
> 
> You can't use a static pcr_dev_attrs, this has to be allocated in the
> chip structure (because of the NULL). Otherwise looks about right
> (although there are more problematic core details here, like racing of the
> tpm dev create with the creation of the sysfs files)

I improved things and pushed a commit that encapsulates PCR bank into
struct pcrs_kobj (internal struct in tpm2-sysfs.c).

This brings me to my question. In TPM2 there is not just PCR bank with
SHA-1 hashes but there are multiple PCR banks.

The way I plan to represent this is:

pcrs/<tag for banks hash algorithm>/<PCR index>

My last commit provides most of the code for implementing this although
in the initial patch set I would take a short cut and only show SHA-1
PCRs. The important thing is that it is put into pcrs/sha1 directory.

> Do you have a reason to have the pcrs in sysfs? I'd be just as happy
> to see them dropped or moved to debugfs for TPM2 as well.

Well, I at least find it useful debugging tool sometimes. And now it
is more useful because PCR values are machine readable.

I will include this to the v2 patch set. When we have a clean patch
set with all the fixes applied into tpm2-v1 branch it will be a better
time to evaluate what is mandatory and what can be postponed.

> Jason

/Jarkko

Re: [PATCH v1 00/12] tpm: TPM2 support

[Jarkko Sakkinen]

On Wed, Sep 24, 2014 at 12:05:50PM +0300, Jarkko Sakkinen wrote:
> This patch set enables TPM2 protocol and provides drivers for FIFO and
> CRB interfaces.

Known issues so far:

- le32/le64_to_cpu()'s and one cpu_to_le32 missing from tpm_crb.c
  assuming that CRB driver would be used in big-endian architecture.
- I still have troubles having FIFO testing environment so it would
  not be a big surprise if it had broke for some reason (help with
  testing and/or bug fixes for FIFO would be highly appreciated).

Notes about the implementation:

- Reworked the code base to the direction that Jason Gunthorpe suggested.
- Moved to use devres in tpm_crb as Jason suggested.
- Did some groundwork for implementing Stefan Bergers suggestions to
  tpm_crb.

I hope that it is now more in right direction than in my preview.

I'll push fixes on top of tpm2-v1 branch [1] and do not rebase it anymore
and therefore it is a good testing branch.

When things look good enough I'll create tpm2-v2 branch and prepare and
send a new patch set.

[1] https://github.com/jsakkine/linux-tpm2/

/Jarko

> Jarkko Sakkinen (8):
>   tpm: prepare TPM driver for adding TPM2 support
>   tpm: TPM2 support for tpm_pcr_read()
>   tpm: TPM2 support for tpm_do_selftest()
>   tpm: added tpm2_get_tpm_pt()
>   tpm: TPM2 support for tpm_pcr_extend()
>   tpm: TPM2 support for tpm_get_random().
>   tpm: Driver for TPM 2.0 CRB Interface
>   tpm: TPM2 sysfs attributes
> 
> Will Arthur (4):
>   tpm: TPM2 support for tpm_calc_ordinal_durations()
>   tpm: TPM2 support for tpm_startup()
>   tpm: TPM2 support for tpm_gen_interrupt().
>   tpm: TPM 2.0 FIFO Interface
> 
>  drivers/char/tpm/Kconfig         |   9 +
>  drivers/char/tpm/Makefile        |   3 +-
>  drivers/char/tpm/tpm-chip.c      | 175 ++++++++++++++++
>  drivers/char/tpm/tpm-interface.c | 160 +++++----------
>  drivers/char/tpm/tpm.h           |  84 ++++++++
>  drivers/char/tpm/tpm2-commands.c | 422 +++++++++++++++++++++++++++++++++++++++
>  drivers/char/tpm/tpm2-sysfs.c    | 242 ++++++++++++++++++++++
>  drivers/char/tpm/tpm2.h          | 108 ++++++++++
>  drivers/char/tpm/tpm_crb.c       | 332 ++++++++++++++++++++++++++++++
>  drivers/char/tpm/tpm_tis.c       |  53 ++++-
>  10 files changed, 1463 insertions(+), 125 deletions(-)
>  create mode 100644 drivers/char/tpm/tpm-chip.c
>  create mode 100644 drivers/char/tpm/tpm2-commands.c
>  create mode 100644 drivers/char/tpm/tpm2-sysfs.c
>  create mode 100644 drivers/char/tpm/tpm2.h
>  create mode 100644 drivers/char/tpm/tpm_crb.c
> 
> -- 
> 2.1.0
>