On Tue, 28 Mar 2023 11:59:26 -0400
Eric DeVolder <eric.devol...@oracle.com> 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).
So spec mandates 3 possible states
00t - not present and not can't be added later ever
01t - present
10t - not present but might be added later
and outlawed 11t combination
00t - doesn't make much sense (i.e. why put such entry in MADT in the 1st place)
but looking at kernel commit aa06e20f1be, it looks like
ACPI_MADT_ONLINE_CAPABLE was introduced to accommodate
firmware/hw folks who would stuff MADT with LAPIC entries
for all possible CPU models, and then patch it depending on
actually used CPU model instead of dynamically creating LAPIC
entries. (insane)
> Signed-off-by: Eric DeVolder <eric.devol...@oracle.com>
> ---
> 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;
align the last line with onlinecapable ....'
move /* Enabled */ and /* Online Capable */ comments right to magic values
i.e. onlinecapable ? 0x2 : 0x0 | /* Online Capable */ ...
>
> /* 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);
