| Message ID | 20170515173438.13420-1-joshz@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
(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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> -----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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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);
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(+)