[RFC 0/3] PCI/PM: RTD3 fixes/changes for 3.7

[RFC 0/3] PCI/PM: RTD3 fixes/changes for 3.7

[Huang Ying]

[RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending
[RFC 2/3] PCI/PM: Make PCI devices notified when its power
[RFC 3/3] PCI/PM: Disable PME poll for PCIe devices

Best Regards,
Huang Ying

[RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Huang Ying]

If PCI devices are put into D3cold before kexec, because the
configuration registers of PCI devices in D3cold are not accessible.

And if PCI bridges are put into low power state before kexec,
configuration registers of PCI devices underneath the PCI bridges are
not accessible too.

These will make some PCI devices can not be scanned after kexec, so
resume the PCI devices in D3cold or PCI bridges in low power state
before kexec.

Signed-off-by: Huang Ying <ying.huang@intel.com>
---
 drivers/pci/pci-driver.c |    4 ++++
 1 file changed, 4 insertions(+)

--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	struct pci_driver *drv = pci_dev->driver;
 
+	/* Resume bridges and devices in D3cold for kexec to work properly */
+	if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
+		pm_runtime_resume(dev);
+
 	if (drv && drv->shutdown)
 		drv->shutdown(pci_dev);
 	pci_msi_shutdown(pci_dev);

Re: [RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Bjorn Helgaas]

+cc Eric and kexec list

On Mon, Sep 17, 2012 at 2:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> If PCI devices are put into D3cold before kexec, because the
> configuration registers of PCI devices in D3cold are not accessible.
>
> And if PCI bridges are put into low power state before kexec,
> configuration registers of PCI devices underneath the PCI bridges are
> not accessible too.
>
> These will make some PCI devices can not be scanned after kexec, so
> resume the PCI devices in D3cold or PCI bridges in low power state
> before kexec.

Don't we need to resume the device even without the kexec issue?  And
even if it's in D1 or D2?

It looks to me like pci_msi_shutdown() (and probably drv->shutdown())
depend on the device being in D0.

> Signed-off-by: Huang Ying <ying.huang@intel.com>
> ---
>  drivers/pci/pci-driver.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
>         struct pci_dev *pci_dev = to_pci_dev(dev);
>         struct pci_driver *drv = pci_dev->driver;
>
> +       /* Resume bridges and devices in D3cold for kexec to work properly */
> +       if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
> +               pm_runtime_resume(dev);
> +
>         if (drv && drv->shutdown)
>                 drv->shutdown(pci_dev);
>         pci_msi_shutdown(pci_dev);

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Bjorn Helgaas]

+cc Eric and kexec list

On Mon, Sep 17, 2012 at 2:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> If PCI devices are put into D3cold before kexec, because the
> configuration registers of PCI devices in D3cold are not accessible.
>
> And if PCI bridges are put into low power state before kexec,
> configuration registers of PCI devices underneath the PCI bridges are
> not accessible too.
>
> These will make some PCI devices can not be scanned after kexec, so
> resume the PCI devices in D3cold or PCI bridges in low power state
> before kexec.

Don't we need to resume the device even without the kexec issue?  And
even if it's in D1 or D2?

It looks to me like pci_msi_shutdown() (and probably drv->shutdown())
depend on the device being in D0.

> Signed-off-by: Huang Ying <ying.huang@intel.com>
> ---
>  drivers/pci/pci-driver.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
>         struct pci_dev *pci_dev = to_pci_dev(dev);
>         struct pci_driver *drv = pci_dev->driver;
>
> +       /* Resume bridges and devices in D3cold for kexec to work properly */
> +       if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
> +               pm_runtime_resume(dev);
> +
>         if (drv && drv->shutdown)
>                 drv->shutdown(pci_dev);
>         pci_msi_shutdown(pci_dev);

Re: [RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Eric W. Biederman]

Bjorn Helgaas <bhelgaas@google.com> writes:

> +cc Eric and kexec list
>
> On Mon, Sep 17, 2012 at 2:54 AM, Huang Ying <ying.huang@intel.com> wrote:
>> If PCI devices are put into D3cold before kexec, because the
>> configuration registers of PCI devices in D3cold are not accessible.
>>
>> And if PCI bridges are put into low power state before kexec,
>> configuration registers of PCI devices underneath the PCI bridges are
>> not accessible too.
>>
>> These will make some PCI devices can not be scanned after kexec, so
>> resume the PCI devices in D3cold or PCI bridges in low power state
>> before kexec.
>
> Don't we need to resume the device even without the kexec issue?  And
> even if it's in D1 or D2?

The basic requirement is that the device needs to be visible so we can
auto discover it.  As I recall most sleep states don't make the device
invisible and we can handle the rest in the device initializatoin code.

> It looks to me like pci_msi_shutdown() (and probably drv->shutdown())
> depend on the device being in D0.

There is certainly a depenency on the config registers being visible.
Although I don't know if much will go wrong if they aren't.

Ceratinly pci_msi_shutdown doesn't have anything to do if the device has
had so much power removed that the device is not even exectuing.

Eric

