Re: [PATCH RFC V6 03/24] hw/arm/virt: Clamp 'maxcpus' as-per machine's vCPU deferred online-capability

[Igor Mammedov <imammedo@redhat.com>]

On Thu, 9 Oct 2025 12:32:15 +0000
Miguel Luis <miguel.luis@oracle.com> wrote:

> Hi Salil,
> 
> > On 1 Oct 2025, at 01:01, salil.mehta@opnsrc.net wrote:
> > 
> > From: Salil Mehta <salil.mehta@huawei.com>
> > 
> > To support a vCPU hot-add–like model on ARM, the virt machine may be setup with
> > more CPUs than are active at boot. These additional CPUs are fully realized in
> > KVM and listed in ACPI tables from the start, but begin in a disabled state.
> > They can later be brought online or taken offline under host or platform policy
> > control. The CPU topology is fixed at VM creation time and cannot change
> > dynamically on ARM. Therefore, we must determine precisely the 'maxcpus' value
> > that applies for the full lifetime of the VM.
> > 
> > On ARM, this deferred online-capable model is only valid if:
> >  - The GIC version is 3 or higher, and
> >  - Each non-boot CPU’s GIC CPU Interface is marked “online-capable” in its
> >    ACPI GICC structure (UEFI ACPI Specification 6.5, §5.2.12.14, Table 5.37
> >    “GICC CPU Interface Flags”), and
> >  - The chosen accelerator supports safe deferred CPU online:
> >      * TCG with multi-threaded TCG (MTTCG) enabled
> >      * KVM (on supported hosts)
> >      * Not HVF or QTest
> > 
> > This patch sizes the machine’s max-possible CPUs during VM init:
> >  - If all conditions are satisfied, retain the full set of CPUs corresponding
> >    to (`-smp cpus` + `-smp disabledcpus`), allowing the additional (initially
> >    disabled) CPUs to participate in later policy-driven online.
> >  - Otherwise, clamp the max-possible CPUs to the boot-enabled count
> >    (`-smp disabledcpus=0` equivalent) to avoid advertising CPUs the guest can
> >    never use.
> > 
> > A new MachineClass flag, `has_online_capable_cpus`, records whether the machine
> > supports deferred vCPU online. This is usable by other machine types as well.  
> 
> 
> By the definition of
> 
>  * @has_hotpluggable_cpus:
>  *    If true, board supports CPUs creation with -device/device_add.
> 
>  in include/hw/boards.h

It should be fine to rename it to has_pluggable_cpus.

But we should add support to arm/virt for -device/device_add cpu_foo,
to avoid awkward -device-set and mangling of -smp.

device_add in arm/virt case probably should be limited non hotplug usecase.

> seems one could take advantage of MachineClass's has_hotpluggable_cpus variable
> instead of creating a new has_online_capable_cpus one.
> (Again, IMHO ‘online capable’ is ACPI nomenclature and doesn’t need to be brought
> in MachineClass’s)

the issue with has_hotpluggable_cpus might be QMP ABI,
where libvirt migh use it to figure out if certain command are supported.

CCing libvirt to check if that would break something.

