* [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices. @ 2017-06-16 17:25 Josh Zimmerman via tpmdd-devel [not found] ` <20170616172531.28464-1-joshz-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 14+ messages in thread From: Josh Zimmerman via tpmdd-devel @ 2017-06-16 17:25 UTC (permalink / raw) To: Peter Huewe, Marcel Selhorst, Jarkko Sakkinen, Jason Gunthorpe, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, jmorris-gx6/JNMH7DfYtjvyW6yDsg If a TPM2 loses power without a TPM2_Shutdown command being issued (a "disorderly reboot"), it may lose some state that has yet to be persisted to NVRam, and will increment the DA counter. After the DA counter gets sufficiently large, the TPM will lock the user out. NOTE: This only changes behavior on TPM2 devices. Since TPM1 uses sysfs, and sysfs relies on implicit locking on chip->ops, it is not safe to allow this code to run in TPM1, or to add sysfs support to TPM2, until that locking is made explicit. Signed-off-by: Josh Zimmerman <joshz-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org ---- v2: - Properly split changes between this and another commit - Use proper locking primitive. - Fix commenting style v3: - Re-fix commenting style v4: - Update description and tags (Reviewed-by, Cc). v5: - Update documentation. v6: - Call device and/or bus shutdown from tpm_shutdown. --- drivers/char/tpm/tpm-chip.c | 31 +++++++++++++++++++++++++++++++ drivers/char/tpm/tpm-sysfs.c | 3 +++ 2 files changed, 34 insertions(+) diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index 9dec9f551b83..62691d266f22 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -142,6 +142,36 @@ static void tpm_devs_release(struct device *dev) put_device(&chip->dev); } +/** + * tpm_shutdown() - prepare the TPM device for loss of power. + * @dev: device to which the chip is associated. + * + * Issues a TPM2_Shutdown command prior to loss of power, as required by the + * TPM 2.0 spec. + * + * XXX: This codepath relies on the fact that sysfs is not enabled for + * TPM2: sysfs uses an implicit lock on chip->ops, so this could race if TPM2 + * has sysfs support enabled before TPM sysfs's implicit locking is fixed. + */ +static void tpm_shutdown(struct device *dev) +{ + struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev); + + if (chip->flags & TPM_CHIP_FLAG_TPM2) { + down_write(&chip->ops_sem); + tpm2_shutdown(chip, TPM_SU_CLEAR); + chip->ops = NULL; + up_write(&chip->ops_sem); + } + // Allow bus- and device-specific code to run. Note: since chip->ops + // is NULL, more-specific shutdown code will not be able to issue TPM + // commands. + if (dev->bus->shutdown) + dev->bus->shutdown(dev); + else if (dev->driver && dev->driver->shutdown) + dev->driver->shutdown(dev); +} + /** * tpm_chip_alloc() - allocate a new struct tpm_chip instance * @pdev: device to which the chip is associated @@ -181,6 +211,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev, device_initialize(&chip->devs); chip->dev.class = tpm_class; + chip->dev.class.shutdown = tpm_shutdown; chip->dev.release = tpm_dev_release; chip->dev.parent = pdev; chip->dev.groups = chip->groups; diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c index 55405dbe43fa..59bd0b7b5959 100644 --- a/drivers/char/tpm/tpm-sysfs.c +++ b/drivers/char/tpm/tpm-sysfs.c @@ -294,6 +294,9 @@ static const struct attribute_group tpm_dev_group = { void tpm_sysfs_add_device(struct tpm_chip *chip) { + /* XXX: If you wish to remove this restriction, you must first update + * tpm_sysfs to explicitly lock chip->ops. + */ if (chip->flags & TPM_CHIP_FLAG_TPM2) return; -- 2.13.1.518.g3df882009-goog ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply related [flat|nested] 14+ messages in thread
[parent not found: <20170616172531.28464-1-joshz-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>]
* [PATCH v6 2/2] Add "shutdown" to "struct class". [not found] ` <20170616172531.28464-1-joshz-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> @ 2017-06-16 17:25 ` Josh Zimmerman via tpmdd-devel 2017-06-18 21:53 ` [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices Jarkko Sakkinen 2017-06-18 23:21 ` Jarkko Sakkinen 2 siblings, 0 replies; 14+ messages in thread From: Josh Zimmerman via tpmdd-devel @ 2017-06-16 17:25 UTC (permalink / raw) To: Peter Huewe, Marcel Selhorst, Jarkko Sakkinen, Jason Gunthorpe, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, jmorris-gx6/JNMH7DfYtjvyW6yDsg The TPM class has some common shutdown code that must be executed for all drivers. This adds some needed functionality for that. Signed-off-by: Josh Zimmerman <joshz-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org ----- v2: Add Signed-off-by. v3: Remove logically separate change. v4: Add "acked-by" and "cc". v5: Execute only one of the class/bus/driver's shutdown functions. --- drivers/base/core.c | 6 +++++- include/linux/device.h | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 6bb60fb6a30b..9f426954681e 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2667,7 +2667,11 @@ void device_shutdown(void) pm_runtime_get_noresume(dev); pm_runtime_barrier(dev); - if (dev->bus && dev->bus->shutdown) { + if (dev->class && dev->class->shutdown) { + if (initcall_debug) + dev_info(dev, "shutdown\n"); + dev->class->shutdown(dev); + } else if (dev->bus && dev->bus->shutdown) { if (initcall_debug) dev_info(dev, "shutdown\n"); dev->bus->shutdown(dev); diff --git a/include/linux/device.h b/include/linux/device.h index 9ef518af5515..f240baac2001 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -378,6 +378,7 @@ int subsys_virtual_register(struct bus_type *subsys, * @suspend: Used to put the device to sleep mode, usually to a low power * state. * @resume: Used to bring the device from the sleep mode. + * @shutdown: Called at shut-down time to quiesce the device. * @ns_type: Callbacks so sysfs can detemine namespaces. * @namespace: Namespace of the device belongs to this class. * @pm: The default device power management operations of this class. @@ -407,6 +408,7 @@ struct class { int (*suspend)(struct device *dev, pm_message_t state); int (*resume)(struct device *dev); + int (*shutdown)(struct device *dev); const struct kobj_ns_type_operations *ns_type; const void *(*namespace)(struct device *dev); -- 2.13.1.518.g3df882009-goog ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices. [not found] ` <20170616172531.28464-1-joshz-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2017-06-16 17:25 ` [PATCH v6 2/2] Add "shutdown" to "struct class" Josh Zimmerman via tpmdd-devel @ 2017-06-18 21:53 ` Jarkko Sakkinen 2017-06-18 23:21 ` Jarkko Sakkinen 2 siblings, 0 replies; 14+ messages in thread From: Jarkko Sakkinen @ 2017-06-18 21:53 UTC (permalink / raw) To: Josh Zimmerman, Peter Huewe, Marcel Selhorst, Jason Gunthorpe, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, jmorris-gx6/JNMH7DfYtjvyW6yDsg On Fri, 2017-06-16 at 10:25 -0700, Josh Zimmerman wrote: > If a TPM2 loses power without a TPM2_Shutdown command being issued (a > "disorderly reboot"), it may lose some state that has yet to be > persisted to NVRam, and will increment the DA counter. After the DA > counter gets sufficiently large, the TPM will lock the user out. > > NOTE: This only changes behavior on TPM2 devices. Since TPM1 uses sysfs, > and sysfs relies on implicit locking on chip->ops, it is not safe to > allow this code to run in TPM1, or to add sysfs support to TPM2, until > that locking is made explicit. > > Signed-off-by: Josh Zimmerman <joshz@google.com> > Cc: stable@vger.kernel.org > > ---- > v2: > - Properly split changes between this and another commit > - Use proper locking primitive. > - Fix commenting style > v3: > - Re-fix commenting style > v4: > - Update description and tags (Reviewed-by, Cc). > v5: > - Update documentation. > v6: > - Call device and/or bus shutdown from tpm_shutdown. The change log should be after the dashes *below* right before diff stat. Since this is 1/2 I should be abe to assume that I could apply on this and not apply 2/2. Why this patch set does not have a cover letter? Anyway, I applied these to my master and if they don't break anything I'm including them to my 4.13 pull request. I left the commit messages to the form that they turn out if I just git am -3 the mbox files. Next time, do a git am sanity check if you have changelogs in your patches. /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices. [not found] ` <20170616172531.28464-1-joshz-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2017-06-16 17:25 ` [PATCH v6 2/2] Add "shutdown" to "struct class" Josh Zimmerman via tpmdd-devel 2017-06-18 21:53 ` [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices Jarkko Sakkinen @ 2017-06-18 23:21 ` Jarkko Sakkinen [not found] ` <1497828091.2552.6.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 2 siblings, 1 reply; 14+ messages in thread From: Jarkko Sakkinen @ 2017-06-18 23:21 UTC (permalink / raw) To: Josh Zimmerman, Peter Huewe, Marcel Selhorst, Jason Gunthorpe, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, jmorris-gx6/JNMH7DfYtjvyW6yDsg Hold on. On Fri, 2017-06-16 at 10:25 -0700, Josh Zimmerman wrote: > If a TPM2 loses power without a TPM2_Shutdown command being issued (a > "disorderly reboot"), it may lose some state that has yet to be > persisted to NVRam, and will increment the DA counter. After the DA > counter gets sufficiently large, the TPM will lock the user out. > > NOTE: This only changes behavior on TPM2 devices. Since TPM1 uses sysfs, > and sysfs relies on implicit locking on chip->ops, it is not safe to > allow this code to run in TPM1, or to add sysfs support to TPM2, until > that locking is made explicit. > > Signed-off-by: Josh Zimmerman <joshz@google.com> > Cc: stable@vger.kernel.org > > ---- > v2: > - Properly split changes between this and another commit > - Use proper locking primitive. > - Fix commenting style > v3: > - Re-fix commenting style > v4: > - Update description and tags (Reviewed-by, Cc). > v5: > - Update documentation. > v6: > - Call device and/or bus shutdown from tpm_shutdown. > --- > drivers/char/tpm/tpm-chip.c | 31 +++++++++++++++++++++++++++++++ > drivers/char/tpm/tpm-sysfs.c | 3 +++ > 2 files changed, 34 insertions(+) > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm- chip.c > index 9dec9f551b83..62691d266f22 100644 > --- a/drivers/char/tpm/tpm-chip.c > +++ b/drivers/char/tpm/tpm-chip.c > @@ -142,6 +142,36 @@ static void tpm_devs_release(struct device *dev) > put_device(&chip->dev); > } > > +/** > + * tpm_shutdown() - prepare the TPM device for loss of power. > + * @dev: device to which the chip is associated. > + * > + * Issues a TPM2_Shutdown command prior to loss of power, as required by the > + * TPM 2.0 spec. > + * > + * XXX: This codepath relies on the fact that sysfs is not enabled for > + * TPM2: sysfs uses an implicit lock on chip->ops, so this could race if TPM2 > + * has sysfs support enabled before TPM sysfs's implicit locking is fixed. > + */ > +static void tpm_shutdown(struct device *dev) > +{ > + struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev); > + > + if (chip->flags & TPM_CHIP_FLAG_TPM2) { > + down_write(&chip->ops_sem); > + tpm2_shutdown(chip, TPM_SU_CLEAR); > + chip->ops = NULL; > + up_write(&chip->ops_sem); > + } > + // Allow bus- and device-specific code to run. Note: since chip->ops > + // is NULL, more-specific shutdown code will not be able to issue TPM > + // commands. > + if (dev->bus->shutdown) > + dev->bus->shutdown(dev); > + else if (dev->driver && dev->driver->shutdown) > + dev->driver->shutdown(dev); WTF is this part. And why we have now duplicate code?? I'm dropping these patches for now from my master branch. I don't understand what is going on... /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <1497828091.2552.6.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices. [not found] ` <1497828091.2552.6.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2017-06-18 23:26 ` Jarkko Sakkinen [not found] ` <1497828407.2552.8.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 0 siblings, 1 reply; 14+ messages in thread From: Jarkko Sakkinen @ 2017-06-18 23:26 UTC (permalink / raw) To: Josh Zimmerman, Peter Huewe, Marcel Selhorst, Jason Gunthorpe, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, jmorris-gx6/JNMH7DfYtjvyW6yDsg On Mon, 2017-06-19 at 01:21 +0200, Jarkko Sakkinen wrote: > Hold on. > > On Fri, 2017-06-16 at 10:25 -0700, Josh Zimmerman wrote: > > If a TPM2 loses power without a TPM2_Shutdown command being issued > > (a > > "disorderly reboot"), it may lose some state that has yet to be > > persisted to NVRam, and will increment the DA counter. After the DA > > counter gets sufficiently large, the TPM will lock the user out. > > > > NOTE: This only changes behavior on TPM2 devices. Since TPM1 uses > > sysfs, > > and sysfs relies on implicit locking on chip->ops, it is not safe > > to > > allow this code to run in TPM1, or to add sysfs support to TPM2, > > until > > that locking is made explicit. > > > > Signed-off-by: Josh Zimmerman <joshz@google.com> > > Cc: stable@vger.kernel.org > > > > ---- > > v2: > > - Properly split changes between this and another commit > > - Use proper locking primitive. > > - Fix commenting style > > v3: > > - Re-fix commenting style > > v4: > > - Update description and tags (Reviewed-by, Cc). > > v5: > > - Update documentation. > > v6: > > - Call device and/or bus shutdown from tpm_shutdown. > > --- > > drivers/char/tpm/tpm-chip.c | 31 +++++++++++++++++++++++++++++++ > > drivers/char/tpm/tpm-sysfs.c | 3 +++ > > 2 files changed, 34 insertions(+) > > > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm- > > chip.c > > index 9dec9f551b83..62691d266f22 100644 > > --- a/drivers/char/tpm/tpm-chip.c > > +++ b/drivers/char/tpm/tpm-chip.c > > @@ -142,6 +142,36 @@ static void tpm_devs_release(struct device > > *dev) > > put_device(&chip->dev); > > } > > > > +/** > > + * tpm_shutdown() - prepare the TPM device for loss of power. > > + * @dev: device to which the chip is associated. > > + * > > + * Issues a TPM2_Shutdown command prior to loss of power, as > > required by the > > + * TPM 2.0 spec. > > + * > > + * XXX: This codepath relies on the fact that sysfs is not enabled > > for > > + * TPM2: sysfs uses an implicit lock on chip->ops, so this could > > race if TPM2 > > + * has sysfs support enabled before TPM sysfs's implicit locking > > is > > fixed. > > + */ > > +static void tpm_shutdown(struct device *dev) > > +{ > > + struct tpm_chip *chip = container_of(dev, struct tpm_chip, > > dev); > > + > > + if (chip->flags & TPM_CHIP_FLAG_TPM2) { > > + down_write(&chip->ops_sem); > > + tpm2_shutdown(chip, TPM_SU_CLEAR); > > + chip->ops = NULL; > > + up_write(&chip->ops_sem); > > + } > > + // Allow bus- and device-specific code to run. Note: since > > chip->ops > > + // is NULL, more-specific shutdown code will not be able > > to > > issue TPM > > + // commands. > > + if (dev->bus->shutdown) > > + dev->bus->shutdown(dev); > > + else if (dev->driver && dev->driver->shutdown) > > + dev->driver->shutdown(dev); > > WTF is this part. > > And why we have now duplicate code?? > > I'm dropping these patches for now from my master branch. I don't > understand what is going on... > > /Jarkko This series is too broken to be merged :-( I expect cover letter for the next version... Feels weird that you have to call framework functions like that in the driver. You must have brilliant reason to do so and that should be very well documented in the code. This is terrible... /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <1497828407.2552.8.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices. [not found] ` <1497828407.2552.8.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2017-06-19 15:51 ` Jason Gunthorpe [not found] ` <20170619155122.GA10188-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 0 siblings, 1 reply; 14+ messages in thread From: Jason Gunthorpe @ 2017-06-19 15:51 UTC (permalink / raw) To: Jarkko Sakkinen Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, jmorris-gx6/JNMH7DfYtjvyW6yDsg On Mon, Jun 19, 2017 at 01:26:47AM +0200, Jarkko Sakkinen wrote: > Feels weird that you have to call framework functions like that in the > driver. You must have brilliant reason to do so and that should be very > well documented in the code. This is terrible... This was all discussed on the list. It the way these callbacks work, the higher levels in the callback stack call the lower levels, this allows each level the place the next level's callback properly, eg do things before/after as necessary. Jason ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <20170619155122.GA10188-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices. [not found] ` <20170619155122.GA10188-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2017-06-19 22:12 ` Jarkko Sakkinen [not found] ` <20170619221220.kfk5dldducxvou5b-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 0 siblings, 1 reply; 14+ messages in thread From: Jarkko Sakkinen @ 2017-06-19 22:12 UTC (permalink / raw) To: Jason Gunthorpe Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, jmorris-gx6/JNMH7DfYtjvyW6yDsg On Mon, Jun 19, 2017 at 09:51:22AM -0600, Jason Gunthorpe wrote: > On Mon, Jun 19, 2017 at 01:26:47AM +0200, Jarkko Sakkinen wrote: > > > Feels weird that you have to call framework functions like that in the > > driver. You must have brilliant reason to do so and that should be very > > well documented in the code. This is terrible... > > This was all discussed on the list. It the way these callbacks work, > the higher levels in the callback stack call the lower levels, this > allows each level the place the next level's callback properly, eg do > things before/after as necessary. > > Jason I tried to look up for discussion from the patchwork. These had appeared in v6. I guess I have to backtrack the discussions from my maidir because I honestly don't understand why class shutdown would have to call bus callback explicitly. There's nothing in the commit message about this nor is there any comment in the code. This must be fairly recent development that I've missed? /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <20170619221220.kfk5dldducxvou5b-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices. [not found] ` <20170619221220.kfk5dldducxvou5b-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2017-06-19 22:21 ` Jarkko Sakkinen [not found] ` <20170619222122.55m5goanayw3gkeo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 0 siblings, 1 reply; 14+ messages in thread From: Jarkko Sakkinen @ 2017-06-19 22:21 UTC (permalink / raw) To: Jason Gunthorpe Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, jmorris-gx6/JNMH7DfYtjvyW6yDsg On Tue, Jun 20, 2017 at 12:12:20AM +0200, Jarkko Sakkinen wrote: > On Mon, Jun 19, 2017 at 09:51:22AM -0600, Jason Gunthorpe wrote: > > On Mon, Jun 19, 2017 at 01:26:47AM +0200, Jarkko Sakkinen wrote: > > > > > Feels weird that you have to call framework functions like that in the > > > driver. You must have brilliant reason to do so and that should be very > > > well documented in the code. This is terrible... > > > > This was all discussed on the list. It the way these callbacks work, > > the higher levels in the callback stack call the lower levels, this > > allows each level the place the next level's callback properly, eg do > > things before/after as necessary. > > > > Jason > > I tried to look up for discussion from the patchwork. These had appeared > in v6. I guess I have to backtrack the discussions from my maidir > because I honestly don't understand why class shutdown would have to > call bus callback explicitly. There's nothing in the commit message > about this nor is there any comment in the code. > > This must be fairly recent development that I've missed? > > /Jarkko Found it: "Looking at this closer, now you definately have to change the TPM patch to call through to the other shutdown methods. We can say current TPM drivers have no driver->shutdown, but we cannot be sure about the bus->shutdown, so may as well call both from tpm's class->shutdown. I would say this should be done after the tpm2_shutdown completes as lower level shutdowns could remove register access. Jason" And makes sense. This patch is a NAK for two reasons: 1. No comment explaining this. 2. Patches are broken and they are in wrong order and cover letter is missing /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <20170619222122.55m5goanayw3gkeo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices. [not found] ` <20170619222122.55m5goanayw3gkeo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2017-06-19 22:38 ` Jarkko Sakkinen [not found] ` <20170619223840.a66extf3r5ylmpnb-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 0 siblings, 1 reply; 14+ messages in thread From: Jarkko Sakkinen @ 2017-06-19 22:38 UTC (permalink / raw) To: Jason Gunthorpe Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, jmorris-gx6/JNMH7DfYtjvyW6yDsg On Tue, Jun 20, 2017 at 12:21:22AM +0200, Jarkko Sakkinen wrote: > On Tue, Jun 20, 2017 at 12:12:20AM +0200, Jarkko Sakkinen wrote: > > On Mon, Jun 19, 2017 at 09:51:22AM -0600, Jason Gunthorpe wrote: > > > On Mon, Jun 19, 2017 at 01:26:47AM +0200, Jarkko Sakkinen wrote: > > > > > > > Feels weird that you have to call framework functions like that in the > > > > driver. You must have brilliant reason to do so and that should be very > > > > well documented in the code. This is terrible... > > > > > > This was all discussed on the list. It the way these callbacks work, > > > the higher levels in the callback stack call the lower levels, this > > > allows each level the place the next level's callback properly, eg do > > > things before/after as necessary. > > > > > > Jason > > > > I tried to look up for discussion from the patchwork. These had appeared > > in v6. I guess I have to backtrack the discussions from my maidir > > because I honestly don't understand why class shutdown would have to > > call bus callback explicitly. There's nothing in the commit message > > about this nor is there any comment in the code. > > > > This must be fairly recent development that I've missed? > > > > /Jarkko > > Found it: > > "Looking at this closer, now you definately have to change the TPM > patch to call through to the other shutdown methods. We can say > current TPM drivers have no driver->shutdown, but we cannot be sure > about the bus->shutdown, so may as well call both from tpm's > class->shutdown. > > I would say this should be done after the tpm2_shutdown completes as > lower level shutdowns could remove register access. > > Jason" > > And makes sense. > > This patch is a NAK for two reasons: > > 1. No comment explaining this. > 2. Patches are broken and they are in wrong order and cover letter is > missing > > /Jarkko I *tried* to apply them myself after sending this in order to be helpful but they have compilation errors: drivers/char/tpm/tpm-chip.c: In function ‘tpm_shutdown’: drivers/char/tpm/tpm-chip.c:162:23: error: ‘TPM_SU_CLEAR’ undeclared (first use in this function) tpm2_shutdown(chip, TPM_SU_CLEAR); ^~~~~~~~~~~~ drivers/char/tpm/tpm-chip.c:162:23: note: each undeclared identifier is reported only once for each function it appears in drivers/char/tpm/tpm-chip.c: In function ‘tpm_chip_alloc’: drivers/char/tpm/tpm-chip.c:214:17: error: ‘chip->dev.class’ is a pointer; did you mean to use ‘->’? chip->dev.class.shutdown = tpm_shutdown; /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <20170619223840.a66extf3r5ylmpnb-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices. [not found] ` <20170619223840.a66extf3r5ylmpnb-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2017-06-19 22:44 ` Josh Zimmerman via tpmdd-devel [not found] ` <CAHSjozDy0dsGPkyXzVsDabjtbM4-QkzoE2fH1gDONXETyA19RA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2017-06-20 8:44 ` Jarkko Sakkinen 1 sibling, 1 reply; 14+ messages in thread From: Josh Zimmerman via tpmdd-devel @ 2017-06-19 22:44 UTC (permalink / raw) To: Jarkko Sakkinen Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, jmorris-gx6/JNMH7DfYtjvyW6yDsg Sorry about the mess. I'm a bit swamped today, but I'll work on cleaning up the patch formatting & commit message and fix the compilation problem later today or tomorrow. (It did build on my checkout of the tpmdd branch... guess I didn't pull in some important change.) Josh On Mon, Jun 19, 2017 at 3:38 PM, Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > On Tue, Jun 20, 2017 at 12:21:22AM +0200, Jarkko Sakkinen wrote: >> On Tue, Jun 20, 2017 at 12:12:20AM +0200, Jarkko Sakkinen wrote: >> > On Mon, Jun 19, 2017 at 09:51:22AM -0600, Jason Gunthorpe wrote: >> > > On Mon, Jun 19, 2017 at 01:26:47AM +0200, Jarkko Sakkinen wrote: >> > > >> > > > Feels weird that you have to call framework functions like that in the >> > > > driver. You must have brilliant reason to do so and that should be very >> > > > well documented in the code. This is terrible... >> > > >> > > This was all discussed on the list. It the way these callbacks work, >> > > the higher levels in the callback stack call the lower levels, this >> > > allows each level the place the next level's callback properly, eg do >> > > things before/after as necessary. >> > > >> > > Jason >> > >> > I tried to look up for discussion from the patchwork. These had appeared >> > in v6. I guess I have to backtrack the discussions from my maidir >> > because I honestly don't understand why class shutdown would have to >> > call bus callback explicitly. There's nothing in the commit message >> > about this nor is there any comment in the code. >> > >> > This must be fairly recent development that I've missed? >> > >> > /Jarkko >> >> Found it: >> >> "Looking at this closer, now you definately have to change the TPM >> patch to call through to the other shutdown methods. We can say >> current TPM drivers have no driver->shutdown, but we cannot be sure >> about the bus->shutdown, so may as well call both from tpm's >> class->shutdown. >> >> I would say this should be done after the tpm2_shutdown completes as >> lower level shutdowns could remove register access. >> >> Jason" >> >> And makes sense. >> >> This patch is a NAK for two reasons: >> >> 1. No comment explaining this. >> 2. Patches are broken and they are in wrong order and cover letter is >> missing >> >> /Jarkko > > I *tried* to apply them myself after sending this in order to be helpful > but they have compilation errors: > > drivers/char/tpm/tpm-chip.c: In function ‘tpm_shutdown’: > drivers/char/tpm/tpm-chip.c:162:23: error: ‘TPM_SU_CLEAR’ undeclared (first use in this function) > tpm2_shutdown(chip, TPM_SU_CLEAR); > ^~~~~~~~~~~~ > drivers/char/tpm/tpm-chip.c:162:23: note: each undeclared identifier is reported only once for each function it appears in > drivers/char/tpm/tpm-chip.c: In function ‘tpm_chip_alloc’: > drivers/char/tpm/tpm-chip.c:214:17: error: ‘chip->dev.class’ is a pointer; did you mean to use ‘->’? > chip->dev.class.shutdown = tpm_shutdown; > > /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <CAHSjozDy0dsGPkyXzVsDabjtbM4-QkzoE2fH1gDONXETyA19RA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices. [not found] ` <CAHSjozDy0dsGPkyXzVsDabjtbM4-QkzoE2fH1gDONXETyA19RA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2017-06-20 15:05 ` Jarkko Sakkinen [not found] ` <20170620150541.x5b7vkxyb3w55gjq-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 0 siblings, 1 reply; 14+ messages in thread From: Jarkko Sakkinen @ 2017-06-20 15:05 UTC (permalink / raw) To: Josh Zimmerman Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, jmorris-gx6/JNMH7DfYtjvyW6yDsg No worries. Just fix them and I will include to my next pull request! /Jarkko On Mon, Jun 19, 2017 at 03:44:55PM -0700, Josh Zimmerman wrote: > Sorry about the mess. I'm a bit swamped today, but I'll work on > cleaning up the patch formatting & commit message and fix the > compilation problem later today or tomorrow. (It did build on my > checkout of the tpmdd branch... guess I didn't pull in some important > change.) > Josh > > > On Mon, Jun 19, 2017 at 3:38 PM, Jarkko Sakkinen > <jarkko.sakkinen@linux.intel.com> wrote: > > On Tue, Jun 20, 2017 at 12:21:22AM +0200, Jarkko Sakkinen wrote: > >> On Tue, Jun 20, 2017 at 12:12:20AM +0200, Jarkko Sakkinen wrote: > >> > On Mon, Jun 19, 2017 at 09:51:22AM -0600, Jason Gunthorpe wrote: > >> > > On Mon, Jun 19, 2017 at 01:26:47AM +0200, Jarkko Sakkinen wrote: > >> > > > >> > > > Feels weird that you have to call framework functions like that in the > >> > > > driver. You must have brilliant reason to do so and that should be very > >> > > > well documented in the code. This is terrible... > >> > > > >> > > This was all discussed on the list. It the way these callbacks work, > >> > > the higher levels in the callback stack call the lower levels, this > >> > > allows each level the place the next level's callback properly, eg do > >> > > things before/after as necessary. > >> > > > >> > > Jason > >> > > >> > I tried to look up for discussion from the patchwork. These had appeared > >> > in v6. I guess I have to backtrack the discussions from my maidir > >> > because I honestly don't understand why class shutdown would have to > >> > call bus callback explicitly. There's nothing in the commit message > >> > about this nor is there any comment in the code. > >> > > >> > This must be fairly recent development that I've missed? > >> > > >> > /Jarkko > >> > >> Found it: > >> > >> "Looking at this closer, now you definately have to change the TPM > >> patch to call through to the other shutdown methods. We can say > >> current TPM drivers have no driver->shutdown, but we cannot be sure > >> about the bus->shutdown, so may as well call both from tpm's > >> class->shutdown. > >> > >> I would say this should be done after the tpm2_shutdown completes as > >> lower level shutdowns could remove register access. > >> > >> Jason" > >> > >> And makes sense. > >> > >> This patch is a NAK for two reasons: > >> > >> 1. No comment explaining this. > >> 2. Patches are broken and they are in wrong order and cover letter is > >> missing > >> > >> /Jarkko > > > > I *tried* to apply them myself after sending this in order to be helpful > > but they have compilation errors: > > > > drivers/char/tpm/tpm-chip.c: In function ‘tpm_shutdown’: > > drivers/char/tpm/tpm-chip.c:162:23: error: ‘TPM_SU_CLEAR’ undeclared (first use in this function) > > tpm2_shutdown(chip, TPM_SU_CLEAR); > > ^~~~~~~~~~~~ > > drivers/char/tpm/tpm-chip.c:162:23: note: each undeclared identifier is reported only once for each function it appears in > > drivers/char/tpm/tpm-chip.c: In function ‘tpm_chip_alloc’: > > drivers/char/tpm/tpm-chip.c:214:17: error: ‘chip->dev.class’ is a pointer; did you mean to use ‘->’? > > chip->dev.class.shutdown = tpm_shutdown; > > > > /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <20170620150541.x5b7vkxyb3w55gjq-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices. [not found] ` <20170620150541.x5b7vkxyb3w55gjq-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2017-06-21 7:37 ` Jarkko Sakkinen 0 siblings, 0 replies; 14+ messages in thread From: Jarkko Sakkinen @ 2017-06-21 7:37 UTC (permalink / raw) To: Josh Zimmerman Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, jmorris-gx6/JNMH7DfYtjvyW6yDsg While you still have to work on this, could you name the function as tpm_class_shutdown() to better claarify its purpose? Otherwise, things start to look a bit confusing. Thanks. /Jarkko On Tue, Jun 20, 2017 at 05:05:41PM +0200, Jarkko Sakkinen wrote: > No worries. Just fix them and I will include to my next pull request! > > /Jarkko > > On Mon, Jun 19, 2017 at 03:44:55PM -0700, Josh Zimmerman wrote: > > Sorry about the mess. I'm a bit swamped today, but I'll work on > > cleaning up the patch formatting & commit message and fix the > > compilation problem later today or tomorrow. (It did build on my > > checkout of the tpmdd branch... guess I didn't pull in some important > > change.) > > Josh > > > > > > On Mon, Jun 19, 2017 at 3:38 PM, Jarkko Sakkinen > > <jarkko.sakkinen@linux.intel.com> wrote: > > > On Tue, Jun 20, 2017 at 12:21:22AM +0200, Jarkko Sakkinen wrote: > > >> On Tue, Jun 20, 2017 at 12:12:20AM +0200, Jarkko Sakkinen wrote: > > >> > On Mon, Jun 19, 2017 at 09:51:22AM -0600, Jason Gunthorpe wrote: > > >> > > On Mon, Jun 19, 2017 at 01:26:47AM +0200, Jarkko Sakkinen wrote: > > >> > > > > >> > > > Feels weird that you have to call framework functions like that in the > > >> > > > driver. You must have brilliant reason to do so and that should be very > > >> > > > well documented in the code. This is terrible... > > >> > > > > >> > > This was all discussed on the list. It the way these callbacks work, > > >> > > the higher levels in the callback stack call the lower levels, this > > >> > > allows each level the place the next level's callback properly, eg do > > >> > > things before/after as necessary. > > >> > > > > >> > > Jason > > >> > > > >> > I tried to look up for discussion from the patchwork. These had appeared > > >> > in v6. I guess I have to backtrack the discussions from my maidir > > >> > because I honestly don't understand why class shutdown would have to > > >> > call bus callback explicitly. There's nothing in the commit message > > >> > about this nor is there any comment in the code. > > >> > > > >> > This must be fairly recent development that I've missed? > > >> > > > >> > /Jarkko > > >> > > >> Found it: > > >> > > >> "Looking at this closer, now you definately have to change the TPM > > >> patch to call through to the other shutdown methods. We can say > > >> current TPM drivers have no driver->shutdown, but we cannot be sure > > >> about the bus->shutdown, so may as well call both from tpm's > > >> class->shutdown. > > >> > > >> I would say this should be done after the tpm2_shutdown completes as > > >> lower level shutdowns could remove register access. > > >> > > >> Jason" > > >> > > >> And makes sense. > > >> > > >> This patch is a NAK for two reasons: > > >> > > >> 1. No comment explaining this. > > >> 2. Patches are broken and they are in wrong order and cover letter is > > >> missing > > >> > > >> /Jarkko > > > > > > I *tried* to apply them myself after sending this in order to be helpful > > > but they have compilation errors: > > > > > > drivers/char/tpm/tpm-chip.c: In function ‘tpm_shutdown’: > > > drivers/char/tpm/tpm-chip.c:162:23: error: ‘TPM_SU_CLEAR’ undeclared (first use in this function) > > > tpm2_shutdown(chip, TPM_SU_CLEAR); > > > ^~~~~~~~~~~~ > > > drivers/char/tpm/tpm-chip.c:162:23: note: each undeclared identifier is reported only once for each function it appears in > > > drivers/char/tpm/tpm-chip.c: In function ‘tpm_chip_alloc’: > > > drivers/char/tpm/tpm-chip.c:214:17: error: ‘chip->dev.class’ is a pointer; did you mean to use ‘->’? > > > chip->dev.class.shutdown = tpm_shutdown; > > > > > > /Jarkko > > ------------------------------------------------------------------------------ > Check out the vibrant tech community on one of the world's most > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > _______________________________________________ > tpmdd-devel mailing list > tpmdd-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices. [not found] ` <20170619223840.a66extf3r5ylmpnb-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 2017-06-19 22:44 ` Josh Zimmerman via tpmdd-devel @ 2017-06-20 8:44 ` Jarkko Sakkinen 1 sibling, 0 replies; 14+ messages in thread From: Jarkko Sakkinen @ 2017-06-20 8:44 UTC (permalink / raw) To: Jason Gunthorpe Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, jmorris-gx6/JNMH7DfYtjvyW6yDsg On Tue, Jun 20, 2017 at 12:38:40AM +0200, Jarkko Sakkinen wrote: > On Tue, Jun 20, 2017 at 12:21:22AM +0200, Jarkko Sakkinen wrote: > > On Tue, Jun 20, 2017 at 12:12:20AM +0200, Jarkko Sakkinen wrote: > > > On Mon, Jun 19, 2017 at 09:51:22AM -0600, Jason Gunthorpe wrote: > > > > On Mon, Jun 19, 2017 at 01:26:47AM +0200, Jarkko Sakkinen wrote: > > > > > > > > > Feels weird that you have to call framework functions like that in the > > > > > driver. You must have brilliant reason to do so and that should be very > > > > > well documented in the code. This is terrible... > > > > > > > > This was all discussed on the list. It the way these callbacks work, > > > > the higher levels in the callback stack call the lower levels, this > > > > allows each level the place the next level's callback properly, eg do > > > > things before/after as necessary. > > > > > > > > Jason > > > > > > I tried to look up for discussion from the patchwork. These had appeared > > > in v6. I guess I have to backtrack the discussions from my maidir > > > because I honestly don't understand why class shutdown would have to > > > call bus callback explicitly. There's nothing in the commit message > > > about this nor is there any comment in the code. > > > > > > This must be fairly recent development that I've missed? > > > > > > /Jarkko > > > > Found it: > > > > "Looking at this closer, now you definately have to change the TPM > > patch to call through to the other shutdown methods. We can say > > current TPM drivers have no driver->shutdown, but we cannot be sure > > about the bus->shutdown, so may as well call both from tpm's > > class->shutdown. > > > > I would say this should be done after the tpm2_shutdown completes as > > lower level shutdowns could remove register access. > > > > Jason" > > > > And makes sense. > > > > This patch is a NAK for two reasons: > > > > 1. No comment explaining this. > > 2. Patches are broken and they are in wrong order and cover letter is > > missing > > > > /Jarkko > > I *tried* to apply them myself after sending this in order to be helpful > but they have compilation errors: > > drivers/char/tpm/tpm-chip.c: In function ‘tpm_shutdown’: > drivers/char/tpm/tpm-chip.c:162:23: error: ‘TPM_SU_CLEAR’ undeclared (first use in this function) > tpm2_shutdown(chip, TPM_SU_CLEAR); > ^~~~~~~~~~~~ > drivers/char/tpm/tpm-chip.c:162:23: note: each undeclared identifier is reported only once for each function it appears in > drivers/char/tpm/tpm-chip.c: In function ‘tpm_chip_alloc’: > drivers/char/tpm/tpm-chip.c:214:17: error: ‘chip->dev.class’ is a pointer; did you mean to use ‘->’? > chip->dev.class.shutdown = tpm_shutdown; > > /Jarkko There's one more thing that I noticed. Before I can Cc these to the stable these commits require the Fixes tag. /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices. @ 2017-06-02 21:55 Josh Zimmerman 0 siblings, 0 replies; 14+ messages in thread From: Josh Zimmerman @ 2017-06-02 21:55 UTC (permalink / raw) To: Peter Huewe, Marcel Selhorst, Jarkko Sakkinen, Jason Gunthorpe, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, jmorris-gx6/JNMH7DfYtjvyW6yDsg If a TPM2 loses power without a TPM2_Shutdown command being issued (a "disorderly reboot"), it may lose some state that has yet to be persisted to NVRam, and will increment the DA counter. After the DA counter gets sufficiently large, the TPM will lock the user out. NOTE: This only changes behavior on TPM2 devices. Since TPM1 uses sysfs, and sysfs relies on implicit locking on chip->ops, it is not safe to allow this code to run in TPM1, or to add sysfs support to TPM2, until that locking is made explicit. Signed-off-by: Josh Zimmerman <joshz-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org ---- v2: - Properly split changes between this and another commit - Use proper locking primitive. - Fix commenting style v3: - Re-fix commenting style v4: - Update description and tags (Reviewed-by, Cc). v5: - Update documentation. v6: - Call device or bus shutdown from tpm_shutdown. --- drivers/char/tpm/tpm-chip.c | 31 +++++++++++++++++++++++++++++++ drivers/char/tpm/tpm-sysfs.c | 3 +++ 2 files changed, 34 insertions(+) diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index 9dec9f551b83..62691d266f22 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -142,6 +142,36 @@ static void tpm_devs_release(struct device *dev) put_device(&chip->dev); } +/** + * tpm_shutdown() - prepare the TPM device for loss of power. + * @dev: device to which the chip is associated. + * + * Issues a TPM2_Shutdown command prior to loss of power, as required by the + * TPM 2.0 spec. + * + * XXX: This codepath relies on the fact that sysfs is not enabled for + * TPM2: sysfs uses an implicit lock on chip->ops, so this could race if TPM2 + * has sysfs support enabled before TPM sysfs's implicit locking is fixed. + */ +static void tpm_shutdown(struct device *dev) +{ + struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev); + + if (chip->flags & TPM_CHIP_FLAG_TPM2) { + down_write(&chip->ops_sem); + tpm2_shutdown(chip, TPM_SU_CLEAR); + chip->ops = NULL; + up_write(&chip->ops_sem); + } + // Allow bus- and device-specific code to run. Note: since chip->ops + // is NULL, more-specific shutdown code will not be able to issue TPM + // commands. + if (dev->bus->shutdown) + dev->bus->shutdown(dev); + else if (dev->driver && dev->driver->shutdown) + dev->driver->shutdown(dev); +} + /** * tpm_chip_alloc() - allocate a new struct tpm_chip instance * @pdev: device to which the chip is associated @@ -181,6 +211,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev, device_initialize(&chip->devs); chip->dev.class = tpm_class; + chip->dev.class.shutdown = tpm_shutdown; chip->dev.release = tpm_dev_release; chip->dev.parent = pdev; chip->dev.groups = chip->groups; diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c index 55405dbe43fa..59bd0b7b5959 100644 --- a/drivers/char/tpm/tpm-sysfs.c +++ b/drivers/char/tpm/tpm-sysfs.c @@ -294,6 +294,9 @@ static const struct attribute_group tpm_dev_group = { void tpm_sysfs_add_device(struct tpm_chip *chip) { + /* XXX: If you wish to remove this restriction, you must first update + * tpm_sysfs to explicitly lock chip->ops. + */ if (chip->flags & TPM_CHIP_FLAG_TPM2) return; -- 2.13.0.506.g27d5fe0cd-goog ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-06-21 7:37 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-16 17:25 [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices Josh Zimmerman via tpmdd-devel [not found] ` <20170616172531.28464-1-joshz-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2017-06-16 17:25 ` [PATCH v6 2/2] Add "shutdown" to "struct class" Josh Zimmerman via tpmdd-devel 2017-06-18 21:53 ` [PATCH v6 1/2] tpm: Issue a TPM2_Shutdown for TPM2 devices Jarkko Sakkinen 2017-06-18 23:21 ` Jarkko Sakkinen [not found] ` <1497828091.2552.6.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 2017-06-18 23:26 ` Jarkko Sakkinen [not found] ` <1497828407.2552.8.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 2017-06-19 15:51 ` Jason Gunthorpe [not found] ` <20170619155122.GA10188-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2017-06-19 22:12 ` Jarkko Sakkinen [not found] ` <20170619221220.kfk5dldducxvou5b-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 2017-06-19 22:21 ` Jarkko Sakkinen [not found] ` <20170619222122.55m5goanayw3gkeo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 2017-06-19 22:38 ` Jarkko Sakkinen [not found] ` <20170619223840.a66extf3r5ylmpnb-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 2017-06-19 22:44 ` Josh Zimmerman via tpmdd-devel [not found] ` <CAHSjozDy0dsGPkyXzVsDabjtbM4-QkzoE2fH1gDONXETyA19RA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2017-06-20 15:05 ` Jarkko Sakkinen [not found] ` <20170620150541.x5b7vkxyb3w55gjq-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 2017-06-21 7:37 ` Jarkko Sakkinen 2017-06-20 8:44 ` Jarkko Sakkinen -- strict thread matches above, loose matches on Subject: below -- 2017-06-02 21:55 Josh Zimmerman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).