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

Commit Message

Yubo Miao Feb. 13, 2020, 7:49 a.m. UTC

From: miaoyubo <[e-mail]>

Since devices could not directly plugged into pxb-pcie, under arm, 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.

Signed-off-by: miaoyubo <[e-mail]>
---
 hw/pci-bridge/pci_expander_bridge.c | 9 +++++++++
 include/hw/pci/pcie_port.h          | 1 +
 2 files changed, 10 insertions(+)

Comments

Daniel P. Berrangé Feb. 13, 2020, 1:51 p.m. UTC | #1

On Thu, Feb 13, 2020 at 03:49:52PM +0800, Yubo Miao wrote:
> From: miaoyubo <[e-mail]>
> 
> Since devices could not directly plugged into pxb-pcie, under arm, 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.

What is the cause of this arm specific requirement for pxb-pcie and
more importantly can be fix it so that we don't need this patch ?
I think it is highly undesirable to have such a per-arch difference
in configuration of the pxb-pcie device. It means any mgmt app
which already supports pxb-pcie will be broken and need to special
case arm.

> 
> Signed-off-by: miaoyubo <[e-mail]>
> ---
>  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
>      } 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"
>  #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
> 
> 
> 

Regards,
Daniel

Yubo Miao Feb. 14, 2020, 7:25 a.m. UTC | #2

> -----Original Message-----
> From: Daniel P. Berrangé [mailto:[e-mail]]
> Sent: Thursday, February 13, 2020 9:52 PM
> To: miaoyubo <[e-mail]>
> Cc: [e-mail]; [e-mail];
> [e-mail]; [e-mail]; Xiexiangyou
> <[e-mail]>; [e-mail]
> Subject: Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie
> under arm.
> 
> On Thu, Feb 13, 2020 at 03:49:52PM +0800, Yubo Miao wrote:
> > From: miaoyubo <[e-mail]>
> >
> > Since devices could not directly plugged into pxb-pcie, under arm, 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.
> 
> What is the cause of this arm specific requirement for pxb-pcie and more
> importantly can be fix it so that we don't need this patch ?
> I think it is highly undesirable to have such a per-arch difference in
> configuration of the pxb-pcie device. It means any mgmt app which already
> supports pxb-pcie will be broken and need to special case arm.
> 

Thanks for your reply, Without this patch, the pxb-pcie is also useable, 
however, one extra pcie-root-port or pci-bridge or something else need 
to be defined by mgmt. app. This patch will could be abandoned.

> >
> > Signed-off-by: miaoyubo <[e-mail]>
> > ---
> >  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
> >      } 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"
> >  #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
> >
> >
> >
> 
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|

Regards,
Miao

Yubo Miao Feb. 14, 2020, 7:30 a.m. UTC | #3

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:[e-mail]]
> Sent: Thursday, February 13, 2020 6:17 PM
> To: miaoyubo <[e-mail]>
> Cc: [e-mail]; [e-mail]; Xiexiangyou
> <[e-mail]>; [e-mail]; qemu-
> [e-mail]
> Subject: Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie
> under arm.
> 
> On Thu, Feb 13, 2020 at 03:49:52PM +0800, Yubo Miao wrote:
> > From: miaoyubo <[e-mail]>
> >
> > Since devices could not directly plugged into pxb-pcie,
> 
> Hmm is this different from the root port? intergrated devices do exist for
> that actually.
> 

Thanks for replying
The pxb-pcie is like a host bridge,
 you have to plug pcie-root-port or pci-bridge so devices could be plugged

> > under arm,
> 
> how is arm special?
> 

Cureently, if u define a pxb-pcie device, one pcie-root-port or pci-bridge or something 
else have to be defined also, The patch just auto generate pcie-root-port for arm to 
avoid affect other architecture

> > 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?
> 
> 

Users could specify multiply pxb-devices, it is supported.
But only one device could be plugged for each pxb-pcie,  it is the same with pxb-pci for piix.

> >
> > Signed-off-by: miaoyubo <[e-mail]>
> > ---
> >  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?
> 

Based on the comments, this patch would be abandoned in patch V2, 
PXB-PCIE would also be useful but pcie-root-port or pci-bridge have to Be defined by user.

