* [Qemu-devel] [PATCH V2 1/3] hw/pcie: Introduce a base class for PCI Express Root Ports
2017-01-11 12:18 [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port Marcel Apfelbaum
@ 2017-01-11 12:18 ` Marcel Apfelbaum
2017-01-12 15:36 ` Michael S. Tsirkin
2017-01-11 12:18 ` [Qemu-devel] [PATCH V2 2/3] hw/ioh3420: derive from PCI Express Root Port base class Marcel Apfelbaum
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Marcel Apfelbaum @ 2017-01-11 12:18 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel
The 'base' PCI Express Root Port includes
the common code to be re-used for all
Root Ports implementations. Most of the code
was taken from the current implementation
of Intel's IOH 3420 Root Port.
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
default-configs/arm-softmmu.mak | 1 +
default-configs/i386-softmmu.mak | 1 +
default-configs/x86_64-softmmu.mak | 1 +
hw/pci-bridge/Makefile.objs | 1 +
hw/pci-bridge/pcie_root_port.c | 194 +++++++++++++++++++++++++++++++++++++
include/hw/pci/pcie_port.h | 18 ++++
6 files changed, 216 insertions(+)
create mode 100644 hw/pci-bridge/pcie_root_port.c
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 6de3e16..6f2a180 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -108,6 +108,7 @@ CONFIG_FSL_IMX25=y
CONFIG_IMX_I2C=y
+CONFIG_PCIE_PORT=y
CONFIG_XIO3130=y
CONFIG_IOH3420=y
CONFIG_I82801B11=y
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 0b51360..9288838 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -51,6 +51,7 @@ CONFIG_PVPANIC=y
CONFIG_MEM_HOTPLUG=y
CONFIG_NVDIMM=y
CONFIG_ACPI_NVDIMM=y
+CONFIG_PCIE_PORT=y
CONFIG_XIO3130=y
CONFIG_IOH3420=y
CONFIG_I82801B11=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 7f89503..7d2c2d4 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -51,6 +51,7 @@ CONFIG_PVPANIC=y
CONFIG_MEM_HOTPLUG=y
CONFIG_NVDIMM=y
CONFIG_ACPI_NVDIMM=y
+CONFIG_PCIE_PORT=y
CONFIG_XIO3130=y
CONFIG_IOH3420=y
CONFIG_I82801B11=y
diff --git a/hw/pci-bridge/Makefile.objs b/hw/pci-bridge/Makefile.objs
index f2adfe3..4f2039f 100644
--- a/hw/pci-bridge/Makefile.objs
+++ b/hw/pci-bridge/Makefile.objs
@@ -1,5 +1,6 @@
common-obj-y += pci_bridge_dev.o
common-obj-y += pci_expander_bridge.o
+common-obj-$(CONFIG_PCIE_PORT) += pcie_root_port.o
common-obj-$(CONFIG_XIO3130) += xio3130_upstream.o xio3130_downstream.o
common-obj-$(CONFIG_IOH3420) += ioh3420.o
common-obj-$(CONFIG_I82801B11) += i82801b11.o
diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
new file mode 100644
index 0000000..e84ae14
--- /dev/null
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -0,0 +1,194 @@
+/*
+ * Generic PCI Express Root Port emulation
+ *
+ * Copyright (C) 2016 Red Hat Inc
+ *
+ * Authors:
+ * Marcel Apfelbaum <marcel@redhat.com>
+ *
+ * Most of the code was migrated from hw/pci-bridge/ioh3420.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/pci/pcie_port.h"
+#include "hw/pci/msi.h"
+
+#define PCIE_ROOT_PORT_MSI_VECTORS 2
+#define PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS PCI_MSI_FLAGS_MASKBIT
+#define PCIE_ROOT_PORT_AER_OFFSET 0x100
+
+/*
+ * If two MSI vector are allocated, Advanced Error Interrupt Message Number
+ * is 1. otherwise 0.
+ * 17.12.5.10 RPERRSTS, 32:27 bit Advanced Error Interrupt Message Number.
+ */
+static uint8_t rp_aer_vector(const PCIDevice *d)
+{
+ switch (msi_nr_vectors_allocated(d)) {
+ case 1:
+ return 0;
+ case 2:
+ return 1;
+ case 4:
+ case 8:
+ case 16:
+ case 32:
+ default:
+ break;
+ }
+ abort();
+ return 0;
+}
+
+static void rp_aer_vector_update(PCIDevice *d)
+{
+ pcie_aer_root_set_vector(d, rp_aer_vector(d));
+}
+
+static void rp_write_config(PCIDevice *d, uint32_t address,
+ uint32_t val, int len)
+{
+ uint32_t root_cmd =
+ pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_ROOT_COMMAND);
+
+ pci_bridge_write_config(d, address, val, len);
+ rp_aer_vector_update(d);
+ pcie_cap_slot_write_config(d, address, val, len);
+ pcie_aer_write_config(d, address, val, len);
+ pcie_aer_root_write_config(d, address, val, len, root_cmd);
+}
+
+static void rp_reset(DeviceState *qdev)
+{
+ PCIDevice *d = PCI_DEVICE(qdev);
+
+ rp_aer_vector_update(d);
+ pcie_cap_root_reset(d);
+ pcie_cap_deverr_reset(d);
+ pcie_cap_slot_reset(d);
+ pcie_cap_arifwd_reset(d);
+ pcie_aer_root_reset(d);
+ pci_bridge_reset(qdev);
+ pci_bridge_disable_base_limit(d);
+}
+
+static void rp_realize(PCIDevice *d, Error **errp)
+{
+ PCIEPort *p = PCIE_PORT(d);
+ PCIESlot *s = PCIE_SLOT(d);
+ PCIDeviceClass *dc = PCI_DEVICE_GET_CLASS(d);
+ PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
+ int rc;
+ Error *local_err = NULL;
+
+ pci_config_set_interrupt_pin(d->config, 1);
+ pci_bridge_initfn(d, TYPE_PCIE_BUS);
+ pcie_port_init_reg(d);
+
+ rc = pci_bridge_ssvid_init(d, rpc->ssvid_offset, dc->vendor_id, rpc->ssid);
+ if (rc < 0) {
+ error_setg(errp, "Can't init SSV ID, error %d", rc);
+ goto err_bridge;
+ }
+
+ rc = msi_init(d, rpc->msi_offset, PCIE_ROOT_PORT_MSI_VECTORS,
+ PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
+ PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT,
+ &local_err);
+ if (rc < 0) {
+ assert(rc == -ENOTSUP);
+ error_propagate(errp, local_err);
+ goto err_bridge;
+ }
+
+ rc = pcie_cap_init(d, rpc->exp_offset, PCI_EXP_TYPE_ROOT_PORT, p->port);
+ if (rc < 0) {
+ error_setg(errp, "Can't add Root Port capability, error %d", rc);
+ goto err_msi;
+ }
+
+ pcie_cap_arifwd_init(d);
+ pcie_cap_deverr_init(d);
+ pcie_cap_slot_init(d, s->slot);
+ pcie_cap_root_init(d);
+
+ pcie_chassis_create(s->chassis);
+ rc = pcie_chassis_add_slot(s);
+ if (rc < 0) {
+ error_setg(errp, "Can't add chassis slot, error %d", rc);
+ goto err_pcie_cap;
+ }
+
+ rc = pcie_aer_init(d, PCI_ERR_VER, rpc->aer_offset,
+ PCI_ERR_SIZEOF, &local_err);
+ if (rc < 0) {
+ error_propagate(errp, local_err);
+ goto err;
+ }
+ pcie_aer_root_init(d);
+ rp_aer_vector_update(d);
+
+ return;
+
+err:
+ pcie_chassis_del_slot(s);
+err_pcie_cap:
+ pcie_cap_exit(d);
+err_msi:
+ msi_uninit(d);
+err_bridge:
+ pci_bridge_exitfn(d);
+}
+
+static void rp_exit(PCIDevice *d)
+{
+ PCIESlot *s = PCIE_SLOT(d);
+
+ pcie_aer_exit(d);
+ pcie_chassis_del_slot(s);
+ pcie_cap_exit(d);
+ msi_uninit(d);
+ pci_bridge_exitfn(d);
+}
+
+static Property rp_props[] = {
+ DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
+ QEMU_PCIE_SLTCAP_PCP_BITNR, true),
+ DEFINE_PROP_END_OF_LIST()
+};
+
+static void rp_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
+
+ k->is_express = 1;
+ k->is_bridge = 1;
+ k->config_write = rp_write_config;
+ k->realize = rp_realize;
+ k->exit = rp_exit;
+ set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+ dc->reset = rp_reset;
+ dc->props = rp_props;
+ rpc->aer_offset = PCIE_ROOT_PORT_AER_OFFSET;
+}
+
+static const TypeInfo rp_info = {
+ .name = TYPE_PCIE_ROOT_PORT,
+ .parent = TYPE_PCIE_SLOT,
+ .class_init = rp_class_init,
+ .abstract = true,
+ .class_size = sizeof(PCIERootPortClass),
+};
+
+static void rp_register_types(void)
+{
+ type_register_static(&rp_info);
+}
+
+type_init(rp_register_types)
diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
index f7b64db..a8eee56 100644
--- a/include/hw/pci/pcie_port.h
+++ b/include/hw/pci/pcie_port.h
@@ -57,4 +57,22 @@ PCIESlot *pcie_chassis_find_slot(uint8_t chassis, uint16_t slot);
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_CLASS(klass) \
+ OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
+#define PCIE_ROOT_PORT_CLASS(klass) \
+ OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
+#define PCIE_ROOT_PORT_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(PCIERootPortClass, (obj), TYPE_PCIE_ROOT_PORT)
+
+typedef struct PCIERootPortClass {
+ PCIDeviceClass parent_class;
+
+ int msi_offset;
+ int exp_offset;
+ int aer_offset;
+ int ssvid_offset;
+ int ssid;
+} PCIERootPortClass;
+
#endif /* QEMU_PCIE_PORT_H */
--
2.5.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH V2 1/3] hw/pcie: Introduce a base class for PCI Express Root Ports
2017-01-11 12:18 ` [Qemu-devel] [PATCH V2 1/3] hw/pcie: Introduce a base class for PCI Express Root Ports Marcel Apfelbaum
@ 2017-01-12 15:36 ` Michael S. Tsirkin
2017-01-12 15:51 ` Marcel Apfelbaum
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2017-01-12 15:36 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel
On Wed, Jan 11, 2017 at 02:18:54PM +0200, Marcel Apfelbaum wrote:
> The 'base' PCI Express Root Port includes
> the common code to be re-used for all
> Root Ports implementations. Most of the code
> was taken from the current implementation
> of Intel's IOH 3420 Root Port.
>
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
> default-configs/arm-softmmu.mak | 1 +
> default-configs/i386-softmmu.mak | 1 +
> default-configs/x86_64-softmmu.mak | 1 +
> hw/pci-bridge/Makefile.objs | 1 +
> hw/pci-bridge/pcie_root_port.c | 194 +++++++++++++++++++++++++++++++++++++
> include/hw/pci/pcie_port.h | 18 ++++
> 6 files changed, 216 insertions(+)
> create mode 100644 hw/pci-bridge/pcie_root_port.c
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 6de3e16..6f2a180 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -108,6 +108,7 @@ CONFIG_FSL_IMX25=y
>
> CONFIG_IMX_I2C=y
>
> +CONFIG_PCIE_PORT=y
> CONFIG_XIO3130=y
> CONFIG_IOH3420=y
> CONFIG_I82801B11=y
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index 0b51360..9288838 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -51,6 +51,7 @@ CONFIG_PVPANIC=y
> CONFIG_MEM_HOTPLUG=y
> CONFIG_NVDIMM=y
> CONFIG_ACPI_NVDIMM=y
> +CONFIG_PCIE_PORT=y
> CONFIG_XIO3130=y
> CONFIG_IOH3420=y
> CONFIG_I82801B11=y
> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> index 7f89503..7d2c2d4 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -51,6 +51,7 @@ CONFIG_PVPANIC=y
> CONFIG_MEM_HOTPLUG=y
> CONFIG_NVDIMM=y
> CONFIG_ACPI_NVDIMM=y
> +CONFIG_PCIE_PORT=y
> CONFIG_XIO3130=y
> CONFIG_IOH3420=y
> CONFIG_I82801B11=y
> diff --git a/hw/pci-bridge/Makefile.objs b/hw/pci-bridge/Makefile.objs
> index f2adfe3..4f2039f 100644
> --- a/hw/pci-bridge/Makefile.objs
> +++ b/hw/pci-bridge/Makefile.objs
> @@ -1,5 +1,6 @@
> common-obj-y += pci_bridge_dev.o
> common-obj-y += pci_expander_bridge.o
> +common-obj-$(CONFIG_PCIE_PORT) += pcie_root_port.o
> common-obj-$(CONFIG_XIO3130) += xio3130_upstream.o xio3130_downstream.o
> common-obj-$(CONFIG_IOH3420) += ioh3420.o
> common-obj-$(CONFIG_I82801B11) += i82801b11.o
> diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
> new file mode 100644
> index 0000000..e84ae14
> --- /dev/null
> +++ b/hw/pci-bridge/pcie_root_port.c
> @@ -0,0 +1,194 @@
> +/*
> + * Generic PCI Express Root Port emulation
> + *
> + * Copyright (C) 2016 Red Hat Inc
> + *
> + * Authors:
> + * Marcel Apfelbaum <marcel@redhat.com>
> + *
> + * Most of the code was migrated from hw/pci-bridge/ioh3420.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/pci/pcie_port.h"
> +#include "hw/pci/msi.h"
> +
> +#define PCIE_ROOT_PORT_MSI_VECTORS 2
> +#define PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS PCI_MSI_FLAGS_MASKBIT
> +#define PCIE_ROOT_PORT_AER_OFFSET 0x100
This isn't generic either.
> +
> +/*
> + * If two MSI vector
MSI vectors
> are allocated, Advanced Error Interrupt Message Number
> + * is 1. otherwise 0.
> + * 17.12.5.10 RPERRSTS, 32:27 bit Advanced Error Interrupt Message Number.
Hmm I see it in
7.10.10. Root Error Status Register (Offset 30h)
which spec is this number from?
> + */
> +static uint8_t rp_aer_vector(const PCIDevice *d)
> +{
> + switch (msi_nr_vectors_allocated(d)) {
> + case 1:
> + return 0;
> + case 2:
> + return 1;
> + case 4:
> + case 8:
> + case 16:
> + case 32:
what are these doing here?
> + default:
> + break;
> + }
> + abort();
> + return 0;
> +}
I don't believe it's a generic thing. This is just how
intel did it, isn't it?
> +
> +static void rp_aer_vector_update(PCIDevice *d)
> +{
> + pcie_aer_root_set_vector(d, rp_aer_vector(d));
> +}
> +
> +static void rp_write_config(PCIDevice *d, uint32_t address,
> + uint32_t val, int len)
> +{
> + uint32_t root_cmd =
> + pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_ROOT_COMMAND);
> +
> + pci_bridge_write_config(d, address, val, len);
> + rp_aer_vector_update(d);
> + pcie_cap_slot_write_config(d, address, val, len);
> + pcie_aer_write_config(d, address, val, len);
> + pcie_aer_root_write_config(d, address, val, len, root_cmd);
> +}
> +
> +static void rp_reset(DeviceState *qdev)
> +{
> + PCIDevice *d = PCI_DEVICE(qdev);
> +
> + rp_aer_vector_update(d);
> + pcie_cap_root_reset(d);
> + pcie_cap_deverr_reset(d);
> + pcie_cap_slot_reset(d);
> + pcie_cap_arifwd_reset(d);
> + pcie_aer_root_reset(d);
> + pci_bridge_reset(qdev);
> + pci_bridge_disable_base_limit(d);
> +}
> +
> +static void rp_realize(PCIDevice *d, Error **errp)
> +{
> + PCIEPort *p = PCIE_PORT(d);
> + PCIESlot *s = PCIE_SLOT(d);
> + PCIDeviceClass *dc = PCI_DEVICE_GET_CLASS(d);
> + PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
> + int rc;
> + Error *local_err = NULL;
> +
> + pci_config_set_interrupt_pin(d->config, 1);
> + pci_bridge_initfn(d, TYPE_PCIE_BUS);
> + pcie_port_init_reg(d);
> +
> + rc = pci_bridge_ssvid_init(d, rpc->ssvid_offset, dc->vendor_id, rpc->ssid);
> + if (rc < 0) {
> + error_setg(errp, "Can't init SSV ID, error %d", rc);
> + goto err_bridge;
> + }
> +
> + rc = msi_init(d, rpc->msi_offset, PCIE_ROOT_PORT_MSI_VECTORS,
> + PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
> + PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT,
> + &local_err);
> + if (rc < 0) {
> + assert(rc == -ENOTSUP);
> + error_propagate(errp, local_err);
> + goto err_bridge;
> + }
> +
> + rc = pcie_cap_init(d, rpc->exp_offset, PCI_EXP_TYPE_ROOT_PORT, p->port);
> + if (rc < 0) {
> + error_setg(errp, "Can't add Root Port capability, error %d", rc);
> + goto err_msi;
> + }
> +
> + pcie_cap_arifwd_init(d);
> + pcie_cap_deverr_init(d);
> + pcie_cap_slot_init(d, s->slot);
> + pcie_cap_root_init(d);
> +
> + pcie_chassis_create(s->chassis);
> + rc = pcie_chassis_add_slot(s);
> + if (rc < 0) {
> + error_setg(errp, "Can't add chassis slot, error %d", rc);
> + goto err_pcie_cap;
> + }
> +
> + rc = pcie_aer_init(d, PCI_ERR_VER, rpc->aer_offset,
> + PCI_ERR_SIZEOF, &local_err);
> + if (rc < 0) {
> + error_propagate(errp, local_err);
> + goto err;
> + }
> + pcie_aer_root_init(d);
> + rp_aer_vector_update(d);
> +
> + return;
> +
> +err:
> + pcie_chassis_del_slot(s);
> +err_pcie_cap:
> + pcie_cap_exit(d);
> +err_msi:
> + msi_uninit(d);
> +err_bridge:
> + pci_bridge_exitfn(d);
> +}
> +
> +static void rp_exit(PCIDevice *d)
> +{
> + PCIESlot *s = PCIE_SLOT(d);
> +
> + pcie_aer_exit(d);
> + pcie_chassis_del_slot(s);
> + pcie_cap_exit(d);
> + msi_uninit(d);
> + pci_bridge_exitfn(d);
> +}
> +
> +static Property rp_props[] = {
> + DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
> + QEMU_PCIE_SLTCAP_PCP_BITNR, true),
> + DEFINE_PROP_END_OF_LIST()
> +};
> +
> +static void rp_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
> +
> + k->is_express = 1;
> + k->is_bridge = 1;
> + k->config_write = rp_write_config;
> + k->realize = rp_realize;
> + k->exit = rp_exit;
> + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> + dc->reset = rp_reset;
> + dc->props = rp_props;
> + rpc->aer_offset = PCIE_ROOT_PORT_AER_OFFSET;
> +}
> +
> +static const TypeInfo rp_info = {
> + .name = TYPE_PCIE_ROOT_PORT,
> + .parent = TYPE_PCIE_SLOT,
> + .class_init = rp_class_init,
> + .abstract = true,
> + .class_size = sizeof(PCIERootPortClass),
> +};
> +
> +static void rp_register_types(void)
> +{
> + type_register_static(&rp_info);
> +}
> +
> +type_init(rp_register_types)
> diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
> index f7b64db..a8eee56 100644
> --- a/include/hw/pci/pcie_port.h
> +++ b/include/hw/pci/pcie_port.h
> @@ -57,4 +57,22 @@ PCIESlot *pcie_chassis_find_slot(uint8_t chassis, uint16_t slot);
> 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_CLASS(klass) \
> + OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
> +#define PCIE_ROOT_PORT_CLASS(klass) \
> + OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
> +#define PCIE_ROOT_PORT_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(PCIERootPortClass, (obj), TYPE_PCIE_ROOT_PORT)
> +
> +typedef struct PCIERootPortClass {
> + PCIDeviceClass parent_class;
> +
> + int msi_offset;
> + int exp_offset;
> + int aer_offset;
> + int ssvid_offset;
> + int ssid;
> +} PCIERootPortClass;
> +
> #endif /* QEMU_PCIE_PORT_H */
> --
> 2.5.5
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH V2 1/3] hw/pcie: Introduce a base class for PCI Express Root Ports
2017-01-12 15:36 ` Michael S. Tsirkin
@ 2017-01-12 15:51 ` Marcel Apfelbaum
0 siblings, 0 replies; 15+ messages in thread
From: Marcel Apfelbaum @ 2017-01-12 15:51 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
On 01/12/2017 05:36 PM, Michael S. Tsirkin wrote:
> On Wed, Jan 11, 2017 at 02:18:54PM +0200, Marcel Apfelbaum wrote:
>> The 'base' PCI Express Root Port includes
>> the common code to be re-used for all
>> Root Ports implementations. Most of the code
>> was taken from the current implementation
>> of Intel's IOH 3420 Root Port.
>>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>> ---
>> default-configs/arm-softmmu.mak | 1 +
>> default-configs/i386-softmmu.mak | 1 +
>> default-configs/x86_64-softmmu.mak | 1 +
>> hw/pci-bridge/Makefile.objs | 1 +
>> hw/pci-bridge/pcie_root_port.c | 194 +++++++++++++++++++++++++++++++++++++
>> include/hw/pci/pcie_port.h | 18 ++++
>> 6 files changed, 216 insertions(+)
>> create mode 100644 hw/pci-bridge/pcie_root_port.c
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index 6de3e16..6f2a180 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -108,6 +108,7 @@ CONFIG_FSL_IMX25=y
>>
>> CONFIG_IMX_I2C=y
>>
>> +CONFIG_PCIE_PORT=y
>> CONFIG_XIO3130=y
>> CONFIG_IOH3420=y
>> CONFIG_I82801B11=y
>> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
>> index 0b51360..9288838 100644
>> --- a/default-configs/i386-softmmu.mak
>> +++ b/default-configs/i386-softmmu.mak
>> @@ -51,6 +51,7 @@ CONFIG_PVPANIC=y
>> CONFIG_MEM_HOTPLUG=y
>> CONFIG_NVDIMM=y
>> CONFIG_ACPI_NVDIMM=y
>> +CONFIG_PCIE_PORT=y
>> CONFIG_XIO3130=y
>> CONFIG_IOH3420=y
>> CONFIG_I82801B11=y
>> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
>> index 7f89503..7d2c2d4 100644
>> --- a/default-configs/x86_64-softmmu.mak
>> +++ b/default-configs/x86_64-softmmu.mak
>> @@ -51,6 +51,7 @@ CONFIG_PVPANIC=y
>> CONFIG_MEM_HOTPLUG=y
>> CONFIG_NVDIMM=y
>> CONFIG_ACPI_NVDIMM=y
>> +CONFIG_PCIE_PORT=y
>> CONFIG_XIO3130=y
>> CONFIG_IOH3420=y
>> CONFIG_I82801B11=y
>> diff --git a/hw/pci-bridge/Makefile.objs b/hw/pci-bridge/Makefile.objs
>> index f2adfe3..4f2039f 100644
>> --- a/hw/pci-bridge/Makefile.objs
>> +++ b/hw/pci-bridge/Makefile.objs
>> @@ -1,5 +1,6 @@
>> common-obj-y += pci_bridge_dev.o
>> common-obj-y += pci_expander_bridge.o
>> +common-obj-$(CONFIG_PCIE_PORT) += pcie_root_port.o
>> common-obj-$(CONFIG_XIO3130) += xio3130_upstream.o xio3130_downstream.o
>> common-obj-$(CONFIG_IOH3420) += ioh3420.o
>> common-obj-$(CONFIG_I82801B11) += i82801b11.o
>> diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
>> new file mode 100644
>> index 0000000..e84ae14
>> --- /dev/null
>> +++ b/hw/pci-bridge/pcie_root_port.c
>> @@ -0,0 +1,194 @@
>> +/*
>> + * Generic PCI Express Root Port emulation
>> + *
>> + * Copyright (C) 2016 Red Hat Inc
>> + *
>> + * Authors:
>> + * Marcel Apfelbaum <marcel@redhat.com>
>> + *
>> + * Most of the code was migrated from hw/pci-bridge/ioh3420.
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "hw/pci/pcie_port.h"
>> +#include "hw/pci/msi.h"
>> +
>> +#define PCIE_ROOT_PORT_MSI_VECTORS 2
>> +#define PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS PCI_MSI_FLAGS_MASKBIT
>> +#define PCIE_ROOT_PORT_AER_OFFSET 0x100
>
Hi Michael
> This isn't generic either.
>
It is used by the generic port which happens to be in the same file (patch 3).
I'll move the define to patch 3.
>> +
>> +/*
>> + * If two MSI vector
>
>
> MSI vectors
>
OK
>> are allocated, Advanced Error Interrupt Message Number
>> + * is 1. otherwise 0.
>> + * 17.12.5.10 RPERRSTS, 32:27 bit Advanced Error Interrupt Message Number.
>
> Hmm I see it in
> 7.10.10. Root Error Status Register (Offset 30h)
>
> which spec is this number from?
>
No idea, the description is from the Intel Port code, I'll update.
>
>> + */
>> +static uint8_t rp_aer_vector(const PCIDevice *d)
>> +{
>> + switch (msi_nr_vectors_allocated(d)) {
>> + case 1:
>> + return 0;
>> + case 2:
>> + return 1;
>> + case 4:
>> + case 8:
>> + case 16:
>> + case 32:
>
> what are these doing here?
>
Will remove.
>> + default:
>> + break;
>> + }
>> + abort();
>> + return 0;
>> +}
>
> I don't believe it's a generic thing. This is just how
> intel did it, isn't it?
>
Yes, but since we have only 2 implementations
and both are working the same I think they can share it
until the functionality will split.
An alternative is to add a function to the base class and
an implementation for each derived class, but since the code
ultimately will look the same I preferred to keep it simple.
Thanks for the review,
Marcel
>
>> +
>> +static void rp_aer_vector_update(PCIDevice *d)
>> +{
>> + pcie_aer_root_set_vector(d, rp_aer_vector(d));
>> +}
>> +
>> +static void rp_write_config(PCIDevice *d, uint32_t address,
>> + uint32_t val, int len)
>> +{
>> + uint32_t root_cmd =
>> + pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_ROOT_COMMAND);
>> +
>> + pci_bridge_write_config(d, address, val, len);
>> + rp_aer_vector_update(d);
>> + pcie_cap_slot_write_config(d, address, val, len);
>> + pcie_aer_write_config(d, address, val, len);
>> + pcie_aer_root_write_config(d, address, val, len, root_cmd);
>> +}
>> +
>> +static void rp_reset(DeviceState *qdev)
>> +{
>> + PCIDevice *d = PCI_DEVICE(qdev);
>> +
>> + rp_aer_vector_update(d);
>> + pcie_cap_root_reset(d);
>> + pcie_cap_deverr_reset(d);
>> + pcie_cap_slot_reset(d);
>> + pcie_cap_arifwd_reset(d);
>> + pcie_aer_root_reset(d);
>> + pci_bridge_reset(qdev);
>> + pci_bridge_disable_base_limit(d);
>> +}
>> +
>> +static void rp_realize(PCIDevice *d, Error **errp)
>> +{
>> + PCIEPort *p = PCIE_PORT(d);
>> + PCIESlot *s = PCIE_SLOT(d);
>> + PCIDeviceClass *dc = PCI_DEVICE_GET_CLASS(d);
>> + PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
>> + int rc;
>> + Error *local_err = NULL;
>> +
>> + pci_config_set_interrupt_pin(d->config, 1);
>> + pci_bridge_initfn(d, TYPE_PCIE_BUS);
>> + pcie_port_init_reg(d);
>> +
>> + rc = pci_bridge_ssvid_init(d, rpc->ssvid_offset, dc->vendor_id, rpc->ssid);
>> + if (rc < 0) {
>> + error_setg(errp, "Can't init SSV ID, error %d", rc);
>> + goto err_bridge;
>> + }
>> +
>> + rc = msi_init(d, rpc->msi_offset, PCIE_ROOT_PORT_MSI_VECTORS,
>> + PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
>> + PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT,
>> + &local_err);
>> + if (rc < 0) {
>> + assert(rc == -ENOTSUP);
>> + error_propagate(errp, local_err);
>> + goto err_bridge;
>> + }
>> +
>> + rc = pcie_cap_init(d, rpc->exp_offset, PCI_EXP_TYPE_ROOT_PORT, p->port);
>> + if (rc < 0) {
>> + error_setg(errp, "Can't add Root Port capability, error %d", rc);
>> + goto err_msi;
>> + }
>> +
>> + pcie_cap_arifwd_init(d);
>> + pcie_cap_deverr_init(d);
>> + pcie_cap_slot_init(d, s->slot);
>> + pcie_cap_root_init(d);
>> +
>> + pcie_chassis_create(s->chassis);
>> + rc = pcie_chassis_add_slot(s);
>> + if (rc < 0) {
>> + error_setg(errp, "Can't add chassis slot, error %d", rc);
>> + goto err_pcie_cap;
>> + }
>> +
>> + rc = pcie_aer_init(d, PCI_ERR_VER, rpc->aer_offset,
>> + PCI_ERR_SIZEOF, &local_err);
>> + if (rc < 0) {
>> + error_propagate(errp, local_err);
>> + goto err;
>> + }
>> + pcie_aer_root_init(d);
>> + rp_aer_vector_update(d);
>> +
>> + return;
>> +
>> +err:
>> + pcie_chassis_del_slot(s);
>> +err_pcie_cap:
>> + pcie_cap_exit(d);
>> +err_msi:
>> + msi_uninit(d);
>> +err_bridge:
>> + pci_bridge_exitfn(d);
>> +}
>> +
>> +static void rp_exit(PCIDevice *d)
>> +{
>> + PCIESlot *s = PCIE_SLOT(d);
>> +
>> + pcie_aer_exit(d);
>> + pcie_chassis_del_slot(s);
>> + pcie_cap_exit(d);
>> + msi_uninit(d);
>> + pci_bridge_exitfn(d);
>> +}
>> +
>> +static Property rp_props[] = {
>> + DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
>> + QEMU_PCIE_SLTCAP_PCP_BITNR, true),
>> + DEFINE_PROP_END_OF_LIST()
>> +};
>> +
>> +static void rp_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> + PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
>> +
>> + k->is_express = 1;
>> + k->is_bridge = 1;
>> + k->config_write = rp_write_config;
>> + k->realize = rp_realize;
>> + k->exit = rp_exit;
>> + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>> + dc->reset = rp_reset;
>> + dc->props = rp_props;
>> + rpc->aer_offset = PCIE_ROOT_PORT_AER_OFFSET;
>> +}
>> +
>> +static const TypeInfo rp_info = {
>> + .name = TYPE_PCIE_ROOT_PORT,
>> + .parent = TYPE_PCIE_SLOT,
>> + .class_init = rp_class_init,
>> + .abstract = true,
>> + .class_size = sizeof(PCIERootPortClass),
>> +};
>> +
>> +static void rp_register_types(void)
>> +{
>> + type_register_static(&rp_info);
>> +}
>> +
>> +type_init(rp_register_types)
>> diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
>> index f7b64db..a8eee56 100644
>> --- a/include/hw/pci/pcie_port.h
>> +++ b/include/hw/pci/pcie_port.h
>> @@ -57,4 +57,22 @@ PCIESlot *pcie_chassis_find_slot(uint8_t chassis, uint16_t slot);
>> 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_CLASS(klass) \
>> + OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
>> +#define PCIE_ROOT_PORT_CLASS(klass) \
>> + OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
>> +#define PCIE_ROOT_PORT_GET_CLASS(obj) \
>> + OBJECT_GET_CLASS(PCIERootPortClass, (obj), TYPE_PCIE_ROOT_PORT)
>> +
>> +typedef struct PCIERootPortClass {
>> + PCIDeviceClass parent_class;
>> +
>> + int msi_offset;
>> + int exp_offset;
>> + int aer_offset;
>> + int ssvid_offset;
>> + int ssid;
>> +} PCIERootPortClass;
>> +
>> #endif /* QEMU_PCIE_PORT_H */
>> --
>> 2.5.5
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH V2 2/3] hw/ioh3420: derive from PCI Express Root Port base class
2017-01-11 12:18 [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port Marcel Apfelbaum
2017-01-11 12:18 ` [Qemu-devel] [PATCH V2 1/3] hw/pcie: Introduce a base class for PCI Express Root Ports Marcel Apfelbaum
@ 2017-01-11 12:18 ` Marcel Apfelbaum
2017-01-11 12:18 ` [Qemu-devel] [PATCH V2 3/3] hw/pcie: Introduce Generic PCI Express Root Port Marcel Apfelbaum
` (2 subsequent siblings)
4 siblings, 0 replies; 15+ messages in thread
From: Marcel Apfelbaum @ 2017-01-11 12:18 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel
Preserve only Intel specific details.
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/pci-bridge/ioh3420.c | 152 +++---------------------------------------------
1 file changed, 7 insertions(+), 145 deletions(-)
diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index 84b7946..431266c 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -38,142 +38,6 @@
#define IOH_EP_EXP_OFFSET 0x90
#define IOH_EP_AER_OFFSET 0x100
-/*
- * If two MSI vector are allocated, Advanced Error Interrupt Message Number
- * is 1. otherwise 0.
- * 17.12.5.10 RPERRSTS, 32:27 bit Advanced Error Interrupt Message Number.
- */
-static uint8_t ioh3420_aer_vector(const PCIDevice *d)
-{
- switch (msi_nr_vectors_allocated(d)) {
- case 1:
- return 0;
- case 2:
- return 1;
- case 4:
- case 8:
- case 16:
- case 32:
- default:
- break;
- }
- abort();
- return 0;
-}
-
-static void ioh3420_aer_vector_update(PCIDevice *d)
-{
- pcie_aer_root_set_vector(d, ioh3420_aer_vector(d));
-}
-
-static void ioh3420_write_config(PCIDevice *d,
- uint32_t address, uint32_t val, int len)
-{
- uint32_t root_cmd =
- pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_ROOT_COMMAND);
-
- pci_bridge_write_config(d, address, val, len);
- ioh3420_aer_vector_update(d);
- pcie_cap_slot_write_config(d, address, val, len);
- pcie_aer_write_config(d, address, val, len);
- pcie_aer_root_write_config(d, address, val, len, root_cmd);
-}
-
-static void ioh3420_reset(DeviceState *qdev)
-{
- PCIDevice *d = PCI_DEVICE(qdev);
-
- ioh3420_aer_vector_update(d);
- pcie_cap_root_reset(d);
- pcie_cap_deverr_reset(d);
- pcie_cap_slot_reset(d);
- pcie_cap_arifwd_reset(d);
- pcie_aer_root_reset(d);
- pci_bridge_reset(qdev);
- pci_bridge_disable_base_limit(d);
-}
-
-static int ioh3420_initfn(PCIDevice *d)
-{
- PCIEPort *p = PCIE_PORT(d);
- PCIESlot *s = PCIE_SLOT(d);
- int rc;
- Error *err = NULL;
-
- pci_config_set_interrupt_pin(d->config, 1);
- pci_bridge_initfn(d, TYPE_PCIE_BUS);
- pcie_port_init_reg(d);
-
- rc = pci_bridge_ssvid_init(d, IOH_EP_SSVID_OFFSET,
- IOH_EP_SSVID_SVID, IOH_EP_SSVID_SSID);
- if (rc < 0) {
- goto err_bridge;
- }
-
- rc = msi_init(d, IOH_EP_MSI_OFFSET, IOH_EP_MSI_NR_VECTOR,
- IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
- IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT, &err);
- if (rc < 0) {
- assert(rc == -ENOTSUP);
- error_report_err(err);
- goto err_bridge;
- }
-
- rc = pcie_cap_init(d, IOH_EP_EXP_OFFSET, PCI_EXP_TYPE_ROOT_PORT, p->port);
- if (rc < 0) {
- goto err_msi;
- }
-
- pcie_cap_arifwd_init(d);
- pcie_cap_deverr_init(d);
- pcie_cap_slot_init(d, s->slot);
- pcie_cap_root_init(d);
-
- pcie_chassis_create(s->chassis);
- rc = pcie_chassis_add_slot(s);
- if (rc < 0) {
- goto err_pcie_cap;
- }
-
- rc = pcie_aer_init(d, PCI_ERR_VER, IOH_EP_AER_OFFSET,
- PCI_ERR_SIZEOF, &err);
- if (rc < 0) {
- error_report_err(err);
- goto err;
- }
- pcie_aer_root_init(d);
- ioh3420_aer_vector_update(d);
-
- return 0;
-
-err:
- pcie_chassis_del_slot(s);
-err_pcie_cap:
- pcie_cap_exit(d);
-err_msi:
- msi_uninit(d);
-err_bridge:
- pci_bridge_exitfn(d);
- return rc;
-}
-
-static void ioh3420_exitfn(PCIDevice *d)
-{
- PCIESlot *s = PCIE_SLOT(d);
-
- pcie_aer_exit(d);
- pcie_chassis_del_slot(s);
- pcie_cap_exit(d);
- msi_uninit(d);
- pci_bridge_exitfn(d);
-}
-
-static Property ioh3420_props[] = {
- DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
- QEMU_PCIE_SLTCAP_PCP_BITNR, true),
- DEFINE_PROP_END_OF_LIST()
-};
-
static const VMStateDescription vmstate_ioh3420 = {
.name = "ioh-3240-express-root-port",
.version_id = 1,
@@ -191,25 +55,23 @@ static void ioh3420_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
- k->is_express = 1;
- k->is_bridge = 1;
- k->config_write = ioh3420_write_config;
- k->init = ioh3420_initfn;
- k->exit = ioh3420_exitfn;
k->vendor_id = PCI_VENDOR_ID_INTEL;
k->device_id = PCI_DEVICE_ID_IOH_EPORT;
k->revision = PCI_DEVICE_ID_IOH_REV;
- set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
dc->desc = "Intel IOH device id 3420 PCIE Root Port";
- dc->reset = ioh3420_reset;
dc->vmsd = &vmstate_ioh3420;
- dc->props = ioh3420_props;
+ rpc->msi_offset = IOH_EP_MSI_OFFSET;
+ rpc->exp_offset = IOH_EP_EXP_OFFSET;
+ rpc->aer_offset = IOH_EP_AER_OFFSET;
+ rpc->ssvid_offset = IOH_EP_SSVID_OFFSET;
+ rpc->ssid = IOH_EP_SSVID_SSID;
}
static const TypeInfo ioh3420_info = {
.name = "ioh3420",
- .parent = TYPE_PCIE_SLOT,
+ .parent = TYPE_PCIE_ROOT_PORT,
.class_init = ioh3420_class_init,
};
--
2.5.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH V2 3/3] hw/pcie: Introduce Generic PCI Express Root Port
2017-01-11 12:18 [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port Marcel Apfelbaum
2017-01-11 12:18 ` [Qemu-devel] [PATCH V2 1/3] hw/pcie: Introduce a base class for PCI Express Root Ports Marcel Apfelbaum
2017-01-11 12:18 ` [Qemu-devel] [PATCH V2 2/3] hw/ioh3420: derive from PCI Express Root Port base class Marcel Apfelbaum
@ 2017-01-11 12:18 ` Marcel Apfelbaum
2017-01-12 15:56 ` [Qemu-devel] [PATCH V2 0/3] " Michael S. Tsirkin
2017-01-16 16:46 ` Andrea Bolognani
4 siblings, 0 replies; 15+ messages in thread
From: Marcel Apfelbaum @ 2017-01-11 12:18 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel
The Generic Root Port behaves the same as the
Intel's IOH device with id 3420, without having
Intel specific attributes.
The device has two purposes:
(1) Can be used on both X86 and ARM machines.
(2) It will allow us to tweak the behaviour
(e.g add vendor-specific PCI capabilities)
- something that obviously cannot be done
on a known device.
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/pci-bridge/pcie_root_port.c | 34 ++++++++++++++++++++++++++++++++++
include/hw/pci/pci.h | 1 +
2 files changed, 35 insertions(+)
diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index e84ae14..32c6201 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -21,6 +21,9 @@
#define PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS PCI_MSI_FLAGS_MASKBIT
#define PCIE_ROOT_PORT_AER_OFFSET 0x100
+#define TYPE_PCIE_ROOT_PORT_DEV "pcie-root-port"
+
+
/*
* If two MSI vector are allocated, Advanced Error Interrupt Message Number
* is 1. otherwise 0.
@@ -186,9 +189,40 @@ static const TypeInfo rp_info = {
.class_size = sizeof(PCIERootPortClass),
};
+static const VMStateDescription vmstate_rp_dev = {
+ .name = "pcie-root-port",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .post_load = pcie_cap_slot_post_load,
+ .fields = (VMStateField[]) {
+ VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
+ VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
+ PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void rp_dev_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+ k->vendor_id = PCI_VENDOR_ID_REDHAT;
+ k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_RP;
+ dc->desc = "PCI Express Root Port";
+ dc->vmsd = &vmstate_rp_dev;
+}
+
+static const TypeInfo rp_dev_info = {
+ .name = TYPE_PCIE_ROOT_PORT_DEV,
+ .parent = TYPE_PCIE_ROOT_PORT,
+ .class_init = rp_dev_class_init,
+};
+
static void rp_register_types(void)
{
type_register_static(&rp_info);
+ type_register_static(&rp_dev_info);
}
type_init(rp_register_types)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 772692f..cbc1fdf 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -96,6 +96,7 @@
#define PCI_DEVICE_ID_REDHAT_PXB 0x0009
#define PCI_DEVICE_ID_REDHAT_BRIDGE_SEAT 0x000a
#define PCI_DEVICE_ID_REDHAT_PXB_PCIE 0x000b
+#define PCI_DEVICE_ID_REDHAT_PCIE_RP 0x000c
#define PCI_DEVICE_ID_REDHAT_QXL 0x0100
#define FMT_PCIBUS PRIx64
--
2.5.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port
2017-01-11 12:18 [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port Marcel Apfelbaum
` (2 preceding siblings ...)
2017-01-11 12:18 ` [Qemu-devel] [PATCH V2 3/3] hw/pcie: Introduce Generic PCI Express Root Port Marcel Apfelbaum
@ 2017-01-12 15:56 ` Michael S. Tsirkin
2017-01-12 16:05 ` Marcel Apfelbaum
2017-01-16 16:46 ` Andrea Bolognani
4 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2017-01-12 15:56 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel
On Wed, Jan 11, 2017 at 02:18:53PM +0200, Marcel Apfelbaum wrote:
> v1 -> v2:
> - Rebased on master.
>
> The Generic Root Port behaves the same as the
> Intel's IOH device with id 3420, without having
> Intel specific attributes.
>
> The device has two purposes:
> (1) Can be used on both X86 and ARM machines.
> (2) It will allow us to tweak the behaviour
> (e.g add vendor-specific PCI capabilities)
> - something that obviously cannot be done
> on a known device.
>
> Patch 1/3: Introduce a base class for Root Ports - most of the code
> is migrated from IOH3420 implementation.
> Patch 2/3: Derives the IOH3420 from the new base class
> Patch 3/3: Introduces the generic Root Port.
>
> Tested with Linux and Windows guests only on x86 hosts.
So I understand how it's helpful to share code
with an existing bridge, but I worries me that
you picked up some arbitrary values from the intel chip
and promote them as a standard pcie thing.
You want to have e.g. base-pcie-root-port and
inherit that. Only put really common stuff in there.
Similar to how we do it in hw/pci/pci_bridge.c
Now we do not want to over-engineer, so I understand
how you might want to say hey I use this option
and intel does the same so we do not need a flag
for that now. And that's fine but please add a comment
to that end.
And when you do you will discover there's some stuff you
might be able to simplify, e.g. I don't yet see why
you would want AER support there.
> Marcel Apfelbaum (3):
> hw/pcie: Introduce a base class for PCI Express Root Ports
> hw/ioh3420: derive from PCI Express Root Port base class
> hw/pcie: Introduce Generic PCI Express Root Port
>
> default-configs/arm-softmmu.mak | 1 +
> default-configs/i386-softmmu.mak | 1 +
> default-configs/x86_64-softmmu.mak | 1 +
> hw/pci-bridge/Makefile.objs | 1 +
> hw/pci-bridge/ioh3420.c | 152 ++-----------------------
> hw/pci-bridge/pcie_root_port.c | 228 +++++++++++++++++++++++++++++++++++++
> include/hw/pci/pci.h | 1 +
> include/hw/pci/pcie_port.h | 18 +++
> 8 files changed, 258 insertions(+), 145 deletions(-)
> create mode 100644 hw/pci-bridge/pcie_root_port.c
>
> --
> 2.5.5
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port
2017-01-12 15:56 ` [Qemu-devel] [PATCH V2 0/3] " Michael S. Tsirkin
@ 2017-01-12 16:05 ` Marcel Apfelbaum
2017-01-12 16:20 ` Michael S. Tsirkin
0 siblings, 1 reply; 15+ messages in thread
From: Marcel Apfelbaum @ 2017-01-12 16:05 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
On 01/12/2017 05:56 PM, Michael S. Tsirkin wrote:
> On Wed, Jan 11, 2017 at 02:18:53PM +0200, Marcel Apfelbaum wrote:
>> v1 -> v2:
>> - Rebased on master.
>>
>> The Generic Root Port behaves the same as the
>> Intel's IOH device with id 3420, without having
>> Intel specific attributes.
>>
>> The device has two purposes:
>> (1) Can be used on both X86 and ARM machines.
>> (2) It will allow us to tweak the behaviour
>> (e.g add vendor-specific PCI capabilities)
>> - something that obviously cannot be done
>> on a known device.
>>
>> Patch 1/3: Introduce a base class for Root Ports - most of the code
>> is migrated from IOH3420 implementation.
>> Patch 2/3: Derives the IOH3420 from the new base class
>> Patch 3/3: Introduces the generic Root Port.
>>
>> Tested with Linux and Windows guests only on x86 hosts.
>
Hi Michael,
> So I understand how it's helpful to share code
> with an existing bridge, but I worries me that
> you picked up some arbitrary values from the intel chip
> and promote them as a standard pcie thing.
>
I suppose you are referring to the macros, I can duplicate
them, of course.
> You want to have e.g. base-pcie-root-port and
> inherit that. Only put really common stuff in there.
> Similar to how we do it in hw/pci/pci_bridge.c
>
So you prefer a different code file for the generic device.
I currently put them both in the same file, I'll split.
>
> Now we do not want to over-engineer, so I understand
> how you might want to say hey I use this option
> and intel does the same so we do not need a flag
> for that now. And that's fine but please add a comment
> to that end.
>
Indeed, I wanted to keep things simple. I'll add a comment
on the functions that are not generic but are the same on
both implementations 'by chance'.
> And when you do you will discover there's some stuff you
> might be able to simplify, e.g. I don't yet see why
> you would want AER support there.
>
I am not sure *why not*. I am 'afraid' people will not use the new port
because of missing features and they can see the AER as one of them.
Do you see a special issue in imitating Intel's behavior on AER?
Thanks,
Marcel
>> Marcel Apfelbaum (3):
>> hw/pcie: Introduce a base class for PCI Express Root Ports
>> hw/ioh3420: derive from PCI Express Root Port base class
>> hw/pcie: Introduce Generic PCI Express Root Port
>>
>> default-configs/arm-softmmu.mak | 1 +
>> default-configs/i386-softmmu.mak | 1 +
>> default-configs/x86_64-softmmu.mak | 1 +
>> hw/pci-bridge/Makefile.objs | 1 +
>> hw/pci-bridge/ioh3420.c | 152 ++-----------------------
>> hw/pci-bridge/pcie_root_port.c | 228 +++++++++++++++++++++++++++++++++++++
>> include/hw/pci/pci.h | 1 +
>> include/hw/pci/pcie_port.h | 18 +++
>> 8 files changed, 258 insertions(+), 145 deletions(-)
>> create mode 100644 hw/pci-bridge/pcie_root_port.c
>>
>> --
>> 2.5.5
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port
2017-01-12 16:05 ` Marcel Apfelbaum
@ 2017-01-12 16:20 ` Michael S. Tsirkin
2017-01-12 16:23 ` Marcel Apfelbaum
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2017-01-12 16:20 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel
On Thu, Jan 12, 2017 at 06:05:20PM +0200, Marcel Apfelbaum wrote:
> On 01/12/2017 05:56 PM, Michael S. Tsirkin wrote:
> > On Wed, Jan 11, 2017 at 02:18:53PM +0200, Marcel Apfelbaum wrote:
> > > v1 -> v2:
> > > - Rebased on master.
> > >
> > > The Generic Root Port behaves the same as the
> > > Intel's IOH device with id 3420, without having
> > > Intel specific attributes.
> > >
> > > The device has two purposes:
> > > (1) Can be used on both X86 and ARM machines.
> > > (2) It will allow us to tweak the behaviour
> > > (e.g add vendor-specific PCI capabilities)
> > > - something that obviously cannot be done
> > > on a known device.
> > >
> > > Patch 1/3: Introduce a base class for Root Ports - most of the code
> > > is migrated from IOH3420 implementation.
> > > Patch 2/3: Derives the IOH3420 from the new base class
> > > Patch 3/3: Introduces the generic Root Port.
> > >
> > > Tested with Linux and Windows guests only on x86 hosts.
> >
>
> Hi Michael,
>
> > So I understand how it's helpful to share code
> > with an existing bridge, but I worries me that
> > you picked up some arbitrary values from the intel chip
> > and promote them as a standard pcie thing.
> >
>
> I suppose you are referring to the macros, I can duplicate
> them, of course.
>
> > You want to have e.g. base-pcie-root-port and
> > inherit that. Only put really common stuff in there.
> > Similar to how we do it in hw/pci/pci_bridge.c
> >
>
> So you prefer a different code file for the generic device.
> I currently put them both in the same file, I'll split.
>
> >
> > Now we do not want to over-engineer, so I understand
> > how you might want to say hey I use this option
> > and intel does the same so we do not need a flag
> > for that now. And that's fine but please add a comment
> > to that end.
> >
>
> Indeed, I wanted to keep things simple. I'll add a comment
> on the functions that are not generic but are the same on
> both implementations 'by chance'.
>
> > And when you do you will discover there's some stuff you
> > might be able to simplify, e.g. I don't yet see why
> > you would want AER support there.
> >
>
> I am not sure *why not*.
> I am 'afraid' people will not use the new port
> because of missing features and they can see the AER as one of them.
> Do you see a special issue in imitating Intel's behavior on AER?
>
> Thanks,
> Marcel
Not really, fair enough. Just keep an eye out for caps
you don't need. E.g. it's worth considering whether
msi-x would be better since then you can just use
a fixed vector number without playing tricks like
you have to with msi.
> > > Marcel Apfelbaum (3):
> > > hw/pcie: Introduce a base class for PCI Express Root Ports
> > > hw/ioh3420: derive from PCI Express Root Port base class
> > > hw/pcie: Introduce Generic PCI Express Root Port
> > >
> > > default-configs/arm-softmmu.mak | 1 +
> > > default-configs/i386-softmmu.mak | 1 +
> > > default-configs/x86_64-softmmu.mak | 1 +
> > > hw/pci-bridge/Makefile.objs | 1 +
> > > hw/pci-bridge/ioh3420.c | 152 ++-----------------------
> > > hw/pci-bridge/pcie_root_port.c | 228 +++++++++++++++++++++++++++++++++++++
> > > include/hw/pci/pci.h | 1 +
> > > include/hw/pci/pcie_port.h | 18 +++
> > > 8 files changed, 258 insertions(+), 145 deletions(-)
> > > create mode 100644 hw/pci-bridge/pcie_root_port.c
> > >
> > > --
> > > 2.5.5
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port
2017-01-12 16:20 ` Michael S. Tsirkin
@ 2017-01-12 16:23 ` Marcel Apfelbaum
2017-01-12 16:46 ` Michael S. Tsirkin
2017-01-13 8:35 ` Gerd Hoffmann
0 siblings, 2 replies; 15+ messages in thread
From: Marcel Apfelbaum @ 2017-01-12 16:23 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
On 01/12/2017 06:20 PM, Michael S. Tsirkin wrote:
> On Thu, Jan 12, 2017 at 06:05:20PM +0200, Marcel Apfelbaum wrote:
>> On 01/12/2017 05:56 PM, Michael S. Tsirkin wrote:
>>> On Wed, Jan 11, 2017 at 02:18:53PM +0200, Marcel Apfelbaum wrote:
>>>> v1 -> v2:
>>>> - Rebased on master.
>>>>
>>>> The Generic Root Port behaves the same as the
>>>> Intel's IOH device with id 3420, without having
>>>> Intel specific attributes.
>>>>
>>>> The device has two purposes:
>>>> (1) Can be used on both X86 and ARM machines.
>>>> (2) It will allow us to tweak the behaviour
>>>> (e.g add vendor-specific PCI capabilities)
>>>> - something that obviously cannot be done
>>>> on a known device.
>>>>
>>>> Patch 1/3: Introduce a base class for Root Ports - most of the code
>>>> is migrated from IOH3420 implementation.
>>>> Patch 2/3: Derives the IOH3420 from the new base class
>>>> Patch 3/3: Introduces the generic Root Port.
>>>>
>>>> Tested with Linux and Windows guests only on x86 hosts.
>>>
>>
>> Hi Michael,
>>
>>> So I understand how it's helpful to share code
>>> with an existing bridge, but I worries me that
>>> you picked up some arbitrary values from the intel chip
>>> and promote them as a standard pcie thing.
>>>
>>
>> I suppose you are referring to the macros, I can duplicate
>> them, of course.
>>
>>> You want to have e.g. base-pcie-root-port and
>>> inherit that. Only put really common stuff in there.
>>> Similar to how we do it in hw/pci/pci_bridge.c
>>>
>>
>> So you prefer a different code file for the generic device.
>> I currently put them both in the same file, I'll split.
>>
>>>
>>> Now we do not want to over-engineer, so I understand
>>> how you might want to say hey I use this option
>>> and intel does the same so we do not need a flag
>>> for that now. And that's fine but please add a comment
>>> to that end.
>>>
>>
>> Indeed, I wanted to keep things simple. I'll add a comment
>> on the functions that are not generic but are the same on
>> both implementations 'by chance'.
>>
>>> And when you do you will discover there's some stuff you
>>> might be able to simplify, e.g. I don't yet see why
>>> you would want AER support there.
>>>
>>
>> I am not sure *why not*.
>> I am 'afraid' people will not use the new port
>> because of missing features and they can see the AER as one of them.
>> Do you see a special issue in imitating Intel's behavior on AER?
>>
>> Thanks,
>> Marcel
>
> Not really, fair enough. Just keep an eye out for caps
> you don't need. E.g. it's worth considering whether
> msi-x would be better since then you can just use
> a fixed vector number without playing tricks like
> you have to with msi.
>
Sure, thanks for the pointers!
I suppose I can change later to msi-x in a follow up patch
to avoid introducing new bugs from the beginning :)
Thanks,
Marcel
>>>> Marcel Apfelbaum (3):
>>>> hw/pcie: Introduce a base class for PCI Express Root Ports
>>>> hw/ioh3420: derive from PCI Express Root Port base class
>>>> hw/pcie: Introduce Generic PCI Express Root Port
>>>>
>>>> default-configs/arm-softmmu.mak | 1 +
>>>> default-configs/i386-softmmu.mak | 1 +
>>>> default-configs/x86_64-softmmu.mak | 1 +
>>>> hw/pci-bridge/Makefile.objs | 1 +
>>>> hw/pci-bridge/ioh3420.c | 152 ++-----------------------
>>>> hw/pci-bridge/pcie_root_port.c | 228 +++++++++++++++++++++++++++++++++++++
>>>> include/hw/pci/pci.h | 1 +
>>>> include/hw/pci/pcie_port.h | 18 +++
>>>> 8 files changed, 258 insertions(+), 145 deletions(-)
>>>> create mode 100644 hw/pci-bridge/pcie_root_port.c
>>>>
>>>> --
>>>> 2.5.5
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port
2017-01-12 16:23 ` Marcel Apfelbaum
@ 2017-01-12 16:46 ` Michael S. Tsirkin
2017-01-13 8:35 ` Gerd Hoffmann
1 sibling, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2017-01-12 16:46 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel
On Thu, Jan 12, 2017 at 06:23:08PM +0200, Marcel Apfelbaum wrote:
> On 01/12/2017 06:20 PM, Michael S. Tsirkin wrote:
> > On Thu, Jan 12, 2017 at 06:05:20PM +0200, Marcel Apfelbaum wrote:
> > > On 01/12/2017 05:56 PM, Michael S. Tsirkin wrote:
> > > > On Wed, Jan 11, 2017 at 02:18:53PM +0200, Marcel Apfelbaum wrote:
> > > > > v1 -> v2:
> > > > > - Rebased on master.
> > > > >
> > > > > The Generic Root Port behaves the same as the
> > > > > Intel's IOH device with id 3420, without having
> > > > > Intel specific attributes.
> > > > >
> > > > > The device has two purposes:
> > > > > (1) Can be used on both X86 and ARM machines.
> > > > > (2) It will allow us to tweak the behaviour
> > > > > (e.g add vendor-specific PCI capabilities)
> > > > > - something that obviously cannot be done
> > > > > on a known device.
> > > > >
> > > > > Patch 1/3: Introduce a base class for Root Ports - most of the code
> > > > > is migrated from IOH3420 implementation.
> > > > > Patch 2/3: Derives the IOH3420 from the new base class
> > > > > Patch 3/3: Introduces the generic Root Port.
> > > > >
> > > > > Tested with Linux and Windows guests only on x86 hosts.
> > > >
> > >
> > > Hi Michael,
> > >
> > > > So I understand how it's helpful to share code
> > > > with an existing bridge, but I worries me that
> > > > you picked up some arbitrary values from the intel chip
> > > > and promote them as a standard pcie thing.
> > > >
> > >
> > > I suppose you are referring to the macros, I can duplicate
> > > them, of course.
> > >
> > > > You want to have e.g. base-pcie-root-port and
> > > > inherit that. Only put really common stuff in there.
> > > > Similar to how we do it in hw/pci/pci_bridge.c
> > > >
> > >
> > > So you prefer a different code file for the generic device.
> > > I currently put them both in the same file, I'll split.
> > >
> > > >
> > > > Now we do not want to over-engineer, so I understand
> > > > how you might want to say hey I use this option
> > > > and intel does the same so we do not need a flag
> > > > for that now. And that's fine but please add a comment
> > > > to that end.
> > > >
> > >
> > > Indeed, I wanted to keep things simple. I'll add a comment
> > > on the functions that are not generic but are the same on
> > > both implementations 'by chance'.
> > >
> > > > And when you do you will discover there's some stuff you
> > > > might be able to simplify, e.g. I don't yet see why
> > > > you would want AER support there.
> > > >
> > >
> > > I am not sure *why not*.
> > > I am 'afraid' people will not use the new port
> > > because of missing features and they can see the AER as one of them.
> > > Do you see a special issue in imitating Intel's behavior on AER?
> > >
> > > Thanks,
> > > Marcel
> >
> > Not really, fair enough. Just keep an eye out for caps
> > you don't need. E.g. it's worth considering whether
> > msi-x would be better since then you can just use
> > a fixed vector number without playing tricks like
> > you have to with msi.
> >
>
> Sure, thanks for the pointers!
> I suppose I can change later to msi-x in a follow up patch
> to avoid introducing new bugs from the beginning :)
>
> Thanks,
> Marcel
If I'm right and msix will be less code, that would be a
strage approach imho. But you decide.
> > > > > Marcel Apfelbaum (3):
> > > > > hw/pcie: Introduce a base class for PCI Express Root Ports
> > > > > hw/ioh3420: derive from PCI Express Root Port base class
> > > > > hw/pcie: Introduce Generic PCI Express Root Port
> > > > >
> > > > > default-configs/arm-softmmu.mak | 1 +
> > > > > default-configs/i386-softmmu.mak | 1 +
> > > > > default-configs/x86_64-softmmu.mak | 1 +
> > > > > hw/pci-bridge/Makefile.objs | 1 +
> > > > > hw/pci-bridge/ioh3420.c | 152 ++-----------------------
> > > > > hw/pci-bridge/pcie_root_port.c | 228 +++++++++++++++++++++++++++++++++++++
> > > > > include/hw/pci/pci.h | 1 +
> > > > > include/hw/pci/pcie_port.h | 18 +++
> > > > > 8 files changed, 258 insertions(+), 145 deletions(-)
> > > > > create mode 100644 hw/pci-bridge/pcie_root_port.c
> > > > >
> > > > > --
> > > > > 2.5.5
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port
2017-01-12 16:23 ` Marcel Apfelbaum
2017-01-12 16:46 ` Michael S. Tsirkin
@ 2017-01-13 8:35 ` Gerd Hoffmann
2017-01-15 16:59 ` Marcel Apfelbaum
1 sibling, 1 reply; 15+ messages in thread
From: Gerd Hoffmann @ 2017-01-13 8:35 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: Michael S. Tsirkin, qemu-devel
Hi,
> Sure, thanks for the pointers!
> I suppose I can change later to msi-x in a follow up patch
> to avoid introducing new bugs from the beginning :)
I'd use msi-x right from start to avoid any backward compatibility
issues.
cheers,
Gerd
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port
2017-01-13 8:35 ` Gerd Hoffmann
@ 2017-01-15 16:59 ` Marcel Apfelbaum
0 siblings, 0 replies; 15+ messages in thread
From: Marcel Apfelbaum @ 2017-01-15 16:59 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Michael S. Tsirkin, qemu-devel
On 01/13/2017 10:35 AM, Gerd Hoffmann wrote:
> Hi,
>
>> Sure, thanks for the pointers!
>> I suppose I can change later to msi-x in a follow up patch
>> to avoid introducing new bugs from the beginning :)
>
> I'd use msi-x right from start to avoid any backward compatibility
> issues.
>
Hi Gerd, Michael
I'll post a version with msi-x.
Thanks,
Marcel
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port
2017-01-11 12:18 [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port Marcel Apfelbaum
` (3 preceding siblings ...)
2017-01-12 15:56 ` [Qemu-devel] [PATCH V2 0/3] " Michael S. Tsirkin
@ 2017-01-16 16:46 ` Andrea Bolognani
2017-01-18 12:30 ` Marcel Apfelbaum
4 siblings, 1 reply; 15+ messages in thread
From: Andrea Bolognani @ 2017-01-16 16:46 UTC (permalink / raw)
To: Marcel Apfelbaum, qemu-devel; +Cc: mst
On Wed, 2017-01-11 at 14:18 +0200, Marcel Apfelbaum wrote:
> v1 -> v2:
> - Rebased on master.
>
> The Generic Root Port behaves the same as the
> Intel's IOH device with id 3420, without having
> Intel specific attributes.
>
> The device has two purposes:
> (1) Can be used on both X86 and ARM machines.
> (2) It will allow us to tweak the behaviour
> (e.g add vendor-specific PCI capabilities)
> - something that obviously cannot be done
> on a known device.
>
> Patch 1/3: Introduce a base class for Root Ports - most of the code
> is migrated from IOH3420 implementation.
> Patch 2/3: Derives the IOH3420 from the new base class
> Patch 3/3: Introduces the generic Root Port.
>
> Tested with Linux and Windows guests only on x86 hosts.
I tested this both on x86/q35 (Debian guest) and on
aarch64/virt (Fedora guest) very briefly, eg. started
the guest, performed some network I/O and shut it down.
It seems to be working fine :)
--
Andrea Bolognani / Red Hat / Virtualization
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port
2017-01-16 16:46 ` Andrea Bolognani
@ 2017-01-18 12:30 ` Marcel Apfelbaum
0 siblings, 0 replies; 15+ messages in thread
From: Marcel Apfelbaum @ 2017-01-18 12:30 UTC (permalink / raw)
To: Andrea Bolognani, qemu-devel; +Cc: mst
On 01/16/2017 06:46 PM, Andrea Bolognani wrote:
> On Wed, 2017-01-11 at 14:18 +0200, Marcel Apfelbaum wrote:
>> v1 -> v2:
>> - Rebased on master.
>>
>> The Generic Root Port behaves the same as the
>> Intel's IOH device with id 3420, without having
>> Intel specific attributes.
>>
>> The device has two purposes:
>> (1) Can be used on both X86 and ARM machines.
>> (2) It will allow us to tweak the behaviour
>> (e.g add vendor-specific PCI capabilities)
>> - something that obviously cannot be done
>> on a known device.
>>
>> Patch 1/3: Introduce a base class for Root Ports - most of the code
>> is migrated from IOH3420 implementation.
>> Patch 2/3: Derives the IOH3420 from the new base class
>> Patch 3/3: Introduces the generic Root Port.
>>
>> Tested with Linux and Windows guests only on x86 hosts.
>
> I tested this both on x86/q35 (Debian guest) and on
> aarch64/virt (Fedora guest) very briefly, eg. started
> the guest, performed some network I/O and shut it down.
>
> It seems to be working fine :)
Thanks for testing it! A tested-by tag would be appreciated for the next version :)
Marcel
>
> --
> Andrea Bolognani / Red Hat / Virtualization
>
^ permalink raw reply [flat|nested] 15+ messages in thread