[PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Nayna Jain]

This patch set includes the cleanup and bug fixes patches, previously
part of the "tpm: add the securityfs pseudo files support for TPM 2.0
firmware event log" patch set, in order to upstream them more quickly.

Changelog History:

v6:

- Patch "tpm: replace symbolic permission with octal for securityfs files"
  - New Patch.
- Patch "tpm: have event log use the tpm_chip"
  - Changed commit description as per Jason's suggestion.
  - Fixed bug related to kfree() for bios_event_log.
  - Moved inode_unlock() just after get_device() in open().
  - Returned -ENODEV for read_log() ENOMEM error and other errors as it is.
  - Added comment in tpm_bios_log_teardown() to explain inode_lock()/unlock
    reasoning.
  - Splitted .owner into different patch.
- Patch "tpm: fix the missing .owner in tpm_bios_measurements_ops"
  - New Patch.
- Patch "tpm: cleanup of printk error messages"
  - Replaced dev_info() with dev_warn().
  - Updated commit description subject line.

v5:

- Moved cleanup/fixes patches into this patch set.
- Patch "fix the race condition between event log access and chip
getting unregistered"
  - updated subject line and commit description.
  - modified fops code to use chip kref.
  - modified fops to lock inode before accessing inode private data.
  - renamed tpm_securityfs_data to tpm_chip_seqops, as it no more
  holds bios log, but associates seqops with respective chip. For
  the same reason, moved it to tpm.h
- Patch "replace or remove printk error messages"
  - cleaned up dev_dbg and used dev_info as applicable.

v4:

- Includes feedbacks from Jarkko and Jason.
- Patch "tpm: define a generic open() method for ascii & bios
measurements".
  - Fix indentation issue.
- Patch "tpm: replace the dynamically allocated bios_dir as
  struct dentry array". 
  - Continue to use bios_dir_count variable to use is_bad() checks and
  to maintain correct order for securityfs_remove() during teardown.
  - Reset chip->bios_dir_count in teardown() function.
- Patch "tpm: validate the event log access before tpm_bios_log_setup".
  - Retain TPM2 check which was removed in previous patch.
  - Add tpm_bios_log_setup failure handling.
  - Remove use of private data from v3 version of patch. Add a new
  member to struct tpm_chip to achieve the same purpose.
- Patch "tpm: redefine the read_log method to check for ACPI/OF 
properties sequentially".
  - Move replacement of CONFIG_TCG_IBMVTPM with CONFIG_OF to
  this patch from patch 3.
  - Replace -1 error code with -ENODEV.
- Patch "tpm: replace the of_find_node_by_name() with dev of_node
property".
 - Uses chip->dev.parent->of_node.
 - Created separate patch for cleanup of pr_err messages.
- Patch "tpm: remove printk error messages".
 - New Patch.
- Patch "tpm: add the securityfs file support for TPM 2.0 event log".
 - Parses event digests using event alg_id rather than event log header
 alg_id.
 - Uses of_property_match_string to differentiate tpm/vtpm compatible
 property.
 - Adds the comment for difference in tpm/vtpm endianness.

v3:

- Includes the review feedbacks as suggested by Jason.
- Split of patches into one patch per idea.
- Generic open() method for ascii/bios measurements.
- Replacement of of **bios_dir with *bios_dir[3].
- Verifying readlog() is successful before creating securityfs entries.
- Generic readlog() to check for ACPI/OF in sequence.
- read_log_of() method now uses of_node propertry rather than
calling find_device_by_name.
- read_log differentiates vtpm/tpm using its compatible property.
- Cleans pr_err with dev_dbg.
- Commit msgs subject line prefixed with tpm.

v2:

- Fixes issues as given in feedback by Jason.
- Adds documentation for device tree.

Nayna Jain (9):
  tpm: define a generic open() method for ascii & bios measurements
  tpm: replace symbolic permission with octal for securityfs files
  tpm: replace dynamically allocated bios_dir with a static array
  tpm: drop tpm1_chip_register(/unregister)
  tpm: have event log use the tpm_chip
  tpm: fix the missing .owner in tpm_bios_measurements_ops
  tpm: redefine read_log() to handle ACPI/OF at runtime
  tpm: replace of_find_node_by_name() with dev of_node property
  tpm: cleanup of printk error messages

 drivers/char/tpm/Makefile       |  14 +--
 drivers/char/tpm/tpm-chip.c     |  33 ++----
 drivers/char/tpm/tpm-sysfs.c    |   3 +
 drivers/char/tpm/tpm.h          |  14 ++-
 drivers/char/tpm/tpm_acpi.c     |  38 +++----
 drivers/char/tpm/tpm_eventlog.c | 222 +++++++++++++++++++++-------------------
 drivers/char/tpm/tpm_eventlog.h |  22 ++--
 drivers/char/tpm/tpm_of.c       |  45 +++-----
 8 files changed, 187 insertions(+), 204 deletions(-)

-- 
2.5.0

[PATCH v6 1/9] tpm: define a generic open() method for ascii & bios measurements

[Nayna Jain]

open() method for event log ascii and binary bios measurements file
operations are very similar. This patch refactors the code into a
single open() call by passing seq_operations as i_node->private data.

Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm_eventlog.c | 63 ++++++++++-------------------------------
 1 file changed, 15 insertions(+), 48 deletions(-)

diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index e722886..42b49c4 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -7,6 +7,7 @@
  *	Stefan Berger <stefanb@us.ibm.com>
  *	Reiner Sailer <sailer@watson.ibm.com>
  *	Kylene Hall <kjhall@us.ibm.com>
+ *	Nayna Jain <nayna@linux.vnet.ibm.com>
  *
  * Maintained by: <tpmdd-devel@lists.sourceforge.net>
  *
@@ -304,26 +305,28 @@ static int tpm_ascii_bios_measurements_show(struct seq_file *m, void *v)
 	return 0;
 }
 
