Skip to contentSkip to conversation

ACPI hardware-reduced support

PATCH In review Samuel Ortiz 4 participants 39 messages Oct 22, 2018 – Oct 24, 2018

lore
Share & tools
lore
#0 lore raw

cover ACPI hardware-reduced support

This patch set implements support for the ACPI hardware-reduced specification.

The changes are coming from the NEMU [1] project where we're defining a new x86 machine type: i386/virt. This is an EFI only, ACPI hardware-reduced platform and as such we had to implement support for the latter.

As a preliminary for adding hardware-reduced support to QEMU, we did some ACPI code reorganization with the following goals:

* Share as much as possible of the current ACPI build APIs between legacy and hardware-reduced ACPI.
* Share the ACPI build code across machine types and architectures and remove the typical PC machine type dependency.
Eventually we hope to see arm/virt also re-use much of that code.

The rest of the patchset adds the hardware-reduced support on top of this code reorganization. Here again, the implementation is machine type, platform and architecture independent.

[1] https://github.com/intel/nemu

The following changes since commit b312532fd03413d0e6ae6767ec793a3e30f487b8:

Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2018-10-19 19:01:07 +0100)

are available in the Git repository at:

[email protected]:intel/nemu topic/upstream/acpi

for you to fetch changes up to 4133ebe0d638d0a3d583bd89aa2360ef35ba746b:

hw: acpi: reduced: Add PCI hotplug support (2018-10-22 20:32:23 +0200)

----------------------------------------------------------------
Jing Liu (2):
hw: acpi: reduced: Add shutdown support
hw: acpi: reduced: Add reboot support

Samuel Ortiz (10):
hw: i386: Decouple the ACPI build from the PC machine type
hw: acpi: Export ACPI build alignment API
hw: acpi: Export the RSDP build API
hw: arm: Switch to the AML build RSDP building routine
hw: acpi: Do not create hotplug method when handler is not defined
hw: i386: Make the hotpluggable memory size property more generic
hw: fw-build: Add firmware build methods and state
hw: i386: Convert PC machine type to firmware build methods
hw: acpi: Initial hardware-reduced support
hw: acpi: reduced: Generic Event Device support

Sebastien Boeuf (4):
hw: acpi: Export the PCI hotplug API
hw: acpi: Retrieve the PCI bus from AcpiPciHpState
hw: acpi: reduced: Add NFIT support
hw: acpi: reduced: Add PCI hotplug support

Yang Zhong (10):
hw: acpi: Generalize AML build routines
hw: acpi: Factorize _OSC AML across architectures
hw: i386: Refactor PCI host getter
hw: acpi: Export and generalize the PCI host AML API
hw: acpi: Export the MCFG getter
hw: acpi: Export the SRAT AML build API
hw: acpi: Fix memory hotplug AML generation error
hw: acpi: reduced: Add MCFG support
hw: acpi: reduced: Add memory hotplug support
hw: acpi: reduced: Add SRAT table

 default-configs/i386-softmmu.mak |    1 +
 hw/acpi/Makefile.objs            |    1 +
 hw/acpi/aml-build.c              | 1139 +++++++++++++++++++++++++++++++++++-
 hw/acpi/cpu.c                    |    8 +-
 hw/acpi/cpu_hotplug.c            |    9 +-
 hw/acpi/memory_hotplug.c         |   21 +-
 hw/acpi/pcihp.c                  |   10 +-
 hw/acpi/reduced.c                |  472 +++++++++++++++
 hw/arm/virt-acpi-build.c         |   86 +--
 hw/i386/acpi-build.c             | 1172 +++-----------------------------------
 hw/i386/acpi-build.h             |    4 +-
 hw/i386/pc.c                     |  188 +++---
 hw/i386/pc_piix.c                |   21 +-
 hw/i386/pc_q35.c                 |   21 +-
 hw/pci-host/piix.c               |    8 -
 include/hw/acpi/acpi-defs.h      |   15 +
 include/hw/acpi/acpi.h           |   47 ++
 include/hw/acpi/aml-build.h      |   48 ++
 include/hw/acpi/reduced.h        |   46 ++
 include/hw/boards.h              |    5 +
 include/hw/fw-build.h            |   57 ++
 include/hw/i386/acpi.h           |   27 +
 include/hw/i386/pc.h             |   21 +-
 include/hw/mem/memory-device.h   |    2 +
 stubs/Makefile.objs              |    1 -
 stubs/pci-host-piix.c            |    6 -
 26 files changed, 2107 insertions(+), 1329 deletions(-)
 create mode 100644 hw/acpi/reduced.c
 create mode 100644 include/hw/acpi/reduced.h
 create mode 100644 include/hw/fw-build.h
 create mode 100644 include/hw/i386/acpi.h
 delete mode 100644 stubs/pci-host-piix.c
Signature · 1 line
2.17.2
#1 lore raw

PATCH 1/26 hw: i386: Decouple the ACPI build from the PC machine type

ACPI tables are platform and machine type and even architecture agnostic, and as such we want to provide an internal ACPI API that only depends on platform agnostic information.

For the x86 architecture, in order to build ACPI tables independently from the PC or Q35 machine types, we are moving a few MachineState structure fields into a machine type agnostic structure called AcpiConfiguration. The structure fields we move are:

HotplugHandler *acpi_dev AcpiNVDIMMState acpi_nvdimm_state; FWCfgState *fw_cfg
ram_addr_t below_4g_mem_size, above_4g_mem_size bool apic_xrupt_override
unsigned apic_id_limit
uint64_t numa_nodes
uint64_t numa_mem

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Shannon Zhao <[email protected]>
Cc Peter Maydell <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Richard Henderson <[email protected]>
Cc Eduardo Habkost <[email protected]>
Cc Marcel Apfelbaum <[email protected]>
---
 hw/acpi/cpu_hotplug.c    |   9 +-
 hw/arm/virt-acpi-build.c |  10 ---
 hw/i386/acpi-build.c     | 135 +++++++++++++-----------------
 hw/i386/acpi-build.h     |   4 +-
 hw/i386/pc.c             | 176 ++++++++++++++++++++++++---------------
 hw/i386/pc_piix.c        |  21 ++---
 hw/i386/pc_q35.c         |  21 ++---
 include/hw/acpi/acpi.h   |  43 ++++++++++
 include/hw/i386/pc.h     |  19 ++---
 9 files changed, 245 insertions(+), 193 deletions(-)
hw/acpi/cpu_hotplug.c +5 -4
index 5243918125..634dc3b846 100644
@@ -237,9 +237,9 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
     /* The current AML generator can cover the APIC ID range [0..255],
      * inclusive, for VCPU hotplug. */
     QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
-    if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
+    if (pcms->acpi_configuration.apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
         error_report("max_cpus is too large. APIC ID of last CPU is %u",
-                     pcms->apic_id_limit - 1);
+                     pcms->acpi_configuration.apic_id_limit - 1);
         exit(1);
     }
 
@@ -316,8 +316,9 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
      * ith up to 255 elements. Windows guests up to win2k8 fail when
      * VarPackageOp is used.
      */
-    pkg = pcms->apic_id_limit <= 255 ? aml_package(pcms->apic_id_limit) :
-                                       aml_varpackage(pcms->apic_id_limit);
+    pkg = pcms->acpi_configuration.apic_id_limit <= 255 ?
+        aml_package(pcms->acpi_configuration.apic_id_limit) :
+        aml_varpackage(pcms->acpi_configuration.apic_id_limit);
 
     for (i = 0, apic_idx = 0; i < apic_ids->len; i++) {
         int apic_id = apic_ids->cpus[i].arch_id;
hw/arm/virt-acpi-build.c +0 -10
index 5785fb697c..f28a2faa53 100644
@@ -790,16 +790,6 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     free_aml_allocator();
 }
 
-typedef
-struct AcpiBuildState {
-    /* Copy of table in RAM (for patching). */
-    MemoryRegion *table_mr;
-    MemoryRegion *rsdp_mr;
-    MemoryRegion *linker_mr;
-    /* Is table patched? */
-    bool patched;
-} AcpiBuildState;
-
 static
 void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
 {
hw/i386/acpi-build.c +60 -75

341 lines not on this page · raw message

hw/i386/acpi-build.h +3 -1
index 007332e51c..1e6b5a7c2e 100644
@@ -2,6 +2,8 @@
 #ifndef HW_I386_ACPI_BUILD_H
 #define HW_I386_ACPI_BUILD_H
 
-void acpi_setup(void);
+#include "hw/acpi/acpi.h"
+
+void acpi_setup(MachineState *machine, AcpiConfiguration *conf);
 
 #endif
hw/i386/pc.c +107 -69

478 lines not on this page · raw message

hw/i386/pc_piix.c +11 -10
index dc09466b3e..1d10374dfa 100644
@@ -71,6 +71,7 @@ static void pc_init1(MachineState *machine,
 {
     PCMachineState *pcms = PC_MACHINE(machine);
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+    AcpiConfiguration *conf = &pcms->acpi_configuration;
     MemoryRegion *system_memory = get_system_memory();
     MemoryRegion *system_io = get_system_io();
     int i;
@@ -142,11 +143,11 @@ static void pc_init1(MachineState *machine,
         }
 
         if (machine->ram_size >= lowmem) {
-            pcms->above_4g_mem_size = machine->ram_size - lowmem;
-            pcms->below_4g_mem_size = lowmem;
+            conf->above_4g_mem_size = machine->ram_size - lowmem;
+            conf->below_4g_mem_size = lowmem;
         } else {
-            pcms->above_4g_mem_size = 0;
-            pcms->below_4g_mem_size = machine->ram_size;
+            conf->above_4g_mem_size = 0;
+            conf->below_4g_mem_size = machine->ram_size;
         }
     }
 
@@ -199,8 +200,8 @@ static void pc_init1(MachineState *machine,
                               pci_type,
                               &i440fx_state, &piix3_devfn, &isa_bus, pcms->gsi,
                               system_memory, system_io, machine->ram_size,
-                              pcms->below_4g_mem_size,
-                              pcms->above_4g_mem_size,
+                              conf->below_4g_mem_size,
+                              conf->above_4g_mem_size,
                               pci_memory, ram_memory);
         pcms->bus = pci_bus;
     } else {
@@ -289,16 +290,16 @@ static void pc_init1(MachineState *machine,
 
         object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
                                  TYPE_HOTPLUG_HANDLER,
-                                 (Object **)&pcms->acpi_dev,
+                                 (Object **)&conf->acpi_dev,
                                  object_property_allow_set_link,
                                  OBJ_PROP_LINK_STRONG, &error_abort);
         object_property_set_link(OBJECT(machine), OBJECT(piix4_pm),
                                  PC_MACHINE_ACPI_DEVICE_PROP, &error_abort);
     }
 
-    if (pcms->acpi_nvdimm_state.is_enabled) {
-        nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
-                               pcms->fw_cfg, OBJECT(pcms));
+    if (conf->acpi_nvdimm_state.is_enabled) {
+        nvdimm_init_acpi_state(&conf->acpi_nvdimm_state, system_io,
+                               conf->fw_cfg, OBJECT(pcms));
     }
 }
 
hw/i386/pc_q35.c +11 -10
index 532241e3f8..fb5c89bf7c 100644
@@ -63,6 +63,7 @@ static void pc_q35_init(MachineState *machine)
 {
     PCMachineState *pcms = PC_MACHINE(machine);
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+    AcpiConfiguration *conf = &pcms->acpi_configuration;
     Q35PCIHost *q35_host;
     PCIHostState *phb;
     PCIBus *host_bus;
@@ -116,11 +117,11 @@ static void pc_q35_init(MachineState *machine)
     }
 
     if (machine->ram_size >= lowmem) {
-        pcms->above_4g_mem_size = machine->ram_size - lowmem;
-        pcms->below_4g_mem_size = lowmem;
+        conf->above_4g_mem_size = machine->ram_size - lowmem;
+        conf->below_4g_mem_size = lowmem;
     } else {
-        pcms->above_4g_mem_size = 0;
-        pcms->below_4g_mem_size = machine->ram_size;
+        conf->above_4g_mem_size = 0;
+        conf->below_4g_mem_size = machine->ram_size;
     }
 
     if (xen_enabled()) {
@@ -179,9 +180,9 @@ static void pc_q35_init(MachineState *machine)
                              MCH_HOST_PROP_SYSTEM_MEM, NULL);
     object_property_set_link(OBJECT(q35_host), OBJECT(system_io),
                              MCH_HOST_PROP_IO_MEM, NULL);
-    object_property_set_int(OBJECT(q35_host), pcms->below_4g_mem_size,
+    object_property_set_int(OBJECT(q35_host), conf->below_4g_mem_size,
                             PCI_HOST_BELOW_4G_MEM_SIZE, NULL);
-    object_property_set_int(OBJECT(q35_host), pcms->above_4g_mem_size,
+    object_property_set_int(OBJECT(q35_host), conf->above_4g_mem_size,
                             PCI_HOST_ABOVE_4G_MEM_SIZE, NULL);
     /* pci */
     qdev_init_nofail(DEVICE(q35_host));
@@ -194,7 +195,7 @@ static void pc_q35_init(MachineState *machine)
 
     object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
                              TYPE_HOTPLUG_HANDLER,
-                             (Object **)&pcms->acpi_dev,
+                             (Object **)&conf->acpi_dev,
                              object_property_allow_set_link,
                              OBJ_PROP_LINK_STRONG, &error_abort);
     object_property_set_link(OBJECT(machine), OBJECT(lpc),
@@ -276,9 +277,9 @@ static void pc_q35_init(MachineState *machine)
     pc_vga_init(isa_bus, host_bus);
     pc_nic_init(pcmc, isa_bus, host_bus);
 
-    if (pcms->acpi_nvdimm_state.is_enabled) {
-        nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
-                               pcms->fw_cfg, OBJECT(pcms));
+    if (conf->acpi_nvdimm_state.is_enabled) {
+        nvdimm_init_acpi_state(&conf->acpi_nvdimm_state, system_io,
+                               conf->fw_cfg, OBJECT(pcms));
     }
 }
 
include/hw/acpi/acpi.h +43 -0

55 lines not on this page · raw message

include/hw/i386/pc.h +5 -14
index dfe6746692..da0bd39741 100644
@@ -12,6 +12,7 @@
 #include "qemu/range.h"
 #include "qemu/bitmap.h"
 #include "sysemu/sysemu.h"
+#include "hw/acpi/acpi.h"
 #include "hw/pci/pci.h"
 #include "hw/compat.h"
 #include "hw/mem/pc-dimm.h"
@@ -35,10 +36,8 @@ struct PCMachineState {
     Notifier machine_done;
 
     /* Pointers to devices and objects: */
-    HotplugHandler *acpi_dev;
     ISADevice *rtc;
     PCIBus *bus;
-    FWCfgState *fw_cfg;
     qemu_irq *gsi;
 
     /* Configuration options: */
@@ -46,28 +45,20 @@ struct PCMachineState {
     OnOffAuto vmport;
     OnOffAuto smm;
 
-    AcpiNVDIMMState acpi_nvdimm_state;
-
     bool acpi_build_enabled;
     bool smbus;
     bool sata;
     bool pit;
 
-    /* RAM information (sizes, addresses, configuration): */
-    ram_addr_t below_4g_mem_size, above_4g_mem_size;
-
-    /* CPU and apic information: */
-    bool apic_xrupt_override;
-    unsigned apic_id_limit;
+    /* CPU information */
     uint16_t boot_cpus;
 
-    /* NUMA information: */
-    uint64_t numa_nodes;
-    uint64_t *node_mem;
-
     /* Address space used by IOAPIC device. All IOAPIC interrupts
      * will be translated to MSI messages in the address space. */
     AddressSpace *ioapic_as;
+
+    /* ACPI configuration */
+    AcpiConfiguration acpi_configuration;
 };
 
 #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
Signature · 1 line
2.17.2
#2 lore raw

Re: [Qemu-devel] [PATCH 01/26] hw: i386: Decouple the ACPI build from the PC machine type

Quoted 1 line · Samuel Ortiz

+static void acpi_conf_pc_init(MachineState *machine)

This should take a PCMachineState*, since you never use "machine" except for downcasting.

Paolo

Quoted 14 lines
hw/i386/pc.c +14 -0
+{
+    PCMachineState *pcms = PC_MACHINE(machine);
+    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
+    AcpiConfiguration *conf = &pcms->acpi_configuration;
+
+    /* Machine class settings */
+    conf->legacy_acpi_table_size = pcmc->legacy_acpi_table_size;
+    conf->legacy_cpu_hotplug = pcmc->legacy_cpu_hotplug;
+    conf->rsdp_in_ram = pcmc->rsdp_in_ram;
+
+    /* ACPI build state */
+    conf->build_state = NULL;
+}
+
#3 lore raw

PATCH 2/26 hw: acpi: Export ACPI build alignment API

In reply to Samuel Ortiz

This is going to be needed by the Hardware-reduced ACPI routines.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Marcel Apfelbaum <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Richard Henderson <[email protected]>
Cc Eduardo Habkost <[email protected]>
---
 hw/acpi/aml-build.c         | 8 ++++++++
 hw/i386/acpi-build.c        | 8 --------
 include/hw/acpi/aml-build.h | 2 ++
 3 files changed, 10 insertions(+), 8 deletions(-)
hw/acpi/aml-build.c +8 -0
index 1e43cd736d..51b608432f 100644
@@ -1565,6 +1565,14 @@ unsigned acpi_data_len(GArray *table)
     return table->len;
 }
 
+void acpi_align_size(GArray *blob, unsigned align)
+{
+    /* Align size to multiple of given size. This reduces the chance
+     * we need to change size in the future (breaking cross version migration).
+     */
+    g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
+}
+
 void acpi_add_table(GArray *table_offsets, GArray *table_data)
 {
     uint32_t offset = table_data->len;
hw/i386/acpi-build.c +0 -8
index c8545238c4..1bf02b6a93 100644
@@ -282,14 +282,6 @@ static void acpi_get_pci_holes(Range *hole, Range *hole64)
                                                NULL));
 }
 
-static void acpi_align_size(GArray *blob, unsigned align)
-{
-    /* Align size to multiple of given size. This reduces the chance
-     * we need to change size in the future (breaking cross version migration).
-     */
-    g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
-}
-
 /* FACS */
 static void
 build_facs(GArray *table_data, BIOSLinker *linker)
include/hw/acpi/aml-build.h +2 -0
index 6c36903c0a..813f51317c 100644
@@ -6,6 +6,7 @@
 
 /* Reserve RAM space for tables: add another order of magnitude. */
 #define ACPI_BUILD_TABLE_MAX_SIZE         0x200000
+#define ACPI_BUILD_ALIGN_SIZE             0x1000
 
 #define ACPI_BUILD_APPNAME6 "BOCHS "
 #define ACPI_BUILD_APPNAME4 "BXPC"
@@ -384,6 +385,7 @@ build_header(BIOSLinker *linker, GArray *table_data,
              const char *oem_id, const char *oem_table_id);
 void *acpi_data_push(GArray *table_data, unsigned size);
 unsigned acpi_data_len(GArray *table);
