Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm.

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Feb 13, 2020 at 03:49:52PM +0800, Yubo Miao wrote:
> From: miaoyubo <miaoyubo@huawei.com>
> 
> Since devices could not directly plugged into pxb-pcie,

Hmm is this different from the root port? intergrated devices
do exist for that actually.

> under arm,

how is arm special?

> one
> pcie-root port is plugged into pxb-pcie. Due to the bus for each pxb-pcie
> is defined as 2 in acpi dsdt tables(one for pxb-pcie, one for pcie-root-port),
> only one device could be plugged into one pxb-pcie.

So why can't we have users specify any number of root ports using
-device? then make acpi tables match the # of ports created?


> 
> Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> ---
>  hw/pci-bridge/pci_expander_bridge.c | 9 +++++++++
>  include/hw/pci/pcie_port.h          | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 47aaaf8fd1..3d896dd452 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -15,6 +15,7 @@
>  #include "hw/pci/pci.h"
>  #include "hw/pci/pci_bus.h"
>  #include "hw/pci/pci_host.h"
> +#include "hw/pci/pcie_port.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/pci/pci_bridge.h"
>  #include "qemu/range.h"
> @@ -233,7 +234,15 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>  
>      ds = qdev_create(NULL, TYPE_PXB_HOST);
>      if (pcie) {
> +#ifdef __aarch64__
> +        bus = pci_root_bus_new(ds, "pxb-pcie-internal",
> +                               NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> +        bds = qdev_create(BUS(bus), "pcie-root-port");
> +        bds->id = dev_name;
> +        qdev_prop_set_uint8(bds, PCIE_ROOT_PORT_PROP_CHASSIS, pxb->bus_nr);
> +#else
>          bus = pci_root_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> +#endif

What does all this have to do with building on aarch64?

>      } else {
>          bus = pci_root_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
>          bds = qdev_create(BUS(bus), "pci-bridge");
> diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
> index 4b3d254b08..b41d473220 100644
> --- a/include/hw/pci/pcie_port.h
> +++ b/include/hw/pci/pcie_port.h
> @@ -64,6 +64,7 @@ int pcie_chassis_add_slot(struct PCIESlot *slot);
>  void pcie_chassis_del_slot(PCIESlot *s);
>  
>  #define TYPE_PCIE_ROOT_PORT         "pcie-root-port-base"
> +#define PCIE_ROOT_PORT_PROP_CHASSIS "chassis"

If you are going to do this, replace other instances of "chassis"
with the macro.

>  #define PCIE_ROOT_PORT_CLASS(klass) \
>       OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
>  #define PCIE_ROOT_PORT_GET_CLASS(obj) \
> -- 
> 2.19.1
>