All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM
@ 2020-04-08 12:58 Yubo Miao
  2020-04-08 12:58 ` [PATCH v6 1/8] acpi: Extract two APIs from acpi_dsdt_add_pci Yubo Miao
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Yubo Miao @ 2020-04-08 12:58 UTC (permalink / raw)
  To: peter.maydell, shannon.zhaosl, lersek
  Cc: berrange, mst, qemu-devel, xiexiangyou, miaoyubo, imammedo

From: miaoyubo <miaoyubo@huawei.com>

Changes with v5
v5->v6: stat crs_range_insert in aml_build.h

Changes with v4
v4->v5: Not using specific resources for PXB.
Instead, the resources for pxb are composed of the bar space of the
pci-bridge/pcie-root-port behined it and the config space of devices
behind it.

Only if the bios(uefi for arm) support multiple roots,
configure space of devices behind pxbs could be obtained.
The uefi work is updated for discussion by the following link:
https://edk2.groups.io/g/devel/message/56901?p=,,,20,0,0,0::Created,,add+extra+roots+for+Arm,20,2,0,72723351 
[PATCH] ArmVirtPkg/FdtPciHostBridgeLib: add extra roots for Arm.

Currently pxb-pcie is not supported by arm,
the reason for it is pxb-pcie is not described in DSDT table
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

miaoyubo (8):
  acpi: Extract two APIs from acpi_dsdt_add_pci
  fw_cfg: Write the extra roots into the fw_cfg
  acpi: Extract crs build form acpi_build.c
  acpi: Refactor the source of host bridge and build tables for pxb
  acpi: Align the size to 128k
  unit-test: The files changed.
  unit-test: Add testcase for pxb
  unit-test: Add the binary file and clear diff.h

 hw/acpi/aml-build.c            | 275 +++++++++++++++++++++++++++++++
 hw/arm/virt-acpi-build.c       | 251 ++++++++++++++++++++++-------
 hw/arm/virt.c                  |  23 +++
 hw/i386/acpi-build.c           | 285 ---------------------------------
 include/hw/acpi/aml-build.h    |  25 +++
 tests/data/acpi/virt/DSDT.pxb  | Bin 0 -> 7802 bytes
 tests/qtest/bios-tables-test.c |  58 ++++++-
 7 files changed, 566 insertions(+), 351 deletions(-)
 create mode 100644 tests/data/acpi/virt/DSDT.pxb

-- 
2.19.1




^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v6 1/8] acpi: Extract two APIs from acpi_dsdt_add_pci
  2020-04-08 12:58 [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM Yubo Miao
@ 2020-04-08 12:58 ` Yubo Miao
  2020-04-08 12:58 ` [PATCH v6 2/8] fw_cfg: Write the extra roots into the fw_cfg Yubo Miao
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Yubo Miao @ 2020-04-08 12:58 UTC (permalink / raw)
  To: peter.maydell, shannon.zhaosl, lersek
  Cc: berrange, mst, qemu-devel, xiexiangyou, miaoyubo, imammedo

From: miaoyubo <miaoyubo@huawei.com>

Extract two APIs acpi_dsdt_add_pci_route_table and
acpi_dsdt_add_pci_osc form acpi_dsdt_add_pci. The first
API is used to specify the pci route table and the second
API is used to declare the operation system capabilities.
These two APIs would be used to specify the pxb-pcie in DSDT.

Signed-off-by: miaoyubo <miaoyubo@huawei.com>
---
 hw/arm/virt-acpi-build.c | 129 ++++++++++++++++++++++-----------------
 1 file changed, 72 insertions(+), 57 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 7ef0733d71..e8ba09855c 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -146,29 +146,11 @@ 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)
+static void acpi_dsdt_add_pci_route_table(Aml *dev, Aml *scope,
+                                          uint32_t irq)
 {
-    int ecam_id = VIRT_ECAM_ID(highmem_ecam);
-    Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
     int i, slot_no;
-    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;
-    int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
-
-    Aml *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)));
-    aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
-    aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
-    aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
-    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
-
+    Aml *method, *crs;
     /* Declare the PCI Routing Table. */
     Aml *rt_pkg = aml_varpackage(PCI_SLOT_MAX * PCI_NUM_PINS);
     for (slot_no = 0; slot_no < PCI_SLOT_MAX; slot_no++) {
@@ -204,41 +186,11 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
         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, 0x0000, nr_pcie_buses - 1, 0x0000,
-                            nr_pcie_buses));
-    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));
-    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));
-
-    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,
-                             base_mmio_high + size_mmio_high - 1, 0x0000,
-                             size_mmio_high));
-    }
-
-    aml_append(method, aml_return(rbuf));
-    aml_append(dev, method);
-
+static void acpi_dsdt_add_pci_osc(Aml *dev, Aml *scope)
+{
+    Aml *method, *UUID, *ifctx, *ifctx1, *elsectx, *buf;
     /* 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)));
@@ -246,7 +198,8 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
     aml_append(method,
         aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
 
-    /* PCI Firmware Specification 3.0
+    /*
+     * 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)
@@ -291,7 +244,8 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
 
     method = aml_method("_DSM", 4, AML_NOTSERIALIZED);
 
-    /* PCI Firmware Specification 3.0
+    /*
+     * 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}
@@ -309,6 +263,67 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
     buf = aml_buffer(1, byte_list);
     aml_append(method, aml_return(buf));
     aml_append(dev, method);
+}
+
+static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
+                              uint32_t irq, bool use_highmem, bool highmem_ecam)
+{
+    int ecam_id = VIRT_ECAM_ID(highmem_ecam);
+    Aml *method, *crs;
+    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;
+    int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
+
+    Aml *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)));
+    aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
+    aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
+    aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
+    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
+
+    acpi_dsdt_add_pci_route_table(dev, scope, irq);
+
+    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, 0x0000, nr_pcie_buses - 1, 0x0000,
+                            nr_pcie_buses));
+    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));
+    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));
+
+    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,
+                             base_mmio_high + size_mmio_high - 1, 0x0000,
+                             size_mmio_high));
+    }
+
+    aml_append(method, aml_return(rbuf));
+    aml_append(dev, method);
+
+    acpi_dsdt_add_pci_osc(dev, scope);
 
     Aml *dev_res0 = aml_device("%s", "RES0");
     aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02")));
-- 
2.19.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 2/8] fw_cfg: Write the extra roots into the fw_cfg
  2020-04-08 12:58 [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM Yubo Miao
  2020-04-08 12:58 ` [PATCH v6 1/8] acpi: Extract two APIs from acpi_dsdt_add_pci Yubo Miao