+void acpi_align_size(GArray *blob, unsigned align);
 void acpi_add_table(GArray *table_offsets, GArray *table_data);
 void acpi_build_tables_init(AcpiBuildTables *tables);
 void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre);
Signature · 1 line
2.17.2
#4 lore raw

PATCH 3/26 hw: acpi: Export the RSDP build API

In reply to Samuel Ortiz

The hardware-reduced API will need to build RSDP as well, so we should export this routine.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Richard Henderson <[email protected]>
Cc Eduardo Habkost <[email protected]>
Cc Marcel Apfelbaum <[email protected]>
---
 hw/acpi/aml-build.c         | 65 +++++++++++++++++++++++++++++++++++++
 hw/i386/acpi-build.c        | 28 +---------------
 include/hw/acpi/aml-build.h |  4 +++
 3 files changed, 70 insertions(+), 27 deletions(-)
hw/acpi/aml-build.c +65 -0
index 51b608432f..8f02af0659 100644
@@ -1651,6 +1651,71 @@ build_xsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
                  (void *)xsdt, "XSDT", xsdt_len, 1, oem_id, oem_table_id);
 }
 
+/* Legacy RSDP pointing at an RSDT. This is deprecated */
+GArray *build_rsdp_rsdt(GArray *rsdp_table,
+                        BIOSLinker *linker, unsigned rsdt_tbl_offset)
+{
+    AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
+    unsigned rsdt_pa_size = sizeof(rsdp->rsdt_physical_address);
+    unsigned rsdt_pa_offset =
+        (char *)&rsdp->rsdt_physical_address - rsdp_table->data;
+
+    bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
+                             true /* fseg memory */);
+
+    memcpy(&rsdp->signature, "RSD PTR ", 8);
+    memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6);
+    /* Address to be filled by Guest linker */
+    bios_linker_loader_add_pointer(linker,
+        ACPI_BUILD_RSDP_FILE, rsdt_pa_offset, rsdt_pa_size,
+        ACPI_BUILD_TABLE_FILE, rsdt_tbl_offset);
+
+    /* Checksum to be filled by Guest linker */
+    bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
+        (char *)rsdp - rsdp_table->data, sizeof *rsdp,
+        (char *)&rsdp->checksum - rsdp_table->data);
+
+    return rsdp_table;
+}
+
+/* RSDP pointing at an XSDT */
+GArray *build_rsdp(GArray *rsdp_table,
+                   BIOSLinker *linker, unsigned xsdt_tbl_offset)
+{
+    AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
+    unsigned xsdt_pa_size = sizeof(rsdp->xsdt_physical_address);
+    unsigned xsdt_pa_offset =
+        (char *)&rsdp->xsdt_physical_address - rsdp_table->data;
+    unsigned xsdt_offset =
+        (char *)&rsdp->length - rsdp_table->data;
+
+    bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
+                             true /* fseg memory */);
+
+    memcpy(&rsdp->signature, "RSD PTR ", 8);
+    memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6);
+    rsdp->length = cpu_to_le32(sizeof(*rsdp));
+    /* version 2, we will use the XSDT pointer */
+    rsdp->revision = 0x02;
+
+    /* Address to be filled by Guest linker */
+    bios_linker_loader_add_pointer(linker,
+        ACPI_BUILD_RSDP_FILE, xsdt_pa_offset, xsdt_pa_size,
+        ACPI_BUILD_TABLE_FILE, xsdt_tbl_offset);
+
+    /* Legacy checksum to be filled by Guest linker */
+    bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
+        (char *)rsdp - rsdp_table->data, xsdt_offset,
+        (char *)&rsdp->checksum - rsdp_table->data);
+
+    /* Extended checksum to be filled by Guest linker */
+    bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
+        (char *)rsdp - rsdp_table->data, sizeof *rsdp,
+        (char *)&rsdp->extended_checksum - rsdp_table->data);
+
+    return rsdp_table;
+}
+
 void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
                        uint64_t len, int node, MemoryAffinityFlags flags)
 {
hw/i386/acpi-build.c +1 -27
index 1bf02b6a93..eee2eb3ed2 100644
@@ -2512,32 +2512,6 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
                  "IVRS", table_data->len - iommu_start, 1, NULL, NULL);
 }
 
-static GArray *
-build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset)
-{
-    AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
-    unsigned rsdt_pa_size = sizeof(rsdp->rsdt_physical_address);
-    unsigned rsdt_pa_offset =
-        (char *)&rsdp->rsdt_physical_address - rsdp_table->data;
-
-    bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
-                             true /* fseg memory */);
-
-    memcpy(&rsdp->signature, "RSD PTR ", 8);
-    memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6);
-    /* Address to be filled by Guest linker */
-    bios_linker_loader_add_pointer(linker,
-        ACPI_BUILD_RSDP_FILE, rsdt_pa_offset, rsdt_pa_size,
-        ACPI_BUILD_TABLE_FILE, rsdt_tbl_offset);
-
-    /* Checksum to be filled by Guest linker */
-    bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
-        (char *)rsdp - rsdp_table->data, sizeof *rsdp,
-        (char *)&rsdp->checksum - rsdp_table->data);
-
-    return rsdp_table;
-}
-
 static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
 {
     Object *pci_host;
@@ -2682,7 +2656,7 @@ void acpi_build(AcpiBuildTables *tables,
                slic_oem.id, slic_oem.table_id);
 
     /* RSDP is in FSEG memory, so allocate it separately */
-    build_rsdp(tables->rsdp, tables->linker, rsdt);
+    build_rsdp_rsdt(tables->rsdp, tables->linker, rsdt);
 
     /* We'll expose it all to Guest so we want to reduce
      * chance of size changes.
include/hw/acpi/aml-build.h +4 -0
index 813f51317c..b4345c95bd 100644
@@ -389,6 +389,10 @@ void acpi_align_size(GArray *blob, unsigned align);
 void acpi_add_table(GArray *table_offsets, GArray *table_data);
 void acpi_build_tables_init(AcpiBuildTables *tables);
 void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre);
+GArray *build_rsdp_rsdt(GArray *rsdp_table,
+                        BIOSLinker *linker, unsigned rsdt_tbl_offset);
+GArray *build_rsdp(GArray *rsdp_table,
+                   BIOSLinker *linker, unsigned xsdt_tbl_offset);
 void
 build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
            const char *oem_id, const char *oem_table_id);
Signature · 1 line
2.17.2
#5 lore raw

PATCH 4/26 hw: arm: Switch to the AML build RSDP building routine

In reply to Samuel Ortiz

We make the ARM virt ACPI code use the now shared build_rsdp() API from aml-build.c. By doing so we fix a bug where the ARM implementation was missing adding both the legacy and extended checksums, which was building an invalid RSDP table.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Shannon Zhao <[email protected]>
Cc Peter Maydell <[email protected]>
---
 hw/arm/virt-acpi-build.c | 31 +------------------------------
 1 file changed, 1 insertion(+), 30 deletions(-)
hw/arm/virt-acpi-build.c +1 -30
index f28a2faa53..0a6a88380a 100644
@@ -35,6 +35,7 @@
 #include "target/arm/cpu.h"
 #include "hw/acpi/acpi-defs.h"
 #include "hw/acpi/acpi.h"
+#include "hw/acpi/aml-build.h"
 #include "hw/nvram/fw_cfg.h"
 #include "hw/acpi/bios-linker-loader.h"
 #include "hw/loader.h"
@@ -366,36 +367,6 @@ static void acpi_dsdt_add_power_button(Aml *scope)
     aml_append(scope, dev);
 }
 
-/* RSDP */
-static GArray *
-build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned xsdt_tbl_offset)
-{
-    AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
-    unsigned xsdt_pa_size = sizeof(rsdp->xsdt_physical_address);
-    unsigned xsdt_pa_offset =
-        (char *)&rsdp->xsdt_physical_address - rsdp_table->data;
-
-    bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
-                             true /* fseg memory */);
-
-    memcpy(&rsdp->signature, "RSD PTR ", sizeof(rsdp->signature));
-    memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, sizeof(rsdp->oem_id));
-    rsdp->length = cpu_to_le32(sizeof(*rsdp));
-    rsdp->revision = 0x02;
-
-    /* Address to be filled by Guest linker */
-    bios_linker_loader_add_pointer(linker,
-        ACPI_BUILD_RSDP_FILE, xsdt_pa_offset, xsdt_pa_size,
-        ACPI_BUILD_TABLE_FILE, xsdt_tbl_offset);
-
-    /* Checksum to be filled by Guest linker */
-    bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
-        (char *)rsdp - rsdp_table->data, sizeof *rsdp,
-        (char *)&rsdp->checksum - rsdp_table->data);
-
-    return rsdp_table;
-}
-
 static void
 build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 {
Signature · 1 line
2.17.2
#6 lore raw

PATCH 5/26 hw: acpi: Generalize AML build routines

In reply to Samuel Ortiz

From: Yang Zhong <[email protected]>

Most of the AML build routines under acpi-build are not even architecture specific. They can be moved to the more generic hw/acpi folder where they could be shared across machine types and architectures.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Richard Henderson <[email protected]>
Cc Eduardo Habkost <[email protected]>
Cc Marcel Apfelbaum <[email protected]>
---
 hw/acpi/aml-build.c         | 498 ++++++++++++++++++++++++++++++++++
 hw/i386/acpi-build.c        | 516 +-----------------------------------
 include/hw/acpi/aml-build.h |  25 ++
 3 files changed, 526 insertions(+), 513 deletions(-)
hw/acpi/aml-build.c +498 -0
index 8f02af0659..6daa0f6df2 100644
@@ -25,6 +25,10 @@
 #include "qemu/bswap.h"
 #include "qemu/bitops.h"
 #include "sysemu/numa.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
+#include "qemu/range.h"
+#include "hw/pci/pci_bridge.h"
 
 static GArray *build_alloc_array(void)
 {
@@ -1597,6 +1601,500 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre)
     g_array_free(tables->vmgenid, mfre);
 }
 
+static void crs_range_insert(GPtrArray *ranges, uint64_t base, uint64_t limit)
+{
+    CrsRangeEntry *entry;
+
+    entry = g_malloc(sizeof(*entry));
+    entry->base = base;
+    entry->limit = limit;
+
+    g_ptr_array_add(ranges, entry);
+}
+
+static void crs_range_free(gpointer data)
+{
+    CrsRangeEntry *entry = (CrsRangeEntry *)data;
+    g_free(entry);
+}
+
+void crs_range_set_init(CrsRangeSet *range_set)
+{
+    range_set->io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+    range_set->mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+    range_set->mem_64bit_ranges =
+            g_ptr_array_new_with_free_func(crs_range_free);
+}
+
+void crs_range_set_free(CrsRangeSet *range_set)
+{
+    g_ptr_array_free(range_set->io_ranges, true);
+    g_ptr_array_free(range_set->mem_ranges, true);
+    g_ptr_array_free(range_set->mem_64bit_ranges, true);
+}
+
+static gint crs_range_compare(gconstpointer a, gconstpointer b)
+{
+     CrsRangeEntry *entry_a = *(CrsRangeEntry **)a;
+     CrsRangeEntry *entry_b = *(CrsRangeEntry **)b;
+
+     return (int64_t)entry_a->base - (int64_t)entry_b->base;
+}
+
+/*
+ * crs_replace_with_free_ranges - given the 'used' ranges within [start - end]
+ * interval, computes the 'free' ranges from the same interval.
+ * Example: If the input array is { [a1 - a2],[b1 - b2] }, the function
+ * will return { [base - a1], [a2 - b1], [b2 - limit] }.
+ */
+void crs_replace_with_free_ranges(GPtrArray *ranges,
+                                         uint64_t start, uint64_t end)
+{
+    GPtrArray *free_ranges = g_ptr_array_new();
+    uint64_t free_base = start;
+    int i;
+
+    g_ptr_array_sort(ranges, crs_range_compare);
+    for (i = 0; i < ranges->len; i++) {
+        CrsRangeEntry *used = g_ptr_array_index(ranges, i);
+
+        if (free_base < used->base) {
+            crs_range_insert(free_ranges, free_base, used->base - 1);
+        }
+
+        free_base = used->limit + 1;
+    }
+
+    if (free_base < end) {
+        crs_range_insert(free_ranges, free_base, end);
+    }
+
+    g_ptr_array_set_size(ranges, 0);
+    for (i = 0; i < free_ranges->len; i++) {
+        g_ptr_array_add(ranges, g_ptr_array_index(free_ranges, i));
+    }
+
+    g_ptr_array_free(free_ranges, true);
+}
+
+/*
+ * crs_range_merge - merges adjacent ranges in the given array.
+ * Array elements are deleted and replaced with the merged ranges.
+ */
+static void crs_range_merge(GPtrArray *range)
+{
+    GPtrArray *tmp =  g_ptr_array_new_with_free_func(crs_range_free);
+    CrsRangeEntry *entry;
+    uint64_t range_base, range_limit;
+    int i;
+
+    if (!range->len) {
+        return;
+    }
+
+    g_ptr_array_sort(range, crs_range_compare);
+
+    entry = g_ptr_array_index(range, 0);
+    range_base = entry->base;
+    range_limit = entry->limit;
+    for (i = 1; i < range->len; i++) {
+        entry = g_ptr_array_index(range, i);
+        if (entry->base - 1 == range_limit) {
+            range_limit = entry->limit;
+        } else {
+            crs_range_insert(tmp, range_base, range_limit);
+            range_base = entry->base;
+            range_limit = entry->limit;
+        }
+    }
+    crs_range_insert(tmp, range_base, range_limit);
+
+    g_ptr_array_set_size(range, 0);
+    for (i = 0; i < tmp->len; i++) {
+        entry = g_ptr_array_index(tmp, i);
+        crs_range_insert(range, entry->base, entry->limit);
+    }
+    g_ptr_array_free(tmp, true);
+}
+
+Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
+{
+    Aml *crs = aml_resource_template();
+    CrsRangeSet temp_range_set;
+    CrsRangeEntry *entry;
+    uint8_t max_bus = pci_bus_num(host->bus);
+    uint8_t type;
+    int devfn;
+    int i;
+
+    crs_range_set_init(&temp_range_set);
+    for (devfn = 0; devfn < ARRAY_SIZE(host->bus->devices); devfn++) {
+        uint64_t range_base, range_limit;
+        PCIDevice *dev = host->bus->devices[devfn];
+
+        if (!dev) {
+            continue;
+        }
+
+        for (i = 0; i < PCI_NUM_REGIONS; i++) {
+            PCIIORegion *r = &dev->io_regions[i];
+
+            range_base = r->addr;
+            range_limit = r->addr + r->size - 1;
+
+            /*
+             * Work-around for old bioses
+             * that do not support multiple root buses
+             */
+            if (!range_base || range_base > range_limit) {
+                continue;
+            }
+
+            if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
+                crs_range_insert(temp_range_set.io_ranges,
+                                 range_base, range_limit);
+            } else { /* "memory" */
+                crs_range_insert(temp_range_set.mem_ranges,
+                                 range_base, range_limit);
+            }
+        }
+
+        type = dev->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION;
+        if (type == PCI_HEADER_TYPE_BRIDGE) {
+            uint8_t subordinate = dev->config[PCI_SUBORDINATE_BUS];
+            if (subordinate > max_bus) {
+                max_bus = subordinate;
+            }
+
+            range_base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_IO);
+            range_limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_IO);
+
+            /*
+             * Work-around for old bioses
+             * that do not support multiple root buses
+             */
+            if (range_base && range_base <= range_limit) {
+                crs_range_insert(temp_range_set.io_ranges,
+                                 range_base, range_limit);
+            }
+
+            range_base =
+                pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY);
+            range_limit =
+                pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY);
+
+            /*
+             * Work-around for old bioses
+             * that do not support multiple root buses
+             */
+            if (range_base && range_base <= range_limit) {
+                uint64_t length = range_limit - range_base + 1;
+                if (range_limit <= UINT32_MAX && length <= UINT32_MAX)  {
+                    crs_range_insert(temp_range_set.mem_ranges,
+                                     range_base, range_limit);
+                } else {
+                    crs_range_insert(temp_range_set.mem_64bit_ranges,
+                                     range_base, range_limit);
+                }
+            }
+
+            range_base =
+                pci_bridge_get_base(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
+            range_limit =
+                pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
+
+            /*
+             * Work-around for old bioses
+             * that do not support multiple root buses
+             */
+            if (range_base && range_base <= range_limit) {
+                uint64_t length = range_limit - range_base + 1;
+                if (range_limit <= UINT32_MAX && length <= UINT32_MAX) {
+                    crs_range_insert(temp_range_set.mem_ranges,
+                                     range_base, range_limit);
+                } else {
+                    crs_range_insert(temp_range_set.mem_64bit_ranges,
+                                     range_base, range_limit);
+                }
+            }
+        }
+    }
+
+    crs_range_merge(temp_range_set.io_ranges);
+    for (i = 0; i < temp_range_set.io_ranges->len; i++) {
+        entry = g_ptr_array_index(temp_range_set.io_ranges, i);
+        aml_append(crs,
+                   aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED,
+                               AML_POS_DECODE, AML_ENTIRE_RANGE,
+                               0, entry->base, entry->limit, 0,
+                               entry->limit - entry->base + 1));
+        crs_range_insert(range_set->io_ranges, entry->base, entry->limit);
+    }
+
+    crs_range_merge(temp_range_set.mem_ranges);
+    for (i = 0; i < temp_range_set.mem_ranges->len; i++) {
+        entry = g_ptr_array_index(temp_range_set.mem_ranges, i);
+        aml_append(crs,
+                   aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
+                                    AML_MAX_FIXED, AML_NON_CACHEABLE,
+                                    AML_READ_WRITE,
+                                    0, entry->base, entry->limit, 0,
+                                    entry->limit - entry->base + 1));
+        crs_range_insert(range_set->mem_ranges, entry->base, entry->limit);
+    }
+
+    crs_range_merge(temp_range_set.mem_64bit_ranges);
+    for (i = 0; i < temp_range_set.mem_64bit_ranges->len; i++) {
+        entry = g_ptr_array_index(temp_range_set.mem_64bit_ranges, i);
+        aml_append(crs,
+                   aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
+                                    AML_MAX_FIXED, AML_NON_CACHEABLE,
+                                    AML_READ_WRITE,
+                                    0, entry->base, entry->limit, 0,
+                                    entry->limit - entry->base + 1));
+        crs_range_insert(range_set->mem_64bit_ranges,
+                         entry->base, entry->limit);
+    }
+
+    crs_range_set_free(&temp_range_set);
+
+    aml_append(crs,
+        aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
+                            0,
+                            pci_bus_num(host->bus),
+                            max_bus,
+                            0,
+                            max_bus - pci_bus_num(host->bus) + 1));
+
+    return crs;
+}
+
+Aml *build_osc_method(void)
+{
+    Aml *if_ctx;
+    Aml *if_ctx2;
+    Aml *else_ctx;
+    Aml *method;
+    Aml *a_cwd1 = aml_name("CDW1");
+    Aml *a_ctrl = aml_local(0);
+
+    method = aml_method("_OSC", 4, AML_NOTSERIALIZED);
+    aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
+
+    if_ctx = aml_if(aml_equal(
+        aml_arg(0), aml_touuid("33DB4D5B-1FF7-401C-9657-7441C03DD766")));
+    aml_append(if_ctx, aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
+    aml_append(if_ctx, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
+
+    aml_append(if_ctx, aml_store(aml_name("CDW3"), a_ctrl));
+
+    /*
+     * Always allow native PME, AER (no dependencies)
+     * Allow SHPC (PCI bridges can have SHPC controller)
+     */
+    aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl));
+
+    if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1))));
+    /* Unknown revision */
+    aml_append(if_ctx2, aml_or(a_cwd1, aml_int(0x08), a_cwd1));
+    aml_append(if_ctx, if_ctx2);
+
+    if_ctx2 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), a_ctrl)));
+    /* Capabilities bits were masked */
+    aml_append(if_ctx2, aml_or(a_cwd1, aml_int(0x10), a_cwd1));
+    aml_append(if_ctx, if_ctx2);
+
+    /* Update DWORD3 in the buffer */
+    aml_append(if_ctx, aml_store(a_ctrl, aml_name("CDW3")));
+    aml_append(method, if_ctx);
+
+    else_ctx = aml_else();
+    /* Unrecognized UUID */
+    aml_append(else_ctx, aml_or(a_cwd1, aml_int(4), a_cwd1));
+    aml_append(method, else_ctx);
+
+    aml_append(method, aml_return(aml_arg(3)));
+    return method;
+}
+
+void
+build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
+{
+    AcpiTableMcfg *mcfg;
+    const char *sig;
+    int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
+
+    mcfg = acpi_data_push(table_data, len);
+    mcfg->allocation[0].address = cpu_to_le64(info->mcfg_base);
+    /* Only a single allocation so no need to play with segments */
+    mcfg->allocation[0].pci_segment = cpu_to_le16(0);
+    mcfg->allocation[0].start_bus_number = 0;
+    mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->mcfg_size - 1);
+
+    /* MCFG is used for ECAM which can be enabled or disabled by guest.
+     * To avoid table size changes (which create migration issues),
+     * always create the table even if there are no allocations,
+     * but set the signature to a reserved value in this case.
+     * ACPI spec requires OSPMs to ignore such tables.
+     */
+    if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
+        /* Reserved signature: ignored by OSPM */
+        sig = "QEMU";
+    } else {
+        sig = "MCFG";
+    }
+    build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL, NULL);
+}
+
+Aml *build_gsi_link_dev(const char *name, uint8_t uid, uint8_t gsi)
+{
+    Aml *dev;
+    Aml *crs;
+    Aml *method;
+    uint32_t irqs;
+
+    dev = aml_device("%s", name);
+    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C0F")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(uid)));
+
+    crs = aml_resource_template();
+    irqs = gsi;
+    aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
+                                  AML_SHARED, &irqs, 1));
+    aml_append(dev, aml_name_decl("_PRS", crs));
+
+    aml_append(dev, aml_name_decl("_CRS", crs));
+
+    /*
+     * _DIS can be no-op because the interrupt cannot be disabled.
+     */
+    method = aml_method("_DIS", 0, AML_NOTSERIALIZED);
+    aml_append(dev, method);
+
+    method = aml_method("_SRS", 1, AML_NOTSERIALIZED);
+    aml_append(dev, method);
+
+    return dev;
+}
+
+/**
+ * build_prt_entry:
+ * @link_name: link name for PCI route entry
+ *
+ * build AML package containing a PCI route entry for @link_name
+ */
+static Aml *build_prt_entry(const char *link_name)
+{
+    Aml *a_zero = aml_int(0);
+    Aml *pkg = aml_package(4);
+    aml_append(pkg, a_zero);
+    aml_append(pkg, a_zero);
+    aml_append(pkg, aml_name("%s", link_name));
+    aml_append(pkg, a_zero);
+    return pkg;
+}
+
+/*
+ * initialize_route - Initialize the interrupt routing rule
+ * through a specific LINK:
+ *  if (lnk_idx == idx)
+ *      route using link 'link_name'
+ */
+static Aml *initialize_route(Aml *route, const char *link_name,
+                             Aml *lnk_idx, int idx)
+{
+    Aml *if_ctx = aml_if(aml_equal(lnk_idx, aml_int(idx)));
+    Aml *pkg = build_prt_entry(link_name);
+
+    aml_append(if_ctx, aml_store(pkg, route));
+
+    return if_ctx;
+}
+
+/*
+ * build_prt - Define interrupt rounting rules
+ *
+ * Returns an array of 128 routes, one for each device,
+ * based on device location.
+ * The main goal is to equaly distribute the interrupts
+ * over the 4 existing ACPI links (works only for i440fx).
+ * The hash function is  (slot + pin) & 3 -> "LNK[D|A|B|C]".
+ *
+ */
+Aml *build_prt(bool is_pci0_prt)
+{
+    Aml *method, *while_ctx, *pin, *res;
+
+    method = aml_method("_PRT", 0, AML_NOTSERIALIZED);
+    res = aml_local(0);
+    pin = aml_local(1);
+    aml_append(method, aml_store(aml_package(128), res));
+    aml_append(method, aml_store(aml_int(0), pin));
+
+    /* while (pin < 128) */
+    while_ctx = aml_while(aml_lless(pin, aml_int(128)));
+    {
+        Aml *slot = aml_local(2);
+        Aml *lnk_idx = aml_local(3);
+        Aml *route = aml_local(4);
+
+        /* slot = pin >> 2 */
+        aml_append(while_ctx,
+                   aml_store(aml_shiftright(pin, aml_int(2), NULL), slot));
+        /* lnk_idx = (slot + pin) & 3 */
+        aml_append(while_ctx,
+            aml_store(aml_and(aml_add(pin, slot, NULL), aml_int(3), NULL),
+                      lnk_idx));
+
+        /* route[2] = "LNK[D|A|B|C]", selection based on pin % 3  */
+        aml_append(while_ctx, initialize_route(route, "LNKD", lnk_idx, 0));
+        if (is_pci0_prt) {
+            Aml *if_device_1, *if_pin_4, *else_pin_4;
+
+            /* device 1 is the power-management device, needs SCI */
+            if_device_1 = aml_if(aml_equal(lnk_idx, aml_int(1)));
+            {
+                if_pin_4 = aml_if(aml_equal(pin, aml_int(4)));
+                {
+                    aml_append(if_pin_4,
+                        aml_store(build_prt_entry("LNKS"), route));
+                }
+                aml_append(if_device_1, if_pin_4);
+                else_pin_4 = aml_else();
+                {
+                    aml_append(else_pin_4,
+                        aml_store(build_prt_entry("LNKA"), route));
+                }
+                aml_append(if_device_1, else_pin_4);
+            }
+            aml_append(while_ctx, if_device_1);
+        } else {
+            aml_append(while_ctx, initialize_route(route, "LNKA", lnk_idx, 1));
+        }
+        aml_append(while_ctx, initialize_route(route, "LNKB", lnk_idx, 2));
+        aml_append(while_ctx, initialize_route(route, "LNKC", lnk_idx, 3));
+
+        /* route[0] = 0x[slot]FFFF */
+        aml_append(while_ctx,
+            aml_store(aml_or(aml_shiftleft(slot, aml_int(16)), aml_int(0xFFFF),
+                             NULL),
+                      aml_index(route, aml_int(0))));
+        /* route[1] = pin & 3 */
+        aml_append(while_ctx,
+            aml_store(aml_and(pin, aml_int(3), NULL),
+                      aml_index(route, aml_int(1))));
+        /* res[pin] = route */
+        aml_append(while_ctx, aml_store(route, aml_index(res, pin)));
+        /* pin++ */
+        aml_append(while_ctx, aml_increment(pin));
+    }
+    aml_append(method, while_ctx);
+    /* return res*/
+    aml_append(method, aml_return(res));
+
+    return method;
+}
+
 /* Build rsdt table */
 void
 build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