>> Signed-off-by: Huang Ying <ying.huang@intel.com>
>> ---
>>  drivers/pci/pci-driver.c |    4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> --- a/drivers/pci/pci-driver.c
>> +++ b/drivers/pci/pci-driver.c
>> @@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
>>         struct pci_dev *pci_dev = to_pci_dev(dev);
>>         struct pci_driver *drv = pci_dev->driver;
>>
>> +       /* Resume bridges and devices in D3cold for kexec to work properly */
>> +       if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
>> +               pm_runtime_resume(dev);
>> +
>>         if (drv && drv->shutdown)
>>                 drv->shutdown(pci_dev);
>>         pci_msi_shutdown(pci_dev);
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Eric W. Biederman]

Bjorn Helgaas <bhelgaas@google.com> writes:

> +cc Eric and kexec list
>
> On Mon, Sep 17, 2012 at 2:54 AM, Huang Ying <ying.huang@intel.com> wrote:
>> If PCI devices are put into D3cold before kexec, because the
>> configuration registers of PCI devices in D3cold are not accessible.
>>
>> And if PCI bridges are put into low power state before kexec,
>> configuration registers of PCI devices underneath the PCI bridges are
>> not accessible too.
>>
>> These will make some PCI devices can not be scanned after kexec, so
>> resume the PCI devices in D3cold or PCI bridges in low power state
>> before kexec.
>
> Don't we need to resume the device even without the kexec issue?  And
> even if it's in D1 or D2?

The basic requirement is that the device needs to be visible so we can
auto discover it.  As I recall most sleep states don't make the device
invisible and we can handle the rest in the device initializatoin code.

> It looks to me like pci_msi_shutdown() (and probably drv->shutdown())
> depend on the device being in D0.

There is certainly a depenency on the config registers being visible.
Although I don't know if much will go wrong if they aren't.

Ceratinly pci_msi_shutdown doesn't have anything to do if the device has
had so much power removed that the device is not even exectuing.

Eric

>> Signed-off-by: Huang Ying <ying.huang@intel.com>
>> ---
>>  drivers/pci/pci-driver.c |    4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> --- a/drivers/pci/pci-driver.c
>> +++ b/drivers/pci/pci-driver.c
>> @@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
>>         struct pci_dev *pci_dev = to_pci_dev(dev);
>>         struct pci_driver *drv = pci_dev->driver;
>>
>> +       /* Resume bridges and devices in D3cold for kexec to work properly */
>> +       if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
>> +               pm_runtime_resume(dev);
>> +
>>         if (drv && drv->shutdown)
>>                 drv->shutdown(pci_dev);
>>         pci_msi_shutdown(pci_dev);
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Re: [RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Huang Ying]

On Thu, 2012-09-20 at 00:38 -0700, Eric W. Biederman wrote:
> Bjorn Helgaas <bhelgaas@google.com> writes:
> 
> > +cc Eric and kexec list
> >
> > On Mon, Sep 17, 2012 at 2:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> >> If PCI devices are put into D3cold before kexec, because the
> >> configuration registers of PCI devices in D3cold are not accessible.
> >>
> >> And if PCI bridges are put into low power state before kexec,
> >> configuration registers of PCI devices underneath the PCI bridges are
> >> not accessible too.
> >>
> >> These will make some PCI devices can not be scanned after kexec, so
> >> resume the PCI devices in D3cold or PCI bridges in low power state
> >> before kexec.
> >
> > Don't we need to resume the device even without the kexec issue?  And
> > even if it's in D1 or D2?
> 
> The basic requirement is that the device needs to be visible so we can
> auto discover it.  As I recall most sleep states don't make the device
> invisible and we can handle the rest in the device initializatoin code.

PCI devices in D3cold or under a bridge in D3hot will not be visible, so
we must fix that for kexec to run properly.

> > It looks to me like pci_msi_shutdown() (and probably drv->shutdown())
> > depend on the device being in D0.
> 
> There is certainly a depenency on the config registers being visible.
> Although I don't know if much will go wrong if they aren't.
> 
> Ceratinly pci_msi_shutdown doesn't have anything to do if the device has
> had so much power removed that the device is not even exectuing.

Don't know which power state device should be in for pci_msi_shutdown
etc.  But it appears that normal shutdown/reboot and kexec works at most
times so far.  D3cold and bridge in D3hot works for normal
shutdown/reboot, but not for kexec.  So I write some fix.

Best Regards,
Huang Ying

> >> Signed-off-by: Huang Ying <ying.huang@intel.com>
> >> ---
> >>  drivers/pci/pci-driver.c |    4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> --- a/drivers/pci/pci-driver.c
> >> +++ b/drivers/pci/pci-driver.c
> >> @@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
> >>         struct pci_dev *pci_dev = to_pci_dev(dev);
> >>         struct pci_driver *drv = pci_dev->driver;
> >>
> >> +       /* Resume bridges and devices in D3cold for kexec to work properly */
> >> +       if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
> >> +               pm_runtime_resume(dev);
> >> +
> >>         if (drv && drv->shutdown)
> >>                 drv->shutdown(pci_dev);
> >>         pci_msi_shutdown(pci_dev);
> >
> > _______________________________________________
> > kexec mailing list
> > kexec@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kexec



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Huang Ying]

