linux-kernel - Re: [PATCH v4] PCI: Call _REG when transitioning D-states

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: <20230622220856.GA150386@bhelgaas>
Date:   Thu, 22 Jun 2023 17:08:56 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     "Limonciello, Mario" <mario.limonciello@....com>
Cc:     "Rafael J . Wysocki" <rafael@...nel.org>,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        Len Brown <lenb@...nel.org>, linux-acpi@...r.kernel.org
Subject: Re: [PATCH v4] PCI: Call _REG when transitioning D-states

On Thu, Jun 22, 2023 at 12:43:56PM -0500, Limonciello, Mario wrote:
> On 6/21/2023 5:52 PM, Limonciello, Mario wrote:
> > On 6/21/2023 5:28 PM, Bjorn Helgaas wrote:
> > > On Tue, Jun 20, 2023 at 09:04:51AM -0500, Mario Limonciello wrote:
> > > > Section 6.5.4 of the ACPI 6.4 spec describes how AML is unable
> > > > to access an OperationRegion unless `_REG` has been called.

> > > If so, I guess AML that uses ACPI_ADR_SPACE_PCI_CONFIG won't work
> > > until after we set the relevant device to D0?
> >
> > The particular problem that that exposed this issue doesn't
> > happen until suspend/resume time, but yes I think this should
> > be called when setting the device to D0.

> > > Do we explicitly set devices to D0 during enumeration, e.g., somewhere
> > > in the pci_scan_device() path?  If not, should we?
> >
> > AFAICT it's happening for PCIe ports as part of:
> > pcie_portdrv_probe
> > ->pcie_port_device_register
> > ->->pci_enable_device
> > ->->->pci_enable_device_flags
> > ->->->->do_pci_enable_device
> > ->->->->->pci_set_power_state(pci_dev, PCI_D0)
> 
> Just to add to this; I double checked and one of the devices that
> doesn't have a driver on my system has the power_state set to
> "unknown". I don't think it would be appropriate to explicitly put
> "all devices without drivers" into D0 as this could block low power
> states for the SOC.

I think we probably *should* be evaluating _REG for bridges and
endpoints during enumeration, but I don't think we should depend on
pci_enable_device() to do it.

If a method uses the PCI config OpRegion, I think it should work at
any time, but with this patch it looks like it will only work after a
transition to D0.

> On the Intel side, there is some special stuff in intel-pmc-core for example
> that explicitly puts specific driverless devices into D3 to ensure low power
> states.  If we put everything without a driver into D0 we may break stuff
> like
> that.