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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 26 Apr 2020 at 11:58, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
>
> On Sun, 26 Apr 2020 at 11:08, Christian König <christian.koenig@xxxxxxx> wrote:
> >
> > Am 25.04.20 um 19:32 schrieb Ard Biesheuvel:
> > > On Tue, 21 Apr 2020 at 19:07, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> > >> On Tue, 21 Apr 2020 at 18:43, Christian König <christian.koenig@xxxxxxx> 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@xxxxxxxxxx>
> > >>> Sounds like it makes sense, patch is
> > >>> Reviewed-by: Christian König <christian.koenig@xxxxxxx>.
> > >> 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.
> >
> > The resized BAR never overlaps with an existing one since to resize a
> > BAR it needs to be deallocated and disabled. This is done as a
> > precaution to avoid potential incorrect routing and decode of memory access.
> >
> > The call to pci_reassign_bridge_resources() is only there to change the
> > resources of the upstream bridge to the device which is resized and not
> > to adjust the resources of the device itself.
> >
>
> So does that mean that BAR resizing is only possible if no other BARs,
> either on the same device or on other ones, need to be moved?

OK, so obviously, the current code already releases and reassigns
resources on the same device.

What I am not getting is how this works with more complex topologies, e.g.,

RP 1
multi function device (e.g., GPU + HDMI)
GPU BAR 0 256M
GPU BAR 1 16 M
HDMI BAR 0 16 KB

RP 2
some other endpoint using MMIO64 BARs

So in this case, RP1 will get a prefetchable bridge BAR window of 512
M, and RP2 may get one that is directly adjacent to that. When
resizing GPU BAR 0 to 4 GB, the whole hierarchy needs to be
reshuffled, right?




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux