[PATCH 0/2] tpm: driver- and tpm2-specific sysfs attributes

[PATCH 0/2] tpm: driver- and tpm2-specific sysfs attributes

[Andrey Pronin]

This patchset adds sysfs attributes for the cases not covered by the
existing TPM1.2 support:
 - device-specific attributes provided by drivers like tpm_tis_spi
 - TPM2.0

Andrey Pronin (2):
  tpm: add sysfs attributes for tpm2
  tpm: support driver-specific sysfs attrs in tpm_tis_core

 drivers/char/tpm/tpm-chip.c     | 48 ++++++++++++++++++++++++++---------------
 drivers/char/tpm/tpm-sysfs.c    | 25 ++++++++++++++++++---
 drivers/char/tpm/tpm.h          |  8 ++++++-
 drivers/char/tpm/tpm_tis_core.c |  3 +++
 drivers/char/tpm/tpm_tis_core.h |  1 +
 5 files changed, 64 insertions(+), 21 deletions(-)

-- 
2.6.6

[PATCH 1/2] tpm: add sysfs attributes for tpm2

[Andrey Pronin]

Break sysfs attributes into common and TPM 1.2/2.0-specific, and
create sysfs groups for TPM2.0.

Signed-off-by: Andrey Pronin <apronin@chromium.org>
---
 drivers/char/tpm/tpm-chip.c  | 48 ++++++++++++++++++++++++++++----------------
 drivers/char/tpm/tpm-sysfs.c | 24 ++++++++++++++++++++--
 2 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 5a2f043..6c72671 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -281,8 +281,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		return 0;
 
-	tpm_sysfs_add_device(chip);
-
 	chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
 
 	return 0;
@@ -300,14 +298,21 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
 static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
 {
 	struct attribute **i;
+	int ngrp;
 
-	if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
+	if (chip->flags & TPM_CHIP_FLAG_VIRTUAL)
 		return;
 
-	sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
-
-	for (i = chip->groups[0]->attrs; *i != NULL; ++i)
-		sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
+	for (ngrp = 0; ngrp < chip->groups_cnt; ++ngrp) {
+		if (chip->groups[ngrp]->name) {
+			sysfs_remove_link(&chip->dev.parent->kobj,
+					  chip->groups[ngrp]->name);
+		} else {
+			for (i = chip->groups[ngrp]->attrs; *i != NULL; ++i)
+				sysfs_remove_link(&chip->dev.parent->kobj,
+						  (*i)->name);
+		}
+	}
 }
 
 /* For compatibility with legacy sysfs paths we provide symlinks from the
@@ -317,20 +322,27 @@ static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
 static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
 {
 	struct attribute **i;
-	int rc;
+	int rc = 0;
+	int ngrp;
 
-	if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
+	if (chip->flags & TPM_CHIP_FLAG_VIRTUAL)
 		return 0;
 
-	rc = __compat_only_sysfs_link_entry_to_kobj(
-		    &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
-	if (rc && rc != -ENOENT)
-		return rc;
-
 	/* All the names from tpm-sysfs */