hw/i386/acpi-build.c +3 -513

564 lines not on this page · raw message

include/hw/acpi/aml-build.h +25 -0
index b4345c95bd..d6dbb399a0 100644
@@ -3,6 +3,7 @@
 
 #include "hw/acpi/acpi-defs.h"
 #include "hw/acpi/bios-linker-loader.h"
+#include "hw/pci/pcie_host.h"
 
 /* Reserve RAM space for tables: add another order of magnitude. */
 #define ACPI_BUILD_TABLE_MAX_SIZE         0x200000
@@ -224,6 +225,21 @@ struct AcpiBuildTables {
     BIOSLinker *linker;
 } AcpiBuildTables;
 
+typedef struct AcpiMcfgInfo {
+    uint64_t mcfg_base;
+    uint32_t mcfg_size;
+} AcpiMcfgInfo;
+
+typedef struct CrsRangeEntry {
+    uint64_t base;
+    uint64_t limit;
+} CrsRangeEntry;
+
+typedef struct CrsRangeSet {
+    GPtrArray *io_ranges;
+    GPtrArray *mem_ranges;
+    GPtrArray *mem_64bit_ranges;
+} CrsRangeSet;
 /**
  * init_aml_allocator:
  *
@@ -389,6 +405,15 @@ void acpi_align_size(GArray *blob, unsigned align);
 void acpi_add_table(GArray *table_offsets, GArray *table_data);
 void acpi_build_tables_init(AcpiBuildTables *tables);
 void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre);
+Aml *build_osc_method(void);
+void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info);
+Aml *build_gsi_link_dev(const char *name, uint8_t uid, uint8_t gsi);
+Aml *build_prt(bool is_pci0_prt);
+void crs_range_set_init(CrsRangeSet *range_set);
+Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set);
+void crs_replace_with_free_ranges(GPtrArray *ranges,
+                                  uint64_t start, uint64_t end);
+void crs_range_set_free(CrsRangeSet *range_set);
 GArray *build_rsdp_rsdt(GArray *rsdp_table,
                         BIOSLinker *linker, unsigned rsdt_tbl_offset);
 GArray *build_rsdp(GArray *rsdp_table,
Signature · 1 line
2.17.2
#7 lore raw

PATCH 6/26 hw: acpi: Factorize _OSC AML across architectures

In reply to Samuel Ortiz

From: Yang Zhong <[email protected]>

The _OSC AML table is almost identical between the i386 Q35 and arm virt machine types. We can make it slightly more generic and share it across all PCIe architectures.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Shannon Zhao <[email protected]>
Cc Peter Maydell <[email protected]>
Cc Marcel Apfelbaum <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Richard Henderson <[email protected]>
Cc Eduardo Habkost <[email protected]>
---
 hw/acpi/aml-build.c         | 84 +++++++++++++++++++------------------
 hw/arm/virt-acpi-build.c    | 45 ++------------------
 hw/i386/acpi-build.c        |  6 ++-
 include/hw/acpi/acpi-defs.h | 14 +++++++
 include/hw/acpi/aml-build.h |  2 +-
 5 files changed, 66 insertions(+), 85 deletions(-)
hw/acpi/aml-build.c +44 -40
index 6daa0f6df2..1a9948f6a9 100644
@@ -1869,51 +1869,55 @@ Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
     return crs;
 }
 
-Aml *build_osc_method(void)
+/*
+ * ctrl_mask is the _OSC capabilities buffer control field mask.
+ */
+Aml *build_osc_method(uint32_t ctrl_mask)
 {
-    Aml *if_ctx;
-    Aml *if_ctx2;
-    Aml *else_ctx;
-    Aml *method;
-    Aml *a_cwd1 = aml_name("CDW1");
-    Aml *a_ctrl = aml_local(0);
+    Aml *ifctx, *ifctx1, *elsectx, *method, *UUID;
 
     method = aml_method("_OSC", 4, AML_NOTSERIALIZED);
-    aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
-
-    if_ctx = aml_if(aml_equal(
-        aml_arg(0), aml_touuid("33DB4D5B-1FF7-401C-9657-7441C03DD766")));
-    aml_append(if_ctx, aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
-    aml_append(if_ctx, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
-
-    aml_append(if_ctx, aml_store(aml_name("CDW3"), a_ctrl));
-
-    /*
-     * Always allow native PME, AER (no dependencies)
-     * Allow SHPC (PCI bridges can have SHPC controller)
+    aml_append(method,
+        aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
+
+    /* PCI Firmware Specification 3.0
+     * 4.5.1. _OSC Interface for PCI Host Bridge Devices
+     * The _OSC interface for a PCI/PCI-X/PCI Express hierarchy is
+     * identified by the Universal Unique IDentifier (UUID)
+     * 33DB4D5B-1FF7-401C-9657-7441C03DD766
      */
-    aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl));
-
-    if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1))));
-    /* Unknown revision */
-    aml_append(if_ctx2, aml_or(a_cwd1, aml_int(0x08), a_cwd1));
-    aml_append(if_ctx, if_ctx2);
-
-    if_ctx2 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), a_ctrl)));
-    /* Capabilities bits were masked */
-    aml_append(if_ctx2, aml_or(a_cwd1, aml_int(0x10), a_cwd1));
-    aml_append(if_ctx, if_ctx2);
-
-    /* Update DWORD3 in the buffer */
-    aml_append(if_ctx, aml_store(a_ctrl, aml_name("CDW3")));
-    aml_append(method, if_ctx);
-
-    else_ctx = aml_else();
-    /* Unrecognized UUID */
-    aml_append(else_ctx, aml_or(a_cwd1, aml_int(4), a_cwd1));
-    aml_append(method, else_ctx);
+    UUID = aml_touuid("33DB4D5B-1FF7-401C-9657-7441C03DD766");
+    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
+    aml_append(ifctx,
+        aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
+    aml_append(ifctx,
+        aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
+    aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
+    aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
+    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"),
+                                        aml_int(ctrl_mask), NULL),
+                                aml_name("CTRL")));
+
+    ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1))));
+    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL),
+                                 aml_name("CDW1")));
+    aml_append(ifctx, ifctx1);
+
+    ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
+    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL),
+                                 aml_name("CDW1")));
+    aml_append(ifctx, ifctx1);
+
+    aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3")));
+    aml_append(ifctx, aml_return(aml_arg(3)));
+    aml_append(method, ifctx);
+
+    elsectx = aml_else();
+    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL),
+                                  aml_name("CDW1")));
+    aml_append(elsectx, aml_return(aml_arg(3)));
+    aml_append(method, elsectx);
 
-    aml_append(method, aml_return(aml_arg(3)));
     return method;
 }
 
hw/arm/virt-acpi-build.c +3 -42

57 lines not on this page · raw message

hw/i386/acpi-build.c +4 -2
index 3ab68fd24d..d3cb1b439d 100644
@@ -1337,7 +1337,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
         aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
         aml_append(dev, aml_name_decl("_UID", aml_int(1)));
-        aml_append(dev, build_osc_method());
+        aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
+        aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
+        aml_append(dev, build_osc_method(ACPI_OSC_CTRL_PCI_ALL));
         aml_append(sb_scope, dev);
         aml_append(dsdt, sb_scope);
 
@@ -1402,7 +1404,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
             aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
             aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
             if (pci_bus_is_express(bus)) {
-                aml_append(dev, build_osc_method());
+                aml_append(dev, build_osc_method(0x1F));
             }
 
             if (numa_node != NUMA_NODE_UNASSIGNED) {
include/hw/acpi/acpi-defs.h +14 -0
index af8e023968..6e1726e0a2 100644
@@ -652,4 +652,18 @@ struct AcpiIortRC {
 } QEMU_PACKED;
 typedef struct AcpiIortRC AcpiIortRC;
 
+/* _OSC */
+
+#define ACPI_OSC_CTRL_PCIE_NATIVE_HP (1 << 0)
+#define ACPI_OSC_CTRL_SHPC_NATIVE_HP (1 << 1)
+#define ACPI_OSC_CTRL_PCIE_PM_EVT    (1 << 2)
+#define ACPI_OSC_CTRL_PCIE_AER       (1 << 3)
+#define ACPI_OSC_CTRL_PCIE_CAP_CTRL  (1 << 4)
+#define ACPI_OSC_CTRL_PCI_ALL \
+    (ACPI_OSC_CTRL_PCIE_NATIVE_HP |             \
+     ACPI_OSC_CTRL_SHPC_NATIVE_HP |             \
+     ACPI_OSC_CTRL_PCIE_PM_EVT |                \
+     ACPI_OSC_CTRL_PCIE_AER |                   \
+     ACPI_OSC_CTRL_PCIE_CAP_CTRL)
+
 #endif
include/hw/acpi/aml-build.h +1 -1
index d6dbb399a0..a60589d70f 100644
@@ -405,7 +405,7 @@ void acpi_align_size(GArray *blob, unsigned align);
 void acpi_add_table(GArray *table_offsets, GArray *table_data);
 void acpi_build_tables_init(AcpiBuildTables *tables);
 void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre);
-Aml *build_osc_method(void);
+Aml *build_osc_method(uint32_t value);
 void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info);
 Aml *build_gsi_link_dev(const char *name, uint8_t uid, uint8_t gsi);
 Aml *build_prt(bool is_pci0_prt);
Signature · 1 line
2.17.2
#8 lore raw

PATCH 7/26 hw: i386: Refactor PCI host getter

In reply to Samuel Ortiz

From: Yang Zhong <[email protected]>

Make it more flexible by having it parsing a PCI host paths array instead of open coding those paths deep down into the code logic itself. This will be needed for PCI machine types that are neither emulatiing the ich9 nor the i440fx chipsets.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Richard Henderson <[email protected]>
Cc Eduardo Habkost <[email protected]>
Cc Marcel Apfelbaum <[email protected]>
---
 hw/i386/acpi-build.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)
hw/i386/acpi-build.c +19 -10
index d3cb1b439d..6652880c9b 100644
@@ -114,6 +114,12 @@ typedef struct AcpiBuildPciBusHotplugState {
     bool pcihp_bridge_en;
 } AcpiBuildPciBusHotplugState;
 
