Re: [PATCH] PCI: allow pci_resize_resource() to be used on devices on the root bus

From: Ard Biesheuvel <ardb@kernel.org>
To: "Christian König" <christian.koenig@amd.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org, jon@solid-run.com, wasim.khan@nxp.com
Subject: Re: [PATCH] PCI: allow pci_resize_resource() to be used on devices on the root bus
Date: Sat, 25 Apr 2020 19:32:07 +0200	[thread overview]
Message-ID: <CAMj1kXH3WVMehgbMwUEjBSYudAM7PtWAcAYcWspyq4eZJzBwTQ@mail.gmail.com> (raw)
In-Reply-To: <CAMj1kXE0wFtyD7YGxXzKWAx+BT6x9CYreaFyEeFfeYJFeQbo_g@mail.gmail.com>

On Tue, 21 Apr 2020 at 19:07, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 21 Apr 2020 at 18:43, Christian König <christian.koenig@amd.com> wrote:
> >
> > Am 21.04.20 um 18:22 schrieb Ard Biesheuvel:
> > > When resizing a BAR, pci_reassign_bridge_resources() is invoked to
> > > bring the bridge windows of parent bridges in line with the new BAR
> > > assignment.
> > >
> > > This assumes that the device whose BAR is being resized lives on a
> > > subordinate bus, but this is not necessarily the case. A device may
> > > live on the root bus, in which case dev->bus->self is NULL, and
> > > passing a NULL pci_dev pointer to pci_reassign_bridge_resources()
> > > will cause it to crash.
> > >
> > > So let's make the call to pci_reassign_bridge_resources() conditional
> > > on whether dev->bus->self is non-NULL in the first place.
> > >
> > > Fixes: 8bb705e3e79d84e7 ("PCI: Add pci_resize_resource() for resizing BARs")
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >
> > Sounds like it makes sense, patch is
> > Reviewed-by: Christian König <christian.koenig@amd.com>.
>
> Thanks Christian.
>
> >
> > May I ask where you found that condition?
> >
>
> In this particular case, it was on an ARM board with funky PCIe IP
> that does not expose a root port in its bus hierarchy.
>
> But in the general case, PCIe endpoints can be integrated into the
> root complex, in which case they appear on the root bus, and there is
> no reason such endpoints shouldn't be allowed to have resizable BARs.

Actually, looking at this more carefully, I think
pci_reassign_bridge_resources() needs to do /something/ to ensure that
the resources are reshuffled if needed when the resized BAR overlaps
with another one.

Bjorn, did you have any thoughts on this? I could make
pci_reassign_bridge_resources() take a pci_bus, and handle the root
bus as a special case. Alternatively, pci_resize_resource() could make
the distinction, but it will probably need to duplicate some of the
reassignment that goes on in pci_reassign_bridge_resources() as well.