All PC machine type derivatives will use the same ACPI table build
methods. But with that change in place, any new x86 machine type will be
able to re-use the acpi-build API and customize part of it by defining
its own ACPI table build methods.
Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
---
hw/i386/acpi-build.c | 15 ++++++++++-----
hw/i386/pc.c | 19 +++++++++++++++++++
2 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5035dac556..d40599e6d1 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -35,6 +35,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"
@@ -1556,6 +1557,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);
@@ -1605,7 +1607,8 @@ void acpi_build(AcpiBuildTables *tables,
aml_len += tables_blob->len - fadt;
acpi_add_table(table_offsets, tables_blob);
- build_madt(tables_blob, tables->linker, machine, conf);
+ acpi_builder_madt(ab, tables_blob, tables->linker,
+ machine, conf);
vmgenid_dev = find_vmgenid_dev();
if (vmgenid_dev) {
@@ -1629,15 +1632,17 @@ void acpi_build(AcpiBuildTables *tables,
}
if (conf->numa_nodes) {
acpi_add_table(table_offsets, tables_blob);
- build_srat(tables_blob, tables->linker, machine, conf);
+
+ acpi_builder_srat(ab, tables_blob, tables->linker,
+ machine, 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();
@@ -1668,7 +1673,7 @@ void acpi_build(AcpiBuildTables *tables,
slic_oem.id, slic_oem.table_id);
/* RSDP is in FSEG memory, so allocate it separately */
- build_rsdp_rsdt(tables->rsdp, tables->linker, rsdt);
+ 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.
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 1dcbbd5139..986ed0eabd 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -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
@@ -2403,12 +2405,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;
@@ -2443,6 +2453,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);
@@ -2494,6 +2512,7 @@ static const TypeInfo pc_machine_info = {
.interfaces = (InterfaceInfo[]) {
{ TYPE_HOTPLUG_HANDLER },
{ TYPE_NMI },
+ { TYPE_ACPI_BUILDER },
{ }
},
};
--
2.17.2
On 29/10/2018 18:01, Samuel Ortiz wrote:
> @@ -1556,6 +1557,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);
> @@ -1605,7 +1607,8 @@ void acpi_build(AcpiBuildTables *tables,
> aml_len += tables_blob->len - fadt;
>
> acpi_add_table(table_offsets, tables_blob);
> - build_madt(tables_blob, tables->linker, machine, conf);
> + acpi_builder_madt(ab, tables_blob, tables->linker,
> + machine, conf);
>
> vmgenid_dev = find_vmgenid_dev();
> if (vmgenid_dev) {
Just a quick question before I go and actually apply the patches to look
at the resulting code: is there any reason why you didn't add the
MachineState and/or AcpiBuilder to AcpiBuildState, similar to how it was
in v1?
Thanks,
Paolo
Hi Paolo,
On Mon, Oct 29, 2018 at 06:28:39PM +0100, Paolo Bonzini wrote:
> On 29/10/2018 18:01, Samuel Ortiz wrote:
> > @@ -1556,6 +1557,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);
> > @@ -1605,7 +1607,8 @@ void acpi_build(AcpiBuildTables *tables,
> > aml_len += tables_blob->len - fadt;
> >
> > acpi_add_table(table_offsets, tables_blob);
> > - build_madt(tables_blob, tables->linker, machine, conf);
> > + acpi_builder_madt(ab, tables_blob, tables->linker,
> > + machine, conf);
> >
> > vmgenid_dev = find_vmgenid_dev();
> > if (vmgenid_dev) {
>
> Just a quick question before I go and actually apply the patches to look
> at the resulting code: is there any reason why you didn't add the
> MachineState and/or AcpiBuilder to AcpiBuildState, similar to how it was
> in v1?
With v1 I was not adding MachineState to AcpiBuildState, I may be
missing your point.
Do you mean adding an AcpiBuilder pointer to AcpiConfiguration?
Cheers,
Samuel.
On 30/10/2018 15:13, Samuel Ortiz wrote: >> Just a quick question before I go and actually apply the patches to look >> at the resulting code: is there any reason why you didn't add the >> MachineState and/or AcpiBuilder to AcpiBuildState, similar to how it was >> in v1? > > With v1 I was not adding MachineState to AcpiBuildState, I may be > missing your point. > Do you mean adding an AcpiBuilder pointer to AcpiConfiguration? No, what I was remembering is the FirmwareBuildState, which you have removed according to my review. Sorry, KVM Forum was a bit exhausting. :) I think adding the MachineState or AcpiBuilder to AcpiBuildState instead of using qdev_get_machine() makes sense, but it is an independent cleanup. Paolo
On Tue, Oct 30, 2018 at 05:03:28PM +0100, Paolo Bonzini wrote: > On 30/10/2018 15:13, Samuel Ortiz wrote: > >> Just a quick question before I go and actually apply the patches to look > >> at the resulting code: is there any reason why you didn't add the > >> MachineState and/or AcpiBuilder to AcpiBuildState, similar to how it was > >> in v1? > > > > With v1 I was not adding MachineState to AcpiBuildState, I may be > > missing your point. > > Do you mean adding an AcpiBuilder pointer to AcpiConfiguration? > > No, what I was remembering is the FirmwareBuildState, which you have > removed according to my review. Sorry, KVM Forum was a bit exhausting. :) No worries, it was exhausting indeed :) Cheers, Samuel.
On 29/10/18 18:01, Samuel Ortiz wrote:
> All PC machine type derivatives will use the same ACPI table build
> methods. But with that change in place, any new x86 machine type will be
> able to re-use the acpi-build API and customize part of it by defining
> its own ACPI table build methods.
>
> Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
> ---
> hw/i386/acpi-build.c | 15 ++++++++++-----
> hw/i386/pc.c | 19 +++++++++++++++++++
> 2 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 5035dac556..d40599e6d1 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -35,6 +35,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"
> @@ -1556,6 +1557,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);
> @@ -1605,7 +1607,8 @@ void acpi_build(AcpiBuildTables *tables,
> aml_len += tables_blob->len - fadt;
>
> acpi_add_table(table_offsets, tables_blob);
> - build_madt(tables_blob, tables->linker, machine, conf);
> + acpi_builder_madt(ab, tables_blob, tables->linker,
> + machine, conf);
>
> vmgenid_dev = find_vmgenid_dev();
> if (vmgenid_dev) {
> @@ -1629,15 +1632,17 @@ void acpi_build(AcpiBuildTables *tables,
> }
> if (conf->numa_nodes) {
> acpi_add_table(table_offsets, tables_blob);
> - build_srat(tables_blob, tables->linker, machine, conf);
> +
> + acpi_builder_srat(ab, tables_blob, tables->linker,
> + machine, 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();
> @@ -1668,7 +1673,7 @@ void acpi_build(AcpiBuildTables *tables,
> slic_oem.id, slic_oem.table_id);
>
> /* RSDP is in FSEG memory, so allocate it separately */
> - build_rsdp_rsdt(tables->rsdp, tables->linker, rsdt);
> + 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.
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 1dcbbd5139..986ed0eabd 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -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
> @@ -2403,12 +2405,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;
> @@ -2443,6 +2453,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;
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> + 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);
> @@ -2494,6 +2512,7 @@ static const TypeInfo pc_machine_info = {
> .interfaces = (InterfaceInfo[]) {
> { TYPE_HOTPLUG_HANDLER },
> { TYPE_NMI },
> + { TYPE_ACPI_BUILDER },
> { }
> },
> };
>
© 2016 - 2025 Red Hat, Inc.