Re: [PATCH RFC V6 05/24] arm/virt,kvm: Pre-create KVM vCPUs for 'disabled' QOM vCPUs at machine init

[Salil Mehta <salil.mehta@opnsrc.net>]

HI Gavin,

On Thu, Oct 23, 2025 at 12:14 AM Gavin Shan <gshan@redhat.com> wrote:
>
> Hi Salil,
>
> On 10/23/25 4:50 AM, Salil Mehta wrote:
> > On Wed, Oct 22, 2025 at 6:18 PM Salil Mehta <salil.mehta@opnsrc.net> wrote:
> >> On Wed, Oct 22, 2025 at 10:37 AM Gavin Shan <gshan@redhat.com> wrote:
> >>>
> >>> Hi Salil,
> >>>
> >>> On 10/1/25 11:01 AM, salil.mehta@opnsrc.net wrote:
> >>>> From: Salil Mehta <salil.mehta@huawei.com>
> >>>>
> >>>> ARM CPU architecture does not allow CPUs to be plugged after system has
> >>>> initialized. This is a constraint. Hence, the Kernel must know all the CPUs
> >>>> being booted during its initialization. This applies to the Guest Kernel as
> >>>> well and therefore, the number of KVM vCPU descriptors in the host must be
> >>>> fixed at VM initialization time.
> >>>>
> >>>> Also, the GIC must know all the CPUs it is connected to during its
> >>>> initialization, and this cannot change afterward. This must also be ensured
> >>>> during the initialization of the VGIC in KVM. This is necessary because:
> >>>>
> >>>> 1. The association between GICR and MPIDR must be fixed at VM initialization
> >>>>      time. This is represented by the register
> >>>>      `GICR_TYPER(mp_affinity, proc_num)`.
> >>>> 2. Memory regions associated with GICR, etc., cannot be changed (added,
> >>>>      deleted, or modified) after the VM has been initialized. This is not an
> >>>>      ARM architectural constraint but rather invites a difficult and messy
> >>>>      change in VGIC data structures.
> >>>>
> >>>> To enable a hot-add–like model while preserving these constraints, the virt
> >>>> machine may enumerate more CPUs than are enabled at boot using
> >>>> `-smp disabledcpus=N`. Such CPUs are present but start offline (i.e.,
> >>>> administratively disabled at init). The topology remains fixed at VM
> >>>> creation time; only the online/offline status may change later.
> >>>>
> >>>> Administratively disabled vCPUs are not realized in QOM until first enabled,
> >>>> avoiding creation of unnecessary vCPU threads at boot. On large systems, this
> >>>> reduces startup time proportionally to the number of disabled vCPUs. Once a
> >>>> QOM vCPU is realized and its thread created, subsequent enable/disable actions
> >>>> do not unrealize it. This behaviour was adopted following review feedback and
> >>>> differs from earlier RFC versions.
> >>>>
> >>>> Co-developed-by: Keqian Zhu <zhuqian1@huawei.com>
> >>>> Signed-off-by: Keqian Zhu <zhuqian1@huawei.com>
> >>>> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> >>>> ---
> >>>>    accel/kvm/kvm-all.c    |  2 +-
> >>>>    hw/arm/virt.c          | 77 ++++++++++++++++++++++++++++++++++++++----
> >>>>    hw/core/qdev.c         | 17 ++++++++++
> >>>>    include/hw/qdev-core.h | 19 +++++++++++
> >>>>    include/system/kvm.h   |  8 +++++
> >>>>    target/arm/cpu.c       |  2 ++
> >>>>    target/arm/kvm.c       | 40 +++++++++++++++++++++-
> >>>>    target/arm/kvm_arm.h   | 11 ++++++
> >>>>    8 files changed, 168 insertions(+), 8 deletions(-)
> >>>>
> >
> > [...]
> >
> >>>> +void kvm_arm_create_host_vcpu(ARMCPU *cpu)
> >>>> +{
> >>>> +    CPUState *cs = CPU(cpu);
> >>>> +    unsigned long vcpu_id = cs->cpu_index;
> >>>> +    int ret;
> >>>> +
> >>>> +    ret = kvm_create_vcpu(cs);
> >>>> +    if (ret < 0) {
> >>>> +        error_report("Failed to create host vcpu %ld", vcpu_id);
> >>>> +        abort();
> >>>> +    }
> >>>> +
> >>>> +    /*
> >>>> +     * Initialize the vCPU in the host. This will reset the sys regs
> >>>> +     * for this vCPU and related registers like MPIDR_EL1 etc. also
> >>>> +     * get programmed during this call to host. These are referenced
> >>>> +     * later while setting device attributes of the GICR during GICv3
> >>>> +     * reset.
> >>>> +     */
> >>>> +    ret = kvm_arch_init_vcpu(cs);
> >>>> +    if (ret < 0) {
> >>>> +        error_report("Failed to initialize host vcpu %ld", vcpu_id);
> >>>> +        abort();
> >>>> +    }
> >>>> +
> >>>> +    /*
> >>>> +     * park the created vCPU. shall be used during kvm_get_vcpu() when
> >>>> +     * threads are created during realization of ARM vCPUs.
> >>>> +     */
> >>>> +    kvm_park_vcpu(cs);
> >>>> +}
> >>>> +
> >>>
> >>> I don't think we're able to simply call kvm_arch_init_vcpu() in the lazily realized
> >>> path. Otherwise, it can trigger a crash dump on my Nvidia's grace-hopper machine where
> >>> SVE is supported by default.
> >>
> >> Thanks for reporting this. That is not true. As long as we initialize
> >> KVM correctly and
> >> finalize the features like SVE we should be fine. In fact, this is
> >> precisely what we are
> >> doing right now.
> >>
> >> To understand the crash, I need a bit more info.
> >>
> >> 1#  is happening because KVM_ARM_VCPU_INIT is failing. If yes, the can you check
> >>        within the KVM if it is happening because
> >>       a.  features specified by QEMU are not matching the defaults within the KVM
> >>             (HInt: check kvm_vcpu_init_check_features())?
> >>       b. or complaining about init feate change kvm_vcpu_init_changed()?
> >> 2#  or it is happening during the setting of vector length or
> >> finalizing features?
> >>
> >> int kvm_arch_init_vcpu(CPUState *cs)
> >> {
> >>     [...]
> >>           /* Do KVM_ARM_VCPU_INIT ioctl */
> >>          ret = kvm_arm_vcpu_init(cpu);   ---->[1]
> >>          if (ret) {
> >>             return ret;
> >>         }
> >>            if (cpu_isar_feature(aa64_sve, cpu)) {
> >>          ret = kvm_arm_sve_set_vls(cpu); ---->[2]
> >>          if (ret) {
> >>              return ret;
> >>          }
> >>          ret = kvm_arm_vcpu_finalize(cpu, KVM_ARM_VCPU_SVE);--->[3]
> >>          if (ret) {
> >>              return ret;
> >>          }
> >>      }
> >> [...]
> >> }
> >>
> >> I think it's happening because vector length is going uninitialized.
> >> This initialization
> >> happens in context to  arm_cpu_finalize_features() which I forgot to call before
> >> calling KVM finalize.
> >>
> >>>
> >>> kvm_arch_init_vcpu() is supposed to be called in the realization path in current
> >>> implementation (without this series) because the parameters (features) to KVM_ARM_VCPU_INIT
> >>> is populated at vCPU realization time.
> >>
> >> Not necessarily. It is just meant to initialize the KVM. If we take care of the
> >> KVM requirements in the similar way the realize path does we should be
> >> fine. Can you try to add the patch below in your code and test if it works?
> >>
> >>   diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> >> index c4b68a0b17..1091593478 100644
> >> --- a/target/arm/kvm.c
> >> +++ b/target/arm/kvm.c
> >> @@ -1068,6 +1068,9 @@ void kvm_arm_create_host_vcpu(ARMCPU *cpu)
> >>           abort();
> >>       }
> >>
> >> +     /* finalize the features like SVE, SME etc */
> >> +     arm_cpu_finalize_features(cpu, &error_abort);
> >> +
> >>       /*
> >>        * Initialize the vCPU in the host. This will reset the sys regs
> >>        * for this vCPU and related registers like MPIDR_EL1 etc. also
> >>
> >>
> >>
> >>
> >>>
> >>> $ home/gavin/sandbox/qemu.main/build/qemu-system-aarch64           \
> >>>     --enable-kvm -machine virt,gic-version=3 -cpu host               \
> >>>     -smp cpus=4,disabledcpus=2 -m 1024M                              \
> >>>     -kernel /home/gavin/sandbox/linux.guest/arch/arm64/boot/Image    \
> >>>     -initrd /home/gavin/sandbox/images/rootfs.cpio.xz -nographic
> >>> qemu-system-aarch64: Failed to initialize host vcpu 4
> >>> Aborted (core dumped)
> >>>
> >>> Backtrace
> >>> =========
> >>> (gdb) bt
> >>> #0  0x0000ffff9106bc80 in __pthread_kill_implementation () at /lib64/libc.so.6
> >>> #1  0x0000ffff9101aa40 [PAC] in raise () at /lib64/libc.so.6
> >>> #2  0x0000ffff91005988 [PAC] in abort () at /lib64/libc.so.6
> >>> #3  0x0000aaaab1cc26b8 [PAC] in kvm_arm_create_host_vcpu (cpu=0xaaaab9ab1bc0)
> >>>       at ../target/arm/kvm.c:1081
> >>> #4  0x0000aaaab1cd0c94 in virt_setup_lazy_vcpu_realization (cpuobj=0xaaaab9ab1bc0, vms=0xaaaab98870a0)
> >>>       at ../hw/arm/virt.c:2483
> >>> #5  0x0000aaaab1cd180c in machvirt_init (machine=0xaaaab98870a0) at ../hw/arm/virt.c:2777
> >>> #6  0x0000aaaab160f220 in machine_run_board_init
> >>>       (machine=0xaaaab98870a0, mem_path=0x0, errp=0xfffffa86bdc8) at ../hw/core/machine.c:1722
> >>> #7  0x0000aaaab1a25ef4 in qemu_init_board () at ../system/vl.c:2723
> >>> #8  0x0000aaaab1a2635c in qmp_x_exit_preconfig (errp=0xaaaab38a50f0 <error_fatal>)
> >>>       at ../system/vl.c:2821
> >>> #9  0x0000aaaab1a28b08 in qemu_init (argc=15, argv=0xfffffa86c1f8) at ../system/vl.c:3882
> >>> #10 0x0000aaaab221d9e4 in main (argc=15, argv=0xfffffa86c1f8) at ../system/main.c:71
> >>
> >>
> >> Thank you for this. Please let me know if the above fix works and also
> >> the return values in
> >> case you encounter errors.
> >
> > I've pushed the fix to below branch for your convenience:
> >
> > Branch: https://github.com/salil-mehta/qemu/commits/virt-cpuhp-armv8/rfc-v6.2
> > Fix: https://github.com/salil-mehta/qemu/commit/1f1fbc0998ffb1fe26140df3c336bf2be2aa8669
> >
>
> I guess rfc-v6.2 branch isn't ready for test because it runs into another crash
> dump with rfc-v6.2 branch, like below.


rfc-6.2 is not crashing on Kunpeng920 where I tested. But this
chip does not have some ARM extensions like SVE etc so
Unfortunately, I can't test SVE/SME/PAuth etc support.

Can you disable SVE and then try if it comes up just to corner
the case?

>
> host$ /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64                     \
>        -accel kvm -machine virt,gic-version=host,nvdimm=on                         \
>        -cpu host,sve=on                                                            \
>        -smp maxcpus=4,cpus=2,disabledcpus=2,sockets=2,clusters=2,cores=1,threads=1 \
>        -m 4096M,slots=16,maxmem=128G                                               \
>        -object memory-backend-ram,id=mem0,size=2048M                               \
>        -object memory-backend-ram,id=mem1,size=2048M                               \
>        -numa node,nodeid=0,memdev=mem0,cpus=0-1                                    \
>        -numa node,nodeid=1,memdev=mem1,cpus=2-3                                    \
>        -L /home/gavin/sandbox/qemu.main/build/pc-bios                              \
>        -monitor none -serial mon:stdio -nographic -gdb tcp::6666                   \
>        -qmp tcp:localhost:5555,server,wait=off                                     \
>        -bios /home/gavin/sandbox/qemu.main/build/pc-bios/edk2-aarch64-code.fd      \
>        -kernel /home/gavin/sandbox/linux.guest/arch/arm64/boot/Image               \
>        -initrd /home/gavin/sandbox/images/rootfs.cpio.xz                           \
>        -append memhp_default_state=online_movable
>          :
>          :
> guest$ cd /sys/devices/system/cpu/
> guest$ cat present enabled online
> 0-3
> 0-1
> 0-1
> (qemu) device_set host-arm-cpu,socket-id=1,cluster-id=0,core-id=0,thread-id=0,admin-state=enable
> qemu-system-aarch64: kvm_init_vcpu: kvm_arch_init_vcpu failed (2): Operation not permitted


Ah, I see. I think I understand the issue. It's complaining
about calling the  finalize twice. Is it possible to check as
I do not have a way to test it?


int kvm_arm_vcpu_finalize(struct kvm_vcpu *vcpu, int feature)
{
switch (feature) {
case KVM_ARM_VCPU_SVE:
[...]
if (kvm_arm_vcpu_sve_finalized(vcpu))
return -EPERM;-----> this where it must be popping?
[...]
}


>
> I picked the fix (the last patch in rfc-v6.2 branch) to rfc-v6 branch, same crash dump
> can be seen.

Are you getting previously reported abort or above new problem?


Thanks
Salil.