On Wed, Nov 23, 2016 at 09:06:33AM -0600, Bjorn Helgaas wrote: > On Wed, Nov 23, 2016 at 07:28:12AM +0000, Ard Biesheuvel wrote: > > On 23 November 2016 at 01:06, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > > On Tue, Nov 22, 2016 at 10:09:50AM +0000, Ard Biesheuvel wrote: > > >> On 17 November 2016 at 17:59, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote: > > > > > >> > +PCI host bridges are PNP0A03 or PNP0A08 devices. Their _CRS should > > >> > +describe all the address space they consume. In principle, this would > > >> > +be all the windows they forward down to the PCI bus, as well as the > > >> > +bridge registers themselves. The bridge registers include things like > > >> > +secondary/subordinate bus registers that determine the bus range below > > >> > +the bridge, window registers that describe the apertures, etc. These > > >> > +are all device-specific, non-architected things, so the only way a > > >> > +PNP0A03/PNP0A08 driver can manage them is via _PRS/_CRS/_SRS, which > > >> > +contain the device-specific details. These bridge registers also > > >> > +include ECAM space, since it is consumed by the bridge. > > >> > + > > >> > +ACPI defined a Producer/Consumer bit that was intended to distinguish > > >> > +the bridge apertures from the bridge registers [4, 5]. However, > > >> > +BIOSes didn't use that bit correctly, and the result is that OSes have > > >> > +to assume that everything in a PCI host bridge _CRS is a window. That > > >> > +leaves no way to describe the bridge registers in the PNP0A03/PNP0A08 > > >> > +device itself. > > >> > > >> Is that universally true? Or is it still possible to do the right > > >> thing here on new ACPI architectures such as arm64? > > > > > > That's a very good question. I had thought that the ACPI spec had > > > given up on Consumer/Producer completely, but I was wrong. In the 6.0 > > > spec, the Consumer/Producer bit is still documented in the Extended > > > Address Space Descriptor (sec 6.4.3.5.4). It is documented as > > > "ignored" in the QWord, DWord, and Word descriptors (sec 6.4.3.5.1,2,3). > > > > > > Linux looks at the producer_consumer bit in acpi_decode_space(), which > > > I think is used for all these descriptors (QWord, DWord, Word, and > > > Extended). This doesn't quite follow the spec -- we probably should > > > ignore it except for Extended. In any event, acpi_decode_space() sets > > > IORESOURCE_WINDOW for Producer descriptors, but we don't test > > > IORESOURCE_WINDOW in the PCI host bridge code. > > > > > > x86 and ia64 supply their own pci_acpi_root_prepare_resources() > > > functions that call acpi_pci_probe_root_resources(), which parses _CRS > > > and looks at producer_consumer. Then they do a little arch-specific > > > stuff on the result. > > > > > > On arm64 we use acpi_pci_probe_root_resources() directly, with no > > > arch-specific stuff. > > > > > > On all three arches, we ignore the Consumer/Producer bit, so all the > > > resources are treated as Producers, e.g., as bridge windows. > > > > > > I think we *could* implement an arm64 version of > > > pci_acpi_root_prepare_resources() that would pay attention to the > > > Consumer/Producer bit by checking IORESOURCE_WINDOW. To be spec > > > compliant, we would have to use Extended descriptors for all bridge > > > windows, even if they would fit in a DWord or QWord. > > > > > > Should we do that? I dunno. I'd like to hear your opinion(s). > > > > > > > Yes, I think we should. If the spec allows for a way for a PNP0A03 > > device to describe all of its resources unambiguously, we should not > > be relying on workarounds that were designed for another architecture > > in another decade (for, presumably, another OS) > > > > Just for my understanding, we will need to use extended descriptors > > for all consumed *and* produced regions, even though dword/qword are > > implicitly produced-only, due to the fact that the bit is ignored? > > From an ACPI spec point of view, I would say QWord/DWord/Word > descriptors are implicitly *consumer*-only because ResourceConsumer > is the default and they don't have a bit to indicate otherwise. > > The current code assumes all PNP0A03 resources are producers. If we > implement an arm64 pci_acpi_root_prepare_resources() that pays > attention to the Consumer/Producer bit, we would have to: > > - Reserve all producer regions in the iomem/ioport trees. This is > already done via pci_acpi_root_add_resources(), but we might need > a new check to handle consumers differently. > > - Reserve all consumer regions. This corresponds to what > pnp/system.c does for PNP0C02 devices. This is similar to the > producer regions, but I think the consumer ones should be marked > IORESOURCE_BUSY. > > - Use every producer (IORESOURCE_WINDOW) as a host bridge window. > > I think it's a bug that acpi_decode_space() looks at producer_consumer > for QWord/DWord/Word descriptors, but I think QWord/DWord/Word > descriptors for consumed regions should be safe, as long as they don't > set the Consumer/Producer bit. I'm going to post a couple very lightly-tested patches that should make us ignore the Consumer/Producer bit for QWord/DWord/Word. I'd appreciate any discussion about whether that's the right approach. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