> >      } 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.
> 

Thanks for your replay, this patch would be abandoned.

> >  #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
> >

Regards,
Miao

Daniel P. Berrangé Feb. 14, 2020, 10:24 a.m. UTC | #4

On Fri, Feb 14, 2020 at 07:25:43AM +0000, miaoyubo wrote:
> 
> > -----Original Message-----
> > From: Daniel P. Berrangé [mailto:[e-mail]]
> > Sent: Thursday, February 13, 2020 9:52 PM
> > To: miaoyubo <[e-mail]>
> > Cc: [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; Xiexiangyou
> > <[e-mail]>; [e-mail]
> > Subject: Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie
> > under arm.
> > 
> > On Thu, Feb 13, 2020 at 03:49:52PM +0800, Yubo Miao wrote:
> > > From: miaoyubo <[e-mail]>
> > >
> > > Since devices could not directly plugged into pxb-pcie, under arm, 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.
> > 
> > What is the cause of this arm specific requirement for pxb-pcie and more
> > importantly can be fix it so that we don't need this patch ?
> > I think it is highly undesirable to have such a per-arch difference in
> > configuration of the pxb-pcie device. It means any mgmt app which already
> > supports pxb-pcie will be broken and need to special case arm.
> > 
> 
> Thanks for your reply, Without this patch, the pxb-pcie is also useable, 
> however, one extra pcie-root-port or pci-bridge or something else need 
> to be defined by mgmt. app. This patch will could be abandoned.

That's not really answering my question. IIUC, this pxb-pcie device
works fine on x86_64, and I want to know why it doesn't work on arm ?
Requiring different setups by the mgmt apps is not at all nice because
it will inevitably lead to broken arm setups. x86_64 gets far more testing
& usage, developers won't realize arm is different.



Regards,
Daniel

Yubo Miao Feb. 15, 2020, 8:59 a.m. UTC | #5

> -----Original Message-----
> From: Daniel P. Berrangé [mailto:[e-mail]]
> Sent: Friday, February 14, 2020 6:25 PM
> To: miaoyubo <[e-mail]>
> Cc: [e-mail]; [e-mail];
> [e-mail]; [e-mail]; Xiexiangyou
> <[e-mail]>; [e-mail]
> Subject: Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under
> arm.
> 
> On Fri, Feb 14, 2020 at 07:25:43AM +0000, miaoyubo wrote:
> >
> > > -----Original Message-----
> > > From: Daniel P. Berrangé [mailto:[e-mail]]
> > > Sent: Thursday, February 13, 2020 9:52 PM
> > > To: miaoyubo <[e-mail]>
> > > Cc: [e-mail]; [e-mail];
> > > [e-mail]; [e-mail]; Xiexiangyou
> > > <[e-mail]>; [e-mail]
> > > Subject: Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to
> > > pxb-pcie under arm.
> > >
> > > On Thu, Feb 13, 2020 at 03:49:52PM +0800, Yubo Miao wrote:
> > > > From: miaoyubo <[e-mail]>
> > > >
> > > > Since devices could not directly plugged into pxb-pcie, under arm,
> > > > 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.
> > >
> > > What is the cause of this arm specific requirement for pxb-pcie and
> > > more importantly can be fix it so that we don't need this patch ?
> > > I think it is highly undesirable to have such a per-arch difference
> > > in configuration of the pxb-pcie device. It means any mgmt app which
> > > already supports pxb-pcie will be broken and need to special case arm.
> > >
> >
> > Thanks for your reply, Without this patch, the pxb-pcie is also
> > useable, however, one extra pcie-root-port or pci-bridge or something
> > else need to be defined by mgmt. app. This patch will could be abandoned.
> 
> That's not really answering my question. IIUC, this pxb-pcie device works fine
> on x86_64, and I want to know why it doesn't work on arm ?
> Requiring different setups by the mgmt apps is not at all nice because it will
> inevitably lead to broken arm setups. x86_64 gets far more testing & usage,
> developers won't realize arm is different.
> 
>

Thanks for replying. Currently, on x86_64, pxb-pcie devices is presented in acpi tables but on arm, It is not, only one main host bridge is presented for arm in acpi dsdt tables. That's why pxb-pcie works on x86_64 but doesn't work on arm. The patch 1/2 do the work to present and allocate resources for pxb-pcie in arm.
For x86_64, if one device is going to be plugged into pxb-pcie, one extra pcie-root-port or pci-bridge have to be defined and plugged on pxb-pcie, then the device is plugged on the pcie-root-port or pci-bridge.
This patch 2/2 just auto defined one pcie-root-port for arm. If this patch abandoned, the usage of pxb-pcie would be the same with x86_64, therefore, to keep the same step for x86 and arm, this patch 2/2 could be abandonded.

 
> 
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|

Regards,
Miao

Daniel P. Berrangé Feb. 24, 2020, 12:36 p.m. UTC | #6

On Sat, Feb 15, 2020 at 08:59:28AM +0000, miaoyubo wrote:
> 
> > -----Original Message-----
> > From: Daniel P. Berrangé [mailto:[e-mail]]
> > Sent: Friday, February 14, 2020 6:25 PM
> > To: miaoyubo <[e-mail]>
> > Cc: [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; Xiexiangyou
> > <[e-mail]>; [e-mail]
> > Subject: Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under
> > arm.
> > 
> > On Fri, Feb 14, 2020 at 07:25:43AM +0000, miaoyubo wrote:
> > >
> > > > -----Original Message-----
> > > > From: Daniel P. Berrangé [mailto:[e-mail]]
> > > > Sent: Thursday, February 13, 2020 9:52 PM
> > > > To: miaoyubo <[e-mail]>
> > > > Cc: [e-mail]; [e-mail];
> > > > [e-mail]; [e-mail]; Xiexiangyou
> > > > <[e-mail]>; [e-mail]
> > > > Subject: Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to
> > > > pxb-pcie under arm.
> > > >
> > > > On Thu, Feb 13, 2020 at 03:49:52PM +0800, Yubo Miao wrote:
> > > > > From: miaoyubo <[e-mail]>
> > > > >
> > > > > Since devices could not directly plugged into pxb-pcie, under arm,
> > > > > 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.
> > > >
> > > > What is the cause of this arm specific requirement for pxb-pcie and
> > > > more importantly can be fix it so that we don't need this patch ?
> > > > I think it is highly undesirable to have such a per-arch difference
> > > > in configuration of the pxb-pcie device. It means any mgmt app which
> > > > already supports pxb-pcie will be broken and need to special case arm.
> > > >
> > >
> > > Thanks for your reply, Without this patch, the pxb-pcie is also
> > > useable, however, one extra pcie-root-port or pci-bridge or something
> > > else need to be defined by mgmt. app. This patch will could be abandoned.
> > 
> > That's not really answering my question. IIUC, this pxb-pcie device works fine
> > on x86_64, and I want to know why it doesn't work on arm ?
> > Requiring different setups by the mgmt apps is not at all nice because it will
> > inevitably lead to broken arm setups. x86_64 gets far more testing & usage,
> > developers won't realize arm is different.
> > 
> >
> 
> Thanks for replying. Currently, on x86_64, pxb-pcie devices is presented
> in acpi tables but on arm, It is not, only one main host bridge is
> presented for arm in acpi dsdt tables. That's why pxb-pcie works on
> x86_64 but doesn't work on arm. The patch 1/2 do the work to present
> and allocate resources for pxb-pcie in arm.

Yes, this first patch makes sense

> For x86_64, if one device is going to be plugged into pxb-pcie, one
> extra pcie-root-port or pci-bridge have to be defined and plugged on
> pxb-pcie, then the device is plugged on the pcie-root-port or pci-bridge.

> This patch 2/2 just auto defined one pcie-root-port for arm. If this
> patch abandoned, the usage of pxb-pcie would be the same with x86_64,
> therefore, to keep the same step for x86 and arm, this patch 2/2 could
> be abandonded.

Yes, I think abandoning this patch 2 is best. Applications that know
how to use pxb-pcie on x86_64, will already do the right thing on
arm too, once your first patch is merged.

Regards,
Daniel

Yubo Miao Feb. 25, 2020, 1:54 a.m. UTC | #7

> -----Original Message-----
> From: Daniel P. Berrangé [mailto:[e-mail]]
> Sent: Monday, February 24, 2020 8:36 PM
> To: miaoyubo <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail];
> Xiexiangyou <[e-mail]>; [e-mail];
> [e-mail]
> Subject: Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie
> under arm.
> 
> On Sat, Feb 15, 2020 at 08:59:28AM +0000, miaoyubo wrote:
> >
> > > -----Original Message-----
> > > From: Daniel P. Berrangé [mailto:[e-mail]]
> > > Sent: Friday, February 14, 2020 6:25 PM
> > > To: miaoyubo <[e-mail]>
> > > Cc: [e-mail]; [e-mail];
> > > [e-mail]; [e-mail]; Xiexiangyou
> > > <[e-mail]>; [e-mail]
> > > Subject: Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to
> > > pxb-pcie under arm.
> > >
> > > On Fri, Feb 14, 2020 at 07:25:43AM +0000, miaoyubo wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Daniel P. Berrangé [mailto:[e-mail]]
> > > > > Sent: Thursday, February 13, 2020 9:52 PM
> > > > > To: miaoyubo <[e-mail]>
> > > > > Cc: [e-mail]; [e-mail];
> > > > > [e-mail]; [e-mail]; Xiexiangyou
> > > > > <[e-mail]>; [e-mail]
> > > > > Subject: Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to
> > > > > pxb-pcie under arm.
> > > > >
> > > > > On Thu, Feb 13, 2020 at 03:49:52PM +0800, Yubo Miao wrote:
> > > > > > From: miaoyubo <[e-mail]>
> > > > > >
> > > > > > Since devices could not directly plugged into pxb-pcie, under
> > > > > > arm, 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.
> > > > >
> > > > > What is the cause of this arm specific requirement for pxb-pcie
> > > > > and more importantly can be fix it so that we don't need this patch ?
> > > > > I think it is highly undesirable to have such a per-arch
> > > > > difference in configuration of the pxb-pcie device. It means any
> > > > > mgmt app which already supports pxb-pcie will be broken and need
> to special case arm.
> > > > >
> > > >
> > > > Thanks for your reply, Without this patch, the pxb-pcie is also
> > > > useable, however, one extra pcie-root-port or pci-bridge or
> > > > something else need to be defined by mgmt. app. This patch will could
> be abandoned.
> > >
> > > That's not really answering my question. IIUC, this pxb-pcie device
> > > works fine on x86_64, and I want to know why it doesn't work on arm ?
> > > Requiring different setups by the mgmt apps is not at all nice
> > > because it will inevitably lead to broken arm setups. x86_64 gets
> > > far more testing & usage, developers won't realize arm is different.
> > >
> > >
> >
> > Thanks for replying. Currently, on x86_64, pxb-pcie devices is
> > presented in acpi tables but on arm, It is not, only one main host
> > bridge is presented for arm in acpi dsdt tables. That's why pxb-pcie
> > works on
> > x86_64 but doesn't work on arm. The patch 1/2 do the work to present
> > and allocate resources for pxb-pcie in arm.
> 
> Yes, this first patch makes sense
> 

Thanks for the comments, the patch has been updated to v4, pls check it.

> > For x86_64, if one device is going to be plugged into pxb-pcie, one
> > extra pcie-root-port or pci-bridge have to be defined and plugged on
> > pxb-pcie, then the device is plugged on the pcie-root-port or pci-bridge.
> 
> > This patch 2/2 just auto defined one pcie-root-port for arm. If this
> > patch abandoned, the usage of pxb-pcie would be the same with x86_64,
> > therefore, to keep the same step for x86 and arm, this patch 2/2 could
> > be abandonded.
> 
> Yes, I think abandoning this patch 2 is best. Applications that know how to
> use pxb-pcie on x86_64, will already do the right thing on arm too, once your
> first patch is merged.
> 

This patch has been abandoned since v3.

> Regards,
> Daniel
> --
> |: https://berrange.com      -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|


Regards,
Miao

Patch

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
     } 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"
 #define PCIE_ROOT_PORT_CLASS(klass) \
      OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
 #define PCIE_ROOT_PORT_GET_CLASS(obj) \