Re: [Qemu-devel] [RFC v4 1/6] pci_expander_bridge: add type TYPE_PXB_PCIE_HOST
Marcel Apfelbaum 于2018年8月25日周六 下午8:08写道：
>
> Hi Zihan,
>
> On 08/19/2018 04:51 AM, Zihan Yang wrote:
> > Hi Marcel,
> >
> > Marcel Apfelbaum 于2018年8月18日周六 上午1:14写道：
> >> Hi Zihan,
> >>
> >> On 08/09/2018 09:33 AM, Zihan Yang wrote:
> >>> The inner host bridge created by pxb-pcie is TYPE_PXB_PCI_HOST by default,
> >>> change it to a new type TYPE_PXB_PCIE_HOST to better utilize ECAM of PCIe
> >>>
> >>> Signed-off-by: Zihan Yang
> >>> ---
> >>>hw/pci-bridge/pci_expander_bridge.c | 127
> >>> ++--
> >>>1 file changed, 122 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/hw/pci-bridge/pci_expander_bridge.c
> >>> b/hw/pci-bridge/pci_expander_bridge.c
> >>> index e62de42..6dd38de 100644
> >>> --- a/hw/pci-bridge/pci_expander_bridge.c
> >>> +++ b/hw/pci-bridge/pci_expander_bridge.c
> >>> @@ -15,10 +15,12 @@
> >>>#include "hw/pci/pci.h"
> >>>#include "hw/pci/pci_bus.h"
> >>>#include "hw/pci/pci_host.h"
> >>> +#include "hw/pci/pcie_host.h"
> >>>#include "hw/pci/pci_bridge.h"
> >>>#include "qemu/range.h"
> >>>#include "qemu/error-report.h"
> >>>#include "sysemu/numa.h"
> >>> +#include "qapi/visitor.h"
> >>>
> >>>#define TYPE_PXB_BUS "pxb-bus"
> >>>#define PXB_BUS(obj) OBJECT_CHECK(PXBBus, (obj), TYPE_PXB_BUS)
> >>> @@ -40,11 +42,20 @@ typedef struct PXBBus {
> >>>#define TYPE_PXB_PCIE_DEVICE "pxb-pcie"
> >>>#define PXB_PCIE_DEV(obj) OBJECT_CHECK(PXBDev, (obj),
> >>> TYPE_PXB_PCIE_DEVICE)
> >>>
> >>> +#define PROP_PXB_PCIE_DEV "pxbdev"
> >>> +
> >>> +#define PROP_PXB_PCIE_DOMAIN_NR "domain_nr"
> >>> +#define PROP_PXB_PCIE_MAX_BUS "max_bus"
> >>> +#define PROP_PXB_BUS_NR "bus_nr"
> >>> +#define PROP_PXB_NUMA_NODE "numa_node"
> >>> +
> >>>typedef struct PXBDev {
> >>>/*< private >*/
> >>>PCIDevice parent_obj;
> >>>/*< public >*/
> >>>
> >>> +uint32_t domain_nr; /* PCI domain number, non-zero means separate
> >>> domain */
> >> The commit message suggests this patch is only about
> >> re-factoring of the pxb host type, but you add here more fields.
> >> Please use two separate patches.
> >>
> >>> +uint8_t max_bus;/* max bus number to use(including this one) */
> >> That's a great idea! Limiting the max_bus will save us a lot
> >> of resource space, we will not need 256 buses on pxbs probably.
> >>
> >> My concern is what happens with the current mode.
> >> Currently bus_nr is used to divide PCI domain 0 buses between pxbs.
> >> So if you have a pxb with bus_nr 100, and another with bus_nr 200,
> >> we divide them like this:
> >> main host bridge 0...99
> >> pxb1 100 -199
> >> pxb2 200-255
> >>
> >> What will be the meaning of max_bus if we don't use the domain_nr
> >> parameter?
> >> Maybe it will mean that some bus numbers are not assigned at all, for
> >> example:
> >> pxb1: bus_nr 100, max_bus 150
> >> pxb2: bus_nr 200, max_bus 210
> >>
> >> It may work.
> > Yes, it should mean so. Actually max_bus does not have to be specified
> > if domain_nr
> > is not used, but if users decide to use domain_nr and want to save
> > space, max_bus
> > could be used.
> >
> >>>uint8_t bus_nr;
> >>>uint16_t numa_node;
> >>>} PXBDev;
> >>> @@ -58,6 +69,16 @@ static PXBDev *convert_to_pxb(PCIDevice *dev)
> >>>static GList *pxb_dev_list;
> >>>
> >>>#define TYPE_PXB_HOST "pxb-host"
> >>> +#define TYPE_PXB_PCIE_HOST "pxb-pcie-host"
> >>> +#define PXB_PCIE_HOST_DEVICE(obj) \
> >>> + OBJECT_CHECK(PXBPCIEHost, (obj), TYPE_PXB_PCIE_HOST)
> >>> +
> >>> +typedef struct PXBPCIEHost {
> >>> +PCIExpressHost parent_obj;
> >>> +
> >>> +/* pointers to PXBDev */
> >>> +PXBDev *pxbdev;
> >>> +} PXBPCIEHost;
> >>>
> >>>static int pxb_bus_num(PCIBus *bus)
> >>>{
> >>> @@ -111,6 +132,35 @@ static const char
> >>> *pxb_host_root_bus_path(PCIHostState *host_bridge,
> >>>return bus->bus_path;
> >>>}
> >>>
> >>> +/* Use a dedicated function for PCIe since pxb-host does
> >>> + * not have a domain_nr field */
> >>> +static const char *pxb_pcie_host_root_bus_path(PCIHostState *host_bridge,
> >>> + PCIBus *rootbus)
> >>> +{
> >>> +if (!pci_bus_is_express(rootbus)) {
> >>> +/* pxb-pcie-host cannot reside on a PCI bus */
> >>> +return NULL;
> >>> +}
> >>> +PXBBus *bus = PXB_PCIE_BUS(rootbus);
> >>> +
> >>> +/* get the pointer to PXBDev */
> >>> +Object *obj = object_property_get_link(OBJECT(host_bridge),
> >>> + PROP_PXB_PCIE_DEV, NULL);
> >> I don't think you need a link here.
> >> I think rootbus->parent_dev returns the pxb device.
> >> (See the implementation of pxb_bus_num() )
> > OK, I'll change it in next version.
> >
> >>> +
> >>> +snprintf(bus->bus_path, 8, "%04lx:%02x",
> >>> + object_property_get_uint(obj, PROP_PXB_PCIE_DOMAIN_NR,
> >>> NULL),
> >>> + pxb_bus_num(rootbus));
> >>> +
Re: [Qemu-devel] [RFC v4 1/6] pci_expander_bridge: add type TYPE_PXB_PCIE_HOST
Hi Zihan,
On 08/19/2018 04:51 AM, Zihan Yang wrote:
Hi Marcel,
Marcel Apfelbaum 于2018年8月18日周六 上午1:14写道：
Hi Zihan,
On 08/09/2018 09:33 AM, Zihan Yang wrote:
The inner host bridge created by pxb-pcie is TYPE_PXB_PCI_HOST by default,
change it to a new type TYPE_PXB_PCIE_HOST to better utilize ECAM of PCIe
Signed-off-by: Zihan Yang
---
hw/pci-bridge/pci_expander_bridge.c | 127
++--
1 file changed, 122 insertions(+), 5 deletions(-)
diff --git a/hw/pci-bridge/pci_expander_bridge.c
b/hw/pci-bridge/pci_expander_bridge.c
index e62de42..6dd38de 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -15,10 +15,12 @@
#include "hw/pci/pci.h"
#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_host.h"
+#include "hw/pci/pcie_host.h"
#include "hw/pci/pci_bridge.h"
#include "qemu/range.h"
#include "qemu/error-report.h"
#include "sysemu/numa.h"
+#include "qapi/visitor.h"
#define TYPE_PXB_BUS "pxb-bus"
#define PXB_BUS(obj) OBJECT_CHECK(PXBBus, (obj), TYPE_PXB_BUS)
@@ -40,11 +42,20 @@ typedef struct PXBBus {
#define TYPE_PXB_PCIE_DEVICE "pxb-pcie"
#define PXB_PCIE_DEV(obj) OBJECT_CHECK(PXBDev, (obj), TYPE_PXB_PCIE_DEVICE)
+#define PROP_PXB_PCIE_DEV "pxbdev"
+
+#define PROP_PXB_PCIE_DOMAIN_NR "domain_nr"
+#define PROP_PXB_PCIE_MAX_BUS "max_bus"
+#define PROP_PXB_BUS_NR "bus_nr"
+#define PROP_PXB_NUMA_NODE "numa_node"
+
typedef struct PXBDev {
/*< private >*/
PCIDevice parent_obj;
/*< public >*/
+uint32_t domain_nr; /* PCI domain number, non-zero means separate domain */
The commit message suggests this patch is only about
re-factoring of the pxb host type, but you add here more fields.
Please use two separate patches.
+uint8_t max_bus;/* max bus number to use(including this one) */
That's a great idea! Limiting the max_bus will save us a lot
of resource space, we will not need 256 buses on pxbs probably.
My concern is what happens with the current mode.
Currently bus_nr is used to divide PCI domain 0 buses between pxbs.
So if you have a pxb with bus_nr 100, and another with bus_nr 200,
we divide them like this:
main host bridge 0...99
pxb1 100 -199
pxb2 200-255
What will be the meaning of max_bus if we don't use the domain_nr parameter?
Maybe it will mean that some bus numbers are not assigned at all, for
example:
pxb1: bus_nr 100, max_bus 150
pxb2: bus_nr 200, max_bus 210
It may work.
Yes, it should mean so. Actually max_bus does not have to be specified
if domain_nr
is not used, but if users decide to use domain_nr and want to save
space, max_bus
could be used.
uint8_t bus_nr;
uint16_t numa_node;
} PXBDev;
@@ -58,6 +69,16 @@ static PXBDev *convert_to_pxb(PCIDevice *dev)
static GList *pxb_dev_list;
#define TYPE_PXB_HOST "pxb-host"
+#define TYPE_PXB_PCIE_HOST "pxb-pcie-host"
+#define PXB_PCIE_HOST_DEVICE(obj) \
+ OBJECT_CHECK(PXBPCIEHost, (obj), TYPE_PXB_PCIE_HOST)
+
+typedef struct PXBPCIEHost {
+PCIExpressHost parent_obj;
+
+/* pointers to PXBDev */
+PXBDev *pxbdev;
+} PXBPCIEHost;
static int pxb_bus_num(PCIBus *bus)
{
@@ -111,6 +132,35 @@ static const char *pxb_host_root_bus_path(PCIHostState
*host_bridge,
return bus->bus_path;
}
+/* Use a dedicated function for PCIe since pxb-host does
+ * not have a domain_nr field */
+static const char *pxb_pcie_host_root_bus_path(PCIHostState *host_bridge,
+ PCIBus *rootbus)
+{
+if (!pci_bus_is_express(rootbus)) {
+/* pxb-pcie-host cannot reside on a PCI bus */
+return NULL;
+}
+PXBBus *bus = PXB_PCIE_BUS(rootbus);
+
+/* get the pointer to PXBDev */
+Object *obj = object_property_get_link(OBJECT(host_bridge),
+ PROP_PXB_PCIE_DEV, NULL);
I don't think you need a link here.
I think rootbus->parent_dev returns the pxb device.
(See the implementation of pxb_bus_num() )
OK, I'll change it in next version.
+
+snprintf(bus->bus_path, 8, "%04lx:%02x",
+ object_property_get_uint(obj, PROP_PXB_PCIE_DOMAIN_NR, NULL),
+ pxb_bus_num(rootbus));
+return bus->bus_path;
+}
+
+static void pxb_pcie_host_get_mmcfg_size(Object *obj, Visitor *v, const char
*name,
+void *opaque, Error **errp)
+{
+PCIExpressHost *e = PCIE_HOST_BRIDGE(obj);
+
+visit_type_uint64(v, name, >size, errp);
+}
+
static char *pxb_host_ofw_unit_address(const SysBusDevice *dev)
{
const PCIHostState *pxb_host;
@@ -142,6 +192,31 @@ static char *pxb_host_ofw_unit_address(const SysBusDevice
*dev)
return NULL;
}
+static void pxb_pcie_host_initfn(Object *obj)
+{
+PXBPCIEHost *s = PXB_PCIE_HOST_DEVICE(obj);
+PCIHostState *phb = PCI_HOST_BRIDGE(obj);
+
+memory_region_init_io(>conf_mem, obj, _host_conf_le_ops, phb,
+ "pci-conf-idx", 4);
Re: [Qemu-devel] [RFC v4 1/6] pci_expander_bridge: add type TYPE_PXB_PCIE_HOST
Hi Marcel,
Marcel Apfelbaum 于2018年8月18日周六 上午1:14写道：
>
> Hi Zihan,
>
> On 08/09/2018 09:33 AM, Zihan Yang wrote:
> > The inner host bridge created by pxb-pcie is TYPE_PXB_PCI_HOST by default,
> > change it to a new type TYPE_PXB_PCIE_HOST to better utilize ECAM of PCIe
> >
> > Signed-off-by: Zihan Yang
> > ---
> > hw/pci-bridge/pci_expander_bridge.c | 127
> > ++--
> > 1 file changed, 122 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/pci-bridge/pci_expander_bridge.c
> > b/hw/pci-bridge/pci_expander_bridge.c
> > index e62de42..6dd38de 100644
> > --- a/hw/pci-bridge/pci_expander_bridge.c
> > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > @@ -15,10 +15,12 @@
> > #include "hw/pci/pci.h"
> > #include "hw/pci/pci_bus.h"
> > #include "hw/pci/pci_host.h"
> > +#include "hw/pci/pcie_host.h"
> > #include "hw/pci/pci_bridge.h"
> > #include "qemu/range.h"
> > #include "qemu/error-report.h"
> > #include "sysemu/numa.h"
> > +#include "qapi/visitor.h"
> >
> > #define TYPE_PXB_BUS "pxb-bus"
> > #define PXB_BUS(obj) OBJECT_CHECK(PXBBus, (obj), TYPE_PXB_BUS)
> > @@ -40,11 +42,20 @@ typedef struct PXBBus {
> > #define TYPE_PXB_PCIE_DEVICE "pxb-pcie"
> > #define PXB_PCIE_DEV(obj) OBJECT_CHECK(PXBDev, (obj),
> > TYPE_PXB_PCIE_DEVICE)
> >
> > +#define PROP_PXB_PCIE_DEV "pxbdev"
> > +
> > +#define PROP_PXB_PCIE_DOMAIN_NR "domain_nr"
> > +#define PROP_PXB_PCIE_MAX_BUS "max_bus"
> > +#define PROP_PXB_BUS_NR "bus_nr"
> > +#define PROP_PXB_NUMA_NODE "numa_node"
> > +
> > typedef struct PXBDev {
> > /*< private >*/
> > PCIDevice parent_obj;
> > /*< public >*/
> >
> > +uint32_t domain_nr; /* PCI domain number, non-zero means separate
> > domain */
>
> The commit message suggests this patch is only about
> re-factoring of the pxb host type, but you add here more fields.
> Please use two separate patches.
>
> > +uint8_t max_bus;/* max bus number to use(including this one) */
>
> That's a great idea! Limiting the max_bus will save us a lot
> of resource space, we will not need 256 buses on pxbs probably.
>
> My concern is what happens with the current mode.
> Currently bus_nr is used to divide PCI domain 0 buses between pxbs.
> So if you have a pxb with bus_nr 100, and another with bus_nr 200,
> we divide them like this:
> main host bridge 0...99
> pxb1 100 -199
> pxb2 200-255
>
> What will be the meaning of max_bus if we don't use the domain_nr parameter?
> Maybe it will mean that some bus numbers are not assigned at all, for
> example:
>pxb1: bus_nr 100, max_bus 150
>pxb2: bus_nr 200, max_bus 210
>
> It may work.
Yes, it should mean so. Actually max_bus does not have to be specified
if domain_nr
is not used, but if users decide to use domain_nr and want to save
space, max_bus
could be used.
> > uint8_t bus_nr;
> > uint16_t numa_node;
> > } PXBDev;
> > @@ -58,6 +69,16 @@ static PXBDev *convert_to_pxb(PCIDevice *dev)
> > static GList *pxb_dev_list;
> >
> > #define TYPE_PXB_HOST "pxb-host"
> > +#define TYPE_PXB_PCIE_HOST "pxb-pcie-host"
> > +#define PXB_PCIE_HOST_DEVICE(obj) \
> > + OBJECT_CHECK(PXBPCIEHost, (obj), TYPE_PXB_PCIE_HOST)
> > +
> > +typedef struct PXBPCIEHost {
> > +PCIExpressHost parent_obj;
> > +
> > +/* pointers to PXBDev */
> > +PXBDev *pxbdev;
> > +} PXBPCIEHost;
> >
> > static int pxb_bus_num(PCIBus *bus)
> > {
> > @@ -111,6 +132,35 @@ static const char *pxb_host_root_bus_path(PCIHostState
> > *host_bridge,
> > return bus->bus_path;
> > }
> >
> > +/* Use a dedicated function for PCIe since pxb-host does
> > + * not have a domain_nr field */
> > +static const char *pxb_pcie_host_root_bus_path(PCIHostState *host_bridge,
> > + PCIBus *rootbus)
> > +{
> > +if (!pci_bus_is_express(rootbus)) {
> > +/* pxb-pcie-host cannot reside on a PCI bus */
> > +return NULL;
> > +}
> > +PXBBus *bus = PXB_PCIE_BUS(rootbus);
> > +
> > +/* get the pointer to PXBDev */
> > +Object *obj = object_property_get_link(OBJECT(host_bridge),
> > + PROP_PXB_PCIE_DEV, NULL);
>
> I don't think you need a link here.
> I think rootbus->parent_dev returns the pxb device.
> (See the implementation of pxb_bus_num() )
OK, I'll change it in next version.
> > +
> > +snprintf(bus->bus_path, 8, "%04lx:%02x",
> > + object_property_get_uint(obj, PROP_PXB_PCIE_DOMAIN_NR, NULL),
> > + pxb_bus_num(rootbus));
> > +return bus->bus_path;
> > +}
> > +
> > +static void pxb_pcie_host_get_mmcfg_size(Object *obj, Visitor *v, const
> > char *name,
> > +void *opaque, Error **errp)
> > +{
> > +PCIExpressHost *e = PCIE_HOST_BRIDGE(obj);
> > +
> > +visit_type_uint64(v, name, >size, errp);
> > +}
> > +
> > static char *pxb_host_ofw_unit_address(const SysBusDevice *dev)
> > {
> > const PCIHostState
Re: [Qemu-devel] [RFC v4 1/6] pci_expander_bridge: add type TYPE_PXB_PCIE_HOST
Hi Zihan,
On 08/09/2018 09:33 AM, Zihan Yang wrote:
The inner host bridge created by pxb-pcie is TYPE_PXB_PCI_HOST by default,
change it to a new type TYPE_PXB_PCIE_HOST to better utilize ECAM of PCIe
Signed-off-by: Zihan Yang
---
hw/pci-bridge/pci_expander_bridge.c | 127 ++--
1 file changed, 122 insertions(+), 5 deletions(-)
diff --git a/hw/pci-bridge/pci_expander_bridge.c
b/hw/pci-bridge/pci_expander_bridge.c
index e62de42..6dd38de 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -15,10 +15,12 @@
#include "hw/pci/pci.h"
#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_host.h"
+#include "hw/pci/pcie_host.h"
#include "hw/pci/pci_bridge.h"
#include "qemu/range.h"
#include "qemu/error-report.h"
#include "sysemu/numa.h"
+#include "qapi/visitor.h"
#define TYPE_PXB_BUS "pxb-bus"
#define PXB_BUS(obj) OBJECT_CHECK(PXBBus, (obj), TYPE_PXB_BUS)
@@ -40,11 +42,20 @@ typedef struct PXBBus {
#define TYPE_PXB_PCIE_DEVICE "pxb-pcie"
#define PXB_PCIE_DEV(obj) OBJECT_CHECK(PXBDev, (obj), TYPE_PXB_PCIE_DEVICE)
+#define PROP_PXB_PCIE_DEV "pxbdev"
+
+#define PROP_PXB_PCIE_DOMAIN_NR "domain_nr"
+#define PROP_PXB_PCIE_MAX_BUS "max_bus"
+#define PROP_PXB_BUS_NR "bus_nr"
+#define PROP_PXB_NUMA_NODE "numa_node"
+
typedef struct PXBDev {
/*< private >*/
PCIDevice parent_obj;
/*< public >*/
+uint32_t domain_nr; /* PCI domain number, non-zero means separate domain */
The commit message suggests this patch is only about
re-factoring of the pxb host type, but you add here more fields.
Please use two separate patches.
+uint8_t max_bus;/* max bus number to use(including this one) */
That's a great idea! Limiting the max_bus will save us a lot
of resource space, we will not need 256 buses on pxbs probably.
My concern is what happens with the current mode.
Currently bus_nr is used to divide PCI domain 0 buses between pxbs.
So if you have a pxb with bus_nr 100, and another with bus_nr 200,
we divide them like this:
main host bridge 0...99
pxb1 100 -199
pxb2 200-255
What will be the meaning of max_bus if we don't use the domain_nr parameter?
Maybe it will mean that some bus numbers are not assigned at all, for
example:
pxb1: bus_nr 100, max_bus 150
pxb2: bus_nr 200, max_bus 210
It may work.
uint8_t bus_nr;
uint16_t numa_node;
} PXBDev;
@@ -58,6 +69,16 @@ static PXBDev *convert_to_pxb(PCIDevice *dev)
static GList *pxb_dev_list;
#define TYPE_PXB_HOST "pxb-host"
+#define TYPE_PXB_PCIE_HOST "pxb-pcie-host"
+#define PXB_PCIE_HOST_DEVICE(obj) \
+ OBJECT_CHECK(PXBPCIEHost, (obj), TYPE_PXB_PCIE_HOST)
+
+typedef struct PXBPCIEHost {
+PCIExpressHost parent_obj;
+
+/* pointers to PXBDev */
+PXBDev *pxbdev;
+} PXBPCIEHost;
static int pxb_bus_num(PCIBus *bus)
{
@@ -111,6 +132,35 @@ static const char *pxb_host_root_bus_path(PCIHostState
*host_bridge,
return bus->bus_path;
}
+/* Use a dedicated function for PCIe since pxb-host does
+ * not have a domain_nr field */
+static const char *pxb_pcie_host_root_bus_path(PCIHostState *host_bridge,
+ PCIBus *rootbus)
+{
+if (!pci_bus_is_express(rootbus)) {
+/* pxb-pcie-host cannot reside on a PCI bus */
+return NULL;
+}
+PXBBus *bus = PXB_PCIE_BUS(rootbus);
+
+/* get the pointer to PXBDev */
+Object *obj = object_property_get_link(OBJECT(host_bridge),
+ PROP_PXB_PCIE_DEV, NULL);
I don't think you need a link here.
I think rootbus->parent_dev returns the pxb device.
(See the implementation of pxb_bus_num() )
+
+snprintf(bus->bus_path, 8, "%04lx:%02x",
+ object_property_get_uint(obj, PROP_PXB_PCIE_DOMAIN_NR, NULL),
+ pxb_bus_num(rootbus));
+return bus->bus_path;
+}
+
+static void pxb_pcie_host_get_mmcfg_size(Object *obj, Visitor *v, const char
*name,
+void *opaque, Error **errp)
+{
+PCIExpressHost *e = PCIE_HOST_BRIDGE(obj);
+
+visit_type_uint64(v, name, >size, errp);
+}
+
static char *pxb_host_ofw_unit_address(const SysBusDevice *dev)
{
const PCIHostState *pxb_host;
@@ -142,6 +192,31 @@ static char *pxb_host_ofw_unit_address(const SysBusDevice
*dev)
return NULL;
}
+static void pxb_pcie_host_initfn(Object *obj)
+{
+PXBPCIEHost *s = PXB_PCIE_HOST_DEVICE(obj);
+PCIHostState *phb = PCI_HOST_BRIDGE(obj);
+
+memory_region_init_io(>conf_mem, obj, _host_conf_le_ops, phb,
+ "pci-conf-idx", 4);
+memory_region_init_io(>data_mem, obj, _host_data_le_ops, phb,
+ "pci-conf-data", 4);
+
I don't understand the above. Do you want the pxb to respond to
legacy PCI configuration cycles?
I don't think it will be possible since it is accessible only through
addresses
0xcf8 and 0xcfc which are already "taken" by
Re: [Qemu-devel] [RFC v4 1/6] pci_expander_bridge: add type TYPE_PXB_PCIE_HOST
On 08/09/2018 11:39 AM, Zihan Yang wrote: Eric Blake 于2018年8月9日周四 下午9:23写道： On 08/09/2018 01:33 AM, Zihan Yang wrote: The inner host bridge created by pxb-pcie is TYPE_PXB_PCI_HOST by default, change it to a new type TYPE_PXB_PCIE_HOST to better utilize ECAM of PCIe meta-comment: Your messages came through unthreaded (seven separate threads), rather than properly marked 'References:' and 'In-reply-to:' to the 0/6 cover letter <1533796115-15837-1-git-send-email-whois.zihan.y...@gmail.com>, which makes it a bit harder to keep track of the conversation when viewing mails sorted by threads with most recent activity. My mistake, I manually added the cc of cover letter, which is not covered by cccmd, but I didn't add In-reply-to of remaining mails. I will directly add an "cc: " field in cover letter and send them all in later patches to avoid breaking them up. As for this patch set, should I resend them or save it to v5, which I will try working out this weekend? At this point, it's probably still worth waiting for any comments on v4, so that v5 can fix those issues. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [RFC v4 1/6] pci_expander_bridge: add type TYPE_PXB_PCIE_HOST
Eric Blake 于2018年8月9日周四 下午9:23写道： > > On 08/09/2018 01:33 AM, Zihan Yang wrote: > > The inner host bridge created by pxb-pcie is TYPE_PXB_PCI_HOST by default, > > change it to a new type TYPE_PXB_PCIE_HOST to better utilize ECAM of PCIe > > meta-comment: > > Your messages came through unthreaded (seven separate threads), rather > than properly marked 'References:' and 'In-reply-to:' to the 0/6 cover > letter <1533796115-15837-1-git-send-email-whois.zihan.y...@gmail.com>, > which makes it a bit harder to keep track of the conversation when > viewing mails sorted by threads with most recent activity. My mistake, I manually added the cc of cover letter, which is not covered by cccmd, but I didn't add In-reply-to of remaining mails. I will directly add an "cc: " field in cover letter and send them all in later patches to avoid breaking them up. As for this patch set, should I resend them or save it to v5, which I will try working out this weekend?
Re: [Qemu-devel] [RFC v4 1/6] pci_expander_bridge: add type TYPE_PXB_PCIE_HOST
On 08/09/2018 01:33 AM, Zihan Yang wrote: The inner host bridge created by pxb-pcie is TYPE_PXB_PCI_HOST by default, change it to a new type TYPE_PXB_PCIE_HOST to better utilize ECAM of PCIe meta-comment: Your messages came through unthreaded (seven separate threads), rather than properly marked 'References:' and 'In-reply-to:' to the 0/6 cover letter <1533796115-15837-1-git-send-email-whois.zihan.y...@gmail.com>, which makes it a bit harder to keep track of the conversation when viewing mails sorted by threads with most recent activity. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Qemu-devel] [RFC v4 1/6] pci_expander_bridge: add type TYPE_PXB_PCIE_HOST
The inner host bridge created by pxb-pcie is TYPE_PXB_PCI_HOST by default,
change it to a new type TYPE_PXB_PCIE_HOST to better utilize ECAM of PCIe
Signed-off-by: Zihan Yang
---
hw/pci-bridge/pci_expander_bridge.c | 127 ++--
1 file changed, 122 insertions(+), 5 deletions(-)
diff --git a/hw/pci-bridge/pci_expander_bridge.c
b/hw/pci-bridge/pci_expander_bridge.c
index e62de42..6dd38de 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -15,10 +15,12 @@
#include "hw/pci/pci.h"
#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_host.h"
+#include "hw/pci/pcie_host.h"
#include "hw/pci/pci_bridge.h"
#include "qemu/range.h"
#include "qemu/error-report.h"
#include "sysemu/numa.h"
+#include "qapi/visitor.h"
#define TYPE_PXB_BUS "pxb-bus"
#define PXB_BUS(obj) OBJECT_CHECK(PXBBus, (obj), TYPE_PXB_BUS)
@@ -40,11 +42,20 @@ typedef struct PXBBus {
#define TYPE_PXB_PCIE_DEVICE "pxb-pcie"
#define PXB_PCIE_DEV(obj) OBJECT_CHECK(PXBDev, (obj), TYPE_PXB_PCIE_DEVICE)
+#define PROP_PXB_PCIE_DEV "pxbdev"
+
+#define PROP_PXB_PCIE_DOMAIN_NR "domain_nr"
+#define PROP_PXB_PCIE_MAX_BUS "max_bus"
+#define PROP_PXB_BUS_NR "bus_nr"
+#define PROP_PXB_NUMA_NODE "numa_node"
+
typedef struct PXBDev {
/*< private >*/
PCIDevice parent_obj;
/*< public >*/
+uint32_t domain_nr; /* PCI domain number, non-zero means separate domain */
+uint8_t max_bus;/* max bus number to use(including this one) */
uint8_t bus_nr;
uint16_t numa_node;
} PXBDev;
@@ -58,6 +69,16 @@ static PXBDev *convert_to_pxb(PCIDevice *dev)
static GList *pxb_dev_list;
#define TYPE_PXB_HOST "pxb-host"
+#define TYPE_PXB_PCIE_HOST "pxb-pcie-host"
+#define PXB_PCIE_HOST_DEVICE(obj) \
+ OBJECT_CHECK(PXBPCIEHost, (obj), TYPE_PXB_PCIE_HOST)
+
+typedef struct PXBPCIEHost {
+PCIExpressHost parent_obj;
+
+/* pointers to PXBDev */
+PXBDev *pxbdev;
+} PXBPCIEHost;
static int pxb_bus_num(PCIBus *bus)
{
@@ -111,6 +132,35 @@ static const char *pxb_host_root_bus_path(PCIHostState
*host_bridge,
return bus->bus_path;
}
+/* Use a dedicated function for PCIe since pxb-host does
+ * not have a domain_nr field */
+static const char *pxb_pcie_host_root_bus_path(PCIHostState *host_bridge,
+ PCIBus *rootbus)
+{
+if (!pci_bus_is_express(rootbus)) {
+/* pxb-pcie-host cannot reside on a PCI bus */
+return NULL;
+}
+PXBBus *bus = PXB_PCIE_BUS(rootbus);
+
+/* get the pointer to PXBDev */
+Object *obj = object_property_get_link(OBJECT(host_bridge),
+ PROP_PXB_PCIE_DEV, NULL);
+
+snprintf(bus->bus_path, 8, "%04lx:%02x",
+ object_property_get_uint(obj, PROP_PXB_PCIE_DOMAIN_NR, NULL),
+ pxb_bus_num(rootbus));
+return bus->bus_path;
+}
+
+static void pxb_pcie_host_get_mmcfg_size(Object *obj, Visitor *v, const char
*name,
+void *opaque, Error **errp)
+{
+PCIExpressHost *e = PCIE_HOST_BRIDGE(obj);
+
+visit_type_uint64(v, name, >size, errp);
+}
+
static char *pxb_host_ofw_unit_address(const SysBusDevice *dev)
{
const PCIHostState *pxb_host;
@@ -142,6 +192,31 @@ static char *pxb_host_ofw_unit_address(const SysBusDevice
*dev)
return NULL;
}
+static void pxb_pcie_host_initfn(Object *obj)
+{
+PXBPCIEHost *s = PXB_PCIE_HOST_DEVICE(obj);
+PCIHostState *phb = PCI_HOST_BRIDGE(obj);
+
+memory_region_init_io(>conf_mem, obj, _host_conf_le_ops, phb,
+ "pci-conf-idx", 4);
+memory_region_init_io(>data_mem, obj, _host_data_le_ops, phb,
+ "pci-conf-data", 4);
+
+object_property_add(obj, PCIE_HOST_MCFG_SIZE, "uint64",
+ pxb_pcie_host_get_mmcfg_size,
+ NULL, NULL, NULL, NULL);
+
+object_property_add_link(obj, PROP_PXB_PCIE_DEV, TYPE_PXB_PCIE_DEVICE,
+ (Object **)>pxbdev,
+ qdev_prop_allow_set_link_before_realize, 0, NULL);
+}
+
+static Property pxb_pcie_host_props[] = {
+DEFINE_PROP_UINT64(PCIE_HOST_MCFG_BASE, PXBPCIEHost, parent_obj.base_addr,
+PCIE_BASE_ADDR_UNMAPPED),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static void pxb_host_class_init(ObjectClass *class, void *data)
{
DeviceClass *dc = DEVICE_CLASS(class);
@@ -155,12 +230,34 @@ static void pxb_host_class_init(ObjectClass *class, void
*data)
hc->root_bus_path = pxb_host_root_bus_path;
}
+static void pxb_pcie_host_class_init(ObjectClass *class, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(class);
+SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(class);
+PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(class);
+
+dc->fw_name = "pcie";
+dc->props = pxb_pcie_host_props;
+/* Reason: Internal part of the pxb/pxb-pcie device, not usable by itself
*/
+dc->user_creatable = false;
+
