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.