-static const struct seq_operations tpm_ascii_b_measurments_seqops = {
+static const struct seq_operations tpm_ascii_b_measurements_seqops = {
 	.start = tpm_bios_measurements_start,
 	.next = tpm_bios_measurements_next,
 	.stop = tpm_bios_measurements_stop,
 	.show = tpm_ascii_bios_measurements_show,
 };
 
-static const struct seq_operations tpm_binary_b_measurments_seqops = {
+static const struct seq_operations tpm_binary_b_measurements_seqops = {
 	.start = tpm_bios_measurements_start,
 	.next = tpm_bios_measurements_next,
 	.stop = tpm_bios_measurements_stop,
 	.show = tpm_binary_bios_measurements_show,
 };
 
-static int tpm_ascii_bios_measurements_open(struct inode *inode,
+static int tpm_bios_measurements_open(struct inode *inode,
 					    struct file *file)
 {
 	int err;
 	struct tpm_bios_log *log;
 	struct seq_file *seq;
+	const struct seq_operations *seqops =
+		(const struct seq_operations *)inode->i_private;
 
 	log = kzalloc(sizeof(struct tpm_bios_log), GFP_KERNEL);
 	if (!log)
@@ -333,7 +336,7 @@ static int tpm_ascii_bios_measurements_open(struct inode *inode,
 		goto out_free;
 
 	/* now register seq file */
-	err = seq_open(file, &tpm_ascii_b_measurments_seqops);
+	err = seq_open(file, seqops);
 	if (!err) {
 		seq = file->private_data;
 		seq->private = log;
@@ -349,46 +352,8 @@ static int tpm_ascii_bios_measurements_open(struct inode *inode,
 	goto out;
 }
 
-static const struct file_operations tpm_ascii_bios_measurements_ops = {
-	.open = tpm_ascii_bios_measurements_open,
-	.read = seq_read,
-	.llseek = seq_lseek,
-	.release = tpm_bios_measurements_release,
-};
-
-static int tpm_binary_bios_measurements_open(struct inode *inode,
-					     struct file *file)
-{
-	int err;
-	struct tpm_bios_log *log;
-	struct seq_file *seq;
-
-	log = kzalloc(sizeof(struct tpm_bios_log), GFP_KERNEL);
-	if (!log)
-		return -ENOMEM;
-
-	if ((err = read_log(log)))
-		goto out_free;
-
-	/* now register seq file */
-	err = seq_open(file, &tpm_binary_b_measurments_seqops);
-	if (!err) {
-		seq = file->private_data;
-		seq->private = log;
-	} else {
-		goto out_free;
-	}
-
-out:
-	return err;
-out_free:
-	kfree(log->bios_event_log);
-	kfree(log);
-	goto out;
-}
-
-static const struct file_operations tpm_binary_bios_measurements_ops = {
-	.open = tpm_binary_bios_measurements_open,
+static const struct file_operations tpm_bios_measurements_ops = {
+	.open = tpm_bios_measurements_open,
 	.read = seq_read,
 	.llseek = seq_lseek,
 	.release = tpm_bios_measurements_release,
@@ -413,15 +378,17 @@ struct dentry **tpm_bios_log_setup(const char *name)
 
 	bin_file =
 	    securityfs_create_file("binary_bios_measurements",
-				   S_IRUSR | S_IRGRP, tpm_dir, NULL,
-				   &tpm_binary_bios_measurements_ops);
+				   S_IRUSR | S_IRGRP, tpm_dir,
+				   (void *)&tpm_binary_b_measurements_seqops,
+				   &tpm_bios_measurements_ops);
 	if (is_bad(bin_file))
 		goto out_tpm;
 
 	ascii_file =
 	    securityfs_create_file("ascii_bios_measurements",
-				   S_IRUSR | S_IRGRP, tpm_dir, NULL,
-				   &tpm_ascii_bios_measurements_ops);
+				   S_IRUSR | S_IRGRP, tpm_dir,
+				   (void *)&tpm_ascii_b_measurements_seqops,
+				   &tpm_bios_measurements_ops);
 	if (is_bad(ascii_file))
 		goto out_bin;
 
-- 
2.5.0

[PATCH v6 1/9] tpm: define a generic open() method for ascii & bios measurements

[Nayna Jain]

open() method for event log ascii and binary bios measurements file
operations are very similar. This patch refactors the code into a
single open() call by passing seq_operations as i_node->private data.

Suggested-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/char/tpm/tpm_eventlog.c | 63 ++++++++++-------------------------------
 1 file changed, 15 insertions(+), 48 deletions(-)

diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index e722886..42b49c4 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -7,6 +7,7 @@
  *	Stefan Berger <stefanb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
  *	Reiner Sailer <sailer-aZOuKsOsJu3MbYB6QlFGEg@public.gmane.org>
  *	Kylene Hall <kjhall-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
+ *	Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
  *
  * Maintained by: <tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
  *
@@ -304,26 +305,28 @@ static int tpm_ascii_bios_measurements_show(struct seq_file *m, void *v)
 	return 0;
 }
 
-static const struct seq_operations tpm_ascii_b_measurments_seqops = {
+static const struct seq_operations tpm_ascii_b_measurements_seqops = {
 	.start = tpm_bios_measurements_start,
 	.next = tpm_bios_measurements_next,
 	.stop = tpm_bios_measurements_stop,
 	.show = tpm_ascii_bios_measurements_show,
 };
 
-static const struct seq_operations tpm_binary_b_measurments_seqops = {
+static const struct seq_operations tpm_binary_b_measurements_seqops = {
 	.start = tpm_bios_measurements_start,
 	.next = tpm_bios_measurements_next,
 	.stop = tpm_bios_measurements_stop,
 	.show = tpm_binary_bios_measurements_show,
 };
 
-static int tpm_ascii_bios_measurements_open(struct inode *inode,
+static int tpm_bios_measurements_open(struct inode *inode,
 					    struct file *file)
 {
 	int err;
 	struct tpm_bios_log *log;
 	struct seq_file *seq;
+	const struct seq_operations *seqops =
+		(const struct seq_operations *)inode->i_private;
 
 	log = kzalloc(sizeof(struct tpm_bios_log), GFP_KERNEL);
 	if (!log)
@@ -333,7 +336,7 @@ static int tpm_ascii_bios_measurements_open(struct inode *inode,
 		goto out_free;
 
 	/* now register seq file */
-	err = seq_open(file, &tpm_ascii_b_measurments_seqops);
+	err = seq_open(file, seqops);
 	if (!err) {
 		seq = file->private_data;
 		seq->private = log;
@@ -349,46 +352,8 @@ static int tpm_ascii_bios_measurements_open(struct inode *inode,
 	goto out;
 }
 
-static const struct file_operations tpm_ascii_bios_measurements_ops = {
-	.open = tpm_ascii_bios_measurements_open,
-	.read = seq_read,
-	.llseek = seq_lseek,
-	.release = tpm_bios_measurements_release,
-};
-
-static int tpm_binary_bios_measurements_open(struct inode *inode,
-					     struct file *file)
-{
-	int err;
-	struct tpm_bios_log *log;
-	struct seq_file *seq;
-
-	log = kzalloc(sizeof(struct tpm_bios_log), GFP_KERNEL);
-	if (!log)
-		return -ENOMEM;
-
-	if ((err = read_log(log)))
-		goto out_free;
-
-	/* now register seq file */
-	err = seq_open(file, &tpm_binary_b_measurments_seqops);
-	if (!err) {
-		seq = file->private_data;
-		seq->private = log;
-	} else {
-		goto out_free;
-	}
-
-out:
-	return err;
-out_free:
-	kfree(log->bios_event_log);
-	kfree(log);
-	goto out;
-}
-
-static const struct file_operations tpm_binary_bios_measurements_ops = {
-	.open = tpm_binary_bios_measurements_open,
+static const struct file_operations tpm_bios_measurements_ops = {
+	.open = tpm_bios_measurements_open,
 	.read = seq_read,
 	.llseek = seq_lseek,
 	.release = tpm_bios_measurements_release,
@@ -413,15 +378,17 @@ struct dentry **tpm_bios_log_setup(const char *name)
 
 	bin_file =
 	    securityfs_create_file("binary_bios_measurements",
-				   S_IRUSR | S_IRGRP, tpm_dir, NULL,
-				   &tpm_binary_bios_measurements_ops);
+				   S_IRUSR | S_IRGRP, tpm_dir,
+				   (void *)&tpm_binary_b_measurements_seqops,
+				   &tpm_bios_measurements_ops);
 	if (is_bad(bin_file))
 		goto out_tpm;
 
 	ascii_file =
 	    securityfs_create_file("ascii_bios_measurements",
-				   S_IRUSR | S_IRGRP, tpm_dir, NULL,
-				   &tpm_ascii_bios_measurements_ops);
+				   S_IRUSR | S_IRGRP, tpm_dir,
+				   (void *)&tpm_ascii_b_measurements_seqops,
+				   &tpm_bios_measurements_ops);
 	if (is_bad(ascii_file))
 		goto out_bin;
 
-- 
2.5.0


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

[PATCH v6 2/9] tpm: replace symbolic permission with octal for securityfs files

[Nayna Jain]

checkpatch.pl flags warning for symbolic permissions and suggests
to replace with octal value.

This patch changes securityfs pseudo files permission
to octal values in tpm_bios_log_setup().

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm_eventlog.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 42b49c4..9467e31 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -378,7 +378,7 @@ struct dentry **tpm_bios_log_setup(const char *name)
 
 	bin_file =
 	    securityfs_create_file("binary_bios_measurements",
-				   S_IRUSR | S_IRGRP, tpm_dir,
+				   0440, tpm_dir,
 				   (void *)&tpm_binary_b_measurements_seqops,
 				   &tpm_bios_measurements_ops);
 	if (is_bad(bin_file))
@@ -386,7 +386,7 @@ struct dentry **tpm_bios_log_setup(const char *name)
 
 	ascii_file =
 	    securityfs_create_file("ascii_bios_measurements",
-				   S_IRUSR | S_IRGRP, tpm_dir,
+				   0440, tpm_dir,
 				   (void *)&tpm_ascii_b_measurements_seqops,
 				   &tpm_bios_measurements_ops);
 	if (is_bad(ascii_file))
-- 
2.5.0

Re: [PATCH v6 2/9] tpm: replace symbolic permission with octal for securityfs files

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 05:00:49AM -0500, Nayna Jain wrote:
> checkpatch.pl flags warning for symbolic permissions and suggests
> to replace with octal value.
> 
> This patch changes securityfs pseudo files permission
> to octal values in tpm_bios_log_setup().
> 
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

> ---
>  drivers/char/tpm/tpm_eventlog.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 42b49c4..9467e31 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -378,7 +378,7 @@ struct dentry **tpm_bios_log_setup(const char *name)
>  
>  	bin_file =
>  	    securityfs_create_file("binary_bios_measurements",
> -				   S_IRUSR | S_IRGRP, tpm_dir,
> +				   0440, tpm_dir,
>  				   (void *)&tpm_binary_b_measurements_seqops,
>  				   &tpm_bios_measurements_ops);
>  	if (is_bad(bin_file))
> @@ -386,7 +386,7 @@ struct dentry **tpm_bios_log_setup(const char *name)
>  
>  	ascii_file =
>  	    securityfs_create_file("ascii_bios_measurements",
> -				   S_IRUSR | S_IRGRP, tpm_dir,
> +				   0440, tpm_dir,
>  				   (void *)&tpm_ascii_b_measurements_seqops,
>  				   &tpm_bios_measurements_ops);
>  	if (is_bad(ascii_file))
> -- 
> 2.5.0
> 

Re: [PATCH v6 2/9] tpm: replace symbolic permission with octal for securityfs files

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 05:00:49AM -0500, Nayna Jain wrote:
> checkpatch.pl flags warning for symbolic permissions and suggests
> to replace with octal value.
> 
> This patch changes securityfs pseudo files permission
> to octal values in tpm_bios_log_setup().
> 
> Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

/Jarkko

> ---
>  drivers/char/tpm/tpm_eventlog.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 42b49c4..9467e31 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -378,7 +378,7 @@ struct dentry **tpm_bios_log_setup(const char *name)
>  
>  	bin_file =
>  	    securityfs_create_file("binary_bios_measurements",
> -				   S_IRUSR | S_IRGRP, tpm_dir,
> +				   0440, tpm_dir,
>  				   (void *)&tpm_binary_b_measurements_seqops,
>  				   &tpm_bios_measurements_ops);
>  	if (is_bad(bin_file))
> @@ -386,7 +386,7 @@ struct dentry **tpm_bios_log_setup(const char *name)
>  
>  	ascii_file =
>  	    securityfs_create_file("ascii_bios_measurements",
> -				   S_IRUSR | S_IRGRP, tpm_dir,
> +				   0440, tpm_dir,
>  				   (void *)&tpm_ascii_b_measurements_seqops,
>  				   &tpm_bios_measurements_ops);
>  	if (is_bad(ascii_file))
> -- 
> 2.5.0
> 

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

Re: [PATCH v6 2/9] tpm: replace symbolic permission with octal for securityfs files

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 05:00:49AM -0500, Nayna Jain wrote:
> checkpatch.pl flags warning for symbolic permissions and suggests
> to replace with octal value.
> 
> This patch changes securityfs pseudo files permission
> to octal values in tpm_bios_log_setup().
> 
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

> ---
>  drivers/char/tpm/tpm_eventlog.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 42b49c4..9467e31 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -378,7 +378,7 @@ struct dentry **tpm_bios_log_setup(const char *name)
>  
>  	bin_file =
>  	    securityfs_create_file("binary_bios_measurements",
> -				   S_IRUSR | S_IRGRP, tpm_dir,
> +				   0440, tpm_dir,
>  				   (void *)&tpm_binary_b_measurements_seqops,
>  				   &tpm_bios_measurements_ops);
>  	if (is_bad(bin_file))
> @@ -386,7 +386,7 @@ struct dentry **tpm_bios_log_setup(const char *name)
>  
>  	ascii_file =
>  	    securityfs_create_file("ascii_bios_measurements",
> -				   S_IRUSR | S_IRGRP, tpm_dir,
> +				   0440, tpm_dir,
>  				   (void *)&tpm_ascii_b_measurements_seqops,
>  				   &tpm_bios_measurements_ops);
>  	if (is_bad(ascii_file))
> -- 
> 2.5.0
> 

Re: [PATCH v6 2/9] tpm: replace symbolic permission with octal for securityfs files

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 05:00:49AM -0500, Nayna Jain wrote:
> checkpatch.pl flags warning for symbolic permissions and suggests
> to replace with octal value.
> 
> This patch changes securityfs pseudo files permission
> to octal values in tpm_bios_log_setup().
> 
> Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

/Jarkko

> ---
>  drivers/char/tpm/tpm_eventlog.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 42b49c4..9467e31 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -378,7 +378,7 @@ struct dentry **tpm_bios_log_setup(const char *name)
>  
>  	bin_file =
>  	    securityfs_create_file("binary_bios_measurements",
> -				   S_IRUSR | S_IRGRP, tpm_dir,
> +				   0440, tpm_dir,
>  				   (void *)&tpm_binary_b_measurements_seqops,
>  				   &tpm_bios_measurements_ops);
>  	if (is_bad(bin_file))
> @@ -386,7 +386,7 @@ struct dentry **tpm_bios_log_setup(const char *name)
>  
>  	ascii_file =
>  	    securityfs_create_file("ascii_bios_measurements",
> -				   S_IRUSR | S_IRGRP, tpm_dir,
> +				   0440, tpm_dir,
>  				   (void *)&tpm_ascii_b_measurements_seqops,
>  				   &tpm_bios_measurements_ops);
>  	if (is_bad(ascii_file))
> -- 
> 2.5.0
> 

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

Re: [tpmdd-devel] [PATCH v6 2/9] tpm: replace symbolic permission with octal for securityfs files

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 02:21:14PM -0800, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 05:00:49AM -0500, Nayna Jain wrote:
> > checkpatch.pl flags warning for symbolic permissions and suggests
> > to replace with octal value.
> > 
> > This patch changes securityfs pseudo files permission
> > to octal values in tpm_bios_log_setup().
> > 
> > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

Re: [PATCH v6 2/9] tpm: replace symbolic permission with octal for securityfs files

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 02:21:14PM -0800, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 05:00:49AM -0500, Nayna Jain wrote:
> > checkpatch.pl flags warning for symbolic permissions and suggests
> > to replace with octal value.
> > 
> > This patch changes securityfs pseudo files permission
> > to octal values in tpm_bios_log_setup().
> > 
> > Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

/Jarkko

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

[PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Nayna Jain]

This commit is based on a commit by Nayna Jain. Replaced dynamically
allocated bios_dir with a static array as the size is always constant.

Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-chip.c     |  9 ++++---
 drivers/char/tpm/tpm.h          |  3 ++-
 drivers/char/tpm/tpm_eventlog.c | 59 ++++++++++++++++++-----------------------
 drivers/char/tpm/tpm_eventlog.h | 10 +++----
 4 files changed, 38 insertions(+), 43 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 836f056..d0c1872 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -278,14 +278,16 @@ static void tpm_del_char_device(struct tpm_chip *chip)
 
 static int tpm1_chip_register(struct tpm_chip *chip)
 {
+	int rc;
+
 	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));
+	rc = tpm_bios_log_setup(chip);
 
-	return 0;
+	return rc;
 }
 
 static void tpm1_chip_unregister(struct tpm_chip *chip)
@@ -293,8 +295,7 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		return;
 
-	if (chip->bios_dir)
-		tpm_bios_log_teardown(chip->bios_dir);
+	tpm_bios_log_teardown(chip);
 }
 
 static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f9401ca..9d69580 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -40,6 +40,7 @@ enum tpm_const {
 	TPM_BUFSIZE = 4096,
 	TPM_NUM_DEVICES = 65536,
 	TPM_RETRY = 50,		/* 5 seconds */
+	TPM_NUM_EVENT_LOG_FILES = 3,
 };
 
 enum tpm_timeout {
@@ -171,7 +172,7 @@ struct tpm_chip {
 	unsigned long duration[3]; /* jiffies */
 	bool duration_adjusted;
 
-	struct dentry **bios_dir;
+	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
 
 	const struct attribute_group *groups[3];
 	unsigned int groups_cnt;
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 9467e31..62e9da6 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -368,54 +368,47 @@ static int is_bad(void *p)
 	return 0;
 }
 
-struct dentry **tpm_bios_log_setup(const char *name)
+int tpm_bios_log_setup(struct tpm_chip *chip)
 {
-	struct dentry **ret = NULL, *tpm_dir, *bin_file, *ascii_file;
+	const char *name = dev_name(&chip->dev);
+	unsigned int cnt;
 
-	tpm_dir = securityfs_create_dir(name, NULL);
-	if (is_bad(tpm_dir))
-		goto out;
+	cnt = 0;
+	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
+	if (is_bad(chip->bios_dir[cnt]))
+		goto err;
+	cnt++;
 
-	bin_file =
+	chip->bios_dir[cnt] =
 	    securityfs_create_file("binary_bios_measurements",
-				   0440, tpm_dir,
+				   0440, chip->bios_dir[0],
 				   (void *)&tpm_binary_b_measurements_seqops,
 				   &tpm_bios_measurements_ops);
-	if (is_bad(bin_file))
-		goto out_tpm;
+	if (is_bad(chip->bios_dir[cnt]))
+		goto err;
+	cnt++;
 
-	ascii_file =
+	chip->bios_dir[cnt] =
 	    securityfs_create_file("ascii_bios_measurements",
-				   0440, tpm_dir,
+				   0440, chip->bios_dir[0],
 				   (void *)&tpm_ascii_b_measurements_seqops,
 				   &tpm_bios_measurements_ops);
-	if (is_bad(ascii_file))
-		goto out_bin;
+	if (is_bad(chip->bios_dir[cnt]))
+		goto err;
+	cnt++;
 
-	ret = kmalloc(3 * sizeof(struct dentry *), GFP_KERNEL);
-	if (!ret)
-		goto out_ascii;
-
-	ret[0] = ascii_file;
-	ret[1] = bin_file;
-	ret[2] = tpm_dir;
-
-	return ret;
+	return 0;
 
-out_ascii:
-	securityfs_remove(ascii_file);
-out_bin:
-	securityfs_remove(bin_file);
-out_tpm:
-	securityfs_remove(tpm_dir);
-out:
-	return NULL;
+err:
+	chip->bios_dir[cnt] = NULL;
+	tpm_bios_log_teardown(chip);
+	return -EIO;
 }
 
-void tpm_bios_log_teardown(struct dentry **lst)
+void tpm_bios_log_teardown(struct tpm_chip *chip)
 {
 	int i;
 
-	for (i = 0; i < 3; i++)
-		securityfs_remove(lst[i]);
+	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--)
+		securityfs_remove(chip->bios_dir[i]);
 }
diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
index 8de62b0..fd3357e 100644
--- a/drivers/char/tpm/tpm_eventlog.h
+++ b/drivers/char/tpm/tpm_eventlog.h
@@ -77,14 +77,14 @@ int read_log(struct tpm_bios_log *log);
 
 #if defined(CONFIG_TCG_IBMVTPM) || defined(CONFIG_TCG_IBMVTPM_MODULE) || \
 	defined(CONFIG_ACPI)
-extern struct dentry **tpm_bios_log_setup(const char *);
-extern void tpm_bios_log_teardown(struct dentry **);
+extern int tpm_bios_log_setup(struct tpm_chip *chip);
+extern void tpm_bios_log_teardown(struct tpm_chip *chip);
 #else
-static inline struct dentry **tpm_bios_log_setup(const char *name)
+static inline int tpm_bios_log_setup(struct tpm_chip *chip)
 {
-	return NULL;
+	return 0;
 }
-static inline void tpm_bios_log_teardown(struct dentry **dir)
+static inline void tpm_bios_log_teardown(struct tpm_chip *chip)
 {
 }
 #endif
-- 
2.5.0

[PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Nayna Jain]

This commit is based on a commit by Nayna Jain. Replaced dynamically
allocated bios_dir with a static array as the size is always constant.

Suggested-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/char/tpm/tpm-chip.c     |  9 ++++---
 drivers/char/tpm/tpm.h          |  3 ++-
 drivers/char/tpm/tpm_eventlog.c | 59 ++++++++++++++++++-----------------------
 drivers/char/tpm/tpm_eventlog.h | 10 +++----
 4 files changed, 38 insertions(+), 43 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 836f056..d0c1872 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -278,14 +278,16 @@ static void tpm_del_char_device(struct tpm_chip *chip)
 
 static int tpm1_chip_register(struct tpm_chip *chip)
 {
+	int rc;
+
 	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));
+	rc = tpm_bios_log_setup(chip);
 
-	return 0;
+	return rc;
 }
 
 static void tpm1_chip_unregister(struct tpm_chip *chip)
@@ -293,8 +295,7 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		return;
 
-	if (chip->bios_dir)
-		tpm_bios_log_teardown(chip->bios_dir);
+	tpm_bios_log_teardown(chip);
 }
 
 static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f9401ca..9d69580 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -40,6 +40,7 @@ enum tpm_const {
 	TPM_BUFSIZE = 4096,
 	TPM_NUM_DEVICES = 65536,
 	TPM_RETRY = 50,		/* 5 seconds */
+	TPM_NUM_EVENT_LOG_FILES = 3,
 };
 
 enum tpm_timeout {
@@ -171,7 +172,7 @@ struct tpm_chip {
 	unsigned long duration[3]; /* jiffies */
 	bool duration_adjusted;
 
-	struct dentry **bios_dir;
+	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
 
 	const struct attribute_group *groups[3];
 	unsigned int groups_cnt;
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 9467e31..62e9da6 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -368,54 +368,47 @@ static int is_bad(void *p)
 	return 0;
 }
 
-struct dentry **tpm_bios_log_setup(const char *name)
+int tpm_bios_log_setup(struct tpm_chip *chip)
 {
-	struct dentry **ret = NULL, *tpm_dir, *bin_file, *ascii_file;
+	const char *name = dev_name(&chip->dev);
+	unsigned int cnt;
 
-	tpm_dir = securityfs_create_dir(name, NULL);
-	if (is_bad(tpm_dir))
-		goto out;
+	cnt = 0;
+	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
+	if (is_bad(chip->bios_dir[cnt]))
+		goto err;
+	cnt++;
 
-	bin_file =
+	chip->bios_dir[cnt] =
 	    securityfs_create_file("binary_bios_measurements",
-				   0440, tpm_dir,
+				   0440, chip->bios_dir[0],
 				   (void *)&tpm_binary_b_measurements_seqops,
 				   &tpm_bios_measurements_ops);
-	if (is_bad(bin_file))
-		goto out_tpm;
+	if (is_bad(chip->bios_dir[cnt]))
+		goto err;
+	cnt++;
 
-	ascii_file =
+	chip->bios_dir[cnt] =
 	    securityfs_create_file("ascii_bios_measurements",
-				   0440, tpm_dir,
+				   0440, chip->bios_dir[0],
 				   (void *)&tpm_ascii_b_measurements_seqops,
 				   &tpm_bios_measurements_ops);
-	if (is_bad(ascii_file))
-		goto out_bin;
+	if (is_bad(chip->bios_dir[cnt]))
+		goto err;
+	cnt++;
 
-	ret = kmalloc(3 * sizeof(struct dentry *), GFP_KERNEL);
-	if (!ret)
-		goto out_ascii;
-
-	ret[0] = ascii_file;
-	ret[1] = bin_file;
-	ret[2] = tpm_dir;
-
-	return ret;
+	return 0;
 
-out_ascii:
-	securityfs_remove(ascii_file);
-out_bin:
-	securityfs_remove(bin_file);
-out_tpm:
-	securityfs_remove(tpm_dir);
-out:
-	return NULL;
+err:
+	chip->bios_dir[cnt] = NULL;
+	tpm_bios_log_teardown(chip);
+	return -EIO;
 }
 
-void tpm_bios_log_teardown(struct dentry **lst)
+void tpm_bios_log_teardown(struct tpm_chip *chip)
 {
 	int i;
 
-	for (i = 0; i < 3; i++)
-		securityfs_remove(lst[i]);
+	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--)
+		securityfs_remove(chip->bios_dir[i]);
 }
diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
index 8de62b0..fd3357e 100644
--- a/drivers/char/tpm/tpm_eventlog.h
+++ b/drivers/char/tpm/tpm_eventlog.h
@@ -77,14 +77,14 @@ int read_log(struct tpm_bios_log *log);
 
 #if defined(CONFIG_TCG_IBMVTPM) || defined(CONFIG_TCG_IBMVTPM_MODULE) || \
 	defined(CONFIG_ACPI)
-extern struct dentry **tpm_bios_log_setup(const char *);
-extern void tpm_bios_log_teardown(struct dentry **);
+extern int tpm_bios_log_setup(struct tpm_chip *chip);
+extern void tpm_bios_log_teardown(struct tpm_chip *chip);
 #else
-static inline struct dentry **tpm_bios_log_setup(const char *name)
+static inline int tpm_bios_log_setup(struct tpm_chip *chip)
 {
-	return NULL;
+	return 0;
 }
-static inline void tpm_bios_log_teardown(struct dentry **dir)
+static inline void tpm_bios_log_teardown(struct tpm_chip *chip)
 {
 }
 #endif
-- 
2.5.0


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

Re: [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 05:00:50AM -0500, Nayna Jain wrote:
> This commit is based on a commit by Nayna Jain. Replaced dynamically
> allocated bios_dir with a static array as the size is always constant.
> 
> Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

This commit remains unreviewed and tested. I'm in the author role here
so I cannot help with this. If that does not happen soon I cannot put
this into the pull request.

/Jarkko


> ---
>  drivers/char/tpm/tpm-chip.c     |  9 ++++---
>  drivers/char/tpm/tpm.h          |  3 ++-
>  drivers/char/tpm/tpm_eventlog.c | 59 ++++++++++++++++++-----------------------
>  drivers/char/tpm/tpm_eventlog.h | 10 +++----
>  4 files changed, 38 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 836f056..d0c1872 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -278,14 +278,16 @@ static void tpm_del_char_device(struct tpm_chip *chip)
>  
>  static int tpm1_chip_register(struct tpm_chip *chip)
>  {
> +	int rc;
> +
>  	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));
> +	rc = tpm_bios_log_setup(chip);
>  
> -	return 0;
> +	return rc;
>  }
>  
>  static void tpm1_chip_unregister(struct tpm_chip *chip)
> @@ -293,8 +295,7 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>  		return;
>  
> -	if (chip->bios_dir)
> -		tpm_bios_log_teardown(chip->bios_dir);
> +	tpm_bios_log_teardown(chip);
>  }
>  
>  static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index f9401ca..9d69580 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -40,6 +40,7 @@ enum tpm_const {
>  	TPM_BUFSIZE = 4096,
>  	TPM_NUM_DEVICES = 65536,
>  	TPM_RETRY = 50,		/* 5 seconds */
> +	TPM_NUM_EVENT_LOG_FILES = 3,
>  };
>  
>  enum tpm_timeout {
> @@ -171,7 +172,7 @@ struct tpm_chip {
>  	unsigned long duration[3]; /* jiffies */
>  	bool duration_adjusted;
>  
> -	struct dentry **bios_dir;
> +	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
>  
>  	const struct attribute_group *groups[3];
>  	unsigned int groups_cnt;
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 9467e31..62e9da6 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -368,54 +368,47 @@ static int is_bad(void *p)
>  	return 0;
>  }
>  
> -struct dentry **tpm_bios_log_setup(const char *name)
> +int tpm_bios_log_setup(struct tpm_chip *chip)
>  {
> -	struct dentry **ret = NULL, *tpm_dir, *bin_file, *ascii_file;
> +	const char *name = dev_name(&chip->dev);
> +	unsigned int cnt;
>  
> -	tpm_dir = securityfs_create_dir(name, NULL);
> -	if (is_bad(tpm_dir))
> -		goto out;
> +	cnt = 0;
> +	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
> +	if (is_bad(chip->bios_dir[cnt]))
> +		goto err;
> +	cnt++;
>  
> -	bin_file =
> +	chip->bios_dir[cnt] =
>  	    securityfs_create_file("binary_bios_measurements",
> -				   0440, tpm_dir,
> +				   0440, chip->bios_dir[0],
>  				   (void *)&tpm_binary_b_measurements_seqops,
>  				   &tpm_bios_measurements_ops);
> -	if (is_bad(bin_file))
> -		goto out_tpm;
> +	if (is_bad(chip->bios_dir[cnt]))
> +		goto err;
> +	cnt++;
>  
> -	ascii_file =
> +	chip->bios_dir[cnt] =
>  	    securityfs_create_file("ascii_bios_measurements",
> -				   0440, tpm_dir,
> +				   0440, chip->bios_dir[0],
>  				   (void *)&tpm_ascii_b_measurements_seqops,
>  				   &tpm_bios_measurements_ops);
> -	if (is_bad(ascii_file))
> -		goto out_bin;
> +	if (is_bad(chip->bios_dir[cnt]))
> +		goto err;
> +	cnt++;
>  
> -	ret = kmalloc(3 * sizeof(struct dentry *), GFP_KERNEL);
> -	if (!ret)
> -		goto out_ascii;
> -
> -	ret[0] = ascii_file;
> -	ret[1] = bin_file;
> -	ret[2] = tpm_dir;
> -
> -	return ret;
> +	return 0;
>  
> -out_ascii:
> -	securityfs_remove(ascii_file);
> -out_bin:
> -	securityfs_remove(bin_file);
> -out_tpm:
> -	securityfs_remove(tpm_dir);
> -out:
> -	return NULL;
> +err:
> +	chip->bios_dir[cnt] = NULL;
> +	tpm_bios_log_teardown(chip);
> +	return -EIO;
>  }
>  
> -void tpm_bios_log_teardown(struct dentry **lst)
> +void tpm_bios_log_teardown(struct tpm_chip *chip)
>  {
>  	int i;
>  
> -	for (i = 0; i < 3; i++)
> -		securityfs_remove(lst[i]);
> +	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--)
> +		securityfs_remove(chip->bios_dir[i]);
>  }
> diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
> index 8de62b0..fd3357e 100644
> --- a/drivers/char/tpm/tpm_eventlog.h
> +++ b/drivers/char/tpm/tpm_eventlog.h
> @@ -77,14 +77,14 @@ int read_log(struct tpm_bios_log *log);
>  
>  #if defined(CONFIG_TCG_IBMVTPM) || defined(CONFIG_TCG_IBMVTPM_MODULE) || \
>  	defined(CONFIG_ACPI)
> -extern struct dentry **tpm_bios_log_setup(const char *);
> -extern void tpm_bios_log_teardown(struct dentry **);
> +extern int tpm_bios_log_setup(struct tpm_chip *chip);
> +extern void tpm_bios_log_teardown(struct tpm_chip *chip);
>  #else
> -static inline struct dentry **tpm_bios_log_setup(const char *name)
> +static inline int tpm_bios_log_setup(struct tpm_chip *chip)
>  {
> -	return NULL;
> +	return 0;
>  }
> -static inline void tpm_bios_log_teardown(struct dentry **dir)
> +static inline void tpm_bios_log_teardown(struct tpm_chip *chip)
>  {
>  }
>  #endif
> -- 
> 2.5.0
> 

Re: [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Jason Gunthorpe]

On Tue, Nov 22, 2016 at 01:23:33PM +0200, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 05:00:50AM -0500, Nayna Jain wrote:
> > This commit is based on a commit by Nayna Jain. Replaced dynamically
> > allocated bios_dir with a static array as the size is always constant.
> > 
> > Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> This commit remains unreviewed and tested. I'm in the author role here
> so I cannot help with this. If that does not happen soon I cannot put
> this into the pull request.

Nayna must have tested it, looks OK to me..

> > +err:
> > +	chip->bios_dir[cnt] = NULL;
> > +	tpm_bios_log_teardown(chip);
> > +	return -EIO;

Except that return should ideally be PTR_ERR(chip->bios_dir[cnt])

.. and we still set ERR_PTR into bios_dir in the ENODEV case, so the
overall series is still broken if securityfs is compiled out.

Lets fix this all like this - which is a good enough reason to leave the
ENODEV detect alone - just squash this into your patch:

diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 2a15b866ac257a..11bb1138a8282e 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -356,15 +356,6 @@ static const struct file_operations tpm_bios_measurements_ops = {
 	.release = tpm_bios_measurements_release,
 };
 
-static int is_bad(void *p)
-{
-	if (!p)
-		return 1;
-	if (IS_ERR(p) && (PTR_ERR(p) != -ENODEV))
-		return 1;
-	return 0;
-}
-
 static int tpm_read_log(struct tpm_chip *chip)
 {
 	int rc;
@@ -390,7 +381,8 @@ static int tpm_read_log(struct tpm_chip *chip)
  * If an event log is found then the securityfs files are setup to
  * export it to userspace, otherwise nothing is done.
  *
- * Returns -ENODEV if the firmware has no event log.
+ * Returns -ENODEV if the firmware has no event log or securityfs is not
+ * supported.
  */
 int tpm_bios_log_setup(struct tpm_chip *chip)
 {
@@ -407,7 +399,10 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 
 	cnt = 0;
 	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
-	if (is_bad(chip->bios_dir[cnt]))
+	/* NOTE: securityfs_create_dir can return ENODEV if securityfs is
+	 * compiled out. The caller should ignore the ENODEV return code.
+	 */
+	if (IS_ERR(chip->bios_dir[cnt]))
 		goto err;
 	cnt++;
 
@@ -419,7 +414,7 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 				   0440, chip->bios_dir[0],
 				   (void *)&chip->bin_log_seqops,
 				   &tpm_bios_measurements_ops);
-	if (is_bad(chip->bios_dir[cnt]))
+	if (IS_ERR(chip->bios_dir[cnt]))
 		goto err;
 	cnt++;
 
@@ -431,16 +426,17 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 				   0440, chip->bios_dir[0],
 				   (void *)&chip->ascii_log_seqops,
 				   &tpm_bios_measurements_ops);
-	if (is_bad(chip->bios_dir[cnt]))
+	if (IS_ERR(chip->bios_dir[cnt]))
 		goto err;
 	cnt++;
 
 	return 0;
 
 err:
+	rc = PTR_ERR(chip->bios_dir[cnt]);
 	chip->bios_dir[cnt] = NULL;
 	tpm_bios_log_teardown(chip);
-	return -EIO;
+	return rc;
 }
 
 void tpm_bios_log_teardown(struct tpm_chip *chip)

Re: [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Jason Gunthorpe]

On Tue, Nov 22, 2016 at 01:23:33PM +0200, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 05:00:50AM -0500, Nayna Jain wrote:
> > This commit is based on a commit by Nayna Jain. Replaced dynamically
> > allocated bios_dir with a static array as the size is always constant.
> > 
> > Suggested-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> > Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> 
> This commit remains unreviewed and tested. I'm in the author role here
> so I cannot help with this. If that does not happen soon I cannot put
> this into the pull request.

Nayna must have tested it, looks OK to me..

> > +err:
> > +	chip->bios_dir[cnt] = NULL;
> > +	tpm_bios_log_teardown(chip);
> > +	return -EIO;

Except that return should ideally be PTR_ERR(chip->bios_dir[cnt])

.. and we still set ERR_PTR into bios_dir in the ENODEV case, so the
overall series is still broken if securityfs is compiled out.

Lets fix this all like this - which is a good enough reason to leave the
ENODEV detect alone - just squash this into your patch:

diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 2a15b866ac257a..11bb1138a8282e 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -356,15 +356,6 @@ static const struct file_operations tpm_bios_measurements_ops = {
 	.release = tpm_bios_measurements_release,
 };
 
-static int is_bad(void *p)
-{
-	if (!p)
-		return 1;
-	if (IS_ERR(p) && (PTR_ERR(p) != -ENODEV))
-		return 1;
-	return 0;
-}
-
 static int tpm_read_log(struct tpm_chip *chip)
 {
 	int rc;
@@ -390,7 +381,8 @@ static int tpm_read_log(struct tpm_chip *chip)
  * If an event log is found then the securityfs files are setup to
  * export it to userspace, otherwise nothing is done.
  *
- * Returns -ENODEV if the firmware has no event log.
+ * Returns -ENODEV if the firmware has no event log or securityfs is not
+ * supported.
  */
 int tpm_bios_log_setup(struct tpm_chip *chip)
 {
@@ -407,7 +399,10 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 
 	cnt = 0;
 	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
-	if (is_bad(chip->bios_dir[cnt]))
+	/* NOTE: securityfs_create_dir can return ENODEV if securityfs is
+	 * compiled out. The caller should ignore the ENODEV return code.
+	 */
+	if (IS_ERR(chip->bios_dir[cnt]))
 		goto err;
 	cnt++;
 
@@ -419,7 +414,7 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 				   0440, chip->bios_dir[0],
 				   (void *)&chip->bin_log_seqops,
 				   &tpm_bios_measurements_ops);
-	if (is_bad(chip->bios_dir[cnt]))
+	if (IS_ERR(chip->bios_dir[cnt]))
 		goto err;
 	cnt++;
 
@@ -431,16 +426,17 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 				   0440, chip->bios_dir[0],
 				   (void *)&chip->ascii_log_seqops,
 				   &tpm_bios_measurements_ops);
-	if (is_bad(chip->bios_dir[cnt]))
+	if (IS_ERR(chip->bios_dir[cnt]))
 		goto err;
 	cnt++;
 
 	return 0;
 
 err:
+	rc = PTR_ERR(chip->bios_dir[cnt]);
 	chip->bios_dir[cnt] = NULL;
 	tpm_bios_log_teardown(chip);
-	return -EIO;
+	return rc;
 }
 
 void tpm_bios_log_teardown(struct tpm_chip *chip)

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

Re: [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Jarkko Sakkinen]

On Tue, Nov 22, 2016 at 09:58:56AM -0700, Jason Gunthorpe wrote:
> On Tue, Nov 22, 2016 at 01:23:33PM +0200, Jarkko Sakkinen wrote:
> > On Mon, Nov 14, 2016 at 05:00:50AM -0500, Nayna Jain wrote:
> > > This commit is based on a commit by Nayna Jain. Replaced dynamically
> > > allocated bios_dir with a static array as the size is always constant.
> > > 
> > > Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > 
> > This commit remains unreviewed and tested. I'm in the author role here
> > so I cannot help with this. If that does not happen soon I cannot put
> > this into the pull request.
> 
> Nayna must have tested it, looks OK to me..
> 
> > > +err:
> > > +	chip->bios_dir[cnt] = NULL;
> > > +	tpm_bios_log_teardown(chip);
> > > +	return -EIO;
> 
> Except that return should ideally be PTR_ERR(chip->bios_dir[cnt])
> 
> .. and we still set ERR_PTR into bios_dir in the ENODEV case, so the
> overall series is still broken if securityfs is compiled out.
> 
> Lets fix this all like this - which is a good enough reason to leave the
> ENODEV detect alone - just squash this into your patch:

That was a great catch. Thank you.

> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 2a15b866ac257a..11bb1138a8282e 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -356,15 +356,6 @@ static const struct file_operations tpm_bios_measurements_ops = {
>  	.release = tpm_bios_measurements_release,
>  };
>  
> -static int is_bad(void *p)
> -{
> -	if (!p)
> -		return 1;
> -	if (IS_ERR(p) && (PTR_ERR(p) != -ENODEV))
> -		return 1;
> -	return 0;
> -}
> -

This function is only confusing indirection anyway. Does not serve
really any justifiable purpose.

>  static int tpm_read_log(struct tpm_chip *chip)
>  {
>  	int rc;
> @@ -390,7 +381,8 @@ static int tpm_read_log(struct tpm_chip *chip)
>   * If an event log is found then the securityfs files are setup to
>   * export it to userspace, otherwise nothing is done.
>   *
> - * Returns -ENODEV if the firmware has no event log.
> + * Returns -ENODEV if the firmware has no event log or securityfs is not
> + * supported.
>   */
>  int tpm_bios_log_setup(struct tpm_chip *chip)
>  {
> @@ -407,7 +399,10 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
>  
>  	cnt = 0;
>  	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
> -	if (is_bad(chip->bios_dir[cnt]))
> +	/* NOTE: securityfs_create_dir can return ENODEV if securityfs is
> +	 * compiled out. The caller should ignore the ENODEV return code.
> +	 */
> +	if (IS_ERR(chip->bios_dir[cnt]))
>  		goto err;
>  	cnt++;
>  
> @@ -419,7 +414,7 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
>  				   0440, chip->bios_dir[0],
>  				   (void *)&chip->bin_log_seqops,
>  				   &tpm_bios_measurements_ops);
> -	if (is_bad(chip->bios_dir[cnt]))
> +	if (IS_ERR(chip->bios_dir[cnt]))
>  		goto err;
>  	cnt++;
>  
> @@ -431,16 +426,17 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
>  				   0440, chip->bios_dir[0],
>  				   (void *)&chip->ascii_log_seqops,
>  				   &tpm_bios_measurements_ops);
> -	if (is_bad(chip->bios_dir[cnt]))
> +	if (IS_ERR(chip->bios_dir[cnt]))
>  		goto err;
>  	cnt++;
>  
>  	return 0;
>  
>  err:
> +	rc = PTR_ERR(chip->bios_dir[cnt]);
>  	chip->bios_dir[cnt] = NULL;
>  	tpm_bios_log_teardown(chip);
> -	return -EIO;
> +	return rc;
>  }
>  
>  void tpm_bios_log_teardown(struct tpm_chip *chip)

I manually added the changes to:

  tpm: replace dynamically allocated bios_dir with a static array

The code was changed so radically after that patch so it was the
cleanest way.

I need to declare 'rc' in that patch.  I changed also this "int rc = 0;"
to "int rc;" as it does not need to be initialized in the declaration.
This affects:

  tpm: have event log use the tpm_chip

And finally I needed the update this patch too to accomadate the doc
change:

  tpm: Fix handling of missing event log

Could you check through those patches that I didn't blow things up,
which could easily happen given that I needed to update three patches
and give your final reviewed-by if it looks good to you?

In the meanwhile I'll start running sparse, coccicheck etc. for the
release content.

/Jarkko

Re: [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Jarkko Sakkinen]

On Tue, Nov 22, 2016 at 09:58:56AM -0700, Jason Gunthorpe wrote:
> On Tue, Nov 22, 2016 at 01:23:33PM +0200, Jarkko Sakkinen wrote:
> > On Mon, Nov 14, 2016 at 05:00:50AM -0500, Nayna Jain wrote:
> > > This commit is based on a commit by Nayna Jain. Replaced dynamically
> > > allocated bios_dir with a static array as the size is always constant.
> > > 
> > > Suggested-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> > > Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > 
> > This commit remains unreviewed and tested. I'm in the author role here
> > so I cannot help with this. If that does not happen soon I cannot put
> > this into the pull request.
> 
> Nayna must have tested it, looks OK to me..
> 
> > > +err:
> > > +	chip->bios_dir[cnt] = NULL;
> > > +	tpm_bios_log_teardown(chip);
> > > +	return -EIO;
> 
> Except that return should ideally be PTR_ERR(chip->bios_dir[cnt])
> 
> .. and we still set ERR_PTR into bios_dir in the ENODEV case, so the
> overall series is still broken if securityfs is compiled out.
> 
> Lets fix this all like this - which is a good enough reason to leave the
> ENODEV detect alone - just squash this into your patch:

That was a great catch. Thank you.

> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 2a15b866ac257a..11bb1138a8282e 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -356,15 +356,6 @@ static const struct file_operations tpm_bios_measurements_ops = {
>  	.release = tpm_bios_measurements_release,
>  };
>  
> -static int is_bad(void *p)
> -{
> -	if (!p)
> -		return 1;
> -	if (IS_ERR(p) && (PTR_ERR(p) != -ENODEV))
> -		return 1;
> -	return 0;
> -}
> -

This function is only confusing indirection anyway. Does not serve
really any justifiable purpose.

>  static int tpm_read_log(struct tpm_chip *chip)
>  {
>  	int rc;
> @@ -390,7 +381,8 @@ static int tpm_read_log(struct tpm_chip *chip)
>   * If an event log is found then the securityfs files are setup to
>   * export it to userspace, otherwise nothing is done.
>   *
> - * Returns -ENODEV if the firmware has no event log.
> + * Returns -ENODEV if the firmware has no event log or securityfs is not
> + * supported.
>   */
>  int tpm_bios_log_setup(struct tpm_chip *chip)
>  {
> @@ -407,7 +399,10 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
>  
>  	cnt = 0;
>  	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
> -	if (is_bad(chip->bios_dir[cnt]))
> +	/* NOTE: securityfs_create_dir can return ENODEV if securityfs is
> +	 * compiled out. The caller should ignore the ENODEV return code.
> +	 */
> +	if (IS_ERR(chip->bios_dir[cnt]))
>  		goto err;
>  	cnt++;
>  
> @@ -419,7 +414,7 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
>  				   0440, chip->bios_dir[0],
>  				   (void *)&chip->bin_log_seqops,
>  				   &tpm_bios_measurements_ops);
> -	if (is_bad(chip->bios_dir[cnt]))
> +	if (IS_ERR(chip->bios_dir[cnt]))
>  		goto err;
>  	cnt++;
>  
> @@ -431,16 +426,17 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
>  				   0440, chip->bios_dir[0],
>  				   (void *)&chip->ascii_log_seqops,
>  				   &tpm_bios_measurements_ops);
> -	if (is_bad(chip->bios_dir[cnt]))
> +	if (IS_ERR(chip->bios_dir[cnt]))
>  		goto err;
>  	cnt++;
>  
>  	return 0;
>  
>  err:
> +	rc = PTR_ERR(chip->bios_dir[cnt]);
>  	chip->bios_dir[cnt] = NULL;
>  	tpm_bios_log_teardown(chip);
> -	return -EIO;
> +	return rc;
>  }
>  
>  void tpm_bios_log_teardown(struct tpm_chip *chip)

I manually added the changes to:

  tpm: replace dynamically allocated bios_dir with a static array

The code was changed so radically after that patch so it was the
cleanest way.

I need to declare 'rc' in that patch.  I changed also this "int rc = 0;"
to "int rc;" as it does not need to be initialized in the declaration.
This affects:

  tpm: have event log use the tpm_chip

And finally I needed the update this patch too to accomadate the doc
change:

  tpm: Fix handling of missing event log

Could you check through those patches that I didn't blow things up,
which could easily happen given that I needed to update three patches
and give your final reviewed-by if it looks good to you?

In the meanwhile I'll start running sparse, coccicheck etc. for the
release content.

/Jarkko

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

Re: [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Jason Gunthorpe]

On Thu, Nov 24, 2016 at 03:57:23PM +0200, Jarkko Sakkinen wrote:
> I manually added the changes to:
> 
>   tpm: replace dynamically allocated bios_dir with a static array

For this patch..

Could drop 'int rc' from tpm1_chip_register, but it will come back in
a later patch

Could dump TPM_NUM_EVENT_LOG_FILES and just use
ARRAY_SIZE(chip->bios_dir)

Now the the stub for tpm_bios_log_setup can properly return -ENODEV

This is no good at this point in the series - we need the ENODEV
detection in tpm_chip_register() from the 'Fix handle of missing event
log' moved into this patch, because it now returns ENODEV due to
sercurityfs

The commit 'tpm: vtpm_proxy: Do not access host's event log' still
needs a proper commit message - it doesn't match what the patch is
doing at all.

If you are going to be editing the patches I'd suggest squashing all
the bug fix ones with proper credit so it at least makes sense to
read..

Jason

Re: [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Jason Gunthorpe]

On Thu, Nov 24, 2016 at 03:57:23PM +0200, Jarkko Sakkinen wrote:
> I manually added the changes to:
> 
>   tpm: replace dynamically allocated bios_dir with a static array

For this patch..

Could drop 'int rc' from tpm1_chip_register, but it will come back in
a later patch

Could dump TPM_NUM_EVENT_LOG_FILES and just use
ARRAY_SIZE(chip->bios_dir)

Now the the stub for tpm_bios_log_setup can properly return -ENODEV

This is no good at this point in the series - we need the ENODEV
detection in tpm_chip_register() from the 'Fix handle of missing event
log' moved into this patch, because it now returns ENODEV due to
sercurityfs

The commit 'tpm: vtpm_proxy: Do not access host's event log' still
needs a proper commit message - it doesn't match what the patch is
doing at all.

If you are going to be editing the patches I'd suggest squashing all
the bug fix ones with proper credit so it at least makes sense to
read..

Jason

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

Re: [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Jarkko Sakkinen]

On Thu, Nov 24, 2016 at 09:53:13AM -0700, Jason Gunthorpe wrote:
> On Thu, Nov 24, 2016 at 03:57:23PM +0200, Jarkko Sakkinen wrote:
> > I manually added the changes to:
> > 
> >   tpm: replace dynamically allocated bios_dir with a static array
> 
> For this patch..
> 
> Could drop 'int rc' from tpm1_chip_register, but it will come back in
> a later patch
> 
> Could dump TPM_NUM_EVENT_LOG_FILES and just use
> ARRAY_SIZE(chip->bios_dir)

Not a bug fix. Happy take a patch after the pull request.

> Now the the stub for tpm_bios_log_setup can properly return -ENODEV
> 
> This is no good at this point in the series - we need the ENODEV
> detection in tpm_chip_register() from the 'Fix handle of missing event
> log' moved into this patch, because it now returns ENODEV due to
> sercurityfs

Sure it would be cleaner but not really necessary. Do you really think
this is mandatory? No matter how I reorder patches this will require
time and effort to fix various merge conflicts because of the replacemnt
of event log. After that I have to test everything.

Not doing this for light reasons...

> The commit 'tpm: vtpm_proxy: Do not access host's event log' still
> needs a proper commit message - it doesn't match what the patch is
> doing at all.

The commit message otherwise great except for the short summary, which
is now fixed.

> If you are going to be editing the patches I'd suggest squashing all
> the bug fix ones with proper credit so it at least makes sense to
> read..
> 
> Jason

I do not have interest to edit patches more than I have to.

/Jarkko

Re: [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Jarkko Sakkinen]

On Thu, Nov 24, 2016 at 09:53:13AM -0700, Jason Gunthorpe wrote:
> On Thu, Nov 24, 2016 at 03:57:23PM +0200, Jarkko Sakkinen wrote:
> > I manually added the changes to:
> > 
> >   tpm: replace dynamically allocated bios_dir with a static array
> 
> For this patch..
> 
> Could drop 'int rc' from tpm1_chip_register, but it will come back in
> a later patch
> 
> Could dump TPM_NUM_EVENT_LOG_FILES and just use
> ARRAY_SIZE(chip->bios_dir)

Not a bug fix. Happy take a patch after the pull request.

> Now the the stub for tpm_bios_log_setup can properly return -ENODEV
> 
> This is no good at this point in the series - we need the ENODEV
> detection in tpm_chip_register() from the 'Fix handle of missing event
> log' moved into this patch, because it now returns ENODEV due to
> sercurityfs

Sure it would be cleaner but not really necessary. Do you really think
this is mandatory? No matter how I reorder patches this will require
time and effort to fix various merge conflicts because of the replacemnt
of event log. After that I have to test everything.

Not doing this for light reasons...

> The commit 'tpm: vtpm_proxy: Do not access host's event log' still
> needs a proper commit message - it doesn't match what the patch is
> doing at all.

The commit message otherwise great except for the short summary, which
is now fixed.

> If you are going to be editing the patches I'd suggest squashing all
> the bug fix ones with proper credit so it at least makes sense to
> read..
> 
> Jason

I do not have interest to edit patches more than I have to.

/Jarkko

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

Re: [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Jason Gunthorpe]

On Fri, Nov 25, 2016 at 10:08:38AM +0200, Jarkko Sakkinen wrote:

> > This is no good at this point in the series - we need the ENODEV
> > detection in tpm_chip_register() from the 'Fix handle of missing event
> > log' moved into this patch, because it now returns ENODEV due to
> > sercurityfs
> 
> Sure it would be cleaner but not really necessary. Do you really think
> this is mandatory? No matter how I reorder patches this will require
> time and effort to fix various merge conflicts because of the replacemnt
> of event log. After that I have to test everything.

Well, once you started editing patches to fix them you should make
them fully correct so bisection works.

If you applied the patch I gave you on top of the tree then I would
have said to leave it...

> The commit message otherwise great except for the short summary, which
> is now fixed.

It is good now

Jason

Re: [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Jarkko Sakkinen]

On Fri, Nov 25, 2016 at 12:38:13PM -0700, Jason Gunthorpe wrote:
> On Fri, Nov 25, 2016 at 10:08:38AM +0200, Jarkko Sakkinen wrote:
> 
> > > This is no good at this point in the series - we need the ENODEV
> > > detection in tpm_chip_register() from the 'Fix handle of missing event
> > > log' moved into this patch, because it now returns ENODEV due to
> > > sercurityfs
> > 
> > Sure it would be cleaner but not really necessary. Do you really think
> > this is mandatory? No matter how I reorder patches this will require
> > time and effort to fix various merge conflicts because of the replacemnt
> > of event log. After that I have to test everything.
> 
> Well, once you started editing patches to fix them you should make
> them fully correct so bisection works.
> 
> If you applied the patch I gave you on top of the tree then I would
> have said to leave it...

I agree with you on this. I adjusted it to be like that now. Is it good
now?

/Jarkko

Re: [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Nayna]

On 11/22/2016 04:53 PM, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 05:00:50AM -0500, Nayna Jain wrote:
>> This commit is based on a commit by Nayna Jain. Replaced dynamically
>> allocated bios_dir with a static array as the size is always constant.
>>
>> Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
>> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>
> This commit remains unreviewed and tested. I'm in the author role here
> so I cannot help with this. If that does not happen soon I cannot put
> this into the pull request.

Tested-By: Nayna Jain <nayna@linux.vnet.ibm.com>

Thanks & Regards,
   - Nayna

>
> /Jarkko
>
>
>> ---
>>   drivers/char/tpm/tpm-chip.c     |  9 ++++---
>>   drivers/char/tpm/tpm.h          |  3 ++-
>>   drivers/char/tpm/tpm_eventlog.c | 59 ++++++++++++++++++-----------------------
>>   drivers/char/tpm/tpm_eventlog.h | 10 +++----
>>   4 files changed, 38 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>> index 836f056..d0c1872 100644
>> --- a/drivers/char/tpm/tpm-chip.c
>> +++ b/drivers/char/tpm/tpm-chip.c
>> @@ -278,14 +278,16 @@ static void tpm_del_char_device(struct tpm_chip *chip)
>>
>>   static int tpm1_chip_register(struct tpm_chip *chip)
>>   {
>> +	int rc;
>> +
>>   	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));
>> +	rc = tpm_bios_log_setup(chip);
>>
>> -	return 0;
>> +	return rc;
>>   }
>>
>>   static void tpm1_chip_unregister(struct tpm_chip *chip)
>> @@ -293,8 +295,7 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
>>   	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>>   		return;
>>
>> -	if (chip->bios_dir)
>> -		tpm_bios_log_teardown(chip->bios_dir);
>> +	tpm_bios_log_teardown(chip);
>>   }
>>
>>   static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index f9401ca..9d69580 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -40,6 +40,7 @@ enum tpm_const {
>>   	TPM_BUFSIZE = 4096,
>>   	TPM_NUM_DEVICES = 65536,
>>   	TPM_RETRY = 50,		/* 5 seconds */
>> +	TPM_NUM_EVENT_LOG_FILES = 3,
>>   };
>>
>>   enum tpm_timeout {
>> @@ -171,7 +172,7 @@ struct tpm_chip {
>>   	unsigned long duration[3]; /* jiffies */
>>   	bool duration_adjusted;
>>
>> -	struct dentry **bios_dir;
>> +	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
>>
>>   	const struct attribute_group *groups[3];
>>   	unsigned int groups_cnt;
>> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
>> index 9467e31..62e9da6 100644
>> --- a/drivers/char/tpm/tpm_eventlog.c
>> +++ b/drivers/char/tpm/tpm_eventlog.c
>> @@ -368,54 +368,47 @@ static int is_bad(void *p)
>>   	return 0;
>>   }
>>
>> -struct dentry **tpm_bios_log_setup(const char *name)
>> +int tpm_bios_log_setup(struct tpm_chip *chip)
>>   {
>> -	struct dentry **ret = NULL, *tpm_dir, *bin_file, *ascii_file;
>> +	const char *name = dev_name(&chip->dev);
>> +	unsigned int cnt;
>>
>> -	tpm_dir = securityfs_create_dir(name, NULL);
>> -	if (is_bad(tpm_dir))
>> -		goto out;
>> +	cnt = 0;
>> +	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
>> +	if (is_bad(chip->bios_dir[cnt]))
>> +		goto err;
>> +	cnt++;
>>
>> -	bin_file =
>> +	chip->bios_dir[cnt] =
>>   	    securityfs_create_file("binary_bios_measurements",
>> -				   0440, tpm_dir,
>> +				   0440, chip->bios_dir[0],
>>   				   (void *)&tpm_binary_b_measurements_seqops,
>>   				   &tpm_bios_measurements_ops);
>> -	if (is_bad(bin_file))
>> -		goto out_tpm;
>> +	if (is_bad(chip->bios_dir[cnt]))
>> +		goto err;
>> +	cnt++;
>>
>> -	ascii_file =
>> +	chip->bios_dir[cnt] =
>>   	    securityfs_create_file("ascii_bios_measurements",
>> -				   0440, tpm_dir,
>> +				   0440, chip->bios_dir[0],
>>   				   (void *)&tpm_ascii_b_measurements_seqops,
>>   				   &tpm_bios_measurements_ops);
>> -	if (is_bad(ascii_file))
>> -		goto out_bin;
>> +	if (is_bad(chip->bios_dir[cnt]))
>> +		goto err;
>> +	cnt++;
>>
>> -	ret = kmalloc(3 * sizeof(struct dentry *), GFP_KERNEL);
>> -	if (!ret)
>> -		goto out_ascii;
>> -
>> -	ret[0] = ascii_file;
>> -	ret[1] = bin_file;
>> -	ret[2] = tpm_dir;
>> -
>> -	return ret;
>> +	return 0;
>>
>> -out_ascii:
>> -	securityfs_remove(ascii_file);
>> -out_bin:
>> -	securityfs_remove(bin_file);
>> -out_tpm:
>> -	securityfs_remove(tpm_dir);
>> -out:
>> -	return NULL;
>> +err:
>> +	chip->bios_dir[cnt] = NULL;
>> +	tpm_bios_log_teardown(chip);
>> +	return -EIO;
>>   }
>>
>> -void tpm_bios_log_teardown(struct dentry **lst)
>> +void tpm_bios_log_teardown(struct tpm_chip *chip)
>>   {
>>   	int i;
>>
>> -	for (i = 0; i < 3; i++)
>> -		securityfs_remove(lst[i]);
>> +	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--)
>> +		securityfs_remove(chip->bios_dir[i]);
>>   }
>> diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
>> index 8de62b0..fd3357e 100644
>> --- a/drivers/char/tpm/tpm_eventlog.h
>> +++ b/drivers/char/tpm/tpm_eventlog.h
>> @@ -77,14 +77,14 @@ int read_log(struct tpm_bios_log *log);
>>
>>   #if defined(CONFIG_TCG_IBMVTPM) || defined(CONFIG_TCG_IBMVTPM_MODULE) || \
>>   	defined(CONFIG_ACPI)
>> -extern struct dentry **tpm_bios_log_setup(const char *);
>> -extern void tpm_bios_log_teardown(struct dentry **);
>> +extern int tpm_bios_log_setup(struct tpm_chip *chip);
>> +extern void tpm_bios_log_teardown(struct tpm_chip *chip);
>>   #else
>> -static inline struct dentry **tpm_bios_log_setup(const char *name)
>> +static inline int tpm_bios_log_setup(struct tpm_chip *chip)
>>   {
>> -	return NULL;
>> +	return 0;
>>   }
>> -static inline void tpm_bios_log_teardown(struct dentry **dir)
>> +static inline void tpm_bios_log_teardown(struct tpm_chip *chip)
>>   {
>>   }
>>   #endif
>> --
>> 2.5.0
>>
>

Re: [PATCH v6 3/9] tpm: replace dynamically allocated bios_dir with a static array

[Jarkko Sakkinen]

On Wed, Nov 23, 2016 at 12:56:19AM +0530, Nayna wrote:
> 
> 
> On 11/22/2016 04:53 PM, Jarkko Sakkinen wrote:
> > On Mon, Nov 14, 2016 at 05:00:50AM -0500, Nayna Jain wrote:
> > > This commit is based on a commit by Nayna Jain. Replaced dynamically
> > > allocated bios_dir with a static array as the size is always constant.
> > > 
> > > Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > 
> > This commit remains unreviewed and tested. I'm in the author role here
> > so I cannot help with this. If that does not happen soon I cannot put
> > this into the pull request.
> 
> Tested-By: Nayna Jain <nayna@linux.vnet.ibm.com>

Thanks.

/Jarkko