@ 2020-04-08 12:58 ` Yubo Miao
  2020-05-04 14:02   ` Michael S. Tsirkin
  2020-04-08 12:58 ` [PATCH v6 3/8] acpi: Extract crs build form acpi_build.c Yubo Miao
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Yubo Miao @ 2020-04-08 12:58 UTC (permalink / raw)
  To: peter.maydell, shannon.zhaosl, lersek
  Cc: berrange, mst, qemu-devel, xiexiangyou, miaoyubo, imammedo

From: miaoyubo <miaoyubo@huawei.com>

Write the extra roots into the fw_cfg therefore the uefi could
get the extra roots. Only if the uefi know there are extra roots,
the config space of devices behind the root could be obtained.

Signed-off-by: miaoyubo <miaoyubo@huawei.com>
---
 hw/arm/virt.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7dc96abf72..0fdfe4129c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -77,6 +77,7 @@
 #include "hw/acpi/generic_event_device.h"
 #include "hw/virtio/virtio-iommu.h"
 #include "hw/char/pl011.h"
+#include "hw/pci/pci_bus.h"
 
 #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
     static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -1435,6 +1436,12 @@ void virt_machine_done(Notifier *notifier, void *data)
     ARMCPU *cpu = ARM_CPU(first_cpu);
     struct arm_boot_info *info = &vms->bootinfo;
     AddressSpace *as = arm_boot_address_space(cpu, info);
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path_type("",
+                                   "pcie-host-bridge", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
+
+    PCIBus *bus = s->bus;
 
     /*
      * If the user provided a dtb, we assume the dynamic sysbus nodes
@@ -1453,6 +1460,22 @@ void virt_machine_done(Notifier *notifier, void *data)
         exit(1);
     }
 
+    if (bus) {
+        int extra_hosts = 0;
+        QLIST_FOREACH(bus, &bus->child, sibling) {
+            /* look for expander root buses */
+            if (pci_bus_is_root(bus)) {
+                extra_hosts++;
+            }
+        }
+        if (extra_hosts && vms->fw_cfg) {
+            uint64_t *val = g_malloc(sizeof(*val));
+            *val = cpu_to_le64(extra_hosts);
+            fw_cfg_add_file(vms->fw_cfg,
+                   "etc/extra-pci-roots", val, sizeof(*val));
+        }
+    }
+
     virt_acpi_setup(vms);
     virt_build_smbios(vms);
 }
-- 
2.19.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 3/8] acpi: Extract crs build form acpi_build.c
  2020-04-08 12:58 [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM Yubo Miao
  2020-04-08 12:58 ` [PATCH v6 1/8] acpi: Extract two APIs from acpi_dsdt_add_pci Yubo Miao
  2020-04-08 12:58 ` [PATCH v6 2/8] fw_cfg: Write the extra roots into the fw_cfg Yubo Miao
@ 2020-04-08 12:58 ` Yubo Miao
  2020-04-08 12:58 ` [PATCH v6 4/8] acpi: Refactor the source of host bridge and build tables for pxb Yubo Miao
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Yubo Miao @ 2020-04-08 12:58 UTC (permalink / raw)
  To: peter.maydell, shannon.zhaosl, lersek
  Cc: berrange, mst, qemu-devel, xiexiangyou, miaoyubo, imammedo

From: miaoyubo <miaoyubo@huawei.com>

Extract crs build form acpi_build.c, the function could also be used
to build the crs for pxbs for arm. The resources are composed by two parts:
1. The bar space of pci-bridge/pcie-root-ports
2. The resources devices behind PXBs need.
The base and limit of memory/io are obtained from the config via two APIs:
pci_bridge_get_base and pci_bridge_get_limit

Signed-off-by: miaoyubo <miaoyubo@huawei.com>
---
 hw/acpi/aml-build.c         | 275 ++++++++++++++++++++++++++++++++++
 hw/i386/acpi-build.c        | 285 ------------------------------------
 include/hw/acpi/aml-build.h |  25 ++++
 3 files changed, 300 insertions(+), 285 deletions(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 2c3702b882..252349d96d 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -26,6 +26,9 @@
 #include "qemu/bitops.h"
 #include "sysemu/numa.h"
 #include "hw/boards.h"
+#include "hw/pci/pci_host.h"
+#include "hw/pci/pci_bus.h"
+#include "hw/pci/pci_bridge.h"
 
 static GArray *build_alloc_array(void)
 {
@@ -54,6 +57,125 @@ static void build_append_array(GArray *array, GArray *val)
 
 #define ACPI_NAMESEG_LEN 4
 
+void crs_range_insert(GPtrArray *ranges, uint64_t base, uint64_t limit)
+{
+    CrsRangeEntry *entry;
+
+    entry = g_malloc(sizeof(*entry));
+    entry->base = base;
+    entry->limit = limit;
+
+    g_ptr_array_add(ranges, entry);
+}
+
+static void crs_range_free(gpointer data)
+{
+    CrsRangeEntry *entry = (CrsRangeEntry *)data;
+    g_free(entry);
+}
+
+void crs_range_set_init(CrsRangeSet *range_set)
+{
+    range_set->io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+    range_set->mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+    range_set->mem_64bit_ranges =
+            g_ptr_array_new_with_free_func(crs_range_free);
+}
+
+void crs_range_set_free(CrsRangeSet *range_set)
+{
+    g_ptr_array_free(range_set->io_ranges, true);
+    g_ptr_array_free(range_set->mem_ranges, true);
+    g_ptr_array_free(range_set->mem_64bit_ranges, true);
+}
+
+static gint crs_range_compare(gconstpointer a, gconstpointer b)
+{
+    CrsRangeEntry *entry_a = *(CrsRangeEntry **)a;
+    CrsRangeEntry *entry_b = *(CrsRangeEntry **)b;
+
+    if (entry_a->base < entry_b->base) {
+        return -1;
+    } else if (entry_a->base > entry_b->base) {
+        return 1;
+    } else {
+        return 0;
+    }
+}
+
+/*
+ * crs_replace_with_free_ranges - given the 'used' ranges within [start - end]
+ * interval, computes the 'free' ranges from the same interval.
+ * Example: If the input array is { [a1 - a2],[b1 - b2] }, the function
+ * will return { [base - a1], [a2 - b1], [b2 - limit] }.
+ */
+void crs_replace_with_free_ranges(GPtrArray *ranges,
+                                         uint64_t start, uint64_t end)
+{
+    GPtrArray *free_ranges = g_ptr_array_new();
+    uint64_t free_base = start;
+    int i;
+
+    g_ptr_array_sort(ranges, crs_range_compare);
+    for (i = 0; i < ranges->len; i++) {
+        CrsRangeEntry *used = g_ptr_array_index(ranges, i);
+
+        if (free_base < used->base) {
+            crs_range_insert(free_ranges, free_base, used->base - 1);
+        }
+
+        free_base = used->limit + 1;
+    }
+
+    if (free_base < end) {
+        crs_range_insert(free_ranges, free_base, end);
+    }
+
+    g_ptr_array_set_size(ranges, 0);
+    for (i = 0; i < free_ranges->len; i++) {
+        g_ptr_array_add(ranges, g_ptr_array_index(free_ranges, i));
+    }
+
+    g_ptr_array_free(free_ranges, true);
+}
+
+static void crs_range_merge(GPtrArray *range)
+{
+    GPtrArray *tmp =  g_ptr_array_new_with_free_func(crs_range_free);
+    CrsRangeEntry *entry;
+    uint64_t range_base, range_limit;
+    int i;
+
+    if (!range->len) {
+        return;
+    }
+
+    g_ptr_array_sort(range, crs_range_compare);
+
+    entry = g_ptr_array_index(range, 0);
+    range_base = entry->base;
+    range_limit = entry->limit;
+    for (i = 1; i < range->len; i++) {
+        entry = g_ptr_array_index(range, i);
+        if (entry->base - 1 == range_limit) {
+            range_limit = entry->limit;
+        } else {
+            crs_range_insert(tmp, range_base, range_limit);
+            range_base = entry->base;
+            range_limit = entry->limit;
+        }
+    }
+    crs_range_insert(tmp, range_base, range_limit);
+
+    g_ptr_array_set_size(range, 0);
+    for (i = 0; i < tmp->len; i++) {
+        entry = g_ptr_array_index(tmp, i);
+        crs_range_insert(range, entry->base, entry->limit);
+    }
+    g_ptr_array_free(tmp, true);
+}
+
+
 static void
 build_append_nameseg(GArray *array, const char *seg)
 {
@@ -1875,6 +1997,159 @@ build_hdr:
                  "FACP", tbl->len - fadt_start, f->rev, oem_id, oem_table_id);
 }
 
+Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
+{
+    Aml *crs = aml_resource_template();
+    CrsRangeSet temp_range_set;
+    CrsRangeEntry *entry;
+    uint8_t max_bus = pci_bus_num(host->bus);
+    uint8_t type;
+    int devfn;
+    int i;
+
+    crs_range_set_init(&temp_range_set);
+    for (devfn = 0; devfn < ARRAY_SIZE(host->bus->devices); devfn++) {
+        uint64_t range_base, range_limit;
+        PCIDevice *dev = host->bus->devices[devfn];
+
+        if (!dev) {
+            continue;
+        }
+
+        for (i = 0; i < PCI_NUM_REGIONS; i++) {
+            PCIIORegion *r = &dev->io_regions[i];
+
+            range_base = r->addr;
+            range_limit = r->addr + r->size - 1;
+
+            /*
+             * Work-around for old bioses
+             * that do not support multiple root buses
+             */
+            if (!range_base || range_base > range_limit) {
+                continue;
+            }
+
+            if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
+                crs_range_insert(temp_range_set.io_ranges,
+                                 range_base, range_limit);
+            } else { /* "memory" */
+                crs_range_insert(temp_range_set.mem_ranges,
+                                 range_base, range_limit);
+            }
+        }
+
+        type = dev->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION;
+        if (type == PCI_HEADER_TYPE_BRIDGE) {
+            uint8_t subordinate = dev->config[PCI_SUBORDINATE_BUS];
+            if (subordinate > max_bus) {
+                max_bus = subordinate;
+            }
+
+            range_base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_IO);
+            range_limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_IO);
+             /*
+              * Work-around for old bioses
+              * that do not support multiple root buses
+              */
+            if (range_base && range_base <= range_limit) {
+                crs_range_insert(temp_range_set.io_ranges,
+                                 range_base, range_limit);
+            }
+
+            range_base =
+                pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY);
+            range_limit =
+                pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY);
+
+            /*
+             * Work-around for old bioses
+             * that do not support multiple root buses
+             */
+            if (range_base && range_base <= range_limit) {
+                uint64_t length = range_limit - range_base + 1;
+                if (range_limit <= UINT32_MAX && length <= UINT32_MAX) {
+                    crs_range_insert(temp_range_set.mem_ranges,
+                                     range_base, range_limit);
+                } else {
+                    crs_range_insert(temp_range_set.mem_64bit_ranges,
+                                     range_base, range_limit);
+                }
+            }
+
+            range_base =
+                pci_bridge_get_base(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
+            range_limit =
+                pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
+
+            /*
+             * Work-around for old bioses
+             * that do not support multiple root buses
+             */
+            if (range_base && range_base <= range_limit) {
+                uint64_t length = range_limit - range_base + 1;
+                if (range_limit <= UINT32_MAX && length <= UINT32_MAX) {
+                    crs_range_insert(temp_range_set.mem_ranges,
+                                     range_base, range_limit);
+                } else {
+                    crs_range_insert(temp_range_set.mem_64bit_ranges,
+                                     range_base, range_limit);
+                }
+            }
+        }
+    }
+
+    crs_range_merge(temp_range_set.io_ranges);
+    for (i = 0; i < temp_range_set.io_ranges->len; i++) {
+        entry = g_ptr_array_index(temp_range_set.io_ranges, i);
+        aml_append(crs,
+                   aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED,
+                               AML_POS_DECODE, AML_ENTIRE_RANGE,
+                               0, entry->base, entry->limit, 0,
+                               entry->limit - entry->base + 1));
+        crs_range_insert(range_set->io_ranges, entry->base, entry->limit);
+    }
+
+    crs_range_merge(temp_range_set.mem_ranges);
+    for (i = 0; i < temp_range_set.mem_ranges->len; i++) {
+        entry = g_ptr_array_index(temp_range_set.mem_ranges, i);
+        aml_append(crs,
+                   aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
+                                    AML_MAX_FIXED, AML_NON_CACHEABLE,
+                                    AML_READ_WRITE,
+                                    0, entry->base, entry->limit, 0,
+                                    entry->limit - entry->base + 1));
+        crs_range_insert(range_set->mem_ranges, entry->base, entry->limit);
+    }
+
+    crs_range_merge(temp_range_set.mem_64bit_ranges);
+    for (i = 0; i < temp_range_set.mem_64bit_ranges->len; i++) {
+        entry = g_ptr_array_index(temp_range_set.mem_64bit_ranges, i);
+        aml_append(crs,
+                   aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
+                                    AML_MAX_FIXED, AML_NON_CACHEABLE,
+                                    AML_READ_WRITE,
+                                    0, entry->base, entry->limit, 0,
+                                    entry->limit - entry->base + 1));
+        crs_range_insert(range_set->mem_64bit_ranges,
+                         entry->base, entry->limit);
+    }
+
+    crs_range_set_free(&temp_range_set);
+
+    aml_append(crs,
+        aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
+                            0,
+                            pci_bus_num(host->bus),
+                            max_bus,
+                            0,
+                            max_bus - pci_bus_num(host->bus) + 1));
+
+    return crs;
+}
+
+
+
 /* ACPI 5.0: 6.4.3.8.2 Serial Bus Connection Descriptors */
 static Aml *aml_serial_bus_device(uint8_t serial_bus_type, uint8_t flags,
                                   uint16_t type_flags,
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 2a7e55bae7..ba7750480f 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -715,291 +715,6 @@ static Aml *build_prt(bool is_pci0_prt)
     return method;
 }
 
-typedef struct CrsRangeEntry {
-    uint64_t base;
-    uint64_t limit;
-} CrsRangeEntry;
-
-static void crs_range_insert(GPtrArray *ranges, uint64_t base, uint64_t limit)
-{
-    CrsRangeEntry *entry;
-
-    entry = g_malloc(sizeof(*entry));
-    entry->base = base;
-    entry->limit = limit;
-
-    g_ptr_array_add(ranges, entry);
-}
-
-static void crs_range_free(gpointer data)
-{
-    CrsRangeEntry *entry = (CrsRangeEntry *)data;
-    g_free(entry);
-}
-
-typedef struct CrsRangeSet {
-    GPtrArray *io_ranges;
-    GPtrArray *mem_ranges;
-    GPtrArray *mem_64bit_ranges;
- } CrsRangeSet;
-
-static void crs_range_set_init(CrsRangeSet *range_set)
-{
-    range_set->io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
-    range_set->mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
-    range_set->mem_64bit_ranges =
-            g_ptr_array_new_with_free_func(crs_range_free);
-}
-
-static void crs_range_set_free(CrsRangeSet *range_set)
-{
-    g_ptr_array_free(range_set->io_ranges, true);
-    g_ptr_array_free(range_set->mem_ranges, true);
-    g_ptr_array_free(range_set->mem_64bit_ranges, true);
-}
-
-static gint crs_range_compare(gconstpointer a, gconstpointer b)
-{
-    CrsRangeEntry *entry_a = *(CrsRangeEntry **)a;
-    CrsRangeEntry *entry_b = *(CrsRangeEntry **)b;
-
-    if (entry_a->base < entry_b->base) {
-        return -1;
-    } else if (entry_a->base > entry_b->base) {
-        return 1;
-    } else {
-        return 0;
-    }
-}
-
-/*
- * crs_replace_with_free_ranges - given the 'used' ranges within [start - end]
- * interval, computes the 'free' ranges from the same interval.
- * Example: If the input array is { [a1 - a2],[b1 - b2] }, the function
- * will return { [base - a1], [a2 - b1], [b2 - limit] }.
- */
-static void crs_replace_with_free_ranges(GPtrArray *ranges,
-                                         uint64_t start, uint64_t end)
-{
-    GPtrArray *free_ranges = g_ptr_array_new();
-    uint64_t free_base = start;
-    int i;
-
-    g_ptr_array_sort(ranges, crs_range_compare);
-    for (i = 0; i < ranges->len; i++) {
-        CrsRangeEntry *used = g_ptr_array_index(ranges, i);
-
-        if (free_base < used->base) {
-            crs_range_insert(free_ranges, free_base, used->base - 1);
-        }
-
-        free_base = used->limit + 1;
-    }
-
-    if (free_base < end) {
-        crs_range_insert(free_ranges, free_base, end);
-    }
-
-    g_ptr_array_set_size(ranges, 0);
-    for (i = 0; i < free_ranges->len; i++) {
-        g_ptr_array_add(ranges, g_ptr_array_index(free_ranges, i));
-    }
-
-    g_ptr_array_free(free_ranges, true);
-}
-
-/*
- * crs_range_merge - merges adjacent ranges in the given array.
- * Array elements are deleted and replaced with the merged ranges.
- */
-static void crs_range_merge(GPtrArray *range)
-{
-    GPtrArray *tmp =  g_ptr_array_new_with_free_func(crs_range_free);
-    CrsRangeEntry *entry;
-    uint64_t range_base, range_limit;
-    int i;
-
-    if (!range->len) {
-        return;
-    }
-
-    g_ptr_array_sort(range, crs_range_compare);
-
-    entry = g_ptr_array_index(range, 0);
-    range_base = entry->base;
-    range_limit = entry->limit;
-    for (i = 1; i < range->len; i++) {
-        entry = g_ptr_array_index(range, i);
-        if (entry->base - 1 == range_limit) {
-            range_limit = entry->limit;
-        } else {
-            crs_range_insert(tmp, range_base, range_limit);
-            range_base = entry->base;
-            range_limit = entry->limit;
-        }
-    }
-    crs_range_insert(tmp, range_base, range_limit);
-
-    g_ptr_array_set_size(range, 0);
-    for (i = 0; i < tmp->len; i++) {
-        entry = g_ptr_array_index(tmp, i);
-        crs_range_insert(range, entry->base, entry->limit);
-    }
-    g_ptr_array_free(tmp, true);
-}
-
-static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
-{
-    Aml *crs = aml_resource_template();
-    CrsRangeSet temp_range_set;
-    CrsRangeEntry *entry;
-    uint8_t max_bus = pci_bus_num(host->bus);
-    uint8_t type;
-    int devfn;
-    int i;
-
-    crs_range_set_init(&temp_range_set);
-    for (devfn = 0; devfn < ARRAY_SIZE(host->bus->devices); devfn++) {
-        uint64_t range_base, range_limit;
-        PCIDevice *dev = host->bus->devices[devfn];
-
-        if (!dev) {
-            continue;
-        }
-
-        for (i = 0; i < PCI_NUM_REGIONS; i++) {
-            PCIIORegion *r = &dev->io_regions[i];
-
-            range_base = r->addr;
-            range_limit = r->addr + r->size - 1;
-
-            /*
-             * Work-around for old bioses
-             * that do not support multiple root buses
-             */
-            if (!range_base || range_base > range_limit) {
-                continue;
-            }
-
-            if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
-                crs_range_insert(temp_range_set.io_ranges,
-                                 range_base, range_limit);
-            } else { /* "memory" */
-                crs_range_insert(temp_range_set.mem_ranges,
-                                 range_base, range_limit);
-            }
-        }
-
-        type = dev->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION;
-        if (type == PCI_HEADER_TYPE_BRIDGE) {
-            uint8_t subordinate = dev->config[PCI_SUBORDINATE_BUS];
-            if (subordinate > max_bus) {
-                max_bus = subordinate;
-            }
-
-            range_base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_IO);
-            range_limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_IO);
-
-            /*
-             * Work-around for old bioses
-             * that do not support multiple root buses
-             */
-            if (range_base && range_base <= range_limit) {
-                crs_range_insert(temp_range_set.io_ranges,
-                                 range_base, range_limit);
-            }
-
-            range_base =
-                pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY);
-            range_limit =
-                pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY);
-
-            /*
-             * Work-around for old bioses
-             * that do not support multiple root buses
-             */
-            if (range_base && range_base <= range_limit) {
-                uint64_t length = range_limit - range_base + 1;
-                if (range_limit <= UINT32_MAX && length <= UINT32_MAX) {
-                    crs_range_insert(temp_range_set.mem_ranges,
-                                     range_base, range_limit);
-                } else {
-                    crs_range_insert(temp_range_set.mem_64bit_ranges,
-                                     range_base, range_limit);
-                }
-            }
-
-            range_base =
-                pci_bridge_get_base(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
-            range_limit =
-                pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
-
-            /*
-             * Work-around for old bioses
-             * that do not support multiple root buses
-             */
-            if (range_base && range_base <= range_limit) {
-                uint64_t length = range_limit - range_base + 1;
-                if (range_limit <= UINT32_MAX && length <= UINT32_MAX) {
-                    crs_range_insert(temp_range_set.mem_ranges,
-                                     range_base, range_limit);
-                } else {
-                    crs_range_insert(temp_range_set.mem_64bit_ranges,
-                                     range_base, range_limit);
-                }
-            }
-        }
-    }
-
-    crs_range_merge(temp_range_set.io_ranges);
-    for (i = 0; i < temp_range_set.io_ranges->len; i++) {
-        entry = g_ptr_array_index(temp_range_set.io_ranges, i);
-        aml_append(crs,
-                   aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED,
-                               AML_POS_DECODE, AML_ENTIRE_RANGE,
-                               0, entry->base, entry->limit, 0,
-                               entry->limit - entry->base + 1));
-        crs_range_insert(range_set->io_ranges, entry->base, entry->limit);
-    }
-
-    crs_range_merge(temp_range_set.mem_ranges);
-    for (i = 0; i < temp_range_set.mem_ranges->len; i++) {
-        entry = g_ptr_array_index(temp_range_set.mem_ranges, i);
-        aml_append(crs,
-                   aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
-                                    AML_MAX_FIXED, AML_NON_CACHEABLE,
-                                    AML_READ_WRITE,
-                                    0, entry->base, entry->limit, 0,
-                                    entry->limit - entry->base + 1));
-        crs_range_insert(range_set->mem_ranges, entry->base, entry->limit);
-    }
-
-    crs_range_merge(temp_range_set.mem_64bit_ranges);
-    for (i = 0; i < temp_range_set.mem_64bit_ranges->len; i++) {
-        entry = g_ptr_array_index(temp_range_set.mem_64bit_ranges, i);
-        aml_append(crs,
-                   aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
-                                    AML_MAX_FIXED, AML_NON_CACHEABLE,
-                                    AML_READ_WRITE,
-                                    0, entry->base, entry->limit, 0,
-                                    entry->limit - entry->base + 1));
-        crs_range_insert(range_set->mem_64bit_ranges,
-                         entry->base, entry->limit);
-    }
-
-    crs_range_set_free(&temp_range_set);
-
-    aml_append(crs,
-        aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
-                            0,
-                            pci_bus_num(host->bus),
-                            max_bus,
-                            0,
-                            max_bus - pci_bus_num(host->bus) + 1));
-
-    return crs;
-}
-
 static void build_hpet_aml(Aml *table)
 {
     Aml *crs;
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index de4a406568..2894825ffb 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -223,6 +223,20 @@ struct AcpiBuildTables {
     BIOSLinker *linker;
 } AcpiBuildTables;
 
+typedef
+struct CrsRangeEntry {
+    uint64_t base;
+    uint64_t limit;
+} CrsRangeEntry;
+
+typedef
+struct CrsRangeSet {
+    GPtrArray *io_ranges;
+    GPtrArray *mem_ranges;
+    GPtrArray *mem_64bit_ranges;
+} CrsRangeSet;
+
+
 /*
  * ACPI 5.0: 6.4.3.8.2 Serial Bus Connection Descriptors
  * Serial Bus Type
@@ -429,6 +443,17 @@ build_append_gas_from_struct(GArray *table, const struct AcpiGenericAddress *s)
                      s->access_width, s->address);
 }
 
+Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set);
+
+void crs_range_insert(GPtrArray *ranges, uint64_t base, uint64_t limit);
+
+void crs_replace_with_free_ranges(GPtrArray *ranges,
+                                         uint64_t start, uint64_t end);
+
+void crs_range_set_init(CrsRangeSet *range_set);
+
+void crs_range_set_free(CrsRangeSet *range_set);
+
 void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
                        uint64_t len, int node, MemoryAffinityFlags flags);
 
-- 
2.19.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 4/8] acpi: Refactor the source of host bridge and build tables for pxb
  2020-04-08 12:58 [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM Yubo Miao
                   ` (2 preceding siblings ...)
  2020-04-08 12:58 ` [PATCH v6 3/8] acpi: Extract crs build form acpi_build.c Yubo Miao
@ 2020-04-08 12:58 ` Yubo Miao
  2020-05-04 14:00   ` Michael S. Tsirkin
  2020-04-08 12:58 ` [PATCH v6 5/8] acpi: Align the size to 128k Yubo Miao
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Yubo Miao @ 2020-04-08 12:58 UTC (permalink / raw)
  To: peter.maydell, shannon.zhaosl, lersek
  Cc: berrange, mst, qemu-devel, xiexiangyou, miaoyubo, imammedo

