Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S938769AbcKWPHf (ORCPT ); Wed, 23 Nov 2016 10:07:35 -0500 Received: from mail.kernel.org ([198.145.29.136]:57020 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933700AbcKWPHc (ORCPT ); Wed, 23 Nov 2016 10:07:32 -0500 Date: Wed, 23 Nov 2016 09:06:33 -0600 From: Bjorn Helgaas To: Ard Biesheuvel Cc: Bjorn Helgaas , linux-pci@vger.kernel.org, "linux-acpi@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linaro-acpi@lists.linaro.org" Subject: Re: [PATCH] PCI: Add information about describing PCI in ACPI Message-ID: <20161123150632.GA16033@bhelgaas-glaptop.roam.corp.google.com> References: <20161117175938.17465.45820.stgit@bhelgaas-glaptop.roam.corp.google.com> <20161123010600.GF4832@bhelgaas-glaptop.roam.corp.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4842 Lines: 91 On Wed, Nov 23, 2016 at 07:28:12AM +0000, Ard Biesheuvel wrote: > On 23 November 2016 at 01:06, Bjorn Helgaas wrote: > > On Tue, Nov 22, 2016 at 10:09:50AM +0000, Ard Biesheuvel wrote: > >> On 17 November 2016 at 17:59, Bjorn Helgaas 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.