Re: [PATCH RFC V6 11/24] hw/arm/acpi: MADT change to size the guest with possible vCPUs

[Igor Mammedov <imammedo@redhat.com>]

On Wed,  1 Oct 2025 01:01:14 +0000
salil.mehta@opnsrc.net wrote:

> From: Salil Mehta <salil.mehta@huawei.com>
> 
> When QEMU builds the MADT table, modifications are needed to include information
> about possible vCPUs that are exposed as ACPI-disabled (i.e., `_STA.Enabled=0`).
> This new information will help the guest kernel pre-size its resources during
> boot time. Pre-sizing based on possible vCPUs will facilitate the future
> hot-plugging of the currently disabled vCPUs.
> 
> Additionally, this change addresses updates to the ACPI MADT GIC CPU interface
> flags, as introduced in the UEFI ACPI 6.5 specification [1]. These updates
> enable deferred virtual CPU onlining in the guest kernel.
> 
> Reference:
> [1] 5.2.12.14. GIC CPU Interface (GICC) Structure (Table 5.37 GICC CPU Interface Flags)
>     Link: https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#gic-cpu-interface-gicc-structure
> 
> Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>  hw/arm/virt-acpi-build.c | 40 ++++++++++++++++++++++++++++++++++------
>  hw/core/machine.c        | 14 ++++++++++++++
>  include/hw/boards.h      | 20 ++++++++++++++++++++
>  3 files changed, 68 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index b01fc4f8ef..7c24dd6369 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -760,6 +760,32 @@ static void build_append_gicr(GArray *table_data, uint64_t base, uint32_t size)
>      build_append_int_noprefix(table_data, size, 4); /* Discovery Range Length */
>  }
>  
> +static uint32_t virt_acpi_get_gicc_flags(CPUState *cpu)
> +{
> +    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
> +    const uint32_t GICC_FLAG_ENABLED = BIT(0);
> +    const uint32_t GICC_FLAG_ONLINE_CAPABLE = BIT(3);
> +
> +    /* ARM architecture does not support vCPU hotplug yet */
> +    if (!cpu) {
> +        return 0;
> +    }
> +
> +    /*
> +     * If the machine does not support online-capable CPUs, report the GICC as
> +     * 'enabled' only.
> +     */
> +    if (!mc->has_online_capable_cpus) {
> +        return GICC_FLAG_ENABLED;
> +    }
> +
> +    /*
> +     * ACPI 6.5, 5.2.12.14 (GICC): mark the boot CPU 'enabled' and all others
> +     * 'online-capable'.
> +     */
> +    return (cpu == first_cpu) ? GICC_FLAG_ENABLED : GICC_FLAG_ONLINE_CAPABLE;
> +}
> +
>  static void
>  build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>  {
> @@ -785,12 +811,14 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>      build_append_int_noprefix(table_data, vms->gic_version, 1);
>      build_append_int_noprefix(table_data, 0, 3);   /* Reserved */
>  
> -    for (i = 0; i < MACHINE(vms)->smp.cpus; i++) {
> -        ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i));
> +    for (i = 0; i < MACHINE(vms)->smp.max_cpus; i++) {
                                     ^^^^^^^^^^^^
> +        CPUState *cpu = machine_get_possible_cpu(i);
...
> +        CPUArchId *archid = machine_get_possible_cpu_arch_id(i);

what complexity above adds? /and then you say creating instantiating ARM VM
is slow./

I'd drop machine_get_possible_cpu/machine_get_possible_cpu_arch_id altogether
and mimic what acpi_build_madt() does.

> +        uint32_t flags = virt_acpi_get_gicc_flags(cpu);
> +        uint64_t mpidr = archid->arch_id;
>  
>          if (vms->gic_version == VIRT_GIC_VERSION_2) {
>              physical_base_address = memmap[VIRT_GIC_CPU].base;
> @@ -805,7 +833,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>          build_append_int_noprefix(table_data, i, 4);    /* GIC ID */
>          build_append_int_noprefix(table_data, i, 4);    /* ACPI Processor UID */
>          /* Flags */
> -        build_append_int_noprefix(table_data, 1, 4);    /* Enabled */
> +        build_append_int_noprefix(table_data, flags, 4);
>          /* Parking Protocol Version */
>          build_append_int_noprefix(table_data, 0, 4);
>          /* Performance Interrupt GSIV */
> @@ -819,7 +847,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>          build_append_int_noprefix(table_data, vgic_interrupt, 4);
>          build_append_int_noprefix(table_data, 0, 8);    /* GICR Base Address*/
>          /* MPIDR */
> -        build_append_int_noprefix(table_data, arm_cpu_mp_affinity(armcpu), 8);
> +        build_append_int_noprefix(table_data, mpidr, 8);
>          /* Processor Power Efficiency Class */
>          build_append_int_noprefix(table_data, 0, 1);
>          /* Reserved */
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 69d5632464..65388d859a 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1383,6 +1383,20 @@ CPUState *machine_get_possible_cpu(int64_t cpu_index)
>      return NULL;
>  }
>  
> +CPUArchId *machine_get_possible_cpu_arch_id(int64_t cpu_index)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    CPUArchIdList *possible_cpus = ms->possible_cpus;
> +
> +    for (int i = 0; i < possible_cpus->len; i++) {
> +        if (possible_cpus->cpus[i].cpu &&
> +            possible_cpus->cpus[i].cpu->cpu_index == cpu_index) {
> +            return &possible_cpus->cpus[i];
> +        }
> +    }
> +    return NULL;
> +}
> +
>  static char *cpu_slot_to_string(const CPUArchId *cpu)
>  {
>      GString *s = g_string_new(NULL);
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 3ff77a8b3a..fe51ca58bf 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -461,6 +461,26 @@ struct MachineState {
>      bool acpi_spcr_enabled;
>  };
>  
> +/*
> + * machine_get_possible_cpu_arch_id:
> + * @cpu_index: logical cpu_index to search for
> + *
> + * Return a pointer to the CPUArchId entry matching the given @cpu_index
> + * in the current machine's MachineState. The possible_cpus array holds
> + * the full set of CPUs that the machine could support, including those
> + * that may be created as disabled or taken offline.
> + *
> + * The slot index in ms->possible_cpus[] is always sequential, but the
> + * logical cpu_index values are assigned by QEMU and may or may not be
> + * sequential depending on the implementation of a particular machine.
> + * Direct indexing by cpu_index is therefore unsafe in general. This
> + * helper performs a linear search of the possible_cpus array to find
> + * the matching entry.
> + *
> + * Returns: pointer to the matching CPUArchId, or NULL if not found.
> + */
> +CPUArchId *machine_get_possible_cpu_arch_id(int64_t cpu_index);
> +
>  /*
>   * The macros which follow are intended to facilitate the
>   * definition of versioned machine types, using a somewhat