On Wed, Jun 21, 2023 at 05:52:52PM -0500, 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. > > > > > > "The OS runs _REG control methods to inform AML code of a change in the > > > availability of an operation region. When an operation region handler > > > is unavailable, AML cannot access data fields in that region. > > > (Operation region writes will be ignored and reads will return > > > indeterminate data.)" > > > > > > The PCI core does not call `_REG` at anytime, leading to the undefined > > > behavior mentioned in the spec. > I double checked a BIOS debug log which shows ACPI calls > to confirm and didn't see a single _REG call for any device > before this patch across a boot/suspend/resume cycle. OK, thanks, I didn't see one either, which surprised me. Based on the weird exception in sec 6.5.4: Since the [PCI_Config operation region on a PCI root bus containing a _BBN object] is permanently available, no _REG methods are required, nor will OSPM evaluate any _REG methods that appear in the same scope as the operation region declaration(s) of this type. it seems like when we add a PCI host bridge, we should evaluate any _REG in the host bridge scope if it does not include _BBN. The example: It should be noted that PCI Config Space Operation Regions are ready as soon the host controller or bridge controller has been programmed with a bus number. PCI1’s _REG method would not be run until the PCI-PCI bridge has been properly configured. suggests that when we set a PCI-PCI bridge's secondary bus number, we should evaluate any _REG in its scope. At the same time, the OS will also run ETH0’s _REG method since its PCI Config Space would be also available. The OS will again run ETH0’s _REG method when the ETH0 device is started. So evidently we should evaluate ETH0._REG once after setting PCI1's secondary bus number, and again when "ETH0 is started." I have no idea what "started" means. > Reviewed-by: Mario Limonciello <mario.limonciello@xxxxxxx> > For the new patch. Thanks for taking a look at it! I think we're missing some other _REG stuff, but it seems like what this patch adds does match the per-endpoint power management requires, so I applied these to pci/pm for v6.5. Bjorn
