diff mbox

[v3,08/11] tpm: Introduce TPM_CHIP_FLAG_VIRTUAL

Message ID 1455885728-10315-9-git-send-email-stefanb@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Stefan Berger Feb. 19, 2016, 12:42 p.m. UTC
Introduce TPM_CHIP_FLAG_VIRTUAL to be used when the chip device has no parent
device. Also adapt tpm_chip_alloc so that it can be called with parent device
being NULL.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm-chip.c  | 10 +++++-----
 drivers/char/tpm/tpm-sysfs.c | 13 +++++++++----
 drivers/char/tpm/tpm.h       |  5 +++--
 3 files changed, 17 insertions(+), 11 deletions(-)

Comments

Jason Gunthorpe Feb. 22, 2016, 7:19 p.m. UTC | #1
On Fri, Feb 19, 2016 at 07:42:05AM -0500, Stefan Berger wrote:

>  	cdev_init(&chip->cdev, &tpm_fops);
> -	chip->cdev.owner = dev->driver->owner;

Shouldn't this be:

	chip->cdev.owner = THIS_MODULE;

?

Please drop this hunk into the 'tpm: Get rid of module locking' patch

> +
> +	if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL))
> +		err = sysfs_create_group(&chip->dev.parent->kobj,
> +					 &tpm_dev_group);
> +	else
> +		chip->groups[chip->groups_cnt++] = &tpm_dev_group;

For other reviewers: The intent here is to make another patch that
moves the main sysfs to only use groups with symlinks link ppi does
now.

Does sysfs work with vtpm? I am worried about dev_get_drvdata here
(and other places):

 static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
                               char *buf)
 {
        struct tpm_chip *chip = dev_get_drvdata(dev);

There is only one dev_set_drvdata and it does not apply to chip->dev.

Ultimately this should change to container_of like ppi does.

Jason

------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
Stefan Berger Feb. 23, 2016, 1:20 a.m. UTC | #2
Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote on 02/22/2016 
02:19:22 PM:

> From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> To: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Cc: tpmdd-devel@lists.sourceforge.net
> Date: 02/22/2016 02:19 PM
> Subject: Re: [tpmdd-devel] [PATCH v3 08/11] tpm: Introduce 
> TPM_CHIP_FLAG_VIRTUAL
> 
> On Fri, Feb 19, 2016 at 07:42:05AM -0500, Stefan Berger wrote:
> 
> >     cdev_init(&chip->cdev, &tpm_fops);
> > -   chip->cdev.owner = dev->driver->owner;
> 
> Shouldn't this be:
> 
>    chip->cdev.owner = THIS_MODULE;
> 
> ?
> 
> Please drop this hunk into the 'tpm: Get rid of module locking' patch

Done.

> 
> > +
> > +   if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL))
> > +      err = sysfs_create_group(&chip->dev.parent->kobj,
> > +                &tpm_dev_group);
> > +   else
> > +      chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> 
> For other reviewers: The intent here is to make another patch that
> moves the main sysfs to only use groups with symlinks link ppi does
> now.
> 
> Does sysfs work with vtpm? I am worried about dev_get_drvdata here
> (and other places):
> 
>  static ssize_t durations_show(struct device *dev, struct 
> device_attribute *attr,
>                                char *buf)
>  {
>         struct tpm_chip *chip = dev_get_drvdata(dev);
> 
> There is only one dev_set_drvdata and it does not apply to chip->dev.
> 
> Ultimately this should change to container_of like ppi does.


Fix in this patch or yet another one ?

   Stefan

> 
> Jason
> 
> 
------------------------------------------------------------------------------
> Site24x7 APM Insight: Get Deep Visibility into Application Performance
> APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
> Monitor end-to-end web transactions and take corrective actions now
> Troubleshoot faster and improve end-user experience. Signup Now!
> http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
>
------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
Stefan Berger Feb. 23, 2016, 1:21 a.m. UTC | #3
Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote on 02/22/2016 
02:19:22 PM:


> > +
> > +   if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL))
> > +      err = sysfs_create_group(&chip->dev.parent->kobj,
> > +                &tpm_dev_group);
> > +   else
> > +      chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> 
> For other reviewers: The intent here is to make another patch that
> moves the main sysfs to only use groups with symlinks link ppi does
> now.
> 
> Does sysfs work with vtpm? I am worried about dev_get_drvdata here
> (and other places):

Yes, works. I have a set_drv_data in the vtpm driver.

   Stefan
------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
Jason Gunthorpe Feb. 23, 2016, 2:05 a.m. UTC | #4
On Mon, Feb 22, 2016 at 08:21:09PM -0500, Stefan Berger wrote:
> 
> Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote on 02/22/2016 02:19:22
> PM:
> 
> 
> > > +
> > > +   if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL))
> > > +      err = sysfs_create_group(&chip->dev.parent->kobj,
> > > +                &tpm_dev_group);
> > > +   else
> > > +      chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> >
> > For other reviewers: The intent here is to make another patch that
> > moves the main sysfs to only use groups with symlinks link ppi does
> > now.
> >
> > Does sysfs work with vtpm? I am worried about dev_get_drvdata here
> > (and other places):
> 
> Yes, works. I have a set_drv_data in the vtpm driver.

?? I don't see that in the vtpm patch posted to the list? Can you
point me to it?