On Thu, 2012-09-20 at 00:38 -0700, Eric W. Biederman wrote:
> Bjorn Helgaas <bhelgaas@google.com> writes:
> 
> > +cc Eric and kexec list
> >
> > On Mon, Sep 17, 2012 at 2:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> >> If PCI devices are put into D3cold before kexec, because the
> >> configuration registers of PCI devices in D3cold are not accessible.
> >>
> >> And if PCI bridges are put into low power state before kexec,
> >> configuration registers of PCI devices underneath the PCI bridges are
> >> not accessible too.
> >>
> >> These will make some PCI devices can not be scanned after kexec, so
> >> resume the PCI devices in D3cold or PCI bridges in low power state
> >> before kexec.
> >
> > Don't we need to resume the device even without the kexec issue?  And
> > even if it's in D1 or D2?
> 
> The basic requirement is that the device needs to be visible so we can
> auto discover it.  As I recall most sleep states don't make the device
> invisible and we can handle the rest in the device initializatoin code.

PCI devices in D3cold or under a bridge in D3hot will not be visible, so
we must fix that for kexec to run properly.

> > It looks to me like pci_msi_shutdown() (and probably drv->shutdown())
> > depend on the device being in D0.
> 
> There is certainly a depenency on the config registers being visible.
> Although I don't know if much will go wrong if they aren't.
> 
> Ceratinly pci_msi_shutdown doesn't have anything to do if the device has
> had so much power removed that the device is not even exectuing.

Don't know which power state device should be in for pci_msi_shutdown
etc.  But it appears that normal shutdown/reboot and kexec works at most
times so far.  D3cold and bridge in D3hot works for normal
shutdown/reboot, but not for kexec.  So I write some fix.

Best Regards,
Huang Ying

> >> Signed-off-by: Huang Ying <ying.huang@intel.com>
> >> ---
> >>  drivers/pci/pci-driver.c |    4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> --- a/drivers/pci/pci-driver.c
> >> +++ b/drivers/pci/pci-driver.c
> >> @@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
> >>         struct pci_dev *pci_dev = to_pci_dev(dev);
> >>         struct pci_driver *drv = pci_dev->driver;
> >>
> >> +       /* Resume bridges and devices in D3cold for kexec to work properly */
> >> +       if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
> >> +               pm_runtime_resume(dev);
> >> +
> >>         if (drv && drv->shutdown)
> >>                 drv->shutdown(pci_dev);
> >>         pci_msi_shutdown(pci_dev);
> >
> > _______________________________________________
> > kexec mailing list
> > kexec@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kexec

Re: [RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Eric W. Biederman]

Huang Ying <ying.huang@intel.com> writes:

> On Thu, 2012-09-20 at 00:38 -0700, Eric W. Biederman wrote:
>> Bjorn Helgaas <bhelgaas@google.com> writes:
>> 
>> > +cc Eric and kexec list
>> >
>> > On Mon, Sep 17, 2012 at 2:54 AM, Huang Ying <ying.huang@intel.com> wrote:
>> >> If PCI devices are put into D3cold before kexec, because the
>> >> configuration registers of PCI devices in D3cold are not accessible.
>> >>
>> >> And if PCI bridges are put into low power state before kexec,
>> >> configuration registers of PCI devices underneath the PCI bridges are
>> >> not accessible too.
>> >>
>> >> These will make some PCI devices can not be scanned after kexec, so
>> >> resume the PCI devices in D3cold or PCI bridges in low power state
>> >> before kexec.
>> >
>> > Don't we need to resume the device even without the kexec issue?  And
>> > even if it's in D1 or D2?
>> 
>> The basic requirement is that the device needs to be visible so we can
>> auto discover it.  As I recall most sleep states don't make the device
>> invisible and we can handle the rest in the device initializatoin code.
>
> PCI devices in D3cold or under a bridge in D3hot will not be visible, so
> we must fix that for kexec to run properly.

That seems reasonable to me.

>> > It looks to me like pci_msi_shutdown() (and probably drv->shutdown())
>> > depend on the device being in D0.
>> 
>> There is certainly a depenency on the config registers being visible.
>> Although I don't know if much will go wrong if they aren't.
>> 
>> Ceratinly pci_msi_shutdown doesn't have anything to do if the device has
>> had so much power removed that the device is not even exectuing.
>
> Don't know which power state device should be in for pci_msi_shutdown
> etc.  But it appears that normal shutdown/reboot and kexec works at most
> times so far.  D3cold and bridge in D3hot works for normal
> shutdown/reboot, but not for kexec.  So I write some fix.

To be clear.  Has someone picked up this patch yet?

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

As for reboot it tends to always work because frequently the firmware
code triggers a soft reset of the entire machine, which puts devices in
their power on reset state, which is not a suspended state.  Although
that behavior is not required for a software reboot.

Eric