From: miaoyubo <miaoyubo@huawei.com>

The resources of pxbs and obtained by crs_build and the resources
used by pxbs would be moved form the resources defined for host-bridge.

The resources for pxb are composed of the bar space of the
pci-bridge/pcie-root-port behined it and the config space of devices
behind it.

Signed-off-by: miaoyubo <miaoyubo@huawei.com>
---
 hw/arm/virt-acpi-build.c | 131 +++++++++++++++++++++++++++++++++------
 1 file changed, 111 insertions(+), 20 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index e8ba09855c..7bcd04dfb7 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -49,6 +49,9 @@
 #include "kvm_arm.h"
 #include "migration/vmstate.h"
 
+#include "hw/arm/virt.h"
+#include "hw/pci/pci_bus.h"
+#include "hw/pci/pci_bridge.h"
 #define ARM_SPI_BASE 32
 
 static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
@@ -266,19 +269,81 @@ static void acpi_dsdt_add_pci_osc(Aml *dev, 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;
+    int i;
+    Aml *method, *crs, *dev;
     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;
+    CrsRangeEntry *entry;
+    CrsRangeSet crs_range_set;
+
+    crs_range_set_init(&crs_range_set);
     int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path_type("",
+                                   "pcie-host-bridge", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
+
+    PCIBus *bus = s->bus;
+    /* start to construct the tables for pxb*/
+    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;
+            }
+            /*
+             * Coded up the MIN of the busNr defined for pxb-pcie,
+             * the MIN - 1 would be the MAX bus number for the main
+             * host bridge.
+             */
+            if (bus_num < nr_pcie_buses) {
+                nr_pcie_buses = bus_num;
+            }
+
+            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);
+            }
+
+            acpi_dsdt_add_pci_route_table(dev, scope, irq);
+
+            /*
+             * Resources deined for PXBs are composed by the folling parts:
+             * 1. The resources the pci-brige/pcie-root-port need.
+             * 2. The resources the devices behind pxb need.
+             */
+            crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent), &crs_range_set);
+            aml_append(dev, aml_name_decl("_CRS", crs));
+
+            acpi_dsdt_add_pci_osc(dev, scope);
+
+            aml_append(scope, dev);
+
+        }
+    }
 
