[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot
From: |
Roman Kagan |
Subject: |
Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot |
Date: |
Thu, 21 Jul 2022 19:10:20 +0300 |
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 :)
Me too :)
The commit that introduced it suggested the same:
commit 284f5f9dbac170b054c1e386ef92cbf654e91bba
Author: Bjorn Helgaas <bhelgaas@google.com>
Date: Mon Apr 30 15:21:02 2012 -0600
PCI: work around Stratus ftServer broken PCIe hierarchy
A PCIe downstream port is a P2P bridge. Its secondary interface is
a link that should lead only to device 0 (unless ARI is enabled)[1], so
we don't probe for non-zero device numbers.
Some Stratus ftServer systems have a PCIe downstream port (02:00.0) that
leads to both an upstream port (03:00.0) and a downstream port (03:01.0),
and 03:01.0 has important devices below it:
[0000:02]-+-00.0-[03-3c]--+-00.0-[04-09]--...
\-01.0-[0a-0d]--+-[USB]
+-[NIC]
+-...
Previously, we didn't enumerate device 03:01.0, so USB and the network
didn't work. This patch adds a DMI quirk to scan all device numbers,
not just 0, below a downstream port.
Based on a patch by Prarit Bhargava.
[1] PCIe spec r3.0, sec 7.3.1
CC: Myron Stowe <mstowe@redhat.com>
CC: Don Dutile <ddutile@redhat.com>
CC: James Paradis <james.paradis@stratus.com>
CC: Matthew Wilcox <matthew.r.wilcox@intel.com>
CC: Jesse Barnes <jbarnes@virtuousgeek.org>
CC: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Perhaps if someone has a copy of the PCIe specification they can check the
> wording in section 7.3.1 to see exactly what the correct behaviour should
> be?
I don't, sorry
Thanks,
Roman.
- Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot, (continued)
- Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot, Thomas Huth, 2022/07/20
- Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot, Daniel P . Berrangé, 2022/07/20
- Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot, Roman Kagan, 2022/07/20
- Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot, Mark Cave-Ayland, 2022/07/20
- Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot, Roman Kagan, 2022/07/21
- Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot, Mark Cave-Ayland, 2022/07/21
- Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot, Daniel P . Berrangé, 2022/07/21
- Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot, Mark Cave-Ayland, 2022/07/21
- Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot,
Roman Kagan <=
- Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot, Daniel P . Berrangé, 2022/07/21
- Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot, Thomas Huth, 2022/07/22
- Re: [PATCH v3] hw/pci/pci_bridge: ensure PCIe slots have only one slot, Mark Cave-Ayland, 2022/07/22