> Best Regards,
> Huang Ying
>
>> >> Signed-off-by: Huang Ying <ying.huang@intel.com>
>> >> ---
>> >>  drivers/pci/pci-driver.c |    4 ++++
>> >>  1 file changed, 4 insertions(+)
>> >>
>> >> --- a/drivers/pci/pci-driver.c
>> >> +++ b/drivers/pci/pci-driver.c
>> >> @@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
>> >>         struct pci_dev *pci_dev = to_pci_dev(dev);
>> >>         struct pci_driver *drv = pci_dev->driver;
>> >>
>> >> +       /* Resume bridges and devices in D3cold for kexec to work properly */
>> >> +       if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
>> >> +               pm_runtime_resume(dev);
>> >> +
>> >>         if (drv && drv->shutdown)
>> >>                 drv->shutdown(pci_dev);
>> >>         pci_msi_shutdown(pci_dev);
>> >
>> > _______________________________________________
>> > kexec mailing list
>> > kexec@lists.infradead.org
>> > http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Eric W. Biederman]

Huang Ying <ying.huang@intel.com> writes:

> On Thu, 2012-09-20 at 00:38 -0700, Eric W. Biederman wrote:
>> Bjorn Helgaas <bhelgaas@google.com> writes:
>> 
>> > +cc Eric and kexec list
>> >
>> > On Mon, Sep 17, 2012 at 2:54 AM, Huang Ying <ying.huang@intel.com> wrote:
>> >> If PCI devices are put into D3cold before kexec, because the
>> >> configuration registers of PCI devices in D3cold are not accessible.
>> >>
>> >> And if PCI bridges are put into low power state before kexec,
>> >> configuration registers of PCI devices underneath the PCI bridges are
>> >> not accessible too.
>> >>
>> >> These will make some PCI devices can not be scanned after kexec, so
>> >> resume the PCI devices in D3cold or PCI bridges in low power state
>> >> before kexec.
>> >
>> > Don't we need to resume the device even without the kexec issue?  And
>> > even if it's in D1 or D2?
>> 
>> The basic requirement is that the device needs to be visible so we can
>> auto discover it.  As I recall most sleep states don't make the device
>> invisible and we can handle the rest in the device initializatoin code.
>
> PCI devices in D3cold or under a bridge in D3hot will not be visible, so
> we must fix that for kexec to run properly.

That seems reasonable to me.

>> > It looks to me like pci_msi_shutdown() (and probably drv->shutdown())
>> > depend on the device being in D0.
>> 
>> There is certainly a depenency on the config registers being visible.
>> Although I don't know if much will go wrong if they aren't.
>> 
>> Ceratinly pci_msi_shutdown doesn't have anything to do if the device has
>> had so much power removed that the device is not even exectuing.
>
> Don't know which power state device should be in for pci_msi_shutdown
> etc.  But it appears that normal shutdown/reboot and kexec works at most
> times so far.  D3cold and bridge in D3hot works for normal
> shutdown/reboot, but not for kexec.  So I write some fix.

To be clear.  Has someone picked up this patch yet?

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

As for reboot it tends to always work because frequently the firmware
code triggers a soft reset of the entire machine, which puts devices in
their power on reset state, which is not a suspended state.  Although
that behavior is not required for a software reboot.

Eric



> Best Regards,
> Huang Ying
>
>> >> Signed-off-by: Huang Ying <ying.huang@intel.com>
>> >> ---
>> >>  drivers/pci/pci-driver.c |    4 ++++
>> >>  1 file changed, 4 insertions(+)
>> >>
>> >> --- a/drivers/pci/pci-driver.c
>> >> +++ b/drivers/pci/pci-driver.c
>> >> @@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
>> >>         struct pci_dev *pci_dev = to_pci_dev(dev);
>> >>         struct pci_driver *drv = pci_dev->driver;
>> >>
>> >> +       /* Resume bridges and devices in D3cold for kexec to work properly */
>> >> +       if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
>> >> +               pm_runtime_resume(dev);
>> >> +
>> >>         if (drv && drv->shutdown)
>> >>                 drv->shutdown(pci_dev);
>> >>         pci_msi_shutdown(pci_dev);
>> >
>> > _______________________________________________
>> > kexec mailing list
>> > kexec@lists.infradead.org
>> > http://lists.infradead.org/mailman/listinfo/kexec

Re: [RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Rafael J. Wysocki]

On Monday, September 17, 2012, Bjorn Helgaas wrote:
> +cc Eric and kexec list
> 
> On Mon, Sep 17, 2012 at 2:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> > If PCI devices are put into D3cold before kexec, because the
> > configuration registers of PCI devices in D3cold are not accessible.
> >
> > And if PCI bridges are put into low power state before kexec,
> > configuration registers of PCI devices underneath the PCI bridges are
> > not accessible too.
> >
> > These will make some PCI devices can not be scanned after kexec, so
> > resume the PCI devices in D3cold or PCI bridges in low power state
> > before kexec.
> 
> Don't we need to resume the device even without the kexec issue?  And
> even if it's in D1 or D2?
> 
> It looks to me like pci_msi_shutdown() (and probably drv->shutdown())
> depend on the device being in D0.

We should in theory, but we didn't do any power management of PCI bridges
before, so this is the first time we have a problem with it.