-    Aml *dev = aml_device("%s", "PCI0");
+    /* start to construct the tables for main host bridge */
+    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)));
@@ -299,25 +364,51 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
         aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
                             0x0000, 0x0000, nr_pcie_buses - 1, 0x0000,
                             nr_pcie_buses));
-    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));
-    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));
+
+    /*
+     * Remove the resources used by PXBs.
+     */
+    crs_replace_with_free_ranges(crs_range_set.mem_ranges,
+                                 base_mmio,
+                                 base_mmio + size_mmio - 1);
+    for (i = 0; i < crs_range_set.mem_ranges->len; i++) {
+        entry = g_ptr_array_index(crs_range_set.mem_ranges, i);
+        aml_append(rbuf,
+            aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
+                             AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000,
+                             entry->base, entry->limit,
+                             0x0000, entry->limit - entry->base + 1));
+    }
+
+    crs_replace_with_free_ranges(crs_range_set.io_ranges,
+                                 0x0000,
+                                 size_pio - 1);
+    for (i = 0; i < crs_range_set.io_ranges->len; i++) {
+        entry = g_ptr_array_index(crs_range_set.io_ranges, i);
+        aml_append(rbuf,
+            aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
+                         AML_ENTIRE_RANGE, 0x0000, entry->base,
+                         entry->limit, base_pio,
+                         entry->limit - entry->base + 1));
+    }
+
 
     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,
-                             base_mmio_high + size_mmio_high - 1, 0x0000,
-                             size_mmio_high));
+        crs_replace_with_free_ranges(crs_range_set.mem_64bit_ranges,
+                                     base_mmio_high,
+                                     base_mmio_high + size_mmio_high - 1);
+        for (i = 0; i < crs_range_set.mem_64bit_ranges->len; i++) {
+            entry = g_ptr_array_index(crs_range_set.mem_64bit_ranges, i);
+            aml_append(rbuf,
+                aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
+                                 AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000,
+                                 entry->base,
+                                 entry->limit, 0x0000,
+                                 entry->limit - entry->base + 1));
+        }
     }
 
     aml_append(method, aml_return(rbuf));
@@ -335,6 +426,8 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
     aml_append(dev_res0, aml_name_decl("_CRS", crs));
     aml_append(dev, dev_res0);
     aml_append(scope, dev);
+
+    crs_range_set_free(&crs_range_set);
 }
 
 static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap,
@@ -746,7 +839,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),
@@ -798,7 +891,6 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
     unsigned dsdt, xsdt;
     GArray *tables_blob = tables->table_data;
     MachineState *ms = MACHINE(vms);
-
     table_offsets = g_array_new(false, true /* clear */,
                                         sizeof(uint32_t));
 
@@ -952,7 +1044,6 @@ void virt_acpi_setup(VirtMachineState *vms)
     build_state->rsdp_mr = acpi_add_rom_blob(virt_acpi_build_update,
                                              build_state, tables.rsdp,
                                              ACPI_BUILD_RSDP_FILE, 0);
-
     qemu_register_reset(virt_acpi_build_reset, build_state);
     virt_acpi_build_reset(build_state);
     vmstate_register(NULL, 0, &vmstate_virt_acpi_build, build_state);
