mbox series

[v1,0/3] ACPI: PCI: PM: Power up PCI devices with ACPI companions upfront

Message ID 21439956.EfDdHjke4D@kreacher (mailing list archive)
Headers show
Series ACPI: PCI: PM: Power up PCI devices with ACPI companions upfront | expand

Message

Rafael J. Wysocki April 4, 2022, 3:20 p.m. UTC
Hi All,

There are cases in which the power state of a PCI device depends on an ACPI
power resource (or more of them) in such a way that when the given power
resource is in the "off" state, the PCI device depending on it is in D3cold.

On some systems, the initial state of these power resources is "off", so the
kernel should not access the config space of PCI devices depending on them,
until the power resources in question are turned "on", but currently that is
not respected during PCI device enumeration.  Namely, the PCI device
enumeration code walks the entire bus and enumerates all of the devices it
can find, including the ones whose initial power state in principle depends on
the ACPI power resources in the "off" state.

Apparently, most of the time, the config space of such devices is accessible
regardless of the state of the ACPI power resource associated with the PCI
device, so the device enumeration is successful, but there are two potential
issues related to this behavior.  First off, even if the given PCI device
is accessible when the ACPI power resource depended on by it is "off",
changing its configuration may confuse the platform firmware and lead to
problems when the ACPI power resource in question is turned "on".  Second,
the PCI device may not be actually accessible at all when the ACPI power
resource depended on by it is "off", in which case it won't be found during
the PCI enumeration of devices.

This patch series addresses that problem by turning "on" all ACPI power
resources depended on by PCI devices before attempting to access the config
space of those devices for the first time.

The first two patches introduce the requisite machinery and the actual change
of behavior is done in the last patch.

Thanks!

Comments

Mika Westerberg April 5, 2022, 9:46 a.m. UTC | #1
Hi Rafael,

On Mon, Apr 04, 2022 at 05:20:30PM +0200, Rafael J. Wysocki wrote:
> Hi All,
> 
> There are cases in which the power state of a PCI device depends on an ACPI
> power resource (or more of them) in such a way that when the given power
> resource is in the "off" state, the PCI device depending on it is in D3cold.
> 
> On some systems, the initial state of these power resources is "off", so the
> kernel should not access the config space of PCI devices depending on them,
> until the power resources in question are turned "on", but currently that is
> not respected during PCI device enumeration.  Namely, the PCI device
> enumeration code walks the entire bus and enumerates all of the devices it
> can find, including the ones whose initial power state in principle depends on
> the ACPI power resources in the "off" state.

I guess these devices do not have _PRE() method either.

> Apparently, most of the time, the config space of such devices is accessible
> regardless of the state of the ACPI power resource associated with the PCI
> device, so the device enumeration is successful, but there are two potential
> issues related to this behavior.  First off, even if the given PCI device
> is accessible when the ACPI power resource depended on by it is "off",
> changing its configuration may confuse the platform firmware and lead to
> problems when the ACPI power resource in question is turned "on".  Second,
> the PCI device may not be actually accessible at all when the ACPI power
> resource depended on by it is "off", in which case it won't be found during
> the PCI enumeration of devices.
> 
> This patch series addresses that problem by turning "on" all ACPI power
> resources depended on by PCI devices before attempting to access the config
> space of those devices for the first time.

Makes sense.

For the series,

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Rafael J. Wysocki April 5, 2022, 12:07 p.m. UTC | #2
On Tue, Apr 5, 2022 at 1:45 PM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> Hi Rafael,
>
> On Mon, Apr 04, 2022 at 05:20:30PM +0200, Rafael J. Wysocki wrote:
> > Hi All,
> >
> > There are cases in which the power state of a PCI device depends on an ACPI
> > power resource (or more of them) in such a way that when the given power
> > resource is in the "off" state, the PCI device depending on it is in D3cold.
> >
> > On some systems, the initial state of these power resources is "off", so the
> > kernel should not access the config space of PCI devices depending on them,
> > until the power resources in question are turned "on", but currently that is
> > not respected during PCI device enumeration.  Namely, the PCI device
> > enumeration code walks the entire bus and enumerates all of the devices it
> > can find, including the ones whose initial power state in principle depends on
> > the ACPI power resources in the "off" state.
>
> I guess these devices do not have _PRE() method either.

Personally, I haven't seen any ACPI tables containing any _PRE yet.

> > Apparently, most of the time, the config space of such devices is accessible
> > regardless of the state of the ACPI power resource associated with the PCI
> > device, so the device enumeration is successful, but there are two potential
> > issues related to this behavior.  First off, even if the given PCI device
> > is accessible when the ACPI power resource depended on by it is "off",
> > changing its configuration may confuse the platform firmware and lead to
> > problems when the ACPI power resource in question is turned "on".  Second,
> > the PCI device may not be actually accessible at all when the ACPI power
> > resource depended on by it is "off", in which case it won't be found during
> > the PCI enumeration of devices.
> >
> > This patch series addresses that problem by turning "on" all ACPI power
> > resources depended on by PCI devices before attempting to access the config
> > space of those devices for the first time.
>
> Makes sense.
>
> For the series,
>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thanks!