Re: [PATCH 1/1] PCI: Coalesce host bridge contiguous apertures without sorting

From: Kai-Heng Feng
Date: Tue Jul 13 2021 - 04:49:50 EST


On Tue, Jul 13, 2021 at 4:45 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> On 7/13/21 12:57 AM, Kai-Heng Feng wrote:
> > Commit 65db04053efe ("PCI: Coalesce host bridge contiguous apertures")
> > sorts the resources by address so the resources can be swapped.
> >
> > Before:
> > PCI host bridge to bus 0002:00
> > pci_bus 0002:00: root bus resource [io 0x0000-0xffff]
> > pci_bus 0002:00: root bus resource [mem 0xd80000000-0xdffffffff] (bus address [0x80000000-0xffffffff])
> > pci_bus 0002:00: root bus resource [mem 0xc0ee00000-0xc0eefffff] (bus address [0x00000000-0x000fffff])
> >
> > And after:
> > PCI host bridge to bus 0002:00
> > pci_bus 0002:00: root bus resource [io 0x0000-0xffff]
> > pci_bus 0002:00: root bus resource [mem 0xc0ee00000-0xc0eefffff] (bus address [0x00000000-0x000fffff])
> > pci_bus 0002:00: root bus resource [mem 0xd80000000-0xdffffffff] (bus address [0x80000000-0xffffffff])
> >
> > However, the sorted resources make NVMe stops working on QEMU ppc:sam460ex.
> >
> > Resources in the original issue are already in ascending order:
> > pci_bus 0000:00: root bus resource [mem 0x10020200000-0x100303fffff window]
> > pci_bus 0000:00: root bus resource [mem 0x10030400000-0x100401fffff window]
> >
> > So remove the sorting part to resolve the issue.
> >
> > Reported-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> > Fixes: 65db04053efe ("PCI: Coalesce host bridge contiguous apertures")
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
>
> I think the original commit message would make more sense here. This patch
> doesn't fix 65db04053efe, it replaces it. The commit message now misses
> the point, and the patch coalesces continuous apertures without explaining
> the reason.

Because the message is already in the git log so I didn't think that's
necessary.
Will send another one with the original message along with this one.

Kai-Heng

>
> Guenter
>
> > ---
> > drivers/pci/probe.c | 31 +++++++++++++++++++++++++++----
> > 1 file changed, 27 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 79177ac37880..5de157600466 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -877,11 +877,11 @@ static void pci_set_bus_msi_domain(struct pci_bus *bus)
> > static int pci_register_host_bridge(struct pci_host_bridge *bridge)
> > {
> > struct device *parent = bridge->dev.parent;
> > - struct resource_entry *window, *n;
> > + struct resource_entry *window, *next, *n;
> > struct pci_bus *bus, *b;
> > - resource_size_t offset;
> > + resource_size_t offset, next_offset;
> > LIST_HEAD(resources);
> > - struct resource *res;
> > + struct resource *res, *next_res;
> > char addr[64], *fmt;
> > const char *name;
> > int err;
> > @@ -961,11 +961,34 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
> > if (nr_node_ids > 1 && pcibus_to_node(bus) == NUMA_NO_NODE)
> > dev_warn(&bus->dev, "Unknown NUMA node; performance will be reduced\n");
> >
> > + /* Coalesce contiguous windows */
> > + resource_list_for_each_entry_safe(window, n, &resources) {
> > + if (list_is_last(&window->node, &resources))
> > + break;
> > +
> > + next = list_next_entry(window, node);
> > + offset = window->offset;
> > + res = window->res;
> > + next_offset = next->offset;
> > + next_res = next->res;
> > +
> > + if (res->flags != next_res->flags || offset != next_offset)
> > + continue;
> > +
> > + if (res->end + 1 == next_res->start) {
> > + next_res->start = res->start;
> > + res->flags = res->start = res->end = 0;
> > + }
> > + }
> > +
> > /* Add initial resources to the bus */
> > resource_list_for_each_entry_safe(window, n, &resources) {
> > - list_move_tail(&window->node, &bridge->windows);
> > offset = window->offset;
> > res = window->res;
> > + if (!res->end)
> > + continue;
> > +
> > + list_move_tail(&window->node, &bridge->windows);
> >
> > if (res->flags & IORESOURCE_BUS)
> > pci_bus_insert_busn_res(bus, bus->number, res->end);
> >
>