Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot

[Daniel P . Berrangé]

On Thu, Jul 21, 2022 at 05:05:38PM +0100, Mark Cave-Ayland wrote:
> On 21/07/2022 16:56, Daniel P. Berrangé wrote:
> 
> > On Thu, Jul 21, 2022 at 04:51:51PM +0100, Mark Cave-Ayland wrote:
> > > On 21/07/2022 15:28, Roman Kagan wrote:
> > > 
> > > (lots cut)
> > > 
> > > > In the guest (Fedora 34):
> > > > 
> > > > [root@test ~]# lspci -tv
> > > > -[0000:00]-+-00.0  Intel Corporation 82G33/G31/P35/P31 Express DRAM 
> > > > Controller
> > > >              +-01.0  Device 1234:1111
> > > >              +-02.0  Red Hat, Inc. QEMU XHCI Host Controller
> > > >              +-05.0-[01]----00.0  Red Hat, Inc. Virtio block device
> > > >              +-05.1-[02]----00.0  Red Hat, Inc. Virtio network device
> > > >              +-05.2-[03]--
> > > >              +-05.3-[04]--
> > > >              +-1f.0  Intel Corporation 82801IB (ICH9) LPC Interface 
> > > > Controller
> > > >              \-1f.3  Intel Corporation 82801I (ICH9 Family) SMBus 
> > > > Controller
> > > > 
> > > > Changing addr of the second disk from 4 to 0 makes it appear in the
> > > > guest.
> > > > 
> > > > What exactly do you find odd?
> > > 
> > > Thanks for this, the part I wasn't sure about was whether the device ids 
> > > in
> > > the command line matched the primary PCI bus or the secondary PCI bus.
> > > 
> > > In that case I suspect that the enumeration of non-zero PCIe devices fails
> > > in Linux because of the logic here:
> > > https://github.com/torvalds/linux/blob/master/drivers/pci/probe.c#L2622.
> > 
> > Just above that though is logic that handles 'pci=pcie_scan_all'
> > kernel parameter, to make it look for non-zero devices.
> > 
> > > I don't have a copy of the PCIe specification, but assuming the comment is
> > > true then your patch looks correct to me. I think it would be worth 
> > > adding a
> > > similar comment and reference to your patch to explain why the logic is
> > > required, which should also help the PCI maintainers during review.
> > 
> > The docs above with the pci=pcie_scan_all suggest it is unusual but not
> > forbidden.
> 
> That's interesting as I read it completely the other way around, i.e. PCIe
> downstream ports should only have device 0 and the PCI_SCAN_ALL_PCIE_DEVS
> flag is there for broken/exotic hardware :)

If someone wants to test their guest OS on exotic hardware configs,
shouldn't QEMU let them make such a configuration ? Reproducing
unsual hardware configs when you don't have physical access to real
hardware is one of the benefits of having QEMU available.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|