So I'd say, yeah, let's resume if current_state is between D1 and D3cold
inclusive and the kexec comment is not very helpful (the problem is not
kexec-specific in general).

Speaking of kexec, it might consider using the hibernation device freeze
instead of device shutdown (which the kexec jump feature does).  I've seen
reports of problems that would be solved this way most likely.

Thanks,
Rafael


> > Signed-off-by: Huang Ying <ying.huang@intel.com>
> > ---
> >  drivers/pci/pci-driver.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > --- a/drivers/pci/pci-driver.c
> > +++ b/drivers/pci/pci-driver.c
> > @@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
> >         struct pci_dev *pci_dev = to_pci_dev(dev);
> >         struct pci_driver *drv = pci_dev->driver;
> >
> > +       /* Resume bridges and devices in D3cold for kexec to work properly */
> > +       if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
> > +               pm_runtime_resume(dev);
> > +
> >         if (drv && drv->shutdown)
> >                 drv->shutdown(pci_dev);
> >         pci_msi_shutdown(pci_dev);
> 
> 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Rafael J. Wysocki]

On Monday, September 17, 2012, Bjorn Helgaas wrote:
> +cc Eric and kexec list
> 
> On Mon, Sep 17, 2012 at 2:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> > If PCI devices are put into D3cold before kexec, because the
> > configuration registers of PCI devices in D3cold are not accessible.
> >
> > And if PCI bridges are put into low power state before kexec,
> > configuration registers of PCI devices underneath the PCI bridges are
> > not accessible too.
> >
> > These will make some PCI devices can not be scanned after kexec, so
> > resume the PCI devices in D3cold or PCI bridges in low power state
> > before kexec.
> 
> Don't we need to resume the device even without the kexec issue?  And
> even if it's in D1 or D2?
> 
> It looks to me like pci_msi_shutdown() (and probably drv->shutdown())
> depend on the device being in D0.

We should in theory, but we didn't do any power management of PCI bridges
before, so this is the first time we have a problem with it.

So I'd say, yeah, let's resume if current_state is between D1 and D3cold
inclusive and the kexec comment is not very helpful (the problem is not
kexec-specific in general).

Speaking of kexec, it might consider using the hibernation device freeze
instead of device shutdown (which the kexec jump feature does).  I've seen
reports of problems that would be solved this way most likely.

Thanks,
Rafael


> > Signed-off-by: Huang Ying <ying.huang@intel.com>
> > ---
> >  drivers/pci/pci-driver.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > --- a/drivers/pci/pci-driver.c
> > +++ b/drivers/pci/pci-driver.c
> > @@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
> >         struct pci_dev *pci_dev = to_pci_dev(dev);
> >         struct pci_driver *drv = pci_dev->driver;
> >
> > +       /* Resume bridges and devices in D3cold for kexec to work properly */
> > +       if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
> > +               pm_runtime_resume(dev);
> > +
> >         if (drv && drv->shutdown)
> >                 drv->shutdown(pci_dev);
> >         pci_msi_shutdown(pci_dev);
> 
> 

Re: [RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Huang Ying]

On Thu, 2012-09-20 at 21:20 +0200, Rafael J. Wysocki wrote:
> On Monday, September 17, 2012, Bjorn Helgaas wrote:
> > +cc Eric and kexec list
> > 
> > On Mon, Sep 17, 2012 at 2:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> > > If PCI devices are put into D3cold before kexec, because the
> > > configuration registers of PCI devices in D3cold are not accessible.
> > >
> > > And if PCI bridges are put into low power state before kexec,
> > > configuration registers of PCI devices underneath the PCI bridges are
> > > not accessible too.
> > >
> > > These will make some PCI devices can not be scanned after kexec, so
> > > resume the PCI devices in D3cold or PCI bridges in low power state
> > > before kexec.
> > 
> > Don't we need to resume the device even without the kexec issue?  And
> > even if it's in D1 or D2?
> > 
> > It looks to me like pci_msi_shutdown() (and probably drv->shutdown())
> > depend on the device being in D0.
> 
> We should in theory, but we didn't do any power management of PCI bridges
> before, so this is the first time we have a problem with it.
> 
> So I'd say, yeah, let's resume if current_state is between D1 and D3cold
> inclusive and the kexec comment is not very helpful (the problem is not
> kexec-specific in general).

Resume from D1 to D3cold for any device or just bridges?

Best Regards,
Huang Ying

> Speaking of kexec, it might consider using the hibernation device freeze
> instead of device shutdown (which the kexec jump feature does).  I've seen
> reports of problems that would be solved this way most likely.
> 
> Thanks,
> Rafael
> 
> 
> > > Signed-off-by: Huang Ying <ying.huang@intel.com>
> > > ---
> > >  drivers/pci/pci-driver.c |    4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > --- a/drivers/pci/pci-driver.c
> > > +++ b/drivers/pci/pci-driver.c
> > > @@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
> > >         struct pci_dev *pci_dev = to_pci_dev(dev);
> > >         struct pci_driver *drv = pci_dev->driver;
> > >
> > > +       /* Resume bridges and devices in D3cold for kexec to work properly */
> > > +       if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
> > > +               pm_runtime_resume(dev);
> > > +
> > >         if (drv && drv->shutdown)
> > >                 drv->shutdown(pci_dev);
> > >         pci_msi_shutdown(pci_dev);
> > 
> > 
> 



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Huang Ying]

