Re: [PATCH v2 19/23] hw/i386: Move pc_madt_cpu_entry() to acpi-pc.c

[Igor Mammedov]

On Wed, 16 Jun 2021 22:43:24 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> pc_madt_cpu_entry() is specific to QEMU 'PC' machines,
> move it to acpi-pc.c.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/i386/acpi-common.c | 40 ----------------------------------------
>  hw/i386/acpi-pc.c     | 39 +++++++++++++++++++++++++++++++++++++++

it's used not only by PC machines but also microvm,
which didn't use acpi-build.c (aka acpi-pc.c)
it only links fine by virtue that PC machines
are object files are also included.

>  2 files changed, 39 insertions(+), 40 deletions(-)
> 
> diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> index 77afebd9e1f..5ae1853b6f2 100644
> --- a/hw/i386/acpi-common.c
> +++ b/hw/i386/acpi-common.c
> @@ -23,49 +23,10 @@
>  #include "qemu/osdep.h"
>  #include "hw/acpi/acpi.h"
>  #include "hw/acpi/aml-build.h"
> -#include "hw/i386/pc.h"
>  #include "target/i386/cpu.h"
>  #include "acpi-build.h"
>  #include "acpi-common.h"
>  
> -void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> -                       const CPUArchIdList *apic_ids, GArray *entry)
> -{
> -    uint32_t apic_id = apic_ids->cpus[uid].arch_id;
> -
> -    /* 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) {
> -        AcpiMadtProcessorApic *apic = acpi_data_push(entry, sizeof *apic);
> -
> -        apic->type = ACPI_APIC_PROCESSOR;
> -        apic->length = sizeof(*apic);
> -        apic->processor_id = uid;
> -        apic->local_apic_id = apic_id;
> -        if (apic_ids->cpus[uid].cpu != NULL) {
> -            apic->flags = cpu_to_le32(1);
> -        } else {
> -            apic->flags = cpu_to_le32(0);
> -        }
> -    } else {
> -        AcpiMadtProcessorX2Apic *apic = acpi_data_push(entry, sizeof *apic);
> -
> -        apic->type = ACPI_APIC_LOCAL_X2APIC;
> -        apic->length = sizeof(*apic);
> -        apic->uid = cpu_to_le32(uid);
> -        apic->x2apic_id = cpu_to_le32(apic_id);
> -        if (apic_ids->cpus[uid].cpu != NULL) {
> -            apic->flags = cpu_to_le32(1);
> -        } else {
> -            apic->flags = cpu_to_le32(0);
> -        }
> -    }
> -}
> -
>  void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
>                       X86MachineState *x86ms, AcpiDeviceIf *adev,
>                       const char *oem_id, const char *oem_table_id)
> @@ -155,4 +116,3 @@ void acpi_build_madt(GArray *table_data, BIOSLinker 
> *linker,
>                   (void *)(table_data->data + madt_start), "APIC",
>                   table_data->len - madt_start, 1, oem_id, oem_table_id);
>  }
> -
> diff --git a/hw/i386/acpi-pc.c b/hw/i386/acpi-pc.c
> index 796ffc6f5c4..a3cd60d81e6 100644
> --- a/hw/i386/acpi-pc.c
> +++ b/hw/i386/acpi-pc.c
> @@ -2707,3 +2707,42 @@ void acpi_setup(void)
>       */
>      acpi_build_tables_cleanup(&tables, false);
>  }
> +
> +void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> +                       const CPUArchIdList *apic_ids, GArray *entry)
> +{
> +    uint32_t apic_id = apic_ids->cpus[uid].arch_id;
> +
> +    /*
> +     * 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) {
> +        AcpiMadtProcessorApic *apic = acpi_data_push(entry, sizeof *apic);
> +
> +        apic->type = ACPI_APIC_PROCESSOR;
> +        apic->length = sizeof(*apic);
> +        apic->processor_id = uid;
> +        apic->local_apic_id = apic_id;
> +        if (apic_ids->cpus[uid].cpu != NULL) {
> +            apic->flags = cpu_to_le32(1);
> +        } else {
> +            apic->flags = cpu_to_le32(0);
> +        }
> +    } else {
> +        AcpiMadtProcessorX2Apic *apic = acpi_data_push(entry, sizeof *apic);
> +
> +        apic->type = ACPI_APIC_LOCAL_X2APIC;
> +        apic->length = sizeof(*apic);
> +        apic->uid = cpu_to_le32(uid);
> +        apic->x2apic_id = cpu_to_le32(apic_id);
> +        if (apic_ids->cpus[uid].cpu != NULL) {
> +            apic->flags = cpu_to_le32(1);
> +        } else {
> +            apic->flags = cpu_to_le32(0);
> +        }
> +    }
> +}