[Qemu-devel] [PATCH V5 0/7] pxb: fix 64-bit MMIO allocation

[Qemu-devel] [PATCH V5 0/7] pxb: fix 64-bit MMIO allocation

[Marcel Apfelbaum]

v4 -> v5:
  Addressed the pull request issues: (Peter Maydell)
  See: https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg00882.html
  - cland warning -> "hw/pci/pci.c:196:23: runtime error: shift exponent -1 is negative":
    The PCIe Root port was not initialized properly, the interrupt pin was left 0. This
    is a long standing issue exposed by the new test. (Patch 1/7)
  - 'make check' fails on 32-bit:
     Fix it by changing the ivshmem mem size from 4G
     to 1G, since 4G is not a valid value on 32-bit archs. (Patch 2/7)
     (4G is truncated to 0 on 32-bit systems)
  - Rebased on mst's pci branch.
  Since all the new changes are not related to the series, I kept the existing
  "Reviewed-by"/"Tested-by" signatures.

v3 -> v4:
 Addressed Igor's comments (thanks for the productive review!)
 - Split pxb test patch (previously patch 3/3) into the test itself (patch 1/6) and the blobs (patch 6/6).
 - New patch declaring pxb/pxb-pxie as not hot-pluggable.
    - Note that it does not solve the DSDT issue, but it is a prerequisite for the next patch.
 - New patch solving the DSDT issue spotted by Igor.
     - Using V=1 DIFF=diff make check does make it easier to review the ACPI changes, thanks.
 - Patches 4 and 5 untouched (previously patches 1/3 and 2/3) 

v2 -> v3:
 - split original series "pci: better support for 64-bit MMIO allocation" into 2 series:
    - this is the first part dealing with correct 64-bit MMIO ACPI computation
    - the second one will include 64-bit MMIO reservation for PCI hotplug
 - Add pxb/pxb-pcie tests (Igor) - See diffs below (*)
 - Re-based on latest master.
   
v1 -> v2:
 - resolved some styling issues (Laszlo)
 - rebase on latest master (Laszlo)



64-bit BARs allocations fix for devices behind PXBs/PXB-PCIEs.

In build_crs() the calculation and merging of the ranges already happens
in 64-bit, but the entry boundaries are silently truncated to 32-bit in the
call to aml_dword_memory(). Fix it by handling the 64-bit MMIO ranges separately.


Thank you,
Marcel

Marcel Apfelbaum (7):
  hw/pcie-root-port: Fix PCIe root port initialization
  tests/acpi: add pxb/pxb-pcie tests
  hw/pxb: declare pxb devices as not hot-pluggable
  hw/acpi: fix a DSDT table issue when a pxb is present.
  acpi: refactor pxb crs computation
  hw/apci: handle 64-bit MMIO regions correctly
  tests/acpi: Add pxb/pxb-pcie tests blobs

 hw/i386/acpi-build.c                   | 131 ++++++++++++++++++++++++---------
 hw/pci-bridge/ioh3420.c                |   1 +
 hw/pci-bridge/pci_expander_bridge.c    |   2 +
 tests/acpi-test-data/pc/DSDT.pxb       | Bin 0 -> 6286 bytes
 tests/acpi-test-data/q35/DSDT.pxb_pcie | Bin 0 -> 9098 bytes
 tests/bios-tables-test.c               |  37 ++++++++++
 6 files changed, 135 insertions(+), 36 deletions(-)
 create mode 100644 tests/acpi-test-data/pc/DSDT.pxb
 create mode 100644 tests/acpi-test-data/q35/DSDT.pxb_pcie

-- 
2.4.3

[Qemu-devel] [PATCH V5 1/7] hw/pcie-root-port: Fix PCIe root port initialization

[Marcel Apfelbaum]

Specify the root port interrupt pin as part of the init
process for cases when msi/msix are not enabled.

Fixes "hw/pci/pci.c:196:23: runtime error: shift exponent -1 is negative"
warning from clang's sanitizer.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/pci-bridge/ioh3420.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index 93c6f0b..d88cae5 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -100,6 +100,7 @@ static int ioh3420_initfn(PCIDevice *d)
     int rc;
     Error *err = NULL;
 
+    pci_config_set_interrupt_pin(d->config, 1);
     pci_bridge_initfn(d, TYPE_PCIE_BUS);
     pcie_port_init_reg(d);
 
-- 
2.4.3

[Qemu-devel] [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests

[Marcel Apfelbaum]

Add an ivshmem device with 1G shared memory to
pxb in order to check the ACPI code of 64bit MMIO allocation.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
---
 tests/bios-tables-test.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index de4019e..b23c6b0 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -864,6 +864,41 @@ static void test_acpi_piix4_tcg_ipmi(void)
     free_test_data(&data);
 }
 
+static void test_acpi_piix4_tcg_pxb(void)
+{
+    test_data data;
+
+    memset(&data, 0, sizeof(data));
+    data.machine = MACHINE_PC;
+    data.variant = ".pxb";
+    data.required_struct_types = base_required_struct_types;
+    data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
+    test_acpi_one("-machine accel=tcg"
+                  " -device pxb,id=pxb,bus_nr=0x80,bus=pci.0"
+                  " -object memory-backend-file,size=1G,mem-path=/tmp/shmem,share,id=mb"
+                  " -device ivshmem-plain,memdev=mb,bus=pxb",
+                  &data);
+    free_test_data(&data);
+}
+
+static void test_acpi_q35_tcg_pxb_pcie(void)
+{
+    test_data data;
+
+    memset(&data, 0, sizeof(data));
+    data.machine = MACHINE_Q35;
+    data.variant = ".pxb_pcie";
+    data.required_struct_types = base_required_struct_types;
+    data.required_struct_types_len = ARRAY_SIZE(ipmi_required_struct_types);
+    test_acpi_one("-machine q35,accel=tcg"
+                  " -device pxb-pcie,id=pxb,bus_nr=0x80,bus=pcie.0"
+                  " -device ioh3420,id=rp,bus=pxb,slot=1"
+                  " -object memory-backend-file,size=1G,mem-path=/tmp/shmem,share,id=mb"
+                  " -device ivshmem-plain,memdev=mb,bus=rp",
+                  &data);
+    free_test_data(&data);
+}
+
 int main(int argc, char *argv[])
 {
     const char *arch = qtest_get_arch();
@@ -884,6 +919,8 @@ int main(int argc, char *argv[])
         qtest_add_func("acpi/q35/tcg/ipmi", test_acpi_q35_tcg_ipmi);
         qtest_add_func("acpi/piix4/tcg/cpuhp", test_acpi_piix4_tcg_cphp);
         qtest_add_func("acpi/q35/tcg/cpuhp", test_acpi_q35_tcg_cphp);
+        qtest_add_func("acpi/piix4/tcg/pxb", test_acpi_piix4_tcg_pxb);
+        qtest_add_func("acpi/q35/tcg/pxb-pcie", test_acpi_q35_tcg_pxb_pcie);
     }
     ret = g_test_run();
     boot_sector_cleanup(disk);
-- 
2.4.3

[Qemu-devel] [PATCH V5 3/7] hw/pxb: declare pxb devices as not hot-pluggable

[Marcel Apfelbaum]

Prevent future issues when hotplug will work for devices
attached to pxbs.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/pci-bridge/pci_expander_bridge.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index ab86121..b4f8ca2 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -310,6 +310,7 @@ static void pxb_dev_class_init(ObjectClass *klass, void *data)
 
     dc->desc = "PCI Expander Bridge";
     dc->props = pxb_dev_properties;
+    dc->hotpluggable = false;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
 }
 
@@ -343,6 +344,7 @@ static void pxb_pcie_dev_class_init(ObjectClass *klass, void *data)
 
     dc->desc = "PCI Express Expander Bridge";
     dc->props = pxb_dev_properties;
+    dc->hotpluggable = false;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
 }
 
-- 
2.4.3

[Qemu-devel] [PATCH V5 4/7] hw/acpi: fix a DSDT table issue when a pxb is present.

[Marcel Apfelbaum]

PXBs do not support hotplug so they don't have a PCNT function.
Since the PXB's PCI root-bus is a child bus of bus 0, the
build_dsdt code will add a call to the corresponding PCNT function.

Fix this by skipping the PCNT call for the above case.
While at it skip also PCIe child buses.

Reported-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/i386/acpi-build.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index fbba461..5ed2bbd 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -597,6 +597,10 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
         QLIST_FOREACH(sec, &bus->child, sibling) {
             int32_t devfn = sec->parent_dev->devfn;
 
+            if (pci_bus_is_root(sec) || pci_bus_is_express(sec)) {
+                continue;
+            }
+
             aml_append(method, aml_name("^S%.02X.PCNT", devfn));
         }
     }
-- 
2.4.3

[Qemu-devel] [PATCH V5 5/7] acpi: refactor pxb crs computation

[Marcel Apfelbaum]

Instead of always passing both IO and MEM ranges when
computing CRS ranges, define a new CrsRangeSet structure
that include them both.

This is done before introducing a third type of range,
64-bit MEM, so it will be easier to pass them all around.

Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/i386/acpi-build.c | 81 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 50 insertions(+), 31 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5ed2bbd..d8b3543 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -748,6 +748,23 @@ static void crs_range_free(gpointer data)
     g_free(entry);
 }
 
+typedef struct CrsRangeSet {
+    GPtrArray *io_ranges;
+    GPtrArray *mem_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);
+}
+
+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);
+}
+
 static gint crs_range_compare(gconstpointer a, gconstpointer b)
 {
      CrsRangeEntry *entry_a = *(CrsRangeEntry **)a;
@@ -832,18 +849,17 @@ static void crs_range_merge(GPtrArray *range)
     g_ptr_array_free(tmp, true);
 }
 
-static Aml *build_crs(PCIHostState *host,
-                      GPtrArray *io_ranges, GPtrArray *mem_ranges)
+static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
 {
     Aml *crs = aml_resource_template();
-    GPtrArray *host_io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
-    GPtrArray *host_mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+    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];
@@ -867,9 +883,11 @@ static Aml *build_crs(PCIHostState *host,
             }
 
             if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
-                crs_range_insert(host_io_ranges, range_base, range_limit);
+                crs_range_insert(temp_range_set.io_ranges,
+                                 range_base, range_limit);
             } else { /* "memory" */
-                crs_range_insert(host_mem_ranges, range_base, range_limit);
+                crs_range_insert(temp_range_set.mem_ranges,
+                                 range_base, range_limit);
             }
         }
 
@@ -888,7 +906,8 @@ static Aml *build_crs(PCIHostState *host,
              * that do not support multiple root buses
              */
             if (range_base && range_base <= range_limit) {
-                crs_range_insert(host_io_ranges, range_base, range_limit);
+                crs_range_insert(temp_range_set.io_ranges,
+                                 range_base, range_limit);
             }
 
             range_base =
@@ -901,7 +920,8 @@ static Aml *build_crs(PCIHostState *host,
              * that do not support multiple root buses
              */
             if (range_base && range_base <= range_limit) {
-                crs_range_insert(host_mem_ranges, range_base, range_limit);
+                crs_range_insert(temp_range_set.mem_ranges,
+                                 range_base, range_limit);
             }
 
             range_base =
@@ -914,35 +934,36 @@ static Aml *build_crs(PCIHostState *host,
              * that do not support multiple root buses
              */
             if (range_base && range_base <= range_limit) {
-                crs_range_insert(host_mem_ranges, range_base, range_limit);
+                crs_range_insert(temp_range_set.mem_ranges,
+                                 range_base, range_limit);
             }
         }
     }
 
-    crs_range_merge(host_io_ranges);
-    for (i = 0; i < host_io_ranges->len; i++) {
-        entry = g_ptr_array_index(host_io_ranges, i);
+    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(io_ranges, entry->base, entry->limit);
+        crs_range_insert(range_set->io_ranges, entry->base, entry->limit);
     }
-    g_ptr_array_free(host_io_ranges, true);
 
-    crs_range_merge(host_mem_ranges);
-    for (i = 0; i < host_mem_ranges->len; i++) {
-        entry = g_ptr_array_index(host_mem_ranges, i);
+    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(mem_ranges, entry->base, entry->limit);
+        crs_range_insert(range_set->mem_ranges, entry->base, entry->limit);
     }
-    g_ptr_array_free(host_mem_ranges, true);
+
+    crs_range_set_free(&temp_range_set);
 
     aml_append(crs,
         aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
@@ -1899,8 +1920,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
 {
     CrsRangeEntry *entry;
     Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
-    GPtrArray *mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
-    GPtrArray *io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+    CrsRangeSet crs_range_set;
     PCMachineState *pcms = PC_MACHINE(machine);
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
     uint32_t nr_mem = machine->ram_slots;
@@ -1989,6 +2009,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
     }
     aml_append(dsdt, scope);
 
+    crs_range_set_init(&crs_range_set);
     bus = PC_MACHINE(machine)->bus;
     if (bus) {
         QLIST_FOREACH(bus, &bus->child, sibling) {
@@ -2015,8 +2036,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
             }
 
             aml_append(dev, build_prt(false));
-            crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent),
-                            io_ranges, mem_ranges);
+            crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent), &crs_range_set);
             aml_append(dev, aml_name_decl("_CRS", crs));
             aml_append(scope, dev);
             aml_append(dsdt, scope);
@@ -2037,9 +2057,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
                     AML_POS_DECODE, AML_ENTIRE_RANGE,
                     0x0000, 0x0000, 0x0CF7, 0x0000, 0x0CF8));
 