-- 
2.19.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 5/8] acpi: Align the size to 128k
  2020-04-08 12:58 [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM Yubo Miao
                   ` (3 preceding siblings ...)
  2020-04-08 12:58 ` [PATCH v6 4/8] acpi: Refactor the source of host bridge and build tables for pxb Yubo Miao
@ 2020-04-08 12:58 ` Yubo Miao
  2020-05-04 14:03   ` Michael S. Tsirkin
  2020-04-08 12:58 ` [PATCH v6 6/8] unit-test: The files changed Yubo Miao
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Yubo Miao @ 2020-04-08 12:58 UTC (permalink / raw)
  To: peter.maydell, shannon.zhaosl, lersek
  Cc: berrange, mst, qemu-devel, xiexiangyou, miaoyubo, imammedo

From: miaoyubo <miaoyubo@huawei.com>

If table size is changed between virt_acpi_build and
virt_acpi_build_update, the table size would not be updated to
UEFI, therefore, just align the size to 128kb, which is enough
and same with x86. It would warn if 64k is not enough and the
align size should be updated.

Signed-off-by: miaoyubo <miaoyubo@huawei.com>
---
 hw/arm/virt-acpi-build.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 7bcd04dfb7..89bb768b0c 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -54,6 +54,8 @@
 #include "hw/pci/pci_bridge.h"
 #define ARM_SPI_BASE 32
 
+#define ACPI_BUILD_TABLE_SIZE             0x20000
+
 static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
 {
     uint16_t i;
@@ -883,6 +885,15 @@ struct AcpiBuildState {
     bool patched;
 } AcpiBuildState;
 
+static void acpi_align_size(GArray *blob, unsigned align)
+{
+    /*
+     * Align size to multiple of given size. This reduces the chance
+     * we need to change size in the future (breaking cross version migration).
+     */
+    g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
+}
+
 static
 void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
 {
@@ -953,6 +964,20 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
         build_rsdp(tables->rsdp, tables->linker, &rsdp_data);
     }
 
+    /*
+     * The align size is 128, warn if 64k is not enough therefore
+     * the align size could be resized.
+     */
+    if (tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) {
+        warn_report("ACPI table size %u exceeds %d bytes,"
+                    " migration may not work",
+                    tables_blob->len, ACPI_BUILD_TABLE_SIZE / 2);
+        error_printf("Try removing CPUs, NUMA nodes, memory slots"
+                     " or PCI bridges.");
+    }
+    acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
+
+
     /* Cleanup memory that's no longer used. */
     g_array_free(table_offsets, true);
 }
-- 
2.19.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 6/8] unit-test: The files changed.
  2020-04-08 12:58 [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM Yubo Miao
                   ` (4 preceding siblings ...)
  2020-04-08 12:58 ` [PATCH v6 5/8] acpi: Align the size to 128k Yubo Miao
@ 2020-04-08 12:58 ` Yubo Miao
  2020-04-08 12:58 ` [PATCH v6 7/8] unit-test: Add testcase for pxb Yubo Miao
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Yubo Miao @ 2020-04-08 12:58 UTC (permalink / raw)
  To: peter.maydell, shannon.zhaosl, lersek
  Cc: berrange, mst, qemu-devel, xiexiangyou, miaoyubo, imammedo

From: miaoyubo <miaoyubo@huawei.com>

The unit-test is seperated into three patches:
1. The files changed and list in bios-tables-test-allowed-diff.h
2. The unit-test
3. The binary file and clear bios-tables-test-allowed-diff.h

The ASL diff would also be listed.
Sice there are 1000+lines diff, some changes would be omitted.

  * Original Table Header:
  *     Signature        "DSDT"
- *     Length           0x000014BB (5307)
+ *     Length           0x00001E7A (7802)
  *     Revision         0x02
- *     Checksum         0xD1
+ *     Checksum         0x57
  *     OEM ID           "BOCHS "
  *     OEM Table ID     "BXPCDSDT"
  *     OEM Revision     0x00000001 (1)

+        Device (PC80)
+        {
+            Name (_HID, "PNP0A08" /* PCI Express Bus */)  // _HID: Hardware ID
+            Name (_CID, "PNP0A03" /* PCI Bus */)  // _CID: Compatible ID
+            Name (_ADR, Zero)  // _ADR: Address
+            Name (_CCA, One)  // _CCA: Cache Coherency Attribute
+            Name (_SEG, Zero)  // _SEG: PCI Segment
+            Name (_BBN, 0x80)  // _BBN: BIOS Bus Number
+            Name (_UID, 0x80)  // _UID: Unique ID
+            Name (_STR, Unicode ("pxb Device"))  // _STR: Description String
+            Name (_PRT, Package (0x80)  // _PRT: PCI Routing Table
+            {
+                Package (0x04)
+                {
+                    0xFFFF,
+                    Zero,
+                    GSI0,
+                    Zero
+                },
+

Packages are omitted.

+                Package (0x04)
+                {
+                    0x001FFFFF,
+                    0x03,
+                    GSI2,
+                    Zero
+                }
+            })
+            Device (GSI0)
+            {
+                Name (_HID, "PNP0C0F" /* PCI Interrupt Link Device */)  // _HID: Hardware ID
+                Name (_UID, Zero)  // _UID: Unique ID
+                Name (_PRS, ResourceTemplate ()  // _PRS: Possible Resource Settings
+                {
+                    Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, )
+                    {
+                        0x00000023,
+                    }
+                })
+                Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
+                {
+                    Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, )
+                    {
+                        0x00000023,
+                    }
+                })
+                Method (_SRS, 1, NotSerialized)  // _SRS: Set Resource Settings
+                {
+                }
+            }

GSI1,2,3 are omitted.

+            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
+            {
+                WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
+                    0x0000,             // Granularity
+                    0x0080,             // Range Minimum
+                    0x0080,             // Range Maximum
+                    0x0000,             // Translation Offset
+                    0x0001,             // Length
+                    ,, )
+            })
+            Name (SUPP, Zero)
+            Name (CTRL, Zero)
+            Method (_OSC, 4, NotSerialized)  // _OSC: Operating System Capabilities
+            {
+                CreateDWordField (Arg3, Zero, CDW1)
+                If ((Arg0 == ToUUID ("33db4d5b-1ff7-401c-9657-7441c03dd766") /* PCI Host Bridge Device */))
+                {
+                    CreateDWordField (Arg3, 0x04, CDW2)
+                    CreateDWordField (Arg3, 0x08, CDW3)
+                    SUPP = CDW2 /* \_SB_.PC80._OSC.CDW2 */
+                    CTRL = CDW3 /* \_SB_.PC80._OSC.CDW3 */
+                    CTRL &= 0x1F
+                    If ((Arg1 != One))
+                    {
+                        CDW1 |= 0x08
+                    }
+
+                    If ((CDW3 != CTRL))
+                    {
+                        CDW1 |= 0x10
+                    }
+
+                    CDW3 = CTRL /* \_SB_.PC80.CTRL */
+                    Return (Arg3)
+                }
+                Else
+                {
+                    CDW1 |= 0x04
+                    Return (Arg3)
+                }
+            }

DSM is are omitted

         Device (PCI0)
         {
             Name (_HID, "PNP0A08" /* PCI Express Bus */)  // _HID: Hardware ID
                     WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
                         0x0000,             // Granularity
                         0x0000,             // Range Minimum
-                        0x00FF,             // Range Maximum
+                        0x007F,             // Range Maximum
                         0x0000,             // Translation Offset
-                        0x0100,             // Length
+                        0x0080,             // Length

Signed-off-by: miaoyubo <miaoyubo@huawei.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..90c53925fc 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,2 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/virt/DSDT.pxb",
-- 
2.19.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 7/8] unit-test: Add testcase for pxb
  2020-04-08 12:58 [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM Yubo Miao
                   ` (5 preceding siblings ...)
  2020-04-08 12:58 ` [PATCH v6 6/8] unit-test: The files changed Yubo Miao
@ 2020-04-08 12:58 ` Yubo Miao
  2020-04-08 12:58 ` [PATCH v6 8/8] unit-test: Add the binary file and clear diff.h Yubo Miao
  2020-04-08 13:11 ` [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM no-reply
  8 siblings, 0 replies; 16+ messages in thread
From: Yubo Miao @ 2020-04-08 12:58 UTC (permalink / raw)
  To: peter.maydell, shannon.zhaosl, lersek
  Cc: berrange, mst, qemu-devel, xiexiangyou, miaoyubo, imammedo

From: miaoyubo <miaoyubo@huawei.com>

Add testcase for pxb to make sure the ACPI table is correct for guest.

Signed-off-by: miaoyubo <miaoyubo@huawei.com>
---
 tests/qtest/bios-tables-test.c | 58 ++++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 0a597bbacf..4bba680917 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -621,12 +621,21 @@ static void test_acpi_one(const char *params, test_data *data)
          * TODO: convert '-drive if=pflash' to new syntax (see e33763be7cd3)
          * when arm/virt boad starts to support it.
          */
-        args = g_strdup_printf("-machine %s %s -accel tcg -nodefaults -nographic "
-            "-drive if=pflash,format=raw,file=%s,readonly "
-            "-drive if=pflash,format=raw,file=%s,snapshot=on -cdrom %s %s",
-            data->machine, data->tcg_only ? "" : "-accel kvm",
-            data->uefi_fl1, data->uefi_fl2, data->cd, params ? params : "");
-
+        if (data->cd) {
+            args = g_strdup_printf("-machine %s %s -accel tcg "
+                "-nodefaults -nographic "
+                "-drive if=pflash,format=raw,file=%s,readonly "
+                "-drive if=pflash,format=raw,file=%s,snapshot=on -cdrom %s %s",
+                data->machine, data->tcg_only ? "" : "-accel kvm",
+                data->uefi_fl1, data->uefi_fl2, data->cd, params ? params : "");
+        } else {
+            args = g_strdup_printf("-machine %s %s -accel tcg "
+                "-nodefaults -nographic "
+                "-drive if=pflash,format=raw,file=%s,readonly "
+                "-drive if=pflash,format=raw,file=%s,snapshot=on %s",
+                data->machine, data->tcg_only ? "" : "-accel kvm",
+                data->uefi_fl1, data->uefi_fl2, params ? params : "");
+        }
     } else {
         /* Disable kernel irqchip to be able to override apic irq0. */
         args = g_strdup_printf("-machine %s,kernel-irqchip=off %s -accel tcg "
@@ -961,6 +970,40 @@ static void test_acpi_virt_tcg_numamem(void)
 
 }
 
+#ifdef CONFIG_PXB
+static void test_acpi_virt_tcg_pxb(void)
+{
+    test_data data = {
+        .machine = "virt",
+        .tcg_only = true,
+        .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
+        .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
+        .ram_start = 0x40000000ULL,
+        .scan_len = 128ULL * 1024 * 1024,
+    };
+    /*
+     * While using -cdrom, the cdrom would auto plugged into pxb-pcie,
+     * the reason is the bus of pxb-pcie is also root bus, it would lead
+     * to the error only PCI/PCIE bridge could plug onto pxb.
+     * Therefore,thr cdrom is defined and plugged onto the scsi controller
+     * to solve the conflicts.
+     */
+    data.variant = ".pxb";
+    test_acpi_one(" -device pcie-root-port,chassis=1,id=pci.1"
+                  " -device virtio-scsi-pci,id=scsi0,bus=pci.1"
+                  " -drive file="
+                  "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2,"
+                  "if=none,media=cdrom,id=drive-scsi0-0-0-1,readonly=on"
+                  " -device scsi-cd,bus=scsi0.0,scsi-id=0,"
+                  "drive=drive-scsi0-0-0-1,id=scsi0-0-0-1,bootindex=1"
+                  " -cpu cortex-a57"
+                  " -device pxb-pcie,bus_nr=128",
+                  &data);
+
+    free_test_data(&data);
+}
+#endif
+
 static void test_acpi_tcg_acpi_hmat(const char *machine)
 {
     test_data data;
@@ -1053,6 +1096,9 @@ int main(int argc, char *argv[])
         qtest_add_func("acpi/virt", test_acpi_virt_tcg);
         qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
         qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
+#ifdef CONFIG_PXB
+        qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
+#endif
     }
     ret = g_test_run();
     boot_sector_cleanup(disk);
-- 
2.19.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 8/8] unit-test: Add the binary file and clear diff.h
  2020-04-08 12:58 [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM Yubo Miao
                   ` (6 preceding siblings ...)
  2020-04-08 12:58 ` [PATCH v6 7/8] unit-test: Add testcase for pxb Yubo Miao
@ 2020-04-08 12:58 ` Yubo Miao
  2020-04-08 13:11 ` [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM no-reply
  8 siblings, 0 replies; 16+ messages in thread
From: Yubo Miao @ 2020-04-08 12:58 UTC (permalink / raw)
  To: peter.maydell, shannon.zhaosl, lersek
  Cc: berrange, mst, qemu-devel, xiexiangyou, miaoyubo, imammedo

From: miaoyubo <miaoyubo@huawei.com>

Add the binary file DSDT.pxb and clear bios-tables-test-allowed-diff.h

Signed-off-by: miaoyubo <miaoyubo@huawei.com>
---
 tests/data/acpi/virt/DSDT.pxb               | Bin 0 -> 7802 bytes
 tests/qtest/bios-tables-test-allowed-diff.h |   1 -
 2 files changed, 1 deletion(-)
 create mode 100644 tests/data/acpi/virt/DSDT.pxb

diff --git a/tests/data/acpi/virt/DSDT.pxb b/tests/data/acpi/virt/DSDT.pxb
new file mode 100644
index 0000000000000000000000000000000000000000..d5f0533a02d62bc2ae2db9b9de9484e5c06652fe
GIT binary patch
literal 7802
zcmeI1%WoT16o;=LiS6+tw&OgUms2Pe&&rRcNlRN|kDbINPK+mQkW$GN2t>&y5*4DY
z5GE1@x}%ZUunAHY{255B*s){5x*Prhb`0mvok@O&o()@MN3!S4-1E)-#wYff>!#D(
zdAOidc(<`_Z#avMce{3z_Jx#EdRxC{zj_wB({~#Ey~7#1TrS7^8|`MgZg<-hEUS3`
zR=cV84zJqVo#0rnvr#TrD*mx}-|jiN8EfisLTO+^WtIANRE0w4D0)D-m9<UB&)wYW
zZBy<N%gtFCKbI0zG)SqKsqmDLIo(-GG)P%l+qKtB$~&#jEt-9m&f@IUtt92x^?zrE
z6Vv|u>e1W1K-`?I3==%fJX5q(*jFqgf=xI;=+i!j2&*$h#YZ&sEUM@nAgr*&hytUE
zjGD-ZNQ_Zn)R1vWWJD!K92l37u_Q7^B!&fyC1hL{8KV*-1&qtcSQZ&EiID-uGBQ>~
zMqFZKfw6*&D<UHyG4jB;0*ng#H#)5kOJWp&aTOV2neu;<pwuUU@g_3lI!#IQm<Gl*
zWXN@zmKZa@xQ-0DPBRi?4j4C(A=l}c#8?2vTgZ^>G%GO{fw77VxlVHu;{{;Uks;S<
zUSgaFMgtjgosLV43&5~}QI+eoATeGBMiUuwolZ!MSAo$&hFqtU661AXtRX|L(<zB@
z5g6;pkn40>Vw8cgfeg7$ixQ&>j5adlI-QXimw<5-8FHP@N{q|EcpDjVoz6*&6<};4
zL$1?#iE$Me9bnYtI$e+$*MPBw47pBA65|FiwtYtDhpxTi&!fB5E!WE{)VJ8wgqf&D
zQN7vI`@BBFX|2<Cqp@WTyyi^5I6J*u(V9F^pQ-oMqH3xS)Tip6dY@hu4es`K#y3B)
z2Ki((>AGs&X_uAR4$*c+<x_gU6{esX1Q7~qDxZ#~T$kE9GtQ5677fgpV_qH&4MLqs
zd~YoENoK4c>C9j#H9`7}G}OzaP-oI?ys;54Gnhd{>C9kg#AMP?FOx!@Ni*^?sUtLF
z{m6IphEmhyTLvL|jxf&=@0@|>h{+5lPa%4aGEZuLX$HYiYO>IiLiCI=&lvNJaZd`-
zGtNBYUS@Dfs3}8F3ehvcJgIFrSI@g73GPWDdRolWVxH8*p(lmtnPi?x=9%Q46ryK}
zd8U{rHGSwwA$q2nXPSAYxhI9_nPHw8=1EN=dQym<W6X1md5&>U3el5po1kv9%#)f*
z^rR3ybIdcxJagQWLiEft&ph*_CKNp>M9*>NInF%CxhI9_Szw+8=1EN}dQym<6U=jh
zc}{Ro3ej_tc}_A<YI4z&LiC(so>R<oihELso^*Q&@8>l0q^1}>DMZgA^DHvYBKM>a
zJ!hEb4D+NW8a*jQ&spX<%RFbfCxz%a$2{klCpF#ZNg;a9GtYVEInO;QL{D1OFrQi8
zXZ!;5q$V9bDMZf_^DHsX68EIgc<vpxqx!8hH*orE*)Ffq_o`kR(ci94E@LIVC65=q
zFLnB=er{i3wD0tskdN|v28N=Q0z{n`P-fpL>ZYER-{LZqUNJz{O9IR6<1D|`<t$n`
zK-L9;W%l_jV?SZ#ze%eweR!(@{V7@-dZ6OYt!`Jv?VaAHDy${?+m0Q5vajssZsm9*
zcJxth+{*5C{;2&`np^#T_kR87>%V{aWZ#O?fGWMl>9uyC1I^JJHH~_tpRAI8KF&Tp
zx)=JKj#RwSmE*~$N5MF=JF5>K=)rpb$^MTSvtOU2a<X4|qu+Eo(c^PwHoq<Z`pj8+
z*!gbi&rb0dyK|g4`dFRhBB79eqQ$NCpSm_yhSa{Dwrr+m(mI1Sb=Ow1=DNyOZR*q(
zRaxlWOw%{);DXL(*um+pd)UFb?y!T?l`!n!TzA;P=}H)OaIQP-;DjF4`mY^aA=|eb
zb#+2_!795-PldYI)KTNJ5wq?GeVtNY(6NE~n(mQOv_|ATvab8LUS6k%dy$TWQnZp|
z9<=mC50{RH)RWgB$2&aG$MnOC&YtxC|7GXciS}B-@1myT)<0P4JBON8e(w5s?*m<(
z((2iz(Oa}?V18w7#O_?wzvHgAntf9Q=11I$UO=Qfl{6jj`Q~mV5_-j?4q820Q>0Ek
zp0J{OUnX^Ex184IVqw1Dy1kP)(81l~?9rpUmR_}c+}-Uptij%4QEy<y+2&m8A6W)v
ATL1t6

literal 0
HcmV?d00001

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 90c53925fc..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,2 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/virt/DSDT.pxb",
-- 
2.19.1




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM
  2020-04-08 12:58 [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM Yubo Miao
                   ` (7 preceding siblings ...)
  2020-04-08 12:58 ` [PATCH v6 8/8] unit-test: Add the binary file and clear diff.h Yubo Miao
@ 2020-04-08 13:11 ` no-reply
  8 siblings, 0 replies; 16+ messages in thread
From: no-reply @ 2020-04-08 13:11 UTC (permalink / raw)
  To: miaoyubo
  Cc: peter.maydell, berrange, mst, qemu-devel, xiexiangyou,
	shannon.zhaosl, miaoyubo, imammedo, lersek

Patchew URL: https://patchew.org/QEMU/20200408125816.955-1-miaoyubo@huawei.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM
Message-id: 20200408125816.955-1-miaoyubo@huawei.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
6a2dfea unit-test: Add the binary file and clear diff.h
3b6a166 unit-test: Add testcase for pxb
c5d159c unit-test: The files changed.
e3dd89e acpi: Align the size to 128k
043f5fd acpi: Refactor the source of host bridge and build tables for pxb
22f4d5c acpi: Extract crs build form acpi_build.c
3c167be fw_cfg: Write the extra roots into the fw_cfg
e277786 acpi: Extract two APIs from acpi_dsdt_add_pci

=== OUTPUT BEGIN ===
1/8 Checking commit e27778652473 (acpi: Extract two APIs from acpi_dsdt_add_pci)
2/8 Checking commit 3c167be7ea51 (fw_cfg: Write the extra roots into the fw_cfg)
3/8 Checking commit 22f4d5c3eb1d (acpi: Extract crs build form acpi_build.c)
4/8 Checking commit 043f5fd30894 (acpi: Refactor the source of host bridge and build tables for pxb)
5/8 Checking commit e3dd89e213dc (acpi: Align the size to 128k)
6/8 Checking commit c5d159c8a6b1 (unit-test: The files changed.)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt-acpi-build.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt-acpi-build.c found

total: 2 errors, 0 warnings, 2 lines checked

Patch 6/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

7/8 Checking commit 3b6a166e7ece (unit-test: Add testcase for pxb)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and tests/qtest/bios-tables-test.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and tests/qtest/bios-tables-test.c found

total: 2 errors, 0 warnings, 76 lines checked

Patch 7/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

8/8 Checking commit 6a2dfea13ba3 (unit-test: Add the binary file and clear diff.h)
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/virt/DSDT.pxb and tests/qtest/bios-tables-test.c found

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#13: 
new file mode 100644

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and tests/qtest/bios-tables-test.c found

ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and tests/qtest/bios-tables-test.c found

total: 3 errors, 1 warnings, 1 lines checked

Patch 8/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200408125816.955-1-miaoyubo@huawei.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6 4/8] acpi: Refactor the source of host bridge and build tables for pxb
  2020-04-08 12:58 ` [PATCH v6 4/8] acpi: Refactor the source of host bridge and build tables for pxb Yubo Miao
@ 2020-05-04 14:00   ` Michael S. Tsirkin
  2020-05-08 13:12     ` miaoyubo
  0 siblings, 1 reply; 16+ messages in thread
From: Michael S. Tsirkin @ 2020-05-04 14:00 UTC (permalink / raw)
  To: Yubo Miao
  Cc: peter.maydell, berrange, qemu-devel, xiexiangyou, shannon.zhaosl,
	imammedo, lersek

On Wed, Apr 08, 2020 at 08:58:12PM +0800, Yubo Miao wrote:
> From: miaoyubo <miaoyubo@huawei.com>
> 
> The resources of pxbs and obtained by crs_build and the resources
> used by pxbs would be moved form the resources defined for host-bridge.
> 
> The resources for pxb are composed of the bar space of the
> pci-bridge/pcie-root-port behined it and the config space of devices
> behind it.
> 
> Signed-off-by: miaoyubo <miaoyubo@huawei.com>

A bunch of spelling/syntax mistakes in the log, pls spellcheck.

Pls use the format
 Yubo Miao <miaoyubo@huawei.com>



> ---
>  hw/arm/virt-acpi-build.c | 131 +++++++++++++++++++++++++++++++++------
>  1 file changed, 111 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index e8ba09855c..7bcd04dfb7 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -49,6 +49,9 @@
>  #include "kvm_arm.h"
>  #include "migration/vmstate.h"
>  
> +#include "hw/arm/virt.h"
> +#include "hw/pci/pci_bus.h"
> +#include "hw/pci/pci_bridge.h"
>  #define ARM_SPI_BASE 32
>  
>  static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
> @@ -266,19 +269,81 @@ static void acpi_dsdt_add_pci_osc(Aml *dev, 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;
> +    int i;
> +    Aml *method, *crs, *dev;
>      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;
> +    CrsRangeEntry *entry;
> +    CrsRangeSet crs_range_set;
> +
> +    crs_range_set_init(&crs_range_set);
>      int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path_type("",
> +                                   "pcie-host-bridge", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);

Not TYPE_PCIE_HOST_BRIDGE? And what if it's ambiguous?


> +
> +    PCIBus *bus = s->bus;
> +    /* start to construct the tables for pxb*/


coding style violation. weird that ehckpatch didn't notice it.

> +    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;
> +            }
> +            /*
> +             * Coded up the MIN of the busNr defined for pxb-pcie,
> +             * the MIN - 1 would be the MAX bus number for the main
> +             * host bridge.


Couldn't figure out this comment. Pls rephrase in some way so it's
understandable.

> +             */
> +            if (bus_num < nr_pcie_buses) {
> +                nr_pcie_buses = bus_num;
> +            }
> +
> +            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);
> +            }
> +
> +            acpi_dsdt_add_pci_route_table(dev, scope, irq);
> +
> +            /*
> +             * Resources deined for PXBs are composed by the folling parts:
> +             * 1. The resources the pci-brige/pcie-root-port need.
> +             * 2. The resources the devices behind pxb need.
> +             */


syntax/grammar errors here too.

> +            crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent), &crs_range_set);
> +            aml_append(dev, aml_name_decl("_CRS", crs));
> +
> +            acpi_dsdt_add_pci_osc(dev, scope);
> +
> +            aml_append(scope, dev);
> +
> +        }
> +    }
>  
> -    Aml *dev = aml_device("%s", "PCI0");
> +    /* start to construct the tables for main host bridge */