On Thu, 2012-09-20 at 21:20 +0200, Rafael J. Wysocki wrote:
> On Monday, September 17, 2012, Bjorn Helgaas wrote:
> > +cc Eric and kexec list
> > 
> > On Mon, Sep 17, 2012 at 2:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> > > If PCI devices are put into D3cold before kexec, because the
> > > configuration registers of PCI devices in D3cold are not accessible.
> > >
> > > And if PCI bridges are put into low power state before kexec,
> > > configuration registers of PCI devices underneath the PCI bridges are
> > > not accessible too.
> > >
> > > These will make some PCI devices can not be scanned after kexec, so
> > > resume the PCI devices in D3cold or PCI bridges in low power state
> > > before kexec.
> > 
> > Don't we need to resume the device even without the kexec issue?  And
> > even if it's in D1 or D2?
> > 
> > It looks to me like pci_msi_shutdown() (and probably drv->shutdown())
> > depend on the device being in D0.
> 
> We should in theory, but we didn't do any power management of PCI bridges
> before, so this is the first time we have a problem with it.
> 
> So I'd say, yeah, let's resume if current_state is between D1 and D3cold
> inclusive and the kexec comment is not very helpful (the problem is not
> kexec-specific in general).

Resume from D1 to D3cold for any device or just bridges?

Best Regards,
Huang Ying

> Speaking of kexec, it might consider using the hibernation device freeze
> instead of device shutdown (which the kexec jump feature does).  I've seen
> reports of problems that would be solved this way most likely.
> 
> Thanks,
> Rafael
> 
> 
> > > Signed-off-by: Huang Ying <ying.huang@intel.com>
> > > ---
> > >  drivers/pci/pci-driver.c |    4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > --- a/drivers/pci/pci-driver.c
> > > +++ b/drivers/pci/pci-driver.c
> > > @@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
> > >         struct pci_dev *pci_dev = to_pci_dev(dev);
> > >         struct pci_driver *drv = pci_dev->driver;
> > >
> > > +       /* Resume bridges and devices in D3cold for kexec to work properly */
> > > +       if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
> > > +               pm_runtime_resume(dev);
> > > +
> > >         if (drv && drv->shutdown)
> > >                 drv->shutdown(pci_dev);
> > >         pci_msi_shutdown(pci_dev);
> > 
> > 
> 

Re: [RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Rafael J. Wysocki]

On Friday, September 21, 2012, Huang Ying wrote:
> On Thu, 2012-09-20 at 21:20 +0200, Rafael J. Wysocki wrote:
> > On Monday, September 17, 2012, Bjorn Helgaas wrote:
> > > +cc Eric and kexec list
> > > 
> > > On Mon, Sep 17, 2012 at 2:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> > > > If PCI devices are put into D3cold before kexec, because the
> > > > configuration registers of PCI devices in D3cold are not accessible.
> > > >
> > > > And if PCI bridges are put into low power state before kexec,
> > > > configuration registers of PCI devices underneath the PCI bridges are
> > > > not accessible too.
> > > >
> > > > These will make some PCI devices can not be scanned after kexec, so
> > > > resume the PCI devices in D3cold or PCI bridges in low power state
> > > > before kexec.
> > > 
> > > Don't we need to resume the device even without the kexec issue?  And
> > > even if it's in D1 or D2?
> > > 
> > > It looks to me like pci_msi_shutdown() (and probably drv->shutdown())
> > > depend on the device being in D0.
> > 
> > We should in theory, but we didn't do any power management of PCI bridges
> > before, so this is the first time we have a problem with it.
> > 
> > So I'd say, yeah, let's resume if current_state is between D1 and D3cold
> > inclusive and the kexec comment is not very helpful (the problem is not
> > kexec-specific in general).
> 
> Resume from D1 to D3cold for any device or just bridges?

I'd say every device for starters.

Thanks,
Rafael


> > Speaking of kexec, it might consider using the hibernation device freeze
> > instead of device shutdown (which the kexec jump feature does).  I've seen
> > reports of problems that would be solved this way most likely.
> > 
> > Thanks,
> > Rafael
> > 
> > 
> > > > Signed-off-by: Huang Ying <ying.huang@intel.com>
> > > > ---
> > > >  drivers/pci/pci-driver.c |    4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > --- a/drivers/pci/pci-driver.c
> > > > +++ b/drivers/pci/pci-driver.c
> > > > @@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
> > > >         struct pci_dev *pci_dev = to_pci_dev(dev);
> > > >         struct pci_driver *drv = pci_dev->driver;
> > > >
> > > > +       /* Resume bridges and devices in D3cold for kexec to work properly */
> > > > +       if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
> > > > +               pm_runtime_resume(dev);
> > > > +
> > > >         if (drv && drv->shutdown)
> > > >                 drv->shutdown(pci_dev);
> > > >         pci_msi_shutdown(pci_dev);
> > > 
> > > 
> > 
> 
> 
> 
> 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [RFC 1/3] PCI/PM: Fix kexec for D3cold and bridge suspending

