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?