tables for the main.



> +    dev = aml_device("%s", "PCI0");


Make dev above local in scope, then this can stay unchanged.

>      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)));
> @@ -299,25 +364,51 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>          aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
>                              0x0000, 0x0000, nr_pcie_buses - 1, 0x0000,
>                              nr_pcie_buses));
> -    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));
> -    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));
> +
> +    /*
> +     * Remove the resources used by PXBs.
> +     */
> +    crs_replace_with_free_ranges(crs_range_set.mem_ranges,
> +                                 base_mmio,
> +                                 base_mmio + size_mmio - 1);
> +    for (i = 0; i < crs_range_set.mem_ranges->len; i++) {
> +        entry = g_ptr_array_index(crs_range_set.mem_ranges, i);
> +        aml_append(rbuf,
> +            aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
> +                             AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000,
> +                             entry->base, entry->limit,
> +                             0x0000, entry->limit - entry->base + 1));
> +    }
> +
> +    crs_replace_with_free_ranges(crs_range_set.io_ranges,
> +                                 0x0000,
> +                                 size_pio - 1);
> +    for (i = 0; i < crs_range_set.io_ranges->len; i++) {
> +        entry = g_ptr_array_index(crs_range_set.io_ranges, i);
> +        aml_append(rbuf,
> +            aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
> +                         AML_ENTIRE_RANGE, 0x0000, entry->base,
> +                         entry->limit, base_pio,
> +                         entry->limit - entry->base + 1));
> +    }
> +
>  
>      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,
> -                             base_mmio_high + size_mmio_high - 1, 0x0000,
> -                             size_mmio_high));
> +        crs_replace_with_free_ranges(crs_range_set.mem_64bit_ranges,
> +                                     base_mmio_high,
> +                                     base_mmio_high + size_mmio_high - 1);
> +        for (i = 0; i < crs_range_set.mem_64bit_ranges->len; i++) {
> +            entry = g_ptr_array_index(crs_range_set.mem_64bit_ranges, i);
> +            aml_append(rbuf,
> +                aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
> +                                 AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000,
> +                                 entry->base,
> +                                 entry->limit, 0x0000,
> +                                 entry->limit - entry->base + 1));
> +        }
>      }
>  
>      aml_append(method, aml_return(rbuf));
> @@ -335,6 +426,8 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>      aml_append(dev_res0, aml_name_decl("_CRS", crs));
>      aml_append(dev, dev_res0);
>      aml_append(scope, dev);
> +
> +    crs_range_set_free(&crs_range_set);
>  }
>  
>  static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap,
> @@ -746,7 +839,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),
> @@ -798,7 +891,6 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>      unsigned dsdt, xsdt;
>      GArray *tables_blob = tables->table_data;
>      MachineState *ms = MACHINE(vms);
> -
>      table_offsets = g_array_new(false, true /* clear */,
>                                          sizeof(uint32_t));
>