+static const char *pci_hosts[] = {
+   "/machine/i440fx",
+   "/machine/q35",
+   NULL,
+};
+
 static void init_common_fadt_data(Object *o, AcpiFadtData *data)
 {
     uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
@@ -238,27 +244,30 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
  * Because of the PXB hosts we cannot simply query TYPE_PCI_HOST_BRIDGE.
  * On i386 arch we only have two pci hosts, so we can look only for them.
  */
-static Object *acpi_get_i386_pci_host(void)
+static Object *acpi_get_pci_host(void)
 {
     PCIHostState *host;
+    int i = 0;
 
-    host = OBJECT_CHECK(PCIHostState,
-                        object_resolve_path("/machine/i440fx", NULL),
-                        TYPE_PCI_HOST_BRIDGE);
-    if (!host) {
+    while (pci_hosts[i]) {
         host = OBJECT_CHECK(PCIHostState,
-                            object_resolve_path("/machine/q35", NULL),
+                            object_resolve_path(pci_hosts[i], NULL),
                             TYPE_PCI_HOST_BRIDGE);
+        if (host) {
+            return OBJECT(host);
+        }
+
+        i++;
     }
 
-    return OBJECT(host);
+    return NULL;
 }
 
 static void acpi_get_pci_holes(Range *hole, Range *hole64)
 {
     Object *pci_host;
 
-    pci_host = acpi_get_i386_pci_host();
+    pci_host = acpi_get_pci_host();
     g_assert(pci_host);
 
     range_set_bounds1(hole,
@@ -1636,7 +1645,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         Object *pci_host;
         PCIBus *bus = NULL;
 
-        pci_host = acpi_get_i386_pci_host();
+        pci_host = acpi_get_pci_host();
         if (pci_host) {
             bus = PCI_HOST_BRIDGE(pci_host)->bus;
         }
@@ -2009,7 +2018,7 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
     Object *pci_host;
     QObject *o;
 
-    pci_host = acpi_get_i386_pci_host();
+    pci_host = acpi_get_pci_host();
     g_assert(pci_host);
 
     o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_BASE, NULL);
Signature · 1 line
2.17.2
#9 lore raw

PATCH 8/26 hw: acpi: Export and generalize the PCI host AML API

In reply to Samuel Ortiz

From: Yang Zhong <[email protected]>

The AML build routines for the PCI host bridge and the corresponding DSDT addition are neither x86 nor PC machine type specific. We can move them to the architecture agnostic hw/acpi folder, and by carrying all the needed information through a new AcpiPciBus structure, we can make them PC machine type independent.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Marcel Apfelbaum <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Richard Henderson <[email protected]>
Cc Eduardo Habkost <[email protected]>
---
 hw/acpi/aml-build.c         | 208 ++++++++++++++++++++++++++++++++++++
 hw/i386/acpi-build.c        | 167 ++---------------------------
 include/hw/acpi/aml-build.h |  10 ++
 3 files changed, 226 insertions(+), 159 deletions(-)
hw/acpi/aml-build.c +208 -0
index 1a9948f6a9..3e0511bd46 100644
@@ -29,6 +29,25 @@
 #include "hw/pci/pci_bus.h"
 #include "qemu/range.h"
 #include "hw/pci/pci_bridge.h"
+#include "hw/i386/pc.h"
+#include "sysemu/tpm.h"
+#include "hw/acpi/tpm.h"
+
+#define PCI_HOST_BRIDGE_CONFIG_ADDR        0xcf8
+#define PCI_HOST_BRIDGE_IO_0_MIN_ADDR      0x0000
+#define PCI_HOST_BRIDGE_IO_0_MAX_ADDR      0x0cf7
+#define PCI_HOST_BRIDGE_IO_1_MIN_ADDR      0x0d00
+#define PCI_HOST_BRIDGE_IO_1_MAX_ADDR      0xffff
+#define PCI_VGA_MEM_BASE_ADDR              0x000a0000
+#define PCI_VGA_MEM_MAX_ADDR               0x000bffff
+#define IO_0_LEN                           0xcf8
+#define VGA_MEM_LEN                        0x20000
+
+static const char *pci_hosts[] = {
+   "/machine/i440fx",
+   "/machine/q35",
+   NULL,
+};
 
 static GArray *build_alloc_array(void)
 {
@@ -1601,6 +1620,51 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre)
     g_array_free(tables->vmgenid, mfre);
 }
 
+/*
+ * Because of the PXB hosts we cannot simply query TYPE_PCI_HOST_BRIDGE.
+ */
+Object *acpi_get_pci_host(void)
+{
+    PCIHostState *host;
+    int i = 0;
+
+    while (pci_hosts[i]) {
+        host = OBJECT_CHECK(PCIHostState,
+                            object_resolve_path(pci_hosts[i], NULL),
+                            TYPE_PCI_HOST_BRIDGE);
+        if (host) {
+            return OBJECT(host);
+        }
+
+        i++;
+    }
+
+    return NULL;
+}
+
+void acpi_get_pci_holes(Range *hole, Range *hole64)
+{
+    Object *pci_host;
+
+    pci_host = acpi_get_pci_host();
+    g_assert(pci_host);
+
+    range_set_bounds1(hole,
+                      object_property_get_uint(pci_host,
+                                               PCI_HOST_PROP_PCI_HOLE_START,
+                                               NULL),
+                      object_property_get_uint(pci_host,
+                                               PCI_HOST_PROP_PCI_HOLE_END,
+                                               NULL));
+    range_set_bounds1(hole64,
+                      object_property_get_uint(pci_host,
+                                               PCI_HOST_PROP_PCI_HOLE64_START,
+                                               NULL),
+                      object_property_get_uint(pci_host,
+                                               PCI_HOST_PROP_PCI_HOLE64_END,
+                                               NULL));
+}
+
 static void crs_range_insert(GPtrArray *ranges, uint64_t base, uint64_t limit)
 {
     CrsRangeEntry *entry;
@@ -2099,6 +2163,150 @@ Aml *build_prt(bool is_pci0_prt)
     return method;
 }
 
+Aml *build_pci_host_bridge(Aml *table, AcpiPciBus *pci_host)
+{
+    CrsRangeEntry *entry;
+    Aml *scope, *dev, *crs;
+    CrsRangeSet crs_range_set;
+    Range *pci_hole = NULL;
+    Range *pci_hole64 = NULL;
+    PCIBus *bus = NULL;
+    int root_bus_limit = 0xFF;
+    int i;
+
+    bus = pci_host->pci_bus;
+    assert(bus);
+    pci_hole = pci_host->pci_hole;
+    pci_hole64 = pci_host->pci_hole64;
+
+    crs_range_set_init(&crs_range_set);
+    QLIST_FOREACH(bus, &bus->child, sibling) {
+        uint8_t bus_num = pci_bus_num(bus);
+        uint8_t numa_node = pci_bus_numa_node(bus);
+
+        /* look only for expander root buses */
+        if (!pci_bus_is_root(bus)) {
+            continue;
+        }
+
+        if (bus_num < root_bus_limit) {
+            root_bus_limit = bus_num - 1;
+        }
+
+        scope = aml_scope("\\_SB");
+        dev = aml_device("PC%.02X", bus_num);
+        aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
+        aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
+        aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
+        if (pci_bus_is_express(bus)) {
+            aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
+            aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
+            aml_append(dev, build_osc_method(0x1F));
+        }
+        if (numa_node != NUMA_NODE_UNASSIGNED) {
+            aml_append(dev, aml_name_decl("_PXM", aml_int(numa_node)));
+        }
+
+        aml_append(dev, build_prt(false));
+        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(table, scope);
+    }
+    scope = aml_scope("\\_SB.PCI0");
+    /* build PCI0._CRS */
+    crs = aml_resource_template();
+    /* set the pcie bus num */
+    aml_append(crs,
+        aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
+                            0x0000, 0x0, root_bus_limit,
+                            0x0000, root_bus_limit + 1));
+    aml_append(crs, aml_io(AML_DECODE16, PCI_HOST_BRIDGE_CONFIG_ADDR,
+                           PCI_HOST_BRIDGE_CONFIG_ADDR, 0x01, 0x08));
+    /* set the io region 0 in pci host bridge */
+    aml_append(crs,
+        aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED,
+                    AML_POS_DECODE, AML_ENTIRE_RANGE,
+                    0x0000, PCI_HOST_BRIDGE_IO_0_MIN_ADDR,
+                    PCI_HOST_BRIDGE_IO_0_MAX_ADDR, 0x0000, IO_0_LEN));
+
+    /* set the io region 1 in pci host bridge */
+    crs_replace_with_free_ranges(crs_range_set.io_ranges,
+                                 PCI_HOST_BRIDGE_IO_1_MIN_ADDR,
+                                 PCI_HOST_BRIDGE_IO_1_MAX_ADDR);
+    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,
+                        0x0000, entry->base, entry->limit,
+                        0x0000, entry->limit - entry->base + 1));
+    }
+
+    /* set the vga mem region(0) in pci host bridge */
+    aml_append(crs,
+        aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
+                         AML_CACHEABLE, AML_READ_WRITE,
+                         0, PCI_VGA_MEM_BASE_ADDR, PCI_VGA_MEM_MAX_ADDR,
+                         0, VGA_MEM_LEN));
+
+    /* set the mem region 1 in pci host bridge */
+    crs_replace_with_free_ranges(crs_range_set.mem_ranges,
+                                 range_lob(pci_hole),
+                                 range_upb(pci_hole));
+    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,
+                             0, entry->base, entry->limit,
+                             0, entry->limit - entry->base + 1));
+    }
+
+    /* set the mem region 2 in pci host bridge */
+    if (!range_is_empty(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 (TPM_IS_TIS(tpm_find())) {
+        aml_append(crs, aml_memory32_fixed(TPM_TIS_ADDR_BASE,
+                   TPM_TIS_ADDR_SIZE, AML_READ_WRITE));
+    }
+
+    aml_append(scope, aml_name_decl("_CRS", crs));
+    crs_range_set_free(&crs_range_set);
+    return scope;
+}
+
+void acpi_dsdt_add_pci_bus(Aml *dsdt, AcpiPciBus *pci_host)
+{
+    Aml *dev, *pci_scope;
+
+    dev = aml_device("\\_SB.PCI0");
+    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
+    aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
+    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
+    aml_append(dev, aml_name_decl("_UID", aml_int(1)));
+    aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
+    aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
+    aml_append(dev, build_osc_method(0x1F));
+    aml_append(dsdt, dev);
+
+    pci_scope = build_pci_host_bridge(dsdt, pci_host);
+    aml_append(dsdt, pci_scope);
+}
+
 /* Build rsdt table */
 void
 build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
hw/i386/acpi-build.c +8 -159

213 lines not on this page · raw message

include/hw/acpi/aml-build.h +10 -0
index a60589d70f..6647fa8791 100644
@@ -230,6 +230,12 @@ typedef struct AcpiMcfgInfo {
     uint32_t mcfg_size;
 } AcpiMcfgInfo;
 
+typedef struct AcpiPciBus {
+    PCIBus *pci_bus;
+    Range *pci_hole;
+    Range *pci_hole64;
+} AcpiPciBus;
+
 typedef struct CrsRangeEntry {
     uint64_t base;
     uint64_t limit;
@@ -401,6 +407,8 @@ build_header(BIOSLinker *linker, GArray *table_data,
              const char *oem_id, const char *oem_table_id);
 void *acpi_data_push(GArray *table_data, unsigned size);
 unsigned acpi_data_len(GArray *table);
+Object *acpi_get_pci_host(void);
+void acpi_get_pci_holes(Range *hole, Range *hole64);
 void acpi_align_size(GArray *blob, unsigned align);
 void acpi_add_table(GArray *table_offsets, GArray *table_data);
 void acpi_build_tables_init(AcpiBuildTables *tables);
@@ -409,6 +417,8 @@ Aml *build_osc_method(uint32_t value);
 void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info);
 Aml *build_gsi_link_dev(const char *name, uint8_t uid, uint8_t gsi);
 Aml *build_prt(bool is_pci0_prt);
+void acpi_dsdt_add_pci_bus(Aml *dsdt, AcpiPciBus *pci_host);
+Aml *build_pci_host_bridge(Aml *table, AcpiPciBus *pci_host);
 void crs_range_set_init(CrsRangeSet *range_set);
 Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set);
 void crs_replace_with_free_ranges(GPtrArray *ranges,
Signature · 1 line
2.17.2
#10 lore raw

PATCH 9/26 hw: acpi: Export the MCFG getter

In reply to Samuel Ortiz

From: Yang Zhong <[email protected]>

The ACPI MCFG getter is not x86 specific and could be called from anywhere within generic ACPI API, so let's export it.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Richard Henderson <[email protected]>
Cc Eduardo Habkost <[email protected]>
Cc Marcel Apfelbaum <[email protected]>
---
 hw/acpi/aml-build.c         | 24 ++++++++++++++++++++++++
 hw/i386/acpi-build.c        | 22 ----------------------
 include/hw/acpi/aml-build.h |  1 +
 3 files changed, 25 insertions(+), 22 deletions(-)
hw/acpi/aml-build.c +24 -0
index 3e0511bd46..0d21c854a6 100644
@@ -32,6 +32,8 @@
 #include "hw/i386/pc.h"
 #include "sysemu/tpm.h"
 #include "hw/acpi/tpm.h"
+#include "qom/qom-qobject.h"
+#include "qapi/qmp/qnum.h"
 
 #define PCI_HOST_BRIDGE_CONFIG_ADDR        0xcf8
 #define PCI_HOST_BRIDGE_IO_0_MIN_ADDR      0x0000
@@ -1665,6 +1667,28 @@ void acpi_get_pci_holes(Range *hole, Range *hole64)
                                                NULL));
 }
 
+bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
+{
+    Object *pci_host;
+    QObject *o;
+
+    pci_host = acpi_get_pci_host();
+    g_assert(pci_host);
+
+    o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_BASE, NULL);
+    if (!o) {
+        return false;
+    }
+    mcfg->mcfg_base = qnum_get_uint(qobject_to(QNum, o));
+    qobject_unref(o);
+
+    o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_SIZE, NULL);
+    assert(o);
+    mcfg->mcfg_size = qnum_get_uint(qobject_to(QNum, o));
+    qobject_unref(o);
+    return true;
+}
+
 static void crs_range_insert(GPtrArray *ranges, uint64_t base, uint64_t limit)
 {
     CrsRangeEntry *entry;
hw/i386/acpi-build.c +0 -22
index 12a8d8210a..414a6c4c4e 100644
@@ -1857,28 +1857,6 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
                  "IVRS", table_data->len - iommu_start, 1, NULL, NULL);
 }
 
-static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
-{
-    Object *pci_host;
-    QObject *o;
-
-    pci_host = acpi_get_pci_host();
-    g_assert(pci_host);
-
-    o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_BASE, NULL);
-    if (!o) {
-        return false;
-    }
-    mcfg->mcfg_base = qnum_get_uint(qobject_to(QNum, o));
-    qobject_unref(o);
-
-    o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_SIZE, NULL);
-    assert(o);
-    mcfg->mcfg_size = qnum_get_uint(qobject_to(QNum, o));
-    qobject_unref(o);
-    return true;
-}
-
 static
 void acpi_build(AcpiBuildTables *tables,
                 MachineState *machine, AcpiConfiguration *conf)
include/hw/acpi/aml-build.h +1 -0
index 6647fa8791..befe260251 100644
@@ -409,6 +409,7 @@ void *acpi_data_push(GArray *table_data, unsigned size);
 unsigned acpi_data_len(GArray *table);
 Object *acpi_get_pci_host(void);
 void acpi_get_pci_holes(Range *hole, Range *hole64);
+bool acpi_get_mcfg(AcpiMcfgInfo *mcfg);
 void acpi_align_size(GArray *blob, unsigned align);
 void acpi_add_table(GArray *table_offsets, GArray *table_data);
 void acpi_build_tables_init(AcpiBuildTables *tables);
Signature · 1 line
2.17.2
#11 lore raw

PATCH 10/26 hw: acpi: Do not create hotplug method when handler is not defined

In reply to Samuel Ortiz

CPU and memory ACPI hotplug are not necessarily handled through SCI events. For example, with Hardware-reduced ACPI, the GED device will manage ACPI hotplug entirely.
As a consequence, we make the CPU and memory specific events AML generation optional. The code will only be added when the method name is not NULL.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
---
 hw/acpi/cpu.c            |  8 +++++---
 hw/acpi/memory_hotplug.c | 11 +++++++----
 2 files changed, 12 insertions(+), 7 deletions(-)
hw/acpi/cpu.c +5 -3
index f10b190019..cd41377b5a 100644
@@ -569,9 +569,11 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
     aml_append(sb_scope, cpus_dev);
     aml_append(table, sb_scope);
 
-    method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
-    aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
-    aml_append(table, method);
+    if (event_handler_method) {
+        method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
+        aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
+        aml_append(table, method);
+    }
 
     g_free(cphp_res_path);
 }
hw/acpi/memory_hotplug.c +7 -4
index 8c7c1013f3..db2c4df961 100644
@@ -715,10 +715,13 @@ void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
     }
     aml_append(table, dev_container);
 
-    method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
-    aml_append(method,
-        aml_call0(MEMORY_DEVICES_CONTAINER "." MEMORY_SLOT_SCAN_METHOD));
-    aml_append(table, method);
+    if (event_handler_method) {
+        method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
+        aml_append(method,
+                   aml_call0(MEMORY_DEVICES_CONTAINER "."
+                             MEMORY_SLOT_SCAN_METHOD));
+        aml_append(table, method);
+    }
 
     g_free(mhp_res_path);
 }
Signature · 1 line
2.17.2
#12 lore raw

PATCH 11/26 hw: i386: Make the hotpluggable memory size property more generic

In reply to Samuel Ortiz

This property is currently defined under i386/pc while it only describes a region size that's eventually fetched from the AML ACPI code.

We can make it more generic and shareable across machine types by moving it to memory-device.h instead.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Marcel Apfelbaum <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Richard Henderson <[email protected]>
Cc Eduardo Habkost <[email protected]>
---
 hw/i386/acpi-build.c           | 2 +-
 hw/i386/pc.c                   | 3 ++-
 include/hw/i386/pc.h           | 1 -
 include/hw/mem/memory-device.h | 2 ++
 4 files changed, 5 insertions(+), 3 deletions(-)
