On 6/14/24 9:36 AM, Salil Mehta wrote:
> If Virtual CPU Hotplug support does not exist on a particular Accel platform or
> ARM GIC version, we should limit the possible vCPUs to those available during
> boot time (i.e SMP CPUs) and explicitly disable Virtual CPU Hotplug support.
>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
> hw/arm/virt.c | 66 +++++++++++++++++++++++++++++----------------------
> 1 file changed, 38 insertions(+), 28 deletions(-)
>
Most of the code changes are moving the check between @max_cpus and @max_supported_cpus.
It would make the review easier if the code movement can be put into a preparatory patch
if you agree.
Thanks,
Gavin
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 11fc7fc318..3e1c4d2d2f 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2082,8 +2082,6 @@ static void machvirt_init(MachineState *machine)
> unsigned int smp_cpus = machine->smp.cpus;
> unsigned int max_cpus = machine->smp.max_cpus;
>
> - possible_cpus = mc->possible_cpu_arch_ids(machine);
> -
> /*
> * In accelerated mode, the memory map is computed earlier in kvm_type()
> * to create a VM with the right number of IPA bits.
> @@ -2098,7 +2096,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);
> @@ -2113,6 +2111,43 @@ 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() || hvf_enabled() || qtest_enabled() ||
> + (vms->gic_version < VIRT_GIC_VERSION_3)) {
> + max_cpus = machine->smp.max_cpus = smp_cpus;
> + mc->has_hotpluggable_cpus = false;
> + if (vms->gic_version >= VIRT_GIC_VERSION_3) {
> + warn_report("cpu hotplug feature has been disabled");
> + }
> + }
> +
> + 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);
> + }
> +
> + /* 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,
> @@ -2147,31 +2182,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 && (kvm_enabled() || hvf_enabled())) {
> error_report("mach-virt: %s does not support providing "
> "Security extensions (TrustZone) to the guest CPU",