Re: [Qemu-devel] [PATCH 2/2] pci: add check for pcie root ports and downstream ports

[Gonglei (Arei)]

> From: Marcel Apfelbaum [mailto:address@hidden
> Subject: Re: [PATCH 2/2] pci: add check for pcie root ports and downstream
> ports
> 
> On Wed, 2014-08-20 at 03:20 +0000, Gonglei (Arei) wrote:
> > Hi,
> >
> > > > Right now, ARI Forwarding dose not support in QEMU.
> > > I would replace the above sentence with "ARI Forwarding is not supported".
> > >
> > OK.
> >
> > > By the way, there is some support for ARI, I don't know if
> > > is enabled yet. I'll have a look.
> > >
> > MST had pointed out the pcie_ari_init(), but not completed.
> >
> > > > According to PCIe spec section 7.3.1, only slot 0 with
> > > > the device attached to logic bus representing the link from
> > > > downstream ports and root ports.
> > > >
> > > > So, adding check about slot 0 for PCIe downstream ports and
> > > > root ports, which avoid useless operation, both hotplug and
> > > > coldplug.
> > > >
> > > > Signed-off-by: Gonglei <address@hidden>
> > > > ---
> > > >  hw/pci/pci.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 41 insertions(+)
> > > >
> > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > > index 351d320..f2d267f 100644
> > > > --- a/hw/pci/pci.c
> > > > +++ b/hw/pci/pci.c
> > > > @@ -773,6 +773,42 @@ static int pci_init_multifunction(PCIBus *bus,
> > > PCIDevice *dev)
> > > >      return 0;
> > > >  }
> > > >
> > > > +static int pci_check_pcie_port(PCIBus *bus, PCIDevice *dev)
> > > > +{
> > > > +    Object *obj = OBJECT(bus);
> > > > +
> > > > +    if (!strcmp(object_get_typename(obj), TYPE_PCIE_BUS)) {
> > > Maybe there is another way to check that this is a PCIe bus?
> > >
> > Yes. Paolo has said that object_dynamic_cast() is appropriated.
> >
> > > > +        DeviceState *parent = qbus_get_parent(BUS(obj));
> > > > +        const char *name = object_get_typename(OBJECT(parent));
> > > > +
> > > > +        /*
> > > > +         * 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.
> > > > +         *
> > > > +         * Right now, ARI Forwarding dose not support. So, only slot 0
> is
> > > As above, maybe replace it with "ARI Forwarding is not supported"
> > >
> > OK.
> >
> > > > +         * supported, regardless of hotplug or coldplug.
> > > > +         */
> > > > +        if (!strcmp(name, "ioh3420") || !strcmp(name,
> > > "xio3130-downstream")) {
> > > Please use port_type flag from extended configuration space, don't use
> device
> > > names.
> > > If you need help for this, let me know.
> > >
> > Yes, please. I appreciate very much that you can help me.
> Sure,
> 
> I checked and we already have the pcie_cap_get_type function that returns the
> port type.
> 
>     port_type = pcie_cap_get_type(dev);
>     if (port_type == PCI_EXP_TYPE_DOWNSTREAM ||
>         port_type == PCI_EXP_TYPE_ROOT_PORT) {
>         ...
>     }
> 
Great! Thanks a lot. :)

Best regards,
-Gonglei