hw/i386/acpi-build.c +1 -1
index 414a6c4c4e..dfc02a8a85 100644
@@ -1630,7 +1630,7 @@ build_srat(GArray *table_data, BIOSLinker *linker,
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
     ram_addr_t hotplugabble_address_space_size =
-        object_property_get_int(OBJECT(machine), PC_MACHINE_DEVMEM_REGION_SIZE,
+        object_property_get_int(OBJECT(machine), MEMORY_DEVICE_REGION_SIZE,
                                 NULL);
 
     srat_start = table_data->len;
hw/i386/pc.c +2 -1
index 940485a728..fa12583096 100644
@@ -67,6 +67,7 @@
 #include "hw/boards.h"
 #include "acpi-build.h"
 #include "hw/mem/pc-dimm.h"
+#include "hw/mem/memory-device.h"
 #include "qapi/error.h"
 #include "qapi/qapi-visit-common.h"
 #include "qapi/visitor.h"
@@ -2442,7 +2443,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     nc->nmi_monitor_handler = x86_nmi;
     mc->default_cpu_type = TARGET_DEFAULT_CPU_TYPE;
 
-    object_class_property_add(oc, PC_MACHINE_DEVMEM_REGION_SIZE, "int",
+    object_class_property_add(oc, MEMORY_DEVICE_REGION_SIZE, "int",
         pc_machine_get_device_memory_region_size, NULL,
         NULL, NULL, &error_abort);
 
include/hw/i386/pc.h +0 -1
index da0bd39741..7d177cd207 100644
@@ -62,7 +62,6 @@ struct PCMachineState {
 };
 
 #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
-#define PC_MACHINE_DEVMEM_REGION_SIZE "device-memory-region-size"
 #define PC_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g"
 #define PC_MACHINE_VMPORT           "vmport"
 #define PC_MACHINE_SMM              "smm"
include/hw/mem/memory-device.h +2 -0
index 2853b084b5..fc39fbe38f 100644
@@ -39,6 +39,8 @@ typedef struct MemoryDeviceClass {
                              MemoryDeviceInfo *info);
 } MemoryDeviceClass;
 
+#define MEMORY_DEVICE_REGION_SIZE "memory-device-region-size"
+
 MemoryDeviceInfoList *qmp_memory_device_list(void);
 uint64_t get_plugged_memory_size(void);
 uint64_t memory_device_get_free_addr(MachineState *ms, const uint64_t *hint,
Signature · 1 line
2.17.2
#13 lore raw

PATCH 12/26 hw: acpi: Export the SRAT AML build API

In reply to Samuel Ortiz

From: Yang Zhong <[email protected]>

The SRAT ACPI table is not x86 specific and will be needed for the Hardware-reduced ACPI implementation. So we should export it through the architecture independent hw/acpi folder.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Richard Henderson <[email protected]>
Cc Eduardo Habkost <[email protected]>
Cc Marcel Apfelbaum <[email protected]>
---
 hw/acpi/aml-build.c         | 130 ++++++++++++++++++++++++++++++++++++
 hw/i386/acpi-build.c        | 129 -----------------------------------
 include/hw/acpi/aml-build.h |   3 +
 3 files changed, 133 insertions(+), 129 deletions(-)
hw/acpi/aml-build.c +130 -0
index 0d21c854a6..cb706ca975 100644
@@ -22,6 +22,7 @@
 #include "qemu/osdep.h"
 #include <glib/gprintf.h>
 #include "hw/acpi/aml-build.h"
+#include "hw/mem/memory-device.h"
 #include "qemu/bswap.h"
 #include "qemu/bitops.h"
 #include "sysemu/numa.h"
@@ -2461,6 +2462,135 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
     numamem->range_length = cpu_to_le64(len);
 }
 
+#define HOLE_640K_START  (640 * KiB)
+#define HOLE_640K_END   (1 * MiB)
+
+void
+build_srat(GArray *table_data, BIOSLinker *linker,
+           MachineState *machine, AcpiConfiguration *conf)
+{
+    AcpiSystemResourceAffinityTable *srat;
+    AcpiSratMemoryAffinity *numamem;
+
+    int i;
+    int srat_start, numa_start, slots;
+    uint64_t mem_len, mem_base, next_base;
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
+    const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
+    ram_addr_t hotplugabble_address_space_size =
+        object_property_get_int(OBJECT(machine), MEMORY_DEVICE_REGION_SIZE,
+                                NULL);
+
+    srat_start = table_data->len;
+
+    srat = acpi_data_push(table_data, sizeof *srat);
+    srat->reserved1 = cpu_to_le32(1);
+
+    for (i = 0; i < apic_ids->len; i++) {
+        int node_id = apic_ids->cpus[i].props.node_id;
+        uint32_t apic_id = apic_ids->cpus[i].arch_id;
+
+        if (apic_id < 255) {
+            AcpiSratProcessorAffinity *core;
+
+            core = acpi_data_push(table_data, sizeof *core);
+            core->type = ACPI_SRAT_PROCESSOR_APIC;
+            core->length = sizeof(*core);
+            core->local_apic_id = apic_id;
+            core->proximity_lo = node_id;
+            memset(core->proximity_hi, 0, 3);
+            core->local_sapic_eid = 0;
+            core->flags = cpu_to_le32(1);
+        } else {
+            AcpiSratProcessorX2ApicAffinity *core;
+
+            core = acpi_data_push(table_data, sizeof *core);
+            core->type = ACPI_SRAT_PROCESSOR_x2APIC;
+            core->length = sizeof(*core);
+            core->x2apic_id = cpu_to_le32(apic_id);
+            core->proximity_domain = cpu_to_le32(node_id);
+            core->flags = cpu_to_le32(1);
+        }
+    }
+
+
+    /* the memory map is a bit tricky, it contains at least one hole
+     * from 640k-1M and possibly another one from 3.5G-4G.
+     */
+    next_base = 0;
+    numa_start = table_data->len;
+
+    for (i = 1; i < conf->numa_nodes + 1; ++i) {
+        mem_base = next_base;
+        mem_len = conf->node_mem[i - 1];
+        next_base = mem_base + mem_len;
+
+        /* Cut out the 640K hole */
+        if (mem_base <= HOLE_640K_START &&
+            next_base > HOLE_640K_START) {
+            mem_len -= next_base - HOLE_640K_START;
+            if (mem_len > 0) {
+                numamem = acpi_data_push(table_data, sizeof *numamem);
+                build_srat_memory(numamem, mem_base, mem_len, i - 1,
+                                  MEM_AFFINITY_ENABLED);
+            }
+
+            /* Check for the rare case: 640K < RAM < 1M */
+            if (next_base <= HOLE_640K_END) {
+                next_base = HOLE_640K_END;
+                continue;
+            }
+            mem_base = HOLE_640K_END;
+            mem_len = next_base - HOLE_640K_END;
+        }
+
+        /* Cut out the ACPI_PCI hole */
+        if (mem_base <= conf->below_4g_mem_size &&
+            next_base > conf->below_4g_mem_size) {
+            mem_len -= next_base - conf->below_4g_mem_size;
+            if (mem_len > 0) {
+                numamem = acpi_data_push(table_data, sizeof *numamem);
+                build_srat_memory(numamem, mem_base, mem_len, i - 1,
+                                  MEM_AFFINITY_ENABLED);
+            }
+            mem_base = 1ULL << 32;
+            mem_len = next_base - conf->below_4g_mem_size;
+            next_base = mem_base + mem_len;
+        }
+
+        if (mem_len > 0) {
+            numamem = acpi_data_push(table_data, sizeof *numamem);
+            build_srat_memory(numamem, mem_base, mem_len, i - 1,
+                              MEM_AFFINITY_ENABLED);
+        }
+    }
+    slots = (table_data->len - numa_start) / sizeof *numamem;
+    for (; slots < conf->numa_nodes + 2; slots++) {
+        numamem = acpi_data_push(table_data, sizeof *numamem);
+        build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
+    }
+
+    /*
+     * Entry is required for Windows to enable memory hotplug in OS
+     * and for Linux to enable SWIOTLB when booted with less than
+     * 4G of RAM. Windows works better if the entry sets proximity
+     * to the highest NUMA node in the machine.
+     * Memory devices may override proximity set by this entry,
+     * providing _PXM method if necessary.
+     */
+    if (hotplugabble_address_space_size) {
+        numamem = acpi_data_push(table_data, sizeof *numamem);
+        build_srat_memory(numamem, machine->device_memory->base,
+                          hotplugabble_address_space_size, conf->numa_nodes - 1,
+                          MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
+    }
+
+    build_header(linker, table_data,
+                 (void *)(table_data->data + srat_start),
+                 "SRAT",
+                 table_data->len - srat_start, 1, NULL, NULL);
+}
+
 /*
  * ACPI spec 5.2.17 System Locality Distance Information Table
  * (Revision 2.0 or later)
hw/i386/acpi-build.c +0 -129

135 lines not on this page · raw message

include/hw/acpi/aml-build.h +3 -0
index befe260251..45dcea711b 100644
@@ -1,6 +1,7 @@
 #ifndef HW_ACPI_AML_BUILD_H
 #define HW_ACPI_AML_BUILD_H
 
+#include "hw/acpi/acpi.h"
 #include "hw/acpi/acpi-defs.h"
 #include "hw/acpi/bios-linker-loader.h"
 #include "hw/pci/pcie_host.h"
@@ -436,6 +437,8 @@ void
 build_xsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
            const char *oem_id, const char *oem_table_id);
 
+void build_srat(GArray *table_data, BIOSLinker *linker,
+                MachineState *machine, AcpiConfiguration *conf);
 int
 build_append_named_dword(GArray *array, const char *name_format, ...)
 GCC_FMT_ATTR(2, 3);
Signature · 1 line
2.17.2
#14 lore raw

PATCH 13/26 hw: acpi: Fix memory hotplug AML generation error

In reply to Samuel Ortiz

From: Yang Zhong <[email protected]>

When using the generated memory hotplug AML, the iasl compiler would give the following error:

dsdt.dsl 266: Return (MOST (_UID, Arg0, Arg1, Arg2)) Error 6080 - Called method returns no value ^

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
---
 hw/acpi/memory_hotplug.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
hw/acpi/memory_hotplug.c +5 -5
index db2c4df961..893fc2bd27 100644
@@ -686,15 +686,15 @@ void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
 
             method = aml_method("_OST", 3, AML_NOTSERIALIZED);
             s = MEMORY_SLOT_OST_METHOD;
-            aml_append(method, aml_return(aml_call4(
-                s, aml_name("_UID"), aml_arg(0), aml_arg(1), aml_arg(2)
-            )));
+            aml_append(method,
+                       aml_call4(s, aml_name("_UID"), aml_arg(0),
+                                 aml_arg(1), aml_arg(2)));
             aml_append(dev, method);
 
             method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
             s = MEMORY_SLOT_EJECT_METHOD;
-            aml_append(method, aml_return(aml_call2(
-                       s, aml_name("_UID"), aml_arg(0))));
+            aml_append(method,
+                       aml_call2(s, aml_name("_UID"), aml_arg(0)));
             aml_append(dev, method);
 
             aml_append(dev_container, dev);
Signature · 1 line
2.17.2
#15 lore raw

PATCH 14/26 hw: acpi: Export the PCI hotplug API

In reply to Samuel Ortiz

From: Sebastien Boeuf <[email protected]>

The ACPI hotplug support for PCI devices APIs are not x86 or even machine type specific. In order for future machine types to be able to re-use that code, we export it through the architecture agnostic hw/acpi folder.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Richard Henderson <[email protected]>
Cc Eduardo Habkost <[email protected]>
Cc Marcel Apfelbaum <[email protected]>
---
 hw/acpi/aml-build.c         | 194 ++++++++++++++++++++++++++++++++++++
 hw/i386/acpi-build.c        | 192 +----------------------------------
 include/hw/acpi/aml-build.h |   3 +
 3 files changed, 199 insertions(+), 190 deletions(-)
hw/acpi/aml-build.c +194 -0
index cb706ca975..f462bb8313 100644
@@ -35,6 +35,7 @@
 #include "hw/acpi/tpm.h"
 #include "qom/qom-qobject.h"
 #include "qapi/qmp/qnum.h"
+#include "hw/acpi/pcihp.h"
 
 #define PCI_HOST_BRIDGE_CONFIG_ADDR        0xcf8
 #define PCI_HOST_BRIDGE_IO_0_MIN_ADDR      0x0000
@@ -2314,6 +2315,199 @@ Aml *build_pci_host_bridge(Aml *table, AcpiPciBus *pci_host)
     return scope;
 }
 
+void build_acpi_pci_hotplug(Aml *scope)
+{
+    Aml *field;
+    Aml *method;
+
+    aml_append(scope,
+        aml_operation_region("PCST", AML_SYSTEM_IO, aml_int(0xae00), 0x08));
+    field = aml_field("PCST", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
+    aml_append(field, aml_named_field("PCIU", 32));
+    aml_append(field, aml_named_field("PCID", 32));
+    aml_append(scope, field);
+
+    aml_append(scope,
+        aml_operation_region("SEJ", AML_SYSTEM_IO, aml_int(0xae08), 0x04));
+    field = aml_field("SEJ", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
+    aml_append(field, aml_named_field("B0EJ", 32));
+    aml_append(scope, field);
+
+    aml_append(scope,
+        aml_operation_region("BNMR", AML_SYSTEM_IO, aml_int(0xae10), 0x04));
+    field = aml_field("BNMR", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
+    aml_append(field, aml_named_field("BNUM", 32));
+    aml_append(scope, field);
+
+    aml_append(scope, aml_mutex("BLCK", 0));
+
+    method = aml_method("PCEJ", 2, AML_NOTSERIALIZED);
+    aml_append(method, aml_acquire(aml_name("BLCK"), 0xFFFF));
+    aml_append(method, aml_store(aml_arg(0), aml_name("BNUM")));
+    aml_append(method,
+        aml_store(aml_shiftleft(aml_int(1), aml_arg(1)), aml_name("B0EJ")));
+    aml_append(method, aml_release(aml_name("BLCK")));
+    aml_append(method, aml_return(aml_int(0)));
+    aml_append(scope, method);
+}
+
+static void build_append_pcihp_notify_entry(Aml *method, int slot)
+{
+    Aml *if_ctx;
+    int32_t devfn = PCI_DEVFN(slot, 0);
+
+    if_ctx = aml_if(aml_and(aml_arg(0), aml_int(0x1U << slot), NULL));
+    aml_append(if_ctx, aml_notify(aml_name("S%.02X", devfn), aml_arg(1)));
+    aml_append(method, if_ctx);
+}
+
+void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
+                                  bool pcihp_bridge_en)
+{
+    Aml *dev, *notify_method = NULL, *method;
+    QObject *bsel;
+    PCIBus *sec;
+    int i;
+
+    bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, NULL);
+    if (bsel) {
+        uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel));
+
+        aml_append(parent_scope, aml_name_decl("BSEL", aml_int(bsel_val)));
+        notify_method = aml_method("DVNT", 2, AML_NOTSERIALIZED);
+    }
+
+    for (i = 0; i < ARRAY_SIZE(bus->devices); i += PCI_FUNC_MAX) {
+        DeviceClass *dc;
+        PCIDeviceClass *pc;
+        PCIDevice *pdev = bus->devices[i];
+        int slot = PCI_SLOT(i);
+        bool hotplug_enabled_dev;
+        bool bridge_in_acpi;
+
+        if (!pdev) {
+            if (bsel) { /* add hotplug slots for non present devices */
+                dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
+                aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
+                aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
+                method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
+                aml_append(method,
+                    aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
+                );
+                aml_append(dev, method);
+                aml_append(parent_scope, dev);
+
+                build_append_pcihp_notify_entry(notify_method, slot);
+            }
+            continue;
+        }
+
+        pc = PCI_DEVICE_GET_CLASS(pdev);
+        dc = DEVICE_GET_CLASS(pdev);
+
+        /* When hotplug for bridges is enabled, bridges are
+         * described in ACPI separately (see build_pci_bus_end).
+         * In this case they aren't themselves hot-pluggable.
+         * Hotplugged bridges *are* hot-pluggable.
+         */
+        bridge_in_acpi = pc->is_bridge && pcihp_bridge_en &&
+            !DEVICE(pdev)->hotplugged;
+
+        hotplug_enabled_dev = bsel && dc->hotpluggable && !bridge_in_acpi;
+
+        if (pc->class_id == PCI_CLASS_BRIDGE_ISA) {
+            continue;
+        }
+
+        /* start to compose PCI slot descriptor */
+        dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
+        aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
+
+        if (pc->class_id == PCI_CLASS_DISPLAY_VGA) {
+            /* add VGA specific AML methods */
+            int s3d;
+
+            if (object_dynamic_cast(OBJECT(pdev), "qxl-vga")) {
+                s3d = 3;
+            } else {
+                s3d = 0;
+            }
+
+            method = aml_method("_S1D", 0, AML_NOTSERIALIZED);
+            aml_append(method, aml_return(aml_int(0)));
+            aml_append(dev, method);
+
+            method = aml_method("_S2D", 0, AML_NOTSERIALIZED);
+            aml_append(method, aml_return(aml_int(0)));
+            aml_append(dev, method);
+
+            method = aml_method("_S3D", 0, AML_NOTSERIALIZED);
+            aml_append(method, aml_return(aml_int(s3d)));
+            aml_append(dev, method);
+        } else if (hotplug_enabled_dev) {
+            /* add _SUN/_EJ0 to make slot hotpluggable  */
+            aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
+
+            method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
+            aml_append(method,
+                aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
+            );
+            aml_append(dev, method);
+
+            if (bsel) {
+                build_append_pcihp_notify_entry(notify_method, slot);
+            }
+        } else if (bridge_in_acpi) {
+            /*
+             * device is coldplugged bridge,
+             * add child device descriptions into its scope
+             */
+            PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
+
+            build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en);
+        }
+        /* slot descriptor has been composed, add it into parent context */
+        aml_append(parent_scope, dev);
+    }
+
+    if (bsel) {
+        aml_append(parent_scope, notify_method);
+    }
+
+    /* Append PCNT method to notify about events on local and child buses.
+     * Add unconditionally for root since DSDT expects it.
+     */
+    method = aml_method("PCNT", 0, AML_NOTSERIALIZED);
+
+    /* If bus supports hotplug select it and notify about local events */
+    if (bsel) {
+        uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel));
+
+        aml_append(method, aml_store(aml_int(bsel_val), aml_name("BNUM")));
+        aml_append(method,
+            aml_call2("DVNT", aml_name("PCIU"), aml_int(1) /* Device Check */)
+        );
+        aml_append(method,
+            aml_call2("DVNT", aml_name("PCID"), aml_int(3)/* Eject Request */)
+        );
+    }
+
+    /* Notify about child bus events in any case */
+    if (pcihp_bridge_en) {
+        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));
+        }
+    }
+    aml_append(parent_scope, method);
+    qobject_unref(bsel);
+}
+
 void acpi_dsdt_add_pci_bus(Aml *dsdt, AcpiPciBus *pci_host)
 {
     Aml *dev, *pci_scope;
hw/i386/acpi-build.c +2 -190

206 lines not on this page · raw message

include/hw/acpi/aml-build.h +3 -0
index 45dcea711b..06fdbeafa7 100644
@@ -419,6 +419,9 @@ Aml *build_osc_method(uint32_t value);
 void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info);
 Aml *build_gsi_link_dev(const char *name, uint8_t uid, uint8_t gsi);
 Aml *build_prt(bool is_pci0_prt);
+void build_acpi_pci_hotplug(Aml *scope);
+void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
+                                  bool pcihp_bridge_en);
 void acpi_dsdt_add_pci_bus(Aml *dsdt, AcpiPciBus *pci_host);
 Aml *build_pci_host_bridge(Aml *table, AcpiPciBus *pci_host);
 void crs_range_set_init(CrsRangeSet *range_set);
Signature · 1 line
2.17.2
#16 lore raw

PATCH 15/26 hw: acpi: Retrieve the PCI bus from AcpiPciHpState

In reply to Samuel Ortiz

From: Sebastien Boeuf <[email protected]>

Instead of using the machine type specific method find_i440fx() to retrieve the PCI bus, this commit aims to rely on the fact that the PCI bus is known by the structure AcpiPciHpState.