[Rafael J. Wysocki]

On Friday, September 21, 2012, Huang Ying wrote:
> On Thu, 2012-09-20 at 21:20 +0200, Rafael J. Wysocki wrote:
> > On Monday, September 17, 2012, Bjorn Helgaas wrote:
> > > +cc Eric and kexec list
> > > 
> > > On Mon, Sep 17, 2012 at 2:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> > > > If PCI devices are put into D3cold before kexec, because the
> > > > configuration registers of PCI devices in D3cold are not accessible.
> > > >
> > > > And if PCI bridges are put into low power state before kexec,
> > > > configuration registers of PCI devices underneath the PCI bridges are
> > > > not accessible too.
> > > >
> > > > These will make some PCI devices can not be scanned after kexec, so
> > > > resume the PCI devices in D3cold or PCI bridges in low power state
> > > > before kexec.
> > > 
> > > Don't we need to resume the device even without the kexec issue?  And
> > > even if it's in D1 or D2?
> > > 
> > > It looks to me like pci_msi_shutdown() (and probably drv->shutdown())
> > > depend on the device being in D0.
> > 
> > We should in theory, but we didn't do any power management of PCI bridges
> > before, so this is the first time we have a problem with it.
> > 
> > So I'd say, yeah, let's resume if current_state is between D1 and D3cold
> > inclusive and the kexec comment is not very helpful (the problem is not
> > kexec-specific in general).
> 
> Resume from D1 to D3cold for any device or just bridges?

I'd say every device for starters.

Thanks,
Rafael


