Hi Gavin,
> 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.
> 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.
Thanks
Salil.
>
> Thanks,
> Gavin
