Re: [PATCH v7 17/26] PCI: hotplug: Ignore the MEM BAR offsets from BIOS/bootloader

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Jan 29, 2020 at 06:29:28PM +0300, Sergei Miroshnichenko wrote:
> BAR allocation by BIOS/UEFI/bootloader/firmware may be non-optimal and
> it may even clash with the kernel's BAR assignment algorithm.
> 
> For example, sometimes BIOS doesn't reserve space for SR-IOV BARs, and
> this bridge window can neither extend (blocked by immovable BARs) nor
> move (the device itself is immovable).
> 
> With this patch the kernel will use its own methods of BAR allocating
> when possible, increasing the chances of successful boot and hotplug.
> 
> Signed-off-by: Sergei Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
>  drivers/pci/probe.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index bb584038d5b4..f8f643dac6d1 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -306,6 +306,14 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
>  			 pos, (unsigned long long)region.start);
>  	}
>  
> +	if (pci_can_move_bars && res->start && !(res->flags & IORESOURCE_IO)) {
> +		pci_warn(dev, "ignore the current offset of BAR %llx-%llx\n",
> +			 l64, l64 + sz64 - 1);
> +		res->start = 0;
> +		res->end = sz64 - 1;
> +		res->flags |= IORESOURCE_SIZEALIGN;
> +	}
> +
>  	goto out;
>  
>  
> @@ -528,8 +536,10 @@ void pci_read_bridge_bases(struct pci_bus *child)
>  		child->resource[i] = &dev->resource[PCI_BRIDGE_RESOURCES+i];
>  
>  	pci_read_bridge_io(child);
> -	pci_read_bridge_mmio(child);
> -	pci_read_bridge_mmio_pref(child);
> +	if (!pci_can_move_bars) {
> +		pci_read_bridge_mmio(child);
> +		pci_read_bridge_mmio_pref(child);
> +	}

I mentioned this in another response, but I'll repeat it here to
comment on the code directly: I don't think we should have feature
tests like this "!pci_can_move_bars" scattered around, and I don't
want basic behaviors like reading bridge windows during enumeration to
depend on it.

There's no obvious reason why we should ignore bridge windows if we
support movable BARs.

>  	if (dev->transparent) {
>  		pci_bus_for_each_resource(child->parent, res, i) {
> @@ -2945,6 +2955,8 @@ int pci_host_probe(struct pci_host_bridge *bridge)
>  		pci_bus_claim_resources(bus);
>  	} else {
>  		pci_bus_size_bridges(bus);
> +		if (pci_can_move_bars)
> +			pci_bus_update_realloc_range(bus);
>  		pci_bus_assign_resources(bus);
>  
>  		list_for_each_entry(child, &bus->children, node)
> -- 
> 2.24.1
>