[PATCH v3] Add "shutdown" to "struct class".

[PATCH v3] Add "shutdown" to "struct class".

[Josh Zimmerman]

The TPM class has some common shutdown code that must be executed for
all drivers. This adds some needed functionality for that.

Usage example: 'tpm: Issue a TPM2_Shutdown for TPM2 devices.'
(see https://patchwork.kernel.org/patch/9724919/ for v2).

Signed-off-by: Josh Zimmerman <joshz@google.com>

-----
v2: Add Signed-off-by.
v3: Remove logically separate change.
---
 drivers/base/core.c    | 5 +++++
 include/linux/device.h | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index bbecaf9293be..5c1648875e94 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2667,6 +2667,11 @@ void device_shutdown(void)
 		pm_runtime_get_noresume(dev);
 		pm_runtime_barrier(dev);
 
+		if (dev->class && dev->class->shutdown) {
+			if (initcall_debug)
+				dev_info(dev, "shutdown\n");
+			dev->class->shutdown(dev);
+		}
 		if (dev->bus && dev->bus->shutdown) {
 			if (initcall_debug)
 				dev_info(dev, "shutdown\n");
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.0.rc2.291.g57267f2277-goog

Re: [PATCH v3] Add "shutdown" to "struct class".

[Josh Zimmerman]

(Continuing thread from patch v1)
> > On Sat, May 13, 2017 at 12:43:11PM +0000, Winkler, Tomas wrote:
> > > > The TPM class has some common shutdown code that must be executed
> > > > for all drivers. This adds some needed functionality for that
> > >
> > > The issue with this is, that on some platforms the only storage can be
> > > eMMC and TPM is using it,. It has to be ensured that the storage
> > > device won't go down before TPM2_shutdown is called.  And there is no
> > > direct device hierarchy to ensure an orderly shutdown.
> >
> > Something will have to use the new device links stuff to define that
> > dependency, but that seems unrelated to this patch?
>
>
> Yep, it's not directly related to this specific patch, this is more relevant particularly to TPM2_shutdown.

Jason, do you want me to do that in my patch on the tpmdd-devel list?
If so, mind giving me a documentation pointer or two? I'm not familiar
with this area.

Thanks,
Josh

Re: [PATCH v3] Add "shutdown" to "struct class".

[Jason Gunthorpe]

On Mon, May 15, 2017 at 10:39:08AM -0700, Josh Zimmerman wrote:
> (Continuing thread from patch v1)
> > > On Sat, May 13, 2017 at 12:43:11PM +0000, Winkler, Tomas wrote:
> > > > > The TPM class has some common shutdown code that must be executed
> > > > > for all drivers. This adds some needed functionality for that
> > > >
> > > > The issue with this is, that on some platforms the only storage can be
> > > > eMMC and TPM is using it,. It has to be ensured that the storage
> > > > device won't go down before TPM2_shutdown is called.  And there is no
> > > > direct device hierarchy to ensure an orderly shutdown.
> > >
> > > Something will have to use the new device links stuff to define that
> > > dependency, but that seems unrelated to this patch?
> >
> >
> > Yep, it's not directly related to this specific patch, this is more relevant particularly to TPM2_shutdown.
> 
> Jason, do you want me to do that in my patch on the tpmdd-devel list?
> If so, mind giving me a documentation pointer or two? I'm not familiar
> with this area.

No.. Ordering power management events is something someone with
knowledge of the specific emmc platform is going to have to tackle..
It isn't really a core problem, platform specific code will have to
setup the needed device links to order power management properly..

Jason

RE: [PATCH v3] Add "shutdown" to "struct class".

[Winkler, Tomas]

> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgunthorpe@obsidianresearch.com]
> Sent: Monday, May 15, 2017 20:46
> To: Josh Zimmerman <joshz@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; linux-
> kernel@vger.kernel.org; Winkler, Tomas <tomas.winkler@intel.com>; Jarkko
> Sakkinen <jarkko.sakkinen@linux.intel.com>; tpmdd-
> devel@lists.sourceforge.net
> Subject: Re: [PATCH v3] Add "shutdown" to "struct class".
> 
> On Mon, May 15, 2017 at 10:39:08AM -0700, Josh Zimmerman wrote:
> > (Continuing thread from patch v1)
> > > > On Sat, May 13, 2017 at 12:43:11PM +0000, Winkler, Tomas wrote:
> > > > > > The TPM class has some common shutdown code that must be
> > > > > > executed for all drivers. This adds some needed functionality
> > > > > > for that
> > > > >
> > > > > The issue with this is, that on some platforms the only storage
> > > > > can be eMMC and TPM is using it,. It has to be ensured that the
> > > > > storage device won't go down before TPM2_shutdown is called.
> > > > > And there is no direct device hierarchy to ensure an orderly
> shutdown.
> > > >
> > > > Something will have to use the new device links stuff to define
> > > > that dependency, but that seems unrelated to this patch?
> > >
> > >
> > > Yep, it's not directly related to this specific patch, this is more relevant
> particularly to TPM2_shutdown.
> >
> > Jason, do you want me to do that in my patch on the tpmdd-devel list?
> > If so, mind giving me a documentation pointer or two? I'm not familiar
> > with this area.
> 
> No.. Ordering power management events is something someone with
> knowledge of the specific emmc platform is going to have to tackle..
> It isn't really a core problem, platform specific code will have to setup the
> needed device links to order power management properly..

eMMC is just an example, that can be other storage device (ufs, nvme) and some type of abstraction of underlying storage
dependency would be required. I just wanted to put it on the table, not sure it has to be solved in this round.
Tomas




> 
> Jason

Re: [PATCH v3] Add "shutdown" to "struct class".

[Josh Zimmerman]

Are there any more changes any of you would like to see in this patch?

Thanks!
Josh


On Mon, May 15, 2017 at 1:49 PM, Winkler, Tomas <tomas.winkler@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: Jason Gunthorpe [mailto:jgunthorpe@obsidianresearch.com]
>> Sent: Monday, May 15, 2017 20:46
>> To: Josh Zimmerman <joshz@google.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; linux-
>> kernel@vger.kernel.org; Winkler, Tomas <tomas.winkler@intel.com>; Jarkko
>> Sakkinen <jarkko.sakkinen@linux.intel.com>; tpmdd-
>> devel@lists.sourceforge.net
>> Subject: Re: [PATCH v3] Add "shutdown" to "struct class".
>>
>> On Mon, May 15, 2017 at 10:39:08AM -0700, Josh Zimmerman wrote:
>> > (Continuing thread from patch v1)
>> > > > On Sat, May 13, 2017 at 12:43:11PM +0000, Winkler, Tomas wrote:
>> > > > > > The TPM class has some common shutdown code that must be
>> > > > > > executed for all drivers. This adds some needed functionality
>> > > > > > for that
>> > > > >
>> > > > > The issue with this is, that on some platforms the only storage
>> > > > > can be eMMC and TPM is using it,. It has to be ensured that the
>> > > > > storage device won't go down before TPM2_shutdown is called.
>> > > > > And there is no direct device hierarchy to ensure an orderly
>> shutdown.
>> > > >
>> > > > Something will have to use the new device links stuff to define
>> > > > that dependency, but that seems unrelated to this patch?
>> > >
>> > >
>> > > Yep, it's not directly related to this specific patch, this is more relevant
>> particularly to TPM2_shutdown.
>> >
>> > Jason, do you want me to do that in my patch on the tpmdd-devel list?
>> > If so, mind giving me a documentation pointer or two? I'm not familiar
>> > with this area.
>>
>> No.. Ordering power management events is something someone with
>> knowledge of the specific emmc platform is going to have to tackle..
>> It isn't really a core problem, platform specific code will have to setup the
>> needed device links to order power management properly..
>
> eMMC is just an example, that can be other storage device (ufs, nvme) and some type of abstraction of underlying storage
> dependency would be required. I just wanted to put it on the table, not sure it has to be solved in this round.
> Tomas
>
>
>
>
>>
>> Jason

Re: [PATCH v3] Add "shutdown" to "struct class".

[Greg Kroah-Hartman]

On Mon, May 15, 2017 at 10:34:38AM -0700, Josh Zimmerman wrote:
> The TPM class has some common shutdown code that must be executed for
> all drivers. This adds some needed functionality for that.
> 
> Usage example: 'tpm: Issue a TPM2_Shutdown for TPM2 devices.'
> (see https://patchwork.kernel.org/patch/9724919/ for v2).
> 
> Signed-off-by: Josh Zimmerman <joshz@google.com>

Given that the tpm code is going to need this, I recommend someone take
it through that tree:

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Otherwise, if you want me to take it, I can, but I doubt you want it in
my driver-core tree as that will not get merged to Linus until 4.13-rc1.

thanks,

greg k-h

Re: [PATCH v3] Add "shutdown" to "struct class".

[Josh Zimmerman]

Thanks, Greg.

Greg, Jarkko: Do either of you you have any objections to me
backporting these changes to 4.4 and 4.9? I'd like to make sure that
at least the couple most recent LTS kernels have this patch. (I don't
care so much about 4.1 as it'll be EOL'd this September, according to
https://www.kernel.org/category/releases.html, but I can backport it
to there as well if desired.)

Thanks,

Josh

On Thu, May 25, 2017 at 6:09 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, May 15, 2017 at 10:34:38AM -0700, Josh Zimmerman wrote:
> > The TPM class has some common shutdown code that must be executed for
> > all drivers. This adds some needed functionality for that.
> >
> > Usage example: 'tpm: Issue a TPM2_Shutdown for TPM2 devices.'
> > (see https://patchwork.kernel.org/patch/9724919/ for v2).
> >
> > Signed-off-by: Josh Zimmerman <joshz@google.com>
>
> Given that the tpm code is going to need this, I recommend someone take
> it through that tree:
>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> Otherwise, if you want me to take it, I can, but I doubt you want it in
> my driver-core tree as that will not get merged to Linus until 4.13-rc1.
>
> thanks,
>
> greg k-h

Re: [PATCH v3] Add "shutdown" to "struct class".

[Greg Kroah-Hartman]

On Thu, May 25, 2017 at 08:40:28AM -0700, Josh Zimmerman wrote:
> Thanks, Greg.
> 
> Greg, Jarkko: Do either of you you have any objections to me
> backporting these changes to 4.4 and 4.9? I'd like to make sure that
> at least the couple most recent LTS kernels have this patch.

Why?  What bug does this solve?  If it meets the rules of
Documentation/stable_kernel_rules.txt (or whereever that file moved to),
that's fine with me.

thanks,

greg k-h

Re: [PATCH v3] Add "shutdown" to "struct class".

[Josh Zimmerman]

On Thu, May 25, 2017 at 9:09 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, May 25, 2017 at 08:40:28AM -0700, Josh Zimmerman wrote:
>> Thanks, Greg.
>>
>> Greg, Jarkko: Do either of you you have any objections to me
>> backporting these changes to 4.4 and 4.9? I'd like to make sure that
>> at least the couple most recent LTS kernels have this patch.
>
> Why?  What bug does this solve?
If a TPM2 device has power removed without a TPM2_Shutdown being
issued, it will increment its "dictionary attack" counter. After that
counter reaches a certain value, the TPM2 device will lock the user
out. Adding the shutdown callback allows the TPM kernel driver to send
TPM2_Shutdown to all TPM2 devices.

> If it meets the rules of
> Documentation/stable_kernel_rules.txt (or whereever that file moved to),
> that's fine with me.
Documentation/process/stable-kernel-rules.rst, right? To comply with
option 1 referred to there (Adding the appropriate "Cc:" to the
description), should I send a new patch email or just reply to this
one and quote the relevant part? (I don't believe the document
specifies.)

Thanks,

Josh

Re: [PATCH v3] Add "shutdown" to "struct class".

[Greg Kroah-Hartman]

On Thu, May 25, 2017 at 09:24:30AM -0700, Josh Zimmerman wrote:
> On Thu, May 25, 2017 at 9:09 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Thu, May 25, 2017 at 08:40:28AM -0700, Josh Zimmerman wrote:
> >> Thanks, Greg.
> >>
> >> Greg, Jarkko: Do either of you you have any objections to me
> >> backporting these changes to 4.4 and 4.9? I'd like to make sure that
> >> at least the couple most recent LTS kernels have this patch.
> >
> > Why?  What bug does this solve?
> If a TPM2 device has power removed without a TPM2_Shutdown being
> issued, it will increment its "dictionary attack" counter. After that
> counter reaches a certain value, the TPM2 device will lock the user
> out. Adding the shutdown callback allows the TPM kernel driver to send
> TPM2_Shutdown to all TPM2 devices.

Is all of that in the tpm patch description?  If so, great, if not,
please add it.

> > If it meets the rules of
> > Documentation/stable_kernel_rules.txt (or whereever that file moved to),
> > that's fine with me.
> Documentation/process/stable-kernel-rules.rst, right? To comply with
> option 1 referred to there (Adding the appropriate "Cc:" to the
> description), should I send a new patch email or just reply to this
> one and quote the relevant part? (I don't believe the document
> specifies.)

You (or who ever applies these patches) needs to add the cc: stable tag
to them.  I suggest resend these, as a patch series, with that in it, so
that it all makes more sense and the tpm maintainer has an easy job of
it.

thanks,

greg k-h

Re: [PATCH v3] Add "shutdown" to "struct class".

[Jarkko Sakkinen]

Josh,

On Thu, May 25, 2017 at 06:41:18PM +0200, Greg Kroah-Hartman wrote:
> On Thu, May 25, 2017 at 09:24:30AM -0700, Josh Zimmerman wrote:
> > On Thu, May 25, 2017 at 9:09 AM, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > > On Thu, May 25, 2017 at 08:40:28AM -0700, Josh Zimmerman wrote:
> > >> Thanks, Greg.
> > >>
> > >> Greg, Jarkko: Do either of you you have any objections to me
> > >> backporting these changes to 4.4 and 4.9? I'd like to make sure that
> > >> at least the couple most recent LTS kernels have this patch.
> > >
> > > Why?  What bug does this solve?
> > If a TPM2 device has power removed without a TPM2_Shutdown being
> > issued, it will increment its "dictionary attack" counter. After that
> > counter reaches a certain value, the TPM2 device will lock the user
> > out. Adding the shutdown callback allows the TPM kernel driver to send
> > TPM2_Shutdown to all TPM2 devices.
> 
> Is all of that in the tpm patch description?  If so, great, if not,
> please add it.
> 
> > > If it meets the rules of
> > > Documentation/stable_kernel_rules.txt (or whereever that file moved to),
> > > that's fine with me.
> > Documentation/process/stable-kernel-rules.rst, right? To comply with
> > option 1 referred to there (Adding the appropriate "Cc:" to the
> > description), should I send a new patch email or just reply to this
> > one and quote the relevant part? (I don't believe the document
> > specifies.)
> 
> You (or who ever applies these patches) needs to add the cc: stable tag
> to them.  I suggest resend these, as a patch series, with that in it, so
> that it all makes more sense and the tpm maintainer has an easy job of
> it.
> 
> thanks,
> 
> greg k-h

Can you send one more patch set with these two patches and Cc-tags and
refined descriptions where needed. If you do this, I will apply them to
my tree and send PR to James Morris. Thank you.

/Jarkko

Re: [PATCH v3] Add "shutdown" to "struct class".

[Jarkko Sakkinen]

On Thu, May 25, 2017 at 08:40:28AM -0700, Josh Zimmerman wrote:
> Thanks, Greg.
> 
> Greg, Jarkko: Do either of you you have any objections to me
> backporting these changes to 4.4 and 4.9? I'd like to make sure that
> at least the couple most recent LTS kernels have this patch. (I don't
> care so much about 4.1 as it'll be EOL'd this September, according to
> https://www.kernel.org/category/releases.html, but I can backport it
> to there as well if desired.)
> 
> Thanks,
> 
> Josh

Nope.

/Jarkko

> 
> On Thu, May 25, 2017 at 6:09 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, May 15, 2017 at 10:34:38AM -0700, Josh Zimmerman wrote:
> > > The TPM class has some common shutdown code that must be executed for
> > > all drivers. This adds some needed functionality for that.
> > >
> > > Usage example: 'tpm: Issue a TPM2_Shutdown for TPM2 devices.'
> > > (see https://patchwork.kernel.org/patch/9724919/ for v2).
> > >
> > > Signed-off-by: Josh Zimmerman <joshz@google.com>
> >
> > Given that the tpm code is going to need this, I recommend someone take
> > it through that tree:
> >
> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> > Otherwise, if you want me to take it, I can, but I doubt you want it in
> > my driver-core tree as that will not get merged to Linus until 4.13-rc1.
> >
> > thanks,
> >
> > greg k-h

Re: [PATCH v3] Add "shutdown" to "struct class".

[Jarkko Sakkinen]

On Thu, May 25, 2017 at 03:09:30PM +0200, Greg Kroah-Hartman wrote:
> On Mon, May 15, 2017 at 10:34:38AM -0700, Josh Zimmerman wrote:
> > The TPM class has some common shutdown code that must be executed for
> > all drivers. This adds some needed functionality for that.
> > 
> > Usage example: 'tpm: Issue a TPM2_Shutdown for TPM2 devices.'
> > (see https://patchwork.kernel.org/patch/9724919/ for v2).
> > 
> > Signed-off-by: Josh Zimmerman <joshz@google.com>
> 
> Given that the tpm code is going to need this, I recommend someone take
> it through that tree:
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> Otherwise, if you want me to take it, I can, but I doubt you want it in
> my driver-core tree as that will not get merged to Linus until 4.13-rc1.
> 
> thanks,
> 
> greg k-h

I can take this to the tpmdd tree. For me only thing that matters that
there isn't collisions. If we all agree on this, I'll apply Josh's
patches and include to my next PR.

/Jarkko