-    crs_replace_with_free_ranges(io_ranges, 0x0D00, 0xFFFF);
-    for (i = 0; i < io_ranges->len; i++) {
-        entry = g_ptr_array_index(io_ranges, i);
+    crs_replace_with_free_ranges(crs_range_set.io_ranges, 0x0D00, 0xFFFF);
+    for (i = 0; i < crs_range_set.io_ranges->len; i++) {
+        entry = g_ptr_array_index(crs_range_set.io_ranges, i);
         aml_append(crs,
             aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED,
                         AML_POS_DECODE, AML_ENTIRE_RANGE,
@@ -2052,11 +2072,11 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
                          AML_CACHEABLE, AML_READ_WRITE,
                          0, 0x000A0000, 0x000BFFFF, 0, 0x00020000));
 
-    crs_replace_with_free_ranges(mem_ranges,
+    crs_replace_with_free_ranges(crs_range_set.mem_ranges,
                                  range_lob(pci_hole),
                                  range_upb(pci_hole));
-    for (i = 0; i < mem_ranges->len; i++) {
-        entry = g_ptr_array_index(mem_ranges, i);
+    for (i = 0; i < crs_range_set.mem_ranges->len; i++) {
+        entry = g_ptr_array_index(crs_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,
@@ -2091,8 +2111,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
     aml_append(dev, aml_name_decl("_CRS", crs));
     aml_append(scope, dev);
 
-    g_ptr_array_free(io_ranges, true);
-    g_ptr_array_free(mem_ranges, true);
+    crs_range_set_free(&crs_range_set);
 
     /* reserve PCIHP resources */
     if (pm->pcihp_io_len) {
-- 
2.4.3

[Qemu-devel] [PATCH V5 6/7] hw/apci: handle 64-bit MMIO regions correctly

[Marcel Apfelbaum]

In build_crs(), the calculation and merging of the ranges already happens
in 64-bit, but the entry boundaries are silently truncated to 32-bit in the
call to aml_dword_memory(). Fix it by handling the 64-bit MMIO ranges separately.
This fixes 64-bit BARs behind PXBs.

Reported-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/i386/acpi-build.c | 54 +++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 45 insertions(+), 9 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index d8b3543..b1adf04 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -751,18 +751,22 @@ static void crs_range_free(gpointer data)
 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)
@@ -920,8 +924,14 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
              * that do not support multiple root buses
              */
             if (range_base && range_base <= range_limit) {
-                crs_range_insert(temp_range_set.mem_ranges,
-                                 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 =
@@ -934,8 +944,14 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
              * that do not support multiple root buses
              */
             if (range_base && range_base <= range_limit) {
-                crs_range_insert(temp_range_set.mem_ranges,
-                                 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);
+                }
             }
         }
     }
@@ -963,6 +979,19 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
         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,
@@ -2085,11 +2114,18 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
     }
 
     if (!range_is_empty(pci_hole64)) {
-        aml_append(crs,
-            aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
-                             AML_CACHEABLE, AML_READ_WRITE,
-                             0, range_lob(pci_hole64), range_upb(pci_hole64), 0,
-                             range_upb(pci_hole64) + 1 - range_lob(pci_hole64)));
+        crs_replace_with_free_ranges(crs_range_set.mem_64bit_ranges,
+                                     range_lob(pci_hole64),
+                                     range_upb(pci_hole64));
+        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(crs,
+                       aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
+                                        AML_MAX_FIXED,
+                                        AML_CACHEABLE, AML_READ_WRITE,
+                                        0, entry->base, entry->limit,
+                                        0, entry->limit - entry->base + 1));
+        }
     }
 
     if (misc->tpm_version != TPM_VERSION_UNSPEC) {
-- 
2.4.3

[Qemu-devel] [PATCH V5 7/7] tests/acpi: Add pxb/pxb-pcie tests blobs

[Marcel Apfelbaum]

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 tests/acpi-test-data/pc/DSDT.pxb       | Bin 0 -> 6286 bytes
 tests/acpi-test-data/q35/DSDT.pxb_pcie | Bin 0 -> 9098 bytes
 2 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tests/acpi-test-data/pc/DSDT.pxb
 create mode 100644 tests/acpi-test-data/q35/DSDT.pxb_pcie

diff --git a/tests/acpi-test-data/pc/DSDT.pxb b/tests/acpi-test-data/pc/DSDT.pxb
new file mode 100644
index 0000000000000000000000000000000000000000..8839fcfe4246cdca093c5890c98e64dc5a10f8b8
GIT binary patch
literal 6286
zcmb_gZEqXL5uPQF(s4;iN9k<KZ!sa;apT4|UmV#%i$LTZCDP(iXCCFcIHPxzWt27&
z_@LOR6(cCc0E&|j1p>xJ`>C{{&-PD9e?opj`WI5P_9v?|cU+35bbtetfT*3FXP=qb
zot@pKZt0D`%mT1<aYZXz>EcaO)3J{M0JZ7AQju<f-PVgs4ogHNW^X0@HhvXl^+Ub5
zy2Sq0a{ubO559I;(}T6=%G0%`_tg`34G8pb&EuRJ%A8{r9lLYf)G8U-DOdDl;YS{o
zoq}Em*;%dVOflpDQ<BSOsUbiNL8wMI%4WlWB*2ev5+K=I3du7!N;N%g)^#VX&O274
z03P+Y9(6UpaJl04lHYY*=(>sB-5uyne%Ecn_st~y>p!s_*x^&Mqt&fcicfCGF=8YK
z31AbHvlq!5a@X!0HE(9~zOTtBFm%Pt=Cug$U1cFQ1k<F7Rd50vPAuUrXS4`+IlBA)
zA1)FLjRYuPg7K<Zsxi0<QLR9Ys8+;(igxN!R2U{UyK|x5(2z8kvII^R`8&%GEin%~
zP$F}{me>YtKxAwa?9P?tqCS8}qiWorl|i8XKo?wKIBFf%@&8-|<Fz~IjFr{Fz^R$h
zfijQHfkME7yr^q~MI7&YbezvFv7e(qM?qR*ucEJTqUILhyySCd7prDLEcT*anNBOf
zV|;~Y7=Otl{4D-W^W%J%f6D*Mr?@OTi+U+_gsYT>!=DHWcIT6-*%<gLfjS#3Jx!_G
z8^h(^foIYnW`$2Iu}9HIQCXgA`~H+9ZN<onNsX}}0|gB&NEprQ=<6uRbBa&1NHn<-
z8Xg(&;EIOwL|#XbM@Al{Ebk>z+tjQEOTY^*V~UfYVGgdQ@k>wJZL4$*x1#)lGAcoW
zR?L*Hp`utb>MWrI3KFzpZPiHItXS2SK|O0~rE3xUG)cR=UW=68Cc6tX+&)L$JD}8X
z3l$w@nN!-UI(N)^1H9=-+x4LNycmDl<ZbVZCXc|H_jseZ20RM4drJ2<g5tPss<mYu
z)_Z5Vw^=XMZE|VQZNd7piGfr=P^x>IM+QB`eou7JqrtlO<>ne!05>i?SqJbQH__gc
zbq3!06SoNun>(((b>>o(yrsQDyN#Lk>lqopvh<asue_|zRIcN!_TrCuyM>!RzQYsh
zeC4`Uk0YR-Nlo&^H)9a@lWh~er9K|R?@BN6w1qLZsJ%nocBeXyVPY}|8#0LngvyhQ
zb7iFt^O<iZAs&rkmU8?{;NK>wQ^6i2C&)cD<>(a)zN9v9`#n>%t6_&+^5r9rRkzr;
zt-uScX7vj7_pKL4tYiMZv-Rwd^{j6_i~bE;&#DJo&)OY>V%J8j^T;nSpBplt^Udc*
z%;!SP=lbULQS*2}ymR#VA@h0Pe161yKE!<9?yN`{y)|lH3e1ZXqEiHyT$J5}I_597
znh55YT*WS0U8Qm*IPf%nYp>{=_0qruAM=xILNiy><@*o3Papm<y>aLB`wus6-}xMD
zrkT~{e)*l?e2KwJc*&AjqcN%cD)N#i?XA-!6d<k9fumJw7Srm+4Tc3F!$8tBL&s7x
zO`xb|m8xk(>V-yHO*)lYMfQL?G4VdU<yR^6CGc;epqZ6SCYvD-g7ZA4rnJW5?;}0l
zZNavd$z<-T<uR;bg}8nC)+I?CY!oB;PL*i18;yD3_KtN&7396gr(FBoK23Fb$5o9O
zMfA%Iu~8~yIFMg0PimG{lf%Mb?(;&T>*A4&rE-)MXSlZ{?F-0X37oD`I8HwE_IZYA
zkGap5rCxbImA{?`Q}O72E`%f3!GRiaZN+LN!vZ~Uz4kea5qozJh|A-V(O!by8|}m?
zIM6%$D?GU|t#FUw2Qw5?OWvUb!z45cjZ}imAk*CowF!qZoRR|6Ne6>UP-M`$drv)%
zxx8eSoKhx-tL0y~D6eUWmD+{9)3{T(V3f_E;!Kz|GgHXSqa`f(6ULp-?r&_STl==9
zI^$RSUc77QwL0EJ@UXF&;UB|=F#}D&o(}WHK25sa-RHl3>e2|$HE<8~X1WF?h=Hq$
zX~jr*cDSc@4O(lzmsXaf4VTuENOqWd!JwT#MC<*%1wLV-Ym_XfzlD6Xs}^Prto*oV
z0v7~UHtO@2Q+b~G_SXP-<sJFF*$qCTyD#qI$w>qGBsyCSn$y@U2hI1ZCDD5xoBiIy
zM&V{3dkmXp&!`v6G>95iBEm+sOarJ<C6W#5rUs2A!B^Ahp8AyfNWVcc^E*?1CwnmH
zp;nc2)t)}yrEb8G^SRV1e&XvR!y!|s6efk}hnLTBj`R-3gw07D+ixC5ha8VAliDM4
zQhMYksroBK!v@Xy%jy}!ylJq}1@N%f5sT`ozcMg!rG22g#!BgicMp|XK&j3dnC*DY
zdwV%CP4K1|K$OZDWxjui^(#20wE8h_g)GpfAG|(-tKv|Hsnz^l`4&PwiX-#GC66`?
zWw}yTZUyJt%9N>H#Rmg!8{QVjCq4d(#Tsv`3voOYhwhoR@prsu;>D8<?u-AwJ7w^~
zx+d{OKNrLcq>U-?LqFvghBa+Wl7GAjG--G;0TFzpLL0X5o1RumMGcNlx~B%@i=00f
zK!88{@Fn&-`8vsD>}SEAdxbo&lLL9ePsM)^Cn(3nq&p^kA~5U}$mlK+)92A!nt!6A
z?{52(VNDMM`C$+UJ@qsRb0v7{jg&v?AG(X+)4fsPi6A=vF7ngMpI$PFNUlj{*{lZ{
z_jV1%2|3_iv!4rIzze40Onz_+tFM)JrpF|P>w@je&a^Dy>k;3DD1Ag4idgy0-jv&&
zi;KV076S*amyA_BKyrj0@mPr|F}gh|+MoQy#UCVwvfn=k{E9vlBwq44%+iIcY|{G+
zIG?q*PLU9$`jw_?hF?``Rk4NJI&E*ABtQWcxL=X6USOT~%R0U8?5#t=c)E+x4#zJ`
zORHm4^drjRXMP$LQHyh%ure8Z2-B}9cIUj6?F*PV5l}J2td0i|HFBbnr-t2`va(l$
zfryP_#|Ku923xB&jyGB)#D08PPy2@`y&lOTseLGoC1^}QV`0#|V8y#^z&aKR6|Ca|
zIvxhSELicEf`C;Hg$mZVfX2h19|=}2pga^RSTW}V2NGe>dBK_#&}1l7uucf*L>Tmn
zV4W1u$xx_Z#kXqU>r@!@s$f+FR0)L&)|7yz!k`}u)@cEq4uuNV83COMgBAoU=64X5
z8VVJxhXnLc7_=x@4-4qwP^e%%BA`dYpqgNv70}sGs9-%Rphv@?x?nvfpvOX?g7vt7
z9uI?-1nUU_JrN2OtS1HZWEgZ&u)Zgt?}b7I>nQ;}br4itKBnKOH6p#_;}-r*v`q!<
zt}moXxY`#MP{1M-Xxb!@X5w)xfzO0(3oA)qijXwWNho1G=<8&X20Ai^ySlF<Yx4sg
znZv!=*O9#|q?>x&!h(ut*0zOvOvrd@p}eQZvuoSJ-6Hh(ltFn<uaJy+FZ5|&-qSCT
zj2SHSZC~Eg`w?R<3O$|@+m<$@r>HTj26~EHAJS9gm>&aun&ct<0?N431AUuh)T^iH
W_K3e%(+`idYIumPvVKG7(f<K`)D2qz

literal 0
HcmV?d00001