this empty line didn't hurt
  
> @@ -952,7 +1044,6 @@ void virt_acpi_setup(VirtMachineState *vms)
>      build_state->rsdp_mr = acpi_add_rom_blob(virt_acpi_build_update,
>                                               build_state, tables.rsdp,
>                                               ACPI_BUILD_RSDP_FILE, 0);
> -
>      qemu_register_reset(virt_acpi_build_reset, build_state);
>      virt_acpi_build_reset(build_state);
>      vmstate_register(NULL, 0, &vmstate_virt_acpi_build, build_state);


this line didn't hurt either.

> -- 
> 2.19.1
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6 2/8] fw_cfg: Write the extra roots into the fw_cfg
  2020-04-08 12:58 ` [PATCH v6 2/8] fw_cfg: Write the extra roots into the fw_cfg Yubo Miao
@ 2020-05-04 14:02   ` Michael S. Tsirkin
  2020-05-08 12:50     ` miaoyubo
  0 siblings, 1 reply; 16+ messages in thread
From: Michael S. Tsirkin @ 2020-05-04 14:02 UTC (permalink / raw)
  To: Yubo Miao
  Cc: peter.maydell, berrange, qemu-devel, xiexiangyou, shannon.zhaosl,
	imammedo, lersek

On Wed, Apr 08, 2020 at 08:58:10PM +0800, Yubo Miao wrote:
> From: miaoyubo <miaoyubo@huawei.com>
> 
> Write the extra roots into the fw_cfg therefore the uefi could
> get the extra roots. Only if the uefi know there are extra roots,
> the config space of devices behind the root could be obtained.
> 
> Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> ---
>  hw/arm/virt.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 7dc96abf72..0fdfe4129c 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -77,6 +77,7 @@
>  #include "hw/acpi/generic_event_device.h"
>  #include "hw/virtio/virtio-iommu.h"
>  #include "hw/char/pl011.h"
> +#include "hw/pci/pci_bus.h"
>  
>  #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>      static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
> @@ -1435,6 +1436,12 @@ void virt_machine_done(Notifier *notifier, void *data)
>      ARMCPU *cpu = ARM_CPU(first_cpu);
>      struct arm_boot_info *info = &vms->bootinfo;
>      AddressSpace *as = arm_boot_address_space(cpu, info);
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path_type("",
> +                                   "pcie-host-bridge", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
> +
> +    PCIBus *bus = s->bus;
>  
>      /*
>       * If the user provided a dtb, we assume the dynamic sysbus nodes


Seems duplicated all over the place. Add an API for that?

> @@ -1453,6 +1460,22 @@ void virt_machine_done(Notifier *notifier, void *data)
>          exit(1);
>      }
>  
> +    if (bus) {
> +        int extra_hosts = 0;
> +        QLIST_FOREACH(bus, &bus->child, sibling) {
> +            /* look for expander root buses */
> +            if (pci_bus_is_root(bus)) {
> +                extra_hosts++;
> +            }
> +        }
> +        if (extra_hosts && vms->fw_cfg) {
> +            uint64_t *val = g_malloc(sizeof(*val));
> +            *val = cpu_to_le64(extra_hosts);
> +            fw_cfg_add_file(vms->fw_cfg,
> +                   "etc/extra-pci-roots", val, sizeof(*val));
> +        }
> +    }
> +
>      virt_acpi_setup(vms);
>      virt_build_smbios(vms);


Duplicated from pc. Pls refactor.

>  }
> -- 
> 2.19.1
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6 5/8] acpi: Align the size to 128k
  2020-04-08 12:58 ` [PATCH v6 5/8] acpi: Align the size to 128k Yubo Miao
@ 2020-05-04 14:03   ` Michael S. Tsirkin
  2020-05-08 13:17     ` miaoyubo
  0 siblings, 1 reply; 16+ messages in thread
From: Michael S. Tsirkin @ 2020-05-04 14:03 UTC (permalink / raw)
  To: Yubo Miao
  Cc: peter.maydell, berrange, qemu-devel, xiexiangyou, shannon.zhaosl,
	imammedo, lersek

On Wed, Apr 08, 2020 at 08:58:13PM +0800, Yubo Miao wrote:
> From: miaoyubo <miaoyubo@huawei.com>
> 
> If table size is changed between virt_acpi_build and
> virt_acpi_build_update, the table size would not be updated to
> UEFI, therefore, just align the size to 128kb, which is enough
> and same with x86. It would warn if 64k is not enough and the
> align size should be updated.
> 
> Signed-off-by: miaoyubo <miaoyubo@huawei.com>

does this affect migration in any way?

> ---
>  hw/arm/virt-acpi-build.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 7bcd04dfb7..89bb768b0c 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -54,6 +54,8 @@
>  #include "hw/pci/pci_bridge.h"
>  #define ARM_SPI_BASE 32
>  
> +#define ACPI_BUILD_TABLE_SIZE             0x20000
> +
>  static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
>  {
>      uint16_t i;
> @@ -883,6 +885,15 @@ struct AcpiBuildState {
>      bool patched;
>  } AcpiBuildState;
>  
> +static void acpi_align_size(GArray *blob, unsigned align)
> +{
> +    /*
> +     * Align size to multiple of given size. This reduces the chance
> +     * we need to change size in the future (breaking cross version migration).
> +     */
> +    g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
> +}
> +
>  static
>  void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>  {
> @@ -953,6 +964,20 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>          build_rsdp(tables->rsdp, tables->linker, &rsdp_data);
>      }
>  
> +    /*
> +     * The align size is 128, warn if 64k is not enough therefore
> +     * the align size could be resized.
> +     */
> +    if (tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) {
> +        warn_report("ACPI table size %u exceeds %d bytes,"
> +                    " migration may not work",
> +                    tables_blob->len, ACPI_BUILD_TABLE_SIZE / 2);
> +        error_printf("Try removing CPUs, NUMA nodes, memory slots"
> +                     " or PCI bridges.");
> +    }
> +    acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
> +
> +
>      /* Cleanup memory that's no longer used. */
>      g_array_free(table_offsets, true);
>  }
> -- 
> 2.19.1
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v6 2/8] fw_cfg: Write the extra roots into the fw_cfg
  2020-05-04 14:02   ` Michael S. Tsirkin
@ 2020-05-08 12:50     ` miaoyubo
  0 siblings, 0 replies; 16+ messages in thread
From: miaoyubo @ 2020-05-08 12:50 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: peter.maydell, berrange, qemu-devel, Xiexiangyou, shannon.zhaosl,
	imammedo, lersek

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Monday, May 4, 2020 10:03 PM
> To: miaoyubo <miaoyubo@huawei.com>
> Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> lersek@redhat.com; imammedo@redhat.com; qemu-devel@nongnu.org;
> berrange@redhat.com; Xiexiangyou <xiexiangyou@huawei.com>
> Subject: Re: [PATCH v6 2/8] fw_cfg: Write the extra roots into the fw_cfg
> 
> On Wed, Apr 08, 2020 at 08:58:10PM +0800, Yubo Miao wrote:
> > From: miaoyubo <miaoyubo@huawei.com>
> >
> > Write the extra roots into the fw_cfg therefore the uefi could get the
> > extra roots. Only if the uefi know there are extra roots, the config
> > space of devices behind the root could be obtained.
> >
> > Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> > ---
> >  hw/arm/virt.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c index
> > 7dc96abf72..0fdfe4129c 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -77,6 +77,7 @@
> >  #include "hw/acpi/generic_event_device.h"
> >  #include "hw/virtio/virtio-iommu.h"
> >  #include "hw/char/pl011.h"
> > +#include "hw/pci/pci_bus.h"
> >
> >  #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
> >      static void virt_##major##_##minor##_class_init(ObjectClass *oc,
> > \ @@ -1435,6 +1436,12 @@ void virt_machine_done(Notifier *notifier,
> void *data)
> >      ARMCPU *cpu = ARM_CPU(first_cpu);
> >      struct arm_boot_info *info = &vms->bootinfo;
> >      AddressSpace *as = arm_boot_address_space(cpu, info);
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path_type("",
> > +                                   "pcie-host-bridge", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> > +
> > +    PCIBus *bus = s->bus;
> >
> >      /*
> >       * If the user provided a dtb, we assume the dynamic sysbus nodes
> 
> 
> Seems duplicated all over the place. Add an API for that?
>

Thanks for your reply. I will add the API in patch v7.
 
> > @@ -1453,6 +1460,22 @@ void virt_machine_done(Notifier *notifier, void
> *data)
> >          exit(1);
> >      }
> >
> > +    if (bus) {
> > +        int extra_hosts = 0;
> > +        QLIST_FOREACH(bus, &bus->child, sibling) {
> > +            /* look for expander root buses */
> > +            if (pci_bus_is_root(bus)) {
> > +                extra_hosts++;
> > +            }
> > +        }
> > +        if (extra_hosts && vms->fw_cfg) {
> > +            uint64_t *val = g_malloc(sizeof(*val));
> > +            *val = cpu_to_le64(extra_hosts);
> > +            fw_cfg_add_file(vms->fw_cfg,
> > +                   "etc/extra-pci-roots", val, sizeof(*val));
> > +        }
> > +    }
> > +
> >      virt_acpi_setup(vms);
> >      virt_build_smbios(vms);
> 
> 
> Duplicated from pc. Pls refactor.
> 

