[PATCH] tpm: Add sysfs interface to show TPM family version

[PATCH] tpm: Add sysfs interface to show TPM family version

[Meng.Li]

From: Limeng <Meng.Li@windriver.com>

So far, there is not a sysfs interface for user space code to
check the TPM family version(TPM1.x or TPM2). So, add a
file named description in /sys/class/tpm/tpmX/ to show it.

Signed-off-by: Meng Li <Meng.Li@windriver.com>
---
 drivers/char/tpm/tpm-chip.c |   70 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index c406343..b222421 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -36,6 +36,68 @@
 dev_t tpm_devt;
 
 /**
+ * show_description - sysfs interface for checking current TPM hardware version.
+ * @dev:	pointer to tpm chip device
+ * @attr:	unused
+ * @buf:	char buffer to be filled with TPM hardware version info
+ *
+ * Provides sysfs interface for showing current TPM hardware version.
+ */
+static ssize_t show_description(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct tpm_chip *chip = (struct tpm_chip *)container_of(dev,struct tpm_chip,dev);
+	int ret;
+
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		ret = sprintf(buf, "TPM 2.0");
+	else
+		ret = sprintf(buf, "TPM 1.x");
+
+	return ret;
+}
+
+static struct device_attribute tpm_attrs[] = {
+	__ATTR(description, S_IRUGO, show_description, NULL),
+};
+
+/**
+ * tpm_create_sysfs - Create tpm sysfs interface.
+ * @dev:	pointer to tpm chip device
+ *
+ * Create sysfs interface for checking current TPM hardware version.
+ */
+static int tpm_create_sysfs(struct device *dev)
+{
+	int r, t;
+
+	for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) {
+		r = device_create_file(dev, &tpm_attrs[t]);
+		if (r) {
+			dev_err(dev, "failed to create sysfs file\n");
+			return r;
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * tpm_remove_sysfs - Remove tpm sysfs interface.
+ * @dev:	pointer to tpm chip device
+ *
+ * Remove sysfs interface for checking current TPM hardware version.
+ */
+static void tpm_remove_sysfs(struct device *dev)
+{
+	int  t;
+
+	for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) {
+		device_remove_file(dev, &tpm_attrs[t]);
+	}
+}
+
+/**
  * tpm_try_get_ops() - Get a ref to the tpm_chip
  * @chip: Chip to ref
  *
@@ -363,6 +425,13 @@ int tpm_chip_register(struct tpm_chip *chip)
 		return rc;
 	}
 
+	rc = tpm_create_sysfs(&chip->dev);
+	if (rc) {
+		tpm_del_legacy_sysfs(chip);
+		tpm_chip_unregister(chip);
+		return rc;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(tpm_chip_register);
@@ -382,6 +451,7 @@ int tpm_chip_register(struct tpm_chip *chip)
  */
 void tpm_chip_unregister(struct tpm_chip *chip)
 {
+	tpm_remove_sysfs(&chip->dev);
 	tpm_del_legacy_sysfs(chip);
 	tpm_bios_log_teardown(chip);
 	tpm_del_char_device(chip);
-- 
1.7.9.5

Re: [PATCH] tpm: Add sysfs interface to show TPM family version

[Jarkko Sakkinen]

On Mon, Mar 13, 2017 at 05:20:17PM +0800, Meng.Li@windriver.com wrote:
> From: Limeng <Meng.Li@windriver.com>
> 
> So far, there is not a sysfs interface for user space code to
> check the TPM family version(TPM1.x or TPM2). So, add a
> file named description in /sys/class/tpm/tpmX/ to show it.
> 
> Signed-off-by: Meng Li <Meng.Li@windriver.com>
> ---

Is this the first or which version of the patch is this? Version
number and changelog are missing :/

/Jarkko

>  drivers/char/tpm/tpm-chip.c |   70 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index c406343..b222421 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -36,6 +36,68 @@
>  dev_t tpm_devt;
>  
>  /**
> + * show_description - sysfs interface for checking current TPM hardware version.
> + * @dev:	pointer to tpm chip device
> + * @attr:	unused
> + * @buf:	char buffer to be filled with TPM hardware version info
> + *
> + * Provides sysfs interface for showing current TPM hardware version.
> + */
> +static ssize_t show_description(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct tpm_chip *chip = (struct tpm_chip *)container_of(dev,struct tpm_chip,dev);
> +	int ret;
> +
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		ret = sprintf(buf, "TPM 2.0");
> +	else
> +		ret = sprintf(buf, "TPM 1.x");
> +
> +	return ret;
> +}
> +
> +static struct device_attribute tpm_attrs[] = {
> +	__ATTR(description, S_IRUGO, show_description, NULL),
> +};
> +
> +/**
> + * tpm_create_sysfs - Create tpm sysfs interface.
> + * @dev:	pointer to tpm chip device
> + *
> + * Create sysfs interface for checking current TPM hardware version.
> + */
> +static int tpm_create_sysfs(struct device *dev)
> +{
> +	int r, t;
> +
> +	for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) {
> +		r = device_create_file(dev, &tpm_attrs[t]);
> +		if (r) {
> +			dev_err(dev, "failed to create sysfs file\n");
> +			return r;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * tpm_remove_sysfs - Remove tpm sysfs interface.
> + * @dev:	pointer to tpm chip device
> + *
> + * Remove sysfs interface for checking current TPM hardware version.
> + */
> +static void tpm_remove_sysfs(struct device *dev)
> +{
> +	int  t;
> +
> +	for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) {
> +		device_remove_file(dev, &tpm_attrs[t]);
> +	}
> +}
> +
> +/**
>   * tpm_try_get_ops() - Get a ref to the tpm_chip
>   * @chip: Chip to ref
>   *
> @@ -363,6 +425,13 @@ int tpm_chip_register(struct tpm_chip *chip)
>  		return rc;
>  	}
>  
> +	rc = tpm_create_sysfs(&chip->dev);
> +	if (rc) {
> +		tpm_del_legacy_sysfs(chip);
> +		tpm_chip_unregister(chip);
> +		return rc;
> +	}
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(tpm_chip_register);
> @@ -382,6 +451,7 @@ int tpm_chip_register(struct tpm_chip *chip)
>   */
>  void tpm_chip_unregister(struct tpm_chip *chip)
>  {
> +	tpm_remove_sysfs(&chip->dev);
>  	tpm_del_legacy_sysfs(chip);
>  	tpm_bios_log_teardown(chip);
>  	tpm_del_char_device(chip);
> -- 
> 1.7.9.5
> 

RE: [PATCH] tpm: Add sysfs interface to show TPM family version

[Li, Meng]

> -----Original Message-----
> From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> Sent: Monday, March 13, 2017 7:54 PM
> To: Li, Meng
> Cc: linux-kernel@vger.kernel.org; peterhuewe@gmx.de;
> tpmdd@selhorst.net; jgunthorpe@obsidianresearch.com; tpmdd-
> devel@lists.sourceforge.net
> Subject: Re: [PATCH] tpm: Add sysfs interface to show TPM family version
> 
> On Mon, Mar 13, 2017 at 05:20:17PM +0800, Meng.Li@windriver.com wrote:
> > From: Limeng <Meng.Li@windriver.com>
> >
> > So far, there is not a sysfs interface for user space code to check
> > the TPM family version(TPM1.x or TPM2). So, add a file named
> > description in /sys/class/tpm/tpmX/ to show it.
> >
> > Signed-off-by: Meng Li <Meng.Li@windriver.com>
> > ---
> 
> Is this the first or which version of the patch is this? Version number and
> changelog are missing :/

Hi Jarkko,

This is the second version of this patch. The first one is reviewed by Peter who give out some good advices.

It is my first time to submit patch to upstream(main line),  and I am not very clear with the submitting rule.
So, could you please give me a template to record the version and changing log?
In my previous experience, I will attach a patch-0000 to record version and describe the background of this patch.
But how to show the changing log when submit patch into mainline, I am not very  clear.

Regards,
Limeng

> 
> /Jarkko
> 
> >  drivers/char/tpm/tpm-chip.c |   70
> +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index c406343..b222421 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -36,6 +36,68 @@
> >  dev_t tpm_devt;
> >
> >  /**
> > + * show_description - sysfs interface for checking current TPM hardware
> version.
> > + * @dev:	pointer to tpm chip device
> > + * @attr:	unused
> > + * @buf:	char buffer to be filled with TPM hardware version info
> > + *
> > + * Provides sysfs interface for showing current TPM hardware version.
> > + */
> > +static ssize_t show_description(struct device *dev,
> > +		struct device_attribute *attr, char *buf) {
> > +	struct tpm_chip *chip = (struct tpm_chip *)container_of(dev,struct
> tpm_chip,dev);
> > +	int ret;
> > +
> > +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > +		ret = sprintf(buf, "TPM 2.0");
> > +	else
> > +		ret = sprintf(buf, "TPM 1.x");
> > +
> > +	return ret;
> > +}
> > +
> > +static struct device_attribute tpm_attrs[] = {
> > +	__ATTR(description, S_IRUGO, show_description, NULL), };
> > +
> > +/**
> > + * tpm_create_sysfs - Create tpm sysfs interface.
> > + * @dev:	pointer to tpm chip device
> > + *
> > + * Create sysfs interface for checking current TPM hardware version.
> > + */
> > +static int tpm_create_sysfs(struct device *dev) {
> > +	int r, t;
> > +
> > +	for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) {
> > +		r = device_create_file(dev, &tpm_attrs[t]);
> > +		if (r) {
> > +			dev_err(dev, "failed to create sysfs file\n");
> > +			return r;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * tpm_remove_sysfs - Remove tpm sysfs interface.
> > + * @dev:	pointer to tpm chip device
> > + *
> > + * Remove sysfs interface for checking current TPM hardware version.
> > + */
> > +static void tpm_remove_sysfs(struct device *dev) {
> > +	int  t;
> > +
> > +	for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) {
> > +		device_remove_file(dev, &tpm_attrs[t]);
> > +	}
> > +}
> > +
> > +/**
> >   * tpm_try_get_ops() - Get a ref to the tpm_chip
> >   * @chip: Chip to ref
> >   *
> > @@ -363,6 +425,13 @@ int tpm_chip_register(struct tpm_chip *chip)
> >  		return rc;
> >  	}
> >
> > +	rc = tpm_create_sysfs(&chip->dev);
> > +	if (rc) {
> > +		tpm_del_legacy_sysfs(chip);
> > +		tpm_chip_unregister(chip);
> > +		return rc;
> > +	}
> > +
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(tpm_chip_register);
> > @@ -382,6 +451,7 @@ int tpm_chip_register(struct tpm_chip *chip)
> >   */
> >  void tpm_chip_unregister(struct tpm_chip *chip)  {
> > +	tpm_remove_sysfs(&chip->dev);
> >  	tpm_del_legacy_sysfs(chip);
> >  	tpm_bios_log_teardown(chip);
> >  	tpm_del_char_device(chip);
> > --
> > 1.7.9.5
> >

Re: [tpmdd-devel] [PATCH] tpm: Add sysfs interface to show TPM family version

[Andrew Lunn]

On Mon, Mar 13, 2017 at 12:54:24PM +0000, Li, Meng wrote:
> 
> 
> > -----Original Message-----
> > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > Sent: Monday, March 13, 2017 7:54 PM
> > To: Li, Meng
> > Cc: linux-kernel@vger.kernel.org; peterhuewe@gmx.de;
> > tpmdd@selhorst.net; jgunthorpe@obsidianresearch.com; tpmdd-
> > devel@lists.sourceforge.net
> > Subject: Re: [PATCH] tpm: Add sysfs interface to show TPM family version
> > 
> > On Mon, Mar 13, 2017 at 05:20:17PM +0800, Meng.Li@windriver.com wrote:
> > > From: Limeng <Meng.Li@windriver.com>
> > >
> > > So far, there is not a sysfs interface for user space code to check
> > > the TPM family version(TPM1.x or TPM2). So, add a file named
> > > description in /sys/class/tpm/tpmX/ to show it.
> > >
> > > Signed-off-by: Meng Li <Meng.Li@windriver.com>
> > > ---
> > 
> > Is this the first or which version of the patch is this? Version number and
> > changelog are missing :/
> 
> Hi Jarkko,
> 
> This is the second version of this patch. The first one is reviewed by Peter who give out some good advices.
> 
> It is my first time to submit patch to upstream(main line),

A tip for you then. Wait at least one day between patch versions. That
gives people time to comment on your patches, and you can fix many
different issues at once.

> and I am not very clear with the submitting rule.

A good start is:

Documentation/process/submitting-patches.rst

> So, could you please give me a template to record the version and changing log?

After the --- write one line per change you made from the previous
version. Also, v3 should also contain the v2 change history etc...

Picking a random example from the web:

http://patchwork.ozlabs.org/patch/710615/

	Andrew

Re: [PATCH] tpm: Add sysfs interface to show TPM family version

[Jarkko Sakkinen]

On Mon, Mar 13, 2017 at 12:54:24PM +0000, Li, Meng wrote:
> 
> 
> > -----Original Message-----
> > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > Sent: Monday, March 13, 2017 7:54 PM
> > To: Li, Meng
> > Cc: linux-kernel@vger.kernel.org; peterhuewe@gmx.de;
> > tpmdd@selhorst.net; jgunthorpe@obsidianresearch.com; tpmdd-
> > devel@lists.sourceforge.net
> > Subject: Re: [PATCH] tpm: Add sysfs interface to show TPM family version
> > 
> > On Mon, Mar 13, 2017 at 05:20:17PM +0800, Meng.Li@windriver.com wrote:
> > > From: Limeng <Meng.Li@windriver.com>
> > >
> > > So far, there is not a sysfs interface for user space code to check
> > > the TPM family version(TPM1.x or TPM2). So, add a file named
> > > description in /sys/class/tpm/tpmX/ to show it.
> > >
> > > Signed-off-by: Meng Li <Meng.Li@windriver.com>
> > > ---
> > 
> > Is this the first or which version of the patch is this? Version number and
> > changelog are missing :/
> 
> Hi Jarkko,
> 
> This is the second version of this patch. The first one is reviewed by Peter who give out some good advices.
> 
> It is my first time to submit patch to upstream(main line),  and I am not very clear with the submitting rule.
> So, could you please give me a template to record the version and changing log?
> In my previous experience, I will attach a patch-0000 to record version and describe the background of this patch.
> But how to show the changing log when submit patch into mainline, I am not very  clear.
> 
> Regards,
> Limeng

Maybe this will help for the moment:

https://kernelnewbies.org/FirstKernelPatch#head-5c81b3c517a1d0bbc24f92594cb734e155fcbbcb

In the long run you probably should check this too:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

You probably should document in the commit message why this new sysfs
attribute is needed. Saying that it is needed to detect the TPM version
does not give any insight of any actual workload. User space facing
things are something where I rather not apply something unless it is
absolutely essetial because they have to be maintained forever.

In addition this looks odd:

if (chip->flags & TPM_CHIP_FLAG_TPM2)
	ret = sprintf(buf, "TPM 2.0");
else
	ret = sprintf(buf, "TPM 1.x");

I do not undertand for what specific purpose is the string "TPM " before
the family number is needed. And I'm not sure whether the attribute
should have both major and minor number, or should they be separate
attributes. For this the commit message does not give any insight on the
design choice.

And please remove the dangling store function...

/Jarkko

Re: [PATCH] tpm: Add sysfs interface to show TPM family version

[Jason Gunthorpe]

On Mon, Mar 13, 2017 at 04:59:58PM +0200, Jarkko Sakkinen wrote:
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> 	ret = sprintf(buf, "TPM 2.0");
> else
> 	ret = sprintf(buf, "TPM 1.x");

And 1.x should be 1.2 or 1.1

Jason

Re: [PATCH] tpm: Add sysfs interface to show TPM family version

[Jarkko Sakkinen]

On Mon, Mar 13, 2017 at 10:55:47AM -0600, Jason Gunthorpe wrote:
> On Mon, Mar 13, 2017 at 04:59:58PM +0200, Jarkko Sakkinen wrote:
> > if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > 	ret = sprintf(buf, "TPM 2.0");
> > else
> > 	ret = sprintf(buf, "TPM 1.x");
> 
> And 1.x should be 1.2 or 1.1
> 
> Jason

Or maybe just create a patch that adds "family_major", which prints
1 or 2.

That is sufficient to pick the right daemon in the user space for
example.

/Jarkko

Re: [tpmdd-devel] [PATCH] tpm: Add sysfs interface to show TPM family version

[James Bottomley]

On Mon, 2017-03-13 at 17:20 +0800, Meng.Li@windriver.com wrote:
[...]
> +/**
> + * tpm_create_sysfs - Create tpm sysfs interface.
> + * @dev:	pointer to tpm chip device
> + *
> + * Create sysfs interface for checking current TPM hardware version.
> + */
> +static int tpm_create_sysfs(struct device *dev)
> +{
> +	int r, t;
> +
> +	for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) {
> +		r = device_create_file(dev, &tpm_attrs[t]);
> +		if (r) {
> +			dev_err(dev, "failed to create sysfs
> file\n");
> +			return r;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * tpm_remove_sysfs - Remove tpm sysfs interface.
> + * @dev:	pointer to tpm chip device
> + *
> + * Remove sysfs interface for checking current TPM hardware version.
> + */
> +static void tpm_remove_sysfs(struct device *dev)
> +{
> +	int  t;
> +
> +	for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) {
> +		device_remove_file(dev, &tpm_attrs[t]);
> +	}
> +}
> +

For non-dynamic attributes, use the attribute_group structure in the
device instead.  It will take care of creation and removal
automatically.

James