On Tue, Mar 28, 2023 at 11:59:26AM -0400, Eric DeVolder wrote: > Currently i386 QEMU generates MADT revision 3, and reports > MADT revision 1. ACPI 6.3 introduces MADT revision 5. > > For MADT revision 4, that introduces ARM GIC structures, which do > not apply to i386. > > For MADT revision 5, the Local APIC flags introduces the Online > Capable bitfield. > > Making MADT generate and report revision 5 will solve problems with > CPU hotplug (the Online Capable flag indicates hotpluggable CPUs). > > Signed-off-by: Eric DeVolder <eric.devol...@oracle.com>
I am looking for ways to reduce risk of breakage with this. We don't currently have a reason to change it if cpu hotplug is off, do we? Maybe make it conditional on that. > --- > hw/i386/acpi-common.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c > index 52e5c1439a..1e3a13a36c 100644 > --- a/hw/i386/acpi-common.c > +++ b/hw/i386/acpi-common.c > @@ -38,8 +38,15 @@ void pc_madt_cpu_entry(int uid, const CPUArchIdList > *apic_ids, > { > 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; > + bool enabled = apic_ids->cpus[uid].cpu != NULL || force_enabled ? > + true /* Enabled */ : false; > + /* > + * ACPI 6.3 5.2.12.2 Local APIC Flags: OnlineCapable must be 0 > + * if Enabled is set. > + */ > + bool onlinecapable = enabled ? false : true; /* Online Capable */ > + uint32_t flags = onlinecapable ? 0x2 : 0x0 | > + enabled ? 0x1 : 0x0; > > /* ACPI spec says that LAPIC entry for non present > * CPU may be omitted from MADT or it must be marked > @@ -102,7 +109,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker > *linker, > 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, > + AcpiTable table = { .sig = "APIC", .rev = 5, .oem_id = oem_id, > .oem_table_id = oem_table_id }; > > acpi_table_begin(&table, table_data); > -- > 2.31.1 > > >