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) That was the idea I floated at LPC16. We can override the acpi_pci_root_ops prepare_resources() function pointer with a function that checks IORESOURCE_WINDOW and filters resources accordingly (and specific quirk "drivers" may know how to intepret resources that aren't IORESOURCE_WINDOW - ie they can use it to describe the PCI ECAM config space quirk region in their _CRS). In a way that's something that makes sense anyway because given that we are starting from a clean slate on ARM64 considering resources that are not IORESOURCE_WINDOW as host bridge windows is just something we are inheriting from x86, it is not really ACPI specs compliant (is it ?). > 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? That's something that has to be clarified within the ASWG ie why the consumer bit is ignored for *some* descriptors and not for others. As things stand unfortunately the answer seems yes (I do not know why). > > It *would* be nice to have bridge registers in the bridge _CRS. That > > would eliminate the need for looking up the HISI0081/PNP0C02 devices > > to find the bridge registers. Avoiding that lookup is only a > > temporary advantage -- the next round of bridges are supposed to fully > > implement ECAM, and then we won't need to know where the registers > > are. > > > > Apart from the lookup, there's still some advantage in describing the > > registers in the PNP0A03 device instead of an unrelated PNP0C02 > > device, because it makes /proc/iomem more accurate and potentially > > makes host bridge hotplug cleaner. We would have to enhance the host > > bridge driver to do the reservations currently done by pnp/system.c. > > > > There's some value in doing it the same way as on x86, even though > > that way is somewhat broken. > > > > Whatever we decide, I think it's very important to get it figured out > > ASAP because it affects the ECAM quirks that we're trying to merge in > > v4.10. > > > > I agree. What exactly is the impact for the quirks mechanism as proposed? The impact is that we could just use the PNP0A03 _CRS to report the PCI ECAM config space quirk region through a consumer resource keeping in mind what I say above (actually I think that's what was done on APM firmware initially, for the records). Lorenzo -- 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