diff --git a/tests/acpi-test-data/q35/DSDT.pxb_pcie b/tests/acpi-test-data/q35/DSDT.pxb_pcie
new file mode 100644
index 0000000000000000000000000000000000000000..afcb5314a298ab9d4a1fd07dade044b164d56411
GIT binary patch
literal 9098
zcmb7KU2hx56`dt1YB{8&rL>aeuZXaVrb*+3l3XX~M{07HB3W@Iilp;V8XzSrt?abO
zLa~83Mi5B>64ws}64pWcRNByU{zCH;^40)->T6$%B7TZ`?#vE7LrOr*hvnSabMBq9
zJ3D8&OTX#29$aM1x>a2FD$QK+t-9x<&ti;Go4%cD;yP>Z`NeX_O2-mbdp|AO*c5H|
zCBJyBZ2fho`(?L#`?IdK8M3X%&cm(E@RR%9Ek>X_TOsGva9-#%i=Fmf-D|e2H2aEY
zM7wCEGhb0!^cSzzy=u{R)-wWnP|hIE+nq|)&lJAlw%ze$D{glReuLSaYgON}o7d`f
zW_zthjk$iRv)=R)58h>-|K?(UWxThOo#9S9_}B7x3+JwW`R47y*T4Gb?;gC%696pX
z+s5~DNCD-c#xkY4M;m^gwteS|-c9Qx>{%4(#s1tEJ%y9q4($vHO#P2CvX@FM9=_`Y
zw$xLyEA>)K>9L>!<Hymh)LQ||F!s&&X_jd3I~32nQCjnJ^@iWcxmP;PR)K}o(+#O>
z^TRH8y8X=Oy)Nr@(?>^#tUvjAZ<GCHGsFJ<zp+Dh$ft0$8qI8$Pkx%hi9~XiVC*h)
zu8zM<!=XdlKJb#&W}zbvCkC*9!PKjbjTrCJ>8W8p#P}YMwf9d`n|bR&A<is(74cQ8
zxBQqum16vWP_salm{%yRSzZyFS{)nb>Zw=asv=cuA6#m*JQS7ZHI4H~Bc127Wh-PM
zb7B{mRkn87E{i4avi8BNt3`hRkH_7VI35Fug4ktUW`VWMw()%-2BqzT=Y#cY1LD-o
zFwy2?7nno9*P&$qOfYv|9&s9fx1YpOK4gy)gMtp9C|e)IKZx7*g*{QIl)y1rREowd
zIWtunIv^*dc@lpTXZ8h0q*<XfOG9aRY>)@X^u#=|Co$x)k;krAy%pwWI{3E29g3Vk
zzirVeaJ>JBORC;z)u&hA-<wV+Fnd^SZMDQ?+d_MVO=qZ|#}JfN5Jym9(-Voz(Givg
ziHjm)0XQl52_B#-Bqs6;Y)mkLjUr+yXp9Lapn{2zm>7+~#syQ2faagbDxh)61XP8@
zR8=tVC067~2&RH2BvTC@p{{ex&^acU3OXj4YVZhkok>GyQZN-XY3f9%>l`<9jvG41
zO`QmJowlLVHgwviPK3HntPt9fP8m8=rcQ*qPHyPrhE8tkM5yab8#>d5&a|l$p{^4v
zm3BTehR%$s6QQni!q7Qk=$tThBGh$G8agKpos*_cgu2ctL+6yCbIR0-P}k`gIvqo&
zW9mex>&zNDvxd&BsS}~DbK1~3ZRngfbt2St&KNpp44pHkPK3Hn*U;%2I$cvILS5&q
zp>x*IIcw@fsOy|Fbj}$%=S-alb)BaSou>?)r%as)b)EBu&Ur)Uyr~nRuJg2D7NV8#
zv|tva#qP9ZdQU@Q`Vsbw!8~Iy&zMYvI`gc-JZmt|noNW`^PIsvXE4v1OoTe~yumzg
zFwdJzggWy%!Bj5uoM5Uo{yE813pYYyDkoVmYAzTx7tERnWlb9fTF?x%+^h}^6m1la
z3={#%${>zF6@`IHoUkMd_d5cp95W`NDkP?o1_PB?WuO|A3{*k|lMECgMxzW=Vxx#^
zDz7C2mC%8KBE--I76vM@QAA92aLGU=R4~av5h|T9P>B^x7^ns%1C>z0Bm+gLbizO-
zRxn|p8k7uFLIsly6rs`y1C?08gn?>MGEfN>Ofpb}N+%3dVg(ZhszJ#>B~&oUKoKgP
zFi?pVOc<yJB?FaE!6XAksC2?WB~~zDpc<46R6+%l3>2Z#2?Lc_!GwWoP%=;n6-+Wv
zgi0q2RAL1a2C6~HKqXW#$v_b*oiI>|6-*eY1|<WPP{AYvMW}SbKqXc%VW1k63{*k|
zlMEE0(g_2VSiyvWYEUvz2^CB-P=rb+3{+wT69%e5$v`DkFv&m>DxEM;i4{y3s0Jki
zl~BPX14XEG!ayZfFkzq?lnhit1(OUEq0$Khl~}=qfof1PPze=GGEjs{Ck#|#1rr9U
zLCHWRR4~av5h|T9P>B^x7^ns%1C>z0Bm+gLbizO-Rxn|p8k7uFLIsly6rs`y1C?08
zgn?>MGEfN>Ofpb}N+%3dVg(ZhszJ#>B~&oUKoKgPFi=FAfg;ik6rpaQ2sH!Mm@rU{
zNd~Gh$v`zG3{+#nKs6>AsKz7%)tE3)jR^zQm}H<DlMGa2!ay}93{+#1foe=LP>l%#
zMWoIy3=|R0m^heZpa^jqR7gy9Y+;~?)UkzuB2vee3>2X{wq&3PedeC&z(QF+(-S{d
z59tT#qa=M5+WS9$B~HIm=~)3ijj-yCjRG8ZqevT%Hond9Y-FQE?G@UTX;ZP?nd$}}
zFZEOR_}&g4f}{?4+Pz%e@ER!q8u{!bPyZ>&QuJsGzmy+PA0NZE8s6LCQ;b>|L<3Jo
zYU8*^D5&4!89Yv*hcz~a-OSGMnLkakR2=TcuzoYF#K`7O$>$O@1dOKOW;=eh$v&bs
z-xEDk^rV9w(tt#Ks?%&Vtp_{N7^pn;N7ai%)$v&22Ujm@)r+Ef5#!UV7v1Bl7uyE`
zY4?Vz^Vl<#_q6h!DDMrG_f9D9MdkhB@_1bKgVp<5d0&+GhsygWl=q|ZrQ!04XDDCN
z%9lj>(op%*3FS*s`IX`FW6w~2MJvA|%C8KSUpb-tN>si)Tt4{><;z<6vM663DqlXK
zd^sv#87@Em4CO0Y`HCoC87f~np?n496SR~Kmv>}&wRqsRf*!Mpr<P`)n-rHkzUaD+
z(hXn6JZq-Iy~ap7*>`NZk?V5WOotngk#w@}*mNV;*^HSEcO@g~WZ$vrMy?yzOov;Q
zk#w@}*mNV;!K|4M_b(&qWZ$vrM&|XLnGQEKBk5${vFS$U=_xZE?sP`d$v!ikQ+-vw
zA@KP1r_ajkjZ#z1M2-s({xjj#ujMN5-46e7=l8kYciw;R&hCwO-e+ygtJhYe89Bju
z+4?r~ZN|#r(KdXC-nV?4!2|vMixd=C&ZFn@UUjW$d5z!=%PajRZ?S~u1-{+hpC(Y;
zYnEy`ApJrsheyfPwW=L5>V!AFbX~kbz*yw?CJJ7?n$IuhX$0~O&6u0@S}SkI`n<Qp
zKK1hX{7tu##H*h|s(tbLD+%>JfHczBkB-8f;cgTQS+{?tH@t&JAM&Yg`}vVQwbjF}
z8;p^nXDeh4r$UBf^ShNvuj#GXC&{q7j|=Hu7jH7UAsbGLBit`1Mg)m7fx|Tn$HBM4
zk&yu%THQyhiGF1?l{lZrr{eL^T!16j@j`=?x87`JP7-?T`5GB46o)sD(G~L8a4$h`
z4R_)Y96P%9CwS+AX@x5cAIwlpE&Grr7$#v*Xl2vfX81i%cY1ERLo=Kv1#>6yg0Ph)
zWo+l>E%z+ua=BjWl=4eBTjD(x&1*Wv&e|pIO<CbmP^rtsa-zOg&lmDnP>HU|;GI9d
zw|h6YGjdtFGvbD@pXvqvS_AKpa5vhvf||F=+(I%y3HEd!=1cn`?K?Vp{L6=3VualQ
z*Fb-!7tjO=a8@y`ED9dY_1#`T=UVjAnWbpAOXrf399r(BfUbNkZTPnxCK9H5L8;k^
zE+K*Lx`p`wH;1@p<Pa3r4jNZ3XP0^Y!A}{>;1!Jd`Tj_LRBsd>;ntdnd=i7L$>ubg
z71{i8t)zNiL^JB03kq*V*fVHW!k|&C5Q!Q!N`ywOLIi5mD9J8$Q-in^h~32QyAP?4
z_U}?`iO#I(v~CZ2s8yqVwZ1>zqizPt%S+h>e(p0Y)lVgpRFX*&g}mh@&XK+YO=wPn
z8#U)JpiW4YX?IA;X=g~|q+QV&BHEyN<+A&up#D~14WEDzy&ZK@yY5c{Ox(l>=w`59
zdLz7rot{kTUI;MTV<SKd-O&IXG4X}LL<=}(w0jy?!m_?@(X6Mb>eRB=Ba_P3z1Q$s
z9&?}X4Gapm=f&b0-*c~~#810$I<|)uaP#rn_`^-U7k;?OV{9wDx4XH8AA#&f-|5|8
z;dDyAob%Z>by$Or0y?t28$34XDMmf<L666_!;kK6afi`M%=_Dnh4(g*`~J4Y!tMLr
zO?GGVuq*G=*2eg6#R(NZ{TSQB`TYC$a^B{cnBCDH*+&7+^skLh#;(%d`x7jNM`&!1
z?PE)?njLRadZPTp0=>?z+5El&GWM~6m#rt6CmG8|za?kZcgXW3Gl(a|9Q&_6^Eie<
zzc1+%W4Z+P_0iGCk%Yb1^*w!v`^P{Wz2No|SmfkiDTY=r>ep^1@K=Rafv4p`6Soq%
zWG-ppExGf;bu4(z<<4|6Vd1c#eYrDjCwL4emZG209vY_ei>C*peem+io1VOlo7gA?
z*RW<U5qhYKGv<uZ4U6MXiD`yEO=!15V(Ar&s5UVC^yX}y?r|#hpn?CCa6aGOU!WjP
zOGb_s5NvAHs-cA|Dc9aVPk;iu%EcmxOB741SV?hZ;EyP?l25lkbWvf`^m1;J*4da-
z9+Q8YNH_C&{9l0ngwj5Ev6+8O1~{1(!wqk9(Z^~}jV0Asrbhc<s<{{ufsHXYS@+gj
F?EeUajd=h7

literal 0
HcmV?d00001

-- 
2.4.3

Re: [Qemu-devel] [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests

[Igor Mammedov]

On Sun, 17 Jul 2016 19:53:09 +0300
Marcel Apfelbaum <marcel@redhat.com> wrote:

> Add an ivshmem device with 1G shared memory to
> pxb in order to check the ACPI code of 64bit MMIO allocation.
what is forcing ivshmem to be mapped above 4G?

> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> Tested-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  tests/bios-tables-test.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index de4019e..b23c6b0 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -864,6 +864,41 @@ static void test_acpi_piix4_tcg_ipmi(void)
>      free_test_data(&data);
>  }
>  
> +static void test_acpi_piix4_tcg_pxb(void)
> +{
> +    test_data data;
> +
> +    memset(&data, 0, sizeof(data));
> +    data.machine = MACHINE_PC;
> +    data.variant = ".pxb";
> +    data.required_struct_types = base_required_struct_types;
> +    data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
> +    test_acpi_one("-machine accel=tcg"
> +                  " -device pxb,id=pxb,bus_nr=0x80,bus=pci.0"
> +                  " -object memory-backend-file,size=1G,mem-path=/tmp/shmem,share,id=mb"
> +                  " -device ivshmem-plain,memdev=mb,bus=pxb",
> +                  &data);
> +    free_test_data(&data);
> +}
> +
> +static void test_acpi_q35_tcg_pxb_pcie(void)
> +{
> +    test_data data;
> +
> +    memset(&data, 0, sizeof(data));
> +    data.machine = MACHINE_Q35;
> +    data.variant = ".pxb_pcie";
> +    data.required_struct_types = base_required_struct_types;
> +    data.required_struct_types_len = ARRAY_SIZE(ipmi_required_struct_types);
> +    test_acpi_one("-machine q35,accel=tcg"
> +                  " -device pxb-pcie,id=pxb,bus_nr=0x80,bus=pcie.0"
> +                  " -device ioh3420,id=rp,bus=pxb,slot=1"
> +                  " -object memory-backend-file,size=1G,mem-path=/tmp/shmem,share,id=mb"
> +                  " -device ivshmem-plain,memdev=mb,bus=rp",
> +                  &data);
> +    free_test_data(&data);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>      const char *arch = qtest_get_arch();
> @@ -884,6 +919,8 @@ int main(int argc, char *argv[])
>          qtest_add_func("acpi/q35/tcg/ipmi", test_acpi_q35_tcg_ipmi);
>          qtest_add_func("acpi/piix4/tcg/cpuhp", test_acpi_piix4_tcg_cphp);
>          qtest_add_func("acpi/q35/tcg/cpuhp", test_acpi_q35_tcg_cphp);
> +        qtest_add_func("acpi/piix4/tcg/pxb", test_acpi_piix4_tcg_pxb);
> +        qtest_add_func("acpi/q35/tcg/pxb-pcie", test_acpi_q35_tcg_pxb_pcie);
>      }
>      ret = g_test_run();
>      boot_sector_cleanup(disk);

Re: [Qemu-devel] [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests

[Marcel Apfelbaum]

On 07/18/2016 04:34 PM, Igor Mammedov wrote:
> On Sun, 17 Jul 2016 19:53:09 +0300
> Marcel Apfelbaum <marcel@redhat.com> wrote:
>
>> Add an ivshmem device with 1G shared memory to
>> pxb in order to check the ACPI code of 64bit MMIO allocation.
> what is forcing ivshmem to be mapped above 4G?

For PC machine: nothing -> ivshmem gets mapped in 32-bit area
For Q35 machine: there is not enough space for mapping 1G
in 32-bit area, and maybe there are also alignment constrains.
You can use this test in verbose mode and see exactly how
the ranges are distributed. If you want me to run it and add the output,
please let me know.

Thanks,
Marcel

>
>>
>> Suggested-by: Igor Mammedov <imammedo@redhat.com>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>> Tested-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>   tests/bios-tables-test.c | 37 +++++++++++++++++++++++++++++++++++++
>>   1 file changed, 37 insertions(+)
>>
>> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
>> index de4019e..b23c6b0 100644
>> --- a/tests/bios-tables-test.c
>> +++ b/tests/bios-tables-test.c
>> @@ -864,6 +864,41 @@ static void test_acpi_piix4_tcg_ipmi(void)
>>       free_test_data(&data);
>>   }
>>
>> +static void test_acpi_piix4_tcg_pxb(void)
>> +{
>> +    test_data data;
>> +
>> +    memset(&data, 0, sizeof(data));
>> +    data.machine = MACHINE_PC;
>> +    data.variant = ".pxb";
>> +    data.required_struct_types = base_required_struct_types;
>> +    data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
>> +    test_acpi_one("-machine accel=tcg"
>> +                  " -device pxb,id=pxb,bus_nr=0x80,bus=pci.0"
>> +                  " -object memory-backend-file,size=1G,mem-path=/tmp/shmem,share,id=mb"
>> +                  " -device ivshmem-plain,memdev=mb,bus=pxb",
>> +                  &data);
>> +    free_test_data(&data);
>> +}
>> +
>> +static void test_acpi_q35_tcg_pxb_pcie(void)
>> +{
>> +    test_data data;
>> +
>> +    memset(&data, 0, sizeof(data));
>> +    data.machine = MACHINE_Q35;
>> +    data.variant = ".pxb_pcie";
>> +    data.required_struct_types = base_required_struct_types;
>> +    data.required_struct_types_len = ARRAY_SIZE(ipmi_required_struct_types);
>> +    test_acpi_one("-machine q35,accel=tcg"
>> +                  " -device pxb-pcie,id=pxb,bus_nr=0x80,bus=pcie.0"
>> +                  " -device ioh3420,id=rp,bus=pxb,slot=1"
>> +                  " -object memory-backend-file,size=1G,mem-path=/tmp/shmem,share,id=mb"
>> +                  " -device ivshmem-plain,memdev=mb,bus=rp",
>> +                  &data);
>> +    free_test_data(&data);
>> +}
>> +
>>   int main(int argc, char *argv[])
>>   {
>>       const char *arch = qtest_get_arch();
>> @@ -884,6 +919,8 @@ int main(int argc, char *argv[])
>>           qtest_add_func("acpi/q35/tcg/ipmi", test_acpi_q35_tcg_ipmi);
>>           qtest_add_func("acpi/piix4/tcg/cpuhp", test_acpi_piix4_tcg_cphp);
>>           qtest_add_func("acpi/q35/tcg/cpuhp", test_acpi_q35_tcg_cphp);
>> +        qtest_add_func("acpi/piix4/tcg/pxb", test_acpi_piix4_tcg_pxb);
>> +        qtest_add_func("acpi/q35/tcg/pxb-pcie", test_acpi_q35_tcg_pxb_pcie);
>>       }
>>       ret = g_test_run();
>>       boot_sector_cleanup(disk);
>

Re: [Qemu-devel] [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests

[Igor Mammedov]

On Mon, 18 Jul 2016 17:17:48 +0300
Marcel Apfelbaum <marcel@redhat.com> wrote:

> On 07/18/2016 04:34 PM, Igor Mammedov wrote:
> > On Sun, 17 Jul 2016 19:53:09 +0300
> > Marcel Apfelbaum <marcel@redhat.com> wrote:
> >  
> >> Add an ivshmem device with 1G shared memory to
> >> pxb in order to check the ACPI code of 64bit MMIO allocation.  
> > what is forcing ivshmem to be mapped above 4G?  
> 
> For PC machine: nothing -> ivshmem gets mapped in 32-bit area
> For Q35 machine: there is not enough space for mapping 1G
> in 32-bit area, and maybe there are also alignment constrains.
> You can use this test in verbose mode and see exactly how
> the ranges are distributed. If you want me to run it and add the output,
> please let me know.
No need to post diff, I can do V=1 manually,
the question though is: doesn't PC need ivshmem to go above 4G as well?

> 
> Thanks,
> Marcel
> 
> >  
> >>
> >> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> >> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> >> Tested-by: Laszlo Ersek <lersek@redhat.com>
> >> ---
> >>   tests/bios-tables-test.c | 37 +++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 37 insertions(+)
> >>
> >> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> >> index de4019e..b23c6b0 100644
> >> --- a/tests/bios-tables-test.c
> >> +++ b/tests/bios-tables-test.c
> >> @@ -864,6 +864,41 @@ static void test_acpi_piix4_tcg_ipmi(void)
> >>       free_test_data(&data);
> >>   }
> >>
> >> +static void test_acpi_piix4_tcg_pxb(void)
> >> +{
> >> +    test_data data;
> >> +
> >> +    memset(&data, 0, sizeof(data));
> >> +    data.machine = MACHINE_PC;
> >> +    data.variant = ".pxb";
> >> +    data.required_struct_types = base_required_struct_types;
> >> +    data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
> >> +    test_acpi_one("-machine accel=tcg"
> >> +                  " -device pxb,id=pxb,bus_nr=0x80,bus=pci.0"
> >> +                  " -object memory-backend-file,size=1G,mem-path=/tmp/shmem,share,id=mb"
> >> +                  " -device ivshmem-plain,memdev=mb,bus=pxb",
> >> +                  &data);
> >> +    free_test_data(&data);
> >> +}
> >> +
> >> +static void test_acpi_q35_tcg_pxb_pcie(void)
> >> +{
> >> +    test_data data;
> >> +
> >> +    memset(&data, 0, sizeof(data));
> >> +    data.machine = MACHINE_Q35;
> >> +    data.variant = ".pxb_pcie";
> >> +    data.required_struct_types = base_required_struct_types;
> >> +    data.required_struct_types_len = ARRAY_SIZE(ipmi_required_struct_types);
> >> +    test_acpi_one("-machine q35,accel=tcg"
> >> +                  " -device pxb-pcie,id=pxb,bus_nr=0x80,bus=pcie.0"
> >> +                  " -device ioh3420,id=rp,bus=pxb,slot=1"
> >> +                  " -object memory-backend-file,size=1G,mem-path=/tmp/shmem,share,id=mb"
> >> +                  " -device ivshmem-plain,memdev=mb,bus=rp",
> >> +                  &data);
> >> +    free_test_data(&data);
> >> +}
> >> +
> >>   int main(int argc, char *argv[])
> >>   {
> >>       const char *arch = qtest_get_arch();
> >> @@ -884,6 +919,8 @@ int main(int argc, char *argv[])
> >>           qtest_add_func("acpi/q35/tcg/ipmi", test_acpi_q35_tcg_ipmi);
> >>           qtest_add_func("acpi/piix4/tcg/cpuhp", test_acpi_piix4_tcg_cphp);
> >>           qtest_add_func("acpi/q35/tcg/cpuhp", test_acpi_q35_tcg_cphp);
> >> +        qtest_add_func("acpi/piix4/tcg/pxb", test_acpi_piix4_tcg_pxb);
> >> +        qtest_add_func("acpi/q35/tcg/pxb-pcie", test_acpi_q35_tcg_pxb_pcie);
> >>       }
> >>       ret = g_test_run();
> >>       boot_sector_cleanup(disk);  
> >  
> 
> 

Re: [Qemu-devel] [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests

[Laszlo Ersek]

On 07/18/16 15:34, Igor Mammedov wrote:
> On Sun, 17 Jul 2016 19:53:09 +0300
> Marcel Apfelbaum <marcel@redhat.com> wrote:
> 
>> Add an ivshmem device with 1G shared memory to
>> pxb in order to check the ACPI code of 64bit MMIO allocation.
> what is forcing ivshmem to be mapped above 4G?

Speaking for OVMF: unlike SeaBIOS, the edk2 PCI bus driver prefers to
map 64-bit BARs outside of 32-bit address space, regardless of how much
room is left in the 32-bit MMIO aperture.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests

[Marcel Apfelbaum]

On 07/18/2016 05:54 PM, Igor Mammedov wrote:
> On Mon, 18 Jul 2016 17:17:48 +0300
> Marcel Apfelbaum <marcel@redhat.com> wrote:
>
>> On 07/18/2016 04:34 PM, Igor Mammedov wrote:
>>> On Sun, 17 Jul 2016 19:53:09 +0300
>>> Marcel Apfelbaum <marcel@redhat.com> wrote:
>>>
>>>> Add an ivshmem device with 1G shared memory to
>>>> pxb in order to check the ACPI code of 64bit MMIO allocation.
>>> what is forcing ivshmem to be mapped above 4G?
>>
>> For PC machine: nothing -> ivshmem gets mapped in 32-bit area
>> For Q35 machine: there is not enough space for mapping 1G
>> in 32-bit area, and maybe there are also alignment constrains.
>> You can use this test in verbose mode and see exactly how
>> the ranges are distributed. If you want me to run it and add the output,
>> please let me know.
> No need to post diff, I can do V=1 manually,
> the question though is: doesn't PC need ivshmem to go above 4G as well?

We have ivshmem in the PC, but if doesn't go over 4G.
I left it that way in order to show what happens in both cases. (32-bit and 64-bit)
Since is the same code for PC/Q35 it should be enough.

Thanks,
Marcel

>
>>
>> Thanks,
>> Marcel
>>
>>>
>>>>
>>>> Suggested-by: Igor Mammedov <imammedo@redhat.com>
>>>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>>>> Tested-by: Laszlo Ersek <lersek@redhat.com>
>>>> ---
>>>>    tests/bios-tables-test.c | 37 +++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 37 insertions(+)
>>>>
>>>> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
>>>> index de4019e..b23c6b0 100644
>>>> --- a/tests/bios-tables-test.c
>>>> +++ b/tests/bios-tables-test.c
>>>> @@ -864,6 +864,41 @@ static void test_acpi_piix4_tcg_ipmi(void)
>>>>        free_test_data(&data);
>>>>    }
>>>>
>>>> +static void test_acpi_piix4_tcg_pxb(void)
>>>> +{
>>>> +    test_data data;
>>>> +
>>>> +    memset(&data, 0, sizeof(data));
>>>> +    data.machine = MACHINE_PC;
>>>> +    data.variant = ".pxb";
>>>> +    data.required_struct_types = base_required_struct_types;
>>>> +    data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
>>>> +    test_acpi_one("-machine accel=tcg"
>>>> +                  " -device pxb,id=pxb,bus_nr=0x80,bus=pci.0"
>>>> +                  " -object memory-backend-file,size=1G,mem-path=/tmp/shmem,share,id=mb"
>>>> +                  " -device ivshmem-plain,memdev=mb,bus=pxb",
>>>> +                  &data);
>>>> +    free_test_data(&data);
>>>> +}
>>>> +
>>>> +static void test_acpi_q35_tcg_pxb_pcie(void)
>>>> +{
>>>> +    test_data data;
>>>> +
>>>> +    memset(&data, 0, sizeof(data));
>>>> +    data.machine = MACHINE_Q35;
>>>> +    data.variant = ".pxb_pcie";
>>>> +    data.required_struct_types = base_required_struct_types;
>>>> +    data.required_struct_types_len = ARRAY_SIZE(ipmi_required_struct_types);
>>>> +    test_acpi_one("-machine q35,accel=tcg"
>>>> +                  " -device pxb-pcie,id=pxb,bus_nr=0x80,bus=pcie.0"
>>>> +                  " -device ioh3420,id=rp,bus=pxb,slot=1"
>>>> +                  " -object memory-backend-file,size=1G,mem-path=/tmp/shmem,share,id=mb"
>>>> +                  " -device ivshmem-plain,memdev=mb,bus=rp",
>>>> +                  &data);
>>>> +    free_test_data(&data);
>>>> +}
>>>> +
>>>>    int main(int argc, char *argv[])
>>>>    {
>>>>        const char *arch = qtest_get_arch();
>>>> @@ -884,6 +919,8 @@ int main(int argc, char *argv[])
>>>>            qtest_add_func("acpi/q35/tcg/ipmi", test_acpi_q35_tcg_ipmi);
>>>>            qtest_add_func("acpi/piix4/tcg/cpuhp", test_acpi_piix4_tcg_cphp);
>>>>            qtest_add_func("acpi/q35/tcg/cpuhp", test_acpi_q35_tcg_cphp);
>>>> +        qtest_add_func("acpi/piix4/tcg/pxb", test_acpi_piix4_tcg_pxb);
>>>> +        qtest_add_func("acpi/q35/tcg/pxb-pcie", test_acpi_q35_tcg_pxb_pcie);
>>>>        }
>>>>        ret = g_test_run();
>>>>        boot_sector_cleanup(disk);
>>>
>>
>>
>

Re: [Qemu-devel] [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests

[Marcel Apfelbaum]

On 07/18/2016 08:52 PM, Laszlo Ersek wrote:
> On 07/18/16 15:34, Igor Mammedov wrote:
>> On Sun, 17 Jul 2016 19:53:09 +0300
>> Marcel Apfelbaum <marcel@redhat.com> wrote:
>>
>>> Add an ivshmem device with 1G shared memory to
>>> pxb in order to check the ACPI code of 64bit MMIO allocation.
>> what is forcing ivshmem to be mapped above 4G?
>
> Speaking for OVMF: unlike SeaBIOS, the edk2 PCI bus driver prefers to
> map 64-bit BARs outside of 32-bit address space, regardless of how much
> room is left in the 32-bit MMIO aperture.

Thanks Laszlo, we need to support make check with ovmf (not immediate, of course).
There is no integration yet with QEMU, right? Having a set of binaries like with SeaBIOS
makes sense? Are there any licensing issues?

Thanks,
Marcel

>
> Thanks
> Laszlo
>

Re: [Qemu-devel] [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests

[Laszlo Ersek]

Adding Gerd

On 07/18/16 21:32, Marcel Apfelbaum wrote:
> On 07/18/2016 08:52 PM, Laszlo Ersek wrote:
>> On 07/18/16 15:34, Igor Mammedov wrote:
>>> On Sun, 17 Jul 2016 19:53:09 +0300
>>> Marcel Apfelbaum <marcel@redhat.com> wrote:
>>>
>>>> Add an ivshmem device with 1G shared memory to
>>>> pxb in order to check the ACPI code of 64bit MMIO allocation.
>>> what is forcing ivshmem to be mapped above 4G?
>>
>> Speaking for OVMF: unlike SeaBIOS, the edk2 PCI bus driver prefers to
>> map 64-bit BARs outside of 32-bit address space, regardless of how much
>> room is left in the 32-bit MMIO aperture.
> 
> Thanks Laszlo, we need to support make check with ovmf (not immediate,
> of course).
> There is no integration yet with QEMU, right? Having a set of binaries
> like with SeaBIOS
> makes sense?

Yes, it makes sense.

> Are there any licensing issues?

If QEMU can bundle 2-clause BSDL binaries, then there shouldn't be.

For the legally inclined, it might be safest to review the
*Pkg/License.txt files, for all the *Pkg top-level directories from
which OVMF pulls in at least one module.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH V5 0/7] pxb: fix 64-bit MMIO allocation

[Marcel Apfelbaum]

On 07/17/2016 07:53 PM, Marcel Apfelbaum wrote:
> v4 -> v5:
>    Addressed the pull request issues: (Peter Maydell)
>    See: https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg00882.html
>    - cland warning -> "hw/pci/pci.c:196:23: runtime error: shift exponent -1 is negative":
>      The PCIe Root port was not initialized properly, the interrupt pin was left 0. This
>      is a long standing issue exposed by the new test. (Patch 1/7)
>    - 'make check' fails on 32-bit:
>       Fix it by changing the ivshmem mem size from 4G
>       to 1G, since 4G is not a valid value on 32-bit archs. (Patch 2/7)
>       (4G is truncated to 0 on 32-bit systems)
>    - Rebased on mst's pci branch.
>    Since all the new changes are not related to the series, I kept the existing
>    "Reviewed-by"/"Tested-by" signatures.
>


Hi Michael,

Can you please add this series to your pull request for 2.7 ?
It is an important fix already reviewed and tested.

The only problems with the series were a test issue and another
long-standing bug exposed by the test and both were fixed.

Thanks,
Marcel

> v3 -> v4:
>   Addressed Igor's comments (thanks for the productive review!)
>   - Split pxb test patch (previously patch 3/3) into the test itself (patch 1/6) and the blobs (patch 6/6).
>   - New patch declaring pxb/pxb-pxie as not hot-pluggable.
>      - Note that it does not solve the DSDT issue, but it is a prerequisite for the next patch.
>   - New patch solving the DSDT issue spotted by Igor.
>       - Using V=1 DIFF=diff make check does make it easier to review the ACPI changes, thanks.
>   - Patches 4 and 5 untouched (previously patches 1/3 and 2/3)
>
> v2 -> v3:
>   - split original series "pci: better support for 64-bit MMIO allocation" into 2 series:
>      - this is the first part dealing with correct 64-bit MMIO ACPI computation
>      - the second one will include 64-bit MMIO reservation for PCI hotplug
>   - Add pxb/pxb-pcie tests (Igor) - See diffs below (*)
>   - Re-based on latest master.
>
> v1 -> v2:
>   - resolved some styling issues (Laszlo)
>   - rebase on latest master (Laszlo)
>
>
>
> 64-bit BARs allocations fix for devices behind PXBs/PXB-PCIEs.
>
> In build_crs() the calculation and merging of the ranges already happens
> in 64-bit, but the entry boundaries are silently truncated to 32-bit in the
> call to aml_dword_memory(). Fix it by handling the 64-bit MMIO ranges separately.
>
>
> Thank you,
> Marcel
>
> Marcel Apfelbaum (7):
>    hw/pcie-root-port: Fix PCIe root port initialization
>    tests/acpi: add pxb/pxb-pcie tests
>    hw/pxb: declare pxb devices as not hot-pluggable
>    hw/acpi: fix a DSDT table issue when a pxb is present.
>    acpi: refactor pxb crs computation
>    hw/apci: handle 64-bit MMIO regions correctly
>    tests/acpi: Add pxb/pxb-pcie tests blobs
>
>   hw/i386/acpi-build.c                   | 131 ++++++++++++++++++++++++---------
>   hw/pci-bridge/ioh3420.c                |   1 +
>   hw/pci-bridge/pci_expander_bridge.c    |   2 +
>   tests/acpi-test-data/pc/DSDT.pxb       | Bin 0 -> 6286 bytes
>   tests/acpi-test-data/q35/DSDT.pxb_pcie | Bin 0 -> 9098 bytes
>   tests/bios-tables-test.c               |  37 ++++++++++
>   6 files changed, 135 insertions(+), 36 deletions(-)
>   create mode 100644 tests/acpi-test-data/pc/DSDT.pxb
>   create mode 100644 tests/acpi-test-data/q35/DSDT.pxb_pcie
>

Re: [Qemu-devel] [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests

[Igor Mammedov]

On Sun, 17 Jul 2016 19:53:09 +0300
Marcel Apfelbaum <marcel@redhat.com> wrote:

> Add an ivshmem device with 1G shared memory to
> pxb in order to check the ACPI code of 64bit MMIO allocation.
> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  tests/bios-tables-test.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index de4019e..b23c6b0 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -864,6 +864,41 @@ static void test_acpi_piix4_tcg_ipmi(void)
>      free_test_data(&data);
>  }
>  
> +static void test_acpi_piix4_tcg_pxb(void)
> +{
> +    test_data data;
> +
> +    memset(&data, 0, sizeof(data));
> +    data.machine = MACHINE_PC;
> +    data.variant = ".pxb";
> +    data.required_struct_types = base_required_struct_types;
> +    data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
> +    test_acpi_one("-machine accel=tcg"
> +                  " -device pxb,id=pxb,bus_nr=0x80,bus=pci.0"
> +                  " -object memory-backend-file,size=1G,mem-path=/tmp/shmem,share,id=mb"
> +                  " -device ivshmem-plain,memdev=mb,bus=pxb",
> +                  &data);
> +    free_test_data(&data);
> +}
> +
> +static void test_acpi_q35_tcg_pxb_pcie(void)
> +{
> +    test_data data;
> +
> +    memset(&data, 0, sizeof(data));
> +    data.machine = MACHINE_Q35;
> +    data.variant = ".pxb_pcie";
> +    data.required_struct_types = base_required_struct_types;
> +    data.required_struct_types_len = ARRAY_SIZE(ipmi_required_struct_types);
> +    test_acpi_one("-machine q35,accel=tcg"
> +                  " -device pxb-pcie,id=pxb,bus_nr=0x80,bus=pcie.0"
> +                  " -device ioh3420,id=rp,bus=pxb,slot=1"
> +                  " -object memory-backend-file,size=1G,mem-path=/tmp/shmem,share,id=mb"
> +                  " -device ivshmem-plain,memdev=mb,bus=rp",
> +                  &data);
> +    free_test_data(&data);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>      const char *arch = qtest_get_arch();
> @@ -884,6 +919,8 @@ int main(int argc, char *argv[])
>          qtest_add_func("acpi/q35/tcg/ipmi", test_acpi_q35_tcg_ipmi);
>          qtest_add_func("acpi/piix4/tcg/cpuhp", test_acpi_piix4_tcg_cphp);
>          qtest_add_func("acpi/q35/tcg/cpuhp", test_acpi_q35_tcg_cphp);
> +        qtest_add_func("acpi/piix4/tcg/pxb", test_acpi_piix4_tcg_pxb);
> +        qtest_add_func("acpi/q35/tcg/pxb-pcie", test_acpi_q35_tcg_pxb_pcie);
>      }
>      ret = g_test_run();
>      boot_sector_cleanup(disk);

Re: [Qemu-devel] [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests

[Marcel Apfelbaum]

On 07/19/2016 10:34 AM, Igor Mammedov wrote:
> On Sun, 17 Jul 2016 19:53:09 +0300
> Marcel Apfelbaum <marcel@redhat.com> wrote:
>
>> Add an ivshmem device with 1G shared memory to
>> pxb in order to check the ACPI code of 64bit MMIO allocation.
>>
>> Suggested-by: Igor Mammedov <imammedo@redhat.com>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>> Tested-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>


Thanks Igor!
Marcel

>
>> ---
>>   tests/bios-tables-test.c | 37 +++++++++++++++++++++++++++++++++++++
>>   1 file changed, 37 insertions(+)
>>
>> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
>> index de4019e..b23c6b0 100644
>> --- a/tests/bios-tables-test.c
>> +++ b/tests/bios-tables-test.c
>> @@ -864,6 +864,41 @@ static void test_acpi_piix4_tcg_ipmi(void)
>>       free_test_data(&data);
>>   }
>>
>> +static void test_acpi_piix4_tcg_pxb(void)
>> +{
>> +    test_data data;
>> +
>> +    memset(&data, 0, sizeof(data));
>> +    data.machine = MACHINE_PC;
>> +    data.variant = ".pxb";
>> +    data.required_struct_types = base_required_struct_types;
>> +    data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
>> +    test_acpi_one("-machine accel=tcg"
>> +                  " -device pxb,id=pxb,bus_nr=0x80,bus=pci.0"
>> +                  " -object memory-backend-file,size=1G,mem-path=/tmp/shmem,share,id=mb"
>> +                  " -device ivshmem-plain,memdev=mb,bus=pxb",
>> +                  &data);
>> +    free_test_data(&data);
>> +}
>> +
>> +static void test_acpi_q35_tcg_pxb_pcie(void)
>> +{
>> +    test_data data;
>> +
>> +    memset(&data, 0, sizeof(data));
>> +    data.machine = MACHINE_Q35;
>> +    data.variant = ".pxb_pcie";
>> +    data.required_struct_types = base_required_struct_types;
>> +    data.required_struct_types_len = ARRAY_SIZE(ipmi_required_struct_types);
>> +    test_acpi_one("-machine q35,accel=tcg"
>> +                  " -device pxb-pcie,id=pxb,bus_nr=0x80,bus=pcie.0"
>> +                  " -device ioh3420,id=rp,bus=pxb,slot=1"
>> +                  " -object memory-backend-file,size=1G,mem-path=/tmp/shmem,share,id=mb"
>> +                  " -device ivshmem-plain,memdev=mb,bus=rp",
>> +                  &data);
>> +    free_test_data(&data);
>> +}
>> +
>>   int main(int argc, char *argv[])
>>   {
>>       const char *arch = qtest_get_arch();
>> @@ -884,6 +919,8 @@ int main(int argc, char *argv[])
>>           qtest_add_func("acpi/q35/tcg/ipmi", test_acpi_q35_tcg_ipmi);
>>           qtest_add_func("acpi/piix4/tcg/cpuhp", test_acpi_piix4_tcg_cphp);
>>           qtest_add_func("acpi/q35/tcg/cpuhp", test_acpi_q35_tcg_cphp);
>> +        qtest_add_func("acpi/piix4/tcg/pxb", test_acpi_piix4_tcg_pxb);
>> +        qtest_add_func("acpi/q35/tcg/pxb-pcie", test_acpi_q35_tcg_pxb_pcie);
>>       }
>>       ret = g_test_run();
>>       boot_sector_cleanup(disk);
>

[Qemu-devel] edk2 submodule + binaries (Re: [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests)

[Gerd Hoffmann]

  Hi,

> > There is no integration yet with QEMU, right? Having a set of binaries
> > like with SeaBIOS
> > makes sense?
> 
> Yes, it makes sense.

Indeed.  edk2 isn't exactly small though.  Adding the submodule would
make the release tarballs noticable larger, and the prebuild binaries
are not exactly small too ...

I remember we discussed moving all the pre-built binaries to a separate
repo a while back.  Maybe we should think about that idea again?

> > Are there any licensing issues?
> 
> If QEMU can bundle 2-clause BSDL binaries, then there shouldn't be.

I think with the FatPkg issue finally solved now there shouldn't be
fundamental roadblocks any more.

Next question would be which architectures we want build edk2 for?

 (1) x64 ovmf is clear.

 (2) ia32 ovmf too?  Will anybody use it?

 (3) ArmVirtPkg for aarch64 should be built IMO.

 (4) What about ArmVirtPkg for 32bit arm?

 (5) There is ArmPlatformPkg/ArmVExpressPkg.
     Anyone tried that with qemu-system-{arm,aarch64} -M vexpress-* ?

While being at it:  Is there any way to setup a 32bit arm guest with a
bootloader, so you don't have to somehow copy the guest kernel from the
image to boot?

At least the upstream kernel doesn't support acpi @ 32bit arm, so the
guest kernel can't find and use the hardware info provided by
ArmVirtPkg.  Instead it depends on the bootloader passing a fdt.  But
the bootloaders (grub+uboot) expect a file with the fdt somewhere.
There isn't a file in the first place for -M virt.  There is one for the
vexpress boards, which actually works (for v9).  But it lists only the
hardware physical vexpress boards have, any virtio-mmio devices you add
are not listed there.  So sdcard storage is the only option.  Hmm.

Comments?

cheers,
  Gerd

Re: [Qemu-devel] edk2 submodule + binaries (Re: [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests)

[Peter Maydell]

On 19 July 2016 at 10:06, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Next question would be which architectures we want build edk2 for?
>
>  (1) x64 ovmf is clear.
>
>  (2) ia32 ovmf too?  Will anybody use it?
>
>  (3) ArmVirtPkg for aarch64 should be built IMO.
>
>  (4) What about ArmVirtPkg for 32bit arm?

I think this would be a good idea.

>  (5) There is ArmPlatformPkg/ArmVExpressPkg.
>      Anyone tried that with qemu-system-{arm,aarch64} -M vexpress-* ?
>
> While being at it:  Is there any way to setup a 32bit arm guest with a
> bootloader, so you don't have to somehow copy the guest kernel from the
> image to boot?
>
> At least the upstream kernel doesn't support acpi @ 32bit arm, so the
> guest kernel can't find and use the hardware info provided by
> ArmVirtPkg.  Instead it depends on the bootloader passing a fdt.  But
> the bootloaders (grub+uboot) expect a file with the fdt somewhere.
> There isn't a file in the first place for -M virt.  There is one for the
> vexpress boards, which actually works (for v9).  But it lists only the
> hardware physical vexpress boards have, any virtio-mmio devices you add
> are not listed there.  So sdcard storage is the only option.  Hmm.

We should be aiming to get 'virt' to work for the 32-bit case.
vexpress-a9/a15 is trying to model real hardware and has a
lot of irritating constraints as a result (like no PCI, only
SD card storage).

This probably means sorting out passing through the DTB
from QEMU into UEFI and then into the boot loader.

thanks
-- PMM

Re: [Qemu-devel] edk2 submodule + binaries (Re: [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests)

[Laszlo Ersek]

On 07/19/16 11:06, Gerd Hoffmann wrote:
>   Hi,
> 
>>> There is no integration yet with QEMU, right? Having a set of binaries
>>> like with SeaBIOS
>>> makes sense?
>>
>> Yes, it makes sense.
> 
> Indeed.  edk2 isn't exactly small though.  Adding the submodule would
> make the release tarballs noticable larger, and the prebuild binaries
> are not exactly small too ...
> 
> I remember we discussed moving all the pre-built binaries to a separate
> repo a while back.  Maybe we should think about that idea again?
> 
>>> Are there any licensing issues?
>>
>> If QEMU can bundle 2-clause BSDL binaries, then there shouldn't be.
> 
> I think with the FatPkg issue finally solved now there shouldn't be
> fundamental roadblocks any more.
> 
> Next question would be which architectures we want build edk2 for?
> 
>  (1) x64 ovmf is clear.
> 
>  (2) ia32 ovmf too?  Will anybody use it?

There are three bitness-combinations:
- Ia32: both PEI and DXE phases are 32-bit
- Ia32X64: PEI is 32-bit, DXE is 64-bit
- X64: both PEI and DXE are 64-bit

The bitness of the DXE phase must match the bitness of the operatig
system. So Ia32 can only boot 32-bit OSes (*), and the other two can
only boot 64-bit OSes.

(*) This is a bit relaxed for Linux guests, because Linux supports being
booted on a 32-bit DXE phase firmware, and then switches to 64-bit
itself. IIRC the idea here was to enable 64-bit Linux to run on tablets
and similar hardware that has a 64-bit CPU, but only comes with a fully
32-bit firmware (see
<https://blogs.intel.com/evangelists/2015/07/22/why-cheap-systems-run-32-bit-uefi-on-x64-systems/>).
But, without such OS-level tricks, in general Ia32 DXE implies 32-bit OS.

Then the next question is, what's the status of 32-bit UEFI OSes? Simple:

- 32-bit UEFI Windows exists, but it doesn't boot on OVMF. I don't know
why. I've spend a lot of time debugging it, on and off, with the
(sporadic) help of a Microsoft developer. We narrowed it down somewhat,
but it remains unsolved.

- 32-bit Linux may exist, but the distros I tried are either not
official (see Fedlet
<https://www.happyassassin.net/fedlet-a-fedora-remix-for-bay-trail-tablets/>)
or I didn't get any usable results with them (IIRC I tried 32-bit UEFI
Debain once before, and it didn't work at all; but I'm fuzzy on that,
sorry).

Matthew Garrett also advises against (fully) 32-bit firmware:
<https://mjg59.dreamwidth.org/26734.html>

The distinction between Ia32X64 and X64 is less clear cut. The
difference between them is inivisible to the user (the bitness of the
PEI phase is not exposed to the OS), but in practice there is one
visible difference: the combination

  SMM driver stack + X64 PEI + S3 resume enabled on QEMU

is not supported by edk2 at the moment. So if the binary should support
SMM + S3, and allow the user not to bother about disabling S3 on the
QEMU command line, then Ia32X64 is the better choice.

Furthermore, the SMM driver stack limits the firmware to pc-q35-2.5+
machine types (no i440fx at all, and no q35 earlier than 2.5). This
might not be useful for everyone, I'm not sure.

CSM should never be enabled (SeaBIOS is bundled with QEMU already).

Enabling Secure Boot in the OVMF binary is orthogonal to all of the
above, but it has a licensing impact. It embeds (a subset of) OpenSSL in
the binary, and changes the terms from "2-clause BSDL" to "2-clause BSDL
and OpenSSL license" ("and" in the restrictive, not permissive, sense).
I'm unsure if QEMU is willing and able to distribute such binaries.

For the widest and simplest usability, X64 (without the SMM driver stack
and without Secure Boot) is likely the best.

>  (3) ArmVirtPkg for aarch64 should be built IMO.

Yup.

>  (4) What about ArmVirtPkg for 32bit arm?

It is a supported configuration in edk2, but I don't know how useful it
would be. We should ask Ard I guess.

>  (5) There is ArmPlatformPkg/ArmVExpressPkg.
>      Anyone tried that with qemu-system-{arm,aarch64} -M vexpress-* ?

Historically I may have used it I think, but that was before the arrival
of ArmVirtPkg (nee ArmPlatformPkg/ArmVirtualizationPkg) for the "virt"
machine type.

> While being at it:  Is there any way to setup a 32bit arm guest with a
> bootloader, so you don't have to somehow copy the guest kernel from the
> image to boot?

I'm unaware of any limitation in ArmVirtPkg or edk2 that would prevent
this; I just think OS bother to support 32-bit UEFI even less on ARM
than they do on x86.

> At least the upstream kernel doesn't support acpi @ 32bit arm, so the
> guest kernel can't find and use the hardware info provided by
> ArmVirtPkg.  Instead it depends on the bootloader passing a fdt.  But
> the bootloaders (grub+uboot) expect a file with the fdt somewhere.

ArmVirtPkg gets a generated DTB from QEMU at the base of system memory,
stashes it safely, uses it, and (unless disabled explicitly by a node in
the DTB) exposes it to the UEFI OS through a dedicated UEFI system
configuration table. Linux is capable of using this sysconfig table
already, so those boot loaders should be taught the same, assuming
32-bit UEFI ARM is so important.

QEMU                   ArmVirtQemu
 | ----- DT via RAM ------> |
 | --- ACPI via fw_cfg ---> |
 | -- SMBIOS via fw_cfg --> |                               guest kernel
                            | ---- DT via UEFI config table ----> |
                            | --- ACPI via UEFI config table ---> |
                            | -- SMBIOS via UEFI config table --> |

(According to my last information, the upstream kernel still prefers DT
over ACPI. I'm aware of two methods to talk it out of that:
- "acpi=force" on the guest kernel cmdline (dynamically),
- the "-D PURE_ACPI_BOOT_ENABLE" ArmVirtQemu build option
<https://github.com/tianocore/edk2/commit/b359fb9115b2>, which
statically removes the right hand side "DT" edge from the above diagram.)

Anyway, I digress. Point is, GRUB is already UEFI capable (I don't know
uboot), so GRUB should be able to look up the DTB sysconfig table, and
use that. The sysconfig table in question is identified by the GUID

  B1B621D5-F19C-41A5-830B-D9152C69AAE0

(or written in C:
<https://github.com/tianocore/edk2/blob/master/EmbeddedPkg/Include/Guid/Fdt.h>.)

> There isn't a file in the first place for -M virt.

It's not necessary, see above. If you would like to investigate the DTB
that QEMU generates, you can use:

  $ qemu-system-(arm|aarch64) \
      -nographic -machine virt,dumpdtb=virt-dump.dtb
  $ dtc -I dtb -O dts <virt-dump.dtb | less

but that's not the right way to carry the DTB from QEMU to guest code
(at least on the "virt" machine type).

> There is one for the
> vexpress boards, which actually works (for v9).  But it lists only the
> hardware physical vexpress boards have, any virtio-mmio devices you add
> are not listed there.

Right, the generated DTB lists all the virtio-mmio transports.

>  So sdcard storage is the only option.  Hmm.

Not necessarily; it can be considered a UEFI GRUB enhancement IMO.

Thanks,
Laszlo

Re: [Qemu-devel] edk2 submodule + binaries (Re: [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests)

[Gerd Hoffmann]

  Hi,

> >  (4) What about ArmVirtPkg for 32bit arm?
> 
> I think this would be a good idea.

> We should be aiming to get 'virt' to work for the 32-bit case.
> vexpress-a9/a15 is trying to model real hardware and has a
> lot of irritating constraints as a result (like no PCI, only
> SD card storage).
> 
> This probably means sorting out passing through the DTB
> from QEMU into UEFI and then into the boot loader.

Looking at aarch64 it looks like the guest kernel doesn't use acpi but
got a fdt somehow.  Dunno how that happened.  I guess ArmVirtPkg exports
the FDT using EFI interfaces, then either the kernel gets it directly or
grub is able to get and pass on the FDT.  In any case it seems think the
same should work for 32bit without too much trouble.

cheers,
  Gerd

Re: [Qemu-devel] edk2 submodule + binaries (Re: [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests)

[Laszlo Ersek]

thinko:

On 07/19/16 11:59, Laszlo Ersek wrote:

> ArmVirtPkg gets a generated DTB from QEMU at the base of system memory,
> stashes it safely, uses it, and (unless disabled explicitly by a node in
> the DTB) exposes it to the UEFI OS through a dedicated UEFI system
> configuration table.

replace

  unless disabled explicitly by a node in the DTB

with

  unless disabled explicitly with "-D PURE_ACPI_BOOT_ENABLE", mentioned
  elsewhere in the message

Thanks
Laszlo

Re: [Qemu-devel] edk2 submodule + binaries (Re: [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests)

[Laszlo Ersek]

On 07/19/16 12:05, Gerd Hoffmann wrote:
>   Hi,
> 
>>>  (4) What about ArmVirtPkg for 32bit arm?
>>
>> I think this would be a good idea.
> 
>> We should be aiming to get 'virt' to work for the 32-bit case.
>> vexpress-a9/a15 is trying to model real hardware and has a
>> lot of irritating constraints as a result (like no PCI, only
>> SD card storage).
>>
>> This probably means sorting out passing through the DTB
>> from QEMU into UEFI and then into the boot loader.

Yes, the piece that is missing is that the boot loader should be capable
of looking up the UEFI sysconfig table with the well-known GUID that
exposes the DTB.

(NB: such a sysconfig table is not *required* by any means, especially
not on aarch64. It is valid for the firmware to not expose any DTB to
stuff that comes after it, in an "ACPI only" configuration.)

... Actually, I'm getting confused about this. I just suggested that the
boot loader look up the right UEFI sysconfig table. But, if the boot
loader is already UEFI capable, why does it need the DTB at all? Gerd
named the virtio-mmio nodes in the DTB, but now I don't see how they are
relevant -- the boot loader should just use the UEFI services to load
and start the OS. Why does it need to know about virtio-mmio at all?

> Looking at aarch64 it looks like the guest kernel doesn't use acpi but
> got a fdt somehow.  Dunno how that happened.

The aarch64 upstream kernel (to my knowledge) defaults to DTB, unless it
gets "acpi=force" on the command line, or ArmVirtQemu is built with -D
PURE_ACPI_BOOT_ENABLE" (in which case the kernel will have no DTB to
look at, only ACPI).

>  I guess ArmVirtPkg exports
> the FDT using EFI interfaces,

Yes (unless built with "-D PURE_ACPI_BOOT_ENABLE").

> then either the kernel gets it directly

The DTB is installed as a UEFI sysconfig table with a well-known GUID.
All guest code that can look up UEFI sysconfig tables can locate and
read it.

> or
> grub is able to get and pass on the FDT.  In any case it seems think the
> same should work for 32bit without too much trouble.

It should be possible, but as I said, now I'm doubting that grub should
be involved in this at all.

First, the DTB reaches the kernel, from the firmware, without any
assitance from grub, through the sysconfig table.

Second, grub should not need the DTB for its own purposes *at all* -- a
boot loader on a UEFI system should use UEFI services only, for booting
the OS. The DTB might be necessary for low-level hardware drivers, I
guess, but in a UEFI guest, grub should not use low-level hw drivers.

... I mean, this is how grub already works in x86_64 and aarch64 UEFI
guests. I think we're sitting on the horse backwards. DTB has probably
been a "last resort" for 32-bit ARM GRUB, because there used to be no
UEFI, hence no UEFI services, hence only the DTB allowed GRUB to do
something with the hardware

But now that there *is* UEFI for 32-bit ARM (guests, anyway), the
bootloader should not use UEFI only as a means to fetch the DTB, and
then do whatever it's been doing earlier. Instead, UEFI should be used
to boot the OS.

Thanks
Laszlo

Re: [Qemu-devel] edk2 submodule + binaries (Re: [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests)

[Gerd Hoffmann]

  Hi,

> >  (2) ia32 ovmf too?  Will anybody use it?
> 
> Then the next question is, what's the status of 32-bit UEFI OSes? Simple:

> [ summary: bad ]

Yep, that matches the impression I have.
Guess we don't want bother then.

> Enabling Secure Boot in the OVMF binary is orthogonal to all of the
> above, but it has a licensing impact. It embeds (a subset of) OpenSSL in
> the binary, and changes the terms from "2-clause BSDL" to "2-clause BSDL
> and OpenSSL license" ("and" in the restrictive, not permissive, sense).
> I'm unsure if QEMU is willing and able to distribute such binaries.
> 
> For the widest and simplest usability, X64 (without the SMM driver stack
> and without Secure Boot) is likely the best.

Yes (also note the smm-enabled one doesn't run on i440fx).

So the options I see are (a) build without smm or (b) build two
variants.

> Anyway, I digress. Point is, GRUB is already UEFI capable (I don't know
> uboot), so GRUB should be able to look up the DTB sysconfig table, and
> use that. The sysconfig table in question is identified by the GUID
> 
>   B1B621D5-F19C-41A5-830B-D9152C69AAE0

grub already does that on aarch64, but not on arm.
So that should be fixable without too much effort.

u-boot is out for -M virt due to missing virtio drivers.

>   $ qemu-system-(arm|aarch64) \
>       -nographic -machine virt,dumpdtb=virt-dump.dtb
>   $ dtc -I dtb -O dts <virt-dump.dtb | less
> 
> but that's not the right way to carry the DTB from QEMU to guest code

Exactly.  That just creates a reverse problem.  Instead of copying the
kernel from the image you now have to copy the dtb file to the image.

> > There is one for the
> > vexpress boards, which actually works (for v9).  But it lists only the
> > hardware physical vexpress boards have, any virtio-mmio devices you add
> > are not listed there.
> 
> Right, the generated DTB lists all the virtio-mmio transports.

But u-boot looks for dtb files in /boot/dtbs/$kernelver/ instead of
using the one provided by qemu.  So virtio isn't there (virtio-mmio @
vexpress works for a direct kernel boot).

cheers,
  Gerd

Re: [Qemu-devel] edk2 submodule + binaries (Re: [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests)

[Laszlo Ersek]

On 07/19/16 12:48, Gerd Hoffmann wrote:
>   Hi,
> 
>>>  (2) ia32 ovmf too?  Will anybody use it?
>>
>> Then the next question is, what's the status of 32-bit UEFI OSes? Simple:
> 
>> [ summary: bad ]
> 
> Yep, that matches the impression I have.
> Guess we don't want bother then.
> 
>> Enabling Secure Boot in the OVMF binary is orthogonal to all of the
>> above, but it has a licensing impact. It embeds (a subset of) OpenSSL in
>> the binary, and changes the terms from "2-clause BSDL" to "2-clause BSDL
>> and OpenSSL license" ("and" in the restrictive, not permissive, sense).
>> I'm unsure if QEMU is willing and able to distribute such binaries.
>>
>> For the widest and simplest usability, X64 (without the SMM driver stack
>> and without Secure Boot) is likely the best.
> 
> Yes (also note the smm-enabled one doesn't run on i440fx).
> 
> So the options I see are (a) build without smm or (b) build two
> variants.

I think people who just want to run "UEFI payloads" are served well
enough by SMM-less. There are some developers who would benefit from a
-D SMM_REQUIRE build as well, but that would be because they actually
focus on SMM, I believe, so they can build their own firmware.

If this is about the convenience of QEMU end-users, then I vote (a). I'd
certainly like to avoid fielding bug reports that boil down to "I booted
the -D SMM_REQUIRE build on i440fx".

>> Anyway, I digress. Point is, GRUB is already UEFI capable (I don't know
>> uboot), so GRUB should be able to look up the DTB sysconfig table, and
>> use that. The sysconfig table in question is identified by the GUID
>>
>>   B1B621D5-F19C-41A5-830B-D9152C69AAE0
> 
> grub already does that on aarch64,

Huh, indeed. I see "grub-core/loader/arm64/fdt.c". What does UEFI grub
need the DTB for in the first place?

Hm... looks like it pokes initrd information into it. Interesting.

> but not on arm.
> So that should be fixable without too much effort.

I guess so.

I'll mention though that "just" for passing in the initrd, the DTB
shouldn't be necessary, at least if the kernel is built with the EFI
stub. Then "initrd=filename" can be passed on the kernel command line,
and the EFI stub should load it, using UEFI services, from the same
directory that the vmlinuz binary (= itself) came from.

... I can never keep the 37 ways to boot the linux kernel in my mind for
longer than 2 minutes. :/

Thanks
Laszlo

Re: [Qemu-devel] edk2 submodule + binaries (Re: [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests)

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 560 bytes --]

  Hi,

> > but not on arm.
> > So that should be fixable without too much effort.
> 
> I guess so.
> 
> I'll mention though that "just" for passing in the initrd, the DTB
> shouldn't be necessary, at least if the kernel is built with the EFI
> stub. Then "initrd=filename" can be passed on the kernel command line,
> and the EFI stub should load it, using UEFI services, from the same
> directory that the vmlinuz binary (= itself) came from.

Well, the kernel command line is passed via fdt too ...

Anyway, it's working.

cheers,
  Gerd


[-- Attachment #2: 0001-arm-lookup-devicetree-via-efi.patch --]
[-- Type: text/x-patch, Size: 2324 bytes --]

From 91c1e9a92a75e34da3aad8184b298654b30cdf6f Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 14 Jul 2016 08:16:06 +0200
Subject: [PATCH 1/2] arm: lookup devicetree via efi

arm64 already does this, arm should support it too.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 grub-core/loader/arm/linux.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/grub-core/loader/arm/linux.c b/grub-core/loader/arm/linux.c
index 5b39f02..106cfc3 100644
--- a/grub-core/loader/arm/linux.c
+++ b/grub-core/loader/arm/linux.c
@@ -221,6 +221,31 @@ failure:
   return grub_error (GRUB_ERR_BAD_ARGUMENT, "unable to prepare FDT");
 }
 
+#ifdef GRUB_MACHINE_EFI
+/* from ../arm64/fdt.c */
+static void *
+get_firmware_fdt (void)
+{
+  grub_efi_configuration_table_t *tables;
+  grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
+  void *firmware_fdt = NULL;
+  unsigned int i;
+
+  /* Look for FDT in UEFI config tables. */
+  tables = grub_efi_system_table->configuration_table;
+
+  for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
+    if (grub_memcmp (&tables[i].vendor_guid, &fdt_guid, sizeof (fdt_guid)) == 0)
+      {
+        firmware_fdt = tables[i].vendor_table;
+        grub_dprintf ("linux", "found registered FDT @ %p\n", firmware_fdt);
+        break;
+      }
+
+  return firmware_fdt;
+}
+#endif
+
 static grub_err_t
 linux_boot (void)
 {
@@ -237,6 +262,23 @@ linux_boot (void)
 		(char *) fdt_addr,
 		(char *) fdt_addr + 1);
 
+#ifdef GRUB_MACHINE_EFI
+  if (!fdt_valid) {
+    void *ptr = get_firmware_fdt();
+    int size = ptr ? grub_fdt_get_totalsize(ptr) : 0;
+    int extra = grub_strlen (linux_args) + 0x100;
+    if (ptr && size) {
+      fdt_addr = grub_efi_allocate_loader_memory (LINUX_FDT_PHYS_OFFSET, size + extra);
+      if (fdt_addr) {
+        grub_memcpy (fdt_addr, ptr, size);
+        fdt_valid = (fdt_addr && grub_fdt_check_header_nosize (fdt_addr) == 0);
+        grub_dprintf ("loader", "firmware fdt: memcpy(%p, %p, %d)\n",
+                      fdt_addr, ptr, size);
+      }
+    }
+  }
+#endif
+
   if (!fdt_valid && machine_type == GRUB_ARM_MACHINE_TYPE_FDT)
     return grub_error (GRUB_ERR_FILE_NOT_FOUND,
 		       N_("device tree must be supplied (see `devicetree' command)"));
-- 
1.8.3.1


[-- Attachment #3: 0002-arm-make-room-for-larger-devicetree.patch --]
[-- Type: text/x-patch, Size: 1855 bytes --]

From b311f8043c5718018c6d361092524d74208c458a Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Tue, 19 Jul 2016 14:14:26 +0200
Subject: [PATCH 2/2] arm: make room for larger devicetree

"qemu-system-arm -M virt" devicetree is > 64k.
Make sure we have enough space for it.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 grub-core/loader/arm/linux.c | 2 ++
 include/grub/arm/linux.h     | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/grub-core/loader/arm/linux.c b/grub-core/loader/arm/linux.c
index 106cfc3..5ac0014 100644
--- a/grub-core/loader/arm/linux.c
+++ b/grub-core/loader/arm/linux.c
@@ -49,9 +49,11 @@ typedef void (*kernel_entry_t) (int, unsigned long, void *);
 #define LINUX_ZIMAGE_OFFSET	0x24
 #define LINUX_ZIMAGE_MAGIC	0x016f2818
 
+#if 0 /* declared in include/grub/arm/linux.h */
 #define LINUX_PHYS_OFFSET        (0x00008000)
 #define LINUX_INITRD_PHYS_OFFSET (LINUX_PHYS_OFFSET + 0x02000000)
 #define LINUX_FDT_PHYS_OFFSET    (LINUX_INITRD_PHYS_OFFSET - 0x10000)
+#endif
 
 static grub_size_t
 get_atag_size (grub_uint32_t *atag)
diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h
index 059dbba..deb8a9c 100644
--- a/include/grub/arm/linux.h
+++ b/include/grub/arm/linux.h
@@ -37,9 +37,9 @@
 # include <grub/machine/loader.h>
 /* On UEFI platforms - load the images at the lowest available address not
    less than *_PHYS_OFFSET from the first available memory location. */
-# define LINUX_PHYS_OFFSET        (0x00008000)
+# define LINUX_PHYS_OFFSET        (0x00020000)
 # define LINUX_INITRD_PHYS_OFFSET (LINUX_PHYS_OFFSET + 0x02000000)
-# define LINUX_FDT_PHYS_OFFSET    (LINUX_INITRD_PHYS_OFFSET - 0x10000)
+# define LINUX_FDT_PHYS_OFFSET    (LINUX_INITRD_PHYS_OFFSET - 0x20000)
 static inline grub_addr_t
 grub_arm_firmware_get_boot_data (void)
 {
-- 
1.8.3.1

Re: [Qemu-devel] edk2 submodule + binaries (Re: [PATCH V5 2/7] tests/acpi: add pxb/pxb-pcie tests)

[Marcel Apfelbaum]

On 07/19/2016 02:42 PM, Laszlo Ersek wrote:
> On 07/19/16 12:48, Gerd Hoffmann wrote:
>>    Hi,
>>
>>>>   (2) ia32 ovmf too?  Will anybody use it?
>>>
>>> Then the next question is, what's the status of 32-bit UEFI OSes? Simple:
>>
>>> [ summary: bad ]
>>
>> Yep, that matches the impression I have.
>> Guess we don't want bother then.
>>
>>> Enabling Secure Boot in the OVMF binary is orthogonal to all of the
>>> above, but it has a licensing impact. It embeds (a subset of) OpenSSL in
>>> the binary, and changes the terms from "2-clause BSDL" to "2-clause BSDL
>>> and OpenSSL license" ("and" in the restrictive, not permissive, sense).
>>> I'm unsure if QEMU is willing and able to distribute such binaries.
>>>
>>> For the widest and simplest usability, X64 (without the SMM driver stack
>>> and without Secure Boot) is likely the best.
>>
>> Yes (also note the smm-enabled one doesn't run on i440fx).
>>
>> So the options I see are (a) build without smm or (b) build two
>> variants.
>
> I think people who just want to run "UEFI payloads" are served well
> enough by SMM-less. There are some developers who would benefit from a
> -D SMM_REQUIRE build as well, but that would be because they actually
> focus on SMM, I believe, so they can build their own firmware.
>
> If this is about the convenience of QEMU end-users, then I vote (a). I'd
> certainly like to avoid fielding bug reports that boil down to "I booted
> the -D SMM_REQUIRE build on i440fx".
>

I agree. For x86_64, OVMF 64-bit binaries without SMM or secure-boot
support are enough for a wide "audience".


Thanks,
Marcel

[...]

Re: [Qemu-devel] [PATCH V5 0/7] pxb: fix 64-bit MMIO allocation

[Michael S. Tsirkin]

On Sun, Jul 17, 2016 at 07:53:07PM +0300, Marcel Apfelbaum wrote:
> 
> v4 -> v5:
>   Addressed the pull request issues: (Peter Maydell)
>   See: https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg00882.html
>   - cland warning -> "hw/pci/pci.c:196:23: runtime error: shift exponent -1 is negative":
>     The PCIe Root port was not initialized properly, the interrupt pin was left 0. This
>     is a long standing issue exposed by the new test. (Patch 1/7)
>   - 'make check' fails on 32-bit:
>      Fix it by changing the ivshmem mem size from 4G
>      to 1G, since 4G is not a valid value on 32-bit archs. (Patch 2/7)
>      (4G is truncated to 0 on 32-bit systems)
>   - Rebased on mst's pci branch.
>   Since all the new changes are not related to the series, I kept the existing
>   "Reviewed-by"/"Tested-by" signatures.

Applied, but dropped tests for now - from experience they are too likely
to break on weird platforms.
Pls post new tests after 2.7.

Also, I would really like to see options to speed tests up.
How about running them under kvm?
There are a couple of minor acpi differences when running with kvm,
we'll need to white-list them in some way.



> v3 -> v4:
>  Addressed Igor's comments (thanks for the productive review!)
>  - Split pxb test patch (previously patch 3/3) into the test itself (patch 1/6) and the blobs (patch 6/6).
>  - New patch declaring pxb/pxb-pxie as not hot-pluggable.
>     - Note that it does not solve the DSDT issue, but it is a prerequisite for the next patch.
>  - New patch solving the DSDT issue spotted by Igor.
>      - Using V=1 DIFF=diff make check does make it easier to review the ACPI changes, thanks.
>  - Patches 4 and 5 untouched (previously patches 1/3 and 2/3) 
> 
> v2 -> v3:
>  - split original series "pci: better support for 64-bit MMIO allocation" into 2 series:
>     - this is the first part dealing with correct 64-bit MMIO ACPI computation
>     - the second one will include 64-bit MMIO reservation for PCI hotplug
>  - Add pxb/pxb-pcie tests (Igor) - See diffs below (*)
>  - Re-based on latest master.
>    
> v1 -> v2:
>  - resolved some styling issues (Laszlo)
>  - rebase on latest master (Laszlo)
> 
> 
> 
> 64-bit BARs allocations fix for devices behind PXBs/PXB-PCIEs.
> 
> In build_crs() the calculation and merging of the ranges already happens
> in 64-bit, but the entry boundaries are silently truncated to 32-bit in the
> call to aml_dword_memory(). Fix it by handling the 64-bit MMIO ranges separately.
> 
> 
> Thank you,
> Marcel
> 
> Marcel Apfelbaum (7):
>   hw/pcie-root-port: Fix PCIe root port initialization
>   tests/acpi: add pxb/pxb-pcie tests
>   hw/pxb: declare pxb devices as not hot-pluggable
>   hw/acpi: fix a DSDT table issue when a pxb is present.
>   acpi: refactor pxb crs computation
>   hw/apci: handle 64-bit MMIO regions correctly
>   tests/acpi: Add pxb/pxb-pcie tests blobs
> 
>  hw/i386/acpi-build.c                   | 131 ++++++++++++++++++++++++---------
>  hw/pci-bridge/ioh3420.c                |   1 +
>  hw/pci-bridge/pci_expander_bridge.c    |   2 +
>  tests/acpi-test-data/pc/DSDT.pxb       | Bin 0 -> 6286 bytes
>  tests/acpi-test-data/q35/DSDT.pxb_pcie | Bin 0 -> 9098 bytes
>  tests/bios-tables-test.c               |  37 ++++++++++
>  6 files changed, 135 insertions(+), 36 deletions(-)
>  create mode 100644 tests/acpi-test-data/pc/DSDT.pxb
>  create mode 100644 tests/acpi-test-data/q35/DSDT.pxb_pcie
> 
> -- 
> 2.4.3

Re: [Qemu-devel] [PATCH V5 0/7] pxb: fix 64-bit MMIO allocation

[Marcel Apfelbaum]

On 07/26/2016 09:30 PM, Michael S. Tsirkin wrote:
> On Sun, Jul 17, 2016 at 07:53:07PM +0300, Marcel Apfelbaum wrote:
>>
>> v4 -> v5:
>>   Addressed the pull request issues: (Peter Maydell)
>>   See: https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg00882.html
>>   - cland warning -> "hw/pci/pci.c:196:23: runtime error: shift exponent -1 is negative":
>>     The PCIe Root port was not initialized properly, the interrupt pin was left 0. This
>>     is a long standing issue exposed by the new test. (Patch 1/7)
>>   - 'make check' fails on 32-bit:
>>      Fix it by changing the ivshmem mem size from 4G
>>      to 1G, since 4G is not a valid value on 32-bit archs. (Patch 2/7)
>>      (4G is truncated to 0 on 32-bit systems)
>>   - Rebased on mst's pci branch.
>>   Since all the new changes are not related to the series, I kept the existing
>>   "Reviewed-by"/"Tested-by" signatures.
>

Hi Michael,

> Applied,

Thanks!

Can you please apply also '[PATCH V3] hw/virtio-pci: fix virtio behaviour'?
(https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg04744.html)

I think it worth having it in QEMU 2.7.

  but dropped tests for now - from experience they are too likely
> to break on weird platforms.
> Pls post new tests after 2.7.
>

Sure, I'll do that.

> Also, I would really like to see options to speed tests up.
> How about running them under kvm?
> There are a couple of minor acpi differences when running with kvm,
> we'll need to white-list them in some way.
>

Sure, we can do kvm 'variant' maybe for for 2.8.

Thanks,
Marcel

>
>

>> v3 -> v4:
>>  Addressed Igor's comments (thanks for the productive review!)
>>  - Split pxb test patch (previously patch 3/3) into the test itself (patch 1/6) and the blobs (patch 6/6).
>>  - New patch declaring pxb/pxb-pxie as not hot-pluggable.
>>     - Note that it does not solve the DSDT issue, but it is a prerequisite for the next patch.
>>  - New patch solving the DSDT issue spotted by Igor.
>>      - Using V=1 DIFF=diff make check does make it easier to review the ACPI changes, thanks.
>>  - Patches 4 and 5 untouched (previously patches 1/3 and 2/3)
>>
>> v2 -> v3:
>>  - split original series "pci: better support for 64-bit MMIO allocation" into 2 series:
>>     - this is the first part dealing with correct 64-bit MMIO ACPI computation
>>     - the second one will include 64-bit MMIO reservation for PCI hotplug
>>  - Add pxb/pxb-pcie tests (Igor) - See diffs below (*)
>>  - Re-based on latest master.
>>
>> v1 -> v2:
>>  - resolved some styling issues (Laszlo)
>>  - rebase on latest master (Laszlo)
>>
>>
>>
>> 64-bit BARs allocations fix for devices behind PXBs/PXB-PCIEs.
>>
>> In build_crs() the calculation and merging of the ranges already happens
>> in 64-bit, but the entry boundaries are silently truncated to 32-bit in the
>> call to aml_dword_memory(). Fix it by handling the 64-bit MMIO ranges separately.
>>
>>
>> Thank you,
>> Marcel
>>
>> Marcel Apfelbaum (7):
>>   hw/pcie-root-port: Fix PCIe root port initialization
>>   tests/acpi: add pxb/pxb-pcie tests
>>   hw/pxb: declare pxb devices as not hot-pluggable
>>   hw/acpi: fix a DSDT table issue when a pxb is present.
>>   acpi: refactor pxb crs computation
>>   hw/apci: handle 64-bit MMIO regions correctly
>>   tests/acpi: Add pxb/pxb-pcie tests blobs
>>
>>  hw/i386/acpi-build.c                   | 131 ++++++++++++++++++++++++---------
>>  hw/pci-bridge/ioh3420.c                |   1 +
>>  hw/pci-bridge/pci_expander_bridge.c    |   2 +
>>  tests/acpi-test-data/pc/DSDT.pxb       | Bin 0 -> 6286 bytes
>>  tests/acpi-test-data/q35/DSDT.pxb_pcie | Bin 0 -> 9098 bytes
>>  tests/bios-tables-test.c               |  37 ++++++++++
>>  6 files changed, 135 insertions(+), 36 deletions(-)
>>  create mode 100644 tests/acpi-test-data/pc/DSDT.pxb
>>  create mode 100644 tests/acpi-test-data/q35/DSDT.pxb_pcie
>>
>> --
>> 2.4.3

Re: [Qemu-devel] [PATCH V5 0/7] pxb: fix 64-bit MMIO allocation

[Michael S. Tsirkin]

On Wed, Jul 27, 2016 at 07:27:11AM +0300, Marcel Apfelbaum wrote:
> On 07/26/2016 09:30 PM, Michael S. Tsirkin wrote:
> > On Sun, Jul 17, 2016 at 07:53:07PM +0300, Marcel Apfelbaum wrote:
> > > 
> > > v4 -> v5:
> > >   Addressed the pull request issues: (Peter Maydell)
> > >   See: https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg00882.html
> > >   - cland warning -> "hw/pci/pci.c:196:23: runtime error: shift exponent -1 is negative":
> > >     The PCIe Root port was not initialized properly, the interrupt pin was left 0. This
> > >     is a long standing issue exposed by the new test. (Patch 1/7)
> > >   - 'make check' fails on 32-bit:
> > >      Fix it by changing the ivshmem mem size from 4G
> > >      to 1G, since 4G is not a valid value on 32-bit archs. (Patch 2/7)
> > >      (4G is truncated to 0 on 32-bit systems)
> > >   - Rebased on mst's pci branch.
> > >   Since all the new changes are not related to the series, I kept the existing
> > >   "Reviewed-by"/"Tested-by" signatures.
> > 
> 
> Hi Michael,
> 
> > Applied,
> 
> Thanks!
> 
> Can you please apply also '[PATCH V3] hw/virtio-pci: fix virtio behaviour'?
> (https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg04744.html)
> 
> I think it worth having it in QEMU 2.7.
> 
>  but dropped tests for now - from experience they are too likely
> > to break on weird platforms.
> > Pls post new tests after 2.7.
> > 
> 
> Sure, I'll do that.
> 
> > Also, I would really like to see options to speed tests up.
> > How about running them under kvm?
> > There are a couple of minor acpi differences when running with kvm,
> > we'll need to white-list them in some way.
> > 
> 
> Sure, we can do kvm 'variant' maybe for for 2.8.


Yes but without maintaining two variants o expected files please.


> Thanks,
> Marcel
> 
> > 
> > 
> 
> > > v3 -> v4:
> > >  Addressed Igor's comments (thanks for the productive review!)
> > >  - Split pxb test patch (previously patch 3/3) into the test itself (patch 1/6) and the blobs (patch 6/6).
> > >  - New patch declaring pxb/pxb-pxie as not hot-pluggable.
> > >     - Note that it does not solve the DSDT issue, but it is a prerequisite for the next patch.
> > >  - New patch solving the DSDT issue spotted by Igor.
> > >      - Using V=1 DIFF=diff make check does make it easier to review the ACPI changes, thanks.
> > >  - Patches 4 and 5 untouched (previously patches 1/3 and 2/3)
> > > 
> > > v2 -> v3:
> > >  - split original series "pci: better support for 64-bit MMIO allocation" into 2 series:
> > >     - this is the first part dealing with correct 64-bit MMIO ACPI computation
> > >     - the second one will include 64-bit MMIO reservation for PCI hotplug
> > >  - Add pxb/pxb-pcie tests (Igor) - See diffs below (*)
> > >  - Re-based on latest master.
> > > 
> > > v1 -> v2:
> > >  - resolved some styling issues (Laszlo)
> > >  - rebase on latest master (Laszlo)
> > > 
> > > 
> > > 
> > > 64-bit BARs allocations fix for devices behind PXBs/PXB-PCIEs.
> > > 
> > > In build_crs() the calculation and merging of the ranges already happens
> > > in 64-bit, but the entry boundaries are silently truncated to 32-bit in the
> > > call to aml_dword_memory(). Fix it by handling the 64-bit MMIO ranges separately.
> > > 
> > > 
> > > Thank you,
> > > Marcel
> > > 
> > > Marcel Apfelbaum (7):
> > >   hw/pcie-root-port: Fix PCIe root port initialization
> > >   tests/acpi: add pxb/pxb-pcie tests
> > >   hw/pxb: declare pxb devices as not hot-pluggable
> > >   hw/acpi: fix a DSDT table issue when a pxb is present.
> > >   acpi: refactor pxb crs computation
> > >   hw/apci: handle 64-bit MMIO regions correctly
> > >   tests/acpi: Add pxb/pxb-pcie tests blobs
> > > 
> > >  hw/i386/acpi-build.c                   | 131 ++++++++++++++++++++++++---------
> > >  hw/pci-bridge/ioh3420.c                |   1 +
> > >  hw/pci-bridge/pci_expander_bridge.c    |   2 +
> > >  tests/acpi-test-data/pc/DSDT.pxb       | Bin 0 -> 6286 bytes
> > >  tests/acpi-test-data/q35/DSDT.pxb_pcie | Bin 0 -> 9098 bytes
> > >  tests/bios-tables-test.c               |  37 ++++++++++
> > >  6 files changed, 135 insertions(+), 36 deletions(-)
> > >  create mode 100644 tests/acpi-test-data/pc/DSDT.pxb
> > >  create mode 100644 tests/acpi-test-data/q35/DSDT.pxb_pcie
> > > 
> > > --
> > > 2.4.3
>