Hi Salil,
On 8/19/24 10:21 PM, Salil Mehta wrote:
From: Gavin Shan <gs...@redhat.com>
Sent: Tuesday, August 13, 2024 2:17 AM
To: Salil Mehta <salil.me...@huawei.com>; qemu-devel@nongnu.org;
qemu-...@nongnu.org; m...@redhat.com
On 6/14/24 9:36 AM, Salil Mehta wrote:
> During `machvirt_init()`, QOM ARMCPU objects are pre-created along
> with the corresponding KVM vCPUs in the host for all possible vCPUs.
> This is necessary due to the architectural constraint that KVM
> restricts the deferred creation of KVM vCPUs and VGIC
> initialization/sizing after VM initialization. Hence, VGIC is pre-sized with
possible vCPUs.
>
> After the initialization of the machine is complete, the disabled
> possible KVM vCPUs are parked in the per-virt-machine list
> "kvm_parked_vcpus," and we release the QOM ARMCPU objects for the
> disabled vCPUs. These will be re-created when the vCPU is hotplugged
> again. The QOM ARMCPU object is then re-attached to the corresponding
parked KVM vCPU.
>
> Alternatively, we could have chosen not to release the QOM CPU objects
> and kept reusing them. This approach might require some modifications
> to the `qdevice_add()` interface to retrieve the old ARMCPU object
> instead of creating a new one for the hotplug request.
>
> Each of these approaches has its own pros and cons. This prototype
> uses the first approach (suggestions are welcome!).
>
> Co-developed-by: Keqian Zhu <zhukeqi...@huawei.com>
> Signed-off-by: Keqian Zhu <zhukeqi...@huawei.com>
> Signed-off-by: Salil Mehta <salil.me...@huawei.com>
> ---
> hw/arm/virt.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c index
> 9d33f30a6a..a72cd3b20d 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2050,6 +2050,7 @@ static void virt_cpu_post_init(VirtMachineState
*vms, MemoryRegion *sysmem)
> {
> CPUArchIdList *possible_cpus = vms->parent.possible_cpus;
> int max_cpus = MACHINE(vms)->smp.max_cpus;
> + MachineState *ms = MACHINE(vms);
> bool aarch64, steal_time;
> CPUState *cpu;
> int n;
> @@ -2111,6 +2112,37 @@ static void virt_cpu_post_init(VirtMachineState
*vms, MemoryRegion *sysmem)
> }
> }
> }
> +
> + if (kvm_enabled() || tcg_enabled()) {
> + for (n = 0; n < possible_cpus->len; n++) {
> + cpu = qemu_get_possible_cpu(n);
> +
> + /*
> + * Now, GIC has been sized with possible CPUs and we dont
require
> + * disabled vCPU objects to be represented in the QOM. Release
the
> + * disabled ARMCPU objects earlier used during init for
pre-sizing.
> + *
> + * We fake to the guest through ACPI about the
presence(_STA.PRES=1)
> + * of these non-existent vCPUs at VMM/qemu and present these
as
> + * disabled vCPUs(_STA.ENA=0) so that they cant be used. These
vCPUs
> + * can be later added to the guest through hotplug exchanges
when
> + * ARMCPU objects are created back again using 'device_add' QMP
> + * command.
> + */
> + /*
> + * RFC: Question: Other approach could've been to keep them
forever
> + * and release it only once when qemu exits as part of
finalize or
> + * when new vCPU is hotplugged. In the later old could be
released
> + * for the newly created object for the same vCPU?
> + */
> + if (!qemu_enabled_cpu(cpu)) {
> + CPUArchId *cpu_slot;
> + cpu_slot = virt_find_cpu_slot(ms, cpu->cpu_index);
> + cpu_slot->cpu = NULL;
> + object_unref(OBJECT(cpu));
> + }
> + }
> + }
> }
It's probably hard to keep those ARMCPU objects forever. First of all, one
vCPU can be hot-added first and then hot-removed afterwards. With those
ARMCPU objects kept forever, the syntax of 'device_add' and 'device_del'
become broken at least.
I had prototyped both approaches 4 years back. Yes, interface problem with
device_add was solved by a trick of keeping the old vCPU object and on
device_add instead of creating a new vCPU object we could use the old vCPU
object and then call qdev_realize() on it.
But bigger problem with this approach is that of migration. Only realized
objects
have state to be migrated. So it might look cleaner on one aspect but had its
own issues.
I think I did share a prototype of this with Igor which he was not in agreement
with
and wanted vCPU objects to be destroyed like in x86. Hence, we stuck with
the current approach.
Migration needn't to be the blocker necessarily. For those states of vCPUs,
which
have been instantiated and not realized yet, those states can be moved around to
be managed under another migratable object (e.g. MachineState). In this way,
those
vCPU states can be migrated together with MachineState. However, it sounds
strange
to me to have vCPU objects instantiated, but unrealized until hot-add is
triggered.
It's not what QOM supposes to support.
Ok, I don't realize x86 also follows this model: instantiate hotpluggable vCPUs
and destroy them on bootup.
The ideal mechanism would be to avoid instanciating those ARMCPU objects
and destroying them soon. I don't know if ms->possible_cpus->cpus[] can
fit and how much efforts needed.
This is what we are doing now in the current approach. Please read the KVMForum
slides of 2023 for more details and the cover letter of RFC V3 for more details.
My question has been parsed in a wrong way. My question was if it's doable to
avoid
instantiating vCPU#1 on bootup when we have command lines '-cpu host -smp
cpus=1,max_cpus=2'
and release it in machvirt_init(). Instead, ms->possible_cpus->cpus[] are
reused to create
GICv3 and vCPU file descriptors. It looks like a clean way at least. There may
be significant
efforts to reuse mc->possible_cpus->cpus[], but vCPU#1 object has ephemeral
life cycle.
It looks unnecessary to create the ephemeral vCPU#1 object.
Thanks,
Gavin