> > Speaking of kexec, it might consider using the hibernation device freeze
> > instead of device shutdown (which the kexec jump feature does).  I've seen
> > reports of problems that would be solved this way most likely.
> > 
> > Thanks,
> > Rafael
> > 
> > 
> > > > Signed-off-by: Huang Ying <ying.huang@intel.com>
> > > > ---
> > > >  drivers/pci/pci-driver.c |    4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > --- a/drivers/pci/pci-driver.c
> > > > +++ b/drivers/pci/pci-driver.c
> > > > @@ -421,6 +421,10 @@ static void pci_device_shutdown(struct d
> > > >         struct pci_dev *pci_dev = to_pci_dev(dev);
> > > >         struct pci_driver *drv = pci_dev->driver;
> > > >
> > > > +       /* Resume bridges and devices in D3cold for kexec to work properly */
> > > > +       if (pci_dev->current_state == PCI_D3cold || pci_dev->subordinate)
> > > > +               pm_runtime_resume(dev);
> > > > +
> > > >         if (drv && drv->shutdown)
> > > >                 drv->shutdown(pci_dev);
> > > >         pci_msi_shutdown(pci_dev);
> > > 
> > > 
> > 
> 
> 
> 
> 

[RFC 2/3] PCI/PM: Make PCI devices notified when its power resource turned on

[Huang Ying]

A set of power resources may be shared by multiple devices.  When all
devices share one set of power resources is put into D3_COLD state,
the power resources will be turned off.  When one of the device is
waked, the power resource will be turned on and all devices share it
will be powered on to D0uninitialized state.  These devices should be
resumed, so that they can get opportunity to go to low power state
later.

Signed-off-by: Huang Ying <ying.huang@intel.com>
---
 drivers/acpi/pci_bind.c |    2 ++
 1 file changed, 2 insertions(+)

--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -45,6 +45,7 @@ static int acpi_pci_unbind(struct acpi_d
 
 	device_set_run_wake(&dev->dev, false);
 	pci_acpi_remove_pm_notifier(device);
+	acpi_power_resource_unregister_device(dev, device->handle);
 
 	if (!dev->subordinate)
 		goto out;
@@ -71,6 +72,7 @@ static int acpi_pci_bind(struct acpi_dev
 		return 0;
 
 	pci_acpi_add_pm_notifier(device, dev);
+	acpi_power_resource_register_device(dev, device->handle);
 	if (device->wakeup.flags.run_wake)
 		device_set_run_wake(&dev->dev, true);
 

Re: [RFC 2/3] PCI/PM: Make PCI devices notified when its power resource turned on

[Rafael J. Wysocki]

On Monday, September 17, 2012, Huang Ying wrote:
> A set of power resources may be shared by multiple devices.  When all
> devices share one set of power resources is put into D3_COLD state,
> the power resources will be turned off.  When one of the device is
> waked, the power resource will be turned on and all devices share it
> will be powered on to D0uninitialized state.  These devices should be
> resumed, so that they can get opportunity to go to low power state
> later.
> 
> Signed-off-by: Huang Ying <ying.huang@intel.com>

Makes sense.

Acked-by: Rafael J. Wysocki <rjw@sisk.pl>

> ---
>  drivers/acpi/pci_bind.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> --- a/drivers/acpi/pci_bind.c
> +++ b/drivers/acpi/pci_bind.c
> @@ -45,6 +45,7 @@ static int acpi_pci_unbind(struct acpi_d
>  
>  	device_set_run_wake(&dev->dev, false);
>  	pci_acpi_remove_pm_notifier(device);
> +	acpi_power_resource_unregister_device(dev, device->handle);
>  
>  	if (!dev->subordinate)
>  		goto out;
> @@ -71,6 +72,7 @@ static int acpi_pci_bind(struct acpi_dev
>  		return 0;
>  
>  	pci_acpi_add_pm_notifier(device, dev);
> +	acpi_power_resource_register_device(dev, device->handle);
>  	if (device->wakeup.flags.run_wake)
>  		device_set_run_wake(&dev->dev, true);
>  
> 
> 

[RFC 3/3] PCI/PM: Disable PME poll for PCIe devices

[Huang Ying]

PME poll is not necessary for PCIe devices, because PCIe devices use
in-band PME message and IRQ on PCIe port to report PME.

PME poll is useful for PCI devices.  Because for PCI devices, PME is
reported via a side-band PME# line and some platform logic, and the
platform logic is often missing on many systems.

Signed-off-by: Huang Ying <ying.huang@intel.com>
---
 drivers/pci/pcie/pme.c |    1 +
 1 file changed, 1 insertion(+)

--- a/drivers/pci/pcie/pme.c
+++ b/drivers/pci/pcie/pme.c
@@ -313,6 +313,7 @@ static int pcie_pme_set_native(struct pc
 
 	device_set_run_wake(&dev->dev, true);
 	dev->pme_interrupt = true;
+	dev->pme_poll = false;
 	return 0;
 }
 

Re: [RFC 3/3] PCI/PM: Disable PME poll for PCIe devices

[Rafael J. Wysocki]

On Monday, September 17, 2012, Huang Ying wrote:
> PME poll is not necessary for PCIe devices, because PCIe devices use
> in-band PME message and IRQ on PCIe port to report PME.

Alas, not all of them as it turns out and even if they do, it doesn't
work for some of them. That's why we've added the PCIe devices polling
(quite recently, for that matter).

If you'd spent some time on some proper research regarding that (like browsing
the changelogs of git commits modifying the relevant part of drivers/pci/pci.c),
you'd have known that already.

And that actually is quite important, because I don't have to remember every
single PM-related change we're making in the PCI layer.  I _incidentally_ do
remember this one, but that may not happen next time.  Please do the research
_before_ proposing changes of this kind.

Thanks,
Rafael


> PME poll is useful for PCI devices.  Because for PCI devices, PME is
> reported via a side-band PME# line and some platform logic, and the
> platform logic is often missing on many systems.
> 
> Signed-off-by: Huang Ying <ying.huang@intel.com>
> ---
>  drivers/pci/pcie/pme.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- a/drivers/pci/pcie/pme.c
> +++ b/drivers/pci/pcie/pme.c
> @@ -313,6 +313,7 @@ static int pcie_pme_set_native(struct pc
>  
>  	device_set_run_wake(&dev->dev, true);
>  	dev->pme_interrupt = true;
> +	dev->pme_poll = false;
>  	return 0;
>  }
>  
> 
> 

Re: [RFC 3/3] PCI/PM: Disable PME poll for PCIe devices

[Matthew Garrett]

On Thu, Sep 20, 2012 at 09:31:13PM +0200, Rafael J. Wysocki wrote:
> On Monday, September 17, 2012, Huang Ying wrote:
> > PME poll is not necessary for PCIe devices, because PCIe devices use
> > in-band PME message and IRQ on PCIe port to report PME.
> 
> Alas, not all of them as it turns out and even if they do, it doesn't
> work for some of them. That's why we've added the PCIe devices polling
> (quite recently, for that matter).

Right. I'd believe that there's some underlying bug that's causing the 
missing PMEs and maybe it does work in theory, but the reality is that 
right now some PCIe devices are not getting their PMEs delivered to us. 
We don't know if it's our bug or a hardware bug, and until we do we need 
to leave the polling code.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFC 3/3] PCI/PM: Disable PME poll for PCIe devices

[Huang Ying]

On Thu, 2012-09-20 at 21:31 +0200, Rafael J. Wysocki wrote:
> On Monday, September 17, 2012, Huang Ying wrote:
> > PME poll is not necessary for PCIe devices, because PCIe devices use
> > in-band PME message and IRQ on PCIe port to report PME.
> 
> Alas, not all of them as it turns out and even if they do, it doesn't
> work for some of them. That's why we've added the PCIe devices polling
> (quite recently, for that matter).
> 
> If you'd spent some time on some proper research regarding that (like browsing
> the changelogs of git commits modifying the relevant part of drivers/pci/pci.c),
> you'd have known that already.
> 
> And that actually is quite important, because I don't have to remember every
> single PM-related change we're making in the PCI layer.  I _incidentally_ do
> remember this one, but that may not happen next time.  Please do the research
> _before_ proposing changes of this kind.

Sorry.  I should have done more research before sending the patch out.
Will do more research next time.

Best Regards,
Huang Ying