When the structure is initialized through acpi_pcihp_init() call, it saves the PCI bus, which means there is no need to invoke a special function later on.

Based on the fact that find_i440fx() was only used there, this patch also removes the function find_i440fx() itself from the entire codebase.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Marcel Apfelbaum <[email protected]>
Cc Paolo Bonzini <[email protected]>
---
 hw/acpi/pcihp.c       | 10 ++++------
 hw/pci-host/piix.c    |  8 --------
 include/hw/i386/pc.h  |  1 -
 stubs/Makefile.objs   |  1 -
 stubs/pci-host-piix.c |  6 ------
 5 files changed, 4 insertions(+), 22 deletions(-)
 delete mode 100644 stubs/pci-host-piix.c
hw/acpi/pcihp.c +4 -6
index 80d42e12ff..254b2e50ab 100644
@@ -93,10 +93,9 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque)
     return bsel_alloc;
 }
 
-static void acpi_set_pci_info(void)
+static void acpi_set_pci_info(AcpiPciHpState *s)
 {
     static bool bsel_is_set;
-    PCIBus *bus;
     unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT;
 
     if (bsel_is_set) {
@@ -104,10 +103,9 @@ static void acpi_set_pci_info(void)
     }
     bsel_is_set = true;
 
-    bus = find_i440fx(); /* TODO: Q35 support */
-    if (bus) {
+    if (s->root) {
         /* Scan all PCI buses. Set property to enable acpi based hotplug. */
-        pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc);
+        pci_for_each_bus_depth_first(s->root, acpi_set_bsel, NULL, &bsel_alloc);
     }
 }
 
@@ -213,7 +211,7 @@ static void acpi_pcihp_update(AcpiPciHpState *s)
 
 void acpi_pcihp_reset(AcpiPciHpState *s)
 {
-    acpi_set_pci_info();
+    acpi_set_pci_info(s);
     acpi_pcihp_update(s);
 }
 
hw/pci-host/piix.c +0 -8
index da73743fa2..4940f59c9b 100644
@@ -445,14 +445,6 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
     return b;
 }
 
-PCIBus *find_i440fx(void)
-{
-    PCIHostState *s = OBJECT_CHECK(PCIHostState,
-                                   object_resolve_path("/machine/i440fx", NULL),
-                                   TYPE_PCI_HOST_BRIDGE);
-    return s ? s->bus : NULL;
-}
-
 /* PIIX3 PCI to ISA bridge */
 static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
 {
include/hw/i386/pc.h +0 -1
index 7d177cd207..f6b2649cf8 100644
@@ -260,7 +260,6 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
                     MemoryRegion *pci_memory,
                     MemoryRegion *ram_memory);
 
-PCIBus *find_i440fx(void);
 /* piix4.c */
 extern PCIDevice *piix4_dev;
 int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn);
stubs/Makefile.objs +0 -1
index 5dd0aeeec6..725f78bedc 100644
@@ -41,6 +41,5 @@ stub-obj-y += pc_madt_cpu_entry.o
 stub-obj-y += vmgenid.o
 stub-obj-y += xen-common.o
 stub-obj-y += xen-hvm.o
-stub-obj-y += pci-host-piix.o
 stub-obj-y += ram-block.o
 stub-obj-y += ramfb.o
stubs/pci-host-piix.c deleted +0 -6
deleted file mode 100644
index 6ed81b1f21..0000000000
@@ -1,6 +0,0 @@
-#include "qemu/osdep.h"
-#include "hw/i386/pc.h"
-PCIBus *find_i440fx(void)
-{
-    return NULL;
-}
Signature · 1 line
2.17.2
#17 lore raw

PATCH 16/26 hw: fw-build: Add firmware build methods and state

In reply to Samuel Ortiz

In order to decouple ACPI APIs from specific machine types, we are adding granular firmware build methods to the generic MachineClass structure. This way, a new machine type can re-use the high level ACPI APIs and define some custom table build methods, without having to duplicate most of the existing implementation only to add small variations to it.

Cc Eduardo Habkost <[email protected]>
Cc Marcel Apfelbaum <[email protected]>
---
 include/hw/boards.h   |  5 ++++
 include/hw/fw-build.h | 57 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)
 create mode 100644 include/hw/fw-build.h
include/hw/boards.h +5 -0
index f82f28468b..a5c8fe6ed2 100644
@@ -5,6 +5,7 @@
 
 #include "sysemu/blockdev.h"
 #include "sysemu/accel.h"
+#include "hw/fw-build.h"
 #include "hw/qdev.h"
 #include "qom/object.h"
 #include "qom/cpu.h"
@@ -214,6 +215,8 @@ struct MachineClass {
                                                          unsigned cpu_index);
     const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
     int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx);
+
+    FirmwareBuildMethods firmware_build_methods;
 };
 
 /**
@@ -269,6 +272,8 @@ struct MachineState {
     const char *cpu_type;
     AccelState *accelerator;
     CPUArchIdList *possible_cpus;
+
+    FirmwareBuildState firmware_build_state;
 };
 
 #define DEFINE_MACHINE(namestr, machine_initfn) \
include/hw/fw-build.h new file +57 -0

57 lines not on this page · raw message

Signature · 1 line
2.17.2
#18 lore raw

PATCH 17/26 hw: i386: Convert PC machine type to firmware build methods

In reply to Samuel Ortiz

All PC machine type derivatives will use the same ACPI table build methods. But with that change in place, any new x86 machine type will be able to re-use the acpi-build API and customize part of it by defining its own ACPI table build methods.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Marcel Apfelbaum <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Richard Henderson <[email protected]>
Cc Eduardo Habkost <[email protected]>
---
 hw/i386/acpi-build.c   | 22 ++++++++++++++--------
 hw/i386/pc.c           |  9 +++++++++
 include/hw/i386/acpi.h | 27 +++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 8 deletions(-)
 create mode 100644 include/hw/i386/acpi.h
hw/i386/acpi-build.c +14 -8
index 9cb739bf5c..4274349053 100644
@@ -62,6 +62,7 @@
 #include "qom/qom-qobject.h"
 #include "hw/i386/amd_iommu.h"
 #include "hw/i386/intel_iommu.h"
+#include "hw/i386/acpi.h"
 
 #include "hw/acpi/ipmi.h"
 
@@ -281,9 +282,8 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
     }
 }
 
-static void
-build_madt(GArray *table_data, BIOSLinker *linker,
-           MachineState *ms, AcpiConfiguration *conf)
+GArray *build_madt(GArray *table_data, BIOSLinker *linker,
+                   MachineState *ms, AcpiConfiguration *conf)
 {
     MachineClass *mc = MACHINE_GET_CLASS(ms);
     const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(ms);
@@ -360,6 +360,8 @@ build_madt(GArray *table_data, BIOSLinker *linker,
     build_header(linker, table_data,
                  (void *)(table_data->data + madt_start), "APIC",
                  table_data->len - madt_start, 1, NULL, NULL);
+
+    return table_data;
 }
 
 static void build_hpet_aml(Aml *table)
@@ -1544,6 +1546,7 @@ static
 void acpi_build(AcpiBuildTables *tables,
                 MachineState *machine, AcpiConfiguration *conf)
 {
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
     GArray *table_offsets;
     unsigned facs, dsdt, rsdt, fadt;
     AcpiPmInfo pm;
@@ -1604,7 +1607,8 @@ void acpi_build(AcpiBuildTables *tables,
     aml_len += tables_blob->len - fadt;
 
     acpi_add_table(table_offsets, tables_blob);
-    build_madt(tables_blob, tables->linker, machine, conf);
+    mc->firmware_build_methods.acpi.madt(tables_blob, tables->linker,
+                                         machine, conf);
 
     vmgenid_dev = find_vmgenid_dev();
     if (vmgenid_dev) {
@@ -1628,15 +1632,17 @@ void acpi_build(AcpiBuildTables *tables,
     }
     if (conf->numa_nodes) {
         acpi_add_table(table_offsets, tables_blob);
-        build_srat(tables_blob, tables->linker, machine, conf);
+        mc->firmware_build_methods.acpi.srat(tables_blob, tables->linker,
+                                             machine, conf);
         if (have_numa_distance) {
             acpi_add_table(table_offsets, tables_blob);
-            build_slit(tables_blob, tables->linker);
+            mc->firmware_build_methods.acpi.slit(tables_blob, tables->linker);
         }
     }
     if (acpi_get_mcfg(&mcfg)) {
         acpi_add_table(table_offsets, tables_blob);
-        build_mcfg(tables_blob, tables->linker, &mcfg);
+        mc->firmware_build_methods.acpi.mcfg(tables_blob, tables->linker,
+                                             &mcfg);
     }
     if (x86_iommu_get_default()) {
         IommuType IOMMUType = x86_iommu_get_type();
@@ -1667,7 +1673,7 @@ void acpi_build(AcpiBuildTables *tables,
                slic_oem.id, slic_oem.table_id);
 
     /* RSDP is in FSEG memory, so allocate it separately */
-    build_rsdp_rsdt(tables->rsdp, tables->linker, rsdt);
+    mc->firmware_build_methods.acpi.rsdp(tables->rsdp, tables->linker, rsdt);
 
     /* We'll expose it all to Guest so we want to reduce
      * chance of size changes.
hw/i386/pc.c +9 -0
index fa12583096..db69dbfef7 100644
@@ -75,6 +75,7 @@
 #include "hw/nmi.h"
 #include "hw/i386/intel_iommu.h"
 #include "hw/net/ne2000-isa.h"
+#include "hw/i386/acpi.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ
@@ -2443,6 +2444,14 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     nc->nmi_monitor_handler = x86_nmi;
     mc->default_cpu_type = TARGET_DEFAULT_CPU_TYPE;
 
+    /* Firmware building handler */
+    mc->firmware_build_methods.acpi.madt = build_madt;
+    mc->firmware_build_methods.acpi.rsdp = build_rsdp_rsdt;
+    mc->firmware_build_methods.acpi.setup = acpi_setup;
+    mc->firmware_build_methods.acpi.mcfg = build_mcfg;
+    mc->firmware_build_methods.acpi.srat = build_srat;
+    mc->firmware_build_methods.acpi.slit = build_slit;
+
     object_class_property_add(oc, MEMORY_DEVICE_REGION_SIZE, "int",
         pc_machine_get_device_memory_region_size, NULL,
         NULL, NULL, &error_abort);
include/hw/i386/acpi.h new file +27 -0
new file mode 100644
index 0000000000..d92c29350b
@@ -0,0 +1,27 @@
+/*
+ *
+ * Copyright (c) 2018 Intel Corportation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QEMU_I386_ACPI_H
+#define QEMU_I386_ACPI_H
+
+#include "hw/acpi/acpi.h"
+
+/* Build methods */
+GArray *build_madt(GArray *table_data, BIOSLinker *linker,
+                   MachineState *ms, AcpiConfiguration *conf);
+
+#endif
Signature · 1 line
2.17.2
#19 lore raw

Re: [Qemu-devel] [PATCH 17/26] hw: i386: Convert PC machine type to firmware build methods

hw/i386/pc.c static void pc_machine_class_init(ObjectClass *oc, void *data) · line 2454

Quoted 8 lines · Samuel Ortiz
hw/i386/pc.c +8 -0
+    /* Firmware building handler */
+    mc->firmware_build_methods.acpi.madt = build_madt;
+    mc->firmware_build_methods.acpi.rsdp = build_rsdp_rsdt;
+    mc->firmware_build_methods.acpi.setup = acpi_setup;
+    mc->firmware_build_methods.acpi.mcfg = build_mcfg;
+    mc->firmware_build_methods.acpi.srat = build_srat;
+    mc->firmware_build_methods.acpi.slit = build_slit;
+

mc->firmware_build_methods.acpi.setup is not called anywhere, as far as I can see?

Paolo

#20 lore raw

PATCH 18/26 hw: acpi: Initial hardware-reduced support

In reply to Samuel Ortiz

We build a minimal set of ACPI hardware-reduced tables: XSDT, FADT, MADT and a DSDT pointed by a RSDP.
The DSDT only contains one PCI host bridge for now.

This API will be consumed by new x86 machine type but also potentially by the ARM virt one.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
---
 default-configs/i386-softmmu.mak |   1 +
 hw/acpi/Makefile.objs            |   1 +
 hw/acpi/reduced.c                | 248 +++++++++++++++++++++++++++++++
 include/hw/acpi/reduced.h        |  24 +++
 4 files changed, 274 insertions(+)
 create mode 100644 hw/acpi/reduced.c
 create mode 100644 include/hw/acpi/reduced.h
default-configs/i386-softmmu.mak +1 -0
index 210cff2781..a80509a111 100644
@@ -66,3 +66,4 @@ CONFIG_I2C=y
 CONFIG_SEV=$(CONFIG_KVM)
 CONFIG_VTD=y
 CONFIG_AMD_IOMMU=y
+CONFIG_ACPI_HW_REDUCED=y
hw/acpi/Makefile.objs +1 -0
index 11c35bcb44..276e0cfbce 100644
@@ -6,6 +6,7 @@ common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
 common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
 common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
 common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
+common-obj-$(CONFIG_ACPI_HW_REDUCED) += reduced.o
 common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
 
 common-obj-y += acpi_interface.o
hw/acpi/reduced.c new file +248 -0

248 lines not on this page · raw message

include/hw/acpi/reduced.h new file +24 -0
new file mode 100644
index 0000000000..94a5aac9eb
@@ -0,0 +1,24 @@
+/* HW reduced ACPI headers
+ *
+ * Copyright (c) 2018 Intel Corportation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HW_ACPI_REDUCED_H
+#define HW_ACPI_REDUCED_H
+
+void acpi_reduced_setup(MachineState *machine, AcpiConfiguration *conf);
+
+#endif
Signature · 1 line
2.17.2
#21 lore raw

Re: [Qemu-devel] [PATCH 18/26] hw: acpi: Initial hardware-reduced support

Quoted 10 lines · Samuel Ortiz

We build a minimal set of ACPI hardware-reduced tables: XSDT, FADT, MADT and a DSDT pointed by a RSDP.
The DSDT only contains one PCI host bridge for now.

This API will be consumed by new x86 machine type but also potentially by the ARM virt one.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>

Do not include patches that essentially add dead code. It is nice to have hardware-reduced support, but if you want to contribute it you need to add a user as well, for example the ARM virt machine type.

In fact, using it in the ARM virt machine type is a no-brainer, so doing that change (even if it's not yet part of NEMU) would be an excellent way to reduce your delta, without going through the processing of convincing QEMU maintainers of the advantages of your new x86 machine type.

Paolo

#22 lore raw

Re: [Qemu-devel] [PATCH 18/26] hw: acpi: Initial hardware-reduced support

Quoted 16 lines · Paolo Bonzini
Quoted 10 lines · Samuel Ortiz

We build a minimal set of ACPI hardware-reduced tables: XSDT, FADT, MADT and a DSDT pointed by a RSDP.
The DSDT only contains one PCI host bridge for now.

This API will be consumed by new x86 machine type but also potentially by the ARM virt one.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>

Do not include patches that essentially add dead code. It is nice to have hardware-reduced support, but if you want to contribute it you need to add a user as well, for example the ARM virt machine type.

Quoted 6 lines

In fact, using it in the ARM virt machine type is a no-brainer, so doing that change (even if it's not yet part of NEMU) would be an excellent way to reduce your delta, without going through the processing of convincing QEMU maintainers of the advantages of your new x86 machine type.

Paolo

Excellent point, thanks Paolo.

Signature · 1 line
MST
#23 lore raw

Re: [Qemu-devel] [PATCH 18/26] hw: acpi: Initial hardware-reduced support

In reply to Samuel Ortiz
Quoted 20 lines · Samuel Ortiz
hw/acpi/reduced.c new file +20 -0
+
+static void acpi_reduced_build_update(void *build_opaque)
+{
+    MachineState *ms = MACHINE(build_opaque);
+    AcpiBuildState *build_state = ms->firmware_build_state.acpi.state;
+    AcpiConfiguration *conf = ms->firmware_build_state.acpi.conf;
+    AcpiBuildTables tables;
+
+    /* No ACPI configuration? Nothing to do. */
+    if (!conf) {
+        return;
+    }
+
+    /* No state to update or already patched? Nothing to do. */
+    if (!build_state || build_state->patched) {
+        return;
+    }
+    build_state->patched = true;
+
+    acpi_build_tables_init(&tables);

hw/acpi/reduced.c line 182

Quoted 10 lines
hw/acpi/reduced.c new file +10 -0
+
+    acpi_reduced_build(ms, &tables, conf);
+
+    acpi_ram_update(build_state->table_mr, tables.table_data);
+    acpi_ram_update(build_state->rsdp_mr, tables.rsdp);
+    acpi_ram_update(build_state->linker_mr, tables.linker->cmd_blob);
+
+    acpi_build_tables_cleanup(&tables, true);
+}
+

ms is not needed here; just pass the FirmwareBuildState as the opaque value in rom_add_blob.

In fact, here:

hw/acpi/reduced.c line 217

Quoted 6 lines
hw/acpi/reduced.c new file +6 -0
+    AcpiBuildState *build_state;
+
+    build_state = g_malloc0(sizeof(*build_state));
+    machine->firmware_build_state.acpi.state = build_state;
+    machine->firmware_build_state.acpi.conf = conf;
+

I would say that you don't need FirmwareBuildState at all. I cannot be 100% sure because I cannot see the caller of acpi_reduced_setup, but I think you can add an AcpiConfiguration* field to AcpiBuildState and encapsulate everything in AcpiBuildState. In addition, the AcpiBuildState need not be stored in the MachineState.