> 
> Thanks & Regards,
>   - Nayna
> 
> > 
> > /Jarkko
> > 
> > 
> > > ---
> > >   drivers/char/tpm/tpm-chip.c     |  9 ++++---
> > >   drivers/char/tpm/tpm.h          |  3 ++-
> > >   drivers/char/tpm/tpm_eventlog.c | 59 ++++++++++++++++++-----------------------
> > >   drivers/char/tpm/tpm_eventlog.h | 10 +++----
> > >   4 files changed, 38 insertions(+), 43 deletions(-)
> > > 
> > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > > index 836f056..d0c1872 100644
> > > --- a/drivers/char/tpm/tpm-chip.c
> > > +++ b/drivers/char/tpm/tpm-chip.c
> > > @@ -278,14 +278,16 @@ static void tpm_del_char_device(struct tpm_chip *chip)
> > > 
> > >   static int tpm1_chip_register(struct tpm_chip *chip)
> > >   {
> > > +	int rc;
> > > +
> > >   	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));
> > > +	rc = tpm_bios_log_setup(chip);
> > > 
> > > -	return 0;
> > > +	return rc;
> > >   }
> > > 
> > >   static void tpm1_chip_unregister(struct tpm_chip *chip)
> > > @@ -293,8 +295,7 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
> > >   	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > >   		return;
> > > 
> > > -	if (chip->bios_dir)
> > > -		tpm_bios_log_teardown(chip->bios_dir);
> > > +	tpm_bios_log_teardown(chip);
> > >   }
> > > 
> > >   static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > index f9401ca..9d69580 100644
> > > --- a/drivers/char/tpm/tpm.h
> > > +++ b/drivers/char/tpm/tpm.h
> > > @@ -40,6 +40,7 @@ enum tpm_const {
> > >   	TPM_BUFSIZE = 4096,
> > >   	TPM_NUM_DEVICES = 65536,
> > >   	TPM_RETRY = 50,		/* 5 seconds */
> > > +	TPM_NUM_EVENT_LOG_FILES = 3,
> > >   };
> > > 
> > >   enum tpm_timeout {
> > > @@ -171,7 +172,7 @@ struct tpm_chip {
> > >   	unsigned long duration[3]; /* jiffies */
> > >   	bool duration_adjusted;
> > > 
> > > -	struct dentry **bios_dir;
> > > +	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
> > > 
> > >   	const struct attribute_group *groups[3];
> > >   	unsigned int groups_cnt;
> > > diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> > > index 9467e31..62e9da6 100644
> > > --- a/drivers/char/tpm/tpm_eventlog.c
> > > +++ b/drivers/char/tpm/tpm_eventlog.c
> > > @@ -368,54 +368,47 @@ static int is_bad(void *p)
> > >   	return 0;
> > >   }
> > > 
> > > -struct dentry **tpm_bios_log_setup(const char *name)
> > > +int tpm_bios_log_setup(struct tpm_chip *chip)
> > >   {
> > > -	struct dentry **ret = NULL, *tpm_dir, *bin_file, *ascii_file;
> > > +	const char *name = dev_name(&chip->dev);
> > > +	unsigned int cnt;
> > > 
> > > -	tpm_dir = securityfs_create_dir(name, NULL);
> > > -	if (is_bad(tpm_dir))
> > > -		goto out;
> > > +	cnt = 0;
> > > +	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
> > > +	if (is_bad(chip->bios_dir[cnt]))
> > > +		goto err;
> > > +	cnt++;
> > > 
> > > -	bin_file =
> > > +	chip->bios_dir[cnt] =
> > >   	    securityfs_create_file("binary_bios_measurements",
> > > -				   0440, tpm_dir,
> > > +				   0440, chip->bios_dir[0],
> > >   				   (void *)&tpm_binary_b_measurements_seqops,
> > >   				   &tpm_bios_measurements_ops);
> > > -	if (is_bad(bin_file))
> > > -		goto out_tpm;
> > > +	if (is_bad(chip->bios_dir[cnt]))
> > > +		goto err;
> > > +	cnt++;
> > > 
> > > -	ascii_file =
> > > +	chip->bios_dir[cnt] =
> > >   	    securityfs_create_file("ascii_bios_measurements",
> > > -				   0440, tpm_dir,
> > > +				   0440, chip->bios_dir[0],
> > >   				   (void *)&tpm_ascii_b_measurements_seqops,
> > >   				   &tpm_bios_measurements_ops);
> > > -	if (is_bad(ascii_file))
> > > -		goto out_bin;
> > > +	if (is_bad(chip->bios_dir[cnt]))
> > > +		goto err;
> > > +	cnt++;
> > > 
> > > -	ret = kmalloc(3 * sizeof(struct dentry *), GFP_KERNEL);
> > > -	if (!ret)
> > > -		goto out_ascii;
> > > -
> > > -	ret[0] = ascii_file;
> > > -	ret[1] = bin_file;
> > > -	ret[2] = tpm_dir;
> > > -
> > > -	return ret;
> > > +	return 0;
> > > 
> > > -out_ascii:
> > > -	securityfs_remove(ascii_file);
> > > -out_bin:
> > > -	securityfs_remove(bin_file);
> > > -out_tpm:
> > > -	securityfs_remove(tpm_dir);
> > > -out:
> > > -	return NULL;
> > > +err:
> > > +	chip->bios_dir[cnt] = NULL;
> > > +	tpm_bios_log_teardown(chip);
> > > +	return -EIO;
> > >   }
> > > 
> > > -void tpm_bios_log_teardown(struct dentry **lst)
> > > +void tpm_bios_log_teardown(struct tpm_chip *chip)
> > >   {
> > >   	int i;
> > > 
> > > -	for (i = 0; i < 3; i++)
> > > -		securityfs_remove(lst[i]);
> > > +	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--)
> > > +		securityfs_remove(chip->bios_dir[i]);
> > >   }
> > > diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
> > > index 8de62b0..fd3357e 100644
> > > --- a/drivers/char/tpm/tpm_eventlog.h
> > > +++ b/drivers/char/tpm/tpm_eventlog.h
> > > @@ -77,14 +77,14 @@ int read_log(struct tpm_bios_log *log);
> > > 
> > >   #if defined(CONFIG_TCG_IBMVTPM) || defined(CONFIG_TCG_IBMVTPM_MODULE) || \
> > >   	defined(CONFIG_ACPI)
> > > -extern struct dentry **tpm_bios_log_setup(const char *);
> > > -extern void tpm_bios_log_teardown(struct dentry **);
> > > +extern int tpm_bios_log_setup(struct tpm_chip *chip);
> > > +extern void tpm_bios_log_teardown(struct tpm_chip *chip);
> > >   #else
> > > -static inline struct dentry **tpm_bios_log_setup(const char *name)
> > > +static inline int tpm_bios_log_setup(struct tpm_chip *chip)
> > >   {
> > > -	return NULL;
> > > +	return 0;
> > >   }
> > > -static inline void tpm_bios_log_teardown(struct dentry **dir)
> > > +static inline void tpm_bios_log_teardown(struct tpm_chip *chip)
> > >   {
> > >   }
> > >   #endif
> > > --
> > > 2.5.0
> > > 
> > 
> 

[PATCH v6 4/9] tpm: drop tpm1_chip_register(/unregister)

[Nayna Jain]

Check for TPM2 chip in tpm_sysfs_add_device, tpm_bios_log_setup and
tpm_bios_log_teardown in order to make code flow cleaner and to enable
to implement TPM 2.0 support later on. This is partially derived from
the commit by Nayna Jain with the extension that also tpm1_chip_register
is dropped.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-chip.c     | 31 +++++--------------------------
 drivers/char/tpm/tpm-sysfs.c    |  3 +++
 drivers/char/tpm/tpm_eventlog.c |  3 +++
 3 files changed, 11 insertions(+), 26 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index d0c1872..250a651 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -276,28 +276,6 @@ static void tpm_del_char_device(struct tpm_chip *chip)
 	up_write(&chip->ops_sem);
 }
 
-static int tpm1_chip_register(struct tpm_chip *chip)
-{
-	int rc;
-
-	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		return 0;
-
-	tpm_sysfs_add_device(chip);
-
-	rc = tpm_bios_log_setup(chip);
-
-	return rc;
-}
-
-static void tpm1_chip_unregister(struct tpm_chip *chip)
-{
-	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		return;
-
-	tpm_bios_log_teardown(chip);
-}
-
 static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
 {
 	struct attribute **i;
@@ -364,7 +342,9 @@ int tpm_chip_register(struct tpm_chip *chip)
 			return rc;
 	}
 
-	rc = tpm1_chip_register(chip);
+	tpm_sysfs_add_device(chip);
+
+	rc = tpm_bios_log_setup(chip);
 	if (rc)
 		return rc;
 
@@ -372,7 +352,7 @@ int tpm_chip_register(struct tpm_chip *chip)
 
 	rc = tpm_add_char_device(chip);
 	if (rc) {
-		tpm1_chip_unregister(chip);
+		tpm_bios_log_teardown(chip);
 		return rc;
 	}
 
