Re: [PATCH v2 1/2] PCI: Take multifunction devices into account when distributing resources

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

 



On Tue, Nov 22, 2022 at 11:45:41AM +0000, Jonathan Cameron wrote:
> On Tue, 22 Nov 2022 08:42:58 +0200
> Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> wrote:
> 
> > Hi,
> > 
> > On Mon, Nov 21, 2022 at 04:45:48PM -0600, Bjorn Helgaas wrote:
> > > IIUC, the summary is this:
> > > 
> > >   00:02.0 bridge window [mem 0x10000000-0x102fffff] to [bus 01-02]
> > >   01:02.0 bridge window [mem 0x10000000-0x100fffff] to [bus 02]
> > >   01:03.0 NIC BAR [mem 0x10200000-0x1021ffff]
> > >   01:04.0 NIC BAR [mem 0x10220000-0x1023ffff]
> > >   02:05.0 NIC BAR [mem 0x10080000-0x1009ffff]
> > > 
> > > and it's the same with and without the current patch.
> > > 
> > > Are all these assignments done by BIOS, or did Linux update them?  
> > 
> > > Did we exercise the same "distribute available resources" path as in
> > > the PCIe case?  I expect we *should*, because there really shouldn't
> > > be any PCI vs PCIe differences in how resources are handled.  This is
> > > why I'm not comfortable with assumptions here that depend on PCIe.
> > > 
> > > I can't tell from Jonathan's PCIe case whether we got a working config
> > > from BIOS or not because our logging of bridge windows is kind of
> > > poor.  
> > 
> > This is ARM64 so there is no "BIOS" involved (something similar though).
> 
> It's EDK2 in my tests  - so very similar to other arch.
> Possible to boot without though and rely on DT, but various things don't
> work yet...

Okay.

> > It is the same "system" that Jonathan used where the regression happened
> > with the multifunction PCIe configuration with the exception that I'm
> > now using PCI devices instead of PCIe as you asked.
> > 
> > I'm not 100% sure if the all the same code paths are used here, though.
> > 
> 
> I wondered if it was possibly to do with fairly minimal handling of pci-pxb
> (the weird root bridge) in EDK2, so tried the obvious of hanging your PCI
> test below one of those rather than directly below a normal bridge.
> Despite shuffling things around into configurations
> I thought might trigger the problem, it all seems fine.

I also did some other experiments like tried to add pcie-root-port first
but that did not trigger the issue either (unless I missed something).

> Note that I can't currently test the pxb-pcie configurations without EDK2
> as arm-virt doesn't provide the relevant DT for those root bridges yet
> (it's on my todo list as it's a prereq for getting the QEMU CXL ARM64
> emulation upstream)
> 
> So no guarantees as I don't understand fully why PCI is ending up
> with different handling.
> 
> From liberal distribution of printk()s it seems that for PCI bridges
> pci_bridge_resources_not_assigned() is false, but for PCI express
> example it is true.  My first instinct is quirk of the EDK2 handling? 
> I'll have a dig, but I'm not an expert in EDK2 at all, so may not get
> to the bottom of this.
> 
> Ultimately it seems that when the OS takes over the prefetchable memory
> resources are not configured for the PCIe case, but are for the PCI case.
> So we aren't currently walking the new code for PCI.

I think the reason why this "difference" is that we have this in
__pci_bus_size_bridges():

               if (bus->self->is_hotplug_bridge) {
                        additional_io_size  = pci_hotplug_io_size;
                        additional_mmio_size = pci_hotplug_mmio_size;
                        additional_mmio_pref_size = pci_hotplug_mmio_pref_size;
                }

For QEMU PCIe root/downstream ports this might be true so we end up with
"additional resources" in the resource list and therfore the kernel
tries to do the allocation wrt. with PCI case it is not. I tried to end
up with the same code path in my command line with this:

  -device pci-bridge,chassis_nr=1,bus=br1,id=br2,shpc=on,addr=2

The "shpc=on" should make it hotplug bridge as well but apparently that
is not happening.



[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