Sure. It would be done in patch v7

> >  }
> > --
> > 2.19.1
> >

Regards,
Miao


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v6 4/8] acpi: Refactor the source of host bridge and build tables for pxb
  2020-05-04 14:00   ` Michael S. Tsirkin
@ 2020-05-08 13:12     ` miaoyubo
  0 siblings, 0 replies; 16+ messages in thread
From: miaoyubo @ 2020-05-08 13:12 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: peter.maydell, berrange, qemu-devel, Xiexiangyou, shannon.zhaosl,
	imammedo, lersek

Thanks so much for such a careful review!!

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Monday, May 4, 2020 10:01 PM
> To: miaoyubo <miaoyubo@huawei.com>
> Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> lersek@redhat.com; imammedo@redhat.com; qemu-devel@nongnu.org;
> berrange@redhat.com; Xiexiangyou <xiexiangyou@huawei.com>
> Subject: Re: [PATCH v6 4/8] acpi: Refactor the source of host bridge and build
> tables for pxb
> 
> On Wed, Apr 08, 2020 at 08:58:12PM +0800, Yubo Miao wrote:
> > From: miaoyubo <miaoyubo@huawei.com>
> >
> > The resources of pxbs and obtained by crs_build and the resources used
> > by pxbs would be moved form the resources defined for host-bridge.
> >
> > The resources for pxb are composed of the bar space of the
> > pci-bridge/pcie-root-port behined it and the config space of devices
> > behind it.
> >
> > Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> 
> A bunch of spelling/syntax mistakes in the log, pls spellcheck.
> 
> Pls use the format
>  Yubo Miao <miaoyubo@huawei.com>
> 
> 
> 

Thanks for pointing out the mistakes!

> > ---
> >  hw/arm/virt-acpi-build.c | 131
> > +++++++++++++++++++++++++++++++++------
> >  1 file changed, 111 insertions(+), 20 deletions(-)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index
> > e8ba09855c..7bcd04dfb7 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -49,6 +49,9 @@
> >  #include "kvm_arm.h"
> >  #include "migration/vmstate.h"
> >
> > +#include "hw/arm/virt.h"
> > +#include "hw/pci/pci_bus.h"
> > +#include "hw/pci/pci_bridge.h"
> >  #define ARM_SPI_BASE 32
> >
> >  static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus) @@ -266,19
> > +269,81 @@ static void acpi_dsdt_add_pci_osc(Aml *dev, 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;
> > +    int i;
> > +    Aml *method, *crs, *dev;
> >      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;
> > +    CrsRangeEntry *entry;
> > +    CrsRangeSet crs_range_set;
> > +
> > +    crs_range_set_init(&crs_range_set);
> >      int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path_type("",
> > +                                   "pcie-host-bridge", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> 
> Not TYPE_PCIE_HOST_BRIDGE? And what if it's ambiguous?
> 
> 

Yes, TYPE_PCI_HOST_BRIDGE is ambiguous, I would fix it.  

> > +
> > +    PCIBus *bus = s->bus;
> > +    /* start to construct the tables for pxb*/
> 
> 
> coding style violation. weird that ehckpatch didn't notice it.
> 

Thanks for such a careful review!

> > +    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;
> > +            }
> > +            /*
> > +             * Coded up the MIN of the busNr defined for pxb-pcie,
> > +             * the MIN - 1 would be the MAX bus number for the main
> > +             * host bridge.
> 
> 
> Couldn't figure out this comment. Pls rephrase in some way so it's
> understandable.
> 

How about 
/* 0 - (nr_pcie_buses - 1) is the bus range for the main
  * host-bridge and it equals the MIN of the
  * busNr defined for pxb-pcie*/

> > +             */
> > +            if (bus_num < nr_pcie_buses) {
> > +                nr_pcie_buses = bus_num;
> > +            }
> > +
> > +            }
> > +
> > +            acpi_dsdt_add_pci_route_table(dev, scope, irq);
> > +
> > +            /*
> > +             * Resources deined for PXBs are composed by the folling parts:
> > +             * 1. The resources the pci-brige/pcie-root-port need.
> > +             * 2. The resources the devices behind pxb need.
> > +             */
> 
> 
> syntax/grammar errors here too.
> 

Sorry for the syntax error!

> > +            crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent),
> &crs_range_set);
> > +            aml_append(dev, aml_name_decl("_CRS", crs));
> > +
> > +            acpi_dsdt_add_pci_osc(dev, scope);
> > +
> > +            aml_append(scope, dev);
> > +
> > +        }
> > +    }
> >
> > -    Aml *dev = aml_device("%s", "PCI0");
> > +    /* start to construct the tables for main host bridge */
> 
> tables for the main.
> 
>
 
Yes, tables for the main is better.

> 
> > +    dev = aml_device("%s", "PCI0");
> 
> 
> Make dev above local in scope, then this can stay unchanged.
> 

Sure.

> >      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))); @@ -299,25
> > +364,51 @@ static void acpi_dsdt_add_pci(Aml *scope, const
> MemMapEntry *memmap,
> >          aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED,
> AML_POS_DECODE,
> >                              0x0000, 0x0000, nr_pcie_buses - 1, 0x0000,
> >                              nr_pcie_buses));
> > -    aml_append(rbuf,
> > -        aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> AML_MAX_FIXED,
> >      unsigned dsdt, xsdt;
> >      GArray *tables_blob = tables->table_data;
> >      MachineState *ms = MACHINE(vms);
> > -
> >      table_offsets = g_array_new(false, true /* clear */,
> >                                          sizeof(uint32_t));
> >
> 
> this empty line didn't hurt
> 

I would keep it.

> > @@ -952,7 +1044,6 @@ void virt_acpi_setup(VirtMachineState *vms)
> >      build_state->rsdp_mr = acpi_add_rom_blob(virt_acpi_build_update,
> >                                               build_state, tables.rsdp,
> >                                               ACPI_BUILD_RSDP_FILE,
> > 0);
> > -
> >      qemu_register_reset(virt_acpi_build_reset, build_state);
> >      virt_acpi_build_reset(build_state);
> >      vmstate_register(NULL, 0, &vmstate_virt_acpi_build, build_state);
> 
> 
> this line didn't hurt either.
> 

I would keep it

> > --
> > 2.19.1
> >

Regards,
Miao


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v6 5/8] acpi: Align the size to 128k
  2020-05-04 14:03   ` Michael S. Tsirkin
@ 2020-05-08 13:17     ` miaoyubo
  0 siblings, 0 replies; 16+ messages in thread
From: miaoyubo @ 2020-05-08 13:17 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: peter.maydell, berrange, qemu-devel, Xiexiangyou, shannon.zhaosl,
	imammedo, lersek



> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Monday, May 4, 2020 10:03 PM
> To: miaoyubo <miaoyubo@huawei.com>
> Cc: peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> lersek@redhat.com; imammedo@redhat.com; qemu-devel@nongnu.org;
> berrange@redhat.com; Xiexiangyou <xiexiangyou@huawei.com>
> Subject: Re: [PATCH v6 5/8] acpi: Align the size to 128k
> 
> On Wed, Apr 08, 2020 at 08:58:13PM +0800, Yubo Miao wrote:
> > From: miaoyubo <miaoyubo@huawei.com>
> >
> > If table size is changed between virt_acpi_build and
> > virt_acpi_build_update, the table size would not be updated to UEFI,
> > therefore, just align the size to 128kb, which is enough and same with
> > x86. It would warn if 64k is not enough and the align size should be
> > updated.
> >
> > Signed-off-by: miaoyubo <miaoyubo@huawei.com>
> 
> does this affect migration in any way?
> 

No, it would not affect migration.
I migrated one vm between two qemus(one with tables aligned to 128k and one not)
and the vm could be migrated. 

> > ---
> >  hw/arm/virt-acpi-build.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index
> > 7bcd04dfb7..89bb768b0c 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -54,6 +54,8 @@
> >  #include "hw/pci/pci_bridge.h"
> >  #define ARM_SPI_BASE 32
> >
> > +#define ACPI_BUILD_TABLE_SIZE             0x20000
> > +
> >  static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)  {
> >      uint16_t i;
> > @@ -883,6 +885,15 @@ struct AcpiBuildState {
> >      bool patched;
> >  } AcpiBuildState;
> >
> > +static void acpi_align_size(GArray *blob, unsigned align) {
> > +    /*
> > +     * Align size to multiple of given size. This reduces the chance
> > +     * we need to change size in the future (breaking cross version
> migration).
> > +     */
> > 2.19.1
> >

Regards,
Miao


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-05-08 13:19 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-08 12:58 [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM Yubo Miao
2020-04-08 12:58 ` [PATCH v6 1/8] acpi: Extract two APIs from acpi_dsdt_add_pci Yubo Miao
2020-04-08 12:58 ` [PATCH v6 2/8] fw_cfg: Write the extra roots into the fw_cfg Yubo Miao
2020-05-04 14:02   ` Michael S. Tsirkin
2020-05-08 12:50     ` miaoyubo
2020-04-08 12:58 ` [PATCH v6 3/8] acpi: Extract crs build form acpi_build.c Yubo Miao
2020-04-08 12:58 ` [PATCH v6 4/8] acpi: Refactor the source of host bridge and build tables for pxb Yubo Miao
2020-05-04 14:00   ` Michael S. Tsirkin
2020-05-08 13:12     ` miaoyubo
2020-04-08 12:58 ` [PATCH v6 5/8] acpi: Align the size to 128k Yubo Miao
2020-05-04 14:03   ` Michael S. Tsirkin
2020-05-08 13:17     ` miaoyubo
2020-04-08 12:58 ` [PATCH v6 6/8] unit-test: The files changed Yubo Miao
2020-04-08 12:58 ` [PATCH v6 7/8] unit-test: Add testcase for pxb Yubo Miao
2020-04-08 12:58 ` [PATCH v6 8/8] unit-test: Add the binary file and clear diff.h Yubo Miao
2020-04-08 13:11 ` [PATCH v6 0/8] pci_expander_brdige:acpi:Support pxb-pcie for ARM no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.