Re: [Qemu-devel] [PATCH v3 0/3] add check for PCIe root ports and downstream ports

[Gonglei (Arei)]

Hi,

> Subject: Re: [PATCH v3 0/3] add check for PCIe root ports and downstream
> ports
> 
> On Mon, Sep 01, 2014 at 09:29:16PM +0800, address@hidden wrote:
> > From: Gonglei <address@hidden>
> >
> > Root ports and downstream ports of switches are the hot
> > pluggable ports in a PCI Express hierarchy.
> > PCI Express supports chip-to-chip interconnect, a PCIe link can
> > only connect one pci device/Switch/EndPoint or PCI-bridge.
> >
> > 7.3. Configuration Transaction Rules (PCI Express specification 3.0)
> > 7.3.1. Device Number
> >
> > Downstream Ports that do not have ARI Forwarding enabled must
> > associate only Device 0 with the device attached to the Logical Bus
> > representing the Link from the Port.
> >
> > In QEMU, ARI Forwarding is enabled defualt at emulation of PCIe
> > ports. ARI Forwarding enable setting at firmware/OS Control handoff.
> > If the bit is Set when a non-ARI Device is present, the non-ARI
> > Device can respond to Configuration Space accesses under what it
> > interprets as being different Device Numbers, and its Functions can
> > be aliased under multiple Device Numbers, generally leading to
> > undesired behavior.
> >
> > So, for pci devices attached in pcie root ports or downstream pots,
> > we shoud assure that its slot is non-zero. For pcie devcies, which
> > ARP capbility is not enabled, we also should assure that its slot
> > is non-zero.
> 
> So what is this patchset about?

Sorry, it should s/"is non-zero"/"is not non-zero"/ about the above 
descriptions.

> Is there a broken configuration that this helps prevent?

Yes. Otherwise it will confuse users who configure a device with 'slot > 0 ',
and the interface return OK, but the guest os report errors as below:

[ 159.035250] pciehp 0000:05:00.0:pcie24: Button pressed on Slot (0 - 4)
[ 159.035274] pciehp 0000:05:00.0:pcie24: Card present on Slot (0 - 4)
[ 159.036517] pciehp 0000:05:00.0:pcie24: PCI slot #0 - 4 - powering on due to 
button press.
[ 159.188049] pciehp 0000:05:00.0:pcie24: Failed to check link status
[ 159.201968] pciehp 0000:05:00.0:pcie24: Card not present on Slot (0 - 4)
[ 159.202529] pciehp 0000:05:00.0:pcie24: Already disabled on Slot (0 - 4)

> Can you show the command-line please?
> 
Of course I can.

$./qemu-system-x86_64 -enable-kvm -m 2048 -machine q35 -device 
ide-drive,bus=ide.2,drive=MacHDD \
 -drive id=MacHDD,if=none,file=/home/redhat_q35.img -monitor stdio -vnc :10 
-readconfig \
 ../docs/q35-chipset.cfg.ori -device 
virtio-net-pci,id=nic1,bus=pcie-switch-downstream-port-1-1,addr=1.0
(qemu) info network
hub 0
 \ user.0: index=0,type=user,net=10.0.2.0,restrict=off
 \ e1000.0: index=0,type=nic,model=e1000,macaddr=52:54:00:12:34:56
(qemu)

The nic1 is not identified by guest os because I configured "addr=1.0"( slot > 
0) .
If we configured "addr=0.0", it will be OK. (The blow steps show a process
hotplugging a virtio-net-pci with "slot = 0")

(qemu) device_add 
virtio-net-pci,id=nic2,bus=pcie-switch-downstream-port-1-1,addr=0
(qemu) info network
hub 0
 \ user.0: index=0,type=user,net=10.0.2.0,restrict=off
 \ e1000.0: index=0,type=nic,model=e1000,macaddr=52:54:00:12:34:56
nic2: index=0,type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:58
(qemu)

Best regards,
-Gonglei

> In particular, non-express devices behind an express bus
> shouldn't exist according to spec, but do in practice, and guests
> seem to be able to handle them.
> 
> > Changes since v2:
> >  - make patch 1/3 more simpler and safer.(Hu Tao)
> >  - change check logic from pci.c to pcie.c and change function's name
> >  - judge devcies' ARI capbility instead of PCIe ports' ARI Forwarding
> >    (Michael)
> >  - add trivial patch 3/3
> >  - update patch's commit messages and code comments.
> >
> > Thanks for your reviewing.
> >
> > Changes since v1:
> >  - using object_dynamic_cast() instead of simple string comparing (Paolo)
> >  - add ARI Forwarding enable bit check
> >  - using pcie_cap_get_type() instead of simple string comparing (Marcel)
> >  - fix some other comments.
> >
> > Gonglei (3):
> >   qdev: Introduce a function to get qbus's parent
> >   pcie: add check for ari capability of pcie devices
> >   pcie: remove confused comments
> >
> >  hw/core/qdev.c         |  9 ++++++++
> >  hw/pci/pci.c           |  4 ++++
> >  hw/pci/pcie.c          | 59
> +++++++++++++++++++++++++++++++++++++++++++-------
> >  include/hw/pci/pcie.h  |  1 +
> >  include/hw/qdev-core.h |  1 +
> >  5 files changed, 66 insertions(+), 8 deletions(-)
> >
> > --
> > 1.7.12.4
> >