* [RFC 0/2] pci_expander_brdige:acpi:Support pxb-pcie for ARM
@ 2020-02-13 7:49 Yubo Miao
2020-02-13 7:49 ` [RFC 1/2] arm: acpi: pci-expender-bus: Make arm to support PXB-PCIE Yubo Miao
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Yubo Miao @ 2020-02-13 7:49 UTC (permalink / raw)
To: peter.maydell, shannon.zhaosl
Cc: imammedo, miaoyubo, qemu-devel, xiexiangyou, mst
From: miaoyubo <miaoyubo@huawei.com>
Currently pxb-pcie is not supported by arm and only one
main host bridge is described in acpi tables, which means
it is not impossible to present different io numas for different
devices. This series of patches make arm to support PXB-PCIE.
Users can configure pxb-pcie with certain numa, Example command
is:
-device pxb-pcie,id=pci.7,bus_nr=128,numa_node=0,bus=pcie.0,addr=0x9
Since devices could not be plugged into pxb-pcie directly, one
pcie-root-port is auto plugged into the pxb, therefore, the devices
could be plugged into pxb-pcie.
With the patches,io numa could be presented to the guest by define a pxb-pcie
with the numa and plug the device into the pxb-pcie.
miaoyubo (2):
arm: acpi: pci-expender-bus: Make arm to support PXB-PCIE
pci-expender-bus:Add pcie-root-port to pxb-pcie under arm.
hw/arm/virt-acpi-build.c | 234 ++++++++++++++++++++++++++--
hw/pci-bridge/pci_expander_bridge.c | 9 ++
hw/pci-host/gpex.c | 4 +
include/hw/arm/virt.h | 1 +
include/hw/pci/pcie_port.h | 1 +
5 files changed, 238 insertions(+), 11 deletions(-)
--
2.19.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC 1/2] arm: acpi: pci-expender-bus: Make arm to support PXB-PCIE
2020-02-13 7:49 [RFC 0/2] pci_expander_brdige:acpi:Support pxb-pcie for ARM Yubo Miao
@ 2020-02-13 7:49 ` Yubo Miao
2020-02-13 10:23 ` Michael S. Tsirkin
2020-02-13 7:49 ` [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm Yubo Miao
2020-02-13 16:06 ` [RFC 0/2] pci_expander_brdige:acpi:Support pxb-pcie for ARM no-reply
2 siblings, 1 reply; 14+ messages in thread
From: Yubo Miao @ 2020-02-13 7:49 UTC (permalink / raw)
To: peter.maydell, shannon.zhaosl
Cc: imammedo, miaoyubo, qemu-devel, xiexiangyou, mst
From: miaoyubo <miaoyubo@huawei.com>
Currently virt machine is not supported by pxb-pcie,
and only one main host bridge described in ACPI tables.
Under this circumstance, different io numas for differnt devices
is not possible, in order to present io numas to the guest,
especially for host pssthrough devices. PXB-PCIE is supproted
by arm and certain resource is allocated for each pxb-pcie
in acpi table.
Signed-off-by: miaoyubo <miaoyubo@huawei.com>
---
hw/arm/virt-acpi-build.c | 234 +++++++++++++++++++++++++++++++++++++--
hw/pci-host/gpex.c | 4 +
include/hw/arm/virt.h | 1 +
3 files changed, 228 insertions(+), 11 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index bd5f771e9b..2e449d0098 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -49,6 +49,8 @@
#include "kvm_arm.h"
#include "migration/vmstate.h"
+#include "hw/arm/virt.h"
+#include "hw/pci/pci_bus.h"
#define ARM_SPI_BASE 32
static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
@@ -152,20 +154,227 @@ static void acpi_dsdt_add_virtio(Aml *scope,
}
static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
- uint32_t irq, bool use_highmem, bool highmem_ecam)
+ uint32_t irq, bool use_highmem, bool highmem_ecam,
+ VirtMachineState *vms)
{
int ecam_id = VIRT_ECAM_ID(highmem_ecam);
- Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
+ Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf, *dev;
int i, bus_no;
+ int count = 0;
hwaddr base_mmio = memmap[VIRT_PCIE_MMIO].base;
hwaddr size_mmio = memmap[VIRT_PCIE_MMIO].size;
hwaddr base_pio = memmap[VIRT_PCIE_PIO].base;
hwaddr size_pio = memmap[VIRT_PCIE_PIO].size;
hwaddr base_ecam = memmap[ecam_id].base;
hwaddr size_ecam = memmap[ecam_id].size;
+ /*
+ * 0x600000 would be enough for pxb device
+ * if it is too small, there is no enough space
+ * for a pcie device plugged in a pcie-root port
+ */
+ hwaddr size_addr = 0x600000;
+ hwaddr size_io = 0x4000;
int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
+ int root_bus_limit = 0xFF;
+ PCIBus *bus = NULL;
+ bus = VIRT_MACHINE(vms)->bus;
+
+ if (bus) {
+ QLIST_FOREACH(bus, &bus->child, sibling) {
+ uint8_t bus_num = pci_bus_num(bus);
+ uint8_t numa_node = pci_bus_numa_node(bus);
+
+ if (!pci_bus_is_root(bus)) {
+ continue;
+ }
+ if (bus_num < root_bus_limit) {
+ root_bus_limit = bus_num - 1;
+ }
+ count++;
+ dev = aml_device("PC%.02X", bus_num);
+ aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
+ aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
+ aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
+ aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
+ aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
+ aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
+ aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
+ aml_append(dev, aml_name_decl("_STR", aml_unicode("pxb Device")));
+ if (numa_node != NUMA_NODE_UNASSIGNED) {
+ method = aml_method("_PXM", 0, AML_NOTSERIALIZED);
+ aml_append(method, aml_return(aml_int(numa_node)));
+ aml_append(dev, method);
+ }
+ /* Declare the PCI Routing Table. */
+ Aml *rt_pkg = aml_varpackage(nr_pcie_buses * PCI_NUM_PINS);
+ for (bus_no = 0; bus_no < nr_pcie_buses; bus_no++) {
+ for (i = 0; i < PCI_NUM_PINS; i++) {
+ int gsi = (i + bus_no) % (PCI_NUM_PINS);
+ Aml *pkg = aml_package(4);
+ aml_append(pkg, aml_int((bus_no << 16) | 0xFFFF));
+ aml_append(pkg, aml_int(i));
+ aml_append(pkg, aml_name("GSI%d", gsi));
+ aml_append(pkg, aml_int(0));
+ aml_append(rt_pkg, pkg);
+ }
+ }
+ aml_append(dev, aml_name_decl("_PRT", rt_pkg));
+
+ for (i = 0; i < PCI_NUM_PINS; i++) {
+ uint32_t irqs = irq + i;
+ Aml *dev_gsi = aml_device("GSI%d", i);
+ aml_append(dev_gsi, aml_name_decl("_HID",
+ aml_string("PNP0C0F")));
+ aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
+ crs = aml_resource_template();
+ aml_append(crs,
+ aml_interrupt(AML_CONSUMER, AML_LEVEL,
+ AML_ACTIVE_HIGH,
+ AML_EXCLUSIVE, &irqs, 1));
+ aml_append(dev_gsi, aml_name_decl("_PRS", crs));
+ crs = aml_resource_template();
+ aml_append(crs,
+ aml_interrupt(AML_CONSUMER, AML_LEVEL,
+ AML_ACTIVE_HIGH,
+ AML_EXCLUSIVE, &irqs, 1));
+ aml_append(dev_gsi, aml_name_decl("_CRS", crs));
+ method = aml_method("_SRS", 1, AML_NOTSERIALIZED);
+ aml_append(dev_gsi, method);
+ aml_append(dev, dev_gsi);
+ }
+
+ method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
+ aml_append(method, aml_return(aml_int(base_ecam)));
+ aml_append(dev, method);
+
+ method = aml_method("_CRS", 0, AML_NOTSERIALIZED);
+ Aml *rbuf = aml_resource_template();
+ aml_append(rbuf,
+ aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED,
+ AML_POS_DECODE, 0x0000,
+ bus_num, bus_num + 1, 0x0000,
+ 2));
+ aml_append(rbuf,
+ aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
+ AML_MAX_FIXED, AML_NON_CACHEABLE,
+ AML_READ_WRITE, 0x0000,
+ base_mmio + size_mmio -
+ size_addr * count,
+ base_mmio + size_mmio - 1 -
+ size_addr * (count - 1),
+ 0x0000, size_addr));
+ aml_append(rbuf,
+ aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED,
+ AML_POS_DECODE, AML_ENTIRE_RANGE,
+ 0x0000, (size_pio) / 2 - size_io * count,
+ (size_pio / 2) - 1 - size_io * (count - 1),
+ base_pio, size_io));
+
+ if (use_highmem) {
+ hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
+ hwaddr size_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].size;
+
+ aml_append(rbuf,
+ aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
+ AML_MAX_FIXED, AML_NON_CACHEABLE,
+ AML_READ_WRITE, 0x0000,
+ base_mmio_high + size_mmio_high -
+ size_addr * count,
+ base_mmio_high + size_mmio_high -
+ 1 - size_addr * (count - 1),
+ 0x0000, size_addr));
+ }
+
+ aml_append(method, aml_name_decl("RBUF", rbuf));
+ aml_append(method, aml_return(rbuf));
+ aml_append(dev, method);
+ /* Declare an _OSC (OS Control Handoff) method */
+ aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
+ aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
+ method = aml_method("_OSC", 4, AML_NOTSERIALIZED);
+ aml_append(method,
+ aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
+
+ /*
+ * PCI Firmware Specification 3.0
+ * 4.5.1. _OSC Interface for PCI Host Bridge Devices
+ * The _OSC interface for a PCI/PCI-X/PCI Express hierarchy is
+ * identified by the Universal Unique IDentifier (UUID)
+ * 33DB4D5B-1FF7-401C-9657-7441C03DD766
+ */
+ UUID = aml_touuid("33DB4D5B-1FF7-401C-9657-7441C03DD766");
+ ifctx = aml_if(aml_equal(aml_arg(0), UUID));
+ aml_append(ifctx,
+ aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
+ aml_append(ifctx,
+ aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
+ aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
+ aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
+ /*
+ * Allow OS control for all 5 features:
+ * PCIeHotplug SHPCHotplug PME AER PCIeCapability.
+ */
+ aml_append(ifctx,
+ aml_store(aml_and(aml_name("CTRL"), aml_int(0x1F), NULL),
+ aml_name("CTRL")));
+
+ ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1))));
+ aml_append(ifctx1,
+ aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL),
+ aml_name("CDW1")));
+ aml_append(ifctx, ifctx1);
+
+ ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"),
+ aml_name("CTRL"))));
+ aml_append(ifctx1,
+ aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL),
+ aml_name("CDW1")));
+ aml_append(ifctx, ifctx1);
+
+ aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3")));
+ aml_append(ifctx, aml_return(aml_arg(3)));
+ aml_append(method, ifctx);
+
+ elsectx = aml_else();
+ aml_append(elsectx,
+ aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL),
+ aml_name("CDW1")));
+ aml_append(elsectx, aml_return(aml_arg(3)));
+ aml_append(method, elsectx);
+ aml_append(dev, method);
+
+ method = aml_method("_DSM", 4, AML_NOTSERIALIZED);
+
+ /*
+ * PCI Firmware Specification 3.0
+ * 4.6.1. _DSM for PCI Express Slot Information
+ * The UUID in _DSM in this context is
+ * {E5C937D0-3553-4D7A-9117-EA4D19C3434D}
+ */
+ UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
+ ifctx = aml_if(aml_equal(aml_arg(0), UUID));
+ ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
+ uint8_t byte_list[1] = {1};
+ buf = aml_buffer(1, byte_list);
+ aml_append(ifctx1, aml_return(buf));
+ aml_append(ifctx, ifctx1);
+ aml_append(method, ifctx);
+
+ byte_list[0] = 0;
+ buf = aml_buffer(1, byte_list);
+ aml_append(method, aml_return(buf));
+ aml_append(dev, method);
+
+ Aml *dev_rp0 = aml_device("%s", "RP0");
+ aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
+ aml_append(dev, dev_rp0);
+
+ aml_append(scope, dev);
+
+ }
+ }
- Aml *dev = aml_device("%s", "PCI0");
+ dev = aml_device("%s", "PCI0");
aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
@@ -219,16 +428,18 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
Aml *rbuf = aml_resource_template();
aml_append(rbuf,
aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
- 0x0000, 0x0000, nr_pcie_buses - 1, 0x0000,
- nr_pcie_buses));
+ 0x0000, 0x0000, root_bus_limit, 0x0000,
+ root_bus_limit + 1));
aml_append(rbuf,
aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000, base_mmio,
- base_mmio + size_mmio - 1, 0x0000, size_mmio));
+ base_mmio + size_mmio - 1 - size_addr * count,
+ 0x0000, size_mmio - size_addr * count));
aml_append(rbuf,
aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
- AML_ENTIRE_RANGE, 0x0000, 0x0000, size_pio - 1, base_pio,
- size_pio));
+ AML_ENTIRE_RANGE, 0x0000, 0x0000,
+ size_pio / 2 - 1 - size_io * count, base_pio,
+ size_pio / 2 - size_io * count));
if (use_highmem) {
hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
@@ -238,8 +449,9 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000,
base_mmio_high,
- base_mmio_high + size_mmio_high - 1, 0x0000,
- size_mmio_high));
+ base_mmio_high + size_mmio_high - 1 -
+ size_addr * count,
+ 0x0000, size_mmio_high - size_addr * count));
}
aml_append(method, aml_name_decl("RBUF", rbuf));
@@ -744,7 +956,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
(irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
- vms->highmem, vms->highmem_ecam);
+ vms->highmem, vms->highmem_ecam, vms);
if (vms->acpi_dev) {
build_ged_aml(scope, "\\_SB."GED_DEVICE,
HOTPLUG_HANDLER(vms->acpi_dev),
diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
index 0ca604dc62..2c18cdfec4 100644
--- a/hw/pci-host/gpex.c
+++ b/hw/pci-host/gpex.c
@@ -36,6 +36,7 @@
#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
+#include "hw/arm/virt.h"
/****************************************************************************
* GPEX host
@@ -98,6 +99,9 @@ static void gpex_host_realize(DeviceState *dev, Error **errp)
pci_swizzle_map_irq_fn, s, &s->io_mmio,
&s->io_ioport, 0, 4, TYPE_PCIE_BUS);
+#ifdef __aarch64__
+ VIRT_MACHINE(qdev_get_machine())->bus = pci->bus;
+#endif
qdev_set_parent_bus(DEVICE(&s->gpex_root), BUS(pci->bus));
pci_bus_set_route_irq_fn(pci->bus, gpex_route_intx_pin_to_irq);
qdev_init_nofail(DEVICE(&s->gpex_root));
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 71508bf40c..cfc65dd19b 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -140,6 +140,7 @@ typedef struct {
DeviceState *gic;
DeviceState *acpi_dev;
Notifier powerdown_notifier;
+ PCIBus *bus;
} VirtMachineState;
#define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
--
2.19.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm.
2020-02-13 7:49 [RFC 0/2] pci_expander_brdige:acpi:Support pxb-pcie for ARM Yubo Miao
2020-02-13 7:49 ` [RFC 1/2] arm: acpi: pci-expender-bus: Make arm to support PXB-PCIE Yubo Miao
@ 2020-02-13 7:49 ` Yubo Miao
2020-02-13 10:17 ` Michael S. Tsirkin
2020-02-13 13:51 ` Daniel P. Berrangé
2020-02-13 16:06 ` [RFC 0/2] pci_expander_brdige:acpi:Support pxb-pcie for ARM no-reply
2 siblings, 2 replies; 14+ messages in thread
From: Yubo Miao @ 2020-02-13 7:49 UTC (permalink / raw)
To: peter.maydell, shannon.zhaosl
Cc: imammedo, miaoyubo, qemu-devel, xiexiangyou, mst
From: miaoyubo <miaoyubo@huawei.com>
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 <miaoyubo@huawei.com>
---
hw/pci-bridge/pci_expander_bridge.c | 9 +++++++++
include/hw/pci/pcie_port.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 47aaaf8fd1..3d896dd452 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -15,6 +15,7 @@
#include "hw/pci/pci.h"
#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_host.h"
+#include "hw/pci/pcie_port.h"
#include "hw/qdev-properties.h"
#include "hw/pci/pci_bridge.h"
#include "qemu/range.h"
@@ -233,7 +234,15 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
ds = qdev_create(NULL, TYPE_PXB_HOST);
if (pcie) {
+#ifdef __aarch64__
+ bus = pci_root_bus_new(ds, "pxb-pcie-internal",
+ NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
+ bds = qdev_create(BUS(bus), "pcie-root-port");
+ bds->id = dev_name;
+ qdev_prop_set_uint8(bds, PCIE_ROOT_PORT_PROP_CHASSIS, pxb->bus_nr);
+#else
bus = pci_root_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
+#endif
} 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm.
2020-02-13 7:49 ` [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm Yubo Miao
@ 2020-02-13 10:17 ` Michael S. Tsirkin
2020-02-14 7:30 ` miaoyubo
2020-02-13 13:51 ` Daniel P. Berrangé
1 sibling, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2020-02-13 10:17 UTC (permalink / raw)
To: Yubo Miao
Cc: peter.maydell, imammedo, qemu-devel, xiexiangyou, shannon.zhaosl
On Thu, Feb 13, 2020 at 03:49:52PM +0800, Yubo Miao wrote:
> From: miaoyubo <miaoyubo@huawei.com>
>
> Since devices could not directly plugged into pxb-pcie,
Hmm is this different from the root port? intergrated devices
do exist for that actually.
> under arm,
how is arm special?
> one
> pcie-root port is plugged into pxb-pcie. Due to the bus for each pxb-pcie
> is defined as 2 in acpi dsdt tables(one for pxb-pcie, one for pcie-root-port),
> only one device could be plugged into one pxb-pcie.
So why can't we have users specify any number of root ports using
-device? then make acpi tables match the # of ports created?
>
> Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> ---
> hw/pci-bridge/pci_expander_bridge.c | 9 +++++++++
> include/hw/pci/pcie_port.h | 1 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 47aaaf8fd1..3d896dd452 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -15,6 +15,7 @@
> #include "hw/pci/pci.h"
> #include "hw/pci/pci_bus.h"
> #include "hw/pci/pci_host.h"
> +#include "hw/pci/pcie_port.h"
> #include "hw/qdev-properties.h"
> #include "hw/pci/pci_bridge.h"
> #include "qemu/range.h"
> @@ -233,7 +234,15 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>
> ds = qdev_create(NULL, TYPE_PXB_HOST);
> if (pcie) {
> +#ifdef __aarch64__
> + bus = pci_root_bus_new(ds, "pxb-pcie-internal",
> + NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> + bds = qdev_create(BUS(bus), "pcie-root-port");
> + bds->id = dev_name;
> + qdev_prop_set_uint8(bds, PCIE_ROOT_PORT_PROP_CHASSIS, pxb->bus_nr);
> +#else
> bus = pci_root_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> +#endif
What does all this have to do with building on aarch64?
> } else {
> bus = pci_root_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
> bds = qdev_create(BUS(bus), "pci-bridge");
> diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
> index 4b3d254b08..b41d473220 100644
> --- a/include/hw/pci/pcie_port.h
> +++ b/include/hw/pci/pcie_port.h
> @@ -64,6 +64,7 @@ int pcie_chassis_add_slot(struct PCIESlot *slot);
> void pcie_chassis_del_slot(PCIESlot *s);
>
> #define TYPE_PCIE_ROOT_PORT "pcie-root-port-base"
> +#define PCIE_ROOT_PORT_PROP_CHASSIS "chassis"
If you are going to do this, replace other instances of "chassis"
with the macro.
> #define PCIE_ROOT_PORT_CLASS(klass) \
> OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
> #define PCIE_ROOT_PORT_GET_CLASS(obj) \
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 1/2] arm: acpi: pci-expender-bus: Make arm to support PXB-PCIE
2020-02-13 7:49 ` [RFC 1/2] arm: acpi: pci-expender-bus: Make arm to support PXB-PCIE Yubo Miao
@ 2020-02-13 10:23 ` Michael S. Tsirkin
2020-02-14 7:28 ` miaoyubo
0 siblings, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2020-02-13 10:23 UTC (permalink / raw)
To: Yubo Miao
Cc: peter.maydell, imammedo, qemu-devel, xiexiangyou, shannon.zhaosl
On Thu, Feb 13, 2020 at 03:49:51PM +0800, Yubo Miao wrote:
> From: miaoyubo <miaoyubo@huawei.com>
>
> Currently virt machine is not supported by pxb-pcie,
> and only one main host bridge described in ACPI tables.
> Under this circumstance, different io numas for differnt devices
> is not possible, in order to present io numas to the guest,
> especially for host pssthrough devices. PXB-PCIE is supproted
> by arm and certain resource is allocated for each pxb-pcie
> in acpi table.
>
> Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> ---
> hw/arm/virt-acpi-build.c | 234 +++++++++++++++++++++++++++++++++++++--
> hw/pci-host/gpex.c | 4 +
> include/hw/arm/virt.h | 1 +
> 3 files changed, 228 insertions(+), 11 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index bd5f771e9b..2e449d0098 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -49,6 +49,8 @@
> #include "kvm_arm.h"
> #include "migration/vmstate.h"
>
> +#include "hw/arm/virt.h"
> +#include "hw/pci/pci_bus.h"
> #define ARM_SPI_BASE 32
>
> static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
> @@ -152,20 +154,227 @@ static void acpi_dsdt_add_virtio(Aml *scope,
> }
>
> static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> - uint32_t irq, bool use_highmem, bool highmem_ecam)
> + uint32_t irq, bool use_highmem, bool highmem_ecam,
> + VirtMachineState *vms)
> {
> int ecam_id = VIRT_ECAM_ID(highmem_ecam);
> - Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
> + Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf, *dev;
> int i, bus_no;
> + int count = 0;
> hwaddr base_mmio = memmap[VIRT_PCIE_MMIO].base;
> hwaddr size_mmio = memmap[VIRT_PCIE_MMIO].size;
> hwaddr base_pio = memmap[VIRT_PCIE_PIO].base;
> hwaddr size_pio = memmap[VIRT_PCIE_PIO].size;
> hwaddr base_ecam = memmap[ecam_id].base;
> hwaddr size_ecam = memmap[ecam_id].size;
> + /*
> + * 0x600000 would be enough for pxb device
> + * if it is too small, there is no enough space
> + * for a pcie device plugged in a pcie-root port
> + */
> + hwaddr size_addr = 0x600000;
> + hwaddr size_io = 0x4000;
> int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
> + int root_bus_limit = 0xFF;
> + PCIBus *bus = NULL;
> + bus = VIRT_MACHINE(vms)->bus;
> +
> + if (bus) {
> + QLIST_FOREACH(bus, &bus->child, sibling) {
> + uint8_t bus_num = pci_bus_num(bus);
> + uint8_t numa_node = pci_bus_numa_node(bus);
> +
> + if (!pci_bus_is_root(bus)) {
> + continue;
> + }
> + if (bus_num < root_bus_limit) {
> + root_bus_limit = bus_num - 1;
> + }
> + count++;
> + dev = aml_device("PC%.02X", bus_num);
> + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
> + aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> + aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> + aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> + aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
> + aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
> + aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
> + aml_append(dev, aml_name_decl("_STR", aml_unicode("pxb Device")));
> + if (numa_node != NUMA_NODE_UNASSIGNED) {
> + method = aml_method("_PXM", 0, AML_NOTSERIALIZED);
> + aml_append(method, aml_return(aml_int(numa_node)));
> + aml_append(dev, method);
> + }
> + /* Declare the PCI Routing Table. */
> + Aml *rt_pkg = aml_varpackage(nr_pcie_buses * PCI_NUM_PINS);
> + for (bus_no = 0; bus_no < nr_pcie_buses; bus_no++) {
> + for (i = 0; i < PCI_NUM_PINS; i++) {
> + int gsi = (i + bus_no) % (PCI_NUM_PINS);
> + Aml *pkg = aml_package(4);
> + aml_append(pkg, aml_int((bus_no << 16) | 0xFFFF));
> + aml_append(pkg, aml_int(i));
> + aml_append(pkg, aml_name("GSI%d", gsi));
> + aml_append(pkg, aml_int(0));
> + aml_append(rt_pkg, pkg);
> + }
> + }
> + aml_append(dev, aml_name_decl("_PRT", rt_pkg));
> +
> + for (i = 0; i < PCI_NUM_PINS; i++) {
> + uint32_t irqs = irq + i;
> + Aml *dev_gsi = aml_device("GSI%d", i);
> + aml_append(dev_gsi, aml_name_decl("_HID",
> + aml_string("PNP0C0F")));
> + aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
> + crs = aml_resource_template();
> + aml_append(crs,
> + aml_interrupt(AML_CONSUMER, AML_LEVEL,
> + AML_ACTIVE_HIGH,
> + AML_EXCLUSIVE, &irqs, 1));
> + aml_append(dev_gsi, aml_name_decl("_PRS", crs));
> + crs = aml_resource_template();
> + aml_append(crs,
> + aml_interrupt(AML_CONSUMER, AML_LEVEL,
> + AML_ACTIVE_HIGH,
> + AML_EXCLUSIVE, &irqs, 1));
> + aml_append(dev_gsi, aml_name_decl("_CRS", crs));
> + method = aml_method("_SRS", 1, AML_NOTSERIALIZED);
> + aml_append(dev_gsi, method);
> + aml_append(dev, dev_gsi);
> + }
> +
> + method = aml_method("_CBA", 0, AML_NOTSERIALIZED);
> + aml_append(method, aml_return(aml_int(base_ecam)));
> + aml_append(dev, method);
> +
> + method = aml_method("_CRS", 0, AML_NOTSERIALIZED);
> + Aml *rbuf = aml_resource_template();
> + aml_append(rbuf,
> + aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED,
> + AML_POS_DECODE, 0x0000,
> + bus_num, bus_num + 1, 0x0000,
> + 2));
> + aml_append(rbuf,
> + aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> + AML_MAX_FIXED, AML_NON_CACHEABLE,
> + AML_READ_WRITE, 0x0000,
> + base_mmio + size_mmio -
> + size_addr * count,
> + base_mmio + size_mmio - 1 -
> + size_addr * (count - 1),
> + 0x0000, size_addr));
> + aml_append(rbuf,
> + aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED,
> + AML_POS_DECODE, AML_ENTIRE_RANGE,
> + 0x0000, (size_pio) / 2 - size_io * count,
> + (size_pio / 2) - 1 - size_io * (count - 1),
> + base_pio, size_io));
> +
> + if (use_highmem) {
> + hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
> + hwaddr size_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].size;
> +
> + aml_append(rbuf,
> + aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> + AML_MAX_FIXED, AML_NON_CACHEABLE,
> + AML_READ_WRITE, 0x0000,
> + base_mmio_high + size_mmio_high -
> + size_addr * count,
> + base_mmio_high + size_mmio_high -
> + 1 - size_addr * (count - 1),
> + 0x0000, size_addr));
> + }
> +
> + aml_append(method, aml_name_decl("RBUF", rbuf));
> + aml_append(method, aml_return(rbuf));
> + aml_append(dev, method);
> + /* Declare an _OSC (OS Control Handoff) method */
> + aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
> + aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
> + method = aml_method("_OSC", 4, AML_NOTSERIALIZED);
> + aml_append(method,
> + aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
> +
> + /*
> + * PCI Firmware Specification 3.0
> + * 4.5.1. _OSC Interface for PCI Host Bridge Devices
> + * The _OSC interface for a PCI/PCI-X/PCI Express hierarchy is
> + * identified by the Universal Unique IDentifier (UUID)
> + * 33DB4D5B-1FF7-401C-9657-7441C03DD766
> + */
> + UUID = aml_touuid("33DB4D5B-1FF7-401C-9657-7441C03DD766");
> + ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> + aml_append(ifctx,
> + aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
> + aml_append(ifctx,
> + aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
> + aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
> + aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
> + /*
> + * Allow OS control for all 5 features:
> + * PCIeHotplug SHPCHotplug PME AER PCIeCapability.
> + */
> + aml_append(ifctx,
> + aml_store(aml_and(aml_name("CTRL"), aml_int(0x1F), NULL),
> + aml_name("CTRL")));
> +
> + ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1))));
> + aml_append(ifctx1,
> + aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL),
> + aml_name("CDW1")));
> + aml_append(ifctx, ifctx1);
> +
> + ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"),
> + aml_name("CTRL"))));
> + aml_append(ifctx1,
> + aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL),
> + aml_name("CDW1")));
> + aml_append(ifctx, ifctx1);
> +
> + aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3")));
> + aml_append(ifctx, aml_return(aml_arg(3)));
> + aml_append(method, ifctx);
> +
> + elsectx = aml_else();
> + aml_append(elsectx,
> + aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL),
> + aml_name("CDW1")));
> + aml_append(elsectx, aml_return(aml_arg(3)));
> + aml_append(method, elsectx);
> + aml_append(dev, method);
> +
> + method = aml_method("_DSM", 4, AML_NOTSERIALIZED);
> +
> + /*
> + * PCI Firmware Specification 3.0
> + * 4.6.1. _DSM for PCI Express Slot Information
> + * The UUID in _DSM in this context is
> + * {E5C937D0-3553-4D7A-9117-EA4D19C3434D}
> + */
> + UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
> + ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> + ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> + uint8_t byte_list[1] = {1};
> + buf = aml_buffer(1, byte_list);
> + aml_append(ifctx1, aml_return(buf));
> + aml_append(ifctx, ifctx1);
> + aml_append(method, ifctx);
> +
> + byte_list[0] = 0;
> + buf = aml_buffer(1, byte_list);
> + aml_append(method, aml_return(buf));
> + aml_append(dev, method);
> +
> + Aml *dev_rp0 = aml_device("%s", "RP0");
> + aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
> + aml_append(dev, dev_rp0);
> +
> + aml_append(scope, dev);
> +
> + }
> + }
There's a bunch of code duplicated here. Please refactor creating
APIs for reused code.
>
> - Aml *dev = aml_device("%s", "PCI0");
> + dev = aml_device("%s", "PCI0");
> aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
> aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
> @@ -219,16 +428,18 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> Aml *rbuf = aml_resource_template();
> aml_append(rbuf,
> aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
> - 0x0000, 0x0000, nr_pcie_buses - 1, 0x0000,
> - nr_pcie_buses));
> + 0x0000, 0x0000, root_bus_limit, 0x0000,
> + root_bus_limit + 1));
> aml_append(rbuf,
> aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
> AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000, base_mmio,
> - base_mmio + size_mmio - 1, 0x0000, size_mmio));
> + base_mmio + size_mmio - 1 - size_addr * count,
> + 0x0000, size_mmio - size_addr * count));
> aml_append(rbuf,
> aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
> - AML_ENTIRE_RANGE, 0x0000, 0x0000, size_pio - 1, base_pio,
> - size_pio));
> + AML_ENTIRE_RANGE, 0x0000, 0x0000,
> + size_pio / 2 - 1 - size_io * count, base_pio,
> + size_pio / 2 - size_io * count));
>
> if (use_highmem) {
> hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
> @@ -238,8 +449,9 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
> AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000,
> base_mmio_high,
> - base_mmio_high + size_mmio_high - 1, 0x0000,
> - size_mmio_high));
> + base_mmio_high + size_mmio_high - 1 -
> + size_addr * count,
> + 0x0000, size_mmio_high - size_addr * count));
> }
>
> aml_append(method, aml_name_decl("RBUF", rbuf));
> @@ -744,7 +956,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
> acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
> - vms->highmem, vms->highmem_ecam);
> + vms->highmem, vms->highmem_ecam, vms);
> if (vms->acpi_dev) {
> build_ged_aml(scope, "\\_SB."GED_DEVICE,
> HOTPLUG_HANDLER(vms->acpi_dev),
> diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
> index 0ca604dc62..2c18cdfec4 100644
> --- a/hw/pci-host/gpex.c
> +++ b/hw/pci-host/gpex.c
> @@ -36,6 +36,7 @@
> #include "hw/qdev-properties.h"
> #include "migration/vmstate.h"
> #include "qemu/module.h"
> +#include "hw/arm/virt.h"
>
> /****************************************************************************
> * GPEX host
> @@ -98,6 +99,9 @@ static void gpex_host_realize(DeviceState *dev, Error **errp)
> pci_swizzle_map_irq_fn, s, &s->io_mmio,
> &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
>
> +#ifdef __aarch64__
> + VIRT_MACHINE(qdev_get_machine())->bus = pci->bus;
> +#endif
> qdev_set_parent_bus(DEVICE(&s->gpex_root), BUS(pci->bus));
> pci_bus_set_route_irq_fn(pci->bus, gpex_route_intx_pin_to_irq);
> qdev_init_nofail(DEVICE(&s->gpex_root));
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 71508bf40c..cfc65dd19b 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -140,6 +140,7 @@ typedef struct {
> DeviceState *gic;
> DeviceState *acpi_dev;
> Notifier powerdown_notifier;
> + PCIBus *bus;
> } VirtMachineState;
Given you only support one bus, why not just look the device
up based on type?
>
> #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm.
2020-02-13 7:49 ` [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm Yubo Miao
2020-02-13 10:17 ` Michael S. Tsirkin
@ 2020-02-13 13:51 ` Daniel P. Berrangé
2020-02-14 7:25 ` miaoyubo
1 sibling, 1 reply; 14+ messages in thread
From: Daniel P. Berrangé @ 2020-02-13 13:51 UTC (permalink / raw)
To: Yubo Miao
Cc: peter.maydell, mst, qemu-devel, xiexiangyou, shannon.zhaosl, imammedo
On Thu, Feb 13, 2020 at 03:49:52PM +0800, Yubo Miao wrote:
> From: miaoyubo <miaoyubo@huawei.com>
>
> Since devices could not directly plugged into pxb-pcie, 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 <miaoyubo@huawei.com>
> ---
> hw/pci-bridge/pci_expander_bridge.c | 9 +++++++++
> include/hw/pci/pcie_port.h | 1 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 47aaaf8fd1..3d896dd452 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -15,6 +15,7 @@
> #include "hw/pci/pci.h"
> #include "hw/pci/pci_bus.h"
> #include "hw/pci/pci_host.h"
> +#include "hw/pci/pcie_port.h"
> #include "hw/qdev-properties.h"
> #include "hw/pci/pci_bridge.h"
> #include "qemu/range.h"
> @@ -233,7 +234,15 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>
> ds = qdev_create(NULL, TYPE_PXB_HOST);
> if (pcie) {
> +#ifdef __aarch64__
> + bus = pci_root_bus_new(ds, "pxb-pcie-internal",
> + NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> + bds = qdev_create(BUS(bus), "pcie-root-port");
> + bds->id = dev_name;
> + qdev_prop_set_uint8(bds, PCIE_ROOT_PORT_PROP_CHASSIS, pxb->bus_nr);
> +#else
> bus = pci_root_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> +#endif
> } 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 :|
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/2] pci_expander_brdige:acpi:Support pxb-pcie for ARM
2020-02-13 7:49 [RFC 0/2] pci_expander_brdige:acpi:Support pxb-pcie for ARM Yubo Miao
2020-02-13 7:49 ` [RFC 1/2] arm: acpi: pci-expender-bus: Make arm to support PXB-PCIE Yubo Miao
2020-02-13 7:49 ` [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm Yubo Miao
@ 2020-02-13 16:06 ` no-reply
2 siblings, 0 replies; 14+ messages in thread
From: no-reply @ 2020-02-13 16:06 UTC (permalink / raw)
To: miaoyubo
Cc: peter.maydell, mst, qemu-devel, xiexiangyou, shannon.zhaosl,
miaoyubo, imammedo
Patchew URL: https://patchew.org/QEMU/20200213074952.544-1-miaoyubo@huawei.com/
Hi,
This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===
Using expected file 'tests/data/acpi/virt/SPCR'
Looking for expected file 'tests/data/acpi/virt/DSDT'
Using expected file 'tests/data/acpi/virt/DSDT'
ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/bios-tables-test.c:490:test_acpi_asl: assertion failed: (all_tables_match)
acpi-test: Warning! DSDT binary file mismatch. Actual [aml:/tmp/aml-FO5XF0], Expected [aml:tests/data/acpi/virt/DSDT].
to see ASL diff between mismatched files install IASL, rebuild QEMU from scratch and re-run tests with V=1 environment variable set**
ERROR:/tmp/qemu-test/src/tests/qtest/bios-tables-test.c:490:test_acpi_asl: assertion failed: (all_tables_match)
make: *** [check-qtest-aarch64] Error 1
make: *** Waiting for unfinished jobs....
TEST check-qtest-x86_64: tests/qtest/device-plug-test
TEST check-qtest-x86_64: tests/qtest/drive_del-test
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=3c1e54fc24d3488783ca8e67775524a0', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-5h5iepzo/src/docker-src.2020-02-13-10.53.26.32512:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=3c1e54fc24d3488783ca8e67775524a0
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-5h5iepzo/src'
make: *** [docker-run-test-quick@centos7] Error 2
real 12m55.032s
user 0m8.477s
The full log is available at
http://patchew.org/logs/20200213074952.544-1-miaoyubo@huawei.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm.
2020-02-13 13:51 ` Daniel P. Berrangé
@ 2020-02-14 7:25 ` miaoyubo
2020-02-14 10:24 ` Daniel P. Berrangé
0 siblings, 1 reply; 14+ messages in thread
From: miaoyubo @ 2020-02-14 7:25 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: peter.maydell, mst, qemu-devel, Xiexiangyou, shannon.zhaosl, imammedo
> -----Original Message-----
> From: Daniel P. Berrangé [mailto:berrange@redhat.com]
> Sent: Thursday, February 13, 2020 9:52 PM
> To: miaoyubo <miaoyubo@huawei.com>
> Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> imammedo@redhat.com; qemu-devel@nongnu.org; Xiexiangyou
> <xiexiangyou@huawei.com>; mst@redhat.com
> 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 <miaoyubo@huawei.com>
> >
> > 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 <miaoyubo@huawei.com>
> > ---
> > hw/pci-bridge/pci_expander_bridge.c | 9 +++++++++
> > include/hw/pci/pcie_port.h | 1 +
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/hw/pci-bridge/pci_expander_bridge.c
> > b/hw/pci-bridge/pci_expander_bridge.c
> > index 47aaaf8fd1..3d896dd452 100644
> > --- a/hw/pci-bridge/pci_expander_bridge.c
> > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > @@ -15,6 +15,7 @@
> > #include "hw/pci/pci.h"
> > #include "hw/pci/pci_bus.h"
> > #include "hw/pci/pci_host.h"
> > +#include "hw/pci/pcie_port.h"
> > #include "hw/qdev-properties.h"
> > #include "hw/pci/pci_bridge.h"
> > #include "qemu/range.h"
> > @@ -233,7 +234,15 @@ static void pxb_dev_realize_common(PCIDevice
> > *dev, bool pcie, Error **errp)
> >
> > ds = qdev_create(NULL, TYPE_PXB_HOST);
> > if (pcie) {
> > +#ifdef __aarch64__
> > + bus = pci_root_bus_new(ds, "pxb-pcie-internal",
> > + NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> > + bds = qdev_create(BUS(bus), "pcie-root-port");
> > + bds->id = dev_name;
> > + qdev_prop_set_uint8(bds, PCIE_ROOT_PORT_PROP_CHASSIS,
> > +pxb->bus_nr); #else
> > bus = pci_root_bus_new(ds, dev_name, NULL, NULL, 0,
> > TYPE_PXB_PCIE_BUS);
> > +#endif
> > } 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [RFC 1/2] arm: acpi: pci-expender-bus: Make arm to support PXB-PCIE
2020-02-13 10:23 ` Michael S. Tsirkin
@ 2020-02-14 7:28 ` miaoyubo
0 siblings, 0 replies; 14+ messages in thread
From: miaoyubo @ 2020-02-14 7:28 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: peter.maydell, imammedo, qemu-devel, Xiexiangyou, shannon.zhaosl
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Thursday, February 13, 2020 6:23 PM
> To: miaoyubo <miaoyubo@huawei.com>
> Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com; Xiexiangyou
> <xiexiangyou@huawei.com>; imammedo@redhat.com; qemu-
> devel@nongnu.org
> Subject: Re: [RFC 1/2] arm: acpi: pci-expender-bus: Make arm to support
> PXB-PCIE
>
> On Thu, Feb 13, 2020 at 03:49:51PM +0800, Yubo Miao wrote:
> > From: miaoyubo <miaoyubo@huawei.com>
> >
> > Currently virt machine is not supported by pxb-pcie, and only one main
> > host bridge described in ACPI tables.
> > Under this circumstance, different io numas for differnt devices is
> > not possible, in order to present io numas to the guest, especially
> > for host pssthrough devices. PXB-PCIE is supproted by arm and certain
> > resource is allocated for each pxb-pcie in acpi table.
> >
> > Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> > ---
> > hw/arm/virt-acpi-build.c | 234
> +++++++++++++++++++++++++++++++++++++--
> > hw/pci-host/gpex.c | 4 +
> > include/hw/arm/virt.h | 1 +
> > 3 files changed, 228 insertions(+), 11 deletions(-)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index
> > bd5f771e9b..2e449d0098 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -49,6 +49,8 @@
> > #include "kvm_arm.h"
> > #include "migration/vmstate.h"
> >
> > +#include "hw/arm/virt.h"
> > +#include "hw/pci/pci_bus.h"
> > #define ARM_SPI_BASE 32
> >
> > + /*
> > + * PCI Firmware Specification 3.0
> > + * 4.6.1. _DSM for PCI Express Slot Information
> > + * The UUID in _DSM in this context is
> > + * {E5C937D0-3553-4D7A-9117-EA4D19C3434D}
> > + */
> > + UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
> > + ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> > + ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> > + uint8_t byte_list[1] = {1};
> > + buf = aml_buffer(1, byte_list);
> > + aml_append(ifctx1, aml_return(buf));
> > + aml_append(ifctx, ifctx1);
> > + aml_append(method, ifctx);
> > +
> > + byte_list[0] = 0;
> > + buf = aml_buffer(1, byte_list);
> > + aml_append(method, aml_return(buf));
> > + aml_append(dev, method);
> > +
> > + Aml *dev_rp0 = aml_device("%s", "RP0");
> > + aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
> > + aml_append(dev, dev_rp0);
> > +
> > + aml_append(scope, dev);
> > +
> > + }
> > + }
>
>
> There's a bunch of code duplicated here. Please refactor creating APIs for
> reused code.
>
Thanks for your reply, this would be done by patch V2.
> >
> > - Aml *dev = aml_device("%s", "PCI0");
> > + dev = aml_device("%s", "PCI0");
> > aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
> > aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> > aml_append(dev, aml_name_decl("_SEG", aml_int(0))); @@ -219,16
> > +428,18 @@ static void acpi_dsdt_add_pci(Aml *scope, const
> MemMapEntry *memmap,
> > Aml *rbuf = aml_resource_template();
> > aml_append(rbuf,
> > aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED,
> AML_POS_DECODE,
> > - 0x0000, 0x0000, nr_pcie_buses - 1, 0x0000,
> > - nr_pcie_buses));
> > + 0x0000, 0x0000, root_bus_limit, 0x0000,
> > + root_bus_limit + 1));
> > aml_append(rbuf,
> > aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> AML_MAX_FIXED,
> > AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000,
> base_mmio,
> > - base_mmio + size_mmio - 1, 0x0000, size_mmio));
> > + base_mmio + size_mmio - 1 - size_addr * count,
> > + 0x0000, size_mmio - size_addr * count));
> > aml_append(rbuf,
> > aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED,
> AML_POS_DECODE,
> > - AML_ENTIRE_RANGE, 0x0000, 0x0000, size_pio - 1, base_pio,
> > - size_pio));
> > + AML_ENTIRE_RANGE, 0x0000, 0x0000,
> > + size_pio / 2 - 1 - size_io * count, base_pio,
> > + size_pio / 2 - size_io * count));
> >
> > if (use_highmem) {
> > hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
> @@
> > -238,8 +449,9 @@ static void acpi_dsdt_add_pci(Aml *scope, const
> MemMapEntry *memmap,
> > aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> AML_MAX_FIXED,
> > AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000,
> > base_mmio_high,
> > - base_mmio_high + size_mmio_high - 1, 0x0000,
> > - size_mmio_high));
> > + base_mmio_high + size_mmio_high - 1 -
> > + size_addr * count,
> > + 0x0000, size_mmio_high - size_addr *
> > + count));
> > }
> >
> > aml_append(method, aml_name_decl("RBUF", rbuf)); @@ -744,7
> +956,7
> > @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState
> *vms)
> > acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> > (irqmap[VIRT_MMIO] + ARM_SPI_BASE),
> NUM_VIRTIO_TRANSPORTS);
> > acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] +
> ARM_SPI_BASE),
> > - vms->highmem, vms->highmem_ecam);
> > + vms->highmem, vms->highmem_ecam, vms);
> > if (vms->acpi_dev) {
> > build_ged_aml(scope, "\\_SB."GED_DEVICE,
> > HOTPLUG_HANDLER(vms->acpi_dev), diff --git
> > a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c index 0ca604dc62..2c18cdfec4
> > 100644
> > --- a/hw/pci-host/gpex.c
> > +++ b/hw/pci-host/gpex.c
> > @@ -36,6 +36,7 @@
> > #include "hw/qdev-properties.h"
> > #include "migration/vmstate.h"
> > #include "qemu/module.h"
> > +#include "hw/arm/virt.h"
> >
> >
> /**********************************************************
> ******************
> > * GPEX host
> > @@ -98,6 +99,9 @@ static void gpex_host_realize(DeviceState *dev, Error
> **errp)
> > pci_swizzle_map_irq_fn, s, &s->io_mmio,
> > &s->io_ioport, 0, 4,
> > TYPE_PCIE_BUS);
> >
> > +#ifdef __aarch64__
> > + VIRT_MACHINE(qdev_get_machine())->bus = pci->bus; #endif
> > qdev_set_parent_bus(DEVICE(&s->gpex_root), BUS(pci->bus));
> > pci_bus_set_route_irq_fn(pci->bus, gpex_route_intx_pin_to_irq);
> > qdev_init_nofail(DEVICE(&s->gpex_root));
> > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index
> > 71508bf40c..cfc65dd19b 100644
> > --- a/include/hw/arm/virt.h
> > +++ b/include/hw/arm/virt.h
> > @@ -140,6 +140,7 @@ typedef struct {
> > DeviceState *gic;
> > DeviceState *acpi_dev;
> > Notifier powerdown_notifier;
> > + PCIBus *bus;
> > } VirtMachineState;
>
>
> Given you only support one bus, why not just look the device up based on
> type?
>
"Given you only support one bus"
What does this mean? the patch enables to define multiply pxb-pcie devices,
and each pxb-pcie is allocated with two bus numbers in acpi tables.
"why not just look the device up based on type?"
We need the structure bus to get the numa_id defined by the user.
> >
> > #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM :
> > VIRT_PCIE_ECAM)
> > --
> > 2.19.1
> >
Regards,
Miao
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm.
2020-02-13 10:17 ` Michael S. Tsirkin
@ 2020-02-14 7:30 ` miaoyubo
0 siblings, 0 replies; 14+ messages in thread
From: miaoyubo @ 2020-02-14 7:30 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: peter.maydell, imammedo, qemu-devel, Xiexiangyou, shannon.zhaosl
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Thursday, February 13, 2020 6:17 PM
> To: miaoyubo <miaoyubo@huawei.com>
> Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com; Xiexiangyou
> <xiexiangyou@huawei.com>; imammedo@redhat.com; qemu-
> devel@nongnu.org
> 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 <miaoyubo@huawei.com>
> >
> > Since devices could not directly plugged into pxb-pcie,
>
> Hmm is this different from the root port? intergrated devices do exist for
> that actually.
>
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 <miaoyubo@huawei.com>
> > ---
> > hw/pci-bridge/pci_expander_bridge.c | 9 +++++++++
> > include/hw/pci/pcie_port.h | 1 +
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/hw/pci-bridge/pci_expander_bridge.c
> > b/hw/pci-bridge/pci_expander_bridge.c
> > index 47aaaf8fd1..3d896dd452 100644
> > --- a/hw/pci-bridge/pci_expander_bridge.c
> > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > @@ -15,6 +15,7 @@
> > #include "hw/pci/pci.h"
> > #include "hw/pci/pci_bus.h"
> > #include "hw/pci/pci_host.h"
> > +#include "hw/pci/pcie_port.h"
> > #include "hw/qdev-properties.h"
> > #include "hw/pci/pci_bridge.h"
> > #include "qemu/range.h"
> > @@ -233,7 +234,15 @@ static void pxb_dev_realize_common(PCIDevice
> > *dev, bool pcie, Error **errp)
> >
> > ds = qdev_create(NULL, TYPE_PXB_HOST);
> > if (pcie) {
> > +#ifdef __aarch64__
> > + bus = pci_root_bus_new(ds, "pxb-pcie-internal",
> > + NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> > + bds = qdev_create(BUS(bus), "pcie-root-port");
> > + bds->id = dev_name;
> > + qdev_prop_set_uint8(bds, PCIE_ROOT_PORT_PROP_CHASSIS,
> > +pxb->bus_nr); #else
> > bus = pci_root_bus_new(ds, dev_name, NULL, NULL, 0,
> > TYPE_PXB_PCIE_BUS);
> > +#endif
>
> What does all this have to do with building on aarch64?
>
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm.
2020-02-14 7:25 ` miaoyubo
@ 2020-02-14 10:24 ` Daniel P. Berrangé
2020-02-15 8:59 ` miaoyubo
0 siblings, 1 reply; 14+ messages in thread
From: Daniel P. Berrangé @ 2020-02-14 10:24 UTC (permalink / raw)
To: miaoyubo
Cc: peter.maydell, mst, qemu-devel, Xiexiangyou, shannon.zhaosl, imammedo
On Fri, Feb 14, 2020 at 07:25:43AM +0000, miaoyubo wrote:
>
> > -----Original Message-----
> > From: Daniel P. Berrangé [mailto:berrange@redhat.com]
> > Sent: Thursday, February 13, 2020 9:52 PM
> > To: miaoyubo <miaoyubo@huawei.com>
> > Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> > imammedo@redhat.com; qemu-devel@nongnu.org; Xiexiangyou
> > <xiexiangyou@huawei.com>; mst@redhat.com
> > 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 <miaoyubo@huawei.com>
> > >
> > > 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
--
|: 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 :|
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm.
2020-02-14 10:24 ` Daniel P. Berrangé
@ 2020-02-15 8:59 ` miaoyubo
2020-02-24 12:36 ` Daniel P. Berrangé
0 siblings, 1 reply; 14+ messages in thread
From: miaoyubo @ 2020-02-15 8:59 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: peter.maydell, mst, qemu-devel, Xiexiangyou, shannon.zhaosl, imammedo
> -----Original Message-----
> From: Daniel P. Berrangé [mailto:berrange@redhat.com]
> Sent: Friday, February 14, 2020 6:25 PM
> To: miaoyubo <miaoyubo@huawei.com>
> Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> imammedo@redhat.com; qemu-devel@nongnu.org; Xiexiangyou
> <xiexiangyou@huawei.com>; mst@redhat.com
> 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:berrange@redhat.com]
> > > Sent: Thursday, February 13, 2020 9:52 PM
> > > To: miaoyubo <miaoyubo@huawei.com>
> > > Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> > > imammedo@redhat.com; qemu-devel@nongnu.org; Xiexiangyou
> > > <xiexiangyou@huawei.com>; mst@redhat.com
> > > 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 <miaoyubo@huawei.com>
> > > >
> > > > 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm.
2020-02-15 8:59 ` miaoyubo
@ 2020-02-24 12:36 ` Daniel P. Berrangé
2020-02-25 1:54 ` miaoyubo
0 siblings, 1 reply; 14+ messages in thread
From: Daniel P. Berrangé @ 2020-02-24 12:36 UTC (permalink / raw)
To: miaoyubo
Cc: peter.maydell, mst, qemu-devel, Xiexiangyou, shannon.zhaosl, imammedo
On Sat, Feb 15, 2020 at 08:59:28AM +0000, miaoyubo wrote:
>
> > -----Original Message-----
> > From: Daniel P. Berrangé [mailto:berrange@redhat.com]
> > Sent: Friday, February 14, 2020 6:25 PM
> > To: miaoyubo <miaoyubo@huawei.com>
> > Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> > imammedo@redhat.com; qemu-devel@nongnu.org; Xiexiangyou
> > <xiexiangyou@huawei.com>; mst@redhat.com
> > 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:berrange@redhat.com]
> > > > Sent: Thursday, February 13, 2020 9:52 PM
> > > > To: miaoyubo <miaoyubo@huawei.com>
> > > > Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> > > > imammedo@redhat.com; qemu-devel@nongnu.org; Xiexiangyou
> > > > <xiexiangyou@huawei.com>; mst@redhat.com
> > > > 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 <miaoyubo@huawei.com>
> > > > >
> > > > > 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
--
|: 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 :|
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm.
2020-02-24 12:36 ` Daniel P. Berrangé
@ 2020-02-25 1:54 ` miaoyubo
0 siblings, 0 replies; 14+ messages in thread
From: miaoyubo @ 2020-02-25 1:54 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: peter.maydell, mst, qemu-devel, Xiexiangyou, shannon.zhaosl, imammedo
> -----Original Message-----
> From: Daniel P. Berrangé [mailto:berrange@redhat.com]
> Sent: Monday, February 24, 2020 8:36 PM
> To: miaoyubo <miaoyubo@huawei.com>
> Cc: peter.maydell@linaro.org; mst@redhat.com; qemu-devel@nongnu.org;
> Xiexiangyou <xiexiangyou@huawei.com>; shannon.zhaosl@gmail.com;
> imammedo@redhat.com
> 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:berrange@redhat.com]
> > > Sent: Friday, February 14, 2020 6:25 PM
> > > To: miaoyubo <miaoyubo@huawei.com>
> > > Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> > > imammedo@redhat.com; qemu-devel@nongnu.org; Xiexiangyou
> > > <xiexiangyou@huawei.com>; mst@redhat.com
> > > 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:berrange@redhat.com]
> > > > > Sent: Thursday, February 13, 2020 9:52 PM
> > > > > To: miaoyubo <miaoyubo@huawei.com>
> > > > > Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> > > > > imammedo@redhat.com; qemu-devel@nongnu.org; Xiexiangyou
> > > > > <xiexiangyou@huawei.com>; mst@redhat.com
> > > > > 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 <miaoyubo@huawei.com>
> > > > > >
> > > > > > 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
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-02-25 1:55 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-13 7:49 [RFC 0/2] pci_expander_brdige:acpi:Support pxb-pcie for ARM Yubo Miao
2020-02-13 7:49 ` [RFC 1/2] arm: acpi: pci-expender-bus: Make arm to support PXB-PCIE Yubo Miao
2020-02-13 10:23 ` Michael S. Tsirkin
2020-02-14 7:28 ` miaoyubo
2020-02-13 7:49 ` [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm Yubo Miao
2020-02-13 10:17 ` Michael S. Tsirkin
2020-02-14 7:30 ` miaoyubo
2020-02-13 13:51 ` Daniel P. Berrangé
2020-02-14 7:25 ` miaoyubo
2020-02-14 10:24 ` Daniel P. Berrangé
2020-02-15 8:59 ` miaoyubo
2020-02-24 12:36 ` Daniel P. Berrangé
2020-02-25 1:54 ` miaoyubo
2020-02-13 16:06 ` [RFC 0/2] pci_expander_brdige:acpi:Support pxb-pcie for ARM no-reply
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).