* [PATCH v4 1/7] hw/i386/acpi-build: Remove unused attributes
2023-01-21 15:19 [PATCH v4 0/7] AML Housekeeping Bernhard Beschow
@ 2023-01-21 15:19 ` Bernhard Beschow
2023-01-21 15:19 ` [PATCH v4 2/7] hw/isa/isa-bus: Turn isa_build_aml() into qbus_build_aml() Bernhard Beschow
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Bernhard Beschow @ 2023-01-21 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, qemu-trivial, Aurelien Jarno, Eduardo Habkost,
Ani Sinha, Michael S. Tsirkin, Philippe Mathieu-Daudé,
Richard Henderson, Igor Mammedov, Paolo Bonzini,
Markus Armbruster, Bernhard Beschow
Ammends commit 3db119da7915 'pc: acpi: switch to AML API composed DSDT'.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/acpi-build.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 127c4e2d50..8c333973f9 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -117,8 +117,6 @@ typedef struct AcpiMiscInfo {
#ifdef CONFIG_TPM
TPMVersion tpm_version;
#endif
- const unsigned char *dsdt_code;
- unsigned dsdt_size;
} AcpiMiscInfo;
typedef struct FwCfgTPMConfig {
--
2.39.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 2/7] hw/isa/isa-bus: Turn isa_build_aml() into qbus_build_aml()
2023-01-21 15:19 [PATCH v4 0/7] AML Housekeeping Bernhard Beschow
2023-01-21 15:19 ` [PATCH v4 1/7] hw/i386/acpi-build: Remove unused attributes Bernhard Beschow
@ 2023-01-21 15:19 ` Bernhard Beschow
2023-01-21 15:19 ` [PATCH v4 3/7] hw/acpi/piix4: No need to #include "hw/southbridge/piix.h" Bernhard Beschow
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Bernhard Beschow @ 2023-01-21 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, qemu-trivial, Aurelien Jarno, Eduardo Habkost,
Ani Sinha, Michael S. Tsirkin, Philippe Mathieu-Daudé,
Richard Henderson, Igor Mammedov, Paolo Bonzini,
Markus Armbruster, Bernhard Beschow
Frees isa-bus.c from implicit ACPI dependency.
While at it, resolve open coding of qbus_build_aml() in piix3 and ich9.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/acpi/acpi_aml_interface.h | 3 +++
include/hw/isa/isa.h | 1 -
hw/acpi/acpi_interface.c | 10 ++++++++++
hw/i2c/smbus_ich9.c | 5 +----
hw/i386/acpi-microvm.c | 3 ++-
hw/isa/isa-bus.c | 10 ----------
hw/isa/lpc_ich9.c | 5 +----
hw/isa/piix3.c | 5 +----
8 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/include/hw/acpi/acpi_aml_interface.h b/include/hw/acpi/acpi_aml_interface.h
index 436da069d6..11748a8866 100644
--- a/include/hw/acpi/acpi_aml_interface.h
+++ b/include/hw/acpi/acpi_aml_interface.h
@@ -3,6 +3,7 @@
#include "qom/object.h"
#include "hw/acpi/aml-build.h"
+#include "hw/qdev-core.h"
#define TYPE_ACPI_DEV_AML_IF "acpi-dev-aml-interface"
typedef struct AcpiDevAmlIfClass AcpiDevAmlIfClass;
@@ -46,4 +47,6 @@ static inline void call_dev_aml_func(DeviceState *dev, Aml *scope)
}
}
+void qbus_build_aml(BusState *bus, Aml *scope);
+
#endif
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 6c8a8a92cb..25acd5c34c 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -86,7 +86,6 @@ bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp);
ISADevice *isa_create_simple(ISABus *bus, const char *name);
ISADevice *isa_vga_init(ISABus *bus);
-void isa_build_aml(ISABus *bus, Aml *scope);
/**
* isa_register_ioport: Install an I/O port region on the ISA bus.
diff --git a/hw/acpi/acpi_interface.c b/hw/acpi/acpi_interface.c
index c668d361f6..8637ff18fc 100644
--- a/hw/acpi/acpi_interface.c
+++ b/hw/acpi/acpi_interface.c
@@ -2,6 +2,7 @@
#include "hw/acpi/acpi_dev_interface.h"
#include "hw/acpi/acpi_aml_interface.h"
#include "qemu/module.h"
+#include "qemu/queue.h"
void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event)
{
@@ -12,6 +13,15 @@ void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event)
}
}
+void qbus_build_aml(BusState *bus, Aml *scope)
+{
+ BusChild *kid;
+
+ QTAILQ_FOREACH(kid, &bus->children, sibling) {
+ call_dev_aml_func(DEVICE(kid->child), scope);
+ }
+}
+
static void register_types(void)
{
static const TypeInfo acpi_dev_if_info = {
diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
index ee50ba1f2c..52ba77f3fc 100644
--- a/hw/i2c/smbus_ich9.c
+++ b/hw/i2c/smbus_ich9.c
@@ -97,13 +97,10 @@ static void ich9_smbus_realize(PCIDevice *d, Error **errp)
static void build_ich9_smb_aml(AcpiDevAmlIf *adev, Aml *scope)
{
- BusChild *kid;
ICH9SMBState *s = ICH9_SMB_DEVICE(adev);
BusState *bus = BUS(s->smb.smbus);
- QTAILQ_FOREACH(kid, &bus->children, sibling) {
- call_dev_aml_func(DEVICE(kid->child), scope);
- }
+ qbus_build_aml(bus, scope);
}
static void ich9_smb_class_init(ObjectClass *klass, void *data)
diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
index fb09185cbd..a075360d85 100644
--- a/hw/i386/acpi-microvm.c
+++ b/hw/i386/acpi-microvm.c
@@ -26,6 +26,7 @@
#include "exec/memory.h"
#include "hw/acpi/acpi.h"
+#include "hw/acpi/acpi_aml_interface.h"
#include "hw/acpi/aml-build.h"
#include "hw/acpi/bios-linker-loader.h"
#include "hw/acpi/generic_event_device.h"
@@ -129,7 +130,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
sb_scope = aml_scope("_SB");
fw_cfg_add_acpi_dsdt(sb_scope, x86ms->fw_cfg);
- isa_build_aml(ISA_BUS(isabus), sb_scope);
+ qbus_build_aml(BUS(isabus), sb_scope);
build_ged_aml(sb_scope, GED_DEVICE, x86ms->acpi_dev,
GED_MMIO_IRQ, AML_SYSTEM_MEMORY, GED_MMIO_BASE);
acpi_dsdt_add_power_button(sb_scope);
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 1bee1a47f1..f155b80010 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -24,7 +24,6 @@
#include "hw/sysbus.h"
#include "sysemu/sysemu.h"
#include "hw/isa/isa.h"
-#include "hw/acpi/acpi_aml_interface.h"
static ISABus *isabus;
@@ -188,15 +187,6 @@ ISADevice *isa_vga_init(ISABus *bus)
}
}
-void isa_build_aml(ISABus *bus, Aml *scope)
-{
- BusChild *kid;
-
- QTAILQ_FOREACH(kid, &bus->parent_obj.children, sibling) {
- call_dev_aml_func(DEVICE(kid->child), scope);
- }
-}
-
static void isabus_bridge_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 8d541e2b54..1fba3c210c 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -813,7 +813,6 @@ static void ich9_send_gpe(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
static void build_ich9_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
{
Aml *field;
- BusChild *kid;
ICH9LPCState *s = ICH9_LPC_DEVICE(adev);
BusState *bus = BUS(s->isa_bus);
Aml *sb_scope = aml_scope("\\_SB");
@@ -835,9 +834,7 @@ static void build_ich9_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
aml_append(sb_scope, field);
aml_append(scope, sb_scope);
- QTAILQ_FOREACH(kid, &bus->children, sibling) {
- call_dev_aml_func(DEVICE(kid->child), scope);
- }
+ qbus_build_aml(bus, scope);
}
static void ich9_lpc_class_init(ObjectClass *klass, void *data)
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 283b971ec4..a9cb39bf21 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -306,7 +306,6 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
{
Aml *field;
- BusChild *kid;
Aml *sb_scope = aml_scope("\\_SB");
BusState *bus = qdev_get_child_bus(DEVICE(adev), "isa.0");
@@ -322,9 +321,7 @@ static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
aml_append(sb_scope, field);
aml_append(scope, sb_scope);
- QTAILQ_FOREACH(kid, &bus->children, sibling) {
- call_dev_aml_func(DEVICE(kid->child), scope);
- }
+ qbus_build_aml(bus, scope);
}
static void pci_piix3_class_init(ObjectClass *klass, void *data)
--
2.39.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 3/7] hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"
2023-01-21 15:19 [PATCH v4 0/7] AML Housekeeping Bernhard Beschow
2023-01-21 15:19 ` [PATCH v4 1/7] hw/i386/acpi-build: Remove unused attributes Bernhard Beschow
2023-01-21 15:19 ` [PATCH v4 2/7] hw/isa/isa-bus: Turn isa_build_aml() into qbus_build_aml() Bernhard Beschow
@ 2023-01-21 15:19 ` Bernhard Beschow
2023-01-21 15:19 ` [PATCH v4 4/7] hw/acpi/acpi_dev_interface: Remove unused parameter from AcpiDeviceIfClass::madt_cpu Bernhard Beschow
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Bernhard Beschow @ 2023-01-21 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, qemu-trivial, Aurelien Jarno, Eduardo Habkost,
Ani Sinha, Michael S. Tsirkin, Philippe Mathieu-Daudé,
Richard Henderson, Igor Mammedov, Paolo Bonzini,
Markus Armbruster, Bernhard Beschow
hw/acpi/piix4 has its own header with its structure definition etc.
Ammends commit 2bfd0845f0 'hw/acpi/piix4: move PIIX4PMState into
separate piix4.h header'.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/acpi/piix4.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 0a81f1ad93..2ab4930f11 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -21,7 +21,6 @@
#include "qemu/osdep.h"
#include "hw/i386/pc.h"
-#include "hw/southbridge/piix.h"
#include "hw/irq.h"
#include "hw/isa/apm.h"
#include "hw/i2c/pm_smbus.h"
--
2.39.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 4/7] hw/acpi/acpi_dev_interface: Remove unused parameter from AcpiDeviceIfClass::madt_cpu
2023-01-21 15:19 [PATCH v4 0/7] AML Housekeeping Bernhard Beschow
` (2 preceding siblings ...)
2023-01-21 15:19 ` [PATCH v4 3/7] hw/acpi/piix4: No need to #include "hw/southbridge/piix.h" Bernhard Beschow
@ 2023-01-21 15:19 ` Bernhard Beschow
2023-01-21 15:19 ` [PATCH v4 5/7] hw/acpi/acpi_dev_interface: Factor out TYPE_ACPI_CPU_AML_IF Bernhard Beschow
` (3 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Bernhard Beschow @ 2023-01-21 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, qemu-trivial, Aurelien Jarno, Eduardo Habkost,
Ani Sinha, Michael S. Tsirkin, Philippe Mathieu-Daudé,
Richard Henderson, Igor Mammedov, Paolo Bonzini,
Markus Armbruster, Bernhard Beschow
The only function ever assigned to AcpiDeviceIfClass::madt_cpu is
pc_madt_cpu_entry() which doesn't use the AcpiDeviceIf parameter.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/acpi/acpi_dev_interface.h | 3 +--
include/hw/i386/pc.h | 6 ++----
hw/acpi/acpi-x86-stub.c | 5 ++---
hw/acpi/cpu.c | 3 +--
hw/i386/acpi-common.c | 7 +++----
5 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/include/hw/acpi/acpi_dev_interface.h b/include/hw/acpi/acpi_dev_interface.h
index ea6056ab92..a1648220ff 100644
--- a/include/hw/acpi/acpi_dev_interface.h
+++ b/include/hw/acpi/acpi_dev_interface.h
@@ -52,8 +52,7 @@ struct AcpiDeviceIfClass {
/* <public> */
void (*ospm_status)(AcpiDeviceIf *adev, ACPIOSTInfoList ***list);
void (*send_event)(AcpiDeviceIf *adev, AcpiEventStatusBits ev);
- void (*madt_cpu)(AcpiDeviceIf *adev, int uid,
- const CPUArchIdList *apic_ids, GArray *entry,
+ void (*madt_cpu)(int uid, const CPUArchIdList *apic_ids, GArray *entry,
bool force_enabled);
};
#endif
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 88a120bc23..66e3d059ef 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -9,7 +9,6 @@
#include "hw/block/flash.h"
#include "hw/i386/x86.h"
-#include "hw/acpi/acpi_dev_interface.h"
#include "hw/hotplug.h"
#include "qom/object.h"
#include "hw/i386/sgx-epc.h"
@@ -193,9 +192,8 @@ bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size);
/* hw/i386/acpi-common.c */
-void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
- const CPUArchIdList *apic_ids, GArray *entry,
- bool force_enabled);
+void pc_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids,
+ GArray *entry, bool force_enabled);
/* sgx.c */
void pc_machine_init_sgx_epc(PCMachineState *pcms);
diff --git a/hw/acpi/acpi-x86-stub.c b/hw/acpi/acpi-x86-stub.c
index 3df1e090f4..d0d399d26b 100644
--- a/hw/acpi/acpi-x86-stub.c
+++ b/hw/acpi/acpi-x86-stub.c
@@ -2,9 +2,8 @@
#include "hw/i386/pc.h"
#include "hw/i386/acpi-build.h"
-void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
- const CPUArchIdList *apic_ids, GArray *entry,
- bool force_enabled)
+void pc_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids,
+ GArray *entry, bool force_enabled)
{
}
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 4e580959a2..19c154d78f 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -355,7 +355,6 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
char *cphp_res_path = g_strdup_printf("%s." CPUHP_RES_DEVICE, res_root);
Object *obj = object_resolve_path_type("", TYPE_ACPI_DEVICE_IF, NULL);
AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(obj);
- AcpiDeviceIf *adev = ACPI_DEVICE_IF(obj);
cpu_ctrl_dev = aml_device("%s", cphp_res_path);
{
@@ -666,7 +665,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
/* build _MAT object */
assert(adevc && adevc->madt_cpu);
- adevc->madt_cpu(adev, i, arch_ids, madt_buf,
+ adevc->madt_cpu(i, arch_ids, madt_buf,
true); /* set enabled flag */
aml_append(dev, aml_name_decl("_MAT",
aml_buffer(madt_buf->len, (uint8_t *)madt_buf->data)));
diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
index 4aaafbdd7b..52e5c1439a 100644
--- a/hw/i386/acpi-common.c
+++ b/hw/i386/acpi-common.c
@@ -33,9 +33,8 @@
#include "acpi-build.h"
#include "acpi-common.h"
-void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
- const CPUArchIdList *apic_ids, GArray *entry,
- bool force_enabled)
+void pc_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids,
+ GArray *entry, bool force_enabled)
{
uint32_t apic_id = apic_ids->cpus[uid].arch_id;
/* Flags – Local APIC Flags */
@@ -112,7 +111,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /* Flags */
for (i = 0; i < apic_ids->len; i++) {
- adevc->madt_cpu(adev, i, apic_ids, table_data, false);
+ adevc->madt_cpu(i, apic_ids, table_data, false);
if (apic_ids->cpus[i].arch_id > 254) {
x2apic_mode = true;
}
--
2.39.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 5/7] hw/acpi/acpi_dev_interface: Factor out TYPE_ACPI_CPU_AML_IF
2023-01-21 15:19 [PATCH v4 0/7] AML Housekeeping Bernhard Beschow
` (3 preceding siblings ...)
2023-01-21 15:19 ` [PATCH v4 4/7] hw/acpi/acpi_dev_interface: Remove unused parameter from AcpiDeviceIfClass::madt_cpu Bernhard Beschow
@ 2023-01-21 15:19 ` Bernhard Beschow
2023-01-25 16:48 ` Igor Mammedov
2023-01-21 15:19 ` [PATCH v4 6/7] hw/acpi/acpi_dev_interface: Remove now unused #include "hw/boards.h" Bernhard Beschow
` (2 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: Bernhard Beschow @ 2023-01-21 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, qemu-trivial, Aurelien Jarno, Eduardo Habkost,
Ani Sinha, Michael S. Tsirkin, Philippe Mathieu-Daudé,
Richard Henderson, Igor Mammedov, Paolo Bonzini,
Markus Armbruster, Bernhard Beschow
This class attribute was always set to pc_madt_cpu_entry().
pc_madt_cpu_entry() is architecture dependent and was assigned to the
attribute even in architecture agnostic code such as in hw/acpi/piix4.c
and hw/isa/lpc_ich9. Not having to set madt_cpu there resolves the
assumption that these device models are only ever used with ACPI on x86
targets.
The only target independent location where madt_cpu was called was hw/
acpi/cpu.c. Here a function pointer can be passed via an argument
instead. The other locations where it was called was in x86-specific code
where pc_madt_cpu_entry() can be used directly.
While at it, move pc_madt_cpu_entry() from the public include/hw/i386/
pc.h to the private hw/i386/acpi-common where it is also implemented.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
TYPE_ACPI_CPU_IF
---
hw/i386/acpi-common.h | 3 +--
include/hw/acpi/acpi_cpu_interface.h | 26 ++++++++++++++++++++++++++
include/hw/acpi/acpi_dev_interface.h | 2 --
hw/acpi/acpi_interface.c | 8 +++++++-
hw/acpi/cpu.c | 11 ++++++-----
hw/acpi/piix4.c | 2 --
hw/i386/acpi-build.c | 3 +--
hw/i386/acpi-common.c | 8 +++++---
hw/i386/acpi-microvm.c | 3 +--
hw/i386/generic_event_device_x86.c | 9 ---------
hw/isa/lpc_ich9.c | 1 -
target/i386/cpu.c | 13 +++++++++++++
12 files changed, 60 insertions(+), 29 deletions(-)
create mode 100644 include/hw/acpi/acpi_cpu_interface.h
diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
index a68825acf5..b3c56ee014 100644
--- a/hw/i386/acpi-common.h
+++ b/hw/i386/acpi-common.h
@@ -1,7 +1,6 @@
#ifndef HW_I386_ACPI_COMMON_H
#define HW_I386_ACPI_COMMON_H
-#include "hw/acpi/acpi_dev_interface.h"
#include "hw/acpi/bios-linker-loader.h"
#include "hw/i386/x86.h"
@@ -9,7 +8,7 @@
#define ACPI_BUILD_IOAPIC_ID 0x0
void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
- X86MachineState *x86ms, AcpiDeviceIf *adev,
+ X86MachineState *x86ms,
const char *oem_id, const char *oem_table_id);
#endif
diff --git a/include/hw/acpi/acpi_cpu_interface.h b/include/hw/acpi/acpi_cpu_interface.h
new file mode 100644
index 0000000000..600f0b68cd
--- /dev/null
+++ b/include/hw/acpi/acpi_cpu_interface.h
@@ -0,0 +1,26 @@
+#ifndef ACPI_CPU_INTERFACE_H
+#define ACPI_CPU_INTERFACE_H
+
+#include "qom/object.h"
+#include "hw/boards.h"
+#include "hw/qdev-core.h"
+
+#define TYPE_ACPI_CPU_AML_IF "acpi-cpu-aml-interface"
+
+typedef struct AcpiCpuAmlIfClass AcpiCpuAmlIfClass;
+DECLARE_CLASS_CHECKERS(AcpiCpuAmlIfClass, ACPI_CPU_AML_IF,
+ TYPE_ACPI_CPU_AML_IF)
+#define ACPI_CPU_AML_IF(obj) \
+ INTERFACE_CHECK(AcpiCpuAmlIf, (obj), TYPE_ACPI_CPU_AML_IF)
+
+typedef struct AcpiCpuAmlIf AcpiCpuAmlIf;
+
+struct AcpiCpuAmlIfClass {
+ /* <private> */
+ InterfaceClass parent_class;
+
+ /* <public> */
+ void (*madt_cpu)(int uid, const CPUArchIdList *apic_ids, GArray *entry,
+ bool force_enabled);
+};
+#endif
diff --git a/include/hw/acpi/acpi_dev_interface.h b/include/hw/acpi/acpi_dev_interface.h
index a1648220ff..ca92928124 100644
--- a/include/hw/acpi/acpi_dev_interface.h
+++ b/include/hw/acpi/acpi_dev_interface.h
@@ -52,7 +52,5 @@ struct AcpiDeviceIfClass {
/* <public> */
void (*ospm_status)(AcpiDeviceIf *adev, ACPIOSTInfoList ***list);
void (*send_event)(AcpiDeviceIf *adev, AcpiEventStatusBits ev);
- void (*madt_cpu)(int uid, const CPUArchIdList *apic_ids, GArray *entry,
- bool force_enabled);
};
#endif
diff --git a/hw/acpi/acpi_interface.c b/hw/acpi/acpi_interface.c
index 8637ff18fc..11a57e2154 100644
--- a/hw/acpi/acpi_interface.c
+++ b/hw/acpi/acpi_interface.c
@@ -1,4 +1,5 @@
#include "qemu/osdep.h"
+#include "hw/acpi/acpi_cpu_interface.h"
#include "hw/acpi/acpi_dev_interface.h"
#include "hw/acpi/acpi_aml_interface.h"
#include "qemu/module.h"
@@ -34,10 +35,15 @@ static void register_types(void)
.parent = TYPE_INTERFACE,
.class_size = sizeof(AcpiDevAmlIfClass),
};
-
+ static const TypeInfo acpi_cpu_aml_if_info = {
+ .name = TYPE_ACPI_CPU_AML_IF,
+ .parent = TYPE_INTERFACE,
+ .class_size = sizeof(AcpiCpuAmlIfClass),
+ };
type_register_static(&acpi_dev_if_info);
type_register_static(&acpi_dev_aml_if_info);
+ type_register_static(&acpi_cpu_aml_if_info);
}
type_init(register_types)
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 19c154d78f..f6647e99b1 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -1,5 +1,6 @@
#include "qemu/osdep.h"
#include "migration/vmstate.h"
+#include "hw/acpi/acpi_cpu_interface.h"
#include "hw/acpi/cpu.h"
#include "qapi/error.h"
#include "qapi/qapi-events-acpi.h"
@@ -353,8 +354,6 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
MachineClass *mc = MACHINE_GET_CLASS(machine);
const CPUArchIdList *arch_ids = mc->possible_cpu_arch_ids(machine);
char *cphp_res_path = g_strdup_printf("%s." CPUHP_RES_DEVICE, res_root);
- Object *obj = object_resolve_path_type("", TYPE_ACPI_DEVICE_IF, NULL);
- AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(obj);
cpu_ctrl_dev = aml_device("%s", cphp_res_path);
{
@@ -648,6 +647,8 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
for (i = 0; i < arch_ids->len; i++) {
Aml *dev;
Aml *uid = aml_int(i);
+ ObjectClass *oc = object_class_by_name(arch_ids->cpus[i].type);
+ AcpiCpuAmlIfClass *acpuac = ACPI_CPU_AML_IF_CLASS(oc);
GArray *madt_buf = g_array_new(0, 1, 1);
int arch_id = arch_ids->cpus[i].arch_id;
@@ -664,9 +665,9 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
aml_append(dev, method);
/* build _MAT object */
- assert(adevc && adevc->madt_cpu);
- adevc->madt_cpu(i, arch_ids, madt_buf,
- true); /* set enabled flag */
+ assert(acpuac && acpuac->madt_cpu);
+ acpuac->madt_cpu(i, arch_ids, madt_buf,
+ true); /* set enabled flag */
aml_append(dev, aml_name_decl("_MAT",
aml_buffer(madt_buf->len, (uint8_t *)madt_buf->data)));
g_array_free(madt_buf, true);
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 2ab4930f11..2e19a55526 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -20,7 +20,6 @@
*/
#include "qemu/osdep.h"
-#include "hw/i386/pc.h"
#include "hw/irq.h"
#include "hw/isa/apm.h"
#include "hw/i2c/pm_smbus.h"
@@ -642,7 +641,6 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
hc->unplug = piix4_device_unplug_cb;
adevc->ospm_status = piix4_ospm_status;
adevc->send_event = piix4_send_gpe;
- adevc->madt_cpu = pc_madt_cpu_entry;
}
static const TypeInfo piix4_pm_info = {
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 8c333973f9..b12a843447 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2422,8 +2422,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
acpi_add_table(table_offsets, tables_blob);
acpi_build_madt(tables_blob, tables->linker, x86ms,
- ACPI_DEVICE_IF(x86ms->acpi_dev), x86ms->oem_id,
- x86ms->oem_table_id);
+ x86ms->oem_id, x86ms->oem_table_id);
#ifdef CONFIG_ACPI_ERST
{
diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
index 52e5c1439a..0d1a2bb8aa 100644
--- a/hw/i386/acpi-common.c
+++ b/hw/i386/acpi-common.c
@@ -25,6 +25,7 @@
#include "exec/memory.h"
#include "hw/acpi/acpi.h"
+#include "hw/acpi/acpi_cpu_interface.h"
#include "hw/acpi/aml-build.h"
#include "hw/acpi/utils.h"
#include "hw/i386/pc.h"
@@ -94,14 +95,13 @@ build_xrupt_override(GArray *entry, uint8_t src, uint32_t gsi, uint16_t flags)
* 5.2.8 Multiple APIC Description Table
*/
void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
- X86MachineState *x86ms, AcpiDeviceIf *adev,
+ X86MachineState *x86ms,
const char *oem_id, const char *oem_table_id)
{
int i;
bool x2apic_mode = false;
MachineClass *mc = MACHINE_GET_CLASS(x86ms);
const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
- AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
AcpiTable table = { .sig = "APIC", .rev = 1, .oem_id = oem_id,
.oem_table_id = oem_table_id };
@@ -111,7 +111,9 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /* Flags */
for (i = 0; i < apic_ids->len; i++) {
- adevc->madt_cpu(i, apic_ids, table_data, false);
+ ObjectClass *oc = object_class_by_name(apic_ids->cpus[i].type);
+ AcpiCpuAmlIfClass *acpuac = ACPI_CPU_AML_IF_CLASS(oc);
+ acpuac->madt_cpu(i, apic_ids, table_data, false);
if (apic_ids->cpus[i].arch_id > 254) {
x2apic_mode = true;
}
diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
index a075360d85..fec22d85c1 100644
--- a/hw/i386/acpi-microvm.c
+++ b/hw/i386/acpi-microvm.c
@@ -214,8 +214,7 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
acpi_add_table(table_offsets, tables_blob);
acpi_build_madt(tables_blob, tables->linker, X86_MACHINE(machine),
- ACPI_DEVICE_IF(x86ms->acpi_dev), x86ms->oem_id,
- x86ms->oem_table_id);
+ x86ms->oem_id, x86ms->oem_table_id);
#ifdef CONFIG_ACPI_ERST
{
diff --git a/hw/i386/generic_event_device_x86.c b/hw/i386/generic_event_device_x86.c
index e26fb02a2e..8fc233e1f1 100644
--- a/hw/i386/generic_event_device_x86.c
+++ b/hw/i386/generic_event_device_x86.c
@@ -8,19 +8,10 @@
#include "qemu/osdep.h"
#include "hw/acpi/generic_event_device.h"
-#include "hw/i386/pc.h"
-
-static void acpi_ged_x86_class_init(ObjectClass *class, void *data)
-{
- AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
-
- adevc->madt_cpu = pc_madt_cpu_entry;
-}
static const TypeInfo acpi_ged_x86_info = {
.name = TYPE_ACPI_GED_X86,
.parent = TYPE_ACPI_GED,
- .class_init = acpi_ged_x86_class_init,
.interfaces = (InterfaceInfo[]) {
{ TYPE_HOTPLUG_HANDLER },
{ TYPE_ACPI_DEVICE_IF },
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 1fba3c210c..d5d4b0f177 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -867,7 +867,6 @@ static void ich9_lpc_class_init(ObjectClass *klass, void *data)
hc->unplug = ich9_pm_device_unplug_cb;
adevc->ospm_status = ich9_pm_ospm_status;
adevc->send_event = ich9_send_gpe;
- adevc->madt_cpu = pc_madt_cpu_entry;
amldevc->build_dev_aml = build_ich9_isa_aml;
}
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 4d2b8d0444..6ac50506a7 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -37,7 +37,9 @@
#include "hw/i386/topology.h"
#ifndef CONFIG_USER_ONLY
#include "exec/address-spaces.h"
+#include "hw/acpi/acpi_cpu_interface.h"
#include "hw/boards.h"
+#include "hw/i386/pc.h"
#include "hw/i386/sgx-epc.h"
#endif
@@ -7114,6 +7116,9 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
CPUClass *cc = CPU_CLASS(oc);
DeviceClass *dc = DEVICE_CLASS(oc);
ResettableClass *rc = RESETTABLE_CLASS(oc);
+#ifndef CONFIG_USER_ONLY
+ AcpiCpuAmlIfClass *acpuac = ACPI_CPU_AML_IF_CLASS(oc);
+#endif
FeatureWord w;
device_class_set_parent_realize(dc, x86_cpu_realizefn,
@@ -7138,6 +7143,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
#ifndef CONFIG_USER_ONLY
cc->sysemu_ops = &i386_sysemu_ops;
+ acpuac->madt_cpu = pc_madt_cpu_entry;
#endif /* !CONFIG_USER_ONLY */
cc->gdb_arch_name = x86_gdb_arch_name;
@@ -7203,6 +7209,13 @@ static const TypeInfo x86_cpu_type_info = {
.abstract = true,
.class_size = sizeof(X86CPUClass),
.class_init = x86_cpu_common_class_init,
+
+#ifndef CONFIG_USER_ONLY
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_ACPI_CPU_AML_IF },
+ { }
+ }
+#endif
};
/* "base" CPU model, used by query-cpu-model-expansion */
--
2.39.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 5/7] hw/acpi/acpi_dev_interface: Factor out TYPE_ACPI_CPU_AML_IF
2023-01-21 15:19 ` [PATCH v4 5/7] hw/acpi/acpi_dev_interface: Factor out TYPE_ACPI_CPU_AML_IF Bernhard Beschow
@ 2023-01-25 16:48 ` Igor Mammedov
0 siblings, 0 replies; 12+ messages in thread
From: Igor Mammedov @ 2023-01-25 16:48 UTC (permalink / raw)
To: Bernhard Beschow
Cc: qemu-devel, Marcel Apfelbaum, qemu-trivial, Aurelien Jarno,
Eduardo Habkost, Ani Sinha, Michael S. Tsirkin,
Philippe Mathieu-Daudé, Richard Henderson, Paolo Bonzini,
Markus Armbruster
On Sat, 21 Jan 2023 16:19:39 +0100
Bernhard Beschow <shentey@gmail.com> wrote:
> This class attribute was always set to pc_madt_cpu_entry().
> pc_madt_cpu_entry() is architecture dependent and was assigned to the
> attribute even in architecture agnostic code such as in hw/acpi/piix4.c
> and hw/isa/lpc_ich9. Not having to set madt_cpu there resolves the
> assumption that these device models are only ever used with ACPI on x86
> targets.
While it would work, I don't see a compelling reason to drop
AcpiDeviceIf::madt_cpu and cause all this churn and extra code.
Simple pc_madt_cpu_entry() stub works just fine for targets
that don't need madt_cpu (it's not elegant but, it's the way
such issues a typically dealt with).
If goal is to clean up piix4 from *all* x86 extensions and have
pure pixx4 and make sure other targets do not pull in not needed
ACPI dependencies, then this patch should be a part of a series
that does all of that to demonstrate that achievable without
breaking migration.
So far I'm not convinced that this patch has merit on its own.
I suggest to drop it for now.
> The only target independent location where madt_cpu was called was hw/
> acpi/cpu.c. Here a function pointer can be passed via an argument
> instead. The other locations where it was called was in x86-specific code
> where pc_madt_cpu_entry() can be used directly.
this part diverged from what this patch does.
> While at it, move pc_madt_cpu_entry() from the public include/hw/i386/
> pc.h to the private hw/i386/acpi-common where it is also implemented.
PS:
usage of 'ifdef's is not welcome in new code,
unless *there is no other way* around that.
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>
> TYPE_ACPI_CPU_IF
> ---
> hw/i386/acpi-common.h | 3 +--
> include/hw/acpi/acpi_cpu_interface.h | 26 ++++++++++++++++++++++++++
> include/hw/acpi/acpi_dev_interface.h | 2 --
> hw/acpi/acpi_interface.c | 8 +++++++-
> hw/acpi/cpu.c | 11 ++++++-----
> hw/acpi/piix4.c | 2 --
> hw/i386/acpi-build.c | 3 +--
> hw/i386/acpi-common.c | 8 +++++---
> hw/i386/acpi-microvm.c | 3 +--
> hw/i386/generic_event_device_x86.c | 9 ---------
> hw/isa/lpc_ich9.c | 1 -
> target/i386/cpu.c | 13 +++++++++++++
> 12 files changed, 60 insertions(+), 29 deletions(-)
> create mode 100644 include/hw/acpi/acpi_cpu_interface.h
>
> diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
> index a68825acf5..b3c56ee014 100644
> --- a/hw/i386/acpi-common.h
> +++ b/hw/i386/acpi-common.h
> @@ -1,7 +1,6 @@
> #ifndef HW_I386_ACPI_COMMON_H
> #define HW_I386_ACPI_COMMON_H
>
> -#include "hw/acpi/acpi_dev_interface.h"
> #include "hw/acpi/bios-linker-loader.h"
> #include "hw/i386/x86.h"
>
> @@ -9,7 +8,7 @@
> #define ACPI_BUILD_IOAPIC_ID 0x0
>
> void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> - X86MachineState *x86ms, AcpiDeviceIf *adev,
> + X86MachineState *x86ms,
> const char *oem_id, const char *oem_table_id);
>
> #endif
> diff --git a/include/hw/acpi/acpi_cpu_interface.h b/include/hw/acpi/acpi_cpu_interface.h
> new file mode 100644
> index 0000000000..600f0b68cd
> --- /dev/null
> +++ b/include/hw/acpi/acpi_cpu_interface.h
> @@ -0,0 +1,26 @@
> +#ifndef ACPI_CPU_INTERFACE_H
> +#define ACPI_CPU_INTERFACE_H
> +
> +#include "qom/object.h"
> +#include "hw/boards.h"
> +#include "hw/qdev-core.h"
> +
> +#define TYPE_ACPI_CPU_AML_IF "acpi-cpu-aml-interface"
> +
> +typedef struct AcpiCpuAmlIfClass AcpiCpuAmlIfClass;
> +DECLARE_CLASS_CHECKERS(AcpiCpuAmlIfClass, ACPI_CPU_AML_IF,
> + TYPE_ACPI_CPU_AML_IF)
> +#define ACPI_CPU_AML_IF(obj) \
> + INTERFACE_CHECK(AcpiCpuAmlIf, (obj), TYPE_ACPI_CPU_AML_IF)
> +
> +typedef struct AcpiCpuAmlIf AcpiCpuAmlIf;
> +
> +struct AcpiCpuAmlIfClass {
> + /* <private> */
> + InterfaceClass parent_class;
> +
> + /* <public> */
> + void (*madt_cpu)(int uid, const CPUArchIdList *apic_ids, GArray *entry,
> + bool force_enabled);
> +};
> +#endif
> diff --git a/include/hw/acpi/acpi_dev_interface.h b/include/hw/acpi/acpi_dev_interface.h
> index a1648220ff..ca92928124 100644
> --- a/include/hw/acpi/acpi_dev_interface.h
> +++ b/include/hw/acpi/acpi_dev_interface.h
> @@ -52,7 +52,5 @@ struct AcpiDeviceIfClass {
> /* <public> */
> void (*ospm_status)(AcpiDeviceIf *adev, ACPIOSTInfoList ***list);
> void (*send_event)(AcpiDeviceIf *adev, AcpiEventStatusBits ev);
> - void (*madt_cpu)(int uid, const CPUArchIdList *apic_ids, GArray *entry,
> - bool force_enabled);
> };
> #endif
> diff --git a/hw/acpi/acpi_interface.c b/hw/acpi/acpi_interface.c
> index 8637ff18fc..11a57e2154 100644
> --- a/hw/acpi/acpi_interface.c
> +++ b/hw/acpi/acpi_interface.c
> @@ -1,4 +1,5 @@
> #include "qemu/osdep.h"
> +#include "hw/acpi/acpi_cpu_interface.h"
> #include "hw/acpi/acpi_dev_interface.h"
> #include "hw/acpi/acpi_aml_interface.h"
> #include "qemu/module.h"
> @@ -34,10 +35,15 @@ static void register_types(void)
> .parent = TYPE_INTERFACE,
> .class_size = sizeof(AcpiDevAmlIfClass),
> };
> -
> + static const TypeInfo acpi_cpu_aml_if_info = {
> + .name = TYPE_ACPI_CPU_AML_IF,
> + .parent = TYPE_INTERFACE,
> + .class_size = sizeof(AcpiCpuAmlIfClass),
> + };
>
> type_register_static(&acpi_dev_if_info);
> type_register_static(&acpi_dev_aml_if_info);
> + type_register_static(&acpi_cpu_aml_if_info);
> }
>
> type_init(register_types)
> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> index 19c154d78f..f6647e99b1 100644
> --- a/hw/acpi/cpu.c
> +++ b/hw/acpi/cpu.c
> @@ -1,5 +1,6 @@
> #include "qemu/osdep.h"
> #include "migration/vmstate.h"
> +#include "hw/acpi/acpi_cpu_interface.h"
> #include "hw/acpi/cpu.h"
> #include "qapi/error.h"
> #include "qapi/qapi-events-acpi.h"
> @@ -353,8 +354,6 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> const CPUArchIdList *arch_ids = mc->possible_cpu_arch_ids(machine);
> char *cphp_res_path = g_strdup_printf("%s." CPUHP_RES_DEVICE, res_root);
> - Object *obj = object_resolve_path_type("", TYPE_ACPI_DEVICE_IF, NULL);
> - AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(obj);
>
> cpu_ctrl_dev = aml_device("%s", cphp_res_path);
> {
> @@ -648,6 +647,8 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> for (i = 0; i < arch_ids->len; i++) {
> Aml *dev;
> Aml *uid = aml_int(i);
> + ObjectClass *oc = object_class_by_name(arch_ids->cpus[i].type);
> + AcpiCpuAmlIfClass *acpuac = ACPI_CPU_AML_IF_CLASS(oc);
> GArray *madt_buf = g_array_new(0, 1, 1);
> int arch_id = arch_ids->cpus[i].arch_id;
>
> @@ -664,9 +665,9 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> aml_append(dev, method);
>
> /* build _MAT object */
> - assert(adevc && adevc->madt_cpu);
> - adevc->madt_cpu(i, arch_ids, madt_buf,
> - true); /* set enabled flag */
> + assert(acpuac && acpuac->madt_cpu);
> + acpuac->madt_cpu(i, arch_ids, madt_buf,
> + true); /* set enabled flag */
> aml_append(dev, aml_name_decl("_MAT",
> aml_buffer(madt_buf->len, (uint8_t *)madt_buf->data)));
> g_array_free(madt_buf, true);
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 2ab4930f11..2e19a55526 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -20,7 +20,6 @@
> */
>
> #include "qemu/osdep.h"
> -#include "hw/i386/pc.h"
> #include "hw/irq.h"
> #include "hw/isa/apm.h"
> #include "hw/i2c/pm_smbus.h"
> @@ -642,7 +641,6 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
> hc->unplug = piix4_device_unplug_cb;
> adevc->ospm_status = piix4_ospm_status;
> adevc->send_event = piix4_send_gpe;
> - adevc->madt_cpu = pc_madt_cpu_entry;
> }
>
> static const TypeInfo piix4_pm_info = {
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 8c333973f9..b12a843447 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2422,8 +2422,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>
> acpi_add_table(table_offsets, tables_blob);
> acpi_build_madt(tables_blob, tables->linker, x86ms,
> - ACPI_DEVICE_IF(x86ms->acpi_dev), x86ms->oem_id,
> - x86ms->oem_table_id);
> + x86ms->oem_id, x86ms->oem_table_id);
>
> #ifdef CONFIG_ACPI_ERST
> {
> diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> index 52e5c1439a..0d1a2bb8aa 100644
> --- a/hw/i386/acpi-common.c
> +++ b/hw/i386/acpi-common.c
> @@ -25,6 +25,7 @@
>
> #include "exec/memory.h"
> #include "hw/acpi/acpi.h"
> +#include "hw/acpi/acpi_cpu_interface.h"
> #include "hw/acpi/aml-build.h"
> #include "hw/acpi/utils.h"
> #include "hw/i386/pc.h"
> @@ -94,14 +95,13 @@ build_xrupt_override(GArray *entry, uint8_t src, uint32_t gsi, uint16_t flags)
> * 5.2.8 Multiple APIC Description Table
> */
> void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> - X86MachineState *x86ms, AcpiDeviceIf *adev,
> + X86MachineState *x86ms,
> const char *oem_id, const char *oem_table_id)
> {
> int i;
> bool x2apic_mode = false;
> MachineClass *mc = MACHINE_GET_CLASS(x86ms);
> const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
> - AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
> AcpiTable table = { .sig = "APIC", .rev = 1, .oem_id = oem_id,
> .oem_table_id = oem_table_id };
>
> @@ -111,7 +111,9 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /* Flags */
>
> for (i = 0; i < apic_ids->len; i++) {
> - adevc->madt_cpu(i, apic_ids, table_data, false);
> + ObjectClass *oc = object_class_by_name(apic_ids->cpus[i].type);
> + AcpiCpuAmlIfClass *acpuac = ACPI_CPU_AML_IF_CLASS(oc);
> + acpuac->madt_cpu(i, apic_ids, table_data, false);
> if (apic_ids->cpus[i].arch_id > 254) {
> x2apic_mode = true;
> }
> diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
> index a075360d85..fec22d85c1 100644
> --- a/hw/i386/acpi-microvm.c
> +++ b/hw/i386/acpi-microvm.c
> @@ -214,8 +214,7 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
>
> acpi_add_table(table_offsets, tables_blob);
> acpi_build_madt(tables_blob, tables->linker, X86_MACHINE(machine),
> - ACPI_DEVICE_IF(x86ms->acpi_dev), x86ms->oem_id,
> - x86ms->oem_table_id);
> + x86ms->oem_id, x86ms->oem_table_id);
>
> #ifdef CONFIG_ACPI_ERST
> {
> diff --git a/hw/i386/generic_event_device_x86.c b/hw/i386/generic_event_device_x86.c
> index e26fb02a2e..8fc233e1f1 100644
> --- a/hw/i386/generic_event_device_x86.c
> +++ b/hw/i386/generic_event_device_x86.c
> @@ -8,19 +8,10 @@
>
> #include "qemu/osdep.h"
> #include "hw/acpi/generic_event_device.h"
> -#include "hw/i386/pc.h"
> -
> -static void acpi_ged_x86_class_init(ObjectClass *class, void *data)
> -{
> - AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
> -
> - adevc->madt_cpu = pc_madt_cpu_entry;
> -}
>
> static const TypeInfo acpi_ged_x86_info = {
> .name = TYPE_ACPI_GED_X86,
> .parent = TYPE_ACPI_GED,
> - .class_init = acpi_ged_x86_class_init,
> .interfaces = (InterfaceInfo[]) {
> { TYPE_HOTPLUG_HANDLER },
> { TYPE_ACPI_DEVICE_IF },
> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> index 1fba3c210c..d5d4b0f177 100644
> --- a/hw/isa/lpc_ich9.c
> +++ b/hw/isa/lpc_ich9.c
> @@ -867,7 +867,6 @@ static void ich9_lpc_class_init(ObjectClass *klass, void *data)
> hc->unplug = ich9_pm_device_unplug_cb;
> adevc->ospm_status = ich9_pm_ospm_status;
> adevc->send_event = ich9_send_gpe;
> - adevc->madt_cpu = pc_madt_cpu_entry;
> amldevc->build_dev_aml = build_ich9_isa_aml;
> }
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 4d2b8d0444..6ac50506a7 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -37,7 +37,9 @@
> #include "hw/i386/topology.h"
> #ifndef CONFIG_USER_ONLY
> #include "exec/address-spaces.h"
> +#include "hw/acpi/acpi_cpu_interface.h"
> #include "hw/boards.h"
> +#include "hw/i386/pc.h"
> #include "hw/i386/sgx-epc.h"
> #endif
>
> @@ -7114,6 +7116,9 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
> CPUClass *cc = CPU_CLASS(oc);
> DeviceClass *dc = DEVICE_CLASS(oc);
> ResettableClass *rc = RESETTABLE_CLASS(oc);
> +#ifndef CONFIG_USER_ONLY
> + AcpiCpuAmlIfClass *acpuac = ACPI_CPU_AML_IF_CLASS(oc);
> +#endif
> FeatureWord w;
>
> device_class_set_parent_realize(dc, x86_cpu_realizefn,
> @@ -7138,6 +7143,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>
> #ifndef CONFIG_USER_ONLY
> cc->sysemu_ops = &i386_sysemu_ops;
> + acpuac->madt_cpu = pc_madt_cpu_entry;
> #endif /* !CONFIG_USER_ONLY */
>
> cc->gdb_arch_name = x86_gdb_arch_name;
> @@ -7203,6 +7209,13 @@ static const TypeInfo x86_cpu_type_info = {
> .abstract = true,
> .class_size = sizeof(X86CPUClass),
> .class_init = x86_cpu_common_class_init,
> +
> +#ifndef CONFIG_USER_ONLY
> + .interfaces = (InterfaceInfo[]) {
> + { TYPE_ACPI_CPU_AML_IF },
> + { }
> + }
> +#endif
> };
>
> /* "base" CPU model, used by query-cpu-model-expansion */
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 6/7] hw/acpi/acpi_dev_interface: Remove now unused #include "hw/boards.h"
2023-01-21 15:19 [PATCH v4 0/7] AML Housekeeping Bernhard Beschow
` (4 preceding siblings ...)
2023-01-21 15:19 ` [PATCH v4 5/7] hw/acpi/acpi_dev_interface: Factor out TYPE_ACPI_CPU_AML_IF Bernhard Beschow
@ 2023-01-21 15:19 ` Bernhard Beschow
2023-01-21 15:19 ` [PATCH v4 7/7] hw/i386/pc: Unexport pc_madt_cpu_entry() Bernhard Beschow
2023-01-25 16:52 ` [PATCH v4 0/7] AML Housekeeping Igor Mammedov
7 siblings, 0 replies; 12+ messages in thread
From: Bernhard Beschow @ 2023-01-21 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, qemu-trivial, Aurelien Jarno, Eduardo Habkost,
Ani Sinha, Michael S. Tsirkin, Philippe Mathieu-Daudé,
Richard Henderson, Igor Mammedov, Paolo Bonzini,
Markus Armbruster, Bernhard Beschow
Removing the "hw/boards.h" include from hw/acpi/acpi_dev_interface.h
requires include fixes in various unrelated files to keep the code
compiling.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
hw/acpi/hmat.h | 3 ++-
include/hw/acpi/acpi_dev_interface.h | 1 -
hw/acpi/cpu.c | 1 +
hw/acpi/hmat.c | 1 +
hw/acpi/memory_hotplug.c | 1 +
monitor/qmp-cmds.c | 1 +
6 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/acpi/hmat.h b/hw/acpi/hmat.h
index b57f0e7e80..fd989cb661 100644
--- a/hw/acpi/hmat.h
+++ b/hw/acpi/hmat.h
@@ -27,7 +27,8 @@
#ifndef HMAT_H
#define HMAT_H
-#include "hw/acpi/aml-build.h"
+#include "hw/acpi/bios-linker-loader.h"
+#include "sysemu/numa.h"
/*
* ACPI 6.3: 5.2.27.3 Memory Proximity Domain Attributes Structure,
diff --git a/include/hw/acpi/acpi_dev_interface.h b/include/hw/acpi/acpi_dev_interface.h
index ca92928124..68d9d15f50 100644
--- a/include/hw/acpi/acpi_dev_interface.h
+++ b/include/hw/acpi/acpi_dev_interface.h
@@ -3,7 +3,6 @@
#include "qapi/qapi-types-acpi.h"
#include "qom/object.h"
-#include "hw/boards.h"
#include "hw/qdev-core.h"
/* These values are part of guest ABI, and can not be changed */
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index f6647e99b1..1a7eb54c98 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -2,6 +2,7 @@
#include "migration/vmstate.h"
#include "hw/acpi/acpi_cpu_interface.h"
#include "hw/acpi/cpu.h"
+#include "hw/core/cpu.h"
#include "qapi/error.h"
#include "qapi/qapi-events-acpi.h"
#include "trace.h"
diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
index 3a6d51282a..d9de0daf89 100644
--- a/hw/acpi/hmat.c
+++ b/hw/acpi/hmat.c
@@ -27,6 +27,7 @@
#include "qemu/osdep.h"
#include "qemu/units.h"
#include "sysemu/numa.h"
+#include "hw/acpi/aml-build.h"
#include "hw/acpi/hmat.h"
/*
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index d926f4f77d..0b883df813 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -1,6 +1,7 @@
#include "qemu/osdep.h"
#include "hw/acpi/memory_hotplug.h"
#include "hw/mem/pc-dimm.h"
+#include "hw/boards.h"
#include "hw/qdev-core.h"
#include "migration/vmstate.h"
#include "trace.h"
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index bf22a8c5a6..051b825986 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -41,6 +41,7 @@
#include "hw/acpi/acpi_dev_interface.h"
#include "hw/intc/intc.h"
#include "hw/rdma/rdma.h"
+#include "hw/boards.h"
#include "monitor/stats.h"
NameInfo *qmp_query_name(Error **errp)
--
2.39.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 7/7] hw/i386/pc: Unexport pc_madt_cpu_entry()
2023-01-21 15:19 [PATCH v4 0/7] AML Housekeeping Bernhard Beschow
` (5 preceding siblings ...)
2023-01-21 15:19 ` [PATCH v4 6/7] hw/acpi/acpi_dev_interface: Remove now unused #include "hw/boards.h" Bernhard Beschow
@ 2023-01-21 15:19 ` Bernhard Beschow
2023-01-25 16:52 ` [PATCH v4 0/7] AML Housekeeping Igor Mammedov
7 siblings, 0 replies; 12+ messages in thread
From: Bernhard Beschow @ 2023-01-21 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, qemu-trivial, Aurelien Jarno, Eduardo Habkost,
Ani Sinha, Michael S. Tsirkin, Philippe Mathieu-Daudé,
Richard Henderson, Igor Mammedov, Paolo Bonzini,
Markus Armbruster, Bernhard Beschow
pc_madt_cpu_entry() is now only used in target/i386/cpu, so move it
there, and unexport and rename it.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
include/hw/i386/pc.h | 4 ----
hw/acpi/acpi-x86-stub.c | 6 ------
hw/i386/acpi-common.c | 33 ---------------------------------
target/i386/cpu.c | 37 +++++++++++++++++++++++++++++++++++--
4 files changed, 35 insertions(+), 45 deletions(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 66e3d059ef..9ab1818812 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -191,10 +191,6 @@ bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
int *data_len);
void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size);
-/* hw/i386/acpi-common.c */
-void pc_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids,
- GArray *entry, bool force_enabled);
-
/* sgx.c */
void pc_machine_init_sgx_epc(PCMachineState *pcms);
diff --git a/hw/acpi/acpi-x86-stub.c b/hw/acpi/acpi-x86-stub.c
index d0d399d26b..9662a594ad 100644
--- a/hw/acpi/acpi-x86-stub.c
+++ b/hw/acpi/acpi-x86-stub.c
@@ -1,12 +1,6 @@
#include "qemu/osdep.h"
-#include "hw/i386/pc.h"
#include "hw/i386/acpi-build.h"
-void pc_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids,
- GArray *entry, bool force_enabled)
-{
-}
-
Object *acpi_get_i386_pci_host(void)
{
return NULL;
diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
index 0d1a2bb8aa..0041623aeb 100644
--- a/hw/i386/acpi-common.c
+++ b/hw/i386/acpi-common.c
@@ -28,44 +28,11 @@
#include "hw/acpi/acpi_cpu_interface.h"
#include "hw/acpi/aml-build.h"
#include "hw/acpi/utils.h"
-#include "hw/i386/pc.h"
#include "target/i386/cpu.h"
#include "acpi-build.h"
#include "acpi-common.h"
-void pc_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids,
- GArray *entry, bool force_enabled)
-{
- uint32_t apic_id = apic_ids->cpus[uid].arch_id;
- /* Flags – Local APIC Flags */
- uint32_t flags = apic_ids->cpus[uid].cpu != NULL || force_enabled ?
- 1 /* Enabled */ : 0;
-
- /* ACPI spec says that LAPIC entry for non present
- * CPU may be omitted from MADT or it must be marked
- * as disabled. However omitting non present CPU from
- * MADT breaks hotplug on linux. So possible CPUs
- * should be put in MADT but kept disabled.
- */
- if (apic_id < 255) {
- /* Rev 1.0b, Table 5-13 Processor Local APIC Structure */
- build_append_int_noprefix(entry, 0, 1); /* Type */
- build_append_int_noprefix(entry, 8, 1); /* Length */
- build_append_int_noprefix(entry, uid, 1); /* ACPI Processor ID */
- build_append_int_noprefix(entry, apic_id, 1); /* APIC ID */
- build_append_int_noprefix(entry, flags, 4); /* Flags */
- } else {
- /* Rev 4.0, 5.2.12.12 Processor Local x2APIC Structure */
- build_append_int_noprefix(entry, 9, 1); /* Type */
- build_append_int_noprefix(entry, 16, 1); /* Length */
- build_append_int_noprefix(entry, 0, 2); /* Reserved */
- build_append_int_noprefix(entry, apic_id, 4); /* X2APIC ID */
- build_append_int_noprefix(entry, flags, 4); /* Flags */
- build_append_int_noprefix(entry, uid, 4); /* ACPI Processor UID */
- }
-}
-
static void build_ioapic(GArray *entry, uint8_t id, uint32_t addr, uint32_t irq)
{
/* Rev 1.0b, 5.2.8.2 IO APIC */
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 6ac50506a7..b05062bc57 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -38,8 +38,8 @@
#ifndef CONFIG_USER_ONLY
#include "exec/address-spaces.h"
#include "hw/acpi/acpi_cpu_interface.h"
+#include "hw/acpi/aml-build.h"
#include "hw/boards.h"
-#include "hw/i386/pc.h"
#include "hw/i386/sgx-epc.h"
#endif
@@ -7108,6 +7108,39 @@ static const struct SysemuCPUOps i386_sysemu_ops = {
.write_elf64_qemunote = x86_cpu_write_elf64_qemunote,
.legacy_vmsd = &vmstate_x86_cpu,
};
+
+static void x86_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids,
+ GArray *entry, bool force_enabled)
+{
+ uint32_t apic_id = apic_ids->cpus[uid].arch_id;
+ /* Flags – Local APIC Flags */
+ uint32_t flags = apic_ids->cpus[uid].cpu != NULL || force_enabled ?
+ 1 /* Enabled */ : 0;
+
+ /*
+ * ACPI spec says that LAPIC entry for non present
+ * CPU may be omitted from MADT or it must be marked
+ * as disabled. However omitting non present CPU from
+ * MADT breaks hotplug on linux. So possible CPUs
+ * should be put in MADT but kept disabled.
+ */
+ if (apic_id < 255) {
+ /* Rev 1.0b, Table 5-13 Processor Local APIC Structure */
+ build_append_int_noprefix(entry, 0, 1); /* Type */
+ build_append_int_noprefix(entry, 8, 1); /* Length */
+ build_append_int_noprefix(entry, uid, 1); /* ACPI Processor ID */
+ build_append_int_noprefix(entry, apic_id, 1); /* APIC ID */
+ build_append_int_noprefix(entry, flags, 4); /* Flags */
+ } else {
+ /* Rev 4.0, 5.2.12.12 Processor Local x2APIC Structure */
+ build_append_int_noprefix(entry, 9, 1); /* Type */
+ build_append_int_noprefix(entry, 16, 1); /* Length */
+ build_append_int_noprefix(entry, 0, 2); /* Reserved */
+ build_append_int_noprefix(entry, apic_id, 4); /* X2APIC ID */
+ build_append_int_noprefix(entry, flags, 4); /* Flags */
+ build_append_int_noprefix(entry, uid, 4); /* ACPI Processor UID */
+ }
+}
#endif
static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
@@ -7143,7 +7176,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
#ifndef CONFIG_USER_ONLY
cc->sysemu_ops = &i386_sysemu_ops;
- acpuac->madt_cpu = pc_madt_cpu_entry;
+ acpuac->madt_cpu = x86_madt_cpu_entry;
#endif /* !CONFIG_USER_ONLY */
cc->gdb_arch_name = x86_gdb_arch_name;
--
2.39.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 0/7] AML Housekeeping
2023-01-21 15:19 [PATCH v4 0/7] AML Housekeeping Bernhard Beschow
` (6 preceding siblings ...)
2023-01-21 15:19 ` [PATCH v4 7/7] hw/i386/pc: Unexport pc_madt_cpu_entry() Bernhard Beschow
@ 2023-01-25 16:52 ` Igor Mammedov
2023-01-26 10:42 ` Bernhard Beschow
7 siblings, 1 reply; 12+ messages in thread
From: Igor Mammedov @ 2023-01-25 16:52 UTC (permalink / raw)
To: Bernhard Beschow
Cc: qemu-devel, Marcel Apfelbaum, qemu-trivial, Aurelien Jarno,
Eduardo Habkost, Ani Sinha, Michael S. Tsirkin,
Philippe Mathieu-Daudé, Richard Henderson, Paolo Bonzini,
Markus Armbruster
On Sat, 21 Jan 2023 16:19:34 +0100
Bernhard Beschow <shentey@gmail.com> wrote:
> This series factors out AcpiCpuAmlIfClass::madt_cpu from AcpiDeviceIfClass.
> By letting the (x86) CPUs implement the new interface, AML generation is
> delegated to the CPUs, freeing the ACPI controllers from worrying about x86 CPU
> specifics. The delegation to the CPUs is especially interesting for the PIIX4 PM
> since it is also used in MIPS only contexts where no ACPI bios is available.
>
> Furthermore, the series introduces qbus_build_aml() which replaces
> isa_build_aml() and resolves some open coding.
I'm done with this series review
(skipped 6-7/7, since they depend on 5/7 which seems unnecessary to me)
>
> v4:
> - Squash qbus_build_aml() patches into one (Igor)
> - Don't use a bare function pointer for AcpiDeviceIfClass::madt_cpu (Igor)
>
> Testing done:
> * `make check`
> * `qemu-system-x86_64 -M pc -m 2G -cdrom manjaro-kde-21.2.6-220416-linux515.iso`
> * `qemu-system-x86_64 -M q35 -m 2G -cdrom \
> manjaro-kde-21.2.6-220416-linux515.iso`
>
> v3:
> - Clean up includes in AcpiDeviceIfClass::madt_cpu sub series last (Markus)
> - Restructure qbus_build_aml() sub series (Phil, me)
>
> v2:
> - Don't inline qbus_build_aml() (Phil)
> - Add 'hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"'
>
> Bernhard Beschow (7):
> hw/i386/acpi-build: Remove unused attributes
> hw/isa/isa-bus: Turn isa_build_aml() into qbus_build_aml()
> hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"
> hw/acpi/acpi_dev_interface: Remove unused parameter from
> AcpiDeviceIfClass::madt_cpu
> hw/acpi/acpi_dev_interface: Factor out TYPE_ACPI_CPU_AML_IF
> hw/acpi/acpi_dev_interface: Remove now unused #include "hw/boards.h"
> hw/i386/pc: Unexport pc_madt_cpu_entry()
>
> hw/acpi/hmat.h | 3 +-
> hw/i386/acpi-common.h | 3 +-
> include/hw/acpi/acpi_aml_interface.h | 3 ++
> include/hw/acpi/acpi_cpu_interface.h | 26 ++++++++++++++++
> include/hw/acpi/acpi_dev_interface.h | 4 ---
> include/hw/i386/pc.h | 6 ----
> include/hw/isa/isa.h | 1 -
> hw/acpi/acpi-x86-stub.c | 7 -----
> hw/acpi/acpi_interface.c | 18 ++++++++++-
> hw/acpi/cpu.c | 13 ++++----
> hw/acpi/hmat.c | 1 +
> hw/acpi/memory_hotplug.c | 1 +
> hw/acpi/piix4.c | 3 --
> hw/i2c/smbus_ich9.c | 5 +--
> hw/i386/acpi-build.c | 5 +--
> hw/i386/acpi-common.c | 42 +++----------------------
> hw/i386/acpi-microvm.c | 6 ++--
> hw/i386/generic_event_device_x86.c | 9 ------
> hw/isa/isa-bus.c | 10 ------
> hw/isa/lpc_ich9.c | 6 +---
> hw/isa/piix3.c | 5 +--
> monitor/qmp-cmds.c | 1 +
> target/i386/cpu.c | 46 ++++++++++++++++++++++++++++
> 23 files changed, 117 insertions(+), 107 deletions(-)
> create mode 100644 include/hw/acpi/acpi_cpu_interface.h
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 0/7] AML Housekeeping
2023-01-25 16:52 ` [PATCH v4 0/7] AML Housekeeping Igor Mammedov
@ 2023-01-26 10:42 ` Bernhard Beschow
2023-01-27 12:58 ` Michael S. Tsirkin
0 siblings, 1 reply; 12+ messages in thread
From: Bernhard Beschow @ 2023-01-26 10:42 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, Marcel Apfelbaum, qemu-trivial, Aurelien Jarno,
Eduardo Habkost, Ani Sinha, Michael S. Tsirkin,
Philippe Mathieu-Daudé, Richard Henderson, Paolo Bonzini,
Markus Armbruster
Am 25. Januar 2023 16:52:34 UTC schrieb Igor Mammedov <imammedo@redhat.com>:
>On Sat, 21 Jan 2023 16:19:34 +0100
>Bernhard Beschow <shentey@gmail.com> wrote:
>
>> This series factors out AcpiCpuAmlIfClass::madt_cpu from AcpiDeviceIfClass.
>> By letting the (x86) CPUs implement the new interface, AML generation is
>> delegated to the CPUs, freeing the ACPI controllers from worrying about x86 CPU
>> specifics. The delegation to the CPUs is especially interesting for the PIIX4 PM
>> since it is also used in MIPS only contexts where no ACPI bios is available.
>>
>> Furthermore, the series introduces qbus_build_aml() which replaces
>> isa_build_aml() and resolves some open coding.
>
>I'm done with this series review
>(skipped 6-7/7, since they depend on 5/7 which seems unnecessary to me)
Thanks!
Okay, let's omit patches 5-7 for now. It makes sense to include them in a dedicated x86 cleanup series.
Michael, shall I respin a v5 with only the reviewed patches?
Best regards,
Bernhard
>
>>
>> v4:
>> - Squash qbus_build_aml() patches into one (Igor)
>> - Don't use a bare function pointer for AcpiDeviceIfClass::madt_cpu (Igor)
>>
>> Testing done:
>> * `make check`
>> * `qemu-system-x86_64 -M pc -m 2G -cdrom manjaro-kde-21.2.6-220416-linux515.iso`
>> * `qemu-system-x86_64 -M q35 -m 2G -cdrom \
>> manjaro-kde-21.2.6-220416-linux515.iso`
>>
>> v3:
>> - Clean up includes in AcpiDeviceIfClass::madt_cpu sub series last (Markus)
>> - Restructure qbus_build_aml() sub series (Phil, me)
>>
>> v2:
>> - Don't inline qbus_build_aml() (Phil)
>> - Add 'hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"'
>>
>> Bernhard Beschow (7):
>> hw/i386/acpi-build: Remove unused attributes
>> hw/isa/isa-bus: Turn isa_build_aml() into qbus_build_aml()
>> hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"
>> hw/acpi/acpi_dev_interface: Remove unused parameter from
>> AcpiDeviceIfClass::madt_cpu
>> hw/acpi/acpi_dev_interface: Factor out TYPE_ACPI_CPU_AML_IF
>> hw/acpi/acpi_dev_interface: Remove now unused #include "hw/boards.h"
>> hw/i386/pc: Unexport pc_madt_cpu_entry()
>>
>> hw/acpi/hmat.h | 3 +-
>> hw/i386/acpi-common.h | 3 +-
>> include/hw/acpi/acpi_aml_interface.h | 3 ++
>> include/hw/acpi/acpi_cpu_interface.h | 26 ++++++++++++++++
>> include/hw/acpi/acpi_dev_interface.h | 4 ---
>> include/hw/i386/pc.h | 6 ----
>> include/hw/isa/isa.h | 1 -
>> hw/acpi/acpi-x86-stub.c | 7 -----
>> hw/acpi/acpi_interface.c | 18 ++++++++++-
>> hw/acpi/cpu.c | 13 ++++----
>> hw/acpi/hmat.c | 1 +
>> hw/acpi/memory_hotplug.c | 1 +
>> hw/acpi/piix4.c | 3 --
>> hw/i2c/smbus_ich9.c | 5 +--
>> hw/i386/acpi-build.c | 5 +--
>> hw/i386/acpi-common.c | 42 +++----------------------
>> hw/i386/acpi-microvm.c | 6 ++--
>> hw/i386/generic_event_device_x86.c | 9 ------
>> hw/isa/isa-bus.c | 10 ------
>> hw/isa/lpc_ich9.c | 6 +---
>> hw/isa/piix3.c | 5 +--
>> monitor/qmp-cmds.c | 1 +
>> target/i386/cpu.c | 46 ++++++++++++++++++++++++++++
>> 23 files changed, 117 insertions(+), 107 deletions(-)
>> create mode 100644 include/hw/acpi/acpi_cpu_interface.h
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 0/7] AML Housekeeping
2023-01-26 10:42 ` Bernhard Beschow
@ 2023-01-27 12:58 ` Michael S. Tsirkin
0 siblings, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2023-01-27 12:58 UTC (permalink / raw)
To: Bernhard Beschow
Cc: Igor Mammedov, qemu-devel, Marcel Apfelbaum, qemu-trivial,
Aurelien Jarno, Eduardo Habkost, Ani Sinha,
Philippe Mathieu-Daudé, Richard Henderson, Paolo Bonzini,
Markus Armbruster
On Thu, Jan 26, 2023 at 10:42:31AM +0000, Bernhard Beschow wrote:
>
>
> Am 25. Januar 2023 16:52:34 UTC schrieb Igor Mammedov <imammedo@redhat.com>:
> >On Sat, 21 Jan 2023 16:19:34 +0100
> >Bernhard Beschow <shentey@gmail.com> wrote:
> >
> >> This series factors out AcpiCpuAmlIfClass::madt_cpu from AcpiDeviceIfClass.
> >> By letting the (x86) CPUs implement the new interface, AML generation is
> >> delegated to the CPUs, freeing the ACPI controllers from worrying about x86 CPU
> >> specifics. The delegation to the CPUs is especially interesting for the PIIX4 PM
> >> since it is also used in MIPS only contexts where no ACPI bios is available.
> >>
> >> Furthermore, the series introduces qbus_build_aml() which replaces
> >> isa_build_aml() and resolves some open coding.
> >
> >I'm done with this series review
> >(skipped 6-7/7, since they depend on 5/7 which seems unnecessary to me)
>
> Thanks!
>
> Okay, let's omit patches 5-7 for now. It makes sense to include them in a dedicated x86 cleanup series.
>
> Michael, shall I respin a v5 with only the reviewed patches?
>
> Best regards,
> Bernhard
No need.
> >
> >>
> >> v4:
> >> - Squash qbus_build_aml() patches into one (Igor)
> >> - Don't use a bare function pointer for AcpiDeviceIfClass::madt_cpu (Igor)
> >>
> >> Testing done:
> >> * `make check`
> >> * `qemu-system-x86_64 -M pc -m 2G -cdrom manjaro-kde-21.2.6-220416-linux515.iso`
> >> * `qemu-system-x86_64 -M q35 -m 2G -cdrom \
> >> manjaro-kde-21.2.6-220416-linux515.iso`
> >>
> >> v3:
> >> - Clean up includes in AcpiDeviceIfClass::madt_cpu sub series last (Markus)
> >> - Restructure qbus_build_aml() sub series (Phil, me)
> >>
> >> v2:
> >> - Don't inline qbus_build_aml() (Phil)
> >> - Add 'hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"'
> >>
> >> Bernhard Beschow (7):
> >> hw/i386/acpi-build: Remove unused attributes
> >> hw/isa/isa-bus: Turn isa_build_aml() into qbus_build_aml()
> >> hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"
> >> hw/acpi/acpi_dev_interface: Remove unused parameter from
> >> AcpiDeviceIfClass::madt_cpu
> >> hw/acpi/acpi_dev_interface: Factor out TYPE_ACPI_CPU_AML_IF
> >> hw/acpi/acpi_dev_interface: Remove now unused #include "hw/boards.h"
> >> hw/i386/pc: Unexport pc_madt_cpu_entry()
> >>
> >> hw/acpi/hmat.h | 3 +-
> >> hw/i386/acpi-common.h | 3 +-
> >> include/hw/acpi/acpi_aml_interface.h | 3 ++
> >> include/hw/acpi/acpi_cpu_interface.h | 26 ++++++++++++++++
> >> include/hw/acpi/acpi_dev_interface.h | 4 ---
> >> include/hw/i386/pc.h | 6 ----
> >> include/hw/isa/isa.h | 1 -
> >> hw/acpi/acpi-x86-stub.c | 7 -----
> >> hw/acpi/acpi_interface.c | 18 ++++++++++-
> >> hw/acpi/cpu.c | 13 ++++----
> >> hw/acpi/hmat.c | 1 +
> >> hw/acpi/memory_hotplug.c | 1 +
> >> hw/acpi/piix4.c | 3 --
> >> hw/i2c/smbus_ich9.c | 5 +--
> >> hw/i386/acpi-build.c | 5 +--
> >> hw/i386/acpi-common.c | 42 +++----------------------
> >> hw/i386/acpi-microvm.c | 6 ++--
> >> hw/i386/generic_event_device_x86.c | 9 ------
> >> hw/isa/isa-bus.c | 10 ------
> >> hw/isa/lpc_ich9.c | 6 +---
> >> hw/isa/piix3.c | 5 +--
> >> monitor/qmp-cmds.c | 1 +
> >> target/i386/cpu.c | 46 ++++++++++++++++++++++++++++
> >> 23 files changed, 117 insertions(+), 107 deletions(-)
> >> create mode 100644 include/hw/acpi/acpi_cpu_interface.h
> >>
> >
^ permalink raw reply [flat|nested] 12+ messages in thread