In any event, the core code should be self contained and not rely on
things in the drivers like this.

Jason

------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
Jason Gunthorpe Feb. 23, 2016, 2:06 a.m. UTC | #5
On Mon, Feb 22, 2016 at 08:20:03PM -0500, Stefan Berger wrote:
> > Ultimately this should change to container_of like ppi does.
> 
> Fix in this patch or yet another one ?

It would have to go with a future patch to change always use compat
symlinks for the parent.

Jason

------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
Stefan Berger Feb. 23, 2016, 3:40 a.m. UTC | #6
Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote on 02/22/2016 
09:05:15 PM:

> 
> On Mon, Feb 22, 2016 at 08:21:09PM -0500, Stefan Berger wrote:
> > 
> > Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote on 02/22/
> 2016 02:19:22
> > PM:
> > 
> > 
> > > > +
> > > > +   if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL))
> > > > +      err = sysfs_create_group(&chip->dev.parent->kobj,
> > > > +                &tpm_dev_group);
> > > > +   else
> > > > +      chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> > >
> > > For other reviewers: The intent here is to make another patch that
> > > moves the main sysfs to only use groups with symlinks link ppi does
> > > now.
> > >
> > > Does sysfs work with vtpm? I am worried about dev_get_drvdata here
> > > (and other places):
> > 
> > Yes, works. I have a set_drv_data in the vtpm driver.
> 
> ?? I don't see that in the vtpm patch posted to the list? Can you
> point me to it?

That was in the past that I had that call ... when there still was a 
device and when sysfs was still working.

I now converted the dev_get_drvdata in tpm-interface.c and tpm-sysfs.c to 
container_of to make sysfs work for vtpm. It has to be part of this patch.

  Stefan

> 
> In any event, the core code should be self contained and not rely on
> things in the drivers like this.
> 
> Jason
>
------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
diff mbox

Patch

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 4fd36ba..2585f6a 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -168,9 +168,7 @@  struct tpm_chip *tpm_chip_alloc(struct device *dev,
 	chip->dev.class = tpm_class;
 	chip->dev.release = tpm_dev_release;
 	chip->dev.parent = dev;
-#ifdef CONFIG_ACPI
 	chip->dev.groups = chip->groups;
-#endif
 
 	if (chip->dev_num == 0)
 		chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
@@ -181,8 +179,10 @@  struct tpm_chip *tpm_chip_alloc(struct device *dev,
 	if (err)
 		goto out;
 
+	if (!dev)
+		chip->flags |= TPM_CHIP_FLAG_VIRTUAL;
+
 	cdev_init(&chip->cdev, &tpm_fops);
-	chip->cdev.owner = dev->driver->owner;
 	chip->cdev.kobj.parent = &chip->dev.kobj;
 
 	return chip;
@@ -318,7 +318,7 @@  int tpm_chip_register(struct tpm_chip *chip)
 
 	chip->flags |= TPM_CHIP_FLAG_REGISTERED;
 
-	if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
+	if (!(chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))) {
 		rc = __compat_only_sysfs_link_entry_to_kobj(
 		    &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
 		if (rc && rc != -ENOENT) {
@@ -359,7 +359,7 @@  void tpm_chip_unregister(struct tpm_chip *chip)
 	idr_replace(&dev_nums_idr, NULL, chip->dev_num);
 	mutex_unlock(&idr_lock);
 
-	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+	if (!(chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL)))
 		sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
 
 	tpm1_chip_unregister(chip);
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 34e7fc7..127b0e5 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -283,9 +283,13 @@  static const struct attribute_group tpm_dev_group = {
 
 int tpm_sysfs_add_device(struct tpm_chip *chip)
 {
-	int err;
-	err = sysfs_create_group(&chip->dev.parent->kobj,
-				 &tpm_dev_group);
+	int err = 0;
+
+	if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL))
+		err = sysfs_create_group(&chip->dev.parent->kobj,
+					 &tpm_dev_group);
+	else
+		chip->groups[chip->groups_cnt++] = &tpm_dev_group;
 
 	if (err)
 		dev_err(&chip->dev,
@@ -300,5 +304,6 @@  void tpm_sysfs_del_device(struct tpm_chip *chip)
 	 * synchronizes this removal so that no callbacks are running or can
 	 * run again
 	 */
-	sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group);
+	if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL))
+		sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group);
 }
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 2ca5fb4..dc50ca6 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -168,6 +168,7 @@  struct tpm_vendor_specific {
 enum tpm_chip_flags {
 	TPM_CHIP_FLAG_REGISTERED	= BIT(0),
 	TPM_CHIP_FLAG_TPM2		= BIT(1),
+	TPM_CHIP_FLAG_VIRTUAL		= BIT(2),
 };
 
 struct tpm_chip {
@@ -193,9 +194,9 @@  struct tpm_chip {
 
 	struct dentry **bios_dir;
 
-#ifdef CONFIG_ACPI
-	const struct attribute_group *groups[2];
+	const struct attribute_group *groups[3];
 	unsigned int groups_cnt;
+#ifdef CONFIG_ACPI
 	acpi_handle acpi_dev_handle;
 	char ppi_version[TPM_PPI_VERSION_LEN + 1];
 #endif /* CONFIG_ACPI */