> 
> Variable which would be initialized in machvirt_init on an assignment based on
> GIC version and/or wether there's inactive CPUs and proceed from there anyways,
> making the default assignment in machine_virt_class_init superfluous.
> 
> We're at hw/arm/virt and we know these CPUs are administratively power state
> coordinated so admin_power_state_supported can still be set there in the
> presence of inactive CPUs.
> 
> Thanks
> Miguel
> 
> > 
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > ---
> > hw/arm/virt.c       | 84 ++++++++++++++++++++++++++++++---------------
> > include/hw/boards.h |  1 +
> > 2 files changed, 57 insertions(+), 28 deletions(-)
> > 
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index ef6be3660f..76f21bd56a 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -2168,8 +2168,7 @@ static void machvirt_init(MachineState *machine)
> >     bool has_ged = !vmc->no_ged;
> >     unsigned int smp_cpus = machine->smp.cpus;
> >     unsigned int max_cpus = machine->smp.max_cpus;
> > -
> > -    possible_cpus = mc->possible_cpu_arch_ids(machine);
> > +    DeviceClass *dc;
> > 
> >     /*
> >      * In accelerated mode, the memory map is computed earlier in kvm_type()
> > @@ -2186,7 +2185,7 @@ static void machvirt_init(MachineState *machine)
> >          * we are about to deal with. Once this is done, get rid of
> >          * the object.
> >          */
> > -        cpuobj = object_new(possible_cpus->cpus[0].type);
> > +        cpuobj = object_new(machine->cpu_type);
> >         armcpu = ARM_CPU(cpuobj);
> > 
> >         pa_bits = arm_pamax(armcpu);
> > @@ -2201,6 +2200,57 @@ static void machvirt_init(MachineState *machine)
> >      */
> >     finalize_gic_version(vms);
> > 
> > +    /*
> > +     * The maximum number of CPUs depends on the GIC version, or on how
> > +     * many redistributors we can fit into the memory map (which in turn
> > +     * depends on whether this is a GICv3 or v4).
> > +     */
> > +    if (vms->gic_version == VIRT_GIC_VERSION_2) {
> > +        virt_max_cpus = GIC_NCPU;
> > +    } else {
> > +        virt_max_cpus = virt_redist_capacity(vms, VIRT_GIC_REDIST);
> > +        if (vms->highmem_redists) {
> > +            virt_max_cpus += virt_redist_capacity(vms, VIRT_HIGH_GIC_REDIST2);
> > +        }
> > +    }
> > +
> > +    if ((tcg_enabled() && !qemu_tcg_mttcg_enabled()) || hvf_enabled() ||
> > +        qtest_enabled() || vms->gic_version == VIRT_GIC_VERSION_2) {
> > +        max_cpus = machine->smp.max_cpus = smp_cpus;
> > +        if (mc->has_online_capable_cpus) {
> > +            if (vms->gic_version == VIRT_GIC_VERSION_2) {
> > +                warn_report("GICv2 does not support online-capable CPUs");
> > +            }
> > +            mc->has_online_capable_cpus = false;
> > +        }
> > +    }
> > +
> > +    if (mc->has_online_capable_cpus) {
> > +        max_cpus = smp_cpus + machine->smp.disabledcpus;
> > +        machine->smp.max_cpus = max_cpus;
> > +    }
> > +
> > +    if (max_cpus > virt_max_cpus) {
> > +        error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
> > +                     "supported by machine 'mach-virt' (%d)",
> > +                     max_cpus, virt_max_cpus);
> > +        if (vms->gic_version != VIRT_GIC_VERSION_2 && !vms->highmem_redists) {
> > +            error_printf("Try 'highmem-redists=on' for more CPUs\n");
> > +        }
> > +
> > +        exit(1);
> > +    }
> > +
> > +    dc = DEVICE_CLASS(object_class_by_name(machine->cpu_type));
> > +    if (!dc) {
> > +        error_report("CPU type '%s' not registered", machine->cpu_type);
> > +        exit(1);
> > +    }
> > +    dc->admin_power_state_supported = mc->has_online_capable_cpus;
> > +
> > +    /* uses smp.max_cpus to initialize all possible vCPUs */
> > +    possible_cpus = mc->possible_cpu_arch_ids(machine);
> > +
> >     if (vms->secure) {
> >         /*
> >          * The Secure view of the world is the same as the NonSecure,
> > @@ -2235,31 +2285,6 @@ static void machvirt_init(MachineState *machine)
> >         vms->psci_conduit = QEMU_PSCI_CONDUIT_HVC;
> >     }
> > 
> > -    /*
> > -     * The maximum number of CPUs depends on the GIC version, or on how
> > -     * many redistributors we can fit into the memory map (which in turn
> > -     * depends on whether this is a GICv3 or v4).
> > -     */
> > -    if (vms->gic_version == VIRT_GIC_VERSION_2) {
> > -        virt_max_cpus = GIC_NCPU;
> > -    } else {
> > -        virt_max_cpus = virt_redist_capacity(vms, VIRT_GIC_REDIST);
> > -        if (vms->highmem_redists) {
> > -            virt_max_cpus += virt_redist_capacity(vms, VIRT_HIGH_GIC_REDIST2);
> > -        }
> > -    }
> > -
> > -    if (max_cpus > virt_max_cpus) {
> > -        error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
> > -                     "supported by machine 'mach-virt' (%d)",
> > -                     max_cpus, virt_max_cpus);
> > -        if (vms->gic_version != VIRT_GIC_VERSION_2 && !vms->highmem_redists) {
> > -            error_printf("Try 'highmem-redists=on' for more CPUs\n");
> > -        }
> > -
> > -        exit(1);
> > -    }
> > -
> >     if (vms->secure && !tcg_enabled() && !qtest_enabled()) {
> >         error_report("mach-virt: %s does not support providing "
> >                      "Security extensions (TrustZone) to the guest CPU",
> > @@ -3245,6 +3270,9 @@ static void virt_machine_class_init(ObjectClass *oc, const void *data)
> >     hc->plug = virt_machine_device_plug_cb;
> >     hc->unplug_request = virt_machine_device_unplug_request_cb;
> >     hc->unplug = virt_machine_device_unplug_cb;
> > +
> > +    mc->has_online_capable_cpus = true;
> > +
> >     mc->nvdimm_supported = true;
> >     mc->smp_props.clusters_supported = true;
> >     mc->auto_enable_numa_with_memhp = true;
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 2b182d7817..b27c2326a2 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -302,6 +302,7 @@ struct MachineClass {
> >     bool rom_file_has_mr;
> >     int minimum_page_bits;
> >     bool has_hotpluggable_cpus;
> > +    bool has_online_capable_cpus;
> >     bool ignore_memory_transaction_failures;
> >     int numa_mem_align_shift;
> >     const char * const *valid_cpu_types;
> > -- 
> > 2.34.1
> >   
>