ACPI reorganization for hardware-reduced support
v4 cover ACPI reorganization for hardware-reduced support
This patch set provides an ACPI code reorganization in preparation for adding hardware-reduced support to QEMU.
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 patches are also available in their own git branch [2].
[1] https://github.com/intel/nemu
[2] https://github.com/intel/nemu/tree/topic/upstream/acpi
v1 -> v2:
* Drop the hardware-reduced implementation for now. Our next patch set will add hardware-reduced and convert arm/virt to it.
* Implement the ACPI build methods as a QOM Interface Class and convert the PC machine type to it.
* acpi_conf_pc_init() uses a PCMachineState pointer and not a MachineState one as its argument.
v2 -> v3:
* Cc all relevant maintainers, no functional changes.
v3 -> v4:
* Renamed all AcpiConfiguration pointers from conf to acpi_conf.
* Removed the ACPI_BUILD_ALIGN_SIZE export.
* Temporarily updated the arm virt build_rsdp() prototype for bisectability purposes.
* Removed unneeded pci headers from acpi-build.c.
* Refactor the acpi PCI host getter so that it truly is architecture agnostic, by carrying the PCI host pointer through the AcpiConfiguration structure.
* Splitted the PCI host AML builder API export patch from the PCI host and holes getter one.
* Reduced the build_srat() export scope to hw/i386 instead of the broader hw/acpi. SRAT builders are truly architecture specific and can hardly be generalized.
* Completed the ACPI builder documentation.
Samuel Ortiz (15):
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: acpi: Implement XSDT support for RSDP
hw: arm: Switch to the AML build RSDP building routine
hw: i386: Move PCI host definitions to pci_host.h
hw: acpi: Export the PCI host and holes getters
hw: acpi: Do not create hotplug method when handler is not defined
hw: i386: Make the hotpluggable memory size property more generic
hw: i386: Export the i386 ACPI SRAT build method
hw: i386: Export the MADT build method
hw: acpi: Define ACPI tables builder interface
hw: i386: Implement the ACPI builder interface for PC
hw: pci-host: piix: Return PCI host pointer instead of PCI bus
hw: i386: Set ACPI configuration PCI host pointer
Sebastien Boeuf (2):
hw: acpi: Export the PCI hotplug API
hw: acpi: Retrieve the PCI bus from AcpiPciHpState
Yang Zhong (6):
hw: acpi: Generalize AML build routines
hw: acpi: Factorize _OSC AML across architectures
hw: acpi: Export and generalize the PCI host AML API
hw: acpi: Export the MCFG getter
hw: acpi: Fix memory hotplug AML generation error
hw: i386: Refactor PCI host getter
hw/acpi/Makefile.objs | 1 + hw/acpi/aml-build.c | 985 +++++++++++++++++++++++++++++ hw/acpi/builder.c | 97 +++ hw/acpi/cpu.c | 8 +- hw/acpi/cpu_hotplug.c | 9 +- hw/acpi/memory_hotplug.c | 21 +- hw/acpi/pcihp.c | 10 +- hw/arm/virt-acpi-build.c | 94 +-- hw/i386/acpi-build.c | 1071 +++----------------------------- hw/i386/acpi-build.h | 9 +- hw/i386/pc.c | 198 +++--- hw/i386/pc_piix.c | 36 +- hw/i386/pc_q35.c | 22 +- hw/i386/xen/xen-hvm.c | 19 +- hw/pci-host/piix.c | 32 +- include/hw/acpi/acpi-defs.h | 14 + include/hw/acpi/acpi.h | 44 ++ include/hw/acpi/aml-build.h | 47 ++ include/hw/acpi/builder.h | 100 +++ include/hw/i386/acpi.h | 27 + include/hw/i386/pc.h | 49 +- include/hw/mem/memory-device.h | 2 + include/hw/pci/pci_host.h | 6 + stubs/Makefile.objs | 1 - stubs/pci-host-piix.c | 6 - 25 files changed, 1648 insertions(+), 1260 deletions(-) create mode 100644 hw/acpi/builder.c create mode 100644 include/hw/acpi/builder.h create mode 100644 include/hw/i386/acpi.h delete mode 100644 stubs/pci-host-piix.c
Signature · 1 line
2.19.1
PATCH v4 1/23 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
--- hw/acpi/cpu_hotplug.c | 9 +- hw/arm/virt-acpi-build.c | 10 --- hw/i386/acpi-build.c | 136 ++++++++++++++---------------- hw/i386/acpi-build.h | 4 +- hw/i386/pc.c | 176 ++++++++++++++++++++++++--------------- hw/i386/pc_piix.c | 21 ++--- hw/i386/pc_q35.c | 21 ++--- hw/i386/xen/xen-hvm.c | 19 +++-- include/hw/acpi/acpi.h | 44 ++++++++++ include/hw/i386/pc.h | 19 ++--- 10 files changed, 257 insertions(+), 202 deletions(-)
hw/acpi/cpu_hotplug.c +5 -4
@@ -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
@@ -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 +61 -75
342 lines not on this page · raw message
hw/i386/acpi-build.h +3 -1
@@ -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 *acpi_conf);#endif
hw/i386/pc.c +107 -69
478 lines not on this page · raw message
hw/i386/pc_piix.c +11 -10
@@ -71,6 +71,7 @@ static void pc_init1(MachineState *machine, {PCMachineState *pcms =PC_MACHINE (machine);PCMachineClass *pcmc =PC_MACHINE_GET_CLASS (pcms); +AcpiConfiguration *acpi_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; + acpi_conf->above_4g_mem_size = machine->ram_size - lowmem; + acpi_conf->below_4g_mem_size = lowmem; }else { - pcms->above_4g_mem_size =0 ; - pcms->below_4g_mem_size = machine->ram_size; + acpi_conf->above_4g_mem_size =0 ; + acpi_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, + acpi_conf->below_4g_mem_size, + acpi_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 **)&acpi_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 (acpi_conf->acpi_nvdimm_state.is_enabled) { +nvdimm_init_acpi_state (&acpi_conf->acpi_nvdimm_state, system_io, + acpi_conf->fw_cfg,OBJECT (pcms)); } }
hw/i386/pc_q35.c +11 -10
@@ -63,6 +63,7 @@ static void pc_q35_init(MachineState *machine) {PCMachineState *pcms =PC_MACHINE (machine);PCMachineClass *pcmc =PC_MACHINE_GET_CLASS (pcms); +AcpiConfiguration *acpi_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; + acpi_conf->above_4g_mem_size = machine->ram_size - lowmem; + acpi_conf->below_4g_mem_size = lowmem; }else { - pcms->above_4g_mem_size =0 ; - pcms->below_4g_mem_size = machine->ram_size; + acpi_conf->above_4g_mem_size =0 ; + acpi_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), acpi_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), acpi_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 **)&acpi_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 (acpi_conf->acpi_nvdimm_state.is_enabled) { +nvdimm_init_acpi_state (&acpi_conf->acpi_nvdimm_state, system_io, + acpi_conf->fw_cfg,OBJECT (pcms)); } }
hw/i386/xen/xen-hvm.c +10 -9
@@ -190,6 +190,7 @@ qemu_irq *xen_interrupt_controller_init(void)static void xen_ram_init (PCMachineState *pcms,ram_addr_t ram_size,MemoryRegion **ram_memory_p) { +AcpiConfiguration *acpi_conf = &pcms->acpi_configuration;MemoryRegion *sysmem =get_system_memory ();ram_addr_t block_len; uint64_t user_lowmem = object_property_get_uint(qdev_get_machine(),
@@ -207,20 +208,20 @@ static void xen_ram_init(PCMachineState *pcms, }if (ram_size >= user_lowmem) { - pcms->above_4g_mem_size = ram_size - user_lowmem; - pcms->below_4g_mem_size = user_lowmem; + acpi_conf->above_4g_mem_size = ram_size - user_lowmem; + acpi_conf->below_4g_mem_size = user_lowmem; }else { - pcms->above_4g_mem_size =0 ; - pcms->below_4g_mem_size = ram_size; + acpi_conf->above_4g_mem_size =0 ; + acpi_conf->below_4g_mem_size = ram_size; } -if (!pcms->above_4g_mem_size) { +if (!acpi_conf->above_4g_mem_size) { block_len = ram_size; }else {/* * Xen does not allocate the memory continuously, it keeps a * hole of the size computed above or passed in. */ - block_len = (1ULL <<32 ) + pcms->above_4g_mem_size; + block_len = (1ULL <<32 ) + acpi_conf->above_4g_mem_size; }memory_region_init_ram (&ram_memory,NULL ,"xen.ram" , block_len, &error_fatal);
@@ -237,12 +238,12 @@ static void xen_ram_init(PCMachineState *pcms,*/ memory_region_init_alias (&ram_lo,NULL ,"xen.ram.lo" , &ram_memory,0xc0000 , - pcms->below_4g_mem_size -0xc0000 ); + acpi_conf->below_4g_mem_size -0xc0000 );memory_region_add_subregion (sysmem,0xc0000 , &ram_lo); -if (pcms->above_4g_mem_size >0 ) { +if (acpi_conf->above_4g_mem_size >0 ) {memory_region_init_alias (&ram_hi,NULL ,"xen.ram.hi" , &ram_memory,0x100000000ULL , - pcms->above_4g_mem_size); + acpi_conf->above_4g_mem_size);memory_region_add_subregion (sysmem,0x100000000ULL , &ram_hi); } }
include/hw/acpi/acpi.h +44 -0
56 lines not on this page · raw message
include/hw/i386/pc.h +5 -14
@@ -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 · 7 lines
2.19.1 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH v4 2/23 hw: acpi: Export ACPI build alignment API
In reply to Samuel OrtizThis is going to be needed by the Hardware-reduced ACPI routines.
--- 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
@@ -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
@@ -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
@@ -384,6 +384,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); +/* Align AML blob size to a multiple of 'align' */ +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.19.1
PATCH v4 3/23 hw: acpi: Export the RSDP build API
In reply to Samuel OrtizThe hardware-reduced API will need to build RSDP as well, so we should export this routine. While doing so, we also slightly change the function prototype. Since no caller needs it, and to make it more consistent with the rest of the AML build API, the function no longer returns its RSDP table.
--- hw/acpi/aml-build.c | 24 ++++++++++++++++++++++++ hw/arm/virt-acpi-build.c | 2 +- hw/i386/acpi-build.c | 26 -------------------------- include/hw/acpi/aml-build.h | 3 +++ 4 files changed, 28 insertions(+), 27 deletions(-)
hw/acpi/aml-build.c +24 -0
@@ -1651,6 +1651,30 @@ build_xsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets, (void *)xsdt, "XSDT", xsdt_len, 1, oem_id, oem_table_id); } +void +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); +} +void build_srat_memory (AcpiSratMemoryAffinity *numamem,uint64_t base,uint64_t len,int node,MemoryAffinityFlags flags) {
hw/arm/virt-acpi-build.c +1 -1
@@ -367,7 +367,7 @@ static void acpi_dsdt_add_power_button(Aml *scope) }/* RSDP */ -static GArray * +static void build_rsdp (GArray *rsdp_table,BIOSLinker *linker,unsigned xsdt_tbl_offset) {AcpiRsdpDescriptor *rsdp =acpi_data_push (rsdp_table,sizeof *rsdp);
hw/i386/acpi-build.c +0 -26
@@ -2513,32 +2513,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;
include/hw/acpi/aml-build.h +3 -0
@@ -390,6 +390,9 @@ 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);void +build_rsdp (GArray *table_data, +BIOSLinker *linker,unsigned rsdt_tbl_offset); +void build_rsdt (GArray *table_data,BIOSLinker *linker,GArray *table_offsets,const char *oem_id,const char *oem_table_id);void
Signature · 1 line
2.19.1
Re: [Qemu-arm] [Qemu-devel] [PATCH v4 03/23] hw: acpi: Export the RSDP build API
Hey Samuel,
Quoted 10 lines · Samuel Ortiz
The hardware-reduced API will need to build RSDP as well, so we should export this routine. While doing so, we also slightly change the function prototype. Since no caller needs it, and to make it more consistent with the rest of the AML build API, the function no longer returns its RSDP table.
Signed-off-by Samuel Ortiz <[email protected]>--- hw/acpi/aml-build.c | 24 ++++++++++++++++++++++++ hw/arm/virt-acpi-build.c | 2 +-
When you modify files from architectures you are not custom to use, you should verify your changes don't break them ;)
Quoted 44 lines
hw/i386/acpi-build.c | 26 -------------------------- include/hw/acpi/aml-build.h | 3 +++ 4 files changed, 28 insertions(+), 27 deletions(-)hw/acpi/aml-build.c +24
-0@@ -1651,6 +1651,30 @@ build_xsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets, (void *)xsdt, "XSDT", xsdt_len, 1, oem_id, oem_table_id); } +void +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); +} +void build_srat_memory (AcpiSratMemoryAffinity *numamem,uint64_t base,uint64_t len,int node,MemoryAffinityFlags flags) {hw/arm/virt-acpi-build.c +0
-0
hw/arm/virt-acpi-build.c static void acpi_dsdt_add_power_button(Aml *scope) · line 372
Quoted 7 lines
hw/arm/virt-acpi-build.c +1
-1@@ -367,7 +367,7 @@ static void acpi_dsdt_add_power_button(Aml *scope) }/* RSDP */ -static GArray * +static void build_rsdp (GArray *rsdp_table,BIOSLinker *linker,unsigned xsdt_tbl_offset) {
Since this function doesn't return anything now, you can simply remove the last line to avoid:
hw/arm/virt-acpi-build.c: In function ‘build_rsdp’:hw/arm/virt-acpi-build.c:396:12: error: ‘return’ with a value, in function returning void [-Werror]
return rsdp_table;
^~~~~~~~~~(I used 'make subdir-aarch64-softmmu' to build).
Also since you declare the prototype in "hw/acpi/aml-build.h", you have to remove the 'static' keyword:
hw/arm/virt-acpi-build.c:371:1: error: static declaration of ‘build_rsdp’ follows non-static declaration
build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsignedxsdt_tbl_offset)
^~~~~~~~~~hw/arm/virt-acpi-build.c:42:include/hw/acpi/aml-build.h:393:1: note: previous declaration of ‘build_rsdp’ was here
build_rsdp(GArray *table_data,
^~~~~~~~~~
Maybe I was not clear enough in my previous review, I suggest doing this in 3 steps:
1: both functions return void (still static)
2: expose the prototype declaration in "hw/acpi/aml-build.h" (remove static keyword) 3: unify codebase moving build_rsdp() to hw/acpi/aml-build.c
steps 1+2 could be merged, but IMHO 3 steps are cleaner.
Quoted 53 lines
hw/arm/virt-acpi-build.c +0
-0AcpiRsdpDescriptor *rsdp =acpi_data_push (rsdp_table,sizeof *rsdp);hw/i386/acpi-build.c +0
-26@@ -2513,32 +2513,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;include/hw/acpi/aml-build.h +3
-0@@ -390,6 +390,9 @@ 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);void +build_rsdp (GArray *table_data, +BIOSLinker *linker,unsigned rsdt_tbl_offset); +void build_rsdt (GArray *table_data,BIOSLinker *linker,GArray *table_offsets,const char *oem_id,const char *oem_table_id);void
Re: [Qemu-arm] [Qemu-devel] [PATCH v4 03/23] hw: acpi: Export the RSDP build API
In reply to Samuel OrtizHi,
hw/arm/virt-acpi-build.c static void acpi_dsdt_add_power_button(Aml *scope) · line 373
Quoted 13 lines · Samuel Ortiz
hw/arm/virt-acpi-build.c +1
-1@@ -367,7 +367,7 @@ static void acpi_dsdt_add_power_button(Aml *scope) }/* RSDP */ -static GArray * +static void build_rsdp (GArray *rsdp_table,BIOSLinker *linker,unsigned xsdt_tbl_offset) {AcpiRsdpDescriptor *rsdp =acpi_data_push (rsdp_table,sizeof *rsdp);
Why change this? It's not related to your patch purpose.
Thanks,
Shannon
Re: [Qemu-arm] [Qemu-devel] [PATCH v4 03/23] hw: acpi: Export the RSDP build API
Hi,
Quoted 17 lines · Shannon Zhao
Hi,
hw/arm/virt-acpi-build.c static void acpi_dsdt_add_power_button(Aml *scope) · line 372
Quoted 12 lines · Samuel Ortiz
hw/arm/virt-acpi-build.c +1
-1@@ -367,7 +367,7 @@ static void acpi_dsdt_add_power_button(Aml *scope) } /* RSDP */ -static GArray * +static void build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned xsdt_tbl_offset) { AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);Why change this? It's not related to your patch purpose.
This patch updates include/hw/acpi/aml-build.h to export the build_rsdp() function. Since this file includes this header, the orototype needs to match.
Regards,
Phil.
Quoted 4 lines
Thanks,
Shannon
Re: [Qemu-arm] [Qemu-devel] [PATCH v4 03/23] hw: acpi: Export the RSDP build API
Quoted 13 lines · Philippe Mathieu-Daudé
Quoted 12 lines
Quoted 10 lines
}
/* RSDP */hw/arm/virt-acpi-build.c +1
-1-static GArray * +static void
build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned xsdt_tbl_offset)
{
AcpiRsdpDescriptor *rsdp =acpi_data_push(rsdp_table, sizeof *rsdp);Why change this? It's not related to your patch purpose.
Quoted 3 lines
This patch updates
include/hw/acpi/aml-build.hto export thebuild_rsdp()function.
Since this file includes this header, the orototype needs to match.
But then it should also remove the "return". I think something is broken for bisectability between this patch and patch 4.
Perhaps this patch should already rename the function to build_rsdp_rsdt, and patch 4 should move build_rsdp from hw/arm to hw/acpi?
Paolo
PATCH v4 4/23 hw: acpi: Implement XSDT support for RSDP
In reply to Samuel OrtizXSDT is the 64-bit version of the legacy ACPI RSDT (Root System Description Table). RSDT only allow for 32-bit addressses and have thus been deprecated. Since ACPI version 2.0, RSDPs should point at XSDTs and no longer RSDTs, although RSDTs are still supported for backward compatibility.
Since version 2.0, RSDPs should add an extended checksum, a complete table length and a version field to the table.
Here we provide a legacy API (RSDP points at RSDTs) for backward compatibility purpose and a default, XSDT pointing API.
--- hw/acpi/aml-build.c | 41 ++++++++++++++++++++++++++++++++++++- hw/i386/acpi-build.c | 2 +- include/hw/acpi/aml-build.h | 5 ++++- 3 files changed, 45 insertions(+), 3 deletions(-)
hw/acpi/aml-build.c +40 -1
@@ -1651,8 +1651,10 @@ 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 */ void -build_rsdp (GArray *rsdp_table,BIOSLinker *linker,unsigned rsdt_tbl_offset) +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);
@@ -1675,6 +1677,43 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset) (char *)&rsdp->checksum - rsdp_table->data); } +/* RSDP pointing at an XSDT */ +void +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); +} +void build_srat_memory (AcpiSratMemoryAffinity *numamem,uint64_t base,uint64_t len,int node,MemoryAffinityFlags flags) {
hw/i386/acpi-build.c +1 -1
@@ -2657,7 +2657,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 -1
@@ -390,8 +390,11 @@ 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);void +build_rsdp_rsdt (GArray *table_data, +BIOSLinker *linker,unsigned rsdt_tbl_offset); +void build_rsdp (GArray *table_data, -BIOSLinker *linker,unsigned rsdt_tbl_offset); +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.19.1
PATCH v4 5/23 hw: arm: Switch to the AML build RSDP building routine
In reply to Samuel OrtizWe 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.
--- hw/arm/virt-acpi-build.c | 31 +------------------------------ 1 file changed, 1 insertion(+), 30 deletions(-)
hw/arm/virt-acpi-build.c +1 -30
@@ -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 void -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.19.1
Re: [Qemu-arm] [Qemu-devel] [PATCH v4 05/23] hw: arm: Switch to the AML build RSDP building routine
Quoted 23 lines · Samuel Ortiz
We make the ARM virt ACPI code use the now shared
build_rsdp()API fromaml-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.Signed-off-by Samuel Ortiz <[email protected]>--- hw/arm/virt-acpi-build.c | 31 +------------------------------ 1 file changed, 1 insertion(+), 30 deletions(-)hw/arm/virt-acpi-build.c +1
-0@@ -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"
hw/arm/virt-acpi-build.c static void acpi_dsdt_add_power_button(Aml *scope) · line 371
Quoted 6 lines
hw/arm/virt-acpi-build.c +0
-3@@ -366,36 +367,6 @@ static void acpi_dsdt_add_power_button(Aml *scope)aml_append (scope, dev); } -/* RSDP */ -static void -build_rsdp (GArray *rsdp_table,BIOSLinker *linker,unsigned xsdt_tbl_offset)
Note: here we use xsdt table not rsdt for ARM. After your change which assigns xsdt table address to rsdt_physical_address, it doesn't work.
IIRC, we discussed this before that ARM virt uses xsdt while pc/q35 uses rsdt. So this patch is not necessary I think.
Thanks,
Shannon
Quoted 31 lines
hw/arm/virt-acpi-build.c +0
-27-{ -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) {
Re: [Qemu-arm] [Qemu-devel] [PATCH v4 05/23] hw: arm: Switch to the AML build RSDP building routine
Quoted 25 lines · Shannon Zhao
Quoted 22 lines · Samuel Ortiz
We make the ARM virt ACPI code use the now shared
build_rsdp()API fromaml-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.Signed-off-by Samuel Ortiz <[email protected]>---
hw/arm/virt-acpi-build.c| 31 +------------------------------1 file changed, 1 insertion(+), 30 deletions(-)hw/arm/virt-acpi-build.c +1
-0@@ -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"
Quoted 9 lines
hw/arm/virt-acpi-build.c static void acpi_dsdt_add_power_button(Aml *scope) · line 369
Quoted 7 lines
hw/arm/virt-acpi-build.c +0
-3@@ -366,36 +367,6 @@ static void acpi_dsdt_add_power_button(Aml *scope) aml_append(scope, dev); } -/* RSDP */ -static void -build_rsdp (GArray *rsdp_table,BIOSLinker *linker,unsigned xsdt_tbl_offset)Note: here we use xsdt table not rsdt for ARM. After your change which assigns xsdt table address to
rsdt_physical_address, it doesn't work.
Oops. I didn't notice your patch "[PATCH v4 04/23] hw: acpi: Implement XSDT support for RSDP".
Thanks,
Shannon
Re: [Qemu-arm] [Qemu-devel] [PATCH v4 05/23] hw: arm: Switch to the AML build RSDP building routine
In reply to Shannon ZhaoQuoted 27 lines · On Fri, 2 Nov 2018 17:35:06 +0800 Shannon Zhao <[email protected]> wrote:
Quoted 26 lines · Samuel Ortiz
We make the ARM virt ACPI code use the now shared
build_rsdp()API fromaml-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.Signed-off-by Samuel Ortiz <[email protected]>--- hw/arm/virt-acpi-build.c | 31 +------------------------------ 1 file changed, 1 insertion(+), 30 deletions(-)hw/arm/virt-acpi-build.c +1
-0@@ -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); }
Quoted 5 lines
hw/arm/virt-acpi-build.c static void acpi_dsdt_add_power_button(Aml *scope) · line 371
Quoted 3 lines
hw/arm/virt-acpi-build.c +0
-3-/* RSDP */ -static void -build_rsdp (GArray *rsdp_table,BIOSLinker *linker,unsigned xsdt_tbl_offset)Note: here we use xsdt table not rsdt for ARM. After your change which assigns xsdt table address to
rsdt_physical_address, it doesn't work.
Hi Shannon,
I do not really like how this series organized/split and doing refactoring backwards but that's another story and I'll do suggestions how to make it better on per patch review later.
But as far as I see this change should only add legacy checksum over the current rsdp, which is bug fix and makes table spec compliant.
Could you point out why it doesn't work and what exactly breaks?
Quoted 36 lines
IIRC, we discussed this before that ARM virt uses xsdt while pc/q35 uses rsdt. So this patch is not necessary I think.
Thanks,
ShannonQuoted 31 lines
hw/arm/virt-acpi-build.c +0
-27-{ -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) {
PATCH v4 6/23 hw: acpi: Generalize AML build routines
In reply to Samuel OrtizFrom: 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.
--- hw/acpi/aml-build.c | 498 ++++++++++++++++++++++++++++++++++ hw/arm/virt-acpi-build.c | 4 +- hw/i386/acpi-build.c | 518 +----------------------------------- include/hw/acpi/aml-build.h | 25 ++ 4 files changed, 528 insertions(+), 517 deletions(-)
hw/acpi/aml-build.c +498 -0
@@ -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/arm/virt-acpi-build.c +2 -2
@@ -546,7 +546,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) }static void -build_mcfg (GArray *table_data,BIOSLinker *linker,VirtMachineState *vms) +virt_build_mcfg (GArray *table_data,BIOSLinker *linker,VirtMachineState *vms) {AcpiTableMcfg *mcfg;const MemMapEntry *memmap = vms->memmap;
@@ -791,7 +791,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)build_gtdt (tables_blob, tables->linker, vms);acpi_add_table (table_offsets, tables_blob); -build_mcfg (tables_blob, tables->linker, vms); +virt_build_mcfg (tables_blob, tables->linker, vms);acpi_add_table (table_offsets, tables_blob);build_spcr (tables_blob, tables->linker, vms);
hw/i386/acpi-build.c +3 -515
578 lines not on this page · raw message
include/hw/acpi/aml-build.h +25 -0
@@ -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
@@ -223,6 +224,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);void build_rsdp_rsdt (GArray *table_data,BIOSLinker *linker,unsigned rsdt_tbl_offset);
Signature · 1 line
2.19.1
Re: [Qemu-arm] [PATCH v4 06/23] hw: acpi: Generalize AML build routines
Quoted 14 lines · Samuel Ortiz
hw/arm/virt-acpi-build.c +1
-1@@ -546,7 +546,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) }static void -build_mcfg (GArray *table_data,BIOSLinker *linker,VirtMachineState *vms) +virt_build_mcfg (GArray *table_data,BIOSLinker *linker,VirtMachineState *vms) {AcpiTableMcfg *mcfg;const MemMapEntry *memmap = vms->memmap;
hw/arm/virt-acpi-build.c void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) · line 794
Quoted 5 lines
hw/arm/virt-acpi-build.c +1
-1@@ -791,7 +791,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)build_gtdt (tables_blob, tables->linker, vms);acpi_add_table (table_offsets, tables_blob); -build_mcfg (tables_blob, tables->linker, vms); +virt_build_mcfg (tables_blob, tables->linker, vms);
Looks like it doesn't share build_mcfg with x86. Why you still export the x86 build_mcfg and introduce this unnecessary change?
Thanks,
Shannon
PATCH v4 7/23 hw: acpi: Factorize _OSC AML across architectures
In reply to Samuel OrtizFrom: 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.
--- 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
@@ -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
@@ -1335,7 +1335,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);
@@ -1400,7 +1402,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 (ACPI_OSC_CTRL_PCI_ALL )); }if (numa_node !=NUMA_NODE_UNASSIGNED ) {
include/hw/acpi/acpi-defs.h +14 -0
@@ -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
@@ -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.19.1
PATCH v4 8/23 hw: i386: Move PCI host definitions to pci_host.h
In reply to Samuel OrtizThe PCI hole properties are not pc or i386 specific. They belong to the PCI host header instead.
--- include/hw/i386/pc.h | 5 ----- include/hw/pci/pci_host.h | 6 ++++++ 2 files changed, 6 insertions(+), 5 deletions(-)
include/hw/i386/pc.h +0 -5
@@ -182,11 +182,6 @@ void pc_acpi_init(const char *default_dsdt);void pc_guest_info_init (PCMachineState *pcms); -#define PCI_HOST_PROP_PCI_HOLE_START "pci-hole-start" -#define PCI_HOST_PROP_PCI_HOLE_END "pci-hole-end" -#define PCI_HOST_PROP_PCI_HOLE64_START "pci-hole64-start" -#define PCI_HOST_PROP_PCI_HOLE64_END "pci-hole64-end" -#define PCI_HOST_PROP_PCI_HOLE64_SIZE "pci-hole64-size"#define PCI_HOST_BELOW_4G_MEM_SIZE "below-4g-mem-size"#define PCI_HOST_ABOVE_4G_MEM_SIZE "above-4g-mem-size"
include/hw/pci/pci_host.h +6 -0
@@ -38,6 +38,12 @@#define PCI_HOST_BRIDGE_GET_CLASS (obj) \ OBJECT_GET_CLASS(PCIHostBridgeClass, (obj), TYPE_PCI_HOST_BRIDGE) +#define PCI_HOST_PROP_PCI_HOLE_START "pci-hole-start" +#define PCI_HOST_PROP_PCI_HOLE_END "pci-hole-end" +#define PCI_HOST_PROP_PCI_HOLE64_START "pci-hole64-start" +#define PCI_HOST_PROP_PCI_HOLE64_END "pci-hole64-end" +#define PCI_HOST_PROP_PCI_HOLE64_SIZE "pci-hole64-size" +struct PCIHostState {SysBusDevice busdev;
Signature · 1 line
2.19.1
PATCH v4 9/23 hw: acpi: Export the PCI host and holes getters
In reply to Samuel OrtizThis is going to be needed by the hardware reduced implementation, so let's export it.
Once the ACPI builder methods and getters will be implemented, the acpi_get_pci_host() implementation will become hardware agnostic.
--- hw/acpi/aml-build.c | 43 +++++++++++++++++++++++++++++++++ hw/i386/acpi-build.c | 47 ++----------------------------------- include/hw/acpi/aml-build.h | 2 ++ 3 files changed, 47 insertions(+), 45 deletions(-)
hw/acpi/aml-build.c +43 -0
@@ -1601,6 +1601,49 @@ 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; + + host =OBJECT_CHECK (PCIHostState, +object_resolve_path ("/machine/i440fx" ,NULL ), +TYPE_PCI_HOST_BRIDGE ); +if (!host) { + host =OBJECT_CHECK (PCIHostState, +object_resolve_path ("/machine/q35" ,NULL ), +TYPE_PCI_HOST_BRIDGE ); + } + +return OBJECT (host); +} + + +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;
hw/i386/acpi-build.c +2 -45
65 lines not on this page · raw message
include/hw/acpi/aml-build.h +2 -0
@@ -400,6 +400,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);/* Align AML blob size to a multiple of 'align' */ void acpi_align_size (GArray *blob,unsigned align);void acpi_add_table (GArray *table_offsets,GArray *table_data);
Signature · 1 line
2.19.1
PATCH v4 10/23 hw: acpi: Export and generalize the PCI host AML API
In reply to Samuel OrtizFrom: 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.
--- hw/acpi/aml-build.c | 157 ++++++++++++++++++++++++++++++++++++ hw/i386/acpi-build.c | 115 ++------------------------ include/hw/acpi/aml-build.h | 8 ++ 3 files changed, 173 insertions(+), 107 deletions(-)
hw/acpi/aml-build.c +157 -0
@@ -29,6 +29,19 @@#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 0x20000static GArray *build_alloc_array (void ) {
@@ -2142,6 +2155,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 -107
149 lines not on this page · raw message
include/hw/acpi/aml-build.h +8 -0
@@ -229,6 +229,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;
@@ -411,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.19.1
PATCH v4 11/23 hw: acpi: Export the MCFG getter
In reply to Samuel OrtizFrom: 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.
--- 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
@@ -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
@@ -1657,6 +1659,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
@@ -1856,28 +1856,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 *acpi_conf)
include/hw/acpi/aml-build.h +1 -0
@@ -408,6 +408,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);/* Align AML blob size to a multiple of 'align' */ void acpi_align_size (GArray *blob,unsigned align);void acpi_add_table (GArray *table_offsets,GArray *table_data);
Signature · 1 line
2.19.1
PATCH v4 12/23 hw: acpi: Do not create hotplug method when handler is not defined
In reply to Samuel OrtizCPU 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.
--- hw/acpi/cpu.c | 8 +++++--- hw/acpi/memory_hotplug.c | 11 +++++++---- 2 files changed, 12 insertions(+), 7 deletions(-)
hw/acpi/cpu.c +5 -3
@@ -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
@@ -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.19.1
PATCH v4 13/23 hw: i386: Make the hotpluggable memory size property more generic
In reply to Samuel OrtizThis 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.
--- 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
@@ -1628,7 +1628,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
@@ -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"
@@ -2443,7 +2444,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
@@ -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
@@ -97,6 +97,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 ); void memory_device_pre_plug(MemoryDeviceState *md, MachineState *ms,
Signature · 1 line
2.19.1
PATCH v4 14/23 hw: i386: Export the i386 ACPI SRAT build method
In reply to Samuel OrtizThis is the standard way of building SRAT on x86 platfoms. But future machine types could decide to define their own custom SRAT build method through the ACPI builder methods.
Moreover, we will also need to reach build_srat() from outside of acpi-build in order to use it as the ACPI builder SRAT build method.
--- hw/i386/acpi-build.c | 2 +- hw/i386/acpi-build.h | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-)
hw/i386/acpi-build.c +1 -1
@@ -1615,7 +1615,7 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)#define HOLE_640K_START (640 * KiB)#define HOLE_640K_END (1 * MiB) -static void +void build_srat (GArray *table_data,BIOSLinker *linker,MachineState *machine,AcpiConfiguration *acpi_conf) {
hw/i386/acpi-build.h +5 -0
@@ -4,6 +4,11 @@#include "hw/acpi/acpi.h" +/* ACPI SRAT (Static Resource Affinity Table) build method for x86 */ +void +build_srat (GArray *table_data,BIOSLinker *linker, +MachineState *machine,AcpiConfiguration *acpi_conf); +void acpi_setup (MachineState *machine,AcpiConfiguration *acpi_conf);#endif
Signature · 1 line
2.19.1
PATCH v4 15/23 hw: acpi: Fix memory hotplug AML generation error
In reply to Samuel OrtizFrom: 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 ^
--- hw/acpi/memory_hotplug.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
hw/acpi/memory_hotplug.c +5 -5
@@ -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.19.1
PATCH v4 16/23 hw: acpi: Export the PCI hotplug API
In reply to Samuel OrtizFrom: 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.
--- 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
@@ -34,6 +34,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
@@ -2305,6 +2306,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
@@ -418,6 +418,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.19.1
Re: [Qemu-devel] [PATCH v4 16/23] hw: acpi: Export the PCI hotplug API
Quoted 30 lines · 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.
Reviewed-by Philippe Mathieu-Daudé <[email protected]>Signed-off-by Sebastien Boeuf <[email protected]>Signed-off-by Jing Liu <[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 +1
-0@@ -34,6 +34,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
hw/acpi/aml-build.c Aml *build_pci_host_bridge(Aml *table, AcpiPciBus *pci_host) · line 2309
Quoted 4 lines
hw/acpi/aml-build.c +1
-0@@ -2305,6 +2306,199 @@ Aml *build_pci_host_bridge(Aml *table, AcpiPciBus *pci_host)return scope; } +void build_acpi_pci_hotplug (Aml *scope)
If you ever respin, Sebastien agreed to rename this build_acpi_pcihp(), which is consistent with build_append_pcihp_notify_entry() and the header name.
Quoted 422 lines
hw/acpi/aml-build.c +192
-0+{ +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@@ -360,163 +360,6 @@ build_madt(GArray *table_data, BIOSLinker *linker, table_data->len - madt_start, 1, NULL, NULL); } -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); -} - -static 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); -} -static void build_hpet_aml (Aml *table) {Aml *crs;@@ -1212,41 +1055,10 @@ static void build_piix4_isa_bridge(Aml *table)static void build_piix4_pci_hotplug (Aml *table) {Aml *scope; -Aml *field; -Aml *method; - - scope =aml_scope ("_SB.PCI0" ); - -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); + scope =aml_scope ("_SB.PCI0" ); +build_acpi_pci_hotplug (scope);aml_append (table, scope); }include/hw/acpi/aml-build.h +3
-0@@ -418,6 +418,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);
PATCH v4 17/23 hw: i386: Export the MADT build method
In reply to Samuel OrtizIt is going to be used by the PC machine type as the MADT table builder method and thus needs to be exported outside of acpi-build.c
Also, now that the generic build_madt() API is exported, we have to rename the ARM static one in order to avoid build time conflicts.
--- hw/arm/virt-acpi-build.c | 4 ++-- hw/i386/acpi-build.c | 3 ++- include/hw/i386/acpi.h | 27 +++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 include/hw/i386/acpi.h
hw/arm/virt-acpi-build.c +2 -2
@@ -565,7 +565,7 @@ build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)/* MADT */ static void -build_madt (GArray *table_data,BIOSLinker *linker,VirtMachineState *vms) +virt_build_madt (GArray *table_data,BIOSLinker *linker,VirtMachineState *vms) {VirtMachineClass *vmc =VIRT_MACHINE_GET_CLASS (vms);int madt_start = table_data->len;
@@ -746,7 +746,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)build_fadt_rev5 (tables_blob, tables->linker, vms, dsdt);acpi_add_table (table_offsets, tables_blob); -build_madt (tables_blob, tables->linker, vms); +virt_build_madt (tables_blob, tables->linker, vms);acpi_add_table (table_offsets, tables_blob);build_gtdt (tables_blob, tables->linker, vms);
hw/i386/acpi-build.c +2 -1
@@ -60,6 +60,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"
@@ -279,7 +280,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, } } -static void +void build_madt (GArray *table_data,BIOSLinker *linker,MachineState *ms,AcpiConfiguration *acpi_conf) {
include/hw/i386/acpi.h new file +27 -0
@@ -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 HW_I386_ACPI_H +#define HW_I386_ACPI_H + +#include "hw/acpi/acpi.h" + +/* ACPI MADT (Multiple APIC Description Table) build method */ +void build_madt (GArray *table_data,BIOSLinker *linker, +MachineState *ms,AcpiConfiguration *conf); + +#endif
Signature · 1 line
2.19.1
Re: [Qemu-devel] [PATCH v4 17/23] hw: i386: Export the MADT build method
Hi Samuel,
Quoted 76 lines · Samuel Ortiz
It is going to be used by the PC machine type as the MADT table builder method and thus needs to be exported outside of
acpi-build.cAlso, now that the generic
build_madt()API is exported, we have to rename the ARM static one in order to avoid build time conflicts.Reviewed-by Philippe Mathieu-Daudé <[email protected]>Signed-off-by Samuel Ortiz <[email protected]>--- hw/arm/virt-acpi-build.c | 4 ++-- hw/i386/acpi-build.c | 3 ++- include/hw/i386/acpi.h | 27 +++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-)create mode 100644
include/hw/i386/acpi.hhw/arm/virt-acpi-build.c +2
-2@@ -565,7 +565,7 @@ build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)/* MADT */ static void -build_madt (GArray *table_data,BIOSLinker *linker,VirtMachineState *vms) +virt_build_madt (GArray *table_data,BIOSLinker *linker,VirtMachineState *vms) {VirtMachineClass *vmc =VIRT_MACHINE_GET_CLASS (vms);int madt_start = table_data->len;@@ -746,7 +746,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)build_fadt_rev5 (tables_blob, tables->linker, vms, dsdt);acpi_add_table (table_offsets, tables_blob); -build_madt (tables_blob, tables->linker, vms); +virt_build_madt (tables_blob, tables->linker, vms);acpi_add_table (table_offsets, tables_blob);build_gtdt (tables_blob, tables->linker, vms);hw/i386/acpi-build.c +2
-1@@ -60,6 +60,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" @@ -279,7 +280,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, } } -static void +void build_madt (GArray *table_data,BIOSLinker *linker,MachineState *ms,AcpiConfiguration *acpi_conf) {include/hw/i386/acpi.h new file +11
-0@@ -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
include/hw/i386/acpi.h line 21
Quoted 10 lines
include/hw/i386/acpi.h new file +10
-0+* 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_I386_ACPI_H +#define HW_I386_ACPI_H + +#include "hw/acpi/acpi.h"
You forgot the following header:
#include "hw/acpi/bios-linker-loader.h"
The BIOSLinker is not provided by "acpi/acpi.h", which results in a failure when trying to precompile this header.
Quoted 7 lines
include/hw/i386/acpi.h new file +6
-0+ +/* ACPI MADT (Multiple APIC Description Table) build method */ +void build_madt (GArray *table_data,BIOSLinker *linker, +MachineState *ms,AcpiConfiguration *conf); + +#endif
Re: [Qemu-devel] [PATCH v4 17/23] hw: i386: Export the MADT build method
Quoted 94 lines · Philippe Mathieu-Daudé
Hi Samuel,
Quoted 83 lines · Samuel Ortiz
It is going to be used by the PC machine type as the MADT table builder method and thus needs to be exported outside of
acpi-build.cAlso, now that the generic
build_madt()API is exported, we have to rename the ARM static one in order to avoid build time conflicts.Reviewed-by Philippe Mathieu-Daudé <[email protected]>Signed-off-by Samuel Ortiz <[email protected]>---
hw/arm/virt-acpi-build.c| 4 ++-- hw/i386/acpi-build.c | 3 ++- include/hw/i386/acpi.h | 27 +++++++++++++++++++++++++++3 files changed, 31 insertions(+), 3 deletions(-)create mode 100644
include/hw/i386/acpi.hhw/arm/virt-acpi-build.c +0
-0@@ -565,7 +565,7 @@ build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) /* MADT */static void
-build_madt(GArray*table_data, BIOSLinker *linker, VirtMachineState *vms)
+virt_build_madt(GArray*table_data, BIOSLinker *linker, VirtMachineState *vms)
{
VirtMachineClass *vmc =VIRT_MACHINE_GET_CLASS(vms); intmadt_start=table_data->len;hw/arm/virt-acpi-build.c +1
-1@@ -746,7 +746,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) build_fadt_rev5(tables_blob, tables->linker, vms, dsdt); acpi_add_table(table_offsets, tables_blob); - build_madt(tables_blob, tables->linker, vms); + virt_build_madt(tables_blob, tables->linker, vms); acpi_add_table(table_offsets, tables_blob); build_gtdt(tables_blob, tables->linker, vms);hw/i386/acpi-build.c +2
-1@@ -60,6 +60,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"@@ -279,7 +280,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, } } -static void +void build_madt(GArray *table_data, BIOSLinker *linker, MachineState *ms, AcpiConfiguration *acpi_conf) {include/hw/i386/acpi.h new file +5
-0@@ -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/>.
+ */include/hw/i386/acpi.h line 21
Quoted 5 lines
include/hw/i386/acpi.h new file +5
-0+ +#ifndef HW_I386_ACPI_H +#define HW_I386_ACPI_H + +#include "hw/acpi/acpi.h" You forgot the following header:
Quoted 1 line
#include "
hw/acpi/bios-linker-loader.h"
You can also remove it from hw/i386/acpi-build.c.
Quoted 10 lines
The BIOSLinker is not provided by "
acpi/acpi.h", which results in a failure when trying to precompile this header.Quoted 7 lines
include/hw/i386/acpi.h new file +6
-0+ +/* ACPI MADT (Multiple APIC Description Table) build method */ +void build_madt (GArray *table_data,BIOSLinker *linker, + MachineState *ms, AcpiConfiguration *conf); + +#endif
PATCH v4 18/23 hw: acpi: Retrieve the PCI bus from AcpiPciHpState
In reply to Samuel OrtizFrom: 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.
--- 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
@@ -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
@@ -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
@@ -255,7 +255,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
@@ -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
@@ -1,6 +0,0 @@ -#include "qemu/osdep.h" -#include "hw/i386/pc.h" -PCIBus *find_i440fx (void ) -{ -return NULL ; -}
Signature · 1 line
2.19.1
PATCH v4 19/23 hw: acpi: Define ACPI tables builder interface
In reply to Samuel OrtizIn order to decouple ACPI APIs from specific machine types, we are creating an ACPI builder interface that each ACPI platform can choose to implement.
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.
--- hw/acpi/Makefile.objs | 1 + hw/acpi/builder.c | 97 ++++++++++++++++++++++++++++++++++++ include/hw/acpi/builder.h | 100 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 198 insertions(+) create mode 100644 hw/acpi/builder.c create mode 100644 include/hw/acpi/builder.h
hw/acpi/Makefile.objs +1 -0
@@ -11,6 +11,7 @@ common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o common-obj-y += acpi_interface.o common-obj-y += bios-linker-loader.o common-obj-y += aml-build.o +common-obj-y += builder.o common-obj-$(CONFIG_IPMI) += ipmi.o common-obj-$(call lnot,$(CONFIG_IPMI)) += ipmi-stub.o
hw/acpi/builder.c new file +97 -0
97 lines not on this page · raw message
include/hw/acpi/builder.h new file +100 -0
100 lines not on this page · raw message
Signature · 1 line
2.19.1
PATCH v4 20/23 hw: i386: Implement the ACPI builder interface for PC
In reply to Samuel OrtizAll 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.
--- hw/i386/acpi-build.c | 14 +++++++++----- hw/i386/pc.c | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-)
hw/i386/acpi-build.c +9 -5
@@ -34,6 +34,7 @@#include "hw/acpi/acpi-defs.h" #include "hw/acpi/acpi.h" #include "hw/acpi/cpu.h" +#include "hw/acpi/builder.h" #include "hw/nvram/fw_cfg.h" #include "hw/acpi/bios-linker-loader.h" #include "hw/loader.h"
@@ -1684,6 +1685,7 @@ void acpi_build(AcpiBuildTables *tables,GArray *tables_blob = tables->table_data;AcpiSlicOem slic_oem = { .id =NULL , .table_id =NULL };Object *vmgenid_dev; +AcpiBuilder *ab =ACPI_BUILDER (machine);acpi_get_pm_info (&pm);acpi_get_misc_info (&misc);
@@ -1733,7 +1735,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, acpi_conf); +acpi_builder_madt (ab, tables_blob, tables->linker, + machine, acpi_conf); vmgenid_dev =find_vmgenid_dev ();if (vmgenid_dev) {
@@ -1757,15 +1760,16 @@ void acpi_build(AcpiBuildTables *tables, }if (acpi_conf->numa_nodes) {acpi_add_table (table_offsets, tables_blob); -build_srat (tables_blob, tables->linker, machine, acpi_conf); +acpi_builder_srat (ab, tables_blob, tables->linker, + machine, acpi_conf);if (have_numa_distance) {acpi_add_table (table_offsets, tables_blob); -build_slit (tables_blob, tables->linker); +acpi_builder_slit (ab, tables_blob, tables->linker); } }if (acpi_get_mcfg (&mcfg)) {acpi_add_table (table_offsets, tables_blob); -build_mcfg (tables_blob, tables->linker, &mcfg); +acpi_builder_mcfg (ab, tables_blob, tables->linker, &mcfg); }if (x86_iommu_get_default ()) {IommuType IOMMUType =x86_iommu_get_type ();
@@ -1796,7 +1800,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); +acpi_builder_rsdp (ab, 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 +19 -0
@@ -64,6 +64,7 @@#include "qemu/option.h" #include "hw/acpi/acpi.h" #include "hw/acpi/cpu_hotplug.h" +#include "hw/acpi/builder.h" #include "hw/boards.h" #include "acpi-build.h" #include "hw/mem/pc-dimm.h"
@@ -75,6 +76,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
@@ -2404,12 +2406,20 @@ static void x86_nmi(NMIState *n, int cpu_index, Error **errp) } } +static AcpiConfiguration *pc_acpi_configuration (AcpiBuilder *builder) +{ +PCMachineState *pcms =PC_MACHINE (builder); + +return &pcms->acpi_configuration; +} +static void pc_machine_class_init (ObjectClass *oc,void *data) {MachineClass *mc =MACHINE_CLASS (oc);PCMachineClass *pcmc =PC_MACHINE_CLASS (oc);HotplugHandlerClass *hc =HOTPLUG_HANDLER_CLASS (oc);NMIClass *nc =NMI_CLASS (oc); +AcpiBuilderMethods *abm =ACPI_BUILDER_METHODS (oc); pcmc->pci_enabled = true; pcmc->has_acpi_build = true;
@@ -2444,6 +2454,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 ; +/* ACPI building methods */ + abm->madt = build_madt; + abm->rsdp = build_rsdp_rsdt; + abm->mcfg = build_mcfg; + abm->srat = build_srat; + abm->slit = build_slit; + abm->configuration = pc_acpi_configuration; +object_class_property_add (oc,MEMORY_DEVICE_REGION_SIZE ,"int" , pc_machine_get_device_memory_region_size,NULL ,NULL ,NULL , &error_abort);
@@ -2495,6 +2513,7 @@ static const TypeInfo pc_machine_info = { .interfaces = (InterfaceInfo[]) { {TYPE_HOTPLUG_HANDLER }, {TYPE_NMI }, + {TYPE_ACPI_BUILDER }, { } }, };
Signature · 1 line
2.19.1
PATCH v4 21/23 hw: pci-host: piix: Return PCI host pointer instead of PCI bus
In reply to Samuel OrtizFor building the MCFG table, we need to track a given machine type PCI host pointer, and we can't get it from the bus pointer alone. As piix returns a PCI bus pointer, we simply modify its builder to return a PCI host pointer instead.
--- hw/i386/pc_piix.c | 18 +++++++++++------- hw/pci-host/piix.c | 24 ++++++++++++------------ include/hw/i386/pc.h | 21 +++++++++++---------- 3 files changed, 34 insertions(+), 29 deletions(-)
hw/i386/pc_piix.c +11 -7
@@ -32,6 +32,7 @@#include "hw/display/ramfb.h" #include "hw/smbios/smbios.h" #include "hw/pci/pci.h" +#include "hw/pci/pci_host.h" #include "hw/pci/pci_ids.h" #include "hw/usb.h" #include "net/net.h"
@@ -75,6 +76,7 @@ static void pc_init1(MachineState *machine,MemoryRegion *system_memory =get_system_memory ();MemoryRegion *system_io =get_system_io ();int i; +struct PCIHostState *pci_host;PCIBus *pci_bus;ISABus *isa_bus;PCII440FXState *i440fx_state;
@@ -196,15 +198,17 @@ static void pc_init1(MachineState *machine, }if (pcmc->pci_enabled) { - pci_bus =i440fx_init (host_type, - pci_type, - &i440fx_state, &piix3_devfn, &isa_bus, pcms->gsi, - system_memory, system_io, machine->ram_size, - acpi_conf->below_4g_mem_size, - acpi_conf->above_4g_mem_size, - pci_memory, ram_memory); + pci_host =i440fx_init (host_type, + pci_type, + &i440fx_state, &piix3_devfn, &isa_bus, pcms->gsi, + system_memory, system_io, machine->ram_size, + acpi_conf->below_4g_mem_size, + acpi_conf->above_4g_mem_size, + pci_memory, ram_memory); + pci_bus = pci_host->bus; pcms->bus = pci_bus; }else { + pci_host =NULL ; pci_bus =NULL ; i440fx_state =NULL ; isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
hw/pci-host/piix.c +12 -12
@@ -342,17 +342,17 @@ static void i440fx_realize(PCIDevice *dev, Error **errp) } } -PCIBus *i440fx_init (const char *host_type,const char *pci_type, -PCII440FXState **pi440fx_state, -int *piix3_devfn, -ISABus **isa_bus,qemu_irq *pic, -MemoryRegion *address_space_mem, -MemoryRegion *address_space_io, -ram_addr_t ram_size, -ram_addr_t below_4g_mem_size, -ram_addr_t above_4g_mem_size, -MemoryRegion *pci_address_space, -MemoryRegion *ram_memory) +struct PCIHostState *i440fx_init (const char *host_type,const char *pci_type, +PCII440FXState **pi440fx_state, +int *piix3_devfn, +ISABus **isa_bus,qemu_irq *pic, +MemoryRegion *address_space_mem, +MemoryRegion *address_space_io, +ram_addr_t ram_size, +ram_addr_t below_4g_mem_size, +ram_addr_t above_4g_mem_size, +MemoryRegion *pci_address_space, +MemoryRegion *ram_memory) {DeviceState *dev;PCIBus *b;
@@ -442,7 +442,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,i440fx_update_memory_mappings (f); -return b; +return s; }/* PIIX3 PCI to ISA bridge */
include/hw/i386/pc.h +11 -10
@@ -244,16 +244,17 @@ typedef struct PCII440FXState PCII440FXState;*/ #define RCR_IOPORT 0xcf9 -PCIBus *i440fx_init (const char *host_type,const char *pci_type, -PCII440FXState **pi440fx_state,int *piix_devfn, -ISABus **isa_bus,qemu_irq *pic, -MemoryRegion *address_space_mem, -MemoryRegion *address_space_io, -ram_addr_t ram_size, -ram_addr_t below_4g_mem_size, -ram_addr_t above_4g_mem_size, -MemoryRegion *pci_memory, -MemoryRegion *ram_memory); +struct PCIHostState *i440fx_init (const char *host_type,const char *pci_type, +PCII440FXState **pi440fx_state, +int *piix_devfn, +ISABus **isa_bus,qemu_irq *pic, +MemoryRegion *address_space_mem, +MemoryRegion *address_space_io, +ram_addr_t ram_size, +ram_addr_t below_4g_mem_size, +ram_addr_t above_4g_mem_size, +MemoryRegion *pci_memory, +MemoryRegion *ram_memory);/* piix4.c */ extern PCIDevice *piix4_dev;
Signature · 1 line
2.19.1
PATCH v4 22/23 hw: i386: Set ACPI configuration PCI host pointer
In reply to Samuel OrtizFor both PC and Q35 machine types, we can set it at the PCI host bridge creation time.
--- hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c | 1 + 2 files changed, 2 insertions(+)
hw/i386/pc_piix.c +1 -0
@@ -216,6 +216,7 @@ static void pc_init1(MachineState *machine, no_hpet =1 ; }isa_bus_irqs (isa_bus, pcms->gsi); + acpi_conf->pci_host = pci_host;if (kvm_pic_in_kernel ()) { i8259 =kvm_i8259_init (isa_bus);
hw/i386/pc_q35.c +1 -0
@@ -188,6 +188,7 @@ static void pc_q35_init(MachineState *machine)qdev_init_nofail (DEVICE (q35_host)); phb =PCI_HOST_BRIDGE (q35_host); host_bus = phb->bus; + acpi_conf->pci_host = phb;/* create ISA bus */ lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV, ICH9_LPC_FUNC), true,
Signature · 1 line
2.19.1
PATCH v4 23/23 hw: i386: Refactor PCI host getter
In reply to Samuel OrtizFrom: Yang Zhong <[email protected]>
Now that the ACPI builder methods are added, we can reach the ACPI configuration pointer from the MachineState pointer. From there we can get to the PCI host pointer and return it.
This makes the PCI host getter an ACPI, architecture agnostic function.
--- hw/acpi/aml-build.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
hw/acpi/aml-build.c +7 -13
@@ -22,6 +22,8 @@#include "qemu/osdep.h" #include <glib/gprintf.h> #include "hw/acpi/aml-build.h" +#include "hw/acpi/builder.h" +#include "hw/mem/memory-device.h" #include "qemu/bswap.h" #include "qemu/bitops.h" #include "sysemu/numa.h"
@@ -1617,23 +1619,15 @@ 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; +MachineState *ms =MACHINE (qdev_get_machine ()); +AcpiBuilder *ab =ACPI_BUILDER (ms); +AcpiConfiguration *acpi_conf; - host =OBJECT_CHECK (PCIHostState, -object_resolve_path ("/machine/i440fx" ,NULL ), -TYPE_PCI_HOST_BRIDGE ); -if (!host) { - host =OBJECT_CHECK (PCIHostState, -object_resolve_path ("/machine/q35" ,NULL ), -TYPE_PCI_HOST_BRIDGE ); - } + acpi_conf =acpi_builder_configuration (ab); -return OBJECT (host); +return OBJECT (acpi_conf->pci_host); }
Signature · 1 line
2.19.1
On Thu, 1 Nov 2018 11:22:40 +0100
Samuel Ortiz <[email protected]> wrote:
Thanks for looking at ACPI mess we have in QEMU and trying to make it better, this series look a bit hackish probably because it was written to suit new virt board, so it needs some more clean up to be done.
Quoted 2 lines
This patch set provides an ACPI code reorganization in preparation for adding hardware-reduced support to QEMU.
QEMU already has hw reduced implementation, specifically in arm/virt board
Quoted 6 lines
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
s:support to QEMU:support for new i386/virt machine:
Quoted 7 lines
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.
it probably should be other way around, generalize and reuse as much of arm/virt acpi code, instead of adding new duplicated code without an actual user and then swapping/dropping old arm version in favor of the new one. It's hard to review when it done in this order and easy to miss issues that would be easier to spot if you reused arm versions (where applicable) as starting point for generalization.
Here are some generic suggestions/nits that apply to whole series:
* s/Factorize/Factor out/
* try to restructure series in following way
1. put bug fixes at the beginning of series.
'make V=1 check' should produce tables diffs to account for changes made in the tables.
After error fixing, add an extra patch to update reference ACPI tables to simplify testing for reviewing and so for self-check when you'll be doing refactoring to make sure there aren't any changes during generalization/refactoring later.
2. instead of adding 'new' implementations try to generalize existing ones so that a user for new code will always exist. It's also makes patches easier to review/test.
3. Since you are touching/moving around existing fixed tables that are using legacy 'struct' based approach to construct them, it's a good as opportunity to switch to a newer approach and use build_append_int_noprefix() API to construct tables. Use build_amd_iommu() as example. And it's doubly true if/when you are adding new fixed tables (i.e. only build_append_int_noprefix() based ones are acceptable).
4. for patches during #2 and #3 stages, 'make V=1 check' should pass without any warnings, that will speed up review process.
5. add i386/virt board and related hardware reduced ACPI code that's specific to it.
I'll try to review series during next week and will do per patch suggestions how to structure it or do other way.
Hopefully after doing refactoring we would end up with simpler/smaller and cleaner ACPI code to the benefit of everyone.
PS:
if you need a quick advice wrt APCI parts, feel free to ping me on IRC (Paris timezone).
Quoted 92 lines
The patches are also available in their own git branch [2].
[1] https://github.com/intel/nemu
[2] https://github.com/intel/nemu/tree/topic/upstream/acpiv1 -> v2:
* Drop the hardware-reduced implementation for now. Our next patch set will add hardware-reduced and convert arm/virt to it.
* Implement the ACPI build methods as a QOM Interface Class and convert the PC machine type to it.
*acpi_conf_pc_init()uses a PCMachineState pointer and not a MachineState one as its argument.v2 -> v3:
* Cc all relevant maintainers, no functional changes.v3 -> v4:
* Renamed all AcpiConfiguration pointers from conf toacpi_conf.
* Removed theACPI_BUILD_ALIGN_SIZEexport.
* Temporarily updated the arm virtbuild_rsdp()prototype for bisectability purposes.
* Removed unneeded pci headers fromacpi-build.c.
* Refactor the acpi PCI host getter so that it truly is architecture agnostic, by carrying the PCI host pointer through the AcpiConfiguration structure.
* Splitted the PCI host AML builder API export patch from the PCI host and holes getter one.
* Reduced thebuild_srat()export scope to hw/i386 instead of the broader hw/acpi. SRAT builders are truly architecture specific and can hardly be generalized.
* Completed the ACPI builder documentation.Samuel Ortiz (15):
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: acpi: Implement XSDT support for RSDP
hw: arm: Switch to the AML build RSDP building routine
hw: i386: Move PCI host definitions topci_host.h
hw: acpi: Export the PCI host and holes getters
hw: acpi: Do not create hotplug method when handler is not defined
hw: i386: Make the hotpluggable memory size property more generic
hw: i386: Export the i386 ACPI SRAT build method
hw: i386: Export the MADT build method
hw: acpi: Define ACPI tables builder interface
hw: i386: Implement the ACPI builder interface for PC
hw: pci-host: piix: Return PCI host pointer instead of PCI bus
hw: i386: Set ACPI configuration PCI host pointerSebastien Boeuf (2):
hw: acpi: Export the PCI hotplug API
hw: acpi: Retrieve the PCI bus from AcpiPciHpStateYang Zhong (6):
hw: acpi: Generalize AML build routines
hw: acpi: Factorize_OSCAML across architectures
hw: acpi: Export and generalize the PCI host AML API
hw: acpi: Export the MCFG getter
hw: acpi: Fix memory hotplug AML generation error
hw: i386: Refactor PCI host getterhw/acpi/Makefile.objs | 1 + hw/acpi/aml-build.c | 985 +++++++++++++++++++++++++++++ hw/acpi/builder.c | 97 +++ hw/acpi/cpu.c | 8 +- hw/acpi/cpu_hotplug.c | 9 +- hw/acpi/memory_hotplug.c | 21 +- hw/acpi/pcihp.c | 10 +- hw/arm/virt-acpi-build.c | 94 +-- hw/i386/acpi-build.c | 1071 +++----------------------------- hw/i386/acpi-build.h | 9 +- hw/i386/pc.c | 198 +++--- hw/i386/pc_piix.c | 36 +- hw/i386/pc_q35.c | 22 +- hw/i386/xen/xen-hvm.c | 19 +- hw/pci-host/piix.c | 32 +- include/hw/acpi/acpi-defs.h | 14 + include/hw/acpi/acpi.h | 44 ++ include/hw/acpi/aml-build.h | 47 ++ include/hw/acpi/builder.h | 100 +++ include/hw/i386/acpi.h | 27 + include/hw/i386/pc.h | 49 +- include/hw/mem/memory-device.h | 2 + include/hw/pci/pci_host.h | 6 + stubs/Makefile.objs | 1 - stubs/pci-host-piix.c | 6 - 25 files changed, 1648 insertions(+), 1260 deletions(-) create mode 100644 hw/acpi/builder.c create mode 100644 include/hw/acpi/builder.h create mode 100644 include/hw/i386/acpi.h delete mode 100644 stubs/pci-host-piix.c
Hi Igor,
Quoted 4 lines · Igor Mammedov
On Thu, 1 Nov 2018 11:22:40 +0100
Samuel Ortiz <[email protected]> wrote:Thanks for looking at ACPI mess we have in QEMU and trying to make it better,
Thanks for the initial review and feedback.
Quoted 6 lines
this series look a bit hackish probably because it was written to suit new virt board, so it needs some more clean up to be done.
Quoted 2 lines
This patch set provides an ACPI code reorganization in preparation for adding hardware-reduced support to QEMU.
QEMU already has hw reduced implementation, specifically in arm/virt board
Back in May, I tried booting a virt machine type with "acpi=force" with no success, and today's HEAD still fails. With "acpi=on":
[ 0.000000] ACPI: Early table checksum verification disabled [ 0.000000] ACPI: Failed to init ACPI tables
So this code has been broken for several months and I suspect it's not really run by anyone.
Moreover, even though the virt-acpi-build.c code aims at building a hardware-reduced ACPI compliant set of tables, the implementation is largely specific to the arm/virt board. We did take the generic parts from it in order to build a shareable API across architectures.
So by "adding hardware-reduced support to QEMU", we meant providing a architecture and machine type agnostic hardware-reduced ACPI implementation that all QEMU machine types could use.
I'll make sure to change the cover letter wording and rephrase it to reflect our intention.
Quoted 7 lines
Quoted 6 lines
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
s:support to QEMU:support for new i386/virt machine:
Most of this serie moves non x86 specific stuff out of i386 into the (theoretically) architecture agnostic hw/acpi folder in order to a) reuse and share as much as possible of the non x86 specific ACPI code that currently lives under hw/i386 and b) improve and extend the current hw/acpi APIs.
So on one hand I agree this cover letter needs massaging, but on the other hand I feel it's unfair to say there's anything specific to i386/virt on this serie. At least we tried really hard to avoid falling into that trap.
Quoted 9 lines
Quoted 7 lines
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.it probably should be other way around, generalize and reuse as much of arm/virt acpi code,
Our hardware-reduced implementation does that indeed, and the next patchset following this one will add the actual hardware-reduced ACPI API implementation together with the arm/virt switch to it (which will mostly be code removal from virt-acpi-build.c).
Quoted 3 lines
instead of adding new duplicated code without
an actual user and then swapping/dropping old arm version in favor of the new one.
This patch set is not adding any code duplication, I hope. It's really preparing the current ACPI code in order to precisely NOT add code duplication when building a machine type agnostic hardware reduced ACPI API.
I initally added our hardware-reduced ACPI API to that patch serie but decided to remove it. Here is what it looks like:
https://github.com/intel/nemu/blob/topic/virt-x86/hw/acpi/reduced.c
It's consuming the factorization work that this serie provides, and the arm/virt machine type will use a fairly large part of that API.
Quoted 8 lines
It's hard to review when it done in this order and easy to miss issues that would be easier to spot if you reused arm versions (where applicable) as starting point for generalization.
Here are some generic suggestions/nits that apply to whole series:
* s/Factorize/Factor out/
* try to restructure series in following way
1. put bug fixes at the beginning of series.
I'll try to do that, yes.
Quoted 2 lines
'make V=1 check' should produce tables diffs to account for changes made in the tables.
I'll run the make check tests, thanks for the reminder.
Quoted 9 lines
After error fixing, add an extra patch to update reference ACPI tables to simplify testing for reviewing and so for self-check when you'll be doing refactoring to make sure there aren't any changes during generalization/refactoring later.
2. instead of adding 'new' implementations try to generalize existing ones so that a user for new code will always exist. It's also makes patches easier to review/test.
The PC machine type is consuming all the exported API and e.g. the new ACPI builder interface as well.
Quoted 7 lines
3. Since you are touching/moving around existing fixed tables that are using legacy 'struct' based approach to construct them, it's a good as opportunity to switch to a newer approach and use
build_append_int_noprefix()API to construct tables. Usebuild_amd_iommu()as example. And it's doubly true if/when you are adding new fixed tables (i.e. onlybuild_append_int_noprefix()based ones are acceptable).
I'd be happy to do that, but I'd appreciate if that could be done as a follow up patch set, in order to decouple the code movement/export from the actual reimplementation.
Quoted 6 lines
4. for patches during #2 and #3 stages, 'make V=1 check' should pass without any warnings, that will speed up review process.
5. add i386/virt board and related hardware reduced ACPI code that's specific to it.
That is definitely going to be our last steps, but I think this would make a very large patch set to review at once.
Quoted 2 lines
I'll try to review series during next week and will do per patch suggestions how to structure it or do other way.
Thanks in advance. FWIW I just sent v5, addressing Philippe's comments. I'll wait for your feedback before sending v6.
Quoted 6 lines
Hopefully after doing refactoring we would end up with simpler/smaller and cleaner ACPI code to the benefit of everyone.
PS:
if you need a quick advice wrt APCI parts, feel free to ping me on IRC (Paris timezone).
Nice, same timezone for me :)
I'll get in touch next week.
Cheers,
Samuel.
Igor,
Quoted 6 lines · Samuel Ortiz
Quoted 1 line
QEMU already has hw reduced implementation, specifically in arm/virt board
Back in May, I tried booting a virt machine type with "acpi=force" with no success, and today's HEAD still fails. With "acpi=on":
[ 0.000000] ACPI: Early table checksum verification disabled [ 0.000000] ACPI: Failed to init ACPI tables
My bad, I was using the wrong firmware/pflash options. With the right ones, I can boot with an edk2 firmware and an EFI variable space. Thanks Philippe and Eric for the help.
Cheers,
Samuel.
Quoted 15 lines · Samuel Ortiz
Hi Igor,
Quoted 4 lines · Igor Mammedov
On Thu, 1 Nov 2018 11:22:40 +0100
Samuel Ortiz <[email protected]> wrote:Thanks for looking at ACPI mess we have in QEMU and trying to make it better,
Thanks for the initial review and feedback.
Quoted 6 lines
this series look a bit hackish probably because it was written to suit new virt board, so it needs some more clean up to be done.
Quoted 2 lines
This patch set provides an ACPI code reorganization in preparation for adding hardware-reduced support to QEMU.
QEMU already has hw reduced implementation, specifically in arm/virt board
Quoted 8 lines
Back in May, I tried booting a virt machine type with "acpi=force" with no success, and today's HEAD still fails. With "acpi=on":
[ 0.000000] ACPI: Early table checksum verification disabled [ 0.000000] ACPI: Failed to init ACPI tablesSo this code has been broken for several months and I suspect it's not really run by anyone.
Hi Samuel,
We use it all the time. To see if there had been an upstream regression I just tried latest qemu (v3.0.0-1763-gb2f7a038bb4c) with a latest kernel (v4.20-rc1) in my guest. It worked just fine, and there was no need for any kernel command line parameters.
What guest kernel version did you try?
Thanks,
drew
Hi Andrew,
Quoted 10 lines · Andrew Jones
Quoted 9 lines
Quoted 1 line
QEMU already has hw reduced implementation, specifically in arm/virt board
Back in May, I tried booting a virt machine type with "acpi=force" with no success, and today's HEAD still fails. With "acpi=on":
[ 0.000000] ACPI: Early table checksum verification disabled [ 0.000000] ACPI: Failed to init ACPI tablesSo this code has been broken for several months and I suspect it's not really run by anyone.
Quoted 8 lines
Hi Samuel,
We use it all the time. To see if there had been an upstream regression I just tried latest qemu (v3.0.0-1763-gb2f7a038bb4c) with a latest kernel (v4.20-rc1) in my guest. It worked just fine, and there was no need for any kernel command line parameters.
What guest kernel version did you try?
This is my bad, apologies for jumping the gun. I was not using the right pflash options and thus the ACPI tables were simply not passed to the kernel. Philippe and Eric helped me fix that, and now I can boot arm/virt on top of ACPI. Sorry for the bad noise.
Cheers,
Samuel.
Quoted 5 lines · On Mon, 5 Nov 2018 03:10:28 +0100 Samuel Ortiz <[email protected]> wrote:
Hi Igor,
Quoted 2 lines · Igor Mammedov
On Thu, 1 Nov 2018 11:22:40 +0100
Samuel Ortiz <[email protected]> wrote:
[...]
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
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 6 lines
Quoted 2 lines
it probably should be other way around, generalize and reuse as much of arm/virt acpi code,
Our hardware-reduced implementation does that indeed, and the next patchset following this one will add the actual hardware-reduced ACPI API implementation together with the arm/virt switch to it (which will mostly be code removal from
virt-acpi-build.c).
That's where I disagree with patch ordering, i.e. you are adding impl. without user and than do sudden switch from old API (dropping old one in the process) to the new one some time in the future.
For example 04/23 add new impl. of build_rsdp() with xsdt support without actual user, which is duplicate of arm/virt code modulo checksum fix and than later in 05/23 drops old arm impl. in favor of the new one.
As you saw it causes confusion during review even within one series and it's much worse in cases where similar changes happen across several series. In addition bisection would point into wrong patch 05/23 (where code started to be used but not where it was introduced).
Quoted 11 lines
Quoted 3 lines
instead of adding new duplicated code without
an actual user and then swapping/dropping old arm version in favor of the new one.This patch set is not adding any code duplication, I hope. It's really preparing the current ACPI code in order to precisely NOT add code duplication when building a machine type agnostic hardware reduced ACPI API.
I initally added our hardware-reduced ACPI API to that patch serie but decided to remove it. Here is what it looks like:
https://github.com/intel/nemu/blob/topic/virt-x86/hw/acpi/reduced.c
Quoted 2 lines
It's consuming the factorization work that this serie provides, and the arm/virt machine type will use a fairly large part of that API.
the point is existing arm/virt machine or any other machine should use new API right away if applicable instead of some point in the future, otherwise we are doing something wrong, change might not belong to series.
It's hard to find reasoning for merging new API/verify correctness without immediate user.
Quoted 3 lines
Quoted 3 lines
It's hard to review when it done in this order and easy to miss issues that would be easier to spot if you reused arm versions (where applicable) as starting point for generalization.
[...]
Quoted 10 lines
Quoted 7 lines
3. Since you are touching/moving around existing fixed tables that are using legacy 'struct' based approach to construct them, it's a good as opportunity to switch to a newer approach and use
build_append_int_noprefix()API to construct tables. Usebuild_amd_iommu()as example. And it's doubly true if/when you are adding new fixed tables (i.e. onlybuild_append_int_noprefix()based ones are acceptable).I'd be happy to do that, but I'd appreciate if that could be done as a follow up patch set, in order to decouple the code movement/export from the actual reimplementation.
Well, it fine to keep old style if one fixes code but in all other cases it's good opportunity to replace legacy approach with the preferred one. So unless it's a fix, the new code (including generalization) should use preferred approach (there are exceptions but this series is not the case). Considering this series is re-factoring to make ACPI code more reusable, we should do it properly instead of cutting corners.
I apply this rule myself 'ex: build_fadt()' and others if re-factoring is related the series topic.
Quoted 8 lines
Quoted 6 lines
4. for patches during #2 and #3 stages, 'make V=1 check' should pass without any warnings, that will speed up review process.
5. add i386/virt board and related hardware reduced ACPI code that's specific to it.
That is definitely going to be our last steps, but I think this would make a very large patch set to review at once.
Agreed, as far as new API is used by already existing code. If API isn't used then it should be moved out of this series to the one that would actually use it.
[...]