RE: [PATCH RFC V3 06/29] arm/virt,kvm: Pre-create disabled possible vCPUs @machine init

[Salil Mehta]

Hi Gavin,

>  From: Gavin Shan <gshan@redhat.com>
>  Sent: Monday, August 19, 2024 6:32 AM
>  To: Salil Mehta <salil.mehta@huawei.com>; qemu-devel@nongnu.org;
>  qemu-arm@nongnu.org; mst@redhat.com
>  
>  On 6/14/24 9:36 AM, Salil Mehta wrote:
>  > In the ARMv8 architecture, the GIC must know all the CPUs it is
>  > connected to during its initialization, and this cannot change
>  > afterward. This must be ensured during the initialization of the VGIC
>  > as well in KVM, which requires all vCPUs to be created and present
>  > during its initialization. This is necessary
>  > because:
>  >
>  > 1. The association between GICC and MPIDR must be fixed at VM
>  initialization
>  >     time. This is represented by the register `GIC_TYPER(mp_affinity,
>  proc_num)`.
>  > 2. GICC (CPU interfaces), GICR (redistributors), etc., must all be 
> initialized
>  >     at boot time.
>  > 3. Memory regions associated with GICR, etc., cannot be changed (added, 
> deleted,
>  >     or modified) after the VM has been initialized.
>  >
>  > This patch adds support to pre-create all possible vCPUs within the
>  > host using the KVM interface as part of the virtual machine
>  > initialization. These vCPUs can later be attached to QOM/ACPI when
>  > they are actually hot-plugged and made present.
>  >
>  > Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
>  > Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
>  > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
>  > Reported-by: Vishnu Pajjuri <vishnu@os.amperecomputing.com>
>  > [VP: Identified CPU stall issue & suggested probable fix]
>  > ---
>  >   hw/arm/virt.c         | 56 +++++++++++++++++++++++++++++++++++-----
>  ---
>  >   include/hw/core/cpu.h |  1 +
>  >   target/arm/cpu64.c    |  1 +
>  >   target/arm/kvm.c      | 41 ++++++++++++++++++++++++++++++-
>  >   target/arm/kvm_arm.h  | 11 +++++++++
>  >   5 files changed, 99 insertions(+), 11 deletions(-)
>  >
>  
>  The vCPU file descriptor is associated with a feature bitmap when the file
>  descriptor is initialized by ioctl(vm_fd, KVM_ARM_VCPU_INIT, &init). The
>  feature bitmap is sorted out based on the vCPU properties. The vCPU
>  properties can be different when the vCPU file descriptor is initialized for
>  the first time when the vCPU is instantiated, and re-initialized when the
>  vCPU is hot added.

  
>  It can lead to system crash as below. We probably need a mechanism to
>  disallow passing extra properties when vCPU is hot added to avoid the
>  conflicts to the global properties from the command line "-cpu
>  host,pmu=on". Some of the properties like "id", "socket-id"
>  are still needed.


Yes, Good catch. I knew that but It almost went under my hood. Thanks for
pointing and reminding it. We need a check there. Will fix it.


>  
>  /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64                  \
>  -accel kvm -machine virt,gic-version=host,nvdimm=on                      \
>  -cpu host -smp maxcpus=2,cpus=1,sockets=2,clusters=1,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-0                                 \
>  -numa node,nodeid=1,memdev=mem1,cpus=1-1                                 \
>  -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                               \
>       :
>  (qemu) device_add host-arm-cpu,id=cpu1,socket-id=1,pmu=off
>  kvm_arch_init_vcpu: Error -22 from kvm_arm_vcpu_init()
>  qemu-system-aarch64: kvm_init_vcpu: kvm_arch_init_vcpu failed (1):
>  Invalid argument

Yes. thanks.

>  
>  Thanks,
>  Gavin
>