-	for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
-		rc = __compat_only_sysfs_link_entry_to_kobj(
-		    &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
+	for (ngrp = 0; ngrp < chip->groups_cnt; ++ngrp) {
+		if (chip->groups[ngrp]->name) {
+			rc = __compat_only_sysfs_link_entry_to_kobj(
+				&chip->dev.parent->kobj, &chip->dev.kobj,
+				chip->groups[ngrp]->name);
+		} else {
+			for (i = chip->groups[ngrp]->attrs;
+			     (*i != NULL) && !rc;
+			     ++i)
+				rc = __compat_only_sysfs_link_entry_to_kobj(
+					&chip->dev.parent->kobj,
+					&chip->dev.kobj,
+					(*i)->name);
+		}
 		if (rc) {
 			tpm_del_legacy_sysfs(chip);
 			return rc;
@@ -354,6 +366,8 @@ int tpm_chip_register(struct tpm_chip *chip)
 {
 	int rc;
 
+	tpm_sysfs_add_device(chip);
+
 	rc = tpm1_chip_register(chip);
 	if (rc)
 		return rc;
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index b46cf70..95ce90d 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -265,6 +265,12 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
 static DEVICE_ATTR_RO(timeouts);
 
 static struct attribute *tpm_dev_attrs[] = {
+	&dev_attr_durations.attr,
+	&dev_attr_timeouts.attr,
+	NULL,
+};
+
+static struct attribute *tpm1_dev_attrs[] = {
 	&dev_attr_pubek.attr,
 	&dev_attr_pcrs.attr,
 	&dev_attr_enabled.attr,
@@ -273,8 +279,10 @@ static struct attribute *tpm_dev_attrs[] = {
 	&dev_attr_temp_deactivated.attr,
 	&dev_attr_caps.attr,
 	&dev_attr_cancel.attr,
-	&dev_attr_durations.attr,
-	&dev_attr_timeouts.attr,
+	NULL,
+};
+
+static struct attribute *tpm2_dev_attrs[] = {
 	NULL,
 };
 
@@ -282,6 +290,14 @@ static const struct attribute_group tpm_dev_group = {
 	.attrs = tpm_dev_attrs,
 };
 
+static const struct attribute_group tpm1_dev_group = {
+	.attrs = tpm1_dev_attrs,
+};
+
+static const struct attribute_group tpm2_dev_group = {
+	.attrs = tpm2_dev_attrs,
+};
+
 void tpm_sysfs_add_device(struct tpm_chip *chip)
 {
 	/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
@@ -290,4 +306,8 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
 	 */
 	WARN_ON(chip->groups_cnt != 0);
 	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
+	else
+		chip->groups[chip->groups_cnt++] = &tpm1_dev_group;
 }
-- 
2.6.6

[PATCH 1/2] tpm: add sysfs attributes for tpm2

[Andrey Pronin]

Break sysfs attributes into common and TPM 1.2/2.0-specific, and
create sysfs groups for TPM2.0.

Signed-off-by: Andrey Pronin <apronin-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 drivers/char/tpm/tpm-chip.c  | 48 ++++++++++++++++++++++++++++----------------
 drivers/char/tpm/tpm-sysfs.c | 24 ++++++++++++++++++++--
 2 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 5a2f043..6c72671 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -281,8 +281,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		return 0;
 
-	tpm_sysfs_add_device(chip);
-
 	chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
 
 	return 0;
@@ -300,14 +298,21 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
 static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
 {
 	struct attribute **i;
+	int ngrp;
 
-	if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
+	if (chip->flags & TPM_CHIP_FLAG_VIRTUAL)
 		return;
 
-	sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
-
-	for (i = chip->groups[0]->attrs; *i != NULL; ++i)
-		sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
+	for (ngrp = 0; ngrp < chip->groups_cnt; ++ngrp) {
+		if (chip->groups[ngrp]->name) {
+			sysfs_remove_link(&chip->dev.parent->kobj,
+					  chip->groups[ngrp]->name);
+		} else {
+			for (i = chip->groups[ngrp]->attrs; *i != NULL; ++i)
+				sysfs_remove_link(&chip->dev.parent->kobj,
+						  (*i)->name);
+		}
+	}
 }
 
 /* For compatibility with legacy sysfs paths we provide symlinks from the
@@ -317,20 +322,27 @@ static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
 static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
 {
 	struct attribute **i;
-	int rc;
+	int rc = 0;
+	int ngrp;
 
-	if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
+	if (chip->flags & TPM_CHIP_FLAG_VIRTUAL)
 		return 0;
 
-	rc = __compat_only_sysfs_link_entry_to_kobj(
-		    &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
-	if (rc && rc != -ENOENT)
-		return rc;
-
 	/* All the names from tpm-sysfs */
-	for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
-		rc = __compat_only_sysfs_link_entry_to_kobj(
-		    &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
+	for (ngrp = 0; ngrp < chip->groups_cnt; ++ngrp) {
+		if (chip->groups[ngrp]->name) {
+			rc = __compat_only_sysfs_link_entry_to_kobj(
+				&chip->dev.parent->kobj, &chip->dev.kobj,
+				chip->groups[ngrp]->name);
+		} else {
+			for (i = chip->groups[ngrp]->attrs;
+			     (*i != NULL) && !rc;
+			     ++i)
+				rc = __compat_only_sysfs_link_entry_to_kobj(
+					&chip->dev.parent->kobj,
+					&chip->dev.kobj,
+					(*i)->name);
+		}
 		if (rc) {
 			tpm_del_legacy_sysfs(chip);
 			return rc;
@@ -354,6 +366,8 @@ int tpm_chip_register(struct tpm_chip *chip)
 {
 	int rc;
 
+	tpm_sysfs_add_device(chip);
+
 	rc = tpm1_chip_register(chip);
 	if (rc)
 		return rc;
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index b46cf70..95ce90d 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -265,6 +265,12 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
 static DEVICE_ATTR_RO(timeouts);
 
 static struct attribute *tpm_dev_attrs[] = {
+	&dev_attr_durations.attr,
+	&dev_attr_timeouts.attr,
+	NULL,
+};
+
+static struct attribute *tpm1_dev_attrs[] = {
 	&dev_attr_pubek.attr,
 	&dev_attr_pcrs.attr,
 	&dev_attr_enabled.attr,
@@ -273,8 +279,10 @@ static struct attribute *tpm_dev_attrs[] = {
 	&dev_attr_temp_deactivated.attr,
 	&dev_attr_caps.attr,
 	&dev_attr_cancel.attr,
-	&dev_attr_durations.attr,
-	&dev_attr_timeouts.attr,
+	NULL,
+};
+
+static struct attribute *tpm2_dev_attrs[] = {
 	NULL,
 };
 
@@ -282,6 +290,14 @@ static const struct attribute_group tpm_dev_group = {
 	.attrs = tpm_dev_attrs,
 };
 
+static const struct attribute_group tpm1_dev_group = {
+	.attrs = tpm1_dev_attrs,
+};
+
+static const struct attribute_group tpm2_dev_group = {
+	.attrs = tpm2_dev_attrs,
+};
+
 void tpm_sysfs_add_device(struct tpm_chip *chip)
 {
 	/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
@@ -290,4 +306,8 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
 	 */
 	WARN_ON(chip->groups_cnt != 0);
 	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
+	else
+		chip->groups[chip->groups_cnt++] = &tpm1_dev_group;
 }
-- 
2.6.6


------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

[PATCH 2/2] tpm: support driver-specific sysfs attrs in tpm_tis_core

[Andrey Pronin]

Add attr_group to phy_ops that a driver relying on tpm_tis_core_init
can set to have its specific attributes registered in sysfs.

Signed-off-by: Andrey Pronin <apronin@chromium.org>
---
 drivers/char/tpm/tpm-sysfs.c    | 1 -
 drivers/char/tpm/tpm.h          | 8 +++++++-
 drivers/char/tpm/tpm_tis_core.c | 3 +++
 drivers/char/tpm/tpm_tis_core.h | 1 +
 4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 95ce90d..22c9874 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -304,7 +304,6 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
 	 * is called before ops is null'd and the sysfs core synchronizes this
 	 * removal so that no callbacks are running or can run again
 	 */
-	WARN_ON(chip->groups_cnt != 0);
 	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 8890df2..8c69649 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -173,7 +173,13 @@ struct tpm_chip {
 
 	struct dentry **bios_dir;
 
-	const struct attribute_group *groups[3];
+	/* up to 4 attribute groups:
+	 * - driver-specific
+	 * - common TPM1.2 and TPM2.0
+	 * - TPM1.2/2.0-specific
+	 * - ppi
+	 */
+	const struct attribute_group *groups[5];
 	unsigned int groups_cnt;
 #ifdef CONFIG_ACPI
 	acpi_handle acpi_dev_handle;
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 8110b52..6d5d8f4 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -796,6 +796,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 		}
 	}
 
+	if (priv->phy_ops->attr_group)
+		chip->groups[chip->groups_cnt++] = priv->phy_ops->attr_group;
+
 	return tpm_chip_register(chip);
 out_err:
 	tpm_tis_remove(chip);
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 9191aab..4417ed9 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -95,6 +95,7 @@ struct tpm_tis_data {
 };
 
 struct tpm_tis_phy_ops {
+	const struct attribute_group *attr_group;
 	int (*read_bytes)(struct tpm_tis_data *data, u32 addr, u16 len,
 			  u8 *result);
 	int (*write_bytes)(struct tpm_tis_data *data, u32 addr, u16 len,
-- 
2.6.6

[PATCH 2/2] tpm: support driver-specific sysfs attrs in tpm_tis_core

[Andrey Pronin]

Add attr_group to phy_ops that a driver relying on tpm_tis_core_init
can set to have its specific attributes registered in sysfs.

Signed-off-by: Andrey Pronin <apronin-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 drivers/char/tpm/tpm-sysfs.c    | 1 -
 drivers/char/tpm/tpm.h          | 8 +++++++-
 drivers/char/tpm/tpm_tis_core.c | 3 +++
 drivers/char/tpm/tpm_tis_core.h | 1 +
 4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 95ce90d..22c9874 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -304,7 +304,6 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
 	 * is called before ops is null'd and the sysfs core synchronizes this
 	 * removal so that no callbacks are running or can run again
 	 */
-	WARN_ON(chip->groups_cnt != 0);
 	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 8890df2..8c69649 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -173,7 +173,13 @@ struct tpm_chip {
 
 	struct dentry **bios_dir;
 
-	const struct attribute_group *groups[3];
+	/* up to 4 attribute groups:
+	 * - driver-specific
+	 * - common TPM1.2 and TPM2.0
+	 * - TPM1.2/2.0-specific
+	 * - ppi
+	 */
+	const struct attribute_group *groups[5];
 	unsigned int groups_cnt;
 #ifdef CONFIG_ACPI
 	acpi_handle acpi_dev_handle;
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 8110b52..6d5d8f4 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -796,6 +796,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 		}
 	}
 
+	if (priv->phy_ops->attr_group)
+		chip->groups[chip->groups_cnt++] = priv->phy_ops->attr_group;
+
 	return tpm_chip_register(chip);
 out_err:
 	tpm_tis_remove(chip);
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 9191aab..4417ed9 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -95,6 +95,7 @@ struct tpm_tis_data {
 };
 
 struct tpm_tis_phy_ops {
+	const struct attribute_group *attr_group;
 	int (*read_bytes)(struct tpm_tis_data *data, u32 addr, u16 len,
 			  u8 *result);
 	int (*write_bytes)(struct tpm_tis_data *data, u32 addr, u16 len,
-- 
2.6.6


------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: [PATCH 1/2] tpm: add sysfs attributes for tpm2

[Jason Gunthorpe]

On Thu, Jul 14, 2016 at 06:51:35PM -0700, Andrey Pronin wrote:
> -	sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> -
> -	for (i = chip->groups[0]->attrs; *i != NULL; ++i)
> -		sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
> +	for (ngrp = 0; ngrp < chip->groups_cnt; ++ngrp) {
> +		if (chip->groups[ngrp]->name) {
> +			sysfs_remove_link(&chip->dev.parent->kobj,
> +					  chip->groups[ngrp]->name);
> +		} else {
> +			for (i = chip->groups[ngrp]->attrs; *i != NULL; ++i)
> +				sysfs_remove_link(&chip->dev.parent->kobj,
> +						  (*i)->name);
> +		}
> +	}

NAK

No new compat symlinks. Only the existing set of links are permitted.

Any new sysfs entries must use only the new location.

> +static struct attribute *tpm2_dev_attrs[] = {
>  	NULL,
>  };

> +static const struct attribute_group tpm2_dev_group = {
> +	.attrs = tpm2_dev_attrs,
> +};

Don't add dead code, add this and related in the patch that requires it.

>  void tpm_sysfs_add_device(struct tpm_chip *chip)
>  {
>  	/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
> @@ -290,4 +306,8 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
>  	 */
>  	WARN_ON(chip->groups_cnt != 0);
>  	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
> +	else
> +		chip->groups[chip->groups_cnt++] = &tpm1_dev_group;

.. and this can't really happen either..

To make things simple you can just have tpm2 not ever create any links
for any files by never using groups[0]. There is no need to try and
create a shared 'tpm_dev_group'.

Jason

Re: [PATCH 2/2] tpm: support driver-specific sysfs attrs in tpm_tis_core

[Jason Gunthorpe]

On Thu, Jul 14, 2016 at 06:51:36PM -0700, Andrey Pronin wrote:
> -	WARN_ON(chip->groups_cnt != 0);

Nope.

> -	const struct attribute_group *groups[3];
> +	/* up to 4 attribute groups:
> +	 * - driver-specific
> +	 * - common TPM1.2 and TPM2.0
> +	 * - TPM1.2/2.0-specific
> +	 * - ppi
> +	 */
> +	const struct attribute_group *groups[5];

The prior patch needed to have groups[4], every patch much work.

> +	if (priv->phy_ops->attr_group)
> +		chip->groups[chip->groups_cnt++] = priv->phy_ops->attr_group;

I am really not excited about having driver specific sysfs
files.

What is the justification for this?

Jason

Re: [PATCH 2/2] tpm: support driver-specific sysfs attrs in tpm_tis_core

[Jason Gunthorpe]

On Thu, Jul 14, 2016 at 06:51:36PM -0700, Andrey Pronin wrote:
> -	WARN_ON(chip->groups_cnt != 0);

Nope.

> -	const struct attribute_group *groups[3];
> +	/* up to 4 attribute groups:
> +	 * - driver-specific
> +	 * - common TPM1.2 and TPM2.0
> +	 * - TPM1.2/2.0-specific
> +	 * - ppi
> +	 */
> +	const struct attribute_group *groups[5];

The prior patch needed to have groups[4], every patch much work.

> +	if (priv->phy_ops->attr_group)
> +		chip->groups[chip->groups_cnt++] = priv->phy_ops->attr_group;

I am really not excited about having driver specific sysfs
files.

What is the justification for this?

Jason

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: [PATCH 1/2] tpm: add sysfs attributes for tpm2

[Andrey Pronin]

On Thu, Jul 14, 2016 at 09:21:45PM -0600, Jason Gunthorpe wrote:
> On Thu, Jul 14, 2016 at 06:51:35PM -0700, Andrey Pronin wrote:
> > -	sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> > -
> > -	for (i = chip->groups[0]->attrs; *i != NULL; ++i)
> > -		sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
> > +	for (ngrp = 0; ngrp < chip->groups_cnt; ++ngrp) {
> > +		if (chip->groups[ngrp]->name) {
> > +			sysfs_remove_link(&chip->dev.parent->kobj,
> > +					  chip->groups[ngrp]->name);
> > +		} else {
> > +			for (i = chip->groups[ngrp]->attrs; *i != NULL; ++i)
> > +				sysfs_remove_link(&chip->dev.parent->kobj,
> > +						  (*i)->name);
> > +		}
> > +	}
> 
> NAK
> 
> No new compat symlinks. Only the existing set of links are permitted.
> 
> Any new sysfs entries must use only the new location.
> 
> > +static struct attribute *tpm2_dev_attrs[] = {
> >  	NULL,
> >  };
> 
> > +static const struct attribute_group tpm2_dev_group = {
> > +	.attrs = tpm2_dev_attrs,
> > +};
> 
> Don't add dead code, add this and related in the patch that requires it.
> 
> >  void tpm_sysfs_add_device(struct tpm_chip *chip)
> >  {
> >  	/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
> > @@ -290,4 +306,8 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
> >  	 */
> >  	WARN_ON(chip->groups_cnt != 0);
> >  	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> > +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > +		chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
> > +	else
> > +		chip->groups[chip->groups_cnt++] = &tpm1_dev_group;
> 
> .. and this can't really happen either..
> 
> To make things simple you can just have tpm2 not ever create any links
> for any files by never using groups[0]. There is no need to try and
> create a shared 'tpm_dev_group'.
> 
> Jason

Hi Jason,

Mostly understood. One question.

tpm2 shares some of the attributes with tpm1 (e.g. timeouts). Do I still
just add those separately for tpm2 to groups[1] and keep groups[0] empty?

Andrey

Re: [PATCH 1/2] tpm: add sysfs attributes for tpm2

[Andrey Pronin]

On Thu, Jul 14, 2016 at 09:21:45PM -0600, Jason Gunthorpe wrote:
> On Thu, Jul 14, 2016 at 06:51:35PM -0700, Andrey Pronin wrote:
> > -	sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> > -
> > -	for (i = chip->groups[0]->attrs; *i != NULL; ++i)
> > -		sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
> > +	for (ngrp = 0; ngrp < chip->groups_cnt; ++ngrp) {
> > +		if (chip->groups[ngrp]->name) {
> > +			sysfs_remove_link(&chip->dev.parent->kobj,
> > +					  chip->groups[ngrp]->name);
> > +		} else {
> > +			for (i = chip->groups[ngrp]->attrs; *i != NULL; ++i)
> > +				sysfs_remove_link(&chip->dev.parent->kobj,
> > +						  (*i)->name);
> > +		}
> > +	}
> 
> NAK
> 
> No new compat symlinks. Only the existing set of links are permitted.
> 
> Any new sysfs entries must use only the new location.
> 
> > +static struct attribute *tpm2_dev_attrs[] = {
> >  	NULL,
> >  };
> 
> > +static const struct attribute_group tpm2_dev_group = {
> > +	.attrs = tpm2_dev_attrs,
> > +};
> 
> Don't add dead code, add this and related in the patch that requires it.
> 
> >  void tpm_sysfs_add_device(struct tpm_chip *chip)
> >  {
> >  	/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
> > @@ -290,4 +306,8 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
> >  	 */
> >  	WARN_ON(chip->groups_cnt != 0);
> >  	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> > +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > +		chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
> > +	else
> > +		chip->groups[chip->groups_cnt++] = &tpm1_dev_group;
> 
> .. and this can't really happen either..
> 
> To make things simple you can just have tpm2 not ever create any links
> for any files by never using groups[0]. There is no need to try and
> create a shared 'tpm_dev_group'.
> 
> Jason

Hi Jason,

Mostly understood. One question.

tpm2 shares some of the attributes with tpm1 (e.g. timeouts). Do I still
just add those separately for tpm2 to groups[1] and keep groups[0] empty?

Andrey


------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: [PATCH 1/2] tpm: add sysfs attributes for tpm2

[Jason Gunthorpe]

On Thu, Jul 14, 2016 at 08:32:01PM -0700, Andrey Pronin wrote:

> tpm2 shares some of the attributes with tpm1 (e.g. timeouts). Do I still
> just add those separately for tpm2 to groups[1] and keep groups[0] empty?

I think so. Since the file never exists for tpm2, nothing coded for
tpm2 will ever look in the old location.

Jason

Re: [PATCH 1/2] tpm: add sysfs attributes for tpm2

[Jason Gunthorpe]

On Thu, Jul 14, 2016 at 08:32:01PM -0700, Andrey Pronin wrote:

> tpm2 shares some of the attributes with tpm1 (e.g. timeouts). Do I still
> just add those separately for tpm2 to groups[1] and keep groups[0] empty?

I think so. Since the file never exists for tpm2, nothing coded for
tpm2 will ever look in the old location.

Jason

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: [PATCH 2/2] tpm: support driver-specific sysfs attrs in tpm_tis_core

[Andrey Pronin]

On Thu, Jul 14, 2016 at 09:23:27PM -0600, Jason Gunthorpe wrote:
> On Thu, Jul 14, 2016 at 06:51:36PM -0700, Andrey Pronin wrote:
> > -	WARN_ON(chip->groups_cnt != 0);
> 
> Nope.
> 
> > -	const struct attribute_group *groups[3];
> > +	/* up to 4 attribute groups:
> > +	 * - driver-specific
> > +	 * - common TPM1.2 and TPM2.0
> > +	 * - TPM1.2/2.0-specific
> > +	 * - ppi
> > +	 */
> > +	const struct attribute_group *groups[5];
> 
> The prior patch needed to have groups[4], every patch much work.
> 
> > +	if (priv->phy_ops->attr_group)
> > +		chip->groups[chip->groups_cnt++] = priv->phy_ops->attr_group;
> 
> I am really not excited about having driver specific sysfs
> files.
> 
> What is the justification for this?
> 
> Jason

Justification: give access to vendor-specific properties that are
specific to a particular chip and its registers.

Andrey

Re: [PATCH 2/2] tpm: support driver-specific sysfs attrs in tpm_tis_core

[Andrey Pronin]

On Thu, Jul 14, 2016 at 09:23:27PM -0600, Jason Gunthorpe wrote:
> On Thu, Jul 14, 2016 at 06:51:36PM -0700, Andrey Pronin wrote:
> > -	WARN_ON(chip->groups_cnt != 0);
> 
> Nope.
> 
> > -	const struct attribute_group *groups[3];
> > +	/* up to 4 attribute groups:
> > +	 * - driver-specific
> > +	 * - common TPM1.2 and TPM2.0
> > +	 * - TPM1.2/2.0-specific
> > +	 * - ppi
> > +	 */
> > +	const struct attribute_group *groups[5];
> 
> The prior patch needed to have groups[4], every patch much work.
> 
> > +	if (priv->phy_ops->attr_group)
> > +		chip->groups[chip->groups_cnt++] = priv->phy_ops->attr_group;
> 
> I am really not excited about having driver specific sysfs
> files.
> 
> What is the justification for this?
> 
> Jason

Justification: give access to vendor-specific properties that are
specific to a particular chip and its registers.

Andrey

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: [PATCH 1/2] tpm: add sysfs attributes for tpm2

[Andrey Pronin]

On Thu, Jul 14, 2016 at 09:34:39PM -0600, Jason Gunthorpe wrote:
> On Thu, Jul 14, 2016 at 08:32:01PM -0700, Andrey Pronin wrote:
> 
> > tpm2 shares some of the attributes with tpm1 (e.g. timeouts). Do I still
> > just add those separately for tpm2 to groups[1] and keep groups[0] empty?

Just realized that if we keep tpm_add_legacy_sysfs() intact, it doesn't
create symlinks for tpm2 case for any of the groups, including groups[0].
So, in tpm_sysfs_add_device() can just do smth like:

if (chip->flags & TPM_CHIP_FLAG_TPM2)
	chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
else
	chip->groups[chip->groups_cnt++] = &tpm_dev_group;

Is that acceptable? Will submit the next version along those lines.

> 
> I think so. Since the file never exists for tpm2, nothing coded for
> tpm2 will ever look in the old location.
> 

There can be a common code working with tpm1.2 that will just continue
working with tpm2 if the common attributes are in the same place. So, some
part of code can be re-used as is, but I agree that there are not too
many common attributes. Will drop for now. Can be addressed later if
the clear need arises.

Re: [PATCH 1/2] tpm: add sysfs attributes for tpm2

[Jason Gunthorpe]

On Fri, Jul 15, 2016 at 09:56:58AM -0700, Andrey Pronin wrote:
> On Thu, Jul 14, 2016 at 09:34:39PM -0600, Jason Gunthorpe wrote:
> > On Thu, Jul 14, 2016 at 08:32:01PM -0700, Andrey Pronin wrote:
> > 
> > > tpm2 shares some of the attributes with tpm1 (e.g. timeouts). Do I still
> > > just add those separately for tpm2 to groups[1] and keep groups[0] empty?
> 
> Just realized that if we keep tpm_add_legacy_sysfs() intact, it doesn't
> create symlinks for tpm2 case for any of the groups, including groups[0].
> So, in tpm_sysfs_add_device() can just do smth like:
> 
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> 	chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
> else
> 	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> 
> Is that acceptable? Will submit the next version along those lines.

Sure, as long as there are no links for TPM2 that sounds fine.

> > I think so. Since the file never exists for tpm2, nothing coded for
> > tpm2 will ever look in the old location.

> There can be a common code working with tpm1.2 that will just
> continue working with tpm2 if the common attributes are in the same
> place. So, some part of code can be re-used as is, but I agree that
> there are not too many common attributes. Will drop for now. Can be
> addressed later if the clear need arises.

Well, we expect people to test & migrate the common code when testing
new tpm2 deployments..

Jason

Re: [PATCH 1/2] tpm: add sysfs attributes for tpm2

[Jason Gunthorpe]

On Fri, Jul 15, 2016 at 09:56:58AM -0700, Andrey Pronin wrote:
> On Thu, Jul 14, 2016 at 09:34:39PM -0600, Jason Gunthorpe wrote:
> > On Thu, Jul 14, 2016 at 08:32:01PM -0700, Andrey Pronin wrote:
> > 
> > > tpm2 shares some of the attributes with tpm1 (e.g. timeouts). Do I still
> > > just add those separately for tpm2 to groups[1] and keep groups[0] empty?
> 
> Just realized that if we keep tpm_add_legacy_sysfs() intact, it doesn't
> create symlinks for tpm2 case for any of the groups, including groups[0].
> So, in tpm_sysfs_add_device() can just do smth like:
> 
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> 	chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
> else
> 	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> 
> Is that acceptable? Will submit the next version along those lines.

Sure, as long as there are no links for TPM2 that sounds fine.

> > I think so. Since the file never exists for tpm2, nothing coded for
> > tpm2 will ever look in the old location.

> There can be a common code working with tpm1.2 that will just
> continue working with tpm2 if the common attributes are in the same
> place. So, some part of code can be re-used as is, but I agree that
> there are not too many common attributes. Will drop for now. Can be
> addressed later if the clear need arises.

Well, we expect people to test & migrate the common code when testing
new tpm2 deployments..

Jason

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: [PATCH 2/2] tpm: support driver-specific sysfs attrs in tpm_tis_core

[Jarkko Sakkinen]

On Thu, Jul 14, 2016 at 06:51:36PM -0700, Andrey Pronin wrote:
> Add attr_group to phy_ops that a driver relying on tpm_tis_core_init
> can set to have its specific attributes registered in sysfs.
> 
> Signed-off-by: Andrey Pronin <apronin@chromium.org>
> ---
>  drivers/char/tpm/tpm-sysfs.c    | 1 -
>  drivers/char/tpm/tpm.h          | 8 +++++++-
>  drivers/char/tpm/tpm_tis_core.c | 3 +++
>  drivers/char/tpm/tpm_tis_core.h | 1 +
>  4 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index 95ce90d..22c9874 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -304,7 +304,6 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
>  	 * is called before ops is null'd and the sysfs core synchronizes this
>  	 * removal so that no callbacks are running or can run again
>  	 */
> -	WARN_ON(chip->groups_cnt != 0);

You should explain this in a commit message if you want to remove it.

In general, this make user space API vendor specific, which is
unacceptable.

/Jarkko

>  	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>  		chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 8890df2..8c69649 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -173,7 +173,13 @@ struct tpm_chip {
>  
>  	struct dentry **bios_dir;
>  
> -	const struct attribute_group *groups[3];
> +	/* up to 4 attribute groups:
> +	 * - driver-specific
> +	 * - common TPM1.2 and TPM2.0
> +	 * - TPM1.2/2.0-specific
> +	 * - ppi
> +	 */
> +	const struct attribute_group *groups[5];
>  	unsigned int groups_cnt;
>  #ifdef CONFIG_ACPI
>  	acpi_handle acpi_dev_handle;
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 8110b52..6d5d8f4 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -796,6 +796,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
>  		}
>  	}
>  
> +	if (priv->phy_ops->attr_group)
> +		chip->groups[chip->groups_cnt++] = priv->phy_ops->attr_group;
> +
>  	return tpm_chip_register(chip);
>  out_err:
>  	tpm_tis_remove(chip);
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index 9191aab..4417ed9 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -95,6 +95,7 @@ struct tpm_tis_data {
>  };
>  
>  struct tpm_tis_phy_ops {
> +	const struct attribute_group *attr_group;
>  	int (*read_bytes)(struct tpm_tis_data *data, u32 addr, u16 len,
>  			  u8 *result);
>  	int (*write_bytes)(struct tpm_tis_data *data, u32 addr, u16 len,
> -- 
> 2.6.6
> 

Re: [PATCH 2/2] tpm: support driver-specific sysfs attrs in tpm_tis_core

[Jarkko Sakkinen]

On Thu, Jul 14, 2016 at 06:51:36PM -0700, Andrey Pronin wrote:
> Add attr_group to phy_ops that a driver relying on tpm_tis_core_init
> can set to have its specific attributes registered in sysfs.
> 
> Signed-off-by: Andrey Pronin <apronin-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>  drivers/char/tpm/tpm-sysfs.c    | 1 -
>  drivers/char/tpm/tpm.h          | 8 +++++++-
>  drivers/char/tpm/tpm_tis_core.c | 3 +++
>  drivers/char/tpm/tpm_tis_core.h | 1 +
>  4 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index 95ce90d..22c9874 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -304,7 +304,6 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
>  	 * is called before ops is null'd and the sysfs core synchronizes this
>  	 * removal so that no callbacks are running or can run again
>  	 */
> -	WARN_ON(chip->groups_cnt != 0);

You should explain this in a commit message if you want to remove it.

In general, this make user space API vendor specific, which is
unacceptable.

/Jarkko

>  	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>  		chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 8890df2..8c69649 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -173,7 +173,13 @@ struct tpm_chip {
>  
>  	struct dentry **bios_dir;
>  
> -	const struct attribute_group *groups[3];
> +	/* up to 4 attribute groups:
> +	 * - driver-specific
> +	 * - common TPM1.2 and TPM2.0
> +	 * - TPM1.2/2.0-specific
> +	 * - ppi
> +	 */
> +	const struct attribute_group *groups[5];
>  	unsigned int groups_cnt;
>  #ifdef CONFIG_ACPI
>  	acpi_handle acpi_dev_handle;
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 8110b52..6d5d8f4 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -796,6 +796,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
>  		}
>  	}
>  
> +	if (priv->phy_ops->attr_group)
> +		chip->groups[chip->groups_cnt++] = priv->phy_ops->attr_group;
> +
>  	return tpm_chip_register(chip);
>  out_err:
>  	tpm_tis_remove(chip);
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index 9191aab..4417ed9 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -95,6 +95,7 @@ struct tpm_tis_data {
>  };
>  
>  struct tpm_tis_phy_ops {
> +	const struct attribute_group *attr_group;
>  	int (*read_bytes)(struct tpm_tis_data *data, u32 addr, u16 len,
>  			  u8 *result);
>  	int (*write_bytes)(struct tpm_tis_data *data, u32 addr, u16 len,
> -- 
> 2.6.6
> 

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: [PATCH 1/2] tpm: add sysfs attributes for tpm2

[Jarkko Sakkinen]

On Thu, Jul 14, 2016 at 06:51:35PM -0700, Andrey Pronin wrote:
> Break sysfs attributes into common and TPM 1.2/2.0-specific, and
> create sysfs groups for TPM2.0.

I agree with Jasons comments and wait for revised version.

/Jarkko

> Signed-off-by: Andrey Pronin <apronin@chromium.org>
> ---
>  drivers/char/tpm/tpm-chip.c  | 48 ++++++++++++++++++++++++++++----------------
>  drivers/char/tpm/tpm-sysfs.c | 24 ++++++++++++++++++++--
>  2 files changed, 53 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 5a2f043..6c72671 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -281,8 +281,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>  		return 0;
>  
> -	tpm_sysfs_add_device(chip);
> -
>  	chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
>  
>  	return 0;
> @@ -300,14 +298,21 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
>  static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
>  {
>  	struct attribute **i;
> +	int ngrp;
>  
> -	if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
> +	if (chip->flags & TPM_CHIP_FLAG_VIRTUAL)
>  		return;
>  
> -	sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> -
> -	for (i = chip->groups[0]->attrs; *i != NULL; ++i)
> -		sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
> +	for (ngrp = 0; ngrp < chip->groups_cnt; ++ngrp) {
> +		if (chip->groups[ngrp]->name) {
> +			sysfs_remove_link(&chip->dev.parent->kobj,
> +					  chip->groups[ngrp]->name);
> +		} else {
> +			for (i = chip->groups[ngrp]->attrs; *i != NULL; ++i)
> +				sysfs_remove_link(&chip->dev.parent->kobj,
> +						  (*i)->name);
> +		}
> +	}
>  }
>  
>  /* For compatibility with legacy sysfs paths we provide symlinks from the
> @@ -317,20 +322,27 @@ static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
>  static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
>  {
>  	struct attribute **i;
> -	int rc;
> +	int rc = 0;
> +	int ngrp;
>  
> -	if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
> +	if (chip->flags & TPM_CHIP_FLAG_VIRTUAL)
>  		return 0;
>  
> -	rc = __compat_only_sysfs_link_entry_to_kobj(
> -		    &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> -	if (rc && rc != -ENOENT)
> -		return rc;
> -
>  	/* All the names from tpm-sysfs */
> -	for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
> -		rc = __compat_only_sysfs_link_entry_to_kobj(
> -		    &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
> +	for (ngrp = 0; ngrp < chip->groups_cnt; ++ngrp) {
> +		if (chip->groups[ngrp]->name) {
> +			rc = __compat_only_sysfs_link_entry_to_kobj(
> +				&chip->dev.parent->kobj, &chip->dev.kobj,
> +				chip->groups[ngrp]->name);
> +		} else {
> +			for (i = chip->groups[ngrp]->attrs;
> +			     (*i != NULL) && !rc;
> +			     ++i)
> +				rc = __compat_only_sysfs_link_entry_to_kobj(
> +					&chip->dev.parent->kobj,
> +					&chip->dev.kobj,
> +					(*i)->name);
> +		}
>  		if (rc) {
>  			tpm_del_legacy_sysfs(chip);
>  			return rc;
> @@ -354,6 +366,8 @@ int tpm_chip_register(struct tpm_chip *chip)
>  {
>  	int rc;
>  
> +	tpm_sysfs_add_device(chip);
> +
>  	rc = tpm1_chip_register(chip);
>  	if (rc)
>  		return rc;
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index b46cf70..95ce90d 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -265,6 +265,12 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
>  static DEVICE_ATTR_RO(timeouts);
>  
>  static struct attribute *tpm_dev_attrs[] = {
> +	&dev_attr_durations.attr,
> +	&dev_attr_timeouts.attr,
> +	NULL,
> +};
> +
> +static struct attribute *tpm1_dev_attrs[] = {
>  	&dev_attr_pubek.attr,
>  	&dev_attr_pcrs.attr,
>  	&dev_attr_enabled.attr,
> @@ -273,8 +279,10 @@ static struct attribute *tpm_dev_attrs[] = {
>  	&dev_attr_temp_deactivated.attr,
>  	&dev_attr_caps.attr,
>  	&dev_attr_cancel.attr,
> -	&dev_attr_durations.attr,
> -	&dev_attr_timeouts.attr,
> +	NULL,
> +};
> +
> +static struct attribute *tpm2_dev_attrs[] = {
>  	NULL,
>  };
>  
> @@ -282,6 +290,14 @@ static const struct attribute_group tpm_dev_group = {
>  	.attrs = tpm_dev_attrs,
>  };
>  
> +static const struct attribute_group tpm1_dev_group = {
> +	.attrs = tpm1_dev_attrs,
> +};
> +
> +static const struct attribute_group tpm2_dev_group = {
> +	.attrs = tpm2_dev_attrs,
> +};
> +
>  void tpm_sysfs_add_device(struct tpm_chip *chip)
>  {
>  	/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
> @@ -290,4 +306,8 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
>  	 */
>  	WARN_ON(chip->groups_cnt != 0);
>  	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
> +	else
> +		chip->groups[chip->groups_cnt++] = &tpm1_dev_group;
>  }
> -- 
> 2.6.6
> 

Re: [PATCH 1/2] tpm: add sysfs attributes for tpm2

[Jarkko Sakkinen]

On Thu, Jul 14, 2016 at 06:51:35PM -0700, Andrey Pronin wrote:
> Break sysfs attributes into common and TPM 1.2/2.0-specific, and
> create sysfs groups for TPM2.0.

I agree with Jasons comments and wait for revised version.

/Jarkko

> Signed-off-by: Andrey Pronin <apronin-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>  drivers/char/tpm/tpm-chip.c  | 48 ++++++++++++++++++++++++++++----------------
>  drivers/char/tpm/tpm-sysfs.c | 24 ++++++++++++++++++++--
>  2 files changed, 53 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 5a2f043..6c72671 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -281,8 +281,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>  		return 0;
>  
> -	tpm_sysfs_add_device(chip);
> -
>  	chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
>  
>  	return 0;
> @@ -300,14 +298,21 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
>  static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
>  {
>  	struct attribute **i;
> +	int ngrp;
>  
> -	if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
> +	if (chip->flags & TPM_CHIP_FLAG_VIRTUAL)
>  		return;
>  
> -	sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> -
> -	for (i = chip->groups[0]->attrs; *i != NULL; ++i)
> -		sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
> +	for (ngrp = 0; ngrp < chip->groups_cnt; ++ngrp) {
> +		if (chip->groups[ngrp]->name) {
> +			sysfs_remove_link(&chip->dev.parent->kobj,
> +					  chip->groups[ngrp]->name);
> +		} else {
> +			for (i = chip->groups[ngrp]->attrs; *i != NULL; ++i)
> +				sysfs_remove_link(&chip->dev.parent->kobj,
> +						  (*i)->name);
> +		}
> +	}
>  }
>  
>  /* For compatibility with legacy sysfs paths we provide symlinks from the
> @@ -317,20 +322,27 @@ static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
>  static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
>  {
>  	struct attribute **i;
> -	int rc;
> +	int rc = 0;
> +	int ngrp;
>  
> -	if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
> +	if (chip->flags & TPM_CHIP_FLAG_VIRTUAL)
>  		return 0;
>  
> -	rc = __compat_only_sysfs_link_entry_to_kobj(
> -		    &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> -	if (rc && rc != -ENOENT)
> -		return rc;
> -
>  	/* All the names from tpm-sysfs */
> -	for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
> -		rc = __compat_only_sysfs_link_entry_to_kobj(
> -		    &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
> +	for (ngrp = 0; ngrp < chip->groups_cnt; ++ngrp) {
> +		if (chip->groups[ngrp]->name) {
> +			rc = __compat_only_sysfs_link_entry_to_kobj(
> +				&chip->dev.parent->kobj, &chip->dev.kobj,
> +				chip->groups[ngrp]->name);
> +		} else {
> +			for (i = chip->groups[ngrp]->attrs;
> +			     (*i != NULL) && !rc;
> +			     ++i)
> +				rc = __compat_only_sysfs_link_entry_to_kobj(
> +					&chip->dev.parent->kobj,
> +					&chip->dev.kobj,
> +					(*i)->name);
> +		}
>  		if (rc) {
>  			tpm_del_legacy_sysfs(chip);
>  			return rc;
> @@ -354,6 +366,8 @@ int tpm_chip_register(struct tpm_chip *chip)
>  {
>  	int rc;
>  
> +	tpm_sysfs_add_device(chip);
> +
>  	rc = tpm1_chip_register(chip);
>  	if (rc)
>  		return rc;
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index b46cf70..95ce90d 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -265,6 +265,12 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
>  static DEVICE_ATTR_RO(timeouts);
>  
>  static struct attribute *tpm_dev_attrs[] = {
> +	&dev_attr_durations.attr,
> +	&dev_attr_timeouts.attr,
> +	NULL,
> +};
> +
> +static struct attribute *tpm1_dev_attrs[] = {
>  	&dev_attr_pubek.attr,
>  	&dev_attr_pcrs.attr,
>  	&dev_attr_enabled.attr,
> @@ -273,8 +279,10 @@ static struct attribute *tpm_dev_attrs[] = {
>  	&dev_attr_temp_deactivated.attr,
>  	&dev_attr_caps.attr,
>  	&dev_attr_cancel.attr,
> -	&dev_attr_durations.attr,
> -	&dev_attr_timeouts.attr,
> +	NULL,
> +};
> +
> +static struct attribute *tpm2_dev_attrs[] = {
>  	NULL,
>  };
>  
> @@ -282,6 +290,14 @@ static const struct attribute_group tpm_dev_group = {
>  	.attrs = tpm_dev_attrs,
>  };
>  
> +static const struct attribute_group tpm1_dev_group = {
> +	.attrs = tpm1_dev_attrs,
> +};
> +
> +static const struct attribute_group tpm2_dev_group = {
> +	.attrs = tpm2_dev_attrs,
> +};
> +
>  void tpm_sysfs_add_device(struct tpm_chip *chip)
>  {
>  	/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
> @@ -290,4 +306,8 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
>  	 */
>  	WARN_ON(chip->groups_cnt != 0);
>  	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
> +	else
> +		chip->groups[chip->groups_cnt++] = &tpm1_dev_group;
>  }
> -- 
> 2.6.6
> 

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: [PATCH 2/2] tpm: support driver-specific sysfs attrs in tpm_tis_core

[Andrey Pronin]

On Mon, Jul 18, 2016 at 10:11:41PM +0300, Jarkko Sakkinen wrote:
> On Thu, Jul 14, 2016 at 06:51:36PM -0700, Andrey Pronin wrote:
> > Add attr_group to phy_ops that a driver relying on tpm_tis_core_init
> > can set to have its specific attributes registered in sysfs.
> > 
> > Signed-off-by: Andrey Pronin <apronin@chromium.org>
> > ---
> >  drivers/char/tpm/tpm-sysfs.c    | 1 -
> >  drivers/char/tpm/tpm.h          | 8 +++++++-
> >  drivers/char/tpm/tpm_tis_core.c | 3 +++
> >  drivers/char/tpm/tpm_tis_core.h | 1 +
> >  4 files changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> > index 95ce90d..22c9874 100644
> > --- a/drivers/char/tpm/tpm-sysfs.c
> > +++ b/drivers/char/tpm/tpm-sysfs.c
> > @@ -304,7 +304,6 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
> >  	 * is called before ops is null'd and the sysfs core synchronizes this
> >  	 * removal so that no callbacks are running or can run again
> >  	 */
> > -	WARN_ON(chip->groups_cnt != 0);
> 
> You should explain this in a commit message if you want to remove it.
> 
> In general, this make user space API vendor specific, which is
> unacceptable.
> 
> /Jarkko
>

I will drop the vendor-specific part of the patchset and just submit
tpm2-specific sysfs attributes in the next rev. WARN_ON will not be
removed.

Re: [PATCH 2/2] tpm: support driver-specific sysfs attrs in tpm_tis_core

[Andrey Pronin]

On Mon, Jul 18, 2016 at 10:11:41PM +0300, Jarkko Sakkinen wrote:
> On Thu, Jul 14, 2016 at 06:51:36PM -0700, Andrey Pronin wrote:
> > Add attr_group to phy_ops that a driver relying on tpm_tis_core_init
> > can set to have its specific attributes registered in sysfs.
> > 
> > Signed-off-by: Andrey Pronin <apronin-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > ---
> >  drivers/char/tpm/tpm-sysfs.c    | 1 -
> >  drivers/char/tpm/tpm.h          | 8 +++++++-
> >  drivers/char/tpm/tpm_tis_core.c | 3 +++
> >  drivers/char/tpm/tpm_tis_core.h | 1 +
> >  4 files changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> > index 95ce90d..22c9874 100644
> > --- a/drivers/char/tpm/tpm-sysfs.c
> > +++ b/drivers/char/tpm/tpm-sysfs.c
> > @@ -304,7 +304,6 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
> >  	 * is called before ops is null'd and the sysfs core synchronizes this
> >  	 * removal so that no callbacks are running or can run again
> >  	 */
> > -	WARN_ON(chip->groups_cnt != 0);
> 
> You should explain this in a commit message if you want to remove it.
> 
> In general, this make user space API vendor specific, which is
> unacceptable.
> 
> /Jarkko
>

I will drop the vendor-specific part of the patchset and just submit
tpm2-specific sysfs attributes in the next rev. WARN_ON will not be
removed.


------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: [PATCH 2/2] tpm: support driver-specific sysfs attrs in tpm_tis_core

[Jarkko Sakkinen]

On Thu, Jul 14, 2016 at 08:35:30PM -0700, Andrey Pronin wrote:
> On Thu, Jul 14, 2016 at 09:23:27PM -0600, Jason Gunthorpe wrote:
> > On Thu, Jul 14, 2016 at 06:51:36PM -0700, Andrey Pronin wrote:
> > > -	WARN_ON(chip->groups_cnt != 0);
> > 
> > Nope.
> > 
> > > -	const struct attribute_group *groups[3];
> > > +	/* up to 4 attribute groups:
> > > +	 * - driver-specific
> > > +	 * - common TPM1.2 and TPM2.0
> > > +	 * - TPM1.2/2.0-specific
> > > +	 * - ppi
> > > +	 */
> > > +	const struct attribute_group *groups[5];
> > 
> > The prior patch needed to have groups[4], every patch much work.
> > 
> > > +	if (priv->phy_ops->attr_group)
> > > +		chip->groups[chip->groups_cnt++] = priv->phy_ops->attr_group;
> > 
> > I am really not excited about having driver specific sysfs
> > files.
> > 
> > What is the justification for this?
> > 
> > Jason
> 
> Justification: give access to vendor-specific properties that are
> specific to a particular chip and its registers.

Please come with a vendor specific property or have this part of a
series where the need becomes somehow obvious so that we can talk about
a real problem and not in an abstract level.

Making user API vendor wobbling is almost over my dead body type of
thing but given the context there might be alternatives to consider.

I honestly don't understand why this was even bundled with TPM2 patch.

/Jarkko

Re: [PATCH 2/2] tpm: support driver-specific sysfs attrs in tpm_tis_core

[Jarkko Sakkinen]

On Thu, Jul 14, 2016 at 08:35:30PM -0700, Andrey Pronin wrote:
> On Thu, Jul 14, 2016 at 09:23:27PM -0600, Jason Gunthorpe wrote:
> > On Thu, Jul 14, 2016 at 06:51:36PM -0700, Andrey Pronin wrote:
> > > -	WARN_ON(chip->groups_cnt != 0);
> > 
> > Nope.
> > 
> > > -	const struct attribute_group *groups[3];
> > > +	/* up to 4 attribute groups:
> > > +	 * - driver-specific
> > > +	 * - common TPM1.2 and TPM2.0
> > > +	 * - TPM1.2/2.0-specific
> > > +	 * - ppi
> > > +	 */
> > > +	const struct attribute_group *groups[5];
> > 
> > The prior patch needed to have groups[4], every patch much work.
> > 
> > > +	if (priv->phy_ops->attr_group)
> > > +		chip->groups[chip->groups_cnt++] = priv->phy_ops->attr_group;
> > 
> > I am really not excited about having driver specific sysfs
> > files.
> > 
> > What is the justification for this?
> > 
> > Jason
> 
> Justification: give access to vendor-specific properties that are
> specific to a particular chip and its registers.

Please come with a vendor specific property or have this part of a
series where the need becomes somehow obvious so that we can talk about
a real problem and not in an abstract level.

Making user API vendor wobbling is almost over my dead body type of
thing but given the context there might be alternatives to consider.

I honestly don't understand why this was even bundled with TPM2 patch.

/Jarkko

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

[PATCH v2] tpm: add sysfs attributes for tpm2

[Andrey Pronin]

Add sysfs attributes in TPM2.0 case for:
 - TPM_PT_PERMANENT flags
 - TPM_PT_STARTUP_CLEAR flags
 - lockout-related properties

v2: Dropped adding driver-specific attributes.
    No legacy links for TPM2 attributes.
    All attributes created in groups[0].
    Added actual attributes for flags and lockout properties.

Signed-off-by: Andrey Pronin <apronin@chromium.org>
---
 drivers/char/tpm/tpm-chip.c  |   4 +-
 drivers/char/tpm/tpm-sysfs.c | 108 +++++++++++++++++++++++++++++++++++++++++--
 drivers/char/tpm/tpm.h       |  30 ++++++++++++
 3 files changed, 136 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index e595013..ede2ca0 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -281,8 +281,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		return 0;
 
-	tpm_sysfs_add_device(chip);
-
 	chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
 
 	return 0;
@@ -363,6 +361,8 @@ int tpm_chip_register(struct tpm_chip *chip)
 			return rc;
 	}
 
+	tpm_sysfs_add_device(chip);
+
 	rc = tpm1_chip_register(chip);
 	if (rc)
 		return rc;
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index b46cf70..fcfc7e0 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -264,7 +264,7 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(timeouts);
 
-static struct attribute *tpm_dev_attrs[] = {
+static struct attribute *tpm1_dev_attrs[] = {
 	&dev_attr_pubek.attr,
 	&dev_attr_pcrs.attr,
 	&dev_attr_enabled.attr,
@@ -278,8 +278,106 @@ static struct attribute *tpm_dev_attrs[] = {
 	NULL,
 };
 
-static const struct attribute_group tpm_dev_group = {
-	.attrs = tpm_dev_attrs,
+static const struct attribute_group tpm1_dev_group = {
+	.attrs = tpm1_dev_attrs,
+};
+
+static ssize_t tpm2_prop_flag_show(struct device *dev, u32 property_id,
+				   u32 flag_mask, char *buf)
+{
+	u32 flags;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(to_tpm_chip(dev), property_id, &flags,
+			     "reading property");
+	if (rc)
+		return 0;
+
+	return sprintf(buf, "%d\n", !!(flags & flag_mask));
+}
+
+static ssize_t tpm2_prop_u32_show(struct device *dev, u32 property_id,
+				  char *buf)
+{
+	u32 value;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(to_tpm_chip(dev), property_id, &value,
+			     "reading property");
+	if (rc)
+		return 0;
+
+	return sprintf(buf, "%u\n", value);
+}
+
+#define DEFINE_TPM2_PROP_FLAG_ATTR(name, property_id, flag_mask)      \
+static ssize_t name##_show(struct device *dev,                        \
+			   struct device_attribute *attr, char *buf)  \
+{                                                                     \
+	return tpm2_prop_flag_show(dev, property_id, flag_mask, buf); \
+}                                                                     \
+static DEVICE_ATTR_RO(name)
+
+#define DEFINE_TPM2_PROP_U32_ATTR(name, property_id)                  \
+static ssize_t name##_show(struct device *dev,                        \
+			   struct device_attribute *attr, char *buf)  \
+{                                                                     \
+	return tpm2_prop_u32_show(dev, property_id, buf);             \
+}                                                                     \
+static DEVICE_ATTR_RO(name)
+
+DEFINE_TPM2_PROP_FLAG_ATTR(owner_auth_set,
+			   TPM2_PT_PERMANENT, TPM2_ATTR_OWNER_AUTH_SET);
+DEFINE_TPM2_PROP_FLAG_ATTR(endorsement_auth_set,
+			   TPM2_PT_PERMANENT, TPM2_ATTR_ENDORSEMENT_AUTH_SET);
+DEFINE_TPM2_PROP_FLAG_ATTR(lockout_auth_set,
+			   TPM2_PT_PERMANENT, TPM2_ATTR_LOCKOUT_AUTH_SET);
+DEFINE_TPM2_PROP_FLAG_ATTR(disable_clear,
+			   TPM2_PT_PERMANENT, TPM2_ATTR_DISABLE_CLEAR);
+DEFINE_TPM2_PROP_FLAG_ATTR(in_lockout,
+			   TPM2_PT_PERMANENT, TPM2_ATTR_IN_LOCKOUT);
+DEFINE_TPM2_PROP_FLAG_ATTR(tpm_generated_eps,
+			   TPM2_PT_PERMANENT, TPM2_ATTR_TPM_GENERATED_EPS);
+
+DEFINE_TPM2_PROP_FLAG_ATTR(ph_enable,
+			   TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_PH_ENABLE);
+DEFINE_TPM2_PROP_FLAG_ATTR(sh_enable,
+			   TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_SH_ENABLE);
+DEFINE_TPM2_PROP_FLAG_ATTR(eh_enable,
+			   TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_EH_ENABLE);
+DEFINE_TPM2_PROP_FLAG_ATTR(ph_enable_nv,
+			   TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_PH_ENABLE_NV);
+DEFINE_TPM2_PROP_FLAG_ATTR(orderly,
+			   TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_ORDERLY);
+
+DEFINE_TPM2_PROP_U32_ATTR(lockout_counter, TPM2_PT_LOCKOUT_COUNTER);
+DEFINE_TPM2_PROP_U32_ATTR(max_auth_fail, TPM2_PT_MAX_AUTH_FAIL);
+DEFINE_TPM2_PROP_U32_ATTR(lockout_interval, TPM2_PT_LOCKOUT_INTERVAL);
+DEFINE_TPM2_PROP_U32_ATTR(lockout_recovery, TPM2_PT_LOCKOUT_RECOVERY);
+
+static struct attribute *tpm2_dev_attrs[] = {
+	&dev_attr_owner_auth_set.attr,
+	&dev_attr_endorsement_auth_set.attr,
+	&dev_attr_lockout_auth_set.attr,
+	&dev_attr_disable_clear.attr,
+	&dev_attr_in_lockout.attr,
+	&dev_attr_tpm_generated_eps.attr,
+	&dev_attr_ph_enable.attr,
+	&dev_attr_sh_enable.attr,
+	&dev_attr_eh_enable.attr,
+	&dev_attr_ph_enable_nv.attr,
+	&dev_attr_orderly.attr,
+	&dev_attr_lockout_counter.attr,
+	&dev_attr_max_auth_fail.attr,
+	&dev_attr_lockout_interval.attr,
+	&dev_attr_lockout_recovery.attr,
+	&dev_attr_durations.attr,
+	&dev_attr_timeouts.attr,
+	NULL,
+};
+
+static const struct attribute_group tpm2_dev_group = {
+	.attrs = tpm2_dev_attrs,
 };
 
 void tpm_sysfs_add_device(struct tpm_chip *chip)
@@ -289,5 +387,7 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
 	 * removal so that no callbacks are running or can run again
 	 */
 	WARN_ON(chip->groups_cnt != 0);
-	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
+	chip->groups[chip->groups_cnt++] =
+		(chip->flags & TPM_CHIP_FLAG_TPM2) ?
+		&tpm2_dev_group : &tpm1_dev_group;
 }
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 3e32d5b..cf4359a 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -127,6 +127,36 @@ enum tpm2_capabilities {
 	TPM2_CAP_TPM_PROPERTIES = 6,
 };
 
+enum tpm2_properties {
+	TPM2_PT_NONE			= 0,
+	TPM2_PT_GROUP			= 0x100,
+	TPM2_PT_FIXED			= TPM2_PT_GROUP,
+	TPM2_PT_VAR			= TPM2_PT_GROUP * 2,
+	TPM2_PT_PERMANENT		= TPM2_PT_VAR + 0,
+	TPM2_PT_STARTUP_CLEAR		= TPM2_PT_VAR + 1,
+	TPM2_PT_LOCKOUT_COUNTER		= TPM2_PT_VAR + 14,
+	TPM2_PT_MAX_AUTH_FAIL		= TPM2_PT_VAR + 15,
+	TPM2_PT_LOCKOUT_INTERVAL	= TPM2_PT_VAR + 16,
+	TPM2_PT_LOCKOUT_RECOVERY	= TPM2_PT_VAR + 17,
+};
+
+enum tpm2_attr_permanent {
+	TPM2_ATTR_OWNER_AUTH_SET	= BIT(0),
+	TPM2_ATTR_ENDORSEMENT_AUTH_SET	= BIT(1),
+	TPM2_ATTR_LOCKOUT_AUTH_SET	= BIT(2),
+	TPM2_ATTR_DISABLE_CLEAR		= BIT(8),
+	TPM2_ATTR_IN_LOCKOUT		= BIT(9),
+	TPM2_ATTR_TPM_GENERATED_EPS	= BIT(10),
+};
+
+enum tpm2_attr_startup_clear {
+	TPM2_ATTR_PH_ENABLE		= BIT(0),
+	TPM2_ATTR_SH_ENABLE		= BIT(1),
+	TPM2_ATTR_EH_ENABLE		= BIT(2),
+	TPM2_ATTR_PH_ENABLE_NV		= BIT(3),
+	TPM2_ATTR_ORDERLY		= BIT(31),
+};
+
 enum tpm2_startup_types {
 	TPM2_SU_CLEAR	= 0x0000,
 	TPM2_SU_STATE	= 0x0001,
-- 
2.6.6

[PATCH v2] tpm: add sysfs attributes for tpm2

[Andrey Pronin]

Add sysfs attributes in TPM2.0 case for:
 - TPM_PT_PERMANENT flags
 - TPM_PT_STARTUP_CLEAR flags
 - lockout-related properties

v2: Dropped adding driver-specific attributes.
    No legacy links for TPM2 attributes.
    All attributes created in groups[0].
    Added actual attributes for flags and lockout properties.

Signed-off-by: Andrey Pronin <apronin-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 drivers/char/tpm/tpm-chip.c  |   4 +-
 drivers/char/tpm/tpm-sysfs.c | 108 +++++++++++++++++++++++++++++++++++++++++--
 drivers/char/tpm/tpm.h       |  30 ++++++++++++
 3 files changed, 136 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index e595013..ede2ca0 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -281,8 +281,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		return 0;
 
-	tpm_sysfs_add_device(chip);
-
 	chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
 
 	return 0;
@@ -363,6 +361,8 @@ int tpm_chip_register(struct tpm_chip *chip)
 			return rc;
 	}
 
+	tpm_sysfs_add_device(chip);
+
 	rc = tpm1_chip_register(chip);
 	if (rc)
 		return rc;
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index b46cf70..fcfc7e0 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -264,7 +264,7 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(timeouts);
 
-static struct attribute *tpm_dev_attrs[] = {
+static struct attribute *tpm1_dev_attrs[] = {
 	&dev_attr_pubek.attr,
 	&dev_attr_pcrs.attr,
 	&dev_attr_enabled.attr,
@@ -278,8 +278,106 @@ static struct attribute *tpm_dev_attrs[] = {
 	NULL,
 };
 
-static const struct attribute_group tpm_dev_group = {
-	.attrs = tpm_dev_attrs,
+static const struct attribute_group tpm1_dev_group = {
+	.attrs = tpm1_dev_attrs,
+};
+
+static ssize_t tpm2_prop_flag_show(struct device *dev, u32 property_id,
+				   u32 flag_mask, char *buf)
+{
+	u32 flags;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(to_tpm_chip(dev), property_id, &flags,
+			     "reading property");
+	if (rc)
+		return 0;
+
+	return sprintf(buf, "%d\n", !!(flags & flag_mask));
+}
+
+static ssize_t tpm2_prop_u32_show(struct device *dev, u32 property_id,
+				  char *buf)
+{
+	u32 value;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(to_tpm_chip(dev), property_id, &value,
+			     "reading property");
+	if (rc)
+		return 0;
+
+	return sprintf(buf, "%u\n", value);
+}
+
+#define DEFINE_TPM2_PROP_FLAG_ATTR(name, property_id, flag_mask)      \
+static ssize_t name##_show(struct device *dev,                        \
+			   struct device_attribute *attr, char *buf)  \
+{                                                                     \
+	return tpm2_prop_flag_show(dev, property_id, flag_mask, buf); \
+}                                                                     \
+static DEVICE_ATTR_RO(name)
+
+#define DEFINE_TPM2_PROP_U32_ATTR(name, property_id)                  \
+static ssize_t name##_show(struct device *dev,                        \
+			   struct device_attribute *attr, char *buf)  \
+{                                                                     \
+	return tpm2_prop_u32_show(dev, property_id, buf);             \
+}                                                                     \
+static DEVICE_ATTR_RO(name)
+
+DEFINE_TPM2_PROP_FLAG_ATTR(owner_auth_set,
+			   TPM2_PT_PERMANENT, TPM2_ATTR_OWNER_AUTH_SET);
+DEFINE_TPM2_PROP_FLAG_ATTR(endorsement_auth_set,
+			   TPM2_PT_PERMANENT, TPM2_ATTR_ENDORSEMENT_AUTH_SET);
+DEFINE_TPM2_PROP_FLAG_ATTR(lockout_auth_set,
+			   TPM2_PT_PERMANENT, TPM2_ATTR_LOCKOUT_AUTH_SET);
+DEFINE_TPM2_PROP_FLAG_ATTR(disable_clear,
+			   TPM2_PT_PERMANENT, TPM2_ATTR_DISABLE_CLEAR);
+DEFINE_TPM2_PROP_FLAG_ATTR(in_lockout,
+			   TPM2_PT_PERMANENT, TPM2_ATTR_IN_LOCKOUT);
+DEFINE_TPM2_PROP_FLAG_ATTR(tpm_generated_eps,
+			   TPM2_PT_PERMANENT, TPM2_ATTR_TPM_GENERATED_EPS);
+
+DEFINE_TPM2_PROP_FLAG_ATTR(ph_enable,
+			   TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_PH_ENABLE);
+DEFINE_TPM2_PROP_FLAG_ATTR(sh_enable,
+			   TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_SH_ENABLE);
+DEFINE_TPM2_PROP_FLAG_ATTR(eh_enable,
+			   TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_EH_ENABLE);
+DEFINE_TPM2_PROP_FLAG_ATTR(ph_enable_nv,
+			   TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_PH_ENABLE_NV);
+DEFINE_TPM2_PROP_FLAG_ATTR(orderly,
+			   TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_ORDERLY);
+
+DEFINE_TPM2_PROP_U32_ATTR(lockout_counter, TPM2_PT_LOCKOUT_COUNTER);
+DEFINE_TPM2_PROP_U32_ATTR(max_auth_fail, TPM2_PT_MAX_AUTH_FAIL);
+DEFINE_TPM2_PROP_U32_ATTR(lockout_interval, TPM2_PT_LOCKOUT_INTERVAL);
+DEFINE_TPM2_PROP_U32_ATTR(lockout_recovery, TPM2_PT_LOCKOUT_RECOVERY);
+
+static struct attribute *tpm2_dev_attrs[] = {
+	&dev_attr_owner_auth_set.attr,
+	&dev_attr_endorsement_auth_set.attr,
+	&dev_attr_lockout_auth_set.attr,
+	&dev_attr_disable_clear.attr,
+	&dev_attr_in_lockout.attr,
+	&dev_attr_tpm_generated_eps.attr,
+	&dev_attr_ph_enable.attr,
+	&dev_attr_sh_enable.attr,
+	&dev_attr_eh_enable.attr,
+	&dev_attr_ph_enable_nv.attr,
+	&dev_attr_orderly.attr,
+	&dev_attr_lockout_counter.attr,
+	&dev_attr_max_auth_fail.attr,
+	&dev_attr_lockout_interval.attr,
+	&dev_attr_lockout_recovery.attr,
+	&dev_attr_durations.attr,
+	&dev_attr_timeouts.attr,
+	NULL,
+};
+
+static const struct attribute_group tpm2_dev_group = {
+	.attrs = tpm2_dev_attrs,
 };
 
 void tpm_sysfs_add_device(struct tpm_chip *chip)
@@ -289,5 +387,7 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
 	 * removal so that no callbacks are running or can run again
 	 */
 	WARN_ON(chip->groups_cnt != 0);
-	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
+	chip->groups[chip->groups_cnt++] =
+		(chip->flags & TPM_CHIP_FLAG_TPM2) ?
+		&tpm2_dev_group : &tpm1_dev_group;
 }
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 3e32d5b..cf4359a 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -127,6 +127,36 @@ enum tpm2_capabilities {
 	TPM2_CAP_TPM_PROPERTIES = 6,
 };
 
+enum tpm2_properties {
+	TPM2_PT_NONE			= 0,
+	TPM2_PT_GROUP			= 0x100,
+	TPM2_PT_FIXED			= TPM2_PT_GROUP,
+	TPM2_PT_VAR			= TPM2_PT_GROUP * 2,
+	TPM2_PT_PERMANENT		= TPM2_PT_VAR + 0,
+	TPM2_PT_STARTUP_CLEAR		= TPM2_PT_VAR + 1,
+	TPM2_PT_LOCKOUT_COUNTER		= TPM2_PT_VAR + 14,
+	TPM2_PT_MAX_AUTH_FAIL		= TPM2_PT_VAR + 15,
+	TPM2_PT_LOCKOUT_INTERVAL	= TPM2_PT_VAR + 16,
+	TPM2_PT_LOCKOUT_RECOVERY	= TPM2_PT_VAR + 17,
+};
+
+enum tpm2_attr_permanent {
+	TPM2_ATTR_OWNER_AUTH_SET	= BIT(0),
+	TPM2_ATTR_ENDORSEMENT_AUTH_SET	= BIT(1),
+	TPM2_ATTR_LOCKOUT_AUTH_SET	= BIT(2),
+	TPM2_ATTR_DISABLE_CLEAR		= BIT(8),
+	TPM2_ATTR_IN_LOCKOUT		= BIT(9),
+	TPM2_ATTR_TPM_GENERATED_EPS	= BIT(10),
+};
+
+enum tpm2_attr_startup_clear {
+	TPM2_ATTR_PH_ENABLE		= BIT(0),
+	TPM2_ATTR_SH_ENABLE		= BIT(1),
+	TPM2_ATTR_EH_ENABLE		= BIT(2),
+	TPM2_ATTR_PH_ENABLE_NV		= BIT(3),
+	TPM2_ATTR_ORDERLY		= BIT(31),
+};
+
 enum tpm2_startup_types {
 	TPM2_SU_CLEAR	= 0x0000,
 	TPM2_SU_STATE	= 0x0001,
-- 
2.6.6


------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: [PATCH v2] tpm: add sysfs attributes for tpm2

[Jason Gunthorpe]

On Tue, Jul 19, 2016 at 07:51:52PM -0700, Andrey Pronin wrote:
> Add sysfs attributes in TPM2.0 case for:
>  - TPM_PT_PERMANENT flags
>  - TPM_PT_STARTUP_CLEAR flags
>  - lockout-related properties

I'm not completely sure we need to have these sysfs attributes. Do you
have a reason to expose them? Does udev do something based on them? Is
it just for debugging?

Otherwise it looks about right to me.

Jason

Re: [PATCH v2] tpm: add sysfs attributes for tpm2

[Jason Gunthorpe]

On Tue, Jul 19, 2016 at 07:51:52PM -0700, Andrey Pronin wrote:
> Add sysfs attributes in TPM2.0 case for:
>  - TPM_PT_PERMANENT flags
>  - TPM_PT_STARTUP_CLEAR flags
>  - lockout-related properties

I'm not completely sure we need to have these sysfs attributes. Do you
have a reason to expose them? Does udev do something based on them? Is
it just for debugging?

Otherwise it looks about right to me.

Jason

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: [PATCH v2] tpm: add sysfs attributes for tpm2

[Andrey Pronin]

On Wed, Jul 20, 2016 at 11:05:53AM -0600, Jason Gunthorpe wrote:
> On Tue, Jul 19, 2016 at 07:51:52PM -0700, Andrey Pronin wrote:
> > Add sysfs attributes in TPM2.0 case for:
> >  - TPM_PT_PERMANENT flags
> >  - TPM_PT_STARTUP_CLEAR flags
> >  - lockout-related properties
> 
> I'm not completely sure we need to have these sysfs attributes. Do you
> have a reason to expose them? Does udev do something based on them? Is
> it just for debugging?
> 
> Otherwise it looks about right to me.
> 

In practice, useful for scripts that monitor in what state
the system started, was there a lockout, can we use tpm for
attestation, can we rely on data stored in NVRAM, etc. And
then interact with the user accordingly.

I don't know of udev rules that do anything based on them,
but in a multi-tpm system, I can envision one that selects
the tpm that was actually used by firmware as the primary one,
or controls permissions for the device based on the state it's
in.

For TPM1.2 we expose some flags from TPM_PERMANENT_FLAGS and
TPM_CAP_PROP_OWNER, for example, to show if the tpm is owned,
enabled etc. A combination of ph/eh/shEnable and *AuthSet flags
from TPM2 provides info allowing to make similar decisions about
the tpm being 'owned' or 'enabled' for userland scripts.

Andrey

Re: [PATCH v2] tpm: add sysfs attributes for tpm2

[Andrey Pronin]

On Wed, Jul 20, 2016 at 11:05:53AM -0600, Jason Gunthorpe wrote:
> On Tue, Jul 19, 2016 at 07:51:52PM -0700, Andrey Pronin wrote:
> > Add sysfs attributes in TPM2.0 case for:
> >  - TPM_PT_PERMANENT flags
> >  - TPM_PT_STARTUP_CLEAR flags
> >  - lockout-related properties
> 
> I'm not completely sure we need to have these sysfs attributes. Do you
> have a reason to expose them? Does udev do something based on them? Is
> it just for debugging?
> 
> Otherwise it looks about right to me.
> 

In practice, useful for scripts that monitor in what state
the system started, was there a lockout, can we use tpm for
attestation, can we rely on data stored in NVRAM, etc. And
then interact with the user accordingly.

I don't know of udev rules that do anything based on them,
but in a multi-tpm system, I can envision one that selects
the tpm that was actually used by firmware as the primary one,
or controls permissions for the device based on the state it's
in.

For TPM1.2 we expose some flags from TPM_PERMANENT_FLAGS and
TPM_CAP_PROP_OWNER, for example, to show if the tpm is owned,
enabled etc. A combination of ph/eh/shEnable and *AuthSet flags
from TPM2 provides info allowing to make similar decisions about
the tpm being 'owned' or 'enabled' for userland scripts.

Andrey

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

[PATCH v3] tpm: add sysfs attributes for tpm2

[Andrey Pronin]

Add sysfs attributes in TPM2.0 case for:
 - TPM_PT_PERMANENT flags
 - TPM_PT_STARTUP_CLEAR flags
 - lockout-related properties

v2: Dropped adding driver-specific attributes.
    No legacy links for TPM2 attributes.
    All attributes created in groups[0].
    Added actual attributes for flags and lockout properties.

v3: Avoid creating a separate 'show' function for each attribute.

Signed-off-by: Andrey Pronin <apronin@chromium.org>
---
 drivers/char/tpm/tpm-chip.c  |   4 +-
 drivers/char/tpm/tpm-sysfs.c | 122 +++++++++++++++++++++++++++++++++++++++++--
 drivers/char/tpm/tpm.h       |  30 +++++++++++
 3 files changed, 150 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index e595013..ede2ca0 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -281,8 +281,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		return 0;
 
-	tpm_sysfs_add_device(chip);
-
 	chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
 
 	return 0;
@@ -363,6 +361,8 @@ int tpm_chip_register(struct tpm_chip *chip)
 			return rc;
 	}
 
+	tpm_sysfs_add_device(chip);
+
 	rc = tpm1_chip_register(chip);
 	if (rc)
 		return rc;
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index b46cf70..62940ef 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -264,7 +264,7 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(timeouts);
 
-static struct attribute *tpm_dev_attrs[] = {
+static struct attribute *tpm1_dev_attrs[] = {
 	&dev_attr_pubek.attr,
 	&dev_attr_pcrs.attr,
 	&dev_attr_enabled.attr,
@@ -278,8 +278,120 @@ static struct attribute *tpm_dev_attrs[] = {
 	NULL,
 };
 
-static const struct attribute_group tpm_dev_group = {
-	.attrs = tpm_dev_attrs,
+static const struct attribute_group tpm1_dev_group = {
+	.attrs = tpm1_dev_attrs,
+};
+
+struct tpm2_prop_flag_dev_attribute {
+	struct device_attribute attr;
+	u32 property_id;
+	u32 flag_mask;
+};
+
+struct tpm2_prop_u32_dev_attribute {
+	struct device_attribute attr;
+	u32 property_id;
+};
+
+static ssize_t tpm2_prop_flag_show(struct device *dev,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	struct tpm2_prop_flag_dev_attribute *pa =
+		container_of(attr, struct tpm2_prop_flag_dev_attribute, attr);
+	u32 flags;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(to_tpm_chip(dev), pa->property_id, &flags,
+			     "reading property");
+	if (rc)
+		return 0;
+
+	return sprintf(buf, "%d\n", !!(flags & pa->flag_mask));
+}
+
+static ssize_t tpm2_prop_u32_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	struct tpm2_prop_u32_dev_attribute *pa =
+		container_of(attr, struct tpm2_prop_u32_dev_attribute, attr);
+	u32 value;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(to_tpm_chip(dev), pa->property_id, &value,
+			     "reading property");
+	if (rc)
+		return 0;
+
+	return sprintf(buf, "%u\n", value);
+}
+
+#define TPM2_PROP_FLAG_ATTR(_name, _property_id, _flag_mask)           \
+	struct tpm2_prop_flag_dev_attribute attr_tpm2_prop_##_name = { \
+		__ATTR(_name, S_IRUGO, tpm2_prop_flag_show, NULL),     \
+		_property_id, _flag_mask                               \
+	}
+
+#define TPM2_PROP_U32_ATTR(_name, _property_id)                        \
+	struct tpm2_prop_u32_dev_attribute attr_tpm2_prop_##_name = {  \
+		__ATTR(_name, S_IRUGO, tpm2_prop_u32_show, NULL),      \
+		_property_id                                           \
+	}
+
+TPM2_PROP_FLAG_ATTR(owner_auth_set,
+		    TPM2_PT_PERMANENT, TPM2_ATTR_OWNER_AUTH_SET);
+TPM2_PROP_FLAG_ATTR(endorsement_auth_set,
+		    TPM2_PT_PERMANENT, TPM2_ATTR_ENDORSEMENT_AUTH_SET);
+TPM2_PROP_FLAG_ATTR(lockout_auth_set,
+		    TPM2_PT_PERMANENT, TPM2_ATTR_LOCKOUT_AUTH_SET);
+TPM2_PROP_FLAG_ATTR(disable_clear,
+		    TPM2_PT_PERMANENT, TPM2_ATTR_DISABLE_CLEAR);
+TPM2_PROP_FLAG_ATTR(in_lockout,
+		    TPM2_PT_PERMANENT, TPM2_ATTR_IN_LOCKOUT);
+TPM2_PROP_FLAG_ATTR(tpm_generated_eps,
+		    TPM2_PT_PERMANENT, TPM2_ATTR_TPM_GENERATED_EPS);
+
+TPM2_PROP_FLAG_ATTR(ph_enable,
+		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_PH_ENABLE);
+TPM2_PROP_FLAG_ATTR(sh_enable,
+		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_SH_ENABLE);
+TPM2_PROP_FLAG_ATTR(eh_enable,
+		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_EH_ENABLE);
+TPM2_PROP_FLAG_ATTR(ph_enable_nv,
+		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_PH_ENABLE_NV);
+TPM2_PROP_FLAG_ATTR(orderly,
+		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_ORDERLY);
+
+TPM2_PROP_U32_ATTR(lockout_counter, TPM2_PT_LOCKOUT_COUNTER);
+TPM2_PROP_U32_ATTR(max_auth_fail, TPM2_PT_MAX_AUTH_FAIL);
+TPM2_PROP_U32_ATTR(lockout_interval, TPM2_PT_LOCKOUT_INTERVAL);
+TPM2_PROP_U32_ATTR(lockout_recovery, TPM2_PT_LOCKOUT_RECOVERY);
+
+#define ATTR_FOR_TPM2_PROP(_name) (&attr_tpm2_prop_##_name.attr.attr)
+static struct attribute *tpm2_dev_attrs[] = {
+	ATTR_FOR_TPM2_PROP(owner_auth_set),
+	ATTR_FOR_TPM2_PROP(endorsement_auth_set),
+	ATTR_FOR_TPM2_PROP(lockout_auth_set),
+	ATTR_FOR_TPM2_PROP(disable_clear),
+	ATTR_FOR_TPM2_PROP(in_lockout),
+	ATTR_FOR_TPM2_PROP(tpm_generated_eps),
+	ATTR_FOR_TPM2_PROP(ph_enable),
+	ATTR_FOR_TPM2_PROP(sh_enable),
+	ATTR_FOR_TPM2_PROP(eh_enable),
+	ATTR_FOR_TPM2_PROP(ph_enable_nv),
+	ATTR_FOR_TPM2_PROP(orderly),
+	ATTR_FOR_TPM2_PROP(lockout_counter),
+	ATTR_FOR_TPM2_PROP(max_auth_fail),
+	ATTR_FOR_TPM2_PROP(lockout_interval),
+	ATTR_FOR_TPM2_PROP(lockout_recovery),
+	&dev_attr_durations.attr,
+	&dev_attr_timeouts.attr,
+	NULL,
+};
+
+static const struct attribute_group tpm2_dev_group = {
+	.attrs = tpm2_dev_attrs,
 };
 
 void tpm_sysfs_add_device(struct tpm_chip *chip)
@@ -289,5 +401,7 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
 	 * removal so that no callbacks are running or can run again
 	 */
 	WARN_ON(chip->groups_cnt != 0);
-	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
+	chip->groups[chip->groups_cnt++] =
+		(chip->flags & TPM_CHIP_FLAG_TPM2) ?
+		&tpm2_dev_group : &tpm1_dev_group;
 }
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 6e002c4..9feb023 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -127,6 +127,36 @@ enum tpm2_capabilities {
 	TPM2_CAP_TPM_PROPERTIES = 6,
 };
 
+enum tpm2_properties {
+	TPM2_PT_NONE			= 0,
+	TPM2_PT_GROUP			= 0x100,
+	TPM2_PT_FIXED			= TPM2_PT_GROUP,
+	TPM2_PT_VAR			= TPM2_PT_GROUP * 2,
+	TPM2_PT_PERMANENT		= TPM2_PT_VAR + 0,
+	TPM2_PT_STARTUP_CLEAR		= TPM2_PT_VAR + 1,
+	TPM2_PT_LOCKOUT_COUNTER		= TPM2_PT_VAR + 14,
+	TPM2_PT_MAX_AUTH_FAIL		= TPM2_PT_VAR + 15,
+	TPM2_PT_LOCKOUT_INTERVAL	= TPM2_PT_VAR + 16,
+	TPM2_PT_LOCKOUT_RECOVERY	= TPM2_PT_VAR + 17,
+};
+
+enum tpm2_attr_permanent {
+	TPM2_ATTR_OWNER_AUTH_SET	= BIT(0),
+	TPM2_ATTR_ENDORSEMENT_AUTH_SET	= BIT(1),
+	TPM2_ATTR_LOCKOUT_AUTH_SET	= BIT(2),
+	TPM2_ATTR_DISABLE_CLEAR		= BIT(8),
+	TPM2_ATTR_IN_LOCKOUT		= BIT(9),
+	TPM2_ATTR_TPM_GENERATED_EPS	= BIT(10),
+};
+
+enum tpm2_attr_startup_clear {
+	TPM2_ATTR_PH_ENABLE		= BIT(0),
+	TPM2_ATTR_SH_ENABLE		= BIT(1),
+	TPM2_ATTR_EH_ENABLE		= BIT(2),
+	TPM2_ATTR_PH_ENABLE_NV		= BIT(3),
+	TPM2_ATTR_ORDERLY		= BIT(31),
+};
+
 enum tpm2_startup_types {
 	TPM2_SU_CLEAR	= 0x0000,
 	TPM2_SU_STATE	= 0x0001,
-- 
2.6.6

[PATCH v3] tpm: add sysfs attributes for tpm2

[Andrey Pronin]

Add sysfs attributes in TPM2.0 case for:
 - TPM_PT_PERMANENT flags
 - TPM_PT_STARTUP_CLEAR flags
 - lockout-related properties

v2: Dropped adding driver-specific attributes.
    No legacy links for TPM2 attributes.
    All attributes created in groups[0].
    Added actual attributes for flags and lockout properties.

v3: Avoid creating a separate 'show' function for each attribute.

Signed-off-by: Andrey Pronin <apronin-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 drivers/char/tpm/tpm-chip.c  |   4 +-
 drivers/char/tpm/tpm-sysfs.c | 122 +++++++++++++++++++++++++++++++++++++++++--
 drivers/char/tpm/tpm.h       |  30 +++++++++++
 3 files changed, 150 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index e595013..ede2ca0 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -281,8 +281,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		return 0;
 
-	tpm_sysfs_add_device(chip);
-
 	chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
 
 	return 0;
@@ -363,6 +361,8 @@ int tpm_chip_register(struct tpm_chip *chip)
 			return rc;
 	}
 
+	tpm_sysfs_add_device(chip);
+
 	rc = tpm1_chip_register(chip);
 	if (rc)
 		return rc;
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index b46cf70..62940ef 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -264,7 +264,7 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(timeouts);
 
-static struct attribute *tpm_dev_attrs[] = {
+static struct attribute *tpm1_dev_attrs[] = {
 	&dev_attr_pubek.attr,
 	&dev_attr_pcrs.attr,
 	&dev_attr_enabled.attr,
@@ -278,8 +278,120 @@ static struct attribute *tpm_dev_attrs[] = {
 	NULL,
 };
 
-static const struct attribute_group tpm_dev_group = {
-	.attrs = tpm_dev_attrs,
+static const struct attribute_group tpm1_dev_group = {
+	.attrs = tpm1_dev_attrs,
+};
+
+struct tpm2_prop_flag_dev_attribute {
+	struct device_attribute attr;
+	u32 property_id;
+	u32 flag_mask;
+};
+
+struct tpm2_prop_u32_dev_attribute {
+	struct device_attribute attr;
+	u32 property_id;
+};
+
+static ssize_t tpm2_prop_flag_show(struct device *dev,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	struct tpm2_prop_flag_dev_attribute *pa =
+		container_of(attr, struct tpm2_prop_flag_dev_attribute, attr);
+	u32 flags;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(to_tpm_chip(dev), pa->property_id, &flags,
+			     "reading property");
+	if (rc)
+		return 0;
+
+	return sprintf(buf, "%d\n", !!(flags & pa->flag_mask));
+}
+
+static ssize_t tpm2_prop_u32_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	struct tpm2_prop_u32_dev_attribute *pa =
+		container_of(attr, struct tpm2_prop_u32_dev_attribute, attr);
+	u32 value;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(to_tpm_chip(dev), pa->property_id, &value,
+			     "reading property");
+	if (rc)
+		return 0;
+
+	return sprintf(buf, "%u\n", value);
+}
+
+#define TPM2_PROP_FLAG_ATTR(_name, _property_id, _flag_mask)           \
+	struct tpm2_prop_flag_dev_attribute attr_tpm2_prop_##_name = { \
+		__ATTR(_name, S_IRUGO, tpm2_prop_flag_show, NULL),     \
+		_property_id, _flag_mask                               \
+	}
+
+#define TPM2_PROP_U32_ATTR(_name, _property_id)                        \
+	struct tpm2_prop_u32_dev_attribute attr_tpm2_prop_##_name = {  \
+		__ATTR(_name, S_IRUGO, tpm2_prop_u32_show, NULL),      \
+		_property_id                                           \
+	}
+
+TPM2_PROP_FLAG_ATTR(owner_auth_set,
+		    TPM2_PT_PERMANENT, TPM2_ATTR_OWNER_AUTH_SET);
+TPM2_PROP_FLAG_ATTR(endorsement_auth_set,
+		    TPM2_PT_PERMANENT, TPM2_ATTR_ENDORSEMENT_AUTH_SET);
+TPM2_PROP_FLAG_ATTR(lockout_auth_set,
+		    TPM2_PT_PERMANENT, TPM2_ATTR_LOCKOUT_AUTH_SET);
+TPM2_PROP_FLAG_ATTR(disable_clear,
+		    TPM2_PT_PERMANENT, TPM2_ATTR_DISABLE_CLEAR);
+TPM2_PROP_FLAG_ATTR(in_lockout,
+		    TPM2_PT_PERMANENT, TPM2_ATTR_IN_LOCKOUT);
+TPM2_PROP_FLAG_ATTR(tpm_generated_eps,
+		    TPM2_PT_PERMANENT, TPM2_ATTR_TPM_GENERATED_EPS);
+
+TPM2_PROP_FLAG_ATTR(ph_enable,
+		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_PH_ENABLE);
+TPM2_PROP_FLAG_ATTR(sh_enable,
+		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_SH_ENABLE);
+TPM2_PROP_FLAG_ATTR(eh_enable,
+		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_EH_ENABLE);
+TPM2_PROP_FLAG_ATTR(ph_enable_nv,
+		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_PH_ENABLE_NV);
+TPM2_PROP_FLAG_ATTR(orderly,
+		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_ORDERLY);
+
+TPM2_PROP_U32_ATTR(lockout_counter, TPM2_PT_LOCKOUT_COUNTER);
+TPM2_PROP_U32_ATTR(max_auth_fail, TPM2_PT_MAX_AUTH_FAIL);
+TPM2_PROP_U32_ATTR(lockout_interval, TPM2_PT_LOCKOUT_INTERVAL);
+TPM2_PROP_U32_ATTR(lockout_recovery, TPM2_PT_LOCKOUT_RECOVERY);
+
+#define ATTR_FOR_TPM2_PROP(_name) (&attr_tpm2_prop_##_name.attr.attr)
+static struct attribute *tpm2_dev_attrs[] = {
+	ATTR_FOR_TPM2_PROP(owner_auth_set),
+	ATTR_FOR_TPM2_PROP(endorsement_auth_set),
+	ATTR_FOR_TPM2_PROP(lockout_auth_set),
+	ATTR_FOR_TPM2_PROP(disable_clear),
+	ATTR_FOR_TPM2_PROP(in_lockout),
+	ATTR_FOR_TPM2_PROP(tpm_generated_eps),
+	ATTR_FOR_TPM2_PROP(ph_enable),
+	ATTR_FOR_TPM2_PROP(sh_enable),
+	ATTR_FOR_TPM2_PROP(eh_enable),
+	ATTR_FOR_TPM2_PROP(ph_enable_nv),
+	ATTR_FOR_TPM2_PROP(orderly),
+	ATTR_FOR_TPM2_PROP(lockout_counter),
+	ATTR_FOR_TPM2_PROP(max_auth_fail),
+	ATTR_FOR_TPM2_PROP(lockout_interval),
+	ATTR_FOR_TPM2_PROP(lockout_recovery),
+	&dev_attr_durations.attr,
+	&dev_attr_timeouts.attr,
+	NULL,
+};
+
+static const struct attribute_group tpm2_dev_group = {
+	.attrs = tpm2_dev_attrs,
 };
 
 void tpm_sysfs_add_device(struct tpm_chip *chip)
@@ -289,5 +401,7 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
 	 * removal so that no callbacks are running or can run again
 	 */
 	WARN_ON(chip->groups_cnt != 0);
-	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
+	chip->groups[chip->groups_cnt++] =
+		(chip->flags & TPM_CHIP_FLAG_TPM2) ?
+		&tpm2_dev_group : &tpm1_dev_group;
 }
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 6e002c4..9feb023 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -127,6 +127,36 @@ enum tpm2_capabilities {
 	TPM2_CAP_TPM_PROPERTIES = 6,
 };
 
+enum tpm2_properties {
+	TPM2_PT_NONE			= 0,
+	TPM2_PT_GROUP			= 0x100,
+	TPM2_PT_FIXED			= TPM2_PT_GROUP,
+	TPM2_PT_VAR			= TPM2_PT_GROUP * 2,
+	TPM2_PT_PERMANENT		= TPM2_PT_VAR + 0,
+	TPM2_PT_STARTUP_CLEAR		= TPM2_PT_VAR + 1,
+	TPM2_PT_LOCKOUT_COUNTER		= TPM2_PT_VAR + 14,
+	TPM2_PT_MAX_AUTH_FAIL		= TPM2_PT_VAR + 15,
+	TPM2_PT_LOCKOUT_INTERVAL	= TPM2_PT_VAR + 16,
+	TPM2_PT_LOCKOUT_RECOVERY	= TPM2_PT_VAR + 17,
+};
+
+enum tpm2_attr_permanent {
+	TPM2_ATTR_OWNER_AUTH_SET	= BIT(0),
+	TPM2_ATTR_ENDORSEMENT_AUTH_SET	= BIT(1),
+	TPM2_ATTR_LOCKOUT_AUTH_SET	= BIT(2),
+	TPM2_ATTR_DISABLE_CLEAR		= BIT(8),
+	TPM2_ATTR_IN_LOCKOUT		= BIT(9),
+	TPM2_ATTR_TPM_GENERATED_EPS	= BIT(10),
+};
+
+enum tpm2_attr_startup_clear {
+	TPM2_ATTR_PH_ENABLE		= BIT(0),
+	TPM2_ATTR_SH_ENABLE		= BIT(1),
+	TPM2_ATTR_EH_ENABLE		= BIT(2),
+	TPM2_ATTR_PH_ENABLE_NV		= BIT(3),
+	TPM2_ATTR_ORDERLY		= BIT(31),
+};
+
 enum tpm2_startup_types {
 	TPM2_SU_CLEAR	= 0x0000,
 	TPM2_SU_STATE	= 0x0001,
-- 
2.6.6


------------------------------------------------------------------------------

Re: [PATCH v3] tpm: add sysfs attributes for tpm2

[Jarkko Sakkinen]

This commit has a really nice changelog in the long description but
completely lacks why and how. Just by reading the description it would
be nice to know why you want to do this and by what means this commit
reaches that goal.

/Jarkko

On Wed, Jul 27, 2016 at 09:06:06PM -0700, Andrey Pronin wrote:
> Add sysfs attributes in TPM2.0 case for:
>  - TPM_PT_PERMANENT flags
>  - TPM_PT_STARTUP_CLEAR flags
>  - lockout-related properties
> 
> v2: Dropped adding driver-specific attributes.
>     No legacy links for TPM2 attributes.
>     All attributes created in groups[0].
>     Added actual attributes for flags and lockout properties.
> 
> v3: Avoid creating a separate 'show' function for each attribute.
> 
> Signed-off-by: Andrey Pronin <apronin@chromium.org>
> ---
>  drivers/char/tpm/tpm-chip.c  |   4 +-
>  drivers/char/tpm/tpm-sysfs.c | 122 +++++++++++++++++++++++++++++++++++++++++--
>  drivers/char/tpm/tpm.h       |  30 +++++++++++
>  3 files changed, 150 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index e595013..ede2ca0 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -281,8 +281,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>  		return 0;
>  
> -	tpm_sysfs_add_device(chip);
> -
>  	chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
>  
>  	return 0;
> @@ -363,6 +361,8 @@ int tpm_chip_register(struct tpm_chip *chip)
>  			return rc;
>  	}
>  
> +	tpm_sysfs_add_device(chip);
> +
>  	rc = tpm1_chip_register(chip);
>  	if (rc)
>  		return rc;
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index b46cf70..62940ef 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -264,7 +264,7 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(timeouts);
>  
> -static struct attribute *tpm_dev_attrs[] = {
> +static struct attribute *tpm1_dev_attrs[] = {
>  	&dev_attr_pubek.attr,
>  	&dev_attr_pcrs.attr,
>  	&dev_attr_enabled.attr,
> @@ -278,8 +278,120 @@ static struct attribute *tpm_dev_attrs[] = {
>  	NULL,
>  };
>  
> -static const struct attribute_group tpm_dev_group = {
> -	.attrs = tpm_dev_attrs,
> +static const struct attribute_group tpm1_dev_group = {
> +	.attrs = tpm1_dev_attrs,
> +};
> +
> +struct tpm2_prop_flag_dev_attribute {
> +	struct device_attribute attr;
> +	u32 property_id;
> +	u32 flag_mask;
> +};
> +
> +struct tpm2_prop_u32_dev_attribute {
> +	struct device_attribute attr;
> +	u32 property_id;
> +};
> +
> +static ssize_t tpm2_prop_flag_show(struct device *dev,
> +				   struct device_attribute *attr,
> +				   char *buf)
> +{
> +	struct tpm2_prop_flag_dev_attribute *pa =
> +		container_of(attr, struct tpm2_prop_flag_dev_attribute, attr);
> +	u32 flags;
> +	ssize_t rc;
> +
> +	rc = tpm2_get_tpm_pt(to_tpm_chip(dev), pa->property_id, &flags,
> +			     "reading property");
> +	if (rc)
> +		return 0;
> +
> +	return sprintf(buf, "%d\n", !!(flags & pa->flag_mask));
> +}
> +
> +static ssize_t tpm2_prop_u32_show(struct device *dev,
> +				  struct device_attribute *attr,
> +				  char *buf)
> +{
> +	struct tpm2_prop_u32_dev_attribute *pa =
> +		container_of(attr, struct tpm2_prop_u32_dev_attribute, attr);
> +	u32 value;
> +	ssize_t rc;
> +
> +	rc = tpm2_get_tpm_pt(to_tpm_chip(dev), pa->property_id, &value,
> +			     "reading property");
> +	if (rc)
> +		return 0;
> +
> +	return sprintf(buf, "%u\n", value);
> +}
> +
> +#define TPM2_PROP_FLAG_ATTR(_name, _property_id, _flag_mask)           \
> +	struct tpm2_prop_flag_dev_attribute attr_tpm2_prop_##_name = { \
> +		__ATTR(_name, S_IRUGO, tpm2_prop_flag_show, NULL),     \
> +		_property_id, _flag_mask                               \
> +	}
> +
> +#define TPM2_PROP_U32_ATTR(_name, _property_id)                        \
> +	struct tpm2_prop_u32_dev_attribute attr_tpm2_prop_##_name = {  \
> +		__ATTR(_name, S_IRUGO, tpm2_prop_u32_show, NULL),      \
> +		_property_id                                           \
> +	}
> +
> +TPM2_PROP_FLAG_ATTR(owner_auth_set,
> +		    TPM2_PT_PERMANENT, TPM2_ATTR_OWNER_AUTH_SET);
> +TPM2_PROP_FLAG_ATTR(endorsement_auth_set,
> +		    TPM2_PT_PERMANENT, TPM2_ATTR_ENDORSEMENT_AUTH_SET);
> +TPM2_PROP_FLAG_ATTR(lockout_auth_set,
> +		    TPM2_PT_PERMANENT, TPM2_ATTR_LOCKOUT_AUTH_SET);
> +TPM2_PROP_FLAG_ATTR(disable_clear,
> +		    TPM2_PT_PERMANENT, TPM2_ATTR_DISABLE_CLEAR);
> +TPM2_PROP_FLAG_ATTR(in_lockout,
> +		    TPM2_PT_PERMANENT, TPM2_ATTR_IN_LOCKOUT);
> +TPM2_PROP_FLAG_ATTR(tpm_generated_eps,
> +		    TPM2_PT_PERMANENT, TPM2_ATTR_TPM_GENERATED_EPS);
> +
> +TPM2_PROP_FLAG_ATTR(ph_enable,
> +		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_PH_ENABLE);
> +TPM2_PROP_FLAG_ATTR(sh_enable,
> +		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_SH_ENABLE);
> +TPM2_PROP_FLAG_ATTR(eh_enable,
> +		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_EH_ENABLE);
> +TPM2_PROP_FLAG_ATTR(ph_enable_nv,
> +		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_PH_ENABLE_NV);
> +TPM2_PROP_FLAG_ATTR(orderly,
> +		    TPM2_PT_STARTUP_CLEAR, TPM2_ATTR_ORDERLY);
> +
> +TPM2_PROP_U32_ATTR(lockout_counter, TPM2_PT_LOCKOUT_COUNTER);
> +TPM2_PROP_U32_ATTR(max_auth_fail, TPM2_PT_MAX_AUTH_FAIL);
> +TPM2_PROP_U32_ATTR(lockout_interval, TPM2_PT_LOCKOUT_INTERVAL);
> +TPM2_PROP_U32_ATTR(lockout_recovery, TPM2_PT_LOCKOUT_RECOVERY);
> +
> +#define ATTR_FOR_TPM2_PROP(_name) (&attr_tpm2_prop_##_name.attr.attr)
> +static struct attribute *tpm2_dev_attrs[] = {
> +	ATTR_FOR_TPM2_PROP(owner_auth_set),
> +	ATTR_FOR_TPM2_PROP(endorsement_auth_set),
> +	ATTR_FOR_TPM2_PROP(lockout_auth_set),
> +	ATTR_FOR_TPM2_PROP(disable_clear),
> +	ATTR_FOR_TPM2_PROP(in_lockout),
> +	ATTR_FOR_TPM2_PROP(tpm_generated_eps),
> +	ATTR_FOR_TPM2_PROP(ph_enable),
> +	ATTR_FOR_TPM2_PROP(sh_enable),
> +	ATTR_FOR_TPM2_PROP(eh_enable),
> +	ATTR_FOR_TPM2_PROP(ph_enable_nv),
> +	ATTR_FOR_TPM2_PROP(orderly),
> +	ATTR_FOR_TPM2_PROP(lockout_counter),
> +	ATTR_FOR_TPM2_PROP(max_auth_fail),
> +	ATTR_FOR_TPM2_PROP(lockout_interval),
> +	ATTR_FOR_TPM2_PROP(lockout_recovery),
> +	&dev_attr_durations.attr,
> +	&dev_attr_timeouts.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group tpm2_dev_group = {
> +	.attrs = tpm2_dev_attrs,
>  };
>  
>  void tpm_sysfs_add_device(struct tpm_chip *chip)
> @@ -289,5 +401,7 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
>  	 * removal so that no callbacks are running or can run again
>  	 */
>  	WARN_ON(chip->groups_cnt != 0);
> -	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> +	chip->groups[chip->groups_cnt++] =
> +		(chip->flags & TPM_CHIP_FLAG_TPM2) ?
> +		&tpm2_dev_group : &tpm1_dev_group;
>  }
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 6e002c4..9feb023 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -127,6 +127,36 @@ enum tpm2_capabilities {
>  	TPM2_CAP_TPM_PROPERTIES = 6,
>  };
>  
> +enum tpm2_properties {
> +	TPM2_PT_NONE			= 0,
> +	TPM2_PT_GROUP			= 0x100,
> +	TPM2_PT_FIXED			= TPM2_PT_GROUP,
> +	TPM2_PT_VAR			= TPM2_PT_GROUP * 2,
> +	TPM2_PT_PERMANENT		= TPM2_PT_VAR + 0,
> +	TPM2_PT_STARTUP_CLEAR		= TPM2_PT_VAR + 1,
> +	TPM2_PT_LOCKOUT_COUNTER		= TPM2_PT_VAR + 14,
> +	TPM2_PT_MAX_AUTH_FAIL		= TPM2_PT_VAR + 15,
> +	TPM2_PT_LOCKOUT_INTERVAL	= TPM2_PT_VAR + 16,
> +	TPM2_PT_LOCKOUT_RECOVERY	= TPM2_PT_VAR + 17,
> +};
> +
> +enum tpm2_attr_permanent {
> +	TPM2_ATTR_OWNER_AUTH_SET	= BIT(0),
> +	TPM2_ATTR_ENDORSEMENT_AUTH_SET	= BIT(1),
> +	TPM2_ATTR_LOCKOUT_AUTH_SET	= BIT(2),
> +	TPM2_ATTR_DISABLE_CLEAR		= BIT(8),
> +	TPM2_ATTR_IN_LOCKOUT		= BIT(9),
> +	TPM2_ATTR_TPM_GENERATED_EPS	= BIT(10),
> +};
> +
> +enum tpm2_attr_startup_clear {
> +	TPM2_ATTR_PH_ENABLE		= BIT(0),
> +	TPM2_ATTR_SH_ENABLE		= BIT(1),
> +	TPM2_ATTR_EH_ENABLE		= BIT(2),
> +	TPM2_ATTR_PH_ENABLE_NV		= BIT(3),
> +	TPM2_ATTR_ORDERLY		= BIT(31),
> +};
> +
>  enum tpm2_startup_types {
>  	TPM2_SU_CLEAR	= 0x0000,
>  	TPM2_SU_STATE	= 0x0001,
> -- 
> 2.6.6
>