linux-kernel - RE: [RFC] platform: detach from PM domains on shutdown

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <HE1PR04MB300483E474BE21D55720BCEC88910@HE1PR04MB3004.eurprd04.prod.outlook.com>
Date:   Thu, 17 May 2018 02:33:50 +0000
From:   Peng Fan <peng.fan@....com>
To:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Ulf Hansson <ulf.hansson@...aro.org>
CC:     "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Fabio Estevam <fabio.estevam@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: RE: [RFC] platform: detach from PM domains on shutdown



> -----Original Message-----
> From: rjwysocki@...il.com [mailto:rjwysocki@...il.com] On Behalf Of Rafael
> J. Wysocki
> Sent: 2018年5月17日 5:35
> To: Ulf Hansson <ulf.hansson@...aro.org>
> Cc: Peng Fan <peng.fan@....com>; Rafael J. Wysocki
> <rafael.j.wysocki@...el.com>; Fabio Estevam <fabio.estevam@....com>; Greg
> Kroah-Hartman <gregkh@...uxfoundation.org>; Linux Kernel Mailing List
> <linux-kernel@...r.kernel.org>; Linux PM <linux-pm@...r.kernel.org>;
> dl-linux-imx <linux-imx@....com>
> Subject: Re: [RFC] platform: detach from PM domains on shutdown
> 
> On Wed, May 16, 2018 at 11:52 AM, Ulf Hansson <ulf.hansson@...aro.org>
> wrote:
> > On 15 May 2018 at 11:01, Peng Fan <peng.fan@....com> wrote:
> >> When reboot Linux, the PM domains attached to a device are not
> >> shutdown. To SoCs which relys on reset the whole SoC, there is no
> >> need to shutdown PM domains, but to Linux running in a virtual
> >> machine with devices pass-through, we could not reset the whole SoC.
> >> Currently we need Linux to shutdown its PM domains when reboot.
> >
> > I am not sure I understand exactly why the PM domain needs to be
> > shutdown for these cases, could you please elaborate a bit on that.
> >
> > BTW, what platform are you running on and also what PM domains are being
> used?
> >
> > Anyway, it seems like there may be need for certain cases, but
> > certainly not all - especially since it may slow down the shutdown
> > process, when not needed.
> >
> > Can we make this runtime configurable, via sysfs or whatever that makes
> sense!?
> >
> >>
> >> commit 2d30bb0b3889 ("platform: Do not detach from PM domains on
> >> shutdown"), removes what this patch tries to add, because of a warning.
> >> commit e79aee49bcf9 ("PM: Avoid false-positive warnings in
> >> dev_pm_domain_set()") already fixes the false alarm warning. So let's
> >> detach the power domain to shutdown PM domains after driver shutdown.
> >>
> >> Signed-off-by: Peng Fan <peng.fan@....com>
> >> ---
> >>
> >> I do not find a better place to shutdown power domain when reboot
> >> Linux, so add back the line that commit 2d30bb0b3889 removes, because
> >> it is a false alarm warning as commit e79aee49bcf9 describes.
> >>
> >>  drivers/base/platform.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/base/platform.c b/drivers/base/platform.c index
> >> 8075ddc70a17..a5929f24dc3c 100644
> >> --- a/drivers/base/platform.c
> >> +++ b/drivers/base/platform.c
> >> @@ -616,6 +616,7 @@ static void platform_drv_shutdown(struct device
> >> *_dev)
> >>
> >>         if (drv->shutdown)
> >>                 drv->shutdown(dev);
> >> +       dev_pm_domain_detach(_dev, true);
> >
> > This would somewhat work, but only for platform devices. To make this
> > fully work, we need to call dev_pm_domain_detach() from amba, spi, etc
> > as well.
> >
> > Perhaps another option to manage this more generally, an without
> > having detach devices, could be to extend the struct dev_pm_domain
> > with a new callback, "->shutdown()" and then make the driver core call
> > it from device_shutdown().
> 
> I'm sensing a possible ordering slippery slope with this (it will only work if all of
> the drivers/bus types etc do the right thing in their
> ->shutdown callbacks so nothing depends on the domain going forward).
> 
> > Typically, for genpd, I would probably count the number of calls being
> > made to ->shutdown() per PM domain, then when it reaches the number of
> > attached devices to it, allow to power off it.
> >
> > Let's see what Rafael thinks about it.
> 
> I'm not sure about the use case.  The hypervisor should be able to take care of
> turning power domains off on the client OS reboot in theory.  If the client OS
> leaving the hypervisor needs to worry about what state it leaves behind, the
> design of the hypervisor is sort of questionable IMO.

This is valid concern. But moving the power domain logic into hypervisor mostly micro-kernel design
will introduce more complexity and make certification harder.
Currently, Let Linux shutdown it's power domain is the easiest way to me and make things work well
after reboot.

Thanks,
Peng