@@ -402,8 +382,7 @@ EXPORT_SYMBOL_GPL(tpm_chip_register);
 void tpm_chip_unregister(struct tpm_chip *chip)
 {
 	tpm_del_legacy_sysfs(chip);
-
-	tpm1_chip_unregister(chip);
+	tpm_bios_log_teardown(chip);
 	tpm_del_char_device(chip);
 }
 EXPORT_SYMBOL_GPL(tpm_chip_unregister);
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 59a1ead..848ad65 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -284,6 +284,9 @@ static const struct attribute_group tpm_dev_group = {
 
 void tpm_sysfs_add_device(struct tpm_chip *chip)
 {
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		return;
+
 	/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
 	 * is called before ops is null'd and the sysfs core synchronizes this
 	 * removal so that no callbacks are running or can run again
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 62e9da6..57ac862 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -373,6 +373,9 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 	const char *name = dev_name(&chip->dev);
 	unsigned int cnt;
 
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		return 0;
+
 	cnt = 0;
 	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
 	if (is_bad(chip->bios_dir[cnt]))
-- 
2.5.0

[PATCH v6 4/9] tpm: drop tpm1_chip_register(/unregister)

[Nayna Jain]

Check for TPM2 chip in tpm_sysfs_add_device, tpm_bios_log_setup and
tpm_bios_log_teardown in order to make code flow cleaner and to enable
to implement TPM 2.0 support later on. This is partially derived from
the commit by Nayna Jain with the extension that also tpm1_chip_register
is dropped.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/char/tpm/tpm-chip.c     | 31 +++++--------------------------
 drivers/char/tpm/tpm-sysfs.c    |  3 +++
 drivers/char/tpm/tpm_eventlog.c |  3 +++
 3 files changed, 11 insertions(+), 26 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index d0c1872..250a651 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -276,28 +276,6 @@ static void tpm_del_char_device(struct tpm_chip *chip)
 	up_write(&chip->ops_sem);
 }
 
-static int tpm1_chip_register(struct tpm_chip *chip)
-{
-	int rc;
-
-	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		return 0;
-
-	tpm_sysfs_add_device(chip);
-
-	rc = tpm_bios_log_setup(chip);
-
-	return rc;
-}
-
-static void tpm1_chip_unregister(struct tpm_chip *chip)
-{
-	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		return;
-
-	tpm_bios_log_teardown(chip);
-}
-
 static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
 {
 	struct attribute **i;
@@ -364,7 +342,9 @@ int tpm_chip_register(struct tpm_chip *chip)
 			return rc;
 	}
 
-	rc = tpm1_chip_register(chip);
+	tpm_sysfs_add_device(chip);
+
+	rc = tpm_bios_log_setup(chip);
 	if (rc)
 		return rc;
 
@@ -372,7 +352,7 @@ int tpm_chip_register(struct tpm_chip *chip)
 
 	rc = tpm_add_char_device(chip);
 	if (rc) {
-		tpm1_chip_unregister(chip);
+		tpm_bios_log_teardown(chip);
 		return rc;
 	}
 
@@ -402,8 +382,7 @@ EXPORT_SYMBOL_GPL(tpm_chip_register);
 void tpm_chip_unregister(struct tpm_chip *chip)
 {
 	tpm_del_legacy_sysfs(chip);
-
-	tpm1_chip_unregister(chip);
+	tpm_bios_log_teardown(chip);
 	tpm_del_char_device(chip);
 }
 EXPORT_SYMBOL_GPL(tpm_chip_unregister);
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 59a1ead..848ad65 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -284,6 +284,9 @@ static const struct attribute_group tpm_dev_group = {
 
 void tpm_sysfs_add_device(struct tpm_chip *chip)
 {
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		return;
+
 	/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
 	 * is called before ops is null'd and the sysfs core synchronizes this
 	 * removal so that no callbacks are running or can run again
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 62e9da6..57ac862 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -373,6 +373,9 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 	const char *name = dev_name(&chip->dev);
 	unsigned int cnt;
 
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		return 0;
+
 	cnt = 0;
 	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
 	if (is_bad(chip->bios_dir[cnt]))
-- 
2.5.0


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

Re: [PATCH v6 4/9] tpm: drop tpm1_chip_register(/unregister)

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 05:00:51AM -0500, Nayna Jain wrote:
> Check for TPM2 chip in tpm_sysfs_add_device, tpm_bios_log_setup and
> tpm_bios_log_teardown in order to make code flow cleaner and to enable
> to implement TPM 2.0 support later on. This is partially derived from
> the commit by Nayna Jain with the extension that also tpm1_chip_register
> is dropped.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

This commit remains unreviewed and tested. I'm in the author role here
so I cannot help with this. If that does not happen soon I cannot put
this into the pull request.

/Jarkko

> ---
>  drivers/char/tpm/tpm-chip.c     | 31 +++++--------------------------
>  drivers/char/tpm/tpm-sysfs.c    |  3 +++
>  drivers/char/tpm/tpm_eventlog.c |  3 +++
>  3 files changed, 11 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index d0c1872..250a651 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -276,28 +276,6 @@ static void tpm_del_char_device(struct tpm_chip *chip)
>  	up_write(&chip->ops_sem);
>  }
>  
> -static int tpm1_chip_register(struct tpm_chip *chip)
> -{
> -	int rc;
> -
> -	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> -		return 0;
> -
> -	tpm_sysfs_add_device(chip);
> -
> -	rc = tpm_bios_log_setup(chip);
> -
> -	return rc;
> -}
> -
> -static void tpm1_chip_unregister(struct tpm_chip *chip)
> -{
> -	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> -		return;
> -
> -	tpm_bios_log_teardown(chip);
> -}
> -
>  static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
>  {
>  	struct attribute **i;
> @@ -364,7 +342,9 @@ int tpm_chip_register(struct tpm_chip *chip)
>  			return rc;
>  	}
>  
> -	rc = tpm1_chip_register(chip);
> +	tpm_sysfs_add_device(chip);
> +
> +	rc = tpm_bios_log_setup(chip);
>  	if (rc)
>  		return rc;
>  
> @@ -372,7 +352,7 @@ int tpm_chip_register(struct tpm_chip *chip)
>  
>  	rc = tpm_add_char_device(chip);
>  	if (rc) {
> -		tpm1_chip_unregister(chip);
> +		tpm_bios_log_teardown(chip);
>  		return rc;
>  	}
>  
> @@ -402,8 +382,7 @@ EXPORT_SYMBOL_GPL(tpm_chip_register);
>  void tpm_chip_unregister(struct tpm_chip *chip)
>  {
>  	tpm_del_legacy_sysfs(chip);
> -
> -	tpm1_chip_unregister(chip);
> +	tpm_bios_log_teardown(chip);
>  	tpm_del_char_device(chip);
>  }
>  EXPORT_SYMBOL_GPL(tpm_chip_unregister);
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index 59a1ead..848ad65 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -284,6 +284,9 @@ static const struct attribute_group tpm_dev_group = {
>  
>  void tpm_sysfs_add_device(struct tpm_chip *chip)
>  {
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		return;
> +
>  	/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
>  	 * is called before ops is null'd and the sysfs core synchronizes this
>  	 * removal so that no callbacks are running or can run again
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 62e9da6..57ac862 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -373,6 +373,9 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
>  	const char *name = dev_name(&chip->dev);
>  	unsigned int cnt;
>  
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		return 0;
> +
>  	cnt = 0;
>  	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
>  	if (is_bad(chip->bios_dir[cnt]))
> -- 
> 2.5.0
> 

Re: [PATCH v6 4/9] tpm: drop tpm1_chip_register(/unregister)

[Jason Gunthorpe]

On Tue, Nov 22, 2016 at 01:22:00PM +0200, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 05:00:51AM -0500, Nayna Jain wrote:
> > Check for TPM2 chip in tpm_sysfs_add_device, tpm_bios_log_setup and
> > tpm_bios_log_teardown in order to make code flow cleaner and to enable
> > to implement TPM 2.0 support later on. This is partially derived from
> > the commit by Nayna Jain with the extension that also tpm1_chip_register
> > is dropped.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> This commit remains unreviewed and tested. I'm in the author role here
> so I cannot help with this. If that does not happen soon I cannot put
> this into the pull request.

I tested it on my ARM system when I tested your branch.

I think it looks better this way..

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

Jason

Re: [PATCH v6 4/9] tpm: drop tpm1_chip_register(/unregister)

[Jason Gunthorpe]

On Tue, Nov 22, 2016 at 01:22:00PM +0200, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 05:00:51AM -0500, Nayna Jain wrote:
> > Check for TPM2 chip in tpm_sysfs_add_device, tpm_bios_log_setup and
> > tpm_bios_log_teardown in order to make code flow cleaner and to enable
> > to implement TPM 2.0 support later on. This is partially derived from
> > the commit by Nayna Jain with the extension that also tpm1_chip_register
> > is dropped.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> 
> This commit remains unreviewed and tested. I'm in the author role here
> so I cannot help with this. If that does not happen soon I cannot put
> this into the pull request.

I tested it on my ARM system when I tested your branch.

I think it looks better this way..

Reviewed-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

Jason

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

Re: [PATCH v6 4/9] tpm: drop tpm1_chip_register(/unregister)

[Nayna]

On 11/22/2016 10:12 PM, Jason Gunthorpe wrote:
> On Tue, Nov 22, 2016 at 01:22:00PM +0200, Jarkko Sakkinen wrote:
>> On Mon, Nov 14, 2016 at 05:00:51AM -0500, Nayna Jain wrote:
>>> Check for TPM2 chip in tpm_sysfs_add_device, tpm_bios_log_setup and
>>> tpm_bios_log_teardown in order to make code flow cleaner and to enable
>>> to implement TPM 2.0 support later on. This is partially derived from
>>> the commit by Nayna Jain with the extension that also tpm1_chip_register
>>> is dropped.
>>>
>>> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>>
>> This commit remains unreviewed and tested. I'm in the author role here
>> so I cannot help with this. If that does not happen soon I cannot put
>> this into the pull request.
>
> I tested it on my ARM system when I tested your branch.
>
> I think it looks better this way..
>
> Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>
> Jason
>

 From my side also, as part of testing the whole patchset, I have tested 
this patch.

Tested-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Reviewed-by: Nayna Jain <nayna@linux.vnet.ibm.com>

Thanks & Regards,
   - Nayna

Re: [PATCH v6 4/9] tpm: drop tpm1_chip_register(/unregister)

[Nayna]

On 11/22/2016 10:12 PM, Jason Gunthorpe wrote:
> On Tue, Nov 22, 2016 at 01:22:00PM +0200, Jarkko Sakkinen wrote:
>> On Mon, Nov 14, 2016 at 05:00:51AM -0500, Nayna Jain wrote:
>>> Check for TPM2 chip in tpm_sysfs_add_device, tpm_bios_log_setup and
>>> tpm_bios_log_teardown in order to make code flow cleaner and to enable
>>> to implement TPM 2.0 support later on. This is partially derived from
>>> the commit by Nayna Jain with the extension that also tpm1_chip_register
>>> is dropped.
>>>
>>> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>
>> This commit remains unreviewed and tested. I'm in the author role here
>> so I cannot help with this. If that does not happen soon I cannot put
>> this into the pull request.
>
> I tested it on my ARM system when I tested your branch.
>
> I think it looks better this way..
>
> Reviewed-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
>
> Jason
>

 From my side also, as part of testing the whole patchset, I have tested 
this patch.

Tested-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Reviewed-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

Thanks & Regards,
   - Nayna


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

Re: [PATCH v6 4/9] tpm: drop tpm1_chip_register(/unregister)

[Jarkko Sakkinen]

On Thu, Nov 24, 2016 at 12:03:39AM +0530, Nayna wrote:
> 
> 
> On 11/22/2016 10:12 PM, Jason Gunthorpe wrote:
> > On Tue, Nov 22, 2016 at 01:22:00PM +0200, Jarkko Sakkinen wrote:
> > > On Mon, Nov 14, 2016 at 05:00:51AM -0500, Nayna Jain wrote:
> > > > Check for TPM2 chip in tpm_sysfs_add_device, tpm_bios_log_setup and
> > > > tpm_bios_log_teardown in order to make code flow cleaner and to enable
> > > > to implement TPM 2.0 support later on. This is partially derived from
> > > > the commit by Nayna Jain with the extension that also tpm1_chip_register
> > > > is dropped.
> > > > 
> > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > 
> > > This commit remains unreviewed and tested. I'm in the author role here
> > > so I cannot help with this. If that does not happen soon I cannot put
> > > this into the pull request.
> > 
> > I tested it on my ARM system when I tested your branch.
> > 
> > I think it looks better this way..
> > 
> > Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > 
> > Jason
> > 
> 
> From my side also, as part of testing the whole patchset, I have tested this
> patch.
> 
> Tested-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> Reviewed-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> 
> Thanks & Regards,
>   - Nayna

Thanks Nayna! I'll go on creating pull request...

/Jarkko

Re: [PATCH v6 4/9] tpm: drop tpm1_chip_register(/unregister)

[Jarkko Sakkinen]

On Tue, Nov 22, 2016 at 09:42:40AM -0700, Jason Gunthorpe wrote:
> On Tue, Nov 22, 2016 at 01:22:00PM +0200, Jarkko Sakkinen wrote:
> > On Mon, Nov 14, 2016 at 05:00:51AM -0500, Nayna Jain wrote:
> > > Check for TPM2 chip in tpm_sysfs_add_device, tpm_bios_log_setup and
> > > tpm_bios_log_teardown in order to make code flow cleaner and to enable
> > > to implement TPM 2.0 support later on. This is partially derived from
> > > the commit by Nayna Jain with the extension that also tpm1_chip_register
> > > is dropped.
> > > 
> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > 
> > This commit remains unreviewed and tested. I'm in the author role here
> > so I cannot help with this. If that does not happen soon I cannot put
> > this into the pull request.
> 
> I tested it on my ARM system when I tested your branch.
> 
> I think it looks better this way..
> 
> Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

Thanks. I'll also add tested-by from you then.

/Jarkko

Re: [PATCH v6 4/9] tpm: drop tpm1_chip_register(/unregister)

[Jarkko Sakkinen]

On Tue, Nov 22, 2016 at 09:42:40AM -0700, Jason Gunthorpe wrote:
> On Tue, Nov 22, 2016 at 01:22:00PM +0200, Jarkko Sakkinen wrote:
> > On Mon, Nov 14, 2016 at 05:00:51AM -0500, Nayna Jain wrote:
> > > Check for TPM2 chip in tpm_sysfs_add_device, tpm_bios_log_setup and
> > > tpm_bios_log_teardown in order to make code flow cleaner and to enable
> > > to implement TPM 2.0 support later on. This is partially derived from
> > > the commit by Nayna Jain with the extension that also tpm1_chip_register
> > > is dropped.
> > > 
> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > 
> > This commit remains unreviewed and tested. I'm in the author role here
> > so I cannot help with this. If that does not happen soon I cannot put
> > this into the pull request.
> 
> I tested it on my ARM system when I tested your branch.
> 
> I think it looks better this way..
> 
> Reviewed-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

Thanks. I'll also add tested-by from you then.

/Jarkko

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

[PATCH v6 5/9] tpm: have event log use the tpm_chip

[Nayna Jain]

Move the backing memory for the event log into tpm_chip and push
the tpm_chip into read_log. This optimizes read_log processing by
only doing it once and prepares things for the next patches in the
series which require the tpm_chip to locate the event log via
ACPI and OF handles instead of searching.

This is straightfoward except for the issue of passing a kref through
i_private with securityfs. Since securityfs_remove does not have any
removal fencing like sysfs we use the inode lock to safely get a
kref on the tpm_chip.

Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm-chip.c     |  3 +-
 drivers/char/tpm/tpm.h          | 11 ++++++
 drivers/char/tpm/tpm_acpi.c     | 15 +++++--
 drivers/char/tpm/tpm_eventlog.c | 88 ++++++++++++++++++++++++++---------------
 drivers/char/tpm/tpm_eventlog.h |  2 +-
 drivers/char/tpm/tpm_of.c       |  4 +-
 6 files changed, 85 insertions(+), 38 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 250a651..3f27753 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -127,6 +127,7 @@ static void tpm_dev_release(struct device *dev)
 	idr_remove(&dev_nums_idr, chip->dev_num);
 	mutex_unlock(&idr_lock);
 
+	kfree(chip->log.bios_event_log);
 	kfree(chip);
 }
 
@@ -345,7 +346,7 @@ int tpm_chip_register(struct tpm_chip *chip)
 	tpm_sysfs_add_device(chip);
 
 	rc = tpm_bios_log_setup(chip);
-	if (rc)
+	if (rc == -ENODEV)
 		return rc;
 
 	tpm_add_ppi(chip);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 9d69580..1ae9768 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -35,6 +35,8 @@
 #include <linux/cdev.h>
 #include <linux/highmem.h>
 
+#include "tpm_eventlog.h"
+
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */
 	TPM_BUFSIZE = 4096,
@@ -146,6 +148,11 @@ enum tpm_chip_flags {
 	TPM_CHIP_FLAG_HAVE_TIMEOUTS	= BIT(4),
 };
 
+struct tpm_chip_seqops {
+	struct tpm_chip *chip;
+	const struct seq_operations *seqops;
+};
+
 struct tpm_chip {
 	struct device dev;
 	struct cdev cdev;
@@ -157,6 +164,10 @@ struct tpm_chip {
 	struct rw_semaphore ops_sem;
 	const struct tpm_class_ops *ops;
 
+	struct tpm_bios_log log;
+	struct tpm_chip_seqops bin_log_seqops;
+	struct tpm_chip_seqops ascii_log_seqops;
+
 	unsigned int flags;
 
 	int dev_num;		/* /dev/tpm# */
diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
index 565a947..01dfb35 100644
--- a/drivers/char/tpm/tpm_acpi.c
+++ b/drivers/char/tpm/tpm_acpi.c
@@ -9,7 +9,7 @@
  *
  * Maintained by: <tpmdd-devel@lists.sourceforge.net>
  *
- * Access to the eventlog extended by the TCG BIOS of PC platform
+ * Access to the event log extended by the TCG BIOS of PC platform
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -45,13 +45,15 @@ struct acpi_tcpa {
 };
 
 /* read binary bios log */
-int read_log(struct tpm_bios_log *log)
+int read_log(struct tpm_chip *chip)
 {
 	struct acpi_tcpa *buff;
 	acpi_status status;
 	void __iomem *virt;
 	u64 len, start;
+	struct tpm_bios_log *log;
 
+	log = &chip->log;
 	if (log->bios_event_log != NULL) {
 		printk(KERN_ERR
 		       "%s: ERROR - Eventlog already initialized\n",
@@ -97,13 +99,18 @@ int read_log(struct tpm_bios_log *log)
 
 	virt = acpi_os_map_iomem(start, len);
 	if (!virt) {
-		kfree(log->bios_event_log);
 		printk("%s: ERROR - Unable to map memory\n", __func__);
-		return -EIO;
+		goto err;
 	}
 
 	memcpy_fromio(log->bios_event_log, virt, len);
 
 	acpi_os_unmap_iomem(virt, len);
 	return 0;
+
+err:
+	kfree(log->bios_event_log);
+	log->bios_event_log = NULL;
+	return -EIO;
+
 }
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 57ac862..f8c42fe 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -11,7 +11,7 @@
  *
  * Maintained by: <tpmdd-devel@lists.sourceforge.net>
  *
- * Access to the eventlog created by a system's firmware / BIOS
+ * Access to the event log created by a system's firmware / BIOS
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -73,7 +73,8 @@ static const char* tcpa_pc_event_id_strings[] = {
 static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
 {
 	loff_t i;
-	struct tpm_bios_log *log = m->private;
+	struct tpm_chip *chip = m->private;
+	struct tpm_bios_log *log = &chip->log;
 	void *addr = log->bios_event_log;
 	void *limit = log->bios_event_log_end;
 	struct tcpa_event *event;
@@ -120,7 +121,8 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,
 					loff_t *pos)
 {
 	struct tcpa_event *event = v;
-	struct tpm_bios_log *log = m->private;
+	struct tpm_chip *chip = m->private;
+	struct tpm_bios_log *log = &chip->log;
 	void *limit = log->bios_event_log_end;
 	u32 converted_event_size;
 	u32 converted_event_type;
@@ -261,13 +263,10 @@ static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v)
 static int tpm_bios_measurements_release(struct inode *inode,
 					 struct file *file)
 {
-	struct seq_file *seq = file->private_data;
-	struct tpm_bios_log *log = seq->private;
+	struct seq_file *seq = (struct seq_file *)file->private_data;
+	struct tpm_chip *chip = (struct tpm_chip *)seq->private;
 
-	if (log) {
-		kfree(log->bios_event_log);
-		kfree(log);
-	}
+	put_device(&chip->dev);
 
 	return seq_release(inode, file);
 }
@@ -323,33 +322,30 @@ static int tpm_bios_measurements_open(struct inode *inode,
 					    struct file *file)
 {
 	int err;
-	struct tpm_bios_log *log;
 	struct seq_file *seq;
-	const struct seq_operations *seqops =
-		(const struct seq_operations *)inode->i_private;
-
-	log = kzalloc(sizeof(struct tpm_bios_log), GFP_KERNEL);
-	if (!log)
-		return -ENOMEM;
-
-	if ((err = read_log(log)))
-		goto out_free;
+	struct tpm_chip_seqops *chip_seqops;
+	const struct seq_operations *seqops;
+	struct tpm_chip *chip;
+
+	inode_lock(inode);
+	if (!inode->i_private) {
+		inode_unlock(inode);
+		return -ENODEV;
+	}
+	chip_seqops = (struct tpm_chip_seqops *)inode->i_private;
+	seqops = chip_seqops->seqops;
+	chip = chip_seqops->chip;
+	get_device(&chip->dev);
+	inode_unlock(inode);
 
 	/* now register seq file */
 	err = seq_open(file, seqops);
 	if (!err) {
 		seq = file->private_data;
-		seq->private = log;
-	} else {
-		goto out_free;
+		seq->private = chip;
 	}
 
-out:
 	return err;
-out_free:
-	kfree(log->bios_event_log);
-	kfree(log);
-	goto out;
 }
 
 static const struct file_operations tpm_bios_measurements_ops = {
@@ -372,29 +368,47 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 {
 	const char *name = dev_name(&chip->dev);
 	unsigned int cnt;
+	int rc = 0;
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		return 0;
 
+	rc = read_log(chip);
+	/*
+	 * read_log failure means event log is not supported except for ENOMEM.
+	 */
+	if (rc < 0) {
+		if (rc == -ENOMEM)
+			return -ENODEV;
+		else
+			return rc;
+	}
+
 	cnt = 0;
 	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
 	if (is_bad(chip->bios_dir[cnt]))
 		goto err;
 	cnt++;
 
+	chip->bin_log_seqops.chip = chip;
+	chip->bin_log_seqops.seqops = &tpm_binary_b_measurements_seqops;
+
 	chip->bios_dir[cnt] =
 	    securityfs_create_file("binary_bios_measurements",
 				   0440, chip->bios_dir[0],
-				   (void *)&tpm_binary_b_measurements_seqops,
+				   (void *)&chip->bin_log_seqops,
 				   &tpm_bios_measurements_ops);
 	if (is_bad(chip->bios_dir[cnt]))
 		goto err;
 	cnt++;
 
+	chip->ascii_log_seqops.chip = chip;
+	chip->ascii_log_seqops.seqops = &tpm_ascii_b_measurements_seqops;
+
 	chip->bios_dir[cnt] =
 	    securityfs_create_file("ascii_bios_measurements",
 				   0440, chip->bios_dir[0],
-				   (void *)&tpm_ascii_b_measurements_seqops,
+				   (void *)&chip->ascii_log_seqops,
 				   &tpm_bios_measurements_ops);
 	if (is_bad(chip->bios_dir[cnt]))
 		goto err;
@@ -411,7 +425,19 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 void tpm_bios_log_teardown(struct tpm_chip *chip)
 {
 	int i;
-
-	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--)
+	struct inode *inode;
+
+	/* securityfs_remove currently doesn't take care of handling sync
+	 * between removal and opening of pseudo files. To handle this, a
+	 * workaround is added by making i_private = NULL here during removal
+	 * and to check it during open(), both within inode_lock()/unlock().
+	 * This design ensures that open() either safely gets kref or fails.
+	 */
+	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--) {
+		inode = d_inode(chip->bios_dir[i]);
+		inode_lock(inode);
+		inode->i_private = NULL;
+		inode_unlock(inode);
 		securityfs_remove(chip->bios_dir[i]);
+	}
 }
diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
index fd3357e..6df2f8e 100644
--- a/drivers/char/tpm/tpm_eventlog.h
+++ b/drivers/char/tpm/tpm_eventlog.h
@@ -73,7 +73,7 @@ enum tcpa_pc_event_ids {
 	HOST_TABLE_OF_DEVICES,
 };
 
-int read_log(struct tpm_bios_log *log);
+int read_log(struct tpm_chip *chip);
 
 #if defined(CONFIG_TCG_IBMVTPM) || defined(CONFIG_TCG_IBMVTPM_MODULE) || \
 	defined(CONFIG_ACPI)
diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
index 570f30c..68d891a 100644
--- a/drivers/char/tpm/tpm_of.c
+++ b/drivers/char/tpm/tpm_of.c
@@ -20,12 +20,14 @@
 #include "tpm.h"
 #include "tpm_eventlog.h"
 
-int read_log(struct tpm_bios_log *log)
+int read_log(struct tpm_chip *chip)
 {
 	struct device_node *np;
 	const u32 *sizep;
 	const u64 *basep;
+	struct tpm_bios_log *log;
 
+	log = &chip->log;
 	if (log->bios_event_log != NULL) {
 		pr_err("%s: ERROR - Eventlog already initialized\n", __func__);
 		return -EFAULT;
-- 
2.5.0

[PATCH v6 5/9] tpm: have event log use the tpm_chip

[Nayna Jain]

Move the backing memory for the event log into tpm_chip and push
the tpm_chip into read_log. This optimizes read_log processing by
only doing it once and prepares things for the next patches in the
series which require the tpm_chip to locate the event log via
ACPI and OF handles instead of searching.

This is straightfoward except for the issue of passing a kref through
i_private with securityfs. Since securityfs_remove does not have any
removal fencing like sysfs we use the inode lock to safely get a
kref on the tpm_chip.

Suggested-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 drivers/char/tpm/tpm-chip.c     |  3 +-
 drivers/char/tpm/tpm.h          | 11 ++++++
 drivers/char/tpm/tpm_acpi.c     | 15 +++++--
 drivers/char/tpm/tpm_eventlog.c | 88 ++++++++++++++++++++++++++---------------
 drivers/char/tpm/tpm_eventlog.h |  2 +-
 drivers/char/tpm/tpm_of.c       |  4 +-
 6 files changed, 85 insertions(+), 38 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 250a651..3f27753 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -127,6 +127,7 @@ static void tpm_dev_release(struct device *dev)
 	idr_remove(&dev_nums_idr, chip->dev_num);
 	mutex_unlock(&idr_lock);
 
+	kfree(chip->log.bios_event_log);
 	kfree(chip);
 }
 
@@ -345,7 +346,7 @@ int tpm_chip_register(struct tpm_chip *chip)
 	tpm_sysfs_add_device(chip);
 
 	rc = tpm_bios_log_setup(chip);
-	if (rc)
+	if (rc == -ENODEV)
 		return rc;
 
 	tpm_add_ppi(chip);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 9d69580..1ae9768 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -35,6 +35,8 @@
 #include <linux/cdev.h>
 #include <linux/highmem.h>
 
+#include "tpm_eventlog.h"
+
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */
 	TPM_BUFSIZE = 4096,
@@ -146,6 +148,11 @@ enum tpm_chip_flags {
 	TPM_CHIP_FLAG_HAVE_TIMEOUTS	= BIT(4),
 };
 
+struct tpm_chip_seqops {
+	struct tpm_chip *chip;
+	const struct seq_operations *seqops;
+};
+
 struct tpm_chip {
 	struct device dev;
 	struct cdev cdev;
@@ -157,6 +164,10 @@ struct tpm_chip {
 	struct rw_semaphore ops_sem;
 	const struct tpm_class_ops *ops;
 
+	struct tpm_bios_log log;
+	struct tpm_chip_seqops bin_log_seqops;
+	struct tpm_chip_seqops ascii_log_seqops;
+
 	unsigned int flags;
 
 	int dev_num;		/* /dev/tpm# */
diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
index 565a947..01dfb35 100644
--- a/drivers/char/tpm/tpm_acpi.c
+++ b/drivers/char/tpm/tpm_acpi.c
@@ -9,7 +9,7 @@
  *
  * Maintained by: <tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
  *
- * Access to the eventlog extended by the TCG BIOS of PC platform
+ * Access to the event log extended by the TCG BIOS of PC platform
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -45,13 +45,15 @@ struct acpi_tcpa {
 };
 
 /* read binary bios log */
-int read_log(struct tpm_bios_log *log)
+int read_log(struct tpm_chip *chip)
 {
 	struct acpi_tcpa *buff;
 	acpi_status status;
 	void __iomem *virt;
 	u64 len, start;
+	struct tpm_bios_log *log;
 
+	log = &chip->log;
 	if (log->bios_event_log != NULL) {
 		printk(KERN_ERR
 		       "%s: ERROR - Eventlog already initialized\n",
@@ -97,13 +99,18 @@ int read_log(struct tpm_bios_log *log)
 
 	virt = acpi_os_map_iomem(start, len);
 	if (!virt) {
-		kfree(log->bios_event_log);
 		printk("%s: ERROR - Unable to map memory\n", __func__);
-		return -EIO;
+		goto err;
 	}
 
 	memcpy_fromio(log->bios_event_log, virt, len);
 
 	acpi_os_unmap_iomem(virt, len);
 	return 0;
+
+err:
+	kfree(log->bios_event_log);
+	log->bios_event_log = NULL;
+	return -EIO;
+
 }
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 57ac862..f8c42fe 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -11,7 +11,7 @@
  *
  * Maintained by: <tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
  *
- * Access to the eventlog created by a system's firmware / BIOS
+ * Access to the event log created by a system's firmware / BIOS
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -73,7 +73,8 @@ static const char* tcpa_pc_event_id_strings[] = {
 static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
 {
 	loff_t i;
-	struct tpm_bios_log *log = m->private;
+	struct tpm_chip *chip = m->private;
+	struct tpm_bios_log *log = &chip->log;
 	void *addr = log->bios_event_log;
 	void *limit = log->bios_event_log_end;
 	struct tcpa_event *event;
@@ -120,7 +121,8 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,
 					loff_t *pos)
 {
 	struct tcpa_event *event = v;
-	struct tpm_bios_log *log = m->private;
+	struct tpm_chip *chip = m->private;
+	struct tpm_bios_log *log = &chip->log;
 	void *limit = log->bios_event_log_end;
 	u32 converted_event_size;
 	u32 converted_event_type;
@@ -261,13 +263,10 @@ static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v)
 static int tpm_bios_measurements_release(struct inode *inode,
 					 struct file *file)
 {
-	struct seq_file *seq = file->private_data;
-	struct tpm_bios_log *log = seq->private;
+	struct seq_file *seq = (struct seq_file *)file->private_data;
+	struct tpm_chip *chip = (struct tpm_chip *)seq->private;
 
-	if (log) {
-		kfree(log->bios_event_log);
-		kfree(log);
-	}
+	put_device(&chip->dev);
 
 	return seq_release(inode, file);
 }
@@ -323,33 +322,30 @@ static int tpm_bios_measurements_open(struct inode *inode,
 					    struct file *file)
 {
 	int err;
-	struct tpm_bios_log *log;
 	struct seq_file *seq;
-	const struct seq_operations *seqops =
-		(const struct seq_operations *)inode->i_private;
-
-	log = kzalloc(sizeof(struct tpm_bios_log), GFP_KERNEL);
-	if (!log)
-		return -ENOMEM;
-
-	if ((err = read_log(log)))
-		goto out_free;
+	struct tpm_chip_seqops *chip_seqops;
+	const struct seq_operations *seqops;
+	struct tpm_chip *chip;
+
+	inode_lock(inode);
+	if (!inode->i_private) {
+		inode_unlock(inode);
+		return -ENODEV;
+	}
+	chip_seqops = (struct tpm_chip_seqops *)inode->i_private;
+	seqops = chip_seqops->seqops;
+	chip = chip_seqops->chip;
+	get_device(&chip->dev);
+	inode_unlock(inode);
 
 	/* now register seq file */
 	err = seq_open(file, seqops);
 	if (!err) {
 		seq = file->private_data;
-		seq->private = log;
-	} else {
-		goto out_free;
+		seq->private = chip;
 	}
 
-out:
 	return err;
-out_free:
-	kfree(log->bios_event_log);
-	kfree(log);
-	goto out;
 }
 
 static const struct file_operations tpm_bios_measurements_ops = {
@@ -372,29 +368,47 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 {
 	const char *name = dev_name(&chip->dev);
 	unsigned int cnt;
+	int rc = 0;
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		return 0;
 
+	rc = read_log(chip);
+	/*
+	 * read_log failure means event log is not supported except for ENOMEM.
+	 */
+	if (rc < 0) {
+		if (rc == -ENOMEM)
+			return -ENODEV;
+		else
+			return rc;
+	}
+
 	cnt = 0;
 	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
 	if (is_bad(chip->bios_dir[cnt]))
 		goto err;
 	cnt++;
 
+	chip->bin_log_seqops.chip = chip;
+	chip->bin_log_seqops.seqops = &tpm_binary_b_measurements_seqops;
+
 	chip->bios_dir[cnt] =
 	    securityfs_create_file("binary_bios_measurements",
 				   0440, chip->bios_dir[0],
-				   (void *)&tpm_binary_b_measurements_seqops,
+				   (void *)&chip->bin_log_seqops,
 				   &tpm_bios_measurements_ops);
 	if (is_bad(chip->bios_dir[cnt]))
 		goto err;
 	cnt++;
 
+	chip->ascii_log_seqops.chip = chip;
+	chip->ascii_log_seqops.seqops = &tpm_ascii_b_measurements_seqops;
+
 	chip->bios_dir[cnt] =
 	    securityfs_create_file("ascii_bios_measurements",
 				   0440, chip->bios_dir[0],
-				   (void *)&tpm_ascii_b_measurements_seqops,
+				   (void *)&chip->ascii_log_seqops,
 				   &tpm_bios_measurements_ops);
 	if (is_bad(chip->bios_dir[cnt]))
 		goto err;
@@ -411,7 +425,19 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 void tpm_bios_log_teardown(struct tpm_chip *chip)
 {
 	int i;
-
-	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--)
+	struct inode *inode;
+
+	/* securityfs_remove currently doesn't take care of handling sync
+	 * between removal and opening of pseudo files. To handle this, a
+	 * workaround is added by making i_private = NULL here during removal
+	 * and to check it during open(), both within inode_lock()/unlock().
+	 * This design ensures that open() either safely gets kref or fails.
+	 */
+	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--) {
+		inode = d_inode(chip->bios_dir[i]);
+		inode_lock(inode);
+		inode->i_private = NULL;
+		inode_unlock(inode);
 		securityfs_remove(chip->bios_dir[i]);
+	}
 }
diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
index fd3357e..6df2f8e 100644
--- a/drivers/char/tpm/tpm_eventlog.h
+++ b/drivers/char/tpm/tpm_eventlog.h
@@ -73,7 +73,7 @@ enum tcpa_pc_event_ids {
 	HOST_TABLE_OF_DEVICES,
 };
 
-int read_log(struct tpm_bios_log *log);
+int read_log(struct tpm_chip *chip);
 
 #if defined(CONFIG_TCG_IBMVTPM) || defined(CONFIG_TCG_IBMVTPM_MODULE) || \
 	defined(CONFIG_ACPI)
diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
index 570f30c..68d891a 100644
--- a/drivers/char/tpm/tpm_of.c
+++ b/drivers/char/tpm/tpm_of.c
@@ -20,12 +20,14 @@
 #include "tpm.h"
 #include "tpm_eventlog.h"
 
-int read_log(struct tpm_bios_log *log)
+int read_log(struct tpm_chip *chip)
 {
 	struct device_node *np;
 	const u32 *sizep;
 	const u64 *basep;
+	struct tpm_bios_log *log;
 
+	log = &chip->log;
 	if (log->bios_event_log != NULL) {
 		pr_err("%s: ERROR - Eventlog already initialized\n", __func__);
 		return -EFAULT;
-- 
2.5.0


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

Re: [PATCH v6 5/9] tpm: have event log use the tpm_chip

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 05:00:52AM -0500, Nayna Jain wrote:
> Move the backing memory for the event log into tpm_chip and push
> the tpm_chip into read_log. This optimizes read_log processing by
> only doing it once and prepares things for the next patches in the
> series which require the tpm_chip to locate the event log via
> ACPI and OF handles instead of searching.
> 
> This is straightfoward except for the issue of passing a kref through
> i_private with securityfs. Since securityfs_remove does not have any
> removal fencing like sysfs we use the inode lock to safely get a
> kref on the tpm_chip.
> 
> Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

> ---
>  drivers/char/tpm/tpm-chip.c     |  3 +-
>  drivers/char/tpm/tpm.h          | 11 ++++++
>  drivers/char/tpm/tpm_acpi.c     | 15 +++++--
>  drivers/char/tpm/tpm_eventlog.c | 88 ++++++++++++++++++++++++++---------------
>  drivers/char/tpm/tpm_eventlog.h |  2 +-
>  drivers/char/tpm/tpm_of.c       |  4 +-
>  6 files changed, 85 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 250a651..3f27753 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -127,6 +127,7 @@ static void tpm_dev_release(struct device *dev)
>  	idr_remove(&dev_nums_idr, chip->dev_num);
>  	mutex_unlock(&idr_lock);
>  
> +	kfree(chip->log.bios_event_log);
>  	kfree(chip);
>  }
>  
> @@ -345,7 +346,7 @@ int tpm_chip_register(struct tpm_chip *chip)
>  	tpm_sysfs_add_device(chip);
>  
>  	rc = tpm_bios_log_setup(chip);
> -	if (rc)
> +	if (rc == -ENODEV)
>  		return rc;
>  
>  	tpm_add_ppi(chip);
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 9d69580..1ae9768 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -35,6 +35,8 @@
>  #include <linux/cdev.h>
>  #include <linux/highmem.h>
>  
> +#include "tpm_eventlog.h"
> +
>  enum tpm_const {
>  	TPM_MINOR = 224,	/* officially assigned */
>  	TPM_BUFSIZE = 4096,
> @@ -146,6 +148,11 @@ enum tpm_chip_flags {
>  	TPM_CHIP_FLAG_HAVE_TIMEOUTS	= BIT(4),
>  };
>  
> +struct tpm_chip_seqops {
> +	struct tpm_chip *chip;
> +	const struct seq_operations *seqops;
> +};
> +
>  struct tpm_chip {
>  	struct device dev;
>  	struct cdev cdev;
> @@ -157,6 +164,10 @@ struct tpm_chip {
>  	struct rw_semaphore ops_sem;
>  	const struct tpm_class_ops *ops;
>  
> +	struct tpm_bios_log log;
> +	struct tpm_chip_seqops bin_log_seqops;
> +	struct tpm_chip_seqops ascii_log_seqops;
> +
>  	unsigned int flags;
>  
>  	int dev_num;		/* /dev/tpm# */
> diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
> index 565a947..01dfb35 100644
> --- a/drivers/char/tpm/tpm_acpi.c
> +++ b/drivers/char/tpm/tpm_acpi.c
> @@ -9,7 +9,7 @@
>   *
>   * Maintained by: <tpmdd-devel@lists.sourceforge.net>
>   *
> - * Access to the eventlog extended by the TCG BIOS of PC platform
> + * Access to the event log extended by the TCG BIOS of PC platform
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License
> @@ -45,13 +45,15 @@ struct acpi_tcpa {
>  };
>  
>  /* read binary bios log */
> -int read_log(struct tpm_bios_log *log)
> +int read_log(struct tpm_chip *chip)
>  {
>  	struct acpi_tcpa *buff;
>  	acpi_status status;
>  	void __iomem *virt;
>  	u64 len, start;
> +	struct tpm_bios_log *log;
>  
> +	log = &chip->log;
>  	if (log->bios_event_log != NULL) {
>  		printk(KERN_ERR
>  		       "%s: ERROR - Eventlog already initialized\n",
> @@ -97,13 +99,18 @@ int read_log(struct tpm_bios_log *log)
>  
>  	virt = acpi_os_map_iomem(start, len);
>  	if (!virt) {
> -		kfree(log->bios_event_log);
>  		printk("%s: ERROR - Unable to map memory\n", __func__);
> -		return -EIO;
> +		goto err;
>  	}
>  
>  	memcpy_fromio(log->bios_event_log, virt, len);
>  
>  	acpi_os_unmap_iomem(virt, len);
>  	return 0;
> +
> +err:
> +	kfree(log->bios_event_log);
> +	log->bios_event_log = NULL;
> +	return -EIO;
> +
>  }
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 57ac862..f8c42fe 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -11,7 +11,7 @@
>   *
>   * Maintained by: <tpmdd-devel@lists.sourceforge.net>
>   *
> - * Access to the eventlog created by a system's firmware / BIOS
> + * Access to the event log created by a system's firmware / BIOS
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License
> @@ -73,7 +73,8 @@ static const char* tcpa_pc_event_id_strings[] = {
>  static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
>  {
>  	loff_t i;
> -	struct tpm_bios_log *log = m->private;
> +	struct tpm_chip *chip = m->private;
> +	struct tpm_bios_log *log = &chip->log;
>  	void *addr = log->bios_event_log;
>  	void *limit = log->bios_event_log_end;
>  	struct tcpa_event *event;
> @@ -120,7 +121,8 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,
>  					loff_t *pos)
>  {
>  	struct tcpa_event *event = v;
> -	struct tpm_bios_log *log = m->private;
> +	struct tpm_chip *chip = m->private;
> +	struct tpm_bios_log *log = &chip->log;
>  	void *limit = log->bios_event_log_end;
>  	u32 converted_event_size;
>  	u32 converted_event_type;
> @@ -261,13 +263,10 @@ static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v)
>  static int tpm_bios_measurements_release(struct inode *inode,
>  					 struct file *file)
>  {
> -	struct seq_file *seq = file->private_data;
> -	struct tpm_bios_log *log = seq->private;
> +	struct seq_file *seq = (struct seq_file *)file->private_data;
> +	struct tpm_chip *chip = (struct tpm_chip *)seq->private;
>  
> -	if (log) {
> -		kfree(log->bios_event_log);
> -		kfree(log);
> -	}
> +	put_device(&chip->dev);
>  
>  	return seq_release(inode, file);
>  }
> @@ -323,33 +322,30 @@ static int tpm_bios_measurements_open(struct inode *inode,
>  					    struct file *file)
>  {
>  	int err;
> -	struct tpm_bios_log *log;
>  	struct seq_file *seq;
> -	const struct seq_operations *seqops =
> -		(const struct seq_operations *)inode->i_private;
> -
> -	log = kzalloc(sizeof(struct tpm_bios_log), GFP_KERNEL);
> -	if (!log)
> -		return -ENOMEM;
> -
> -	if ((err = read_log(log)))
> -		goto out_free;
> +	struct tpm_chip_seqops *chip_seqops;
> +	const struct seq_operations *seqops;
> +	struct tpm_chip *chip;
> +
> +	inode_lock(inode);
> +	if (!inode->i_private) {
> +		inode_unlock(inode);
> +		return -ENODEV;
> +	}
> +	chip_seqops = (struct tpm_chip_seqops *)inode->i_private;
> +	seqops = chip_seqops->seqops;
> +	chip = chip_seqops->chip;
> +	get_device(&chip->dev);
> +	inode_unlock(inode);
>  
>  	/* now register seq file */
>  	err = seq_open(file, seqops);
>  	if (!err) {
>  		seq = file->private_data;
> -		seq->private = log;
> -	} else {
> -		goto out_free;
> +		seq->private = chip;
>  	}
>  
> -out:
>  	return err;
> -out_free:
> -	kfree(log->bios_event_log);
> -	kfree(log);
> -	goto out;
>  }
>  
>  static const struct file_operations tpm_bios_measurements_ops = {
> @@ -372,29 +368,47 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
>  {
>  	const char *name = dev_name(&chip->dev);
>  	unsigned int cnt;
> +	int rc = 0;
>  
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>  		return 0;
>  
> +	rc = read_log(chip);
> +	/*
> +	 * read_log failure means event log is not supported except for ENOMEM.
> +	 */
> +	if (rc < 0) {
> +		if (rc == -ENOMEM)
> +			return -ENODEV;
> +		else
> +			return rc;
> +	}
> +
>  	cnt = 0;
>  	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
>  	if (is_bad(chip->bios_dir[cnt]))
>  		goto err;
>  	cnt++;
>  
> +	chip->bin_log_seqops.chip = chip;
> +	chip->bin_log_seqops.seqops = &tpm_binary_b_measurements_seqops;
> +
>  	chip->bios_dir[cnt] =
>  	    securityfs_create_file("binary_bios_measurements",
>  				   0440, chip->bios_dir[0],
> -				   (void *)&tpm_binary_b_measurements_seqops,
> +				   (void *)&chip->bin_log_seqops,
>  				   &tpm_bios_measurements_ops);
>  	if (is_bad(chip->bios_dir[cnt]))
>  		goto err;
>  	cnt++;
>  
> +	chip->ascii_log_seqops.chip = chip;
> +	chip->ascii_log_seqops.seqops = &tpm_ascii_b_measurements_seqops;
> +
>  	chip->bios_dir[cnt] =
>  	    securityfs_create_file("ascii_bios_measurements",
>  				   0440, chip->bios_dir[0],
> -				   (void *)&tpm_ascii_b_measurements_seqops,
> +				   (void *)&chip->ascii_log_seqops,
>  				   &tpm_bios_measurements_ops);
>  	if (is_bad(chip->bios_dir[cnt]))
>  		goto err;
> @@ -411,7 +425,19 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
>  void tpm_bios_log_teardown(struct tpm_chip *chip)
>  {
>  	int i;
> -
> -	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--)
> +	struct inode *inode;
> +
> +	/* securityfs_remove currently doesn't take care of handling sync
> +	 * between removal and opening of pseudo files. To handle this, a
> +	 * workaround is added by making i_private = NULL here during removal
> +	 * and to check it during open(), both within inode_lock()/unlock().
> +	 * This design ensures that open() either safely gets kref or fails.
> +	 */
> +	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--) {
> +		inode = d_inode(chip->bios_dir[i]);
> +		inode_lock(inode);
> +		inode->i_private = NULL;
> +		inode_unlock(inode);
>  		securityfs_remove(chip->bios_dir[i]);
> +	}
>  }
> diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
> index fd3357e..6df2f8e 100644
> --- a/drivers/char/tpm/tpm_eventlog.h
> +++ b/drivers/char/tpm/tpm_eventlog.h
> @@ -73,7 +73,7 @@ enum tcpa_pc_event_ids {
>  	HOST_TABLE_OF_DEVICES,
>  };
>  
> -int read_log(struct tpm_bios_log *log);
> +int read_log(struct tpm_chip *chip);
>  
>  #if defined(CONFIG_TCG_IBMVTPM) || defined(CONFIG_TCG_IBMVTPM_MODULE) || \
>  	defined(CONFIG_ACPI)
> diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
> index 570f30c..68d891a 100644
> --- a/drivers/char/tpm/tpm_of.c
> +++ b/drivers/char/tpm/tpm_of.c
> @@ -20,12 +20,14 @@
>  #include "tpm.h"
>  #include "tpm_eventlog.h"
>  
> -int read_log(struct tpm_bios_log *log)
> +int read_log(struct tpm_chip *chip)
>  {
>  	struct device_node *np;
>  	const u32 *sizep;
>  	const u64 *basep;
> +	struct tpm_bios_log *log;
>  
> +	log = &chip->log;
>  	if (log->bios_event_log != NULL) {
>  		pr_err("%s: ERROR - Eventlog already initialized\n", __func__);
>  		return -EFAULT;
> -- 
> 2.5.0
> 

Re: [PATCH v6 5/9] tpm: have event log use the tpm_chip

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 03:44:01PM -0800, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 05:00:52AM -0500, Nayna Jain wrote:
> > Move the backing memory for the event log into tpm_chip and push
> > the tpm_chip into read_log. This optimizes read_log processing by
> > only doing it once and prepares things for the next patches in the
> > series which require the tpm_chip to locate the event log via
> > ACPI and OF handles instead of searching.
> > 
> > This is straightfoward except for the issue of passing a kref through
> > i_private with securityfs. Since securityfs_remove does not have any
> > removal fencing like sysfs we use the inode lock to safely get a
> > kref on the tpm_chip.
> > 
> > Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

Re: [PATCH v6 5/9] tpm: have event log use the tpm_chip

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 03:44:01PM -0800, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 05:00:52AM -0500, Nayna Jain wrote:
> > Move the backing memory for the event log into tpm_chip and push
> > the tpm_chip into read_log. This optimizes read_log processing by
> > only doing it once and prepares things for the next patches in the
> > series which require the tpm_chip to locate the event log via
> > ACPI and OF handles instead of searching.
> > 
> > This is straightfoward except for the issue of passing a kref through
> > i_private with securityfs. Since securityfs_remove does not have any
> > removal fencing like sysfs we use the inode lock to safely get a
> > kref on the tpm_chip.
> > 
> > Suggested-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> > Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

/Jarkko

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

[PATCH v6 6/9] tpm: fix the missing .owner in tpm_bios_measurements_ops

[Nayna Jain]

This patch fixes the missing .owner field in
tpm_bios_measurements_ops definition.

Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm_eventlog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index f8c42fe..5575ffc 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -349,6 +349,7 @@ static int tpm_bios_measurements_open(struct inode *inode,
 }
 
 static const struct file_operations tpm_bios_measurements_ops = {
+	.owner = THIS_MODULE,
 	.open = tpm_bios_measurements_open,
 	.read = seq_read,
 	.llseek = seq_lseek,
-- 
2.5.0

[PATCH v6 6/9] tpm: fix the missing .owner in tpm_bios_measurements_ops

[Nayna Jain]

This patch fixes the missing .owner field in
tpm_bios_measurements_ops definition.

Suggested-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 drivers/char/tpm/tpm_eventlog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index f8c42fe..5575ffc 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -349,6 +349,7 @@ static int tpm_bios_measurements_open(struct inode *inode,
 }
 
 static const struct file_operations tpm_bios_measurements_ops = {
+	.owner = THIS_MODULE,
 	.open = tpm_bios_measurements_open,
 	.read = seq_read,
 	.llseek = seq_lseek,
-- 
2.5.0


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

Re: [PATCH v6 6/9] tpm: fix the missing .owner in tpm_bios_measurements_ops

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 05:00:53AM -0500, Nayna Jain wrote:
> This patch fixes the missing .owner field in
> tpm_bios_measurements_ops definition.
> 
> Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

> ---
>  drivers/char/tpm/tpm_eventlog.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index f8c42fe..5575ffc 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -349,6 +349,7 @@ static int tpm_bios_measurements_open(struct inode *inode,
>  }
>  
>  static const struct file_operations tpm_bios_measurements_ops = {
> +	.owner = THIS_MODULE,
>  	.open = tpm_bios_measurements_open,
>  	.read = seq_read,
>  	.llseek = seq_lseek,
> -- 
> 2.5.0
> 

Re: [PATCH v6 6/9] tpm: fix the missing .owner in tpm_bios_measurements_ops

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 02:22:09PM -0800, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 05:00:53AM -0500, Nayna Jain wrote:
> > This patch fixes the missing .owner field in
> > tpm_bios_measurements_ops definition.
> > 
> > Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

[PATCH v6 7/9] tpm: redefine read_log() to handle ACPI/OF at runtime

[Nayna Jain]

Currently, read_log() has two implementations: one for ACPI platforms
and the other for device tree(OF) based platforms. The proper one is
selected at compile time using Kconfig and #ifdef in the Makefile,
which is not the recommended approach.

This patch removes the #ifdef in the Makefile by defining a single
read_log() method, which checks for ACPI/OF event log properties at
runtime.

Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/Makefile       | 14 ++++----------
 drivers/char/tpm/tpm_acpi.c     |  9 ++-------
 drivers/char/tpm/tpm_eventlog.c | 20 ++++++++++++++++++++
 drivers/char/tpm/tpm_eventlog.h | 22 +++++++++++++---------
 drivers/char/tpm/tpm_of.c       |  8 ++------
 5 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index a385fb8..a05b1eb 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -2,16 +2,10 @@
 # Makefile for the kernel tpm device drivers.
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
-tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o
-tpm-$(CONFIG_ACPI) += tpm_ppi.o
-
-ifdef CONFIG_ACPI
-	tpm-y += tpm_eventlog.o tpm_acpi.o
-else
-ifdef CONFIG_TCG_IBMVTPM
-	tpm-y += tpm_eventlog.o tpm_of.o
-endif
-endif
+tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
+		tpm_eventlog.o
+tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_acpi.o
+tpm-$(CONFIG_OF) += tpm_of.o
 obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
 obj-$(CONFIG_TCG_TIS) += tpm_tis.o
 obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi.o
diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
index 01dfb35..fa30c969 100644
--- a/drivers/char/tpm/tpm_acpi.c
+++ b/drivers/char/tpm/tpm_acpi.c
@@ -6,6 +6,7 @@
  *	Stefan Berger <stefanb@us.ibm.com>
  *	Reiner Sailer <sailer@watson.ibm.com>
  *	Kylene Hall <kjhall@us.ibm.com>
+ *	Nayna Jain <nayna@linux.vnet.ibm.com>
  *
  * Maintained by: <tpmdd-devel@lists.sourceforge.net>
  *
@@ -45,7 +46,7 @@ struct acpi_tcpa {
 };
 
 /* read binary bios log */
-int read_log(struct tpm_chip *chip)
+int read_log_acpi(struct tpm_chip *chip)
 {
 	struct acpi_tcpa *buff;
 	acpi_status status;
@@ -54,12 +55,6 @@ int read_log(struct tpm_chip *chip)
 	struct tpm_bios_log *log;
 
 	log = &chip->log;
-	if (log->bios_event_log != NULL) {
-		printk(KERN_ERR
-		       "%s: ERROR - Eventlog already initialized\n",
-		       __func__);
-		return -EFAULT;
-	}
 
 	/* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
 	status = acpi_get_table(ACPI_SIG_TCPA, 1,
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 5575ffc..cce679b 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -365,6 +365,26 @@ static int is_bad(void *p)
 	return 0;
 }
 
+int read_log(struct tpm_chip *chip)
+{
+	int rc;
+
+	if (chip->log.bios_event_log != NULL) {
+		dev_dbg(&chip->dev,
+			"%s: ERROR - event log already initialized\n",
+			__func__);
+		return -EFAULT;
+	}
+
+	rc = read_log_acpi(chip);
+	if ((rc == 0) || (rc == -ENOMEM))
+		return rc;
+
+	rc = read_log_of(chip);
+
+	return rc;
+}
+
 int tpm_bios_log_setup(struct tpm_chip *chip)
 {
 	const char *name = dev_name(&chip->dev);
diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
index 6df2f8e..be529ad 100644
--- a/drivers/char/tpm/tpm_eventlog.h
+++ b/drivers/char/tpm/tpm_eventlog.h
@@ -73,20 +73,24 @@ enum tcpa_pc_event_ids {
 	HOST_TABLE_OF_DEVICES,
 };
 
-int read_log(struct tpm_chip *chip);
-
-#if defined(CONFIG_TCG_IBMVTPM) || defined(CONFIG_TCG_IBMVTPM_MODULE) || \
-	defined(CONFIG_ACPI)
-extern int tpm_bios_log_setup(struct tpm_chip *chip);
-extern void tpm_bios_log_teardown(struct tpm_chip *chip);
+#if defined(CONFIG_ACPI)
+int read_log_acpi(struct tpm_chip *chip);
 #else
-static inline int tpm_bios_log_setup(struct tpm_chip *chip)
+static inline int read_log_acpi(struct tpm_chip *chip)
 {
-	return 0;
+	return -ENODEV;
 }
-static inline void tpm_bios_log_teardown(struct tpm_chip *chip)
+#endif
+#if defined(CONFIG_OF)
+int read_log_of(struct tpm_chip *chip);
+#else
+static inline int read_log_of(struct tpm_chip *chip)
 {
+	return -ENODEV;
 }
 #endif
 
+int tpm_bios_log_setup(struct tpm_chip *chip);
+void tpm_bios_log_teardown(struct tpm_chip *chip);
+
 #endif
diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
index 68d891a..7c30752 100644
--- a/drivers/char/tpm/tpm_of.c
+++ b/drivers/char/tpm/tpm_of.c
@@ -2,6 +2,7 @@
  * Copyright 2012 IBM Corporation
  *
  * Author: Ashley Lai <ashleydlai@gmail.com>
+ *         Nayna Jain <nayna@linux.vnet.ibm.com>
  *
  * Maintained by: <tpmdd-devel@lists.sourceforge.net>
  *
@@ -20,7 +21,7 @@
 #include "tpm.h"
 #include "tpm_eventlog.h"
 
-int read_log(struct tpm_chip *chip)
+int read_log_of(struct tpm_chip *chip)
 {
 	struct device_node *np;
 	const u32 *sizep;
@@ -28,11 +29,6 @@ int read_log(struct tpm_chip *chip)
 	struct tpm_bios_log *log;
 
 	log = &chip->log;
-	if (log->bios_event_log != NULL) {
-		pr_err("%s: ERROR - Eventlog already initialized\n", __func__);
-		return -EFAULT;
-	}
-
 	np = of_find_node_by_name(NULL, "vtpm");
 	if (!np) {
 		pr_err("%s: ERROR - IBMVTPM not supported\n", __func__);
-- 
2.5.0

[PATCH v6 7/9] tpm: redefine read_log() to handle ACPI/OF at runtime

[Nayna Jain]

Currently, read_log() has two implementations: one for ACPI platforms
and the other for device tree(OF) based platforms. The proper one is
selected at compile time using Kconfig and #ifdef in the Makefile,
which is not the recommended approach.

This patch removes the #ifdef in the Makefile by defining a single
read_log() method, which checks for ACPI/OF event log properties at
runtime.

Suggested-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Reviewed-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/char/tpm/Makefile       | 14 ++++----------
 drivers/char/tpm/tpm_acpi.c     |  9 ++-------
 drivers/char/tpm/tpm_eventlog.c | 20 ++++++++++++++++++++
 drivers/char/tpm/tpm_eventlog.h | 22 +++++++++++++---------
 drivers/char/tpm/tpm_of.c       |  8 ++------
 5 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index a385fb8..a05b1eb 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -2,16 +2,10 @@
 # Makefile for the kernel tpm device drivers.
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
-tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o
-tpm-$(CONFIG_ACPI) += tpm_ppi.o
-
-ifdef CONFIG_ACPI
-	tpm-y += tpm_eventlog.o tpm_acpi.o
-else
-ifdef CONFIG_TCG_IBMVTPM
-	tpm-y += tpm_eventlog.o tpm_of.o
-endif
-endif
+tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
+		tpm_eventlog.o
+tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_acpi.o
+tpm-$(CONFIG_OF) += tpm_of.o
 obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
 obj-$(CONFIG_TCG_TIS) += tpm_tis.o
 obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi.o
diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
index 01dfb35..fa30c969 100644
--- a/drivers/char/tpm/tpm_acpi.c
+++ b/drivers/char/tpm/tpm_acpi.c
@@ -6,6 +6,7 @@
  *	Stefan Berger <stefanb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
  *	Reiner Sailer <sailer-aZOuKsOsJu3MbYB6QlFGEg@public.gmane.org>
  *	Kylene Hall <kjhall-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
+ *	Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
  *
  * Maintained by: <tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
  *
@@ -45,7 +46,7 @@ struct acpi_tcpa {
 };
 
 /* read binary bios log */
-int read_log(struct tpm_chip *chip)
+int read_log_acpi(struct tpm_chip *chip)
 {
 	struct acpi_tcpa *buff;
 	acpi_status status;
@@ -54,12 +55,6 @@ int read_log(struct tpm_chip *chip)
 	struct tpm_bios_log *log;
 
 	log = &chip->log;
-	if (log->bios_event_log != NULL) {
-		printk(KERN_ERR
-		       "%s: ERROR - Eventlog already initialized\n",
-		       __func__);
-		return -EFAULT;
-	}
 
 	/* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
 	status = acpi_get_table(ACPI_SIG_TCPA, 1,
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 5575ffc..cce679b 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -365,6 +365,26 @@ static int is_bad(void *p)
 	return 0;
 }
 
+int read_log(struct tpm_chip *chip)
+{
+	int rc;
+
+	if (chip->log.bios_event_log != NULL) {
+		dev_dbg(&chip->dev,
+			"%s: ERROR - event log already initialized\n",
+			__func__);
+		return -EFAULT;
+	}
+
+	rc = read_log_acpi(chip);
+	if ((rc == 0) || (rc == -ENOMEM))
+		return rc;
+
+	rc = read_log_of(chip);
+
+	return rc;
+}
+
 int tpm_bios_log_setup(struct tpm_chip *chip)
 {
 	const char *name = dev_name(&chip->dev);
diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
index 6df2f8e..be529ad 100644
--- a/drivers/char/tpm/tpm_eventlog.h
+++ b/drivers/char/tpm/tpm_eventlog.h
@@ -73,20 +73,24 @@ enum tcpa_pc_event_ids {
 	HOST_TABLE_OF_DEVICES,
 };
 
-int read_log(struct tpm_chip *chip);
-
-#if defined(CONFIG_TCG_IBMVTPM) || defined(CONFIG_TCG_IBMVTPM_MODULE) || \
-	defined(CONFIG_ACPI)
-extern int tpm_bios_log_setup(struct tpm_chip *chip);
-extern void tpm_bios_log_teardown(struct tpm_chip *chip);
+#if defined(CONFIG_ACPI)
+int read_log_acpi(struct tpm_chip *chip);
 #else
-static inline int tpm_bios_log_setup(struct tpm_chip *chip)
+static inline int read_log_acpi(struct tpm_chip *chip)
 {
-	return 0;
+	return -ENODEV;
 }
-static inline void tpm_bios_log_teardown(struct tpm_chip *chip)
+#endif
+#if defined(CONFIG_OF)
+int read_log_of(struct tpm_chip *chip);
+#else
+static inline int read_log_of(struct tpm_chip *chip)
 {
+	return -ENODEV;
 }
 #endif
 
+int tpm_bios_log_setup(struct tpm_chip *chip);
+void tpm_bios_log_teardown(struct tpm_chip *chip);
+
 #endif
diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
index 68d891a..7c30752 100644
--- a/drivers/char/tpm/tpm_of.c
+++ b/drivers/char/tpm/tpm_of.c
@@ -2,6 +2,7 @@
  * Copyright 2012 IBM Corporation
  *
  * Author: Ashley Lai <ashleydlai-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ *         Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
  *
  * Maintained by: <tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
  *
@@ -20,7 +21,7 @@
 #include "tpm.h"
 #include "tpm_eventlog.h"
 
-int read_log(struct tpm_chip *chip)
+int read_log_of(struct tpm_chip *chip)
 {
 	struct device_node *np;
 	const u32 *sizep;
@@ -28,11 +29,6 @@ int read_log(struct tpm_chip *chip)
 	struct tpm_bios_log *log;
 
 	log = &chip->log;
-	if (log->bios_event_log != NULL) {
-		pr_err("%s: ERROR - Eventlog already initialized\n", __func__);
-		return -EFAULT;
-	}
-
 	np = of_find_node_by_name(NULL, "vtpm");
 	if (!np) {
 		pr_err("%s: ERROR - IBMVTPM not supported\n", __func__);
-- 
2.5.0


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

Re: [PATCH v6 7/9] tpm: redefine read_log() to handle ACPI/OF at runtime

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 05:00:54AM -0500, Nayna Jain wrote:
> Currently, read_log() has two implementations: one for ACPI platforms
> and the other for device tree(OF) based platforms. The proper one is
> selected at compile time using Kconfig and #ifdef in the Makefile,
> which is not the recommended approach.
> 
> This patch removes the #ifdef in the Makefile by defining a single
> read_log() method, which checks for ACPI/OF event log properties at
> runtime.
> 
> Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

[PATCH v6 8/9] tpm: replace of_find_node_by_name() with dev of_node property

[Nayna Jain]

Using the device of_node property is a better way to refer to the
device tree node rather than of_find_node_by_name().

Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm_of.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
index 7c30752..22b8f81 100644
--- a/drivers/char/tpm/tpm_of.c
+++ b/drivers/char/tpm/tpm_of.c
@@ -29,7 +29,8 @@ int read_log_of(struct tpm_chip *chip)
 	struct tpm_bios_log *log;
 
 	log = &chip->log;
-	np = of_find_node_by_name(NULL, "vtpm");
+	if (chip->dev.parent->of_node)
+		np = chip->dev.parent->of_node;
 	if (!np) {
 		pr_err("%s: ERROR - IBMVTPM not supported\n", __func__);
 		return -ENODEV;
@@ -55,18 +56,15 @@ int read_log_of(struct tpm_chip *chip)
 	if (!log->bios_event_log) {
 		pr_err("%s: ERROR - Not enough memory for BIOS measurements\n",
 		       __func__);
-		of_node_put(np);
 		return -ENOMEM;
 	}
 
 	log->bios_event_log_end = log->bios_event_log + *sizep;
 
 	memcpy(log->bios_event_log, __va(*basep), *sizep);
-	of_node_put(np);
 
 	return 0;
 
 cleanup_eio:
-	of_node_put(np);
 	return -EIO;
 }
-- 
2.5.0

[PATCH v6 8/9] tpm: replace of_find_node_by_name() with dev of_node property

[Nayna Jain]

Using the device of_node property is a better way to refer to the
device tree node rather than of_find_node_by_name().

Suggested-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/char/tpm/tpm_of.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
index 7c30752..22b8f81 100644
--- a/drivers/char/tpm/tpm_of.c
+++ b/drivers/char/tpm/tpm_of.c
@@ -29,7 +29,8 @@ int read_log_of(struct tpm_chip *chip)
 	struct tpm_bios_log *log;
 
 	log = &chip->log;
-	np = of_find_node_by_name(NULL, "vtpm");
+	if (chip->dev.parent->of_node)
+		np = chip->dev.parent->of_node;
 	if (!np) {
 		pr_err("%s: ERROR - IBMVTPM not supported\n", __func__);
 		return -ENODEV;
@@ -55,18 +56,15 @@ int read_log_of(struct tpm_chip *chip)
 	if (!log->bios_event_log) {
 		pr_err("%s: ERROR - Not enough memory for BIOS measurements\n",
 		       __func__);
-		of_node_put(np);
 		return -ENOMEM;
 	}
 
 	log->bios_event_log_end = log->bios_event_log + *sizep;
 
 	memcpy(log->bios_event_log, __va(*basep), *sizep);
-	of_node_put(np);
 
 	return 0;
 
 cleanup_eio:
-	of_node_put(np);
 	return -EIO;
 }
-- 
2.5.0


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

[PATCH v6 9/9] tpm: cleanup of printk error messages

[Nayna Jain]

This patch removes the unnecessary error messages on failing to
allocate memory and replaces pr_err/printk with dev_dbg/dev_info
as applicable.

Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm_acpi.c | 16 ++++------------
 drivers/char/tpm/tpm_of.c   | 29 +++++++++--------------------
 2 files changed, 13 insertions(+), 32 deletions(-)

diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
index fa30c969..ddbaef2 100644
--- a/drivers/char/tpm/tpm_acpi.c
+++ b/drivers/char/tpm/tpm_acpi.c
@@ -60,11 +60,8 @@ int read_log_acpi(struct tpm_chip *chip)
 	status = acpi_get_table(ACPI_SIG_TCPA, 1,
 				(struct acpi_table_header **)&buff);
 
-	if (ACPI_FAILURE(status)) {
-		printk(KERN_ERR "%s: ERROR - Could not get TCPA table\n",
-		       __func__);
+	if (ACPI_FAILURE(status))
 		return -EIO;
-	}
 
 	switch(buff->platform_class) {
 	case BIOS_SERVER:
@@ -78,25 +75,20 @@ int read_log_acpi(struct tpm_chip *chip)
 		break;
 	}
 	if (!len) {
-		printk(KERN_ERR "%s: ERROR - TCPA log area empty\n", __func__);
+		dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
 		return -EIO;
 	}
 
 	/* malloc EventLog space */
 	log->bios_event_log = kmalloc(len, GFP_KERNEL);
-	if (!log->bios_event_log) {
-		printk("%s: ERROR - Not enough  Memory for BIOS measurements\n",
-			__func__);
+	if (!log->bios_event_log)
 		return -ENOMEM;
-	}
 
 	log->bios_event_log_end = log->bios_event_log + len;
 
 	virt = acpi_os_map_iomem(start, len);
-	if (!virt) {
-		printk("%s: ERROR - Unable to map memory\n", __func__);
+	if (!virt)
 		goto err;
-	}
 
 	memcpy_fromio(log->bios_event_log, virt, len);
 
diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
index 22b8f81..3af829f 100644
--- a/drivers/char/tpm/tpm_of.c
+++ b/drivers/char/tpm/tpm_of.c
@@ -31,40 +31,29 @@ int read_log_of(struct tpm_chip *chip)
 	log = &chip->log;
 	if (chip->dev.parent->of_node)
 		np = chip->dev.parent->of_node;
-	if (!np) {
-		pr_err("%s: ERROR - IBMVTPM not supported\n", __func__);
+	if (!np)
 		return -ENODEV;
-	}
 
 	sizep = of_get_property(np, "linux,sml-size", NULL);
-	if (sizep == NULL) {
-		pr_err("%s: ERROR - SML size not found\n", __func__);
-		goto cleanup_eio;
-	}
+	if (sizep == NULL)
+		return -EIO;
+
 	if (*sizep == 0) {
-		pr_err("%s: ERROR - event log area empty\n", __func__);
-		goto cleanup_eio;
+		dev_warn(&chip->dev, "%s: Event log area empty\n", __func__);
+		return -EIO;
 	}
 
 	basep = of_get_property(np, "linux,sml-base", NULL);
-	if (basep == NULL) {
-		pr_err("%s: ERROR - SML not found\n", __func__);
-		goto cleanup_eio;
-	}
+	if (basep == NULL)
+		return -EIO;
 
 	log->bios_event_log = kmalloc(*sizep, GFP_KERNEL);
-	if (!log->bios_event_log) {
-		pr_err("%s: ERROR - Not enough memory for BIOS measurements\n",
-		       __func__);
+	if (!log->bios_event_log)
 		return -ENOMEM;
-	}
 
 	log->bios_event_log_end = log->bios_event_log + *sizep;
 
 	memcpy(log->bios_event_log, __va(*basep), *sizep);
 
 	return 0;
-
-cleanup_eio:
-	return -EIO;
 }
-- 
2.5.0

[PATCH v6 9/9] tpm: cleanup of printk error messages

[Nayna Jain]

This patch removes the unnecessary error messages on failing to
allocate memory and replaces pr_err/printk with dev_dbg/dev_info
as applicable.

Suggested-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 drivers/char/tpm/tpm_acpi.c | 16 ++++------------
 drivers/char/tpm/tpm_of.c   | 29 +++++++++--------------------
 2 files changed, 13 insertions(+), 32 deletions(-)

diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
index fa30c969..ddbaef2 100644
--- a/drivers/char/tpm/tpm_acpi.c
+++ b/drivers/char/tpm/tpm_acpi.c
@@ -60,11 +60,8 @@ int read_log_acpi(struct tpm_chip *chip)
 	status = acpi_get_table(ACPI_SIG_TCPA, 1,
 				(struct acpi_table_header **)&buff);
 
-	if (ACPI_FAILURE(status)) {
-		printk(KERN_ERR "%s: ERROR - Could not get TCPA table\n",
-		       __func__);
+	if (ACPI_FAILURE(status))
 		return -EIO;
-	}
 
 	switch(buff->platform_class) {
 	case BIOS_SERVER:
@@ -78,25 +75,20 @@ int read_log_acpi(struct tpm_chip *chip)
 		break;
 	}
 	if (!len) {
-		printk(KERN_ERR "%s: ERROR - TCPA log area empty\n", __func__);
+		dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
 		return -EIO;
 	}
 
 	/* malloc EventLog space */
 	log->bios_event_log = kmalloc(len, GFP_KERNEL);
-	if (!log->bios_event_log) {
-		printk("%s: ERROR - Not enough  Memory for BIOS measurements\n",
-			__func__);
+	if (!log->bios_event_log)
 		return -ENOMEM;
-	}
 
 	log->bios_event_log_end = log->bios_event_log + len;
 
 	virt = acpi_os_map_iomem(start, len);
-	if (!virt) {
-		printk("%s: ERROR - Unable to map memory\n", __func__);
+	if (!virt)
 		goto err;
-	}
 
 	memcpy_fromio(log->bios_event_log, virt, len);
 
diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
index 22b8f81..3af829f 100644
--- a/drivers/char/tpm/tpm_of.c
+++ b/drivers/char/tpm/tpm_of.c
@@ -31,40 +31,29 @@ int read_log_of(struct tpm_chip *chip)
 	log = &chip->log;
 	if (chip->dev.parent->of_node)
 		np = chip->dev.parent->of_node;
-	if (!np) {
-		pr_err("%s: ERROR - IBMVTPM not supported\n", __func__);
+	if (!np)
 		return -ENODEV;
-	}
 
 	sizep = of_get_property(np, "linux,sml-size", NULL);
-	if (sizep == NULL) {
-		pr_err("%s: ERROR - SML size not found\n", __func__);
-		goto cleanup_eio;
-	}
+	if (sizep == NULL)
+		return -EIO;
+
 	if (*sizep == 0) {
-		pr_err("%s: ERROR - event log area empty\n", __func__);
-		goto cleanup_eio;
+		dev_warn(&chip->dev, "%s: Event log area empty\n", __func__);
+		return -EIO;
 	}
 
 	basep = of_get_property(np, "linux,sml-base", NULL);
-	if (basep == NULL) {
-		pr_err("%s: ERROR - SML not found\n", __func__);
-		goto cleanup_eio;
-	}
+	if (basep == NULL)
+		return -EIO;
 
 	log->bios_event_log = kmalloc(*sizep, GFP_KERNEL);
-	if (!log->bios_event_log) {
-		pr_err("%s: ERROR - Not enough memory for BIOS measurements\n",
-		       __func__);
+	if (!log->bios_event_log)
 		return -ENOMEM;
-	}
 
 	log->bios_event_log_end = log->bios_event_log + *sizep;
 
 	memcpy(log->bios_event_log, __va(*basep), *sizep);
 
 	return 0;
-
-cleanup_eio:
-	return -EIO;
 }
-- 
2.5.0


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

Re: [PATCH v6 9/9] tpm: cleanup of printk error messages

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 05:00:56AM -0500, Nayna Jain wrote:
> This patch removes the unnecessary error messages on failing to
> allocate memory and replaces pr_err/printk with dev_dbg/dev_info
> as applicable.
> 
> Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

> ---
>  drivers/char/tpm/tpm_acpi.c | 16 ++++------------
>  drivers/char/tpm/tpm_of.c   | 29 +++++++++--------------------
>  2 files changed, 13 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
> index fa30c969..ddbaef2 100644
> --- a/drivers/char/tpm/tpm_acpi.c
> +++ b/drivers/char/tpm/tpm_acpi.c
> @@ -60,11 +60,8 @@ int read_log_acpi(struct tpm_chip *chip)
>  	status = acpi_get_table(ACPI_SIG_TCPA, 1,
>  				(struct acpi_table_header **)&buff);
>  
> -	if (ACPI_FAILURE(status)) {
> -		printk(KERN_ERR "%s: ERROR - Could not get TCPA table\n",
> -		       __func__);
> +	if (ACPI_FAILURE(status))
>  		return -EIO;
> -	}
>  
>  	switch(buff->platform_class) {
>  	case BIOS_SERVER:
> @@ -78,25 +75,20 @@ int read_log_acpi(struct tpm_chip *chip)
>  		break;
>  	}
>  	if (!len) {
> -		printk(KERN_ERR "%s: ERROR - TCPA log area empty\n", __func__);
> +		dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
>  		return -EIO;
>  	}
>  
>  	/* malloc EventLog space */
>  	log->bios_event_log = kmalloc(len, GFP_KERNEL);
> -	if (!log->bios_event_log) {
> -		printk("%s: ERROR - Not enough  Memory for BIOS measurements\n",
> -			__func__);
> +	if (!log->bios_event_log)
>  		return -ENOMEM;
> -	}
>  
>  	log->bios_event_log_end = log->bios_event_log + len;
>  
>  	virt = acpi_os_map_iomem(start, len);
> -	if (!virt) {
> -		printk("%s: ERROR - Unable to map memory\n", __func__);
> +	if (!virt)
>  		goto err;
> -	}
>  
>  	memcpy_fromio(log->bios_event_log, virt, len);
>  
> diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
> index 22b8f81..3af829f 100644
> --- a/drivers/char/tpm/tpm_of.c
> +++ b/drivers/char/tpm/tpm_of.c
> @@ -31,40 +31,29 @@ int read_log_of(struct tpm_chip *chip)
>  	log = &chip->log;
>  	if (chip->dev.parent->of_node)
>  		np = chip->dev.parent->of_node;
> -	if (!np) {
> -		pr_err("%s: ERROR - IBMVTPM not supported\n", __func__);
> +	if (!np)
>  		return -ENODEV;
> -	}
>  
>  	sizep = of_get_property(np, "linux,sml-size", NULL);
> -	if (sizep == NULL) {
> -		pr_err("%s: ERROR - SML size not found\n", __func__);
> -		goto cleanup_eio;
> -	}
> +	if (sizep == NULL)
> +		return -EIO;
> +
>  	if (*sizep == 0) {
> -		pr_err("%s: ERROR - event log area empty\n", __func__);
> -		goto cleanup_eio;
> +		dev_warn(&chip->dev, "%s: Event log area empty\n", __func__);
> +		return -EIO;
>  	}
>  
>  	basep = of_get_property(np, "linux,sml-base", NULL);
> -	if (basep == NULL) {
> -		pr_err("%s: ERROR - SML not found\n", __func__);
> -		goto cleanup_eio;
> -	}
> +	if (basep == NULL)
> +		return -EIO;
>  
>  	log->bios_event_log = kmalloc(*sizep, GFP_KERNEL);
> -	if (!log->bios_event_log) {
> -		pr_err("%s: ERROR - Not enough memory for BIOS measurements\n",
> -		       __func__);
> +	if (!log->bios_event_log)
>  		return -ENOMEM;
> -	}
>  
>  	log->bios_event_log_end = log->bios_event_log + *sizep;
>  
>  	memcpy(log->bios_event_log, __va(*basep), *sizep);
>  
>  	return 0;
> -
> -cleanup_eio:
> -	return -EIO;
>  }
> -- 
> 2.5.0
> 

Re: [PATCH v6 9/9] tpm: cleanup of printk error messages

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 05:00:56AM -0500, Nayna Jain wrote:
> This patch removes the unnecessary error messages on failing to
> allocate memory and replaces pr_err/printk with dev_dbg/dev_info
> as applicable.
> 
> Suggested-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

/Jarkko

> ---
>  drivers/char/tpm/tpm_acpi.c | 16 ++++------------
>  drivers/char/tpm/tpm_of.c   | 29 +++++++++--------------------
>  2 files changed, 13 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
> index fa30c969..ddbaef2 100644
> --- a/drivers/char/tpm/tpm_acpi.c
> +++ b/drivers/char/tpm/tpm_acpi.c
> @@ -60,11 +60,8 @@ int read_log_acpi(struct tpm_chip *chip)
>  	status = acpi_get_table(ACPI_SIG_TCPA, 1,
>  				(struct acpi_table_header **)&buff);
>  
> -	if (ACPI_FAILURE(status)) {
> -		printk(KERN_ERR "%s: ERROR - Could not get TCPA table\n",
> -		       __func__);
> +	if (ACPI_FAILURE(status))
>  		return -EIO;
> -	}
>  
>  	switch(buff->platform_class) {
>  	case BIOS_SERVER:
> @@ -78,25 +75,20 @@ int read_log_acpi(struct tpm_chip *chip)
>  		break;
>  	}
>  	if (!len) {
> -		printk(KERN_ERR "%s: ERROR - TCPA log area empty\n", __func__);
> +		dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
>  		return -EIO;
>  	}
>  
>  	/* malloc EventLog space */
>  	log->bios_event_log = kmalloc(len, GFP_KERNEL);
> -	if (!log->bios_event_log) {
> -		printk("%s: ERROR - Not enough  Memory for BIOS measurements\n",
> -			__func__);
> +	if (!log->bios_event_log)
>  		return -ENOMEM;
> -	}
>  
>  	log->bios_event_log_end = log->bios_event_log + len;
>  
>  	virt = acpi_os_map_iomem(start, len);
> -	if (!virt) {
> -		printk("%s: ERROR - Unable to map memory\n", __func__);
> +	if (!virt)
>  		goto err;
> -	}
>  
>  	memcpy_fromio(log->bios_event_log, virt, len);
>  
> diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
> index 22b8f81..3af829f 100644
> --- a/drivers/char/tpm/tpm_of.c
> +++ b/drivers/char/tpm/tpm_of.c
> @@ -31,40 +31,29 @@ int read_log_of(struct tpm_chip *chip)
>  	log = &chip->log;
>  	if (chip->dev.parent->of_node)
>  		np = chip->dev.parent->of_node;
> -	if (!np) {
> -		pr_err("%s: ERROR - IBMVTPM not supported\n", __func__);
> +	if (!np)
>  		return -ENODEV;
> -	}
>  
>  	sizep = of_get_property(np, "linux,sml-size", NULL);
> -	if (sizep == NULL) {
> -		pr_err("%s: ERROR - SML size not found\n", __func__);
> -		goto cleanup_eio;
> -	}
> +	if (sizep == NULL)
> +		return -EIO;
> +
>  	if (*sizep == 0) {
> -		pr_err("%s: ERROR - event log area empty\n", __func__);
> -		goto cleanup_eio;
> +		dev_warn(&chip->dev, "%s: Event log area empty\n", __func__);
> +		return -EIO;
>  	}
>  
>  	basep = of_get_property(np, "linux,sml-base", NULL);
> -	if (basep == NULL) {
> -		pr_err("%s: ERROR - SML not found\n", __func__);
> -		goto cleanup_eio;
> -	}
> +	if (basep == NULL)
> +		return -EIO;
>  
>  	log->bios_event_log = kmalloc(*sizep, GFP_KERNEL);
> -	if (!log->bios_event_log) {
> -		pr_err("%s: ERROR - Not enough memory for BIOS measurements\n",
> -		       __func__);
> +	if (!log->bios_event_log)
>  		return -ENOMEM;
> -	}
>  
>  	log->bios_event_log_end = log->bios_event_log + *sizep;
>  
>  	memcpy(log->bios_event_log, __va(*basep), *sizep);
>  
>  	return 0;
> -
> -cleanup_eio:
> -	return -EIO;
>  }
> -- 
> 2.5.0
> 

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

Re: [PATCH v6 9/9] tpm: cleanup of printk error messages

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 02:24:06PM -0800, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 05:00:56AM -0500, Nayna Jain wrote:
> > This patch removes the unnecessary error messages on failing to
> > allocate memory and replaces pr_err/printk with dev_dbg/dev_info
> > as applicable.
> > 
> > Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 05:00:47AM -0500, Nayna Jain wrote:
> This patch set includes the cleanup and bug fixes patches, previously
> part of the "tpm: add the securityfs pseudo files support for TPM 2.0
> firmware event log" patch set, in order to upstream them more quickly.

I applied the patches. I'm not yet sure whether these are part of the
4.10 pull request or whether I postpone to 4.11 (my preference would be
4.10 but I do not want to close that right now). I'll do testing next
week before doing pull request.

I hope that the commits gets some reviews and testing now that they are
easily testable in my master branch.

/Jarkko

> 
> Changelog History:
> 
> v6:
> 
> - Patch "tpm: replace symbolic permission with octal for securityfs files"
>   - New Patch.
> - Patch "tpm: have event log use the tpm_chip"
>   - Changed commit description as per Jason's suggestion.
>   - Fixed bug related to kfree() for bios_event_log.
>   - Moved inode_unlock() just after get_device() in open().
>   - Returned -ENODEV for read_log() ENOMEM error and other errors as it is.
>   - Added comment in tpm_bios_log_teardown() to explain inode_lock()/unlock
>     reasoning.
>   - Splitted .owner into different patch.
> - Patch "tpm: fix the missing .owner in tpm_bios_measurements_ops"
>   - New Patch.
> - Patch "tpm: cleanup of printk error messages"
>   - Replaced dev_info() with dev_warn().
>   - Updated commit description subject line.
> 
> v5:
> 
> - Moved cleanup/fixes patches into this patch set.
> - Patch "fix the race condition between event log access and chip
> getting unregistered"
>   - updated subject line and commit description.
>   - modified fops code to use chip kref.
>   - modified fops to lock inode before accessing inode private data.
>   - renamed tpm_securityfs_data to tpm_chip_seqops, as it no more
>   holds bios log, but associates seqops with respective chip. For
>   the same reason, moved it to tpm.h
> - Patch "replace or remove printk error messages"
>   - cleaned up dev_dbg and used dev_info as applicable.
> 
> v4:
> 
> - Includes feedbacks from Jarkko and Jason.
> - Patch "tpm: define a generic open() method for ascii & bios
> measurements".
>   - Fix indentation issue.
> - Patch "tpm: replace the dynamically allocated bios_dir as
>   struct dentry array". 
>   - Continue to use bios_dir_count variable to use is_bad() checks and
>   to maintain correct order for securityfs_remove() during teardown.
>   - Reset chip->bios_dir_count in teardown() function.
> - Patch "tpm: validate the event log access before tpm_bios_log_setup".
>   - Retain TPM2 check which was removed in previous patch.
>   - Add tpm_bios_log_setup failure handling.
>   - Remove use of private data from v3 version of patch. Add a new
>   member to struct tpm_chip to achieve the same purpose.
> - Patch "tpm: redefine the read_log method to check for ACPI/OF 
> properties sequentially".
>   - Move replacement of CONFIG_TCG_IBMVTPM with CONFIG_OF to
>   this patch from patch 3.
>   - Replace -1 error code with -ENODEV.
> - Patch "tpm: replace the of_find_node_by_name() with dev of_node
> property".
>  - Uses chip->dev.parent->of_node.
>  - Created separate patch for cleanup of pr_err messages.
> - Patch "tpm: remove printk error messages".
>  - New Patch.
> - Patch "tpm: add the securityfs file support for TPM 2.0 event log".
>  - Parses event digests using event alg_id rather than event log header
>  alg_id.
>  - Uses of_property_match_string to differentiate tpm/vtpm compatible
>  property.
>  - Adds the comment for difference in tpm/vtpm endianness.
> 
> v3:
> 
> - Includes the review feedbacks as suggested by Jason.
> - Split of patches into one patch per idea.
> - Generic open() method for ascii/bios measurements.
> - Replacement of of **bios_dir with *bios_dir[3].
> - Verifying readlog() is successful before creating securityfs entries.
> - Generic readlog() to check for ACPI/OF in sequence.
> - read_log_of() method now uses of_node propertry rather than
> calling find_device_by_name.
> - read_log differentiates vtpm/tpm using its compatible property.
> - Cleans pr_err with dev_dbg.
> - Commit msgs subject line prefixed with tpm.
> 
> v2:
> 
> - Fixes issues as given in feedback by Jason.
> - Adds documentation for device tree.
> 
> Nayna Jain (9):
>   tpm: define a generic open() method for ascii & bios measurements
>   tpm: replace symbolic permission with octal for securityfs files
>   tpm: replace dynamically allocated bios_dir with a static array
>   tpm: drop tpm1_chip_register(/unregister)
>   tpm: have event log use the tpm_chip
>   tpm: fix the missing .owner in tpm_bios_measurements_ops
>   tpm: redefine read_log() to handle ACPI/OF at runtime
>   tpm: replace of_find_node_by_name() with dev of_node property
>   tpm: cleanup of printk error messages
> 
>  drivers/char/tpm/Makefile       |  14 +--
>  drivers/char/tpm/tpm-chip.c     |  33 ++----
>  drivers/char/tpm/tpm-sysfs.c    |   3 +
>  drivers/char/tpm/tpm.h          |  14 ++-
>  drivers/char/tpm/tpm_acpi.c     |  38 +++----
>  drivers/char/tpm/tpm_eventlog.c | 222 +++++++++++++++++++++-------------------
>  drivers/char/tpm/tpm_eventlog.h |  22 ++--
>  drivers/char/tpm/tpm_of.c       |  45 +++-----
>  8 files changed, 187 insertions(+), 204 deletions(-)
> 
> -- 
> 2.5.0
> 

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 05:00:47AM -0500, Nayna Jain wrote:
> This patch set includes the cleanup and bug fixes patches, previously
> part of the "tpm: add the securityfs pseudo files support for TPM 2.0
> firmware event log" patch set, in order to upstream them more quickly.

I applied the patches. I'm not yet sure whether these are part of the
4.10 pull request or whether I postpone to 4.11 (my preference would be
4.10 but I do not want to close that right now). I'll do testing next
week before doing pull request.

I hope that the commits gets some reviews and testing now that they are
easily testable in my master branch.

/Jarkko

> 
> Changelog History:
> 
> v6:
> 
> - Patch "tpm: replace symbolic permission with octal for securityfs files"
>   - New Patch.
> - Patch "tpm: have event log use the tpm_chip"
>   - Changed commit description as per Jason's suggestion.
>   - Fixed bug related to kfree() for bios_event_log.
>   - Moved inode_unlock() just after get_device() in open().
>   - Returned -ENODEV for read_log() ENOMEM error and other errors as it is.
>   - Added comment in tpm_bios_log_teardown() to explain inode_lock()/unlock
>     reasoning.
>   - Splitted .owner into different patch.
> - Patch "tpm: fix the missing .owner in tpm_bios_measurements_ops"
>   - New Patch.
> - Patch "tpm: cleanup of printk error messages"
>   - Replaced dev_info() with dev_warn().
>   - Updated commit description subject line.
> 
> v5:
> 
> - Moved cleanup/fixes patches into this patch set.
> - Patch "fix the race condition between event log access and chip
> getting unregistered"
>   - updated subject line and commit description.
>   - modified fops code to use chip kref.
>   - modified fops to lock inode before accessing inode private data.
>   - renamed tpm_securityfs_data to tpm_chip_seqops, as it no more
>   holds bios log, but associates seqops with respective chip. For
>   the same reason, moved it to tpm.h
> - Patch "replace or remove printk error messages"
>   - cleaned up dev_dbg and used dev_info as applicable.
> 
> v4:
> 
> - Includes feedbacks from Jarkko and Jason.
> - Patch "tpm: define a generic open() method for ascii & bios
> measurements".
>   - Fix indentation issue.
> - Patch "tpm: replace the dynamically allocated bios_dir as
>   struct dentry array". 
>   - Continue to use bios_dir_count variable to use is_bad() checks and
>   to maintain correct order for securityfs_remove() during teardown.
>   - Reset chip->bios_dir_count in teardown() function.
> - Patch "tpm: validate the event log access before tpm_bios_log_setup".
>   - Retain TPM2 check which was removed in previous patch.
>   - Add tpm_bios_log_setup failure handling.
>   - Remove use of private data from v3 version of patch. Add a new
>   member to struct tpm_chip to achieve the same purpose.
> - Patch "tpm: redefine the read_log method to check for ACPI/OF 
> properties sequentially".
>   - Move replacement of CONFIG_TCG_IBMVTPM with CONFIG_OF to
>   this patch from patch 3.
>   - Replace -1 error code with -ENODEV.
> - Patch "tpm: replace the of_find_node_by_name() with dev of_node
> property".
>  - Uses chip->dev.parent->of_node.
>  - Created separate patch for cleanup of pr_err messages.
> - Patch "tpm: remove printk error messages".
>  - New Patch.
> - Patch "tpm: add the securityfs file support for TPM 2.0 event log".
>  - Parses event digests using event alg_id rather than event log header
>  alg_id.
>  - Uses of_property_match_string to differentiate tpm/vtpm compatible
>  property.
>  - Adds the comment for difference in tpm/vtpm endianness.
> 
> v3:
> 
> - Includes the review feedbacks as suggested by Jason.
> - Split of patches into one patch per idea.
> - Generic open() method for ascii/bios measurements.
> - Replacement of of **bios_dir with *bios_dir[3].
> - Verifying readlog() is successful before creating securityfs entries.
> - Generic readlog() to check for ACPI/OF in sequence.
> - read_log_of() method now uses of_node propertry rather than
> calling find_device_by_name.
> - read_log differentiates vtpm/tpm using its compatible property.
> - Cleans pr_err with dev_dbg.
> - Commit msgs subject line prefixed with tpm.
> 
> v2:
> 
> - Fixes issues as given in feedback by Jason.
> - Adds documentation for device tree.
> 
> Nayna Jain (9):
>   tpm: define a generic open() method for ascii & bios measurements
>   tpm: replace symbolic permission with octal for securityfs files
>   tpm: replace dynamically allocated bios_dir with a static array
>   tpm: drop tpm1_chip_register(/unregister)
>   tpm: have event log use the tpm_chip
>   tpm: fix the missing .owner in tpm_bios_measurements_ops
>   tpm: redefine read_log() to handle ACPI/OF at runtime
>   tpm: replace of_find_node_by_name() with dev of_node property
>   tpm: cleanup of printk error messages
> 
>  drivers/char/tpm/Makefile       |  14 +--
>  drivers/char/tpm/tpm-chip.c     |  33 ++----
>  drivers/char/tpm/tpm-sysfs.c    |   3 +
>  drivers/char/tpm/tpm.h          |  14 ++-
>  drivers/char/tpm/tpm_acpi.c     |  38 +++----
>  drivers/char/tpm/tpm_eventlog.c | 222 +++++++++++++++++++++-------------------
>  drivers/char/tpm/tpm_eventlog.h |  22 ++--
>  drivers/char/tpm/tpm_of.c       |  45 +++-----
>  8 files changed, 187 insertions(+), 204 deletions(-)
> 
> -- 
> 2.5.0
> 

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

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 02:33:23PM -0800, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 05:00:47AM -0500, Nayna Jain wrote:
> > This patch set includes the cleanup and bug fixes patches, previously
> > part of the "tpm: add the securityfs pseudo files support for TPM 2.0
> > firmware event log" patch set, in order to upstream them more quickly.
> 
> I applied the patches. I'm not yet sure whether these are part of the
> 4.10 pull request or whether I postpone to 4.11 (my preference would be
> 4.10 but I do not want to close that right now). I'll do testing next
> week before doing pull request.
> 
> I hope that the commits gets some reviews and testing now that they are
> easily testable in my master branch.

Event log still works and they do not seem to break TPM 2.0 (tried both
machine with tpm_crb and tpm_tis).

Stefan: would you mind check that these do not break your TPM 1.2
environment? I already tried wih TPM 1.2 machine but probably would
make sense to peer test.

/Jarkko

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 04:25:14PM -0800, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 02:33:23PM -0800, Jarkko Sakkinen wrote:
> > On Mon, Nov 14, 2016 at 05:00:47AM -0500, Nayna Jain wrote:
> > > This patch set includes the cleanup and bug fixes patches, previously
> > > part of the "tpm: add the securityfs pseudo files support for TPM 2.0
> > > firmware event log" patch set, in order to upstream them more quickly.
> > 
> > I applied the patches. I'm not yet sure whether these are part of the
> > 4.10 pull request or whether I postpone to 4.11 (my preference would be
> > 4.10 but I do not want to close that right now). I'll do testing next
> > week before doing pull request.
> > 
> > I hope that the commits gets some reviews and testing now that they are
> > easily testable in my master branch.
> 
> Event log still works and they do not seem to break TPM 2.0 (tried both
> machine with tpm_crb and tpm_tis).
> 
> Stefan: would you mind check that these do not break your TPM 1.2
> environment? I already tried wih TPM 1.2 machine but probably would
> make sense to peer test.

They are now also in my next branch, which gets pulled to linux-next in
order to get wider exposure.

/Jarkko

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 04:25:14PM -0800, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 02:33:23PM -0800, Jarkko Sakkinen wrote:
> > On Mon, Nov 14, 2016 at 05:00:47AM -0500, Nayna Jain wrote:
> > > This patch set includes the cleanup and bug fixes patches, previously
> > > part of the "tpm: add the securityfs pseudo files support for TPM 2.0
> > > firmware event log" patch set, in order to upstream them more quickly.
> > 
> > I applied the patches. I'm not yet sure whether these are part of the
> > 4.10 pull request or whether I postpone to 4.11 (my preference would be
> > 4.10 but I do not want to close that right now). I'll do testing next
> > week before doing pull request.
> > 
> > I hope that the commits gets some reviews and testing now that they are
> > easily testable in my master branch.
> 
> Event log still works and they do not seem to break TPM 2.0 (tried both
> machine with tpm_crb and tpm_tis).
> 
> Stefan: would you mind check that these do not break your TPM 1.2
> environment? I already tried wih TPM 1.2 machine but probably would
> make sense to peer test.

They are now also in my next branch, which gets pulled to linux-next in
order to get wider exposure.

/Jarkko

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

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Jarkko Sakkinen]

On Mon, Nov 14, 2016 at 04:25:14PM -0800, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 02:33:23PM -0800, Jarkko Sakkinen wrote:
> > On Mon, Nov 14, 2016 at 05:00:47AM -0500, Nayna Jain wrote:
> > > This patch set includes the cleanup and bug fixes patches, previously
> > > part of the "tpm: add the securityfs pseudo files support for TPM 2.0
> > > firmware event log" patch set, in order to upstream them more quickly.
> > 
> > I applied the patches. I'm not yet sure whether these are part of the
> > 4.10 pull request or whether I postpone to 4.11 (my preference would be
> > 4.10 but I do not want to close that right now). I'll do testing next
> > week before doing pull request.
> > 
> > I hope that the commits gets some reviews and testing now that they are
> > easily testable in my master branch.
> 
> Event log still works and they do not seem to break TPM 2.0 (tried both
> machine with tpm_crb and tpm_tis).
> 
> Stefan: would you mind check that these do not break your TPM 1.2
> environment? I already tried wih TPM 1.2 machine but probably would
> make sense to peer test.

I'm dropping commits 8/9 and 9/9 from my tree and *will not* include
them to my 4.10 pull request.

/Jarkko

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Nayna]

On 11/15/2016 07:45 AM, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 04:25:14PM -0800, Jarkko Sakkinen wrote:
>> On Mon, Nov 14, 2016 at 02:33:23PM -0800, Jarkko Sakkinen wrote:
>>> On Mon, Nov 14, 2016 at 05:00:47AM -0500, Nayna Jain wrote:
>>>> This patch set includes the cleanup and bug fixes patches, previously
>>>> part of the "tpm: add the securityfs pseudo files support for TPM 2.0
>>>> firmware event log" patch set, in order to upstream them more quickly.
>>>
>>> I applied the patches. I'm not yet sure whether these are part of the
>>> 4.10 pull request or whether I postpone to 4.11 (my preference would be
>>> 4.10 but I do not want to close that right now). I'll do testing next
>>> week before doing pull request.
>>>
>>> I hope that the commits gets some reviews and testing now that they are
>>> easily testable in my master branch.
>>
>> Event log still works and they do not seem to break TPM 2.0 (tried both
>> machine with tpm_crb and tpm_tis).
>>
>> Stefan: would you mind check that these do not break your TPM 1.2
>> environment? I already tried wih TPM 1.2 machine but probably would
>> make sense to peer test.
>
> I'm dropping commits 8/9 and 9/9 from my tree and *will not* include
> them to my 4.10 pull request.

Will fix this and resend the patch 8/9 and 9/9 again.

Thanks & Regards,
    - Nayna

>
> /Jarkko
>

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Nayna]

On 11/15/2016 07:45 AM, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2016 at 04:25:14PM -0800, Jarkko Sakkinen wrote:
>> On Mon, Nov 14, 2016 at 02:33:23PM -0800, Jarkko Sakkinen wrote:
>>> On Mon, Nov 14, 2016 at 05:00:47AM -0500, Nayna Jain wrote:
>>>> This patch set includes the cleanup and bug fixes patches, previously
>>>> part of the "tpm: add the securityfs pseudo files support for TPM 2.0
>>>> firmware event log" patch set, in order to upstream them more quickly.
>>>
>>> I applied the patches. I'm not yet sure whether these are part of the
>>> 4.10 pull request or whether I postpone to 4.11 (my preference would be
>>> 4.10 but I do not want to close that right now). I'll do testing next
>>> week before doing pull request.
>>>
>>> I hope that the commits gets some reviews and testing now that they are
>>> easily testable in my master branch.
>>
>> Event log still works and they do not seem to break TPM 2.0 (tried both
>> machine with tpm_crb and tpm_tis).
>>
>> Stefan: would you mind check that these do not break your TPM 1.2
>> environment? I already tried wih TPM 1.2 machine but probably would
>> make sense to peer test.
>
> I'm dropping commits 8/9 and 9/9 from my tree and *will not* include
> them to my 4.10 pull request.

Will fix this and resend the patch 8/9 and 9/9 again.

Thanks & Regards,
    - Nayna

>
> /Jarkko
>


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

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Jarkko Sakkinen]

On Tue, Nov 15, 2016 at 11:05:42AM +0530, Nayna wrote:
> 
> 
> On 11/15/2016 07:45 AM, Jarkko Sakkinen wrote:
> > On Mon, Nov 14, 2016 at 04:25:14PM -0800, Jarkko Sakkinen wrote:
> > > On Mon, Nov 14, 2016 at 02:33:23PM -0800, Jarkko Sakkinen wrote:
> > > > On Mon, Nov 14, 2016 at 05:00:47AM -0500, Nayna Jain wrote:
> > > > > This patch set includes the cleanup and bug fixes patches, previously
> > > > > part of the "tpm: add the securityfs pseudo files support for TPM 2.0
> > > > > firmware event log" patch set, in order to upstream them more quickly.
> > > > 
> > > > I applied the patches. I'm not yet sure whether these are part of the
> > > > 4.10 pull request or whether I postpone to 4.11 (my preference would be
> > > > 4.10 but I do not want to close that right now). I'll do testing next
> > > > week before doing pull request.
> > > > 
> > > > I hope that the commits gets some reviews and testing now that they are
> > > > easily testable in my master branch.
> > > 
> > > Event log still works and they do not seem to break TPM 2.0 (tried both
> > > machine with tpm_crb and tpm_tis).
> > > 
> > > Stefan: would you mind check that these do not break your TPM 1.2
> > > environment? I already tried wih TPM 1.2 machine but probably would
> > > make sense to peer test.
> > 
> > I'm dropping commits 8/9 and 9/9 from my tree and *will not* include
> > them to my 4.10 pull request.
> 
> Will fix this and resend the patch 8/9 and 9/9 again.

I applied fix from Colin. I for OF specific patches in this patch set
I do not have means to test the code paths that exercise OF specific
functionality. This is what worries me a bit. If I had tested-by from
someone running a system that can exercise those code paths, I would
be less worried.

/Jarkko

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Jason Gunthorpe]

On Tue, Nov 15, 2016 at 09:40:12AM -0800, Jarkko Sakkinen wrote:

> I applied fix from Colin. I for OF specific patches in this patch set
> I do not have means to test the code paths that exercise OF specific
> functionality. This is what worries me a bit. If I had tested-by from
> someone running a system that can exercise those code paths, I would
> be less worried.

I can probably check it next week on my OF systems that do not use event log

Jason

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Jarkko Sakkinen]

On Tue, Nov 15, 2016 at 11:06:10AM -0700, Jason Gunthorpe wrote:
> On Tue, Nov 15, 2016 at 09:40:12AM -0800, Jarkko Sakkinen wrote:
> 
> > I applied fix from Colin. I for OF specific patches in this patch set
> > I do not have means to test the code paths that exercise OF specific
> > functionality. This is what worries me a bit. If I had tested-by from
> > someone running a system that can exercise those code paths, I would
> > be less worried.
> 
> I can probably check it next week on my OF systems that do not use event log

Thanks Jason. I'll apppend your Tested-by's to the commits when you're
done. Probably doing pull request in the latter part of next week.

/Jarkko

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Jason Gunthorpe]

On Tue, Nov 15, 2016 at 10:54:53AM -0800, Jarkko Sakkinen wrote:
> On Tue, Nov 15, 2016 at 11:06:10AM -0700, Jason Gunthorpe wrote:
> > On Tue, Nov 15, 2016 at 09:40:12AM -0800, Jarkko Sakkinen wrote:
> > 
> > > I applied fix from Colin. I for OF specific patches in this patch set
> > > I do not have means to test the code paths that exercise OF specific
> > > functionality. This is what worries me a bit. If I had tested-by from
> > > someone running a system that can exercise those code paths, I would
> > > be less worried.
> > 
> > I can probably check it next week on my OF systems that do not use event log
> 
> Thanks Jason. I'll apppend your Tested-by's to the commits when you're
> done. Probably doing pull request in the latter part of next week.

What are we testing? Your master now? It will need the patch I just
sent or it will fail for me.

Jason

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Jason Gunthorpe]

On Tue, Nov 15, 2016 at 10:54:53AM -0800, Jarkko Sakkinen wrote:
> On Tue, Nov 15, 2016 at 11:06:10AM -0700, Jason Gunthorpe wrote:
> > On Tue, Nov 15, 2016 at 09:40:12AM -0800, Jarkko Sakkinen wrote:
> > 
> > > I applied fix from Colin. I for OF specific patches in this patch set
> > > I do not have means to test the code paths that exercise OF specific
> > > functionality. This is what worries me a bit. If I had tested-by from
> > > someone running a system that can exercise those code paths, I would
> > > be less worried.
> > 
> > I can probably check it next week on my OF systems that do not use event log
> 
> Thanks Jason. I'll apppend your Tested-by's to the commits when you're
> done. Probably doing pull request in the latter part of next week.

What are we testing? Your master now? It will need the patch I just
sent or it will fail for me.

Jason

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

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Jarkko Sakkinen]

On Sat, Nov 19, 2016 at 11:36:27AM -0700, Jason Gunthorpe wrote:
> On Tue, Nov 15, 2016 at 10:54:53AM -0800, Jarkko Sakkinen wrote:
> > On Tue, Nov 15, 2016 at 11:06:10AM -0700, Jason Gunthorpe wrote:
> > > On Tue, Nov 15, 2016 at 09:40:12AM -0800, Jarkko Sakkinen wrote:
> > > 
> > > > I applied fix from Colin. I for OF specific patches in this patch set
> > > > I do not have means to test the code paths that exercise OF specific
> > > > functionality. This is what worries me a bit. If I had tested-by from
> > > > someone running a system that can exercise those code paths, I would
> > > > be less worried.
> > > 
> > > I can probably check it next week on my OF systems that do not use event log
> > 
> > Thanks Jason. I'll apppend your Tested-by's to the commits when you're
> > done. Probably doing pull request in the latter part of next week.
> 
> What are we testing? Your master now? It will need the patch I just
> sent or it will fail for me.

$ git am -3 ~/Desktop/foo.txt 
Patch is empty. Was it split wrong?
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

I'm at the Heathrow airport boarding. Look it more properly later on

/Jarkko

Re: [PATCH v6 0/9] tpm: cleanup/fixes in existing event log support

[Jason Gunthorpe]

On Sun, Nov 20, 2016 at 09:58:48AM +0000, Jarkko Sakkinen wrote:
> On Sat, Nov 19, 2016 at 11:36:27AM -0700, Jason Gunthorpe wrote:
> > On Tue, Nov 15, 2016 at 10:54:53AM -0800, Jarkko Sakkinen wrote:
> > > On Tue, Nov 15, 2016 at 11:06:10AM -0700, Jason Gunthorpe wrote:
> > > > On Tue, Nov 15, 2016 at 09:40:12AM -0800, Jarkko Sakkinen wrote:
> > > > 
> > > > > I applied fix from Colin. I for OF specific patches in this patch set
> > > > > I do not have means to test the code paths that exercise OF specific
> > > > > functionality. This is what worries me a bit. If I had tested-by from
> > > > > someone running a system that can exercise those code paths, I would
> > > > > be less worried.
> > > > 
> > > > I can probably check it next week on my OF systems that do not use event log
> > > 
> > > Thanks Jason. I'll apppend your Tested-by's to the commits when you're
> > > done. Probably doing pull request in the latter part of next week.
> > 
> > What are we testing? Your master now? It will need the patch I just
> > sent or it will fail for me.

Your TOT works for me (60632de288aac485b328e0863f0c987062ca9b49)

There is one tiny regression, I'll send a patch for it..

Jason