Instead, FirmwareBuildMethods should be a QOM interface (also please refer explicitly to ACPI in the names, don't call it "firmware"). The setup method of FirmwareBuildMethods can take that QOM interface, not the MachineState, i.e. its prototype should be

void (*setup)(AcpiBuildMethods *acpibuild,

	      AcpiConfiguration *conf);

so that pc_machine_done does

    if (pcms->acpi_build_enabled) {
        acpi_conf_pc_init(pcms);
	/* This calls the ->setup method.  */
        acpi_builder_setup(ACPI_BUILD_METHODS(pcms),
			   &pcms->acpi_configuration);
    }

This is because MachineClass is used for dozens of machines that have nothing to with ACPI. Instead, machines that use ACPI (either reduced or normal) can define the AcpiBuildMethods interface, and invoke the entry point firmware_build_methods->setup (either acpi_setup or acpi_reduced_setup) through acpi_builder_setup.

Paolo

#24 lore raw

PATCH 19/26 hw: acpi: reduced: Add MCFG support

In reply to Samuel Ortiz

From: Yang Zhong <[email protected]>

For PCIe based platform, we need to add an MCFG table to the hardware-reduced DSDT.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
---
 hw/acpi/reduced.c | 7 +++++++
 1 file changed, 7 insertions(+)
hw/acpi/reduced.c +7 -0
index 364b105f58..0f6397c740 100644
@@ -94,6 +94,7 @@ static void acpi_reduced_build(MachineState *ms, AcpiBuildTables *tables,
     Range pci_hole, pci_hole64;
     Object *pci_host;
     PCIBus *bus = NULL;
+    AcpiMcfgInfo mcfg;
     GArray *tables_blob = tables->table_data;
 
     acpi_get_pci_holes(&pci_hole, &pci_hole64);
@@ -127,6 +128,12 @@ static void acpi_reduced_build(MachineState *ms, AcpiBuildTables *tables,
     acpi_add_table(table_offsets, tables_blob);
     mc->firmware_build_methods.acpi.madt(tables_blob, tables->linker, ms, conf);
 
+    if (acpi_get_mcfg(&mcfg)) {
+        acpi_add_table(table_offsets, tables_blob);
+        mc->firmware_build_methods.acpi.mcfg(tables_blob,
+                                             tables->linker, &mcfg);
+    }
+
     /* RSDT is pointed to by RSDP */
     xsdt = tables_blob->len;
     build_xsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
Signature · 1 line
2.17.2
#25 lore raw

PATCH 20/26 hw: acpi: reduced: Generic Event Device support

In reply to Samuel Ortiz

The ACPI Generic Event Device (GED) is a hardware-reduced specific device that handles all platform events, including the hotplug ones. This patch generate the AML code that defines GEDs. Platforms need to specify their own GedEvent array to describe what kind of events they want to support through GED. The build_ged_aml routine takes a GedEvent array that maps a specific GED event to an IRQ number. Then we use that array to build both the _CRS and the _EVT section of the GED device.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
---
 hw/acpi/reduced.c         | 171 +++++++++++++++++++++++++++++++++++++-
 include/hw/acpi/acpi.h    |   4 +
 include/hw/acpi/reduced.h |  16 ++++
 3 files changed, 188 insertions(+), 3 deletions(-)
hw/acpi/reduced.c +168 -3
index 0f6397c740..53b57760eb 100644
@@ -30,6 +30,8 @@
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/aml-build.h"
 #include "hw/acpi/bios-linker-loader.h"
+#include "hw/acpi/cpu.h"
+#include "hw/acpi/pc-hotplug.h"
 #include "hw/acpi/reduced.h"
 
 #include "hw/nvram/fw_cfg.h"
@@ -44,9 +46,34 @@
 
 #include "migration/vmstate.h"
 
+#define GED_DEVICE "GED"
+
+static void acpi_dsdt_add_cpus(MachineState *ms, Aml *dsdt, Aml *scope,
+                               int smp_cpus, AcpiConfiguration *conf)
+{
+    CPUHotplugFeatures opts = {
+        .apci_1_compatible = false,
+        .has_legacy_cphp = false,
+    };
+
+    build_cpus_aml(dsdt, ms, opts, conf->cpu_hotplug_io_base,
+                   "\\_SB", NULL);
+}
+
+static void acpi_dsdt_add_ged(Aml *scope, AcpiConfiguration *conf)
+{
+    if (!conf->ged_events || !conf->ged_events_size) {
+        return;
+    }
+
+    build_ged_aml(scope, "\\_SB."GED_DEVICE,
+                  conf->ged_events, conf->ged_events_size);
+}
+
 /* DSDT */
-static void build_dsdt(GArray *table_data, BIOSLinker *linker,
-                       AcpiPciBus *pci_host)
+static void build_dsdt(MachineState *ms,
+                       GArray *table_data, BIOSLinker *linker,
+                       AcpiPciBus *pci_host, AcpiConfiguration *conf)
 {
     Aml *scope, *dsdt;
 
@@ -58,6 +85,8 @@ static void build_dsdt(GArray *table_data, BIOSLinker *linker,
     if (pci_host->pci_bus) {
         acpi_dsdt_add_pci_bus(dsdt, pci_host);
     }
+    acpi_dsdt_add_cpus(ms, dsdt, scope, smp_cpus, conf);
+    acpi_dsdt_add_ged(dsdt, conf);
     aml_append(dsdt, scope);
 
     /* copy AML table into ACPI tables blob and patch header there */
@@ -118,7 +147,7 @@ static void acpi_reduced_build(MachineState *ms, AcpiBuildTables *tables,
 
     /* DSDT is pointed to by FADT */
     dsdt = tables_blob->len;
-    build_dsdt(tables_blob, tables->linker, &acpi_pci_host);
+    build_dsdt(ms, tables_blob, tables->linker, &acpi_pci_host, conf);
 
     /* FADT pointed to by RSDT */
     acpi_add_table(table_offsets, tables_blob);
@@ -253,3 +282,139 @@ void acpi_reduced_setup(MachineState *machine, AcpiConfiguration *conf)
      */
     acpi_build_tables_cleanup(&tables, false);
 }
+
+#define CPU_SCAN_METHOD   "CSCN"
+
+static Aml *ged_event_aml(GedEvent *event)
+{
+    if (!event) {
+        return NULL;
+    }
+
+    switch (event->event) {
+    case GED_CPU_HOTPLUG:
+        /* We run a complete CPU SCAN when getting a CPU hotplug event */
+        return aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD);
+    case GED_MEMORY_HOTPLUG:
+    case GED_PCI_HOTPLUG:
+    case GED_NVDIMM_HOTPLUG:
+        /* Not supported for now */
+        return NULL;
+    default:
+        break;
+    }
+
+    return NULL;
+}
+
+void build_ged_aml(Aml *table, const char *name,
+                   GedEvent *events, uint8_t events_size)
+{
+    Aml *crs = aml_resource_template();
+    Aml *evt;
+    Aml *zero = aml_int(0);
+    Aml *one = aml_int(1);
+    Aml *dev = aml_device("%s", name);
+    Aml *has_irq = aml_local(0);
+    Aml *while_ctx;
+    uint8_t i;
+
+    /*
+     * For each GED event we:
+     * - Add an interrupt to the CRS section.
+     * - Add a conditional block for each event, inside a while loop.
+     *   This is semantically equivalent to a switch/case implementation.
+     */
+    evt = aml_method("_EVT", 1, AML_SERIALIZED);
+    {
+        Aml *irq = aml_arg(0);
+        Aml *ged_aml;
+        Aml *if_ctx, *else_ctx;
+
+        /* Local0 = One */
+        aml_append(evt, aml_store(one, has_irq));
+
+
+        /*
+         * Here we want to call a method for each supported GED event type.
+         * The resulting ASL code looks like:
+         *
+         * Local0 = One
+         * While ((Local0 == One))
+         * {
+         *    Local0 = Zero
+         *    If (Arg0 == irq0)
+         *    {
+         *        MethodEvent0()
+         *        Local0 = Zero
+         *    }
+         *    ElseIf (Arg0 == irq1)
+         *    {
+         *        MethodEvent1()
+         *        Local0 = Zero
+         *    }
+         *    ElseIf (Arg0 == irq2)
+         *    {
+         *        MethodEvent2()
+         *        Local0 = Zero
+         *    }
+         * }
+         */
+
+        /* While ((Local0 == One)) */
+        while_ctx = aml_while(aml_equal(has_irq, one));
+        {
+            else_ctx = NULL;
+
+            /*
+             * Clear loop condition, we don't want to enter an infinite loop.
+             * Local0 = Zero
+             */
+            aml_append(while_ctx, aml_store(zero, has_irq));
+            for (i = 0; i < events_size; i++) {
+                ged_aml = ged_event_aml(&events[i]);
+                if (!ged_aml) {
+                    continue;
+                }
+
+                /* _CRS interrupt */
+                aml_append(crs, aml_interrupt(AML_CONSUMER,
+                                              AML_LEVEL,
+                                              AML_ACTIVE_HIGH,
+                                              AML_EXCLUSIVE,
+                                              &events[i].irq, 1));
+
+                /* If ((Arg0 == irq))*/
+                if_ctx = aml_if(aml_equal(irq, aml_int(events[i].irq)));
+                {
+                    /* AML for this specific type of event */
+                    aml_append(if_ctx, ged_aml);
+                }
+
+                /*
+                 * We append the first if to the while context.
+                 * Other ifs will be elseifs.
+                 */
+                if (!else_ctx) {
+                    aml_append(while_ctx, if_ctx);
+                } else {
+                    aml_append(else_ctx, if_ctx);
+                    aml_append(while_ctx, else_ctx);
+                }
+
+                if (i != events_size - 1) {
+                    else_ctx = aml_else();
+                }
+            }
+        }
+
+        aml_append(evt, while_ctx);
+    }
+
+    aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0013")));
+    aml_append(dev, aml_name_decl("_UID", zero));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+    aml_append(dev, evt);
+
+    aml_append(table, dev);
+}
include/hw/acpi/acpi.h +4 -0
index 6a94452928..564443ff27 100644
@@ -24,6 +24,7 @@
 #include "exec/memory.h"
 #include "hw/irq.h"
 #include "hw/acpi/acpi_dev_interface.h"
+#include "hw/acpi/reduced.h"
 #include "hw/hotplug.h"
 #include "hw/mem/nvdimm.h"
 
@@ -224,6 +225,9 @@ struct AcpiConfiguration {
     uint64_t *node_mem;
     bool apic_xrupt_override;
     unsigned apic_id_limit;
+    uint16_t cpu_hotplug_io_base;
+    GedEvent *ged_events;
+    uint8_t ged_events_size;
 
     /* Build state */
     AcpiBuildState *build_state;
include/hw/acpi/reduced.h +16 -0
index 94a5aac9eb..b326af9bad 100644
@@ -19,6 +19,22 @@
 #ifndef HW_ACPI_REDUCED_H
 #define HW_ACPI_REDUCED_H
 
+typedef struct Aml Aml;
+
+typedef enum {
+    GED_CPU_HOTPLUG    = 1,
+    GED_MEMORY_HOTPLUG = 2,
+    GED_PCI_HOTPLUG    = 3,
+    GED_NVDIMM_HOTPLUG = 4,
+} GedEventType;
+
+typedef struct GedEvent {
+    uint32_t     irq;
+    GedEventType event;
+} GedEvent;
+
 void acpi_reduced_setup(MachineState *machine, AcpiConfiguration *conf);
+void build_ged_aml(Aml *table, const char* name,
+                   GedEvent *events, uint8_t events_size);
 
 #endif
Signature · 1 line
2.17.2
#26 lore raw

PATCH 21/26 hw: acpi: reduced: Add memory hotplug support

In reply to Samuel Ortiz

From: Yang Zhong <[email protected]>

We add the memory hotplug AML code to the hardware-reduced DSDT. The memory hotplug event is handled through the GED device.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
---
 hw/acpi/reduced.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)
hw/acpi/reduced.c +14 -3
index 53b57760eb..33d413a255 100644
@@ -33,6 +33,7 @@
 #include "hw/acpi/cpu.h"
 #include "hw/acpi/pc-hotplug.h"
 #include "hw/acpi/reduced.h"
+#include "hw/acpi/memory_hotplug.h"
 
 #include "hw/nvram/fw_cfg.h"
 
@@ -46,7 +47,16 @@
 
 #include "migration/vmstate.h"
 
-#define GED_DEVICE "GED"
+#define GED_DEVICE               "GED"
+#define CPU_SCAN_METHOD          "CSCN"
+#define MEMORY_SLOT_SCAN_METHOD  "MSCN"
+
+static void acpi_dsdt_add_memory_hotplug(MachineState *ms, Aml *dsdt)
+{
+    uint32_t nr_mem = ms->ram_slots;
+
+    build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB", NULL);
+}
 
 static void acpi_dsdt_add_cpus(MachineState *ms, Aml *dsdt, Aml *scope,
                                int smp_cpus, AcpiConfiguration *conf)
@@ -85,6 +95,7 @@ static void build_dsdt(MachineState *ms,
     if (pci_host->pci_bus) {
         acpi_dsdt_add_pci_bus(dsdt, pci_host);
     }
+    acpi_dsdt_add_memory_hotplug(ms, dsdt);
     acpi_dsdt_add_cpus(ms, dsdt, scope, smp_cpus, conf);
     acpi_dsdt_add_ged(dsdt, conf);
     aml_append(dsdt, scope);
@@ -283,8 +294,6 @@ void acpi_reduced_setup(MachineState *machine, AcpiConfiguration *conf)
     acpi_build_tables_cleanup(&tables, false);
 }
 
-#define CPU_SCAN_METHOD   "CSCN"
-
 static Aml *ged_event_aml(GedEvent *event)
 {
     if (!event) {
@@ -296,6 +305,8 @@ static Aml *ged_event_aml(GedEvent *event)
         /* We run a complete CPU SCAN when getting a CPU hotplug event */
         return aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD);
     case GED_MEMORY_HOTPLUG:
+        /* We run a complete memory SCAN when getting a memory hotplug event */
+        return aml_call0("\\_SB.MHPC." MEMORY_SLOT_SCAN_METHOD);
     case GED_PCI_HOTPLUG:
     case GED_NVDIMM_HOTPLUG:
         /* Not supported for now */
Signature · 1 line
2.17.2
#27 lore raw

PATCH 22/26 hw: acpi: reduced: Add shutdown support

In reply to Samuel Ortiz

From: Jing Liu <[email protected]>

Hardware-reduced ACPI uses SLEEP_CONTROL_REG to enter S5 sleep state.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
---
 hw/acpi/aml-build.c         |  2 +-
 hw/acpi/reduced.c           | 15 +++++++++++++++
 include/hw/acpi/acpi-defs.h |  1 +
 include/hw/acpi/reduced.h   |  4 ++++
 4 files changed, 21 insertions(+), 1 deletion(-)
hw/acpi/aml-build.c +1 -1
index f462bb8313..752642a67a 100644
@@ -2922,7 +2922,7 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
     }
 
     /* SLEEP_CONTROL_REG */
-    build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
+    build_append_gas_from_struct(tbl, &f->sleep_control_reg);
     /* SLEEP_STATUS_REG */
     build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
 
hw/acpi/reduced.c +15 -0
index 33d413a255..329d5112ca 100644
@@ -80,6 +80,17 @@ static void acpi_dsdt_add_ged(Aml *scope, AcpiConfiguration *conf)
                   conf->ged_events, conf->ged_events_size);
 }
 
+static void acpi_dsdt_add_sleep_state(Aml *scope)
+{
+    Aml *pkg = aml_package(4);
+
+    aml_append(pkg, aml_int(ACPI_REDUCED_SLEEP_LEVEL));
+    aml_append(pkg, aml_int(0));
+    aml_append(pkg, aml_int(0));
+    aml_append(pkg, aml_int(0));
+    aml_append(scope, aml_name_decl("_S5", pkg));
+}
+
 /* DSDT */
 static void build_dsdt(MachineState *ms,
                        GArray *table_data, BIOSLinker *linker,
@@ -98,6 +109,7 @@ static void build_dsdt(MachineState *ms,
     acpi_dsdt_add_memory_hotplug(ms, dsdt);
     acpi_dsdt_add_cpus(ms, dsdt, scope, smp_cpus, conf);
     acpi_dsdt_add_ged(dsdt, conf);
+    acpi_dsdt_add_sleep_state(scope);
     aml_append(dsdt, scope);
 
     /* copy AML table into ACPI tables blob and patch header there */
@@ -120,6 +132,9 @@ static void build_fadt_reduced(GArray *table_data, BIOSLinker *linker,
         .dsdt_tbl_offset = &dsdt_tbl_offset,
         .xdsdt_tbl_offset = &dsdt_tbl_offset,
         .arm_boot_arch = 0,
+        .sleep_control_reg = { .space_id = AML_AS_SYSTEM_IO,
+                              .bit_width = 8, .bit_offset = 0,
+                              .address = ACPI_REDUCED_SLEEP_CONTROL_IOPORT },
     };
 
     build_fadt(table_data, linker, &fadt, NULL, NULL);
include/hw/acpi/acpi-defs.h +1 -0
index 6e1726e0a2..10b7bf9c98 100644
@@ -112,6 +112,7 @@ typedef struct AcpiFadtData {
     unsigned *facs_tbl_offset; /* FACS offset in */
     unsigned *dsdt_tbl_offset;
     unsigned *xdsdt_tbl_offset;
+    struct AcpiGenericAddress sleep_control_reg;  /* SLEEP_CONTROL_REG */
 } AcpiFadtData;
 
 #define ACPI_FADT_ARM_PSCI_COMPLIANT  (1 << 0)
include/hw/acpi/reduced.h +4 -0
index b326af9bad..3d3c003353 100644
@@ -19,6 +19,10 @@
 #ifndef HW_ACPI_REDUCED_H
 #define HW_ACPI_REDUCED_H
 
+#define ACPI_REDUCED_SLEEP_LEVEL          5
+#define ACPI_REDUCED_SLEEP_ENABLE         (1 << 5) /* SLP_EN */
+#define ACPI_REDUCED_SLEEP_CONTROL_IOPORT 0x3B0
+
 typedef struct Aml Aml;
 
 typedef enum {
Signature · 1 line
2.17.2
#28 lore raw

PATCH 23/26 hw: acpi: reduced: Add reboot support

In reply to Samuel Ortiz

From: Jing Liu <[email protected]>

We only need to expose it through FADT.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
---
 hw/acpi/reduced.c         | 7 ++++++-
 include/hw/acpi/reduced.h | 2 ++
 2 files changed, 8 insertions(+), 1 deletion(-)
hw/acpi/reduced.c +6 -1
index 329d5112ca..3754258335 100644
@@ -128,10 +128,15 @@ static void build_fadt_reduced(GArray *table_data, BIOSLinker *linker,
     AcpiFadtData fadt = {
         .rev = 5,
         .minor_ver = 1,
-        .flags = 1 << ACPI_FADT_F_HW_REDUCED_ACPI,
+        .flags = (1 << ACPI_FADT_F_HW_REDUCED_ACPI) |
+                 (1 << ACPI_FADT_F_RESET_REG_SUP),
         .dsdt_tbl_offset = &dsdt_tbl_offset,
         .xdsdt_tbl_offset = &dsdt_tbl_offset,
         .arm_boot_arch = 0,
+        .reset_reg = { .space_id = AML_AS_SYSTEM_IO,
+                      .bit_width = 8, .bit_offset = 0,
+                      .address = ACPI_REDUCED_RESET_IOPORT },
+        .reset_val = ACPI_REDUCED_RESET_VALUE,
         .sleep_control_reg = { .space_id = AML_AS_SYSTEM_IO,
                               .bit_width = 8, .bit_offset = 0,
                               .address = ACPI_REDUCED_SLEEP_CONTROL_IOPORT },
include/hw/acpi/reduced.h +2 -0
index 3d3c003353..bcd266afd0 100644
@@ -22,6 +22,8 @@
 #define ACPI_REDUCED_SLEEP_LEVEL          5
 #define ACPI_REDUCED_SLEEP_ENABLE         (1 << 5) /* SLP_EN */
 #define ACPI_REDUCED_SLEEP_CONTROL_IOPORT 0x3B0
+#define ACPI_REDUCED_RESET_IOPORT         0x3C0
+#define ACPI_REDUCED_RESET_VALUE          4
 
 typedef struct Aml Aml;
 
Signature · 1 line
2.17.2
#29 lore raw

PATCH 24/26 hw: acpi: reduced: Add SRAT table

In reply to Samuel Ortiz

From: Yang Zhong <[email protected]>

If the platform is a NUMA enabled, we add the SRAT table to the ACPI build. It is up to the calling platform to define its own SRAT build method or use the aml-build.c one.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
---
 hw/acpi/reduced.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
hw/acpi/reduced.c +10 -0
index 3754258335..9d0c5ba01e 100644
@@ -188,6 +188,16 @@ static void acpi_reduced_build(MachineState *ms, AcpiBuildTables *tables,
     acpi_add_table(table_offsets, tables_blob);
     mc->firmware_build_methods.acpi.madt(tables_blob, tables->linker, ms, conf);
 
+    if (conf->numa_nodes) {
+        acpi_add_table(table_offsets, tables_blob);
+        mc->firmware_build_methods.acpi.srat(tables_blob, tables->linker,
+                                             ms, conf);
+        if (have_numa_distance) {
+            acpi_add_table(table_offsets, tables_blob);
+            mc->firmware_build_methods.acpi.slit(tables_blob, tables->linker);
+        }
+    }
+
     if (acpi_get_mcfg(&mcfg)) {
         acpi_add_table(table_offsets, tables_blob);
         mc->firmware_build_methods.acpi.mcfg(tables_blob,
Signature · 1 line
2.17.2
#30 lore raw

PATCH 25/26 hw: acpi: reduced: Add NFIT support

In reply to Samuel Ortiz

From: Sebastien Boeuf <[email protected]>

If the platform is NVDIMM enabled, we add the NFIT table to the ACPI build.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
---
 hw/acpi/reduced.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
hw/acpi/reduced.c +6 -2
index 9d0c5ba01e..fac19a978a 100644
@@ -204,6 +204,11 @@ static void acpi_reduced_build(MachineState *ms, AcpiBuildTables *tables,
                                              tables->linker, &mcfg);
     }
 
+    if (conf->acpi_nvdimm_state.is_enabled) {
+        nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
+                          &conf->acpi_nvdimm_state, ms->ram_slots);
+    }
+
     /* RSDT is pointed to by RSDP */
     xsdt = tables_blob->len;
     build_xsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
@@ -339,8 +344,7 @@ static Aml *ged_event_aml(GedEvent *event)
         return aml_call0("\\_SB.MHPC." MEMORY_SLOT_SCAN_METHOD);
     case GED_PCI_HOTPLUG:
     case GED_NVDIMM_HOTPLUG:
-        /* Not supported for now */
-        return NULL;
+        return aml_notify(aml_name("\\_SB.NVDR"), aml_int(0x80));
     default:
         break;
     }
Signature · 1 line
2.17.2
#31 lore raw

PATCH 26/26 hw: acpi: reduced: Add PCI hotplug support

In reply to Samuel Ortiz

From: Sebastien Boeuf <[email protected]>

In order to support PCI hotplug through a hardware-reduced GED, we need to modify the GED device definition in the DSDT table, so that a PCI hotplug related interrupt will trigger a new PCI scan.
We also need to modify the DSDT PCI bus definition in order to make sure a PCI scan of all available slots can be performed when an interrupt comes in.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Igor Mammedov <[email protected]>
---
 hw/acpi/aml-build.c | 8 +++++++-
 hw/acpi/reduced.c   | 7 +++++++
 2 files changed, 14 insertions(+), 1 deletion(-)
hw/acpi/aml-build.c +7 -1
index 752642a67a..99f04a3e71 100644
@@ -2510,7 +2510,7 @@ void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
 
 void acpi_dsdt_add_pci_bus(Aml *dsdt, AcpiPciBus *pci_host)
 {
-    Aml *dev, *pci_scope;
+    Aml *dev, *pci_scope, *hp_scope;
 
     dev = aml_device("\\_SB.PCI0");
     aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
@@ -2522,6 +2522,12 @@ void acpi_dsdt_add_pci_bus(Aml *dsdt, AcpiPciBus *pci_host)
     aml_append(dev, build_osc_method(0x1F));
     aml_append(dsdt, dev);
 
+    /* PCIHP */
+    hp_scope =  aml_scope("\\_SB.PCI0");
+    build_acpi_pci_hotplug(hp_scope);
+    build_append_pci_bus_devices(hp_scope, pci_host->pci_bus, false);
+    aml_append(dsdt, hp_scope);
+
     pci_scope = build_pci_host_bridge(dsdt, pci_host);
     aml_append(dsdt, pci_scope);
 }
hw/acpi/reduced.c +7 -0
index fac19a978a..7faccdc634 100644
@@ -331,6 +331,8 @@ void acpi_reduced_setup(MachineState *machine, AcpiConfiguration *conf)
 
 static Aml *ged_event_aml(GedEvent *event)
 {
+    Aml *method;
+
     if (!event) {
         return NULL;
     }
@@ -343,6 +345,11 @@ static Aml *ged_event_aml(GedEvent *event)
         /* We run a complete memory SCAN when getting a memory hotplug event */
         return aml_call0("\\_SB.MHPC." MEMORY_SLOT_SCAN_METHOD);
     case GED_PCI_HOTPLUG:
+        /* Take the PCI lock and trigger a PCI rescan */
+        method = aml_acquire(aml_name("\\_SB.PCI0.BLCK"), 0xFFFF);
+        aml_append(method, aml_call0("\\_SB.PCI0.PCNT"));
+        aml_append(method, aml_release(aml_name("\\_SB.PCI0.BLCK")));
+        return method;
     case GED_NVDIMM_HOTPLUG:
         return aml_notify(aml_name("\\_SB.NVDR"), aml_int(0x80));
     default:
Signature · 1 line
2.17.2
#32 lore raw
In reply to Samuel Ortiz
Quoted 17 lines · Samuel Ortiz

This patch set implements support for the ACPI hardware-reduced specification.

The changes are coming from the NEMU [1] project where we're defining a new x86 machine type: i386/virt. This is an EFI only, ACPI hardware-reduced platform and as such we had to implement support for the latter.

As a preliminary for adding hardware-reduced support to QEMU, we did some ACPI code reorganization with the following goals:

* Share as much as possible of the current ACPI build APIs between legacy and hardware-reduced ACPI.
* Share the ACPI build code across machine types and architectures and remove the typical PC machine type dependency.
Eventually we hope to see arm/virt also re-use much of that code.

Quoted 5 lines

The rest of the patchset adds the hardware-reduced support on top of this code reorganization. Here again, the implementation is machine type, platform and architecture independent.

[1] https://github.com/intel/nemu

Thanks for the patch!
I'm traveling so won't be able to review until next week.

I wonder whether the code can be refactored slightly differently:
instead of common code calling out to platform specific one, have platform code call out to generic one.
That would avoid indicectiin but I'm not sure at what cost in complexity.

Still that's a generic question as I have not looked at the patches in depth.

Hope this helps,
MST

Quoted 84 lines

The following changes since commit b312532fd03413d0e6ae6767ec793a3e30f487b8:

Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2018-10-19 19:01:07 +0100)

are available in the Git repository at:

[email protected]:intel/nemu topic/upstream/acpi

for you to fetch changes up to 4133ebe0d638d0a3d583bd89aa2360ef35ba746b:

hw: acpi: reduced: Add PCI hotplug support (2018-10-22 20:32:23 +0200)

----------------------------------------------------------------
Jing Liu (2):
hw: acpi: reduced: Add shutdown support
hw: acpi: reduced: Add reboot support

Samuel Ortiz (10):
hw: i386: Decouple the ACPI build from the PC machine type
hw: acpi: Export ACPI build alignment API
hw: acpi: Export the RSDP build API
hw: arm: Switch to the AML build RSDP building routine
hw: acpi: Do not create hotplug method when handler is not defined
hw: i386: Make the hotpluggable memory size property more generic
hw: fw-build: Add firmware build methods and state
hw: i386: Convert PC machine type to firmware build methods
hw: acpi: Initial hardware-reduced support
hw: acpi: reduced: Generic Event Device support

Sebastien Boeuf (4):
hw: acpi: Export the PCI hotplug API
hw: acpi: Retrieve the PCI bus from AcpiPciHpState
hw: acpi: reduced: Add NFIT support
hw: acpi: reduced: Add PCI hotplug support

Yang Zhong (10):
hw: acpi: Generalize AML build routines
hw: acpi: Factorize _OSC AML across architectures
hw: i386: Refactor PCI host getter
hw: acpi: Export and generalize the PCI host AML API
hw: acpi: Export the MCFG getter
hw: acpi: Export the SRAT AML build API
hw: acpi: Fix memory hotplug AML generation error
hw: acpi: reduced: Add MCFG support
hw: acpi: reduced: Add memory hotplug support
hw: acpi: reduced: Add SRAT table

 default-configs/i386-softmmu.mak |    1 +
 hw/acpi/Makefile.objs            |    1 +
 hw/acpi/aml-build.c              | 1139 +++++++++++++++++++++++++++++++++++-
 hw/acpi/cpu.c                    |    8 +-
 hw/acpi/cpu_hotplug.c            |    9 +-
 hw/acpi/memory_hotplug.c         |   21 +-
 hw/acpi/pcihp.c                  |   10 +-
 hw/acpi/reduced.c                |  472 +++++++++++++++
 hw/arm/virt-acpi-build.c         |   86 +--
 hw/i386/acpi-build.c             | 1172 +++-----------------------------------
 hw/i386/acpi-build.h             |    4 +-
 hw/i386/pc.c                     |  188 +++---
 hw/i386/pc_piix.c                |   21 +-
 hw/i386/pc_q35.c                 |   21 +-
 hw/pci-host/piix.c               |    8 -
 include/hw/acpi/acpi-defs.h      |   15 +
 include/hw/acpi/acpi.h           |   47 ++
 include/hw/acpi/aml-build.h      |   48 ++
 include/hw/acpi/reduced.h        |   46 ++
 include/hw/boards.h              |    5 +
 include/hw/fw-build.h            |   57 ++
 include/hw/i386/acpi.h           |   27 +
 include/hw/i386/pc.h             |   21 +-
 include/hw/mem/memory-device.h   |    2 +
 stubs/Makefile.objs              |    1 -
 stubs/pci-host-piix.c            |    6 -
 26 files changed, 2107 insertions(+), 1329 deletions(-)
 create mode 100644 hw/acpi/reduced.c
 create mode 100644 include/hw/acpi/reduced.h
 create mode 100644 include/hw/fw-build.h
 create mode 100644 include/hw/i386/acpi.h
 delete mode 100644 stubs/pci-host-piix.c
Signature · 1 line
2.17.2
#33 lore raw
Quoted 22 lines · Michael S. Tsirkin
Quoted 21 lines · Samuel Ortiz

This patch set implements support for the ACPI hardware-reduced specification.

The changes are coming from the NEMU [1] project where we're defining a new x86 machine type: i386/virt. This is an EFI only, ACPI hardware-reduced platform and as such we had to implement support for the latter.

As a preliminary for adding hardware-reduced support to QEMU, we did some ACPI code reorganization with the following goals:

* Share as much as possible of the current ACPI build APIs between legacy and hardware-reduced ACPI.
* Share the ACPI build code across machine types and architectures and remove the typical PC machine type dependency.
Eventually we hope to see arm/virt also re-use much of that code.

The rest of the patchset adds the hardware-reduced support on top of this code reorganization. Here again, the implementation is machine type, platform and architecture independent.

Quoted 5 lines
Quoted 1 line

[1] https://github.com/intel/nemu

Thanks for the patch!
I'm traveling so won't be able to review until next week.

No problem. I'm assuming you'll be attending the KVM Forum, Rob Bradford and myself will be there as well.

Quoted 5 lines

I wonder whether the code can be refactored slightly differently:
instead of common code calling out to platform specific one, have platform code call out to generic one.
That would avoid indicectiin but I'm not sure at what cost in complexity.

I may be misunderstanding your point, so forgive me if that's the case. But the code refactoring actually lead to both of the code paths you're describing. The generic code calls into platform specific hooks for potentially calling custom ACPI tables, but the platform code also uses the now shared generic code.

Quoted 2 lines

Still that's a generic question as I have not looked at the patches in depth.

Looking forward to getting your review.

Cheers,
Samuel.

#34 lore raw
Quoted 4 lines · Samuel Ortiz
Quoted 4 lines

I wonder whether the code can be refactored slightly differently:
instead of common code calling out to platform specific one, have platform code call out to generic one.
That would avoid indicectiin

should have been indirection

Quoted 7 lines
Quoted 2 lines

but I'm not sure at what cost
in complexity.

I may be misunderstanding your point, so forgive me if that's the case. But the code refactoring actually lead to both of the code paths you're describing. The generic code calls into platform specific hooks for potentially calling custom ACPI tables, but the platform code also uses the now shared generic code.

Right. So if instead of both we could have only platform calling generic code, we could drop some of the hooks.

What I'm saying is that if I look at patches 1-17 which are supposed to be just refactorings, somehow at the surface instead of reducing lines of code they add 100 LOC.

And the whole patchset adds about 1000 LOC, by comparison e.g. pc_q35.c is just 400 LOC since q35 and pc share a bunch of code. I wonder why can't more code be shared with e.g. ARM which also has a virt machine type. In other words, assuming we are doing PV, can't it be more portable.

Again all that's still because all I read is the diffstats, it's without reading the patches, sorry about that.

Signature · 1 line
MST
#35 lore raw
In reply to Samuel Ortiz
Quoted 28 lines · Samuel Ortiz

This patch set implements support for the ACPI hardware-reduced specification.

The changes are coming from the NEMU [1] project where we're defining a new x86 machine type: i386/virt. This is an EFI only, ACPI hardware-reduced platform and as such we had to implement support for the latter.

As a preliminary for adding hardware-reduced support to QEMU, we did some ACPI code reorganization with the following goals:

* Share as much as possible of the current ACPI build APIs between legacy and hardware-reduced ACPI.
* Share the ACPI build code across machine types and architectures and remove the typical PC machine type dependency.
Eventually we hope to see arm/virt also re-use much of that code.

The rest of the patchset adds the hardware-reduced support on top of this code reorganization. Here again, the implementation is machine type, platform and architecture independent.

[1] https://github.com/intel/nemu

The following changes since commit b312532fd03413d0e6ae6767ec793a3e30f487b8:

Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2018-10-19 19:01:07 +0100)

Quoted 7 lines

are available in the Git repository at:

[email protected]:intel/nemu topic/upstream/acpi

for you to fetch changes up to 4133ebe0d638d0a3d583bd89aa2360ef35ba746b:

hw: acpi: reduced: Add PCI hotplug support (2018-10-22 20:32:23 +0200)

Hi -- I'd appreciate it if you don't send pull-requests for patch series. They show up in my queue of things to merge directly into master otherwise. Just send them as a patch-series email with a cover letter, please.

thanks
-- PMM

#36 lore raw

Hi Peter,

Quoted 33 lines · Peter Maydell
Quoted 32 lines · Samuel Ortiz

This patch set implements support for the ACPI hardware-reduced specification.

The changes are coming from the NEMU [1] project where we're defining a new x86 machine type: i386/virt. This is an EFI only, ACPI hardware-reduced platform and as such we had to implement support for the latter.

As a preliminary for adding hardware-reduced support to QEMU, we did some ACPI code reorganization with the following goals:

* Share as much as possible of the current ACPI build APIs between legacy and hardware-reduced ACPI.
* Share the ACPI build code across machine types and architectures and remove the typical PC machine type dependency.
Eventually we hope to see arm/virt also re-use much of that code.

The rest of the patchset adds the hardware-reduced support on top of this code reorganization. Here again, the implementation is machine type, platform and architecture independent.

[1] https://github.com/intel/nemu

The following changes since commit b312532fd03413d0e6ae6767ec793a3e30f487b8:

Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2018-10-19 19:01:07 +0100)

are available in the Git repository at:

[email protected]:intel/nemu topic/upstream/acpi

Quoted 8 lines
Quoted 3 lines

for you to fetch changes up to 4133ebe0d638d0a3d583bd89aa2360ef35ba746b:

hw: acpi: reduced: Add PCI hotplug support (2018-10-22 20:32:23 +0200)

Hi -- I'd appreciate it if you don't send pull-requests for patch series. They show up in my queue of things to merge directly into master otherwise. Just send them as a patch-series email with a cover letter, please.

Apologies, I did not know. I'll fix that with the next revision of this patch set.

Cheers,
Samuel.

#37 lore raw
Quoted 9 lines · Samuel Ortiz

Hi Peter,

Quoted 4 lines · Peter Maydell

Hi -- I'd appreciate it if you don't send pull-requests for patch series. They show up in my queue of things to merge directly into master otherwise. Just send them as a patch-series email with a cover letter, please.

Apologies, I did not know. I'll fix that with the next revision of this patch set.

No problem. You can give a git branch URL for reviewers to look at if you think it's helpful; just avoid using the stock git pull-request email wording about 'for you to fetch changes', because that's what my email filters look for :-)

thanks
-- PMM

#38 lore raw
In reply to Samuel Ortiz
Quoted 17 lines · Samuel Ortiz

This patch set implements support for the ACPI hardware-reduced specification.

The changes are coming from the NEMU [1] project where we're defining a new x86 machine type: i386/virt. This is an EFI only, ACPI hardware-reduced platform and as such we had to implement support for the latter.

As a preliminary for adding hardware-reduced support to QEMU, we did some ACPI code reorganization with the following goals:

* Share as much as possible of the current ACPI build APIs between legacy and hardware-reduced ACPI.
* Share the ACPI build code across machine types and architectures and remove the typical PC machine type dependency.
Eventually we hope to see arm/virt also re-use much of that code.

Quoted 5 lines

The rest of the patchset adds the hardware-reduced support on top of this code reorganization. Here again, the implementation is machine type, platform and architecture independent.

[1] https://github.com/intel/nemu

I think the first part of the series is very valuable, and also the FirmwareBuildMethods idea is good even though I have some comments on how to model it in C. It won't make it in 3.1, but I suppose you expected that.

Including the second part in QEMU however would be premature, since it is not used by anything. As mentioned earlier, my suggestion is that you convert the ARM virt machine type to FirmwareBuildMethods and HW-reduced ACPI. It shouldn't be a large amount of work and it should let you reduce the ACPI delta between QEMU and NEMU.

Also, I would like to understand if you NEMU people are interested in refactoring the ACPI builder code to move DSDT-building code closer to the device, or perhaps have already done that. For example, the infamous floppy cannot be disabled right now because of the call to isa_fdc_get_drive_type in hw/i386/acpi-build.c. As I understand it, your initial patches have the side effect of letting you bypass this issue---because you don't include hw/i386/acpi-build.c in your virt machine type. That is totally fine, but of course it doesn't help if you want to compile the floppy out of a QEMU that supports -M q35 but not -M pc. Therefore I would love to know that I'm wrong, or just that you're planning to fix that too. :)

Paolo