From: "Li, Meng" <Meng.Li@windriver.com>
To: Peter Huewe <peterhuewe@gmx.de>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC: "tpmdd@selhorst.net" <tpmdd@selhorst.net>,
        "jarkko.sakkinen@linux.intel.com" <jarkko.sakkinen@linux.intel.com>,
        "jgunthorpe@obsidianresearch.com" <jgunthorpe@obsidianresearch.com>,
        "tpmdd-devel@lists.sourceforge.net" 
        <tpmdd-devel@lists.sourceforge.net>
Subject: RE: [PATCH] tpm: Add sysfs interface to show TPM hardware version
Thread-Topic: [PATCH] tpm: Add sysfs interface to show TPM hardware version
Thread-Index: AQHSm8kC7D42V0GmsUm7ZxvImdCg9KGSYx6A
Date: Mon, 13 Mar 2017 07:47:19 +0000
Message-ID: <529F9A9100AE8045A7A5B5A00A39FBB83DE931AB@ALA-MBC.corp.ad.wrs.com>
References: <1489382517-9078-1-git-send-email-Meng.Li@windriver.com>
 <29B2C361-A0C0-410E-ACCA-07AFA382A251@gmx.de>
In-Reply-To: <29B2C361-A0C0-410E-ACCA-07AFA382A251@gmx.de>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Transfer-Encoding: 8bit
Content-Length: 4470
Lines: 157


> -----Original Message-----
> From: Peter Huewe [mailto:peterhuewe@gmx.de]
> Sent: Monday, March 13, 2017 3:11 PM
> To: Li, Meng; linux-kernel@vger.kernel.org
> Cc: tpmdd@selhorst.net; jarkko.sakkinen@linux.intel.com;
> jgunthorpe@obsidianresearch.com; tpmdd-devel@lists.sourceforge.net
> Subject: Re: [PATCH] tpm: Add sysfs interface to show TPM hardware
> version
> 
> Hi,
> Thanks for your patch.

Hi Peter,

Thanks for reviewing this patch in your busy time.
I will do modification according to your comment and then send the next patch reviewing.

Thanks,
Limeng

> 
> Am 13. März 2017 06:21:57 MEZ schrieb Meng.Li@windriver.com:
> >From: Limeng <Meng.Li@windriver.com>
> >
> >So far, there is not a sysfs interface for user space code to check the
> >TPM hardware version(TPM1.x or TPM2). So, add a file named description
> >in /sys/class/tpm/tpmX/ to show it.
> It's not really the hardware version but the "TPM Family" according to tcg.
> 
> And yes you are right there is currently no way, except for trial and error, for
> the userspace to determine this.
> So an interface to get this information makes sense to me.
> >
> >Signed-off-by: Meng Li <Meng.Li@windriver.com>
> >---
> >drivers/char/tpm/tpm-chip.c |   85
> >+++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 85 insertions(+)
> >
> >diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> >index c406343..da2cd69 100644
> >--- a/drivers/char/tpm/tpm-chip.c
> >+++ b/drivers/char/tpm/tpm-chip.c
> >@@ -36,6 +36,83 @@
> > 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;
> >+}
> >+
> >+/**
> >+ * store_description - interface for manually setting data.
> >+ * @dev:	unused
> >+ * @attr:	unused
> >+ * @buf:	unused
> >+ * @count:	unused
> >+ *
> >+ * There is not any process in this function, reserve for feature.
> >+ */
> >+static ssize_t store_description(struct device *dev, struct
> >device_attribute *attr,
> >+		const char *buf, size_t count)
> >+{
> >+	return count;
> >+}
> Since it does not do anything
> I would not create this function and leave the sysfs node as S_IRUGO.
> 
> >+
> >+static struct device_attribute tpm_attrs[] = {
> >+	__ATTR(description, S_IRUGO | S_IWUSR, show_description,
> >store_description),
> >+};
> >+
> >+/**
> >+ * 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 +440,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 +466,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);
> 
> --
> Sent from my mobile