Re: [PATCH/RESEND] arm64: acpi/pci: invoke _DSM whether to preserve firmware PCI setup

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Tue, 2019-06-11 at 18:39 -0500, Bjorn Helgaas wrote:
> On Thu, Jun 06, 2019 at 07:00:12PM +1000, Benjamin Herrenschmidt wrote:
> > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > 
> > On arm64 ACPI systems, we unconditionally reconfigure the entire PCI
> > hierarchy at boot. This is a departure from what is customary on ACPI
> > systems, and may break assumptions in some places (e.g., EFIFB), that
> > the kernel will leave BARs of enabled PCI devices where they are.
> > 
> > Given that PCI already specifies a device specific ACPI method (_DSM)
> > for PCI root bridge nodes that tells us whether the firmware thinks
> > the configuration should be left alone, let's sidestep the entire
> > policy debate about whether the PCI configuration should be preserved
> > or not, and put it under the control of the firmware instead.
> 
> The current PCI Firmware spec r3.2 specifies _DSM function 5 for
> PCI-to-PCI bridge objects, which does not include host bridge
> (PNP0A03) nodes, but the proposed revision does allow it under host
> bridges.  So I'm fine with this, but we should update the commit log
> so it doesn't say "PCI *already* specifies this".
> 
> > [BenH: Added pci_assign_unassigned_root_bus_resources()]
> > 
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> I think you should add a signed-off-by for yourself?

I should, I forgot. That said, Lorenzo wants to wait for the actual
ECN... and we're also discussing some details.
> 

 .../...

> > +	/*
> > +	 * Invoke the PCI device specific method (_DSM) #5 'Ignore PCI Boot
> > +	 * Configuration', which tells us whether the firmware wants us to
> > +	 * preserve the configuration of the PCI resource tree for this root
> > +	 * bridge.
> > +	 */
> > +	obj = acpi_evaluate_dsm(ACPI_HANDLE(bus->bridge), &pci_acpi_dsm_guid, 1,
> > +	                        IGNORE_PCI_BOOT_CONFIG_DSM, NULL);
> > +	if (obj && obj->type == ACPI_TYPE_INTEGER && obj->integer.value == 0) {
> 
> This is fine, but can we make a tiny step toward doing this in generic
> code instead of adding more arch-specific stuff?
> 
> E.g., evaluate the _DSM in the generic acpi_pci_root_add(), set a
> "preserve_config" bit in the struct acpi_pci_root, and test the bit
> here?

I'd rather have the flag in the host bridge no ?

> It would also be nice to add a printk in the oter
> pci_acpi_scan_root() implementations if the bit is set so we know that
> the platform supplied the _DSM but we're ignoring it.

Ok.

Talking of which, look at the ongoing discussion I have with Lorenzo
when it comes to pci_bus_claim_resources vs. what x86 does, I'd love
for you to chime in. I'd like to try to consolidate things further
accross architectures but there might be reasons I don't see as to why
things are different in that area, so ...

Cheers,
Ben.