[Qemu-devel] [PATCH 0/2] add check for PCIe root ports and downstream ports

[Qemu-devel] [PATCH 0/2] add check for PCIe root ports and downstream ports

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

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
supported, regardless of hotplug or coldplug.

So, adding check about slot 0 for PCIe downstream ports and
root ports, which avoid useless operation, both hotplug and
coldplug.

Gonglei (2):
  qdev: Introduce a function to get qbus's parent
  pci: add check for pcie root ports and downstream ports

 hw/core/qdev.c         |  7 +++++++
 hw/pci/pci.c           | 41 +++++++++++++++++++++++++++++++++++++++++
 include/hw/qdev-core.h |  1 +
 3 files changed, 49 insertions(+)

-- 
1.7.12.4

[Qemu-devel] [PATCH 1/2] qdev: Introduce a function to get qbus's parent

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

We need to use qbus's parent and get its name.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/core/qdev.c         | 7 +++++++
 include/hw/qdev-core.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index da1ba48..5c49e08 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -616,6 +616,13 @@ BusState *qbus_create(const char *typename, DeviceState *parent, const char *nam
     return bus;
 }
 
+DeviceState *qbus_get_parent(BusState *bus)
+{
+    DeviceState *parent = bus->parent;
+
+    return parent;
+}
+
 static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev)
 {
     BusClass *bc = BUS_GET_CLASS(bus);
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 0799ff2..760e726 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -301,6 +301,7 @@ typedef int (qdev_walkerfn)(DeviceState *dev, void *opaque);
 void qbus_create_inplace(void *bus, size_t size, const char *typename,
                          DeviceState *parent, const char *name);
 BusState *qbus_create(const char *typename, DeviceState *parent, const char *name);
+DeviceState *qbus_get_parent(BusState *bus);
 /* Returns > 0 if either devfn or busfn skip walk somewhere in cursion,
  *         < 0 if either devfn or busfn terminate walk somewhere in cursion,
  *           0 otherwise. */
-- 
1.7.12.4

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

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Right now, ARI Forwarding dose not support in QEMU.
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 <arei.gonglei@huawei.com>
---
 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)) {
+        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
+         * supported, regardless of hotplug or coldplug.
+         */
+        if (!strcmp(name, "ioh3420") || !strcmp(name, "xio3130-downstream")) {
+            if (PCI_SLOT(dev->devfn) != 0) {
+                error_report("Unsupported PCI slot %d for %s ports, only "
+                             "supported slot 0", PCI_SLOT(dev->devfn), name);
+                return -1;
+            }
+        }
+    }
+
+    return 0;
+}
+
 static void pci_config_alloc(PCIDevice *pci_dev)
 {
     int config_size = pci_config_size(pci_dev);
@@ -871,6 +907,11 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
         return NULL;
     }
 
+    if (pci_check_pcie_port(bus, pci_dev)) {
+        do_pci_unregister_device(pci_dev);
+        return NULL;
+    }
+
     if (!config_read)
         config_read = pci_default_read_config;
     if (!config_write)
-- 
1.7.12.4

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

[Marcel Apfelbaum]

On Tue, 2014-08-19 at 21:08 +0800, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
Hi,

> 
> Right now, ARI Forwarding dose not support in QEMU.
I would replace the above sentence with "ARI Forwarding is not supported".

By the way, there is some support for ARI, I don't know if
is enabled yet. I'll have a look.

> 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 <arei.gonglei@huawei.com>
> ---
>  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?
 
> +        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"

> +         * 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.

> +            if (PCI_SLOT(dev->devfn) != 0) {
> +                error_report("Unsupported PCI slot %d for %s ports, only "
> +                             "supported slot 0", PCI_SLOT(dev->devfn), name);
> +                return -1;
> +            }
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  static void pci_config_alloc(PCIDevice *pci_dev)
>  {
>      int config_size = pci_config_size(pci_dev);
> @@ -871,6 +907,11 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>          return NULL;
>      }
>  
> +    if (pci_check_pcie_port(bus, pci_dev)) {
> +        do_pci_unregister_device(pci_dev);
> +        return NULL;
> +    }
It is possible to move the above check earlier in do_pci_register_device function?
Maybe move it into the first if statement(s)?

Thanks,
Marcel

> +
>      if (!config_read)
>          config_read = pci_default_read_config;
>      if (!config_write)

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

[Paolo Bonzini]

Il 19/08/2014 16:37, Marcel Apfelbaum ha scritto:
>> > +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?
>  

It's object_dynamic_cast.

Paolo

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

[Gonglei (Arei)]

> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: Tuesday, August 19, 2014 11:09 PM
> To: Marcel Apfelbaum; Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; mst@redhat.com; Huangweidong (C);
> afaerber@suse.de; imammedo@redhat.com; peter.crosthwaite@xilinx.com;
> Huangpeng (Peter); armbru@redhat.com; Luonengjun
> Subject: Re: [PATCH 2/2] pci: add check for pcie root ports and downstream
> ports
> 
> Il 19/08/2014 16:37, Marcel Apfelbaum ha scritto:
> >> > +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?
> >
> 
> It's object_dynamic_cast.
> 
> Paolo

Great! Thanks. :)

Best regards,
-Gonglei

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

[Gonglei (Arei)]

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 <arei.gonglei@huawei.com>
> > ---
> >  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.

> > +            if (PCI_SLOT(dev->devfn) != 0) {
> > +                error_report("Unsupported PCI slot %d for %s ports,
> only "
> > +                             "supported slot 0",
> PCI_SLOT(dev->devfn), name);
> > +                return -1;
> > +            }
> > +        }
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >  static void pci_config_alloc(PCIDevice *pci_dev)
> >  {
> >      int config_size = pci_config_size(pci_dev);
> > @@ -871,6 +907,11 @@ static PCIDevice
> *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> >          return NULL;
> >      }
> >
> > +    if (pci_check_pcie_port(bus, pci_dev)) {
> > +        do_pci_unregister_device(pci_dev);
> > +        return NULL;
> > +    }
> It is possible to move the above check earlier in do_pci_register_device
> function?
> Maybe move it into the first if statement(s)?
> 
Agreed, Thanks, Marcel.

Best regards,
-Gonglei

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

[Marcel Apfelbaum]

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 <arei.gonglei@huawei.com>
> > > ---
> > >  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) {
        ...
    }

Thanks,
Marcel

> 
> > > +            if (PCI_SLOT(dev->devfn) != 0) {
> > > +                error_report("Unsupported PCI slot %d for %s ports,
> > only "
> > > +                             "supported slot 0",
> > PCI_SLOT(dev->devfn), name);
> > > +                return -1;
> > > +            }
> > > +        }
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > > +
> > >  static void pci_config_alloc(PCIDevice *pci_dev)
> > >  {
> > >      int config_size = pci_config_size(pci_dev);
> > > @@ -871,6 +907,11 @@ static PCIDevice
> > *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> > >          return NULL;
> > >      }
> > >
> > > +    if (pci_check_pcie_port(bus, pci_dev)) {
> > > +        do_pci_unregister_device(pci_dev);
> > > +        return NULL;
> > > +    }
> > It is possible to move the above check earlier in do_pci_register_device
> > function?
> > Maybe move it into the first if statement(s)?
> > 
> Agreed, Thanks, Marcel.
> 
> Best regards,
> -Gonglei

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

[Gonglei (Arei)]

> From: Marcel Apfelbaum [mailto:marcel.a@redhat.com]
> 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 <arei.gonglei@huawei.com>
> > > > ---
> > > >  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

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

[Michael S. Tsirkin]

On Tue, Aug 19, 2014 at 09:08:25PM +0800, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> 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
> supported, regardless of hotplug or coldplug.

I'd rather we added ARI support in bridges (if that's
what you mean) than APIs for the sole purpose of
disabling functionality we'll want to re-enable later.

We have the beginning of an implementation for ARI (pcie_ari_init).



> So, adding check about slot 0 for PCIe downstream ports and
> root ports, which avoid useless operation, both hotplug and
> coldplug.
> 
> Gonglei (2):
>   qdev: Introduce a function to get qbus's parent
>   pci: add check for pcie root ports and downstream ports
> 
>  hw/core/qdev.c         |  7 +++++++
>  hw/pci/pci.c           | 41 +++++++++++++++++++++++++++++++++++++++++
>  include/hw/qdev-core.h |  1 +
>  3 files changed, 49 insertions(+)
> 
> -- 
> 1.7.12.4
> 

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

[Gonglei (Arei)]

Hi,

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> On Tue, Aug 19, 2014 at 09:08:25PM +0800, arei.gonglei@huawei.com wrote:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> > 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
> > supported, regardless of hotplug or coldplug.
> 
> I'd rather we added ARI support in bridges (if that's
> what you mean) than APIs for the sole purpose of
> disabling functionality we'll want to re-enable later.
> 
Yes, we can add ARI support. But I don't think this check is 
superfluous. Because if some bridges don't enable this function,
we still need this check, just add more check conditions, such as check 
the bit of ARI capability be set or not.

> We have the beginning of an implementation for ARI (pcie_ari_init).
> 
Thanks for your reminding, MST.

Best regards,
-Gonglei