* [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch
@ 2020-06-13 21:36 Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 01/22] arm/cpuhp: Add QMP vcpu params validation support Salil Mehta
` (25 more replies)
0 siblings, 26 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
This patch-set introduces the virtual cpu hotplug support for ARMv8
architecture in QEMU. Idea is to be able to hotplug and hot-unplug the vcpus
while guest VM is running and no reboot is required. This does *not* makes any
assumption of the physical cpu hotplug availability within the host system but
rather tries to solve the problem at virtualizer/QEMU layer and by introducing
cpu hotplug hooks and event handling within the guest kernel. No changes are
required within the host kernel/KVM.
Motivation:
This allows scaling the guest VM compute capacity on-demand which would be
useful for the following example scenarios,
1. Vertical Pod Autoscaling[3][4] in the cloud: Part of the orchestration
framework which could adjust resource requests (CPU and Mem requests) for
the containers in a pod, based on usage.
2. Pay-as-you-grow Business Model: Infrastructure provider could allocate and
restrict the total number of compute resources available to the guest VM
according to the SLA(Service Level Agreement). VM owner could request for
more compute to be hot-plugged for some cost.
Terminology:
(*) Present cpus: Total cpus with which guest has/will boot and are available
to guest for use and can be onlined. Qemu parameter(-smp)
(*) Disabled cpus: Possible cpus which will not be available for the guest to
use. These can be hotplugged and made present. These can be
thought of as un-plugged vcpus. These will be included as
part of sizing.
(*) Posssible cpus: Total vcpus which could ever exist in VM. This includes
booted cpus plus any cpus which could be later plugged.
- Qemu parameter(-maxcpus)
- Possible vcpus = Present vcpus (+) Disabled vcpus
Limitations of ARMv8 Architecture:
A. Physical Limitation to CPU Hotplug:
1. ARMv8 architecture does not support the concept of the physical cpu hotplug.
The closest thing which is recomended to achieve the cpu hotplug on ARM is
to bring down power state of the cpu using PSCI.
2. Other ARM components like GIC etc. have not been designed to realize
physical cpu hotplug capability as of now.
B. Limitations of GIC to Support Virtual CPU Hotplug:
1. GIC requires various resources(related to GICR/redistributor, GICC/cpu
interface etc) like memory regions to be fixed at the VM init time and these
could not be changed later on after VM has inited.
2. Associations between GICC(GIC cpu interface) and vcpu get fixed at the VM
init time and GIC does not allows to change this association once GIC has
initialized.
C. Known Limitation of the KVM:
1. As of now KVM allows to create VCPUs but does not allows to delete the
already created vcpus. QEMU already provides an interface to manage created
vcpus at KVM level and then to re-use them.
2. Inconsistency in interpretation of the MPIDR generated by KVM for vcpus
vis-a-vis SMT/threads. This does not looks to be compliant to the MPIDR
format(SMT is present) as mentioned in the ARMv8 spec. (Please correct my
understanding if I am wrong here?)
Workaround to the problems mentioned in Section B & C1:
1. We pre-size the GIC with possible vcpus at VM init time
2. Pre-create all possible vcpus at KVM and associate them with GICC
3. Park the unplugged vcpus (similar to x86)
(*) For all of above please refer to Marc's suggestion here[1]
Overview of the Approach:
At the time of machvirt_init() we pre-create all of the possible ARMCPU
objects along with the corresponding KVM vcpus at the host. Disabled KVM vcpu
(which are *not* "present" vcpus but are part of "possible" vcpu list) are
parked at per VM list "kvm_parked_vcpus" after their initialization.
We create the ARMCPU objects(but these are not *realized* in QOM sense) even
for the disabled vcpus to facilitate the GIC initialization (pre-sized with
possible vcpus). After Initialization of the machine is complete we release
the ARMCPU Objects for the disabled vcpus. These ARMCPU object shall be
re-created at the time when vcpu is hot plugged. This new object is then
re-attached with the earlier parked KVM vcpu which also gets unparked. The
ARMCPU object gets now "realized" in QEMU, which means creation of the
corresponding threads, pre_plug/plug phases, and event notification to the
guest using ACPI GED etc. Similarly, hot-unplug leg will lead to the
"unrealization" of the vcpus and will lead to similar ACPI GED events to the
guest for unplug and cleanup and eventually ARMCPU object shall be released and
KVM vcpus shall be parked again.
During machine init, ACPI MADT Table is sized with *possible* vcpus GICC
entries. The unplugged/disabled vcpus are presented as MADT GICC DISABLED
entries to the guest. This means the guest will have its resources pre-sized
with possible vcpus(=present+disabled)
Other approaches to deal with ARMCPU object release(after machine init):
1. The ARMCPU objects for the disabled vcpus are released in context to the
virt_machine_done() notifier(approach adopted in this patch-set).
2. Defer the release of current ARMCPU object till the new vcpu object is
hot plugged.
3. Never release and keep on reusing them and release once at VM exit. This
solves many problems with above 2 approaches but requires change in the way
qdev_device_add() fetches/creates the ARMCPU object for the new vcpus being
hotplugged. For the arm cpu hotplug case we need to figure out way how to
get access to old object and use it to "re-realize" instead of the new
ARMCPU object.
Concerns/Questions:
1. In ARM arch a cpu is uniquely represented in hierarchy using various
affinity levels which could represent thread, core, cluster, package. This
is generally represented by a value in MPIDR register as per the format
mentioned in specification. Now, the way MPIDR value is derived for vcpus is
done using vcpu-index. The concept of thread is not quite as same and rather
gets lost in the derivation of MPIDR for vcpus.
2. The topology info used to specify the vcpu while hot-plugging might not
match with the MPIDR value given back by the KVM for the vcpu at the time of
init. Concept of SMT bit in MPIDR gets lost as per the derivation being done
in the KVM. Hence, concept of thread-id, core-id, socket-id if used as a
topology info to derive MPIDR value as per ARM specification will not match
with MPIDR actually assigned by the KVM?
Perhaps need to carry forward work of Andrew? please check here[2]
3. Further if this info is supplied to the guest using PPTT(once introduced in
QEMU) or even derived using MPIDR shall be inconsistent with the host vcpu.
4. Any possibilities of interrupts(SGI/PPI/LPI/SPI) always remaining in
*pending* state for the cpus which have been hot-unplugged? IMHO it looks
okay but will need Marc's confirmation on this.
5. If the ARMCPU object is released after the machine init, UEFI could call
back virt_update_table() to re-build the ACPI tables which might need an
ARMCPU object. Please check the discussion here[5]
Commands Used:
A. Qemu launch commands to init the machine
$ qemu-system-aarch64 --enable-kvm -machine virt,gic-version=3 \
-cpu host -smp cpus=4,maxcpus=6 \
-m 300M \
-kernel Image \
-initrd rootfs.cpio.gz \
-append "console=ttyAMA0 root=/dev/ram rdinit=/init maxcpus=2 acpi=force" \
-nographic \
-bios QEMU_EFI.fd \
B. Hot-(un)plug related commands
# Hotplug a host vcpu(accel=kvm)
$ device_add host-arm-cpu,id=core4,core-id=4
# Hotplug a vcpu(accel=tcg)
$ device_add cortex-a57-arm-cpu,id=core4,core-id=4
# Delete the vcpu
$ device_del core4
NOTE: I have not tested the current solution with '-device' interface. The use
is suggested by Igor here[6]. I will test this in coming times but looks
it should work with existing changes.
Sample output on guest after boot:
$ cat /sys/devices/system/cpu/possible
0-5
$ cat /sys/devices/system/cpu/present
0-3
$ cat /sys/devices/system/cpu/online
0-1
$ cat /sys/devices/system/cpu/offline
2-5
Sample output on guest after hotplug of vcpu=4:
$ cat /sys/devices/system/cpu/possible
0-5
$ cat /sys/devices/system/cpu/present
0-4
$ cat /sys/devices/system/cpu/online
0-1,4
$ cat /sys/devices/system/cpu/offline
2-3,5
Note: vcpu=4 was explicitly 'onlined' after hot-plug
$ echo 1 > /sys/devices/system/cpu/cpu4/online
Repository:
(*) QEMU changes for vcpu hotplug could be cloned from below site,
https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v1
(*) Guest Kernel changes required to co-work with the QEMU shall be posted soon
and repo made available at above site.
THINGS TO DO:
(*) Migration support
(*) TCG/Emulation support is not proper right now. Works to a certain extent
but is not complete. especially the unrealize part in which there is a
overflow of tcg contexts. The last is due to the fact tcg maintains a
count on number of context(per thread instance) so as we hotplug the vcpus
this counter keeps on incrementing. But during hot-unplug the counter is
not decremented.
(*) Support of hotplug with NUMA is not proper
(*) CPU Topology right now is not specified using thread/core/socket but
rather flatly indexed using core-id. This needs consideration[2].
(*) Do we need PPTT Support for to specify right topology info to guest about
hot-plugged or unplugged vcpus?
(*) Test cases
(*) Docs need to be updated.
DISCLAIMER:
This is not a complete work but an effort to present the arm vcpu hotplug
implementation to the community. This work is *mostly* in the lines of the
discussions which have happened in the previous years[see refs below]. This
RFC is being used as a way to verify the idea mentioned above and to further
get community views. As of now this is *not* a production level code and might
have bugs. Only a basic testing has been done on HiSilicon Kunpeng920 SoC for
Servers to verify the proof-of-concept. But the concept works!
ACKNOWLEDGEMENTS:
Although this is just a start of work but I would like to take this opportunity
to thank Marc Zyngier, Igor mammedov, James Morse, Andre, Sudeep Holla, Andrew
Jones, Jonathan Cameron, Shameer, Xuwei/Joy, Zengtao/Prime for discussions at
various points with me. Special thanks to Zhukeqian & Wangxiongfeng for their
contributions in the QEMU and the kernel part. And to the other people of
community who have been involved in the discussions in the past related to this
work and have pitched-in their ideas.
Best regards
Salil.
REFERENCES:
[1] https://lists.cs.columbia.edu/pipermail/kvmarm/2018-July/032316.html
[2] http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg01168.html
[3] https://cloud.google.com/kubernetes-engine/docs/concepts/verticalpodautoscaler
[4] https://docs.aws.amazon.com/eks/latest/userguide/vertical-pod-autoscaler.html
[5] https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg00131.html
[6] https://lkml.org/lkml/2019/7/10/235
[7] https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06517.html
Organization of Patches:
[Patch 1-3] Misc logic pre-cursor to machine init
(*) some validation checks
(*) introduces core-id property and some util functions required later.
[Patch 4-7,14] logic required during machine init
(*) Logic to pre-create vcpus
(*) GIC initialization pre-sized with possible vcpus.
(*) some refactoring to have common hot and cold plug logic together.
[Patch 8-13] logic related to ACPI at machine init time
(*) Changes required to Enable ACPI for cpu hotplug
(*) Changes to enhance ACPI GED to cater CPU Hotplug Events
(*) ACPI MADT/MAT changes
[Patch 15-22] Logic required during vcpu hot-(un)plug
(*) Basic framework changes to suppport vcpu hot-(un)plug
(*) ACPI GED changes for hot-(un)plug hooks.
(*) wire-unwire the IRQs
(*) GIC notification logic
(*) ARMCPU unrealize logic
Salil Mehta (22):
arm/cpuhp: Add QMP vcpu params validation support
arm/cpuhp: Add new ARMCPU core-id property
arm/cpuhp: Add common cpu utility for possible vcpus
arm/cpuhp: Machine init time change common to vcpu {cold|hot}-plug
arm/cpuhp: Pre-create disabled possible vcpus @machine init
arm/cpuhp: Changes to pre-size GIC with possible vcpus @machine init
arm/cpuhp: Init PMU at host for all possible vcpus
arm/cpuhp: Enable ACPI support for vcpu hotplug
arm/cpuhp: Init GED framework with cpu hotplug events
arm/cpuhp: Update CPUs AML with cpu-(ctrl)dev change
arm/cpuhp: Update GED _EVT method AML with cpu scan
arm/cpuhp: MADT Tbl change to size the guest with possible vcpus
arm/cpuhp: Add ACPI _MAT entry for Processor object
arm/cpuhp: Release objects for *disabled* possible vcpus after init
arm/cpuhp: Update ACPI GED framework to support vcpu hotplug
arm/cpuhp: Add/update basic hot-(un)plug framework
arm/cpuhp: Changes to (un)wire GICC<->VCPU IRQs during hot-(un)plug
arm/cpuhp: Changes to update GIC with vcpu hot-plug notification
arm/cpuhp: Changes required to (re)init the vcpu register info
arm/cpuhp: Update the guest(via GED) about cpu hot-(un)plug events
arm/cpuhp: Changes required for reset and to support next boot
arm/cpuhp: Add support of *unrealize* ARMCPU during vcpu hot-unplug
accel/kvm/kvm-all.c | 31 ++
cpus-common.c | 20 +
exec.c | 24 +
gdbstub.c | 13 +
hw/acpi/cpu.c | 34 +-
hw/acpi/generic_event_device.c | 54 ++
hw/arm/Kconfig | 1 +
hw/arm/boot.c | 2 +-
hw/arm/virt-acpi-build.c | 51 +-
hw/arm/virt.c | 717 +++++++++++++++++++++----
hw/core/qdev.c | 2 +-
hw/i386/acpi-build.c | 2 +-
hw/intc/arm_gicv3.c | 1 +
hw/intc/arm_gicv3_common.c | 66 ++-
hw/intc/arm_gicv3_cpuif.c | 145 ++---
hw/intc/arm_gicv3_kvm.c | 34 +-
hw/intc/gicv3_internal.h | 2 +
include/exec/exec-all.h | 8 +
include/exec/gdbstub.h | 1 +
include/hw/acpi/cpu.h | 5 +-
include/hw/acpi/cpu_hotplug.h | 5 +
include/hw/acpi/generic_event_device.h | 5 +
include/hw/arm/boot.h | 2 +
include/hw/arm/virt.h | 9 +-
include/hw/core/cpu.h | 23 +
include/hw/intc/arm_gicv3_common.h | 2 +
include/hw/qdev-core.h | 2 +
include/sysemu/kvm.h | 2 +
target/arm/cpu-qom.h | 3 +
target/arm/cpu.c | 98 ++++
target/arm/cpu.h | 14 +
target/arm/cpu64.c | 9 +
target/arm/helper.c | 31 ++
target/arm/internals.h | 1 +
target/arm/kvm.c | 32 ++
target/arm/kvm64.c | 7 +-
target/arm/kvm_arm.h | 11 +
37 files changed, 1260 insertions(+), 209 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH RFC 01/22] arm/cpuhp: Add QMP vcpu params validation support
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-23 8:46 ` Andrew Jones
2020-06-13 21:36 ` [PATCH RFC 02/22] arm/cpuhp: Add new ARMCPU core-id property Salil Mehta
` (24 subsequent siblings)
25 siblings, 1 reply; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
For now, vcpu hotplug is only supported with single socket single thread,
single die. NUMA is not supported either and everthing falls into single
node. Work to properly support these could be taken later once community
agrees with the base framework changes being presented to support ARM vcpu
hotplug in QEMU. Hence, these checks.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/arm/virt.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 37462a6f78..5d1afdd031 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2201,6 +2201,46 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
return NULL;
}
+static void virt_smp_parse(MachineState *ms, QemuOpts *opts)
+{
+ if (opts) {
+ unsigned cpus = qemu_opt_get_number(opts, "cpus", 1);
+ unsigned sockets = qemu_opt_get_number(opts, "sockets", 1);
+ unsigned cores = qemu_opt_get_number(opts, "cores", cpus);
+ unsigned threads = qemu_opt_get_number(opts, "threads", 1);
+ unsigned int max_cpus;
+
+ if (sockets > 1 || threads > 1) {
+ error_report("does not support more than one socket or thread");
+ exit(1);
+ }
+
+ if (cores != cpus) {
+ error_report("cpu topology: "
+ "sockets (%u) * cores (%u) * threads (%u) < "
+ "smp_cpus (%u)",
+ sockets, cores, threads, cpus);
+ exit(1);
+ }
+
+ max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
+ if (sockets * cores * threads > max_cpus) {
+ error_report("cpu topology: "
+ "sockets (%u) * cores (%u) * threads (%u) > "
+ "maxcpus (%u)",
+ sockets, cores, threads,
+ max_cpus);
+ exit(1);
+ }
+
+ ms->smp.max_cpus = max_cpus;
+ ms->smp.sockets = sockets;
+ ms->smp.cpus = cpus;
+ ms->smp.cores = cores;
+ ms->smp.threads = threads;
+ }
+}
+
/*
* for arm64 kvm_type [7-0] encodes the requested number of bits
* in the IPA address space
@@ -2266,6 +2306,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
mc->nvdimm_supported = true;
mc->auto_enable_numa_with_memhp = true;
mc->default_ram_id = "mach-virt.ram";
+ mc->smp_parse = virt_smp_parse;
object_class_property_add(oc, "acpi", "OnOffAuto",
virt_get_acpi, virt_set_acpi,
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 02/22] arm/cpuhp: Add new ARMCPU core-id property
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 01/22] arm/cpuhp: Add QMP vcpu params validation support Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 03/22] arm/cpuhp: Add common cpu utility for possible vcpus Salil Mehta
` (23 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
This shall be used to store user specified core index and shall be directly
used as slot-index during hot{plug|unplug} of vcpu.
For now, we are not taking into account of other topology info like thread-id,
socket-id to derive mp-affinity. Host KVM uses vcpu-id to derive the mpidr for
the vcpu of the guest. This is not in exact corroboration with the ARM spec
view of the MPIDR. Hence, the concept of threads or SMT bit present as part of
the MPIDR_EL1 also gets lost.
Also, we need ACPI PPTT Table support in QEMU to be able to export this
topology info to the guest VM and the info should be consistent with what host
cpu supports if accel=kvm is being used.
Perhaps some comments on this will help? @Andrew/drjones@redhat.com
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/arm/virt.c | 5 +++++
target/arm/cpu.c | 5 +++++
target/arm/cpu.h | 1 +
3 files changed, 11 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 5d1afdd031..c4ed955776 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1778,6 +1778,7 @@ static void machvirt_init(MachineState *machine)
&error_fatal);
aarch64 &= object_property_get_bool(cpuobj, "aarch64", NULL);
+ object_property_set_int(cpuobj, n, "core-id", NULL);
if (!vms->secure) {
object_property_set_bool(cpuobj, false, "has_el3", NULL);
@@ -2081,6 +2082,7 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
{
int n;
unsigned int max_cpus = ms->smp.max_cpus;
+ unsigned int smp_threads = ms->smp.threads;
VirtMachineState *vms = VIRT_MACHINE(ms);
if (ms->possible_cpus) {
@@ -2093,8 +2095,11 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
ms->possible_cpus->len = max_cpus;
for (n = 0; n < ms->possible_cpus->len; n++) {
ms->possible_cpus->cpus[n].type = ms->cpu_type;
+ ms->possible_cpus->cpus[n].vcpus_count = smp_threads;
ms->possible_cpus->cpus[n].arch_id =
virt_cpu_mp_affinity(vms, n);
+ ms->possible_cpus->cpus[n].props.has_core_id = true;
+ ms->possible_cpus->cpus[n].props.core_id = n;
ms->possible_cpus->cpus[n].props.has_thread_id = true;
ms->possible_cpus->cpus[n].props.thread_id = n;
}
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 32bec156f2..33a58086a9 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1086,6 +1086,9 @@ static Property arm_cpu_has_dsp_property =
static Property arm_cpu_has_mpu_property =
DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
+static Property arm_cpu_coreid_property =
+ DEFINE_PROP_INT32("core-id", ARMCPU, core_id, -1);
+
/* This is like DEFINE_PROP_UINT32 but it doesn't set the default value,
* because the CPU initfn will have already set cpu->pmsav7_dregion to
* the right value for that particular CPU type, and we don't want
@@ -1168,6 +1171,8 @@ void arm_cpu_post_init(Object *obj)
qdev_property_add_static(DEVICE(obj), &arm_cpu_rvbar_property);
}
+ qdev_property_add_static(DEVICE(obj), &arm_cpu_coreid_property);
+
#ifndef CONFIG_USER_ONLY
if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) {
/* Add the has_el3 state CPU property only if EL3 is allowed. This will
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 677584e5da..5c4991156e 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -949,6 +949,7 @@ struct ARMCPU {
QLIST_HEAD(, ARMELChangeHook) el_change_hooks;
int32_t node_id; /* NUMA node this CPU belongs to */
+ int32_t core_id; /* core-id of this ARM VCPU */
/* Used to synchronize KVM and QEMU in-kernel device levels */
uint8_t device_irq_level;
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 03/22] arm/cpuhp: Add common cpu utility for possible vcpus
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 01/22] arm/cpuhp: Add QMP vcpu params validation support Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 02/22] arm/cpuhp: Add new ARMCPU core-id property Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 04/22] arm/cpuhp: Machine init time change common to vcpu {cold|hot}-plug Salil Mehta
` (22 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
Adds various utility functions which might be required to fetch or check the
state of the possible vcpus. This also introduces concept of *disabled* vcpus,
which are part of the *possible* vcpus but are not part of the *present* vcpu.
This state shall be used during machine init time to check the presence of
vcpus.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
cpus-common.c | 20 ++++++++++++++++++++
include/hw/core/cpu.h | 21 +++++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/cpus-common.c b/cpus-common.c
index 70a9d12981..7cf900289b 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -23,6 +23,7 @@
#include "hw/core/cpu.h"
#include "sysemu/cpus.h"
#include "qemu/lockable.h"
+#include "hw/boards.h"
static QemuMutex qemu_cpu_list_lock;
static QemuCond exclusive_cond;
@@ -82,6 +83,25 @@ void cpu_list_add(CPUState *cpu)
assert(!cpu_index_auto_assigned);
}
QTAILQ_INSERT_TAIL_RCU(&cpus, cpu, node);
+ qemu_mutex_unlock(&qemu_cpu_list_lock);
+}
+
+CPUState *qemu_get_possible_cpu(int index)
+{
+ MachineState *ms = MACHINE(qdev_get_machine());
+ const CPUArchIdList *possible_cpus = ms->possible_cpus;
+ CPUState *cpu;
+
+ assert((index >= 0) && (index < possible_cpus->len));
+
+ cpu = CPU(possible_cpus->cpus[index].cpu);
+
+ return cpu;
+}
+
+bool qemu_present_cpu(CPUState *cpu)
+{
+ return (cpu && !cpu->disabled);
}
void cpu_list_remove(CPUState *cpu)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 497600c49e..d9cae71ea5 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -419,6 +419,7 @@ struct CPUState {
GArray *plugin_mem_cbs;
+ bool disabled;
/* TODO Move common fields from CPUArchState here. */
int cpu_index;
int cluster_index;
@@ -802,6 +803,26 @@ static inline bool cpu_in_exclusive_context(const CPUState *cpu)
*/
CPUState *qemu_get_cpu(int index);
+/**
+ * qemu_get_possible_cpu:
+ * @index: The CPUState@cpu_index value of the CPU to obtain.
+ *
+ * Gets a CPU matching @index.
+ *
+ * Returns: The possible CPU or %NULL if there is no matching CPU.
+ */
+CPUState *qemu_get_possible_cpu(int index);
+
+/**
+ * qemu_present_cpu:
+ * @cpu: The vCPU to check
+ *
+ * Checks if the vcpu is amongst the present possible vcpus.
+ *
+ * Returns: True if it is present possible vcpu else false
+ */
+bool qemu_present_cpu(CPUState *cpu);
+
/**
* cpu_exists:
* @id: Guest-exposed CPU ID to lookup.
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 04/22] arm/cpuhp: Machine init time change common to vcpu {cold|hot}-plug
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (2 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 03/22] arm/cpuhp: Add common cpu utility for possible vcpus Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 05/22] arm/cpuhp: Pre-create disabled possible vcpus @machine init Salil Mehta
` (21 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
This refactors (+) introduces the common logic required during the
initialization of both cold and hot plugged vcpus. This also initializes the
*disabled* state of the vcpus which shall be used further during init phases
of various other components like GIC, PMU, ACPI etc as part of the virt machine
initialization.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/arm/virt.c | 250 +++++++++++++++++++++++++++++++++---------
include/hw/arm/virt.h | 2 +
target/arm/cpu.c | 7 ++
target/arm/cpu64.c | 9 ++
4 files changed, 214 insertions(+), 54 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index c4ed955776..184bed8716 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -203,6 +203,8 @@ static const char *valid_cpus[] = {
ARM_CPU_TYPE_NAME("max"),
};
+static CPUArchId *virt_find_cpu_slot(MachineState *ms, int vcpuid);
+
static bool cpu_type_valid(const char *cpu)
{
int i;
@@ -1657,6 +1659,62 @@ static void finalize_gic_version(VirtMachineState *vms)
}
}
+static void virt_cpu_set_properties(Object *cpuobj, const CPUArchId *cpu_slot)
+{
+ MachineState *ms = MACHINE(qdev_get_machine());
+ MemoryRegion *sysmem = get_system_memory();
+ VirtMachineState *vms = VIRT_MACHINE(ms);
+ uint64_t mp_affinity = cpu_slot->arch_id;
+ CPUState *cs = CPU(cpuobj);
+ VirtMachineClass *vmc;
+
+ vmc = VIRT_MACHINE_GET_CLASS(ms);
+
+ /* now, set the cpu object property values */
+ object_property_set_int(cpuobj, mp_affinity, "mp-affinity", NULL);
+
+ numa_cpu_pre_plug(cpu_slot, DEVICE(cpuobj), &error_fatal);
+
+ if (!vms->secure) {
+ object_property_set_bool(cpuobj, false, "has_el3", NULL);
+ }
+
+ if (!vms->virt && object_property_find(cpuobj, "has_el2", NULL)) {
+ object_property_set_bool(cpuobj, false, "has_el2", NULL);
+ }
+
+ if (vms->psci_conduit != QEMU_PSCI_CONDUIT_DISABLED) {
+ object_property_set_int(cpuobj, vms->psci_conduit, "psci-conduit",
+ NULL);
+ /* Secondary CPUs start in PSCI powered-down state */
+ if (cs->cpu_index > 0)
+ object_property_set_bool(cpuobj, true, "start-powered-off",
+ NULL);
+ }
+
+ if (vmc->kvm_no_adjvtime &&
+ object_property_find(cpuobj, "kvm-no-adjvtime", NULL)) {
+ object_property_set_bool(cpuobj, true, "kvm-no-adjvtime", NULL);
+ }
+
+ if (vmc->no_pmu && object_property_find(cpuobj, "pmu", NULL)) {
+ object_property_set_bool(cpuobj, false, "pmu", NULL);
+ }
+
+ if (object_property_find(cpuobj, "reset-cbar", NULL)) {
+ object_property_set_int(cpuobj, vms->memmap[VIRT_CPUPERIPHS].base,
+ "reset-cbar", &error_abort);
+ }
+
+ object_property_set_link(cpuobj, OBJECT(sysmem), "memory",
+ &error_abort);
+
+ if (vms->secure) {
+ object_property_set_link(cpuobj, OBJECT(vms->secure_sysmem),
+ "secure-memory", &error_abort);
+ }
+}
+
static void machvirt_init(MachineState *machine)
{
VirtMachineState *vms = VIRT_MACHINE(machine);
@@ -1706,6 +1764,7 @@ static void machvirt_init(MachineState *machine)
memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory",
UINT64_MAX);
memory_region_add_subregion_overlap(secure_sysmem, 0, sysmem, -1);
+ vms->secure_sysmem = secure_sysmem;
}
firmware_loaded = virt_firmware_init(vms, sysmem,
@@ -1749,6 +1808,14 @@ static void machvirt_init(MachineState *machine)
}
vms->smp_cpus = smp_cpus;
+ vms->max_cpus = max_cpus;
+ if (vms->gic_version < VIRT_GIC_VERSION_3) {
+ warn_report("For GICv%d max-cpus must be equal to smp-cpus",
+ vms->gic_version);
+ warn_report("Overriding specified max-cpus(%d) with smp-cpus(%d)",
+ max_cpus, smp_cpus);
+ vms->max_cpus = smp_cpus;
+ }
if (vms->virt && kvm_enabled()) {
error_report("mach-virt: KVM does not support providing "
@@ -1761,65 +1828,12 @@ static void machvirt_init(MachineState *machine)
possible_cpus = mc->possible_cpu_arch_ids(machine);
for (n = 0; n < possible_cpus->len; n++) {
Object *cpuobj;
- CPUState *cs;
-
- if (n >= smp_cpus) {
- break;
- }
cpuobj = object_new(possible_cpus->cpus[n].type);
- object_property_set_int(cpuobj, possible_cpus->cpus[n].arch_id,
- "mp-affinity", NULL);
-
- cs = CPU(cpuobj);
- cs->cpu_index = n;
-
- numa_cpu_pre_plug(&possible_cpus->cpus[cs->cpu_index], DEVICE(cpuobj),
- &error_fatal);
aarch64 &= object_property_get_bool(cpuobj, "aarch64", NULL);
object_property_set_int(cpuobj, n, "core-id", NULL);
- if (!vms->secure) {
- object_property_set_bool(cpuobj, false, "has_el3", NULL);
- }
-
- if (!vms->virt && object_property_find(cpuobj, "has_el2", NULL)) {
- object_property_set_bool(cpuobj, false, "has_el2", NULL);
- }
-
- if (vms->psci_conduit != QEMU_PSCI_CONDUIT_DISABLED) {
- object_property_set_int(cpuobj, vms->psci_conduit,
- "psci-conduit", NULL);
-
- /* Secondary CPUs start in PSCI powered-down state */
- if (n > 0) {
- object_property_set_bool(cpuobj, true,
- "start-powered-off", NULL);
- }
- }
-
- if (vmc->kvm_no_adjvtime &&
- object_property_find(cpuobj, "kvm-no-adjvtime", NULL)) {
- object_property_set_bool(cpuobj, true, "kvm-no-adjvtime", NULL);
- }
-
- if (vmc->no_pmu && object_property_find(cpuobj, "pmu", NULL)) {
- object_property_set_bool(cpuobj, false, "pmu", NULL);
- }
-
- if (object_property_find(cpuobj, "reset-cbar", NULL)) {
- object_property_set_int(cpuobj, vms->memmap[VIRT_CPUPERIPHS].base,
- "reset-cbar", &error_abort);
- }
-
- object_property_set_link(cpuobj, OBJECT(sysmem), "memory",
- &error_abort);
- if (vms->secure) {
- object_property_set_link(cpuobj, OBJECT(secure_sysmem),
- "secure-memory", &error_abort);
- }
-
object_property_set_bool(cpuobj, true, "realized", &error_fatal);
object_unref(cpuobj);
}
@@ -2106,6 +2120,71 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
return ms->possible_cpus;
}
+static int virt_archid_cmp(const void *a, const void *b)
+{
+ CPUArchId *archid_a = (CPUArchId *)a;
+ CPUArchId *archid_b = (CPUArchId *)b;
+
+ return archid_a->arch_id - archid_b->arch_id;
+}
+
+static CPUArchId *virt_find_cpu_slot(MachineState *ms, int vcpuid)
+{
+ VirtMachineState *vms = VIRT_MACHINE(ms);
+ CPUArchId arch_id, *found_cpu;
+ uint64_t mp_affinity;
+
+ mp_affinity = virt_cpu_mp_affinity(vms, vcpuid);
+ arch_id.arch_id = mp_affinity;
+ found_cpu = bsearch(&arch_id, ms->possible_cpus->cpus,
+ ms->possible_cpus->len,
+ sizeof(*ms->possible_cpus->cpus), virt_archid_cmp);
+
+ assert (found_cpu);
+
+ /*
+ * RFC: Question:
+ * For KVM/TCG, MPIDR for vcpu is derived using vcpu-id.
+ * In fact, as of now there is a linear relation between
+ * vcpu-id and mpidr(see below fig.) as derived in host
+ * kvm. Slot-id is the index where vcpu with certain
+ * arch-id(=mpidr/ap-affinity) is plugged.
+ *
+ * Therefore, for now we could use the vcpu-id as slot
+ * index for getting CPUArchId of the vcpu coresponding
+ * to this slot(this view is not perfectly consistent
+ * with the ARM specification view of MPIDR_EL1).
+ * QEMU/KVM view of cpu topology makes it bit difficult
+ * to use topo-info(pkg-id, core-id, thread-id) with
+ * device_add/-device interface which might not match
+ * with what actual underlying host cpu supports.
+ * therefore question is do we care about this? and
+ * is it okay to have view of thread-id inconsistent
+ * with the host cpu? How should QEMU create PPTT
+ * for the Guest?
+ *
+ * +----+----+----+----+----+----+----+----+
+ * MASK | F F | F F | F F | 0 F |
+ * +----+----+----+----+----+----+----+----+
+ *
+ * | | cluster | cluster | |core|
+ * |<---------Package-id-------->| |core|
+ *
+ * +----+----+----+----+----+----+----+----+
+ * MPIDR ||| Res | Aff2 | Aff1 | Aff0 |
+ * +----+----+----+----+----+----+----+----+
+ * \ \ \ | |
+ * \ 8bit \ 8bit \ |4bit|
+ * \<------->\<------->\ |<-->|
+ * \ \ \| |
+ * +----+----+----+----+----+----+----+----+
+ * VCPU-ID | Byte4 | Byte2 | Byte1 | Byte0 |
+ * +----+----+----+----+----+----+----+----+
+ */
+
+ return found_cpu;
+}
+
static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
Error **errp)
{
@@ -2151,11 +2230,71 @@ out:
error_propagate(errp, local_err);
}
+static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp)
+{
+ MachineState *ms = MACHINE(hotplug_dev);
+ ARMCPU *cpu = ARM_CPU(dev);
+ CPUState *cs = CPU(dev);
+ CPUArchId *cpu_slot;
+
+ /* sanity check the cpu */
+ if (!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) {
+ error_setg(errp, "Invalid CPU type, expected cpu type: '%s'",
+ ms->cpu_type);
+ return;
+ }
+
+ if ((cpu->core_id < 0) || (cpu->core_id >= ms->possible_cpus->len)) {
+ error_setg(errp, "Invalid core-id %u specified, must be in range 1:%u",
+ cpu->core_id, ms->possible_cpus->len - 1);
+ return;
+ }
+
+ /*
+ * RFC: Question:
+ * For now we are not taking into account of other topo info like
+ * thread-id, socket-id to generate arch-id/mp-affinity.
+ * The way KVM/Host generates mpidr value and the way ARM spec
+ * identifies uniquely cpu within the heirarchy is bit inconsistent.
+ * Perhaps needs more discussion on this? Hence, directly using
+ * core_id as cpu_index for now. Ideally, slot-index found out using
+ * the topo info should have been the cpu-index.
+ */
+ cs->cpu_index = cpu->core_id;
+
+ cpu_slot = virt_find_cpu_slot(ms, cpu->core_id);
+ if (qemu_present_cpu(CPU(cpu_slot->cpu))) {
+ error_setg(errp, "cpu %d with arch-id %" PRIu64 " exists",
+ cpu->core_id, cpu_slot->arch_id);
+ return;
+ }
+ virt_cpu_set_properties(OBJECT(cs), cpu_slot);
+}
+
+static void virt_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp)
+{
+ MachineState *ms = MACHINE(hotplug_dev);
+ ARMCPU *cpu = ARM_CPU(dev);
+ CPUState *cs = CPU(dev);
+ CPUArchId *cpu_slot;
+
+ /* insert the cold/hot-plugged vcpu in the slot */
+ cpu_slot = virt_find_cpu_slot(ms, cpu->core_id);
+ cpu_slot->cpu = OBJECT(dev);
+
+ cs->disabled = false;
+ return;
+}
+
static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
virt_memory_pre_plug(hotplug_dev, dev, errp);
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+ virt_cpu_pre_plug(hotplug_dev, dev, errp);
}
}
@@ -2172,6 +2311,8 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
}
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
virt_memory_plug(hotplug_dev, dev, errp);
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+ virt_cpu_plug(hotplug_dev, dev, errp);
}
if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
PCIDevice *pdev = PCI_DEVICE(dev);
@@ -2193,7 +2334,8 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
DeviceState *dev)
{
if (object_dynamic_cast(OBJECT(dev), TYPE_SYS_BUS_DEVICE) ||
- (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM))) {
+ (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) ||
+ (object_dynamic_cast(OBJECT(dev), TYPE_CPU))){
return HOTPLUG_HANDLER(machine);
}
if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 31878ddc72..5b8ba64ec2 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -127,6 +127,7 @@ typedef struct {
DeviceState *platform_bus_dev;
FWCfgState *fw_cfg;
PFlashCFI01 *flash[2];
+ MemoryRegion *secure_sysmem;
bool secure;
bool highmem;
bool highmem_ecam;
@@ -142,6 +143,7 @@ typedef struct {
char *pciehb_nodename;
const int *irqmap;
int smp_cpus;
+ int max_cpus;
void *fdt;
int fdt_size;
uint32_t clock_phandle;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 33a58086a9..0c9f5f970e 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2131,6 +2131,12 @@ static gchar *arm_gdb_arch_name(CPUState *cs)
return g_strdup("arm");
}
+static int64_t arm_cpu_get_arch_id(CPUState *cs)
+{
+ ARMCPU *cpu = ARM_CPU(cs);
+ return cpu->mp_affinity;
+}
+
static void arm_cpu_class_init(ObjectClass *oc, void *data)
{
ARMCPUClass *acc = ARM_CPU_CLASS(oc);
@@ -2147,6 +2153,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
cc->has_work = arm_cpu_has_work;
cc->cpu_exec_interrupt = arm_cpu_exec_interrupt;
cc->dump_state = arm_cpu_dump_state;
+ cc->get_arch_id = arm_cpu_get_arch_id;
cc->set_pc = arm_cpu_set_pc;
cc->synchronize_from_tb = arm_cpu_synchronize_from_tb;
cc->gdb_read_register = arm_cpu_gdb_read_register;
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index cbc5c3868f..41e69cd53f 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -766,11 +766,18 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp)
static void aarch64_cpu_initfn(Object *obj)
{
+ CPUState *cs = CPU(obj);
+
object_property_add_bool(obj, "aarch64", aarch64_cpu_get_aarch64,
aarch64_cpu_set_aarch64);
object_property_set_description(obj, "aarch64",
"Set on/off to enable/disable aarch64 "
"execution state ");
+ /*
+ * we start every ARM64 vcpu as disabled possible vcpu. It needs to be
+ * enabled explicitly
+ */
+ cs->disabled = true;
}
static void aarch64_cpu_finalizefn(Object *obj)
@@ -785,7 +792,9 @@ static gchar *aarch64_gdb_arch_name(CPUState *cs)
static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
{
CPUClass *cc = CPU_CLASS(oc);
+ DeviceClass *dc = DEVICE_CLASS(oc);
+ dc->user_creatable = true;
cc->cpu_exec_interrupt = arm_cpu_exec_interrupt;
cc->gdb_read_register = aarch64_cpu_gdb_read_register;
cc->gdb_write_register = aarch64_cpu_gdb_write_register;
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 05/22] arm/cpuhp: Pre-create disabled possible vcpus @machine init
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (3 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 04/22] arm/cpuhp: Machine init time change common to vcpu {cold|hot}-plug Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 06/22] arm/cpuhp: Changes to pre-size GIC with " Salil Mehta
` (20 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
In ARMv8 architecture, GIC needs all the vcpus to be created and present when
it is initialized. This is because:
1. GICC and MPIDR association must be fixed at the VM initialization time.
This is represented by register GIC_TYPER(mp_afffinity, proc_num)
2. GICC(cpu interfaces), GICR(redistributors) etc all must be initialized
at the boot time as well.
3. Memory regions associated with GICR etc. cannot be changed(add/del/mod)
after VM has inited.
This patch adds the support to pre-create all such possible vcpus within the
host using the KVM interface as part of the virt machine initialization. These
vcpus could later be attached to QOM/ACPI while they are actually hot plugged
and made present.
NOTE: There is some refactoring related to the kvm_destroy_vcpu/kvm_get_vcpu
(to make use of the common code) has been intentionaly left out in RFC
version to avoid obscuring the framework change of the cpu hotplug. The
existing code being presented in this patch could further be optimized
later.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
accel/kvm/kvm-all.c | 31 +++++++++++++++++++++++++++++
hw/arm/virt.c | 46 ++++++++++++++++++++++++++++++++++++++++++--
include/sysemu/kvm.h | 2 ++
target/arm/kvm.c | 32 ++++++++++++++++++++++++++++++
target/arm/kvm_arm.h | 11 +++++++++++
5 files changed, 120 insertions(+), 2 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index d06cc04079..8e1c7b3d13 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -329,6 +329,37 @@ err:
return ret;
}
+void kvm_park_vcpu(CPUState *cs)
+{
+ unsigned long vcpu_id = cs->cpu_index;
+ struct KVMParkedVcpu *vcpu;
+
+ vcpu = g_malloc0(sizeof(*vcpu));
+ vcpu->vcpu_id = vcpu_id;
+ vcpu->kvm_fd = cs->kvm_fd;
+ QLIST_INSERT_HEAD(&kvm_state->kvm_parked_vcpus, vcpu, node);
+}
+
+int kvm_create_vcpu(CPUState *cpu)
+{
+ unsigned long vcpu_id = cpu->cpu_index;
+ KVMState *s = kvm_state;
+ int ret = 0;
+
+ DPRINTF("kvm_create_vcpu\n");
+
+ ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)vcpu_id);
+ if (ret < 0) {
+ goto err;
+ }
+ cpu->kvm_fd = ret;
+ cpu->kvm_state = s;
+ cpu->vcpu_dirty = true;
+
+err:
+ return ret;
+}
+
int kvm_destroy_vcpu(CPUState *cpu)
{
KVMState *s = kvm_state;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 184bed8716..8040473d30 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1828,14 +1828,56 @@ static void machvirt_init(MachineState *machine)
possible_cpus = mc->possible_cpu_arch_ids(machine);
for (n = 0; n < possible_cpus->len; n++) {
Object *cpuobj;
+ CPUState *cs;
cpuobj = object_new(possible_cpus->cpus[n].type);
+ cs = CPU(cpuobj);
aarch64 &= object_property_get_bool(cpuobj, "aarch64", NULL);
object_property_set_int(cpuobj, n, "core-id", NULL);
- object_property_set_bool(cpuobj, true, "realized", &error_fatal);
- object_unref(cpuobj);
+ if (n < vms->smp_cpus) {
+ char *core_id = g_strdup_printf("core%d", n);
+ qdev_set_id(DEVICE(cpuobj),core_id);
+ object_property_set_bool(cpuobj, true, "realized", &error_fatal);
+ g_free(core_id);
+ object_unref(OBJECT(cs));
+ } else {
+ CPUArchId *cpu_slot;
+ /* handling for vcpus which are yet to be hot-plugged */
+ cs->cpu_index = n;
+ /* ARM host vcpu features need to be fixed at the boot time */
+ virt_cpu_set_properties(cpuobj, &possible_cpus->cpus[n]);
+ /*
+ * For KVM, we shall be pre-creating the now disabled/un-plugged
+ * possbile host vcpus and park them till the time they are
+ * actually hot plugged. This is required to pre-size the host
+ * GICC and GICR with the all possible vcpus for this VM.
+ */
+ if (kvm_enabled()) {
+ kvm_arm_create_host_vcpu(ARM_CPU(cs));
+ }
+ /*
+ * Add disabled vcpu to cpu slot during the init phase of the virt machine.
+ * 1. We need this ARMCPU object during the GIC init. This object
+ * will facilitate in pre-realizing the gic. Any info like
+ * mp-affinity(required to derive gicr_type) etc could still be
+ * fetched while preserving QOM abstraction akin to realized
+ * vcpus.
+ * 2. Now, after initialization of the virt machine is complete we could use
+ * two approaches to deal with this ARMCPU object:
+ * (i) re-use this ARMCPU object during hotplug of this vcpu.
+ * OR
+ * (ii) defer release this ARMCPU object after gic has been
+ * initialized or during pre-plug phase when a vcpu is
+ * hotplugged.
+ *
+ * We will use the (ii) approach and release the ARMCPU objects after GIC
+ * and machine has been initialized in machine_init_done() phase
+ */
+ cpu_slot = virt_find_cpu_slot(machine, cs->cpu_index);
+ cpu_slot->cpu = OBJECT(cs);
+ }
}
fdt_add_timer_nodes(vms);
fdt_add_cpu_nodes(vms);
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 3b2250471c..ca06bfeb17 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -218,7 +218,9 @@ int kvm_has_intx_set_mask(void);
int kvm_init_vcpu(CPUState *cpu);
int kvm_cpu_exec(CPUState *cpu);
+int kvm_create_vcpu(CPUState *cpu);
int kvm_destroy_vcpu(CPUState *cpu);
+void kvm_park_vcpu(CPUState *cs);
/**
* kvm_arm_supports_user_irq
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 4bdbe6dcac..9fd447d111 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -597,6 +597,38 @@ void kvm_arm_reset_vcpu(ARMCPU *cpu)
write_list_to_cpustate(cpu);
}
+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
+ * gets programmed during this call to host. These are referred
+ * 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);
+}
+
/*
* Update KVM's MP_STATE based on what QEMU thinks it is
*/
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 48bf5e16d5..a9e316cfee 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -155,6 +155,17 @@ void kvm_arm_cpu_post_load(ARMCPU *cpu);
*/
void kvm_arm_reset_vcpu(ARMCPU *cpu);
+/**
+ * kvm_arm_create_host_vcpu:
+ * @cpu: ARMCPU
+ *
+ * Called at to pre create all possible kvm vcpus within the the host at the
+ * virt machine init time. This will also init this pre-created vcpu and
+ * hence result in vcpu reset at host. These pre created and inited vcpus
+ * shall be parked for use when ARM vcpus are actually realized.
+ */
+void kvm_arm_create_host_vcpu(ARMCPU *cpu);
+
/**
* kvm_arm_init_serror_injection:
* @cs: CPUState
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 06/22] arm/cpuhp: Changes to pre-size GIC with possible vcpus @machine init
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (4 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 05/22] arm/cpuhp: Pre-create disabled possible vcpus @machine init Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 07/22] arm/cpuhp: Init PMU at host for all possible vcpus Salil Mehta
` (19 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
GIC needs to be pre-sized with possible vcpus at the initialization time. This
is necessary because Memory regions and resources associated with GICC/GICR
etc cannot be changed (add/del/modified) after VM has inited. Also, GIC_TYPER
needs to be initialized with mp_affinity and cpu interface number association.
This cannot be changed after GIC has initialized.
Once all the cpu interfaces of the GIC has been inited it needs to be ensured
that any updations to the GICC during reset only takes place for the present
vcpus and not the disabled ones. Therefore, proper checks are required at
various places.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/arm/virt.c | 18 +++++++++---------
hw/intc/arm_gicv3_common.c | 8 ++++++--
hw/intc/arm_gicv3_cpuif.c | 6 ++++++
hw/intc/arm_gicv3_kvm.c | 29 ++++++++++++++++++++++++++---
include/hw/arm/virt.h | 2 +-
5 files changed, 48 insertions(+), 15 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 8040473d30..9e55b20685 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -628,19 +628,19 @@ static void create_v2m(VirtMachineState *vms)
static void create_gic(VirtMachineState *vms)
{
- MachineState *ms = MACHINE(vms);
/* We create a standalone GIC */
SysBusDevice *gicbusdev;
const char *gictype;
int type = vms->gic_version, i;
- unsigned int smp_cpus = ms->smp.cpus;
+ unsigned int max_cpus = vms->max_cpus;
+ unsigned int smp_cpus = vms->smp_cpus;
uint32_t nb_redist_regions = 0;
gictype = (type == 3) ? gicv3_class_name() : gic_class_name();
vms->gic = qdev_create(NULL, gictype);
qdev_prop_set_uint32(vms->gic, "revision", type);
- qdev_prop_set_uint32(vms->gic, "num-cpu", smp_cpus);
+ qdev_prop_set_uint32(vms->gic, "num-cpu", max_cpus);
/* Note that the num-irq property counts both internal and external
* interrupts; there are always 32 of the former (mandated by GIC spec).
*/
@@ -652,7 +652,7 @@ static void create_gic(VirtMachineState *vms)
if (type == 3) {
uint32_t redist0_capacity =
vms->memmap[VIRT_GIC_REDIST].size / GICV3_REDIST_SIZE;
- uint32_t redist0_count = MIN(smp_cpus, redist0_capacity);
+ uint32_t redist0_count = MIN(max_cpus, redist0_capacity);
nb_redist_regions = virt_gicv3_redist_region_count(vms);
@@ -665,7 +665,7 @@ static void create_gic(VirtMachineState *vms)
vms->memmap[VIRT_HIGH_GIC_REDIST2].size / GICV3_REDIST_SIZE;
qdev_prop_set_uint32(vms->gic, "redist-region-count[1]",
- MIN(smp_cpus - redist0_count, redist1_capacity));
+ MIN(max_cpus - redist0_count, redist1_capacity));
}
} else {
if (!kvm_irqchip_in_kernel()) {
@@ -722,7 +722,7 @@ static void create_gic(VirtMachineState *vms)
} else if (vms->virt) {
qemu_irq irq = qdev_get_gpio_in(vms->gic,
ppibase + ARCH_GIC_MAINT_IRQ);
- sysbus_connect_irq(gicbusdev, i + 4 * smp_cpus, irq);
+ sysbus_connect_irq(gicbusdev, i + 4 * max_cpus, irq);
}
qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0,
@@ -730,11 +730,11 @@ static void create_gic(VirtMachineState *vms)
+ VIRTUAL_PMU_IRQ));
sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
- sysbus_connect_irq(gicbusdev, i + smp_cpus,
+ sysbus_connect_irq(gicbusdev, i + max_cpus,
qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
- sysbus_connect_irq(gicbusdev, i + 2 * smp_cpus,
+ sysbus_connect_irq(gicbusdev, i + 2 * max_cpus,
qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
- sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus,
+ sysbus_connect_irq(gicbusdev, i + 3 * max_cpus,
qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
}
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 58ef65f589..bfa514444a 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -348,11 +348,15 @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
s->cpu = g_new0(GICv3CPUState, s->num_cpu);
for (i = 0; i < s->num_cpu; i++) {
- CPUState *cpu = qemu_get_cpu(i);
+ CPUState *cpu = qemu_get_possible_cpu(i);
uint64_t cpu_affid;
int last;
- s->cpu[i].cpu = cpu;
+ if (qemu_present_cpu(cpu))
+ s->cpu[i].cpu = cpu;
+ else
+ s->cpu[i].cpu = NULL;
+
s->cpu[i].gic = s;
/* Store GICv3CPUState in CPUARMState gicv3state pointer */
gicv3_set_gicv3state(cpu, &s->cpu[i]);
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index 08e000e33c..90d8b0118e 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -779,6 +779,9 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
ARMCPU *cpu = ARM_CPU(cs->cpu);
CPUARMState *env = &cpu->env;
+ if (!qemu_present_cpu(cs->cpu))
+ return;
+
g_assert(qemu_mutex_iothread_locked());
trace_gicv3_cpuif_update(gicv3_redist_affid(cs), cs->hppi.irq,
@@ -1654,6 +1657,9 @@ static void icc_generate_sgi(CPUARMState *env, GICv3CPUState *cs,
for (i = 0; i < s->num_cpu; i++) {
GICv3CPUState *ocs = &s->cpu[i];
+ if (!qemu_present_cpu(ocs->cpu))
+ continue;
+
if (irm) {
/* IRM == 1 : route to all CPUs except self */
if (cs == ocs) {
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index ca43bf87ca..7fe000e53c 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -25,6 +25,7 @@
#include "hw/sysbus.h"
#include "qemu/error-report.h"
#include "qemu/module.h"
+#include "sysemu/cpus.h"
#include "sysemu/kvm.h"
#include "sysemu/runstate.h"
#include "kvm_arm.h"
@@ -458,6 +459,17 @@ static void kvm_arm_gicv3_put(GICv3State *s)
GICv3CPUState *c = &s->cpu[ncpu];
int num_pri_bits;
+ /*
+ * To support hotplug of vcpus we need to make sure all gic cpuif/GICC
+ * are initialized at machvirt init time. Once the init is done we
+ * release the ARMCPU object for disabled vcpus but this leg could hit
+ * during reset of GICC later as well i.e. after init has happened and
+ * all of the cases we want to make sure we dont acess the GICC for
+ * the disabled VCPUs.
+ */
+ if (!qemu_present_cpu(c->cpu))
+ continue;
+
kvm_gicc_access(s, ICC_SRE_EL1, ncpu, &c->icc_sre_el1, true);
kvm_gicc_access(s, ICC_CTLR_EL1, ncpu,
&c->icc_ctlr_el1[GICV3_NS], true);
@@ -677,10 +689,21 @@ static void arm_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo *ri)
return;
}
+ /*
+ * This shall be called even when vcpu is being hotplugged and other vcpus
+ * might be running. Host kernel KVM code to handle device access of IOCTLs
+ * KVM_{GET|SET}_DEVICE_ATTR might fail due to inability to grab vcpu locks
+ * for all the vcpus. Hence, we need to pause all vcpus to facilitate
+ * locking within host.
+ */
+ if (!qemu_present_cpu(c->cpu))
+ pause_all_vcpus();
/* Initialize to actual HW supported configuration */
kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
KVM_VGIC_ATTR(ICC_CTLR_EL1, c->gicr_typer),
&c->icc_ctlr_el1[GICV3_NS], false, &error_abort);
+ if (!qemu_present_cpu(c->cpu))
+ resume_all_vcpus();
c->icc_ctlr_el1[GICV3_S] = c->icc_ctlr_el1[GICV3_NS];
}
@@ -788,9 +811,9 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
}
for (i = 0; i < s->num_cpu; i++) {
- ARMCPU *cpu = ARM_CPU(qemu_get_cpu(i));
-
- define_arm_cp_regs(cpu, gicv3_cpuif_reginfo);
+ CPUState *cs = qemu_get_cpu(i);
+ if (qemu_present_cpu(cs))
+ define_arm_cp_regs(ARM_CPU(cs), gicv3_cpuif_reginfo);
}
/* Try to create the device via the device control API */
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 5b8ba64ec2..38a9cad168 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -178,7 +178,7 @@ static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)
assert(vms->gic_version == VIRT_GIC_VERSION_3);
- return vms->smp_cpus > redist0_capacity ? 2 : 1;
+ return vms->max_cpus > redist0_capacity ? 2 : 1;
}
#endif /* QEMU_ARM_VIRT_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 07/22] arm/cpuhp: Init PMU at host for all possible vcpus
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (5 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 06/22] arm/cpuhp: Changes to pre-size GIC with " Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-23 9:00 ` Andrew Jones
2020-06-13 21:36 ` [PATCH RFC 08/22] arm/cpuhp: Enable ACPI support for vcpu hotplug Salil Mehta
` (18 subsequent siblings)
25 siblings, 1 reply; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
PMU for all possible vcpus must be initialized at the virt machine
initialization time. This patch refactors existing code to accomodate possible
vcpus. This also assumes that all processor being used are identical at least
for now but does not affect the normal scanarios where they might not be in
future. This assumption only affects the future hotplug scenarios if ever there
exists any hetergenous processors. In such a case PMU might not be enabled on
some vcpus. Is it acceptable and doable tradeoff for now?
This perhaps needs more discussion. please check below link,
Link: https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg00131.html
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/arm/virt.c | 51 ++++++++++++++++++++++++++++++-------------
include/hw/arm/virt.h | 1 +
2 files changed, 37 insertions(+), 15 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 9e55b20685..7f938f289b 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -525,23 +525,9 @@ static void fdt_add_gic_node(VirtMachineState *vms)
static void fdt_add_pmu_nodes(const VirtMachineState *vms)
{
- CPUState *cpu;
ARMCPU *armcpu;
uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
- CPU_FOREACH(cpu) {
- armcpu = ARM_CPU(cpu);
- if (!arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
- return;
- }
- if (kvm_enabled()) {
- if (kvm_irqchip_in_kernel()) {
- kvm_arm_pmu_set_irq(cpu, PPI(VIRTUAL_PMU_IRQ));
- }
- kvm_arm_pmu_init(cpu);
- }
- }
-
if (vms->gic_version == VIRT_GIC_VERSION_2) {
irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
GIC_FDT_IRQ_PPI_CPU_WIDTH,
@@ -1414,6 +1400,38 @@ static void create_secure_ram(VirtMachineState *vms,
g_free(nodename);
}
+static bool virt_pmu_init(VirtMachineState *vms)
+{
+ CPUArchIdList *possible_cpus = vms->parent.possible_cpus;
+ ARMCPU *armcpu;
+ int n;
+
+ /*
+ * As of now KVM ensures that within the host all the vcpus have same
+ * features configured. This cannot be changed later and cannot be diferent
+ * for new vcpus being plugged in. Also, -cpu option/virt machine cpu-type
+ * ensures all the vcpus are identical.
+ */
+ for (n = 0; n < possible_cpus->len; n++) {
+ CPUState *cpu = qemu_get_possible_cpu(n);
+ armcpu = ARM_CPU(cpu);
+
+ if (!arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
+ warn_report("Not all vcpus might have PMU initialized");
+ return false;
+ }
+
+ if (kvm_enabled()) {
+ if (kvm_irqchip_in_kernel()) {
+ kvm_arm_pmu_set_irq(cpu, PPI(VIRTUAL_PMU_IRQ));
+ }
+ kvm_arm_pmu_init(cpu);
+ }
+ }
+
+ return true;
+}
+
static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
{
const VirtMachineState *board = container_of(binfo, VirtMachineState,
@@ -1909,7 +1927,10 @@ static void machvirt_init(MachineState *machine)
create_gic(vms);
- fdt_add_pmu_nodes(vms);
+ if (!vmc->no_pmu && virt_pmu_init(vms)) {
+ vms->pmu = true;
+ fdt_add_pmu_nodes(vms);
+ }
create_uart(vms, VIRT_UART, sysmem, serial_hd(0));
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 38a9cad168..3ffbda6217 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -134,6 +134,7 @@ typedef struct {
bool its;
bool virt;
bool ras;
+ bool pmu;
OnOffAuto acpi;
VirtGICType gic_version;
VirtIOMMUType iommu;
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 08/22] arm/cpuhp: Enable ACPI support for vcpu hotplug
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (6 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 07/22] arm/cpuhp: Init PMU at host for all possible vcpus Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 09/22] arm/cpuhp: Init GED framework with cpu hotplug events Salil Mehta
` (17 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
ACPI is required to interface QEMU with the guest. Roughly falls into below
cases,
1. Convey the possible vcpus config at the machine init time to the guest
using various DSDT tables like MADT etc.
2. Convey vcpu hotplug events to guest(using GED)
3. Assist in evaluation of various ACPI methods(like _EVT, _STA, _OST, _EJ0,
_MAT etc.)
4. Provides ACPI cpu hotplug state and 12 Byte memory mapped cpu hotplug
control register interface to the OSPM/guest corresponding to each possible
vcpu. The register interface consists of various R/W fields and their
handling operations. These are called when ever register fields or memory
regions are accessed(i.e. read or written) by OSPM when ever it evaluates
various ACPI methods.
Note: lot of this framework code is inherited from the changes already done for
x86 but still some minor changes are required to make it compatible with
ARM64.)
This patch enables the ACPI support for virtual cpu hotplug in kconfig and
during initialization.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/acpi/cpu.c | 6 +++++-
hw/arm/Kconfig | 1 +
hw/arm/virt.c | 2 ++
include/hw/acpi/cpu_hotplug.h | 2 ++
include/hw/arm/virt.h | 1 +
5 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 3d6a500fb7..21fe0463b9 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -218,7 +218,11 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
state->dev_count = id_list->len;
state->devs = g_new0(typeof(*state->devs), state->dev_count);
for (i = 0; i < id_list->len; i++) {
- state->devs[i].cpu = CPU(id_list->cpus[i].cpu);
+ struct CPUState *cpu = CPU(id_list->cpus[i].cpu);
+ if (qemu_present_cpu(cpu))
+ state->devs[i].cpu = cpu;
+ else
+ state->devs[i].cpu = NULL;
state->devs[i].arch_id = id_list->cpus[i].arch_id;
}
memory_region_init_io(&state->ctrl_reg, owner, &cpu_hotplug_ops, state,
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 9afa6eee79..cb67fb806b 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -26,6 +26,7 @@ config ARM_VIRT
select ACPI_MEMORY_HOTPLUG
select ACPI_HW_REDUCED
select ACPI_NVDIMM
+ select ACPI_CPU_HOTPLUG
config CHEETAH
bool
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7f938f289b..fe37babe35 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -75,6 +75,7 @@
#include "hw/mem/pc-dimm.h"
#include "hw/mem/nvdimm.h"
#include "hw/acpi/generic_event_device.h"
+#include "hw/acpi/cpu_hotplug.h"
#include "hw/virtio/virtio-iommu.h"
#include "hw/char/pl011.h"
#include "qemu/guest-random.h"
@@ -151,6 +152,7 @@ static const MemMapEntry base_memmap[] = {
[VIRT_PCDIMM_ACPI] = { 0x09070000, MEMORY_HOTPLUG_IO_LEN },
[VIRT_ACPI_GED] = { 0x09080000, ACPI_GED_EVT_SEL_LEN },
[VIRT_NVDIMM_ACPI] = { 0x09090000, NVDIMM_ACPI_IO_LEN},
+ [VIRT_CPUHP_ACPI] = { 0x090a0000, ACPI_CPU_HOTPLUG_REG_LEN},
[VIRT_MMIO] = { 0x0a000000, 0x00000200 },
/* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
[VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 3b932abbbb..48b291e45e 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -19,6 +19,8 @@
#include "hw/hotplug.h"
#include "hw/acpi/cpu.h"
+#define ACPI_CPU_HOTPLUG_REG_LEN 12
+
typedef struct AcpiCpuHotplug {
Object *device;
MemoryRegion io;
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 3ffbda6217..e0bd9df69d 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -80,6 +80,7 @@ enum {
VIRT_PCDIMM_ACPI,
VIRT_ACPI_GED,
VIRT_NVDIMM_ACPI,
+ VIRT_CPUHP_ACPI,
VIRT_LOWMEMMAP_LAST,
};
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 09/22] arm/cpuhp: Init GED framework with cpu hotplug events
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (7 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 08/22] arm/cpuhp: Enable ACPI support for vcpu hotplug Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 10/22] arm/cpuhp: Update CPUs AML with cpu-(ctrl)dev change Salil Mehta
` (16 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
ACPI GED(as described in the ACPI 6.2 spec) can be used to generate ACPI events
when OSPM/guest receives an interrupt listed in the _CRS object of GED. OSPM
then maps or demultiplexes the event by evaluating _EVT method.
This change adds the support of cpu hotplug event initialization in the
existing GED framework.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/acpi/generic_event_device.c | 8 ++++++++
hw/arm/virt.c | 3 ++-
include/hw/acpi/generic_event_device.h | 5 +++++
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 1cb34111e5..0f2c8a959e 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -25,6 +25,7 @@ static const uint32_t ged_supported_events[] = {
ACPI_GED_MEM_HOTPLUG_EVT,
ACPI_GED_PWR_DOWN_EVT,
ACPI_GED_NVDIMM_HOTPLUG_EVT,
+ ACPI_GED_CPU_HOTPLUG_EVT,
};
/*
@@ -305,6 +306,13 @@ static void acpi_ged_initfn(Object *obj)
sysbus_init_mmio(sbd, &s->container_memhp);
acpi_memory_hotplug_init(&s->container_memhp, OBJECT(dev),
&s->memhp_state, 0);
+
+ s->cpuhp.device = OBJECT(s);
+ memory_region_init(&s->container_cpuhp, OBJECT(dev), "cpuhp container",
+ ACPI_CPU_HOTPLUG_REG_LEN);
+ sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container_cpuhp);
+ cpu_hotplug_hw_init(&s->container_cpuhp, OBJECT(dev),
+ &s->cpuhp_state, 0);
}
static void acpi_ged_class_init(ObjectClass *class, void *data)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index fe37babe35..e9ead0e2dd 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -552,7 +552,7 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
DeviceState *dev;
MachineState *ms = MACHINE(vms);
int irq = vms->irqmap[VIRT_ACPI_GED];
- uint32_t event = ACPI_GED_PWR_DOWN_EVT;
+ uint32_t event = ACPI_GED_PWR_DOWN_EVT | ACPI_GED_CPU_HOTPLUG_EVT;
if (ms->ram_slots) {
event |= ACPI_GED_MEM_HOTPLUG_EVT;
@@ -567,6 +567,7 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_ACPI_GED].base);
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, vms->memmap[VIRT_PCDIMM_ACPI].base);
+ sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, vms->memmap[VIRT_CPUHP_ACPI].base);
sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, qdev_get_gpio_in(vms->gic, irq));
qdev_init_nofail(dev);
diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
index 90a9180db5..b04037cf62 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -62,6 +62,7 @@
#include "hw/sysbus.h"
#include "hw/acpi/memory_hotplug.h"
#include "hw/acpi/ghes.h"
+#include "hw/acpi/cpu_hotplug.h"
#define ACPI_POWER_BUTTON_DEVICE "PWRB"
@@ -84,6 +85,7 @@
#define ACPI_GED_MEM_HOTPLUG_EVT 0x1
#define ACPI_GED_PWR_DOWN_EVT 0x2
#define ACPI_GED_NVDIMM_HOTPLUG_EVT 0x4
+#define ACPI_GED_CPU_HOTPLUG_EVT 0x8
typedef struct GEDState {
MemoryRegion evt;
@@ -94,6 +96,9 @@ typedef struct AcpiGedState {
SysBusDevice parent_obj;
MemHotplugState memhp_state;
MemoryRegion container_memhp;
+ CPUHotplugState cpuhp_state;
+ MemoryRegion container_cpuhp;
+ AcpiCpuHotplug cpuhp;
GEDState ged_state;
uint32_t ged_event_bitmap;
qemu_irq irq;
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 10/22] arm/cpuhp: Update CPUs AML with cpu-(ctrl)dev change
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (8 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 09/22] arm/cpuhp: Init GED framework with cpu hotplug events Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 11/22] arm/cpuhp: Update GED _EVT method AML with cpu scan Salil Mehta
` (15 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
CPUs Control device(\\_SB.PCI0) register interface for the x86 arch is based on
PCI and is IO port based and hence existing cpus AML code assumes _CRS objects
would evaluate to a system resource which describes IO Port address. But on ARM
arch CPUs control device(\\_SB.PRES) register interface is memory-mapped hence
_CRS object should evaluate to system resource which describes memory-mapped
base address.
This cpus AML code change updates the existing inerface of the build cpus AML
function to accept both IO/MEMORY type regions and update the _CRS object
correspondingly.
NOTE: Beside above CPU scan shall be triggered when OSPM evaluates _EVT method
part of the GED framework which is covered in subsequent patch.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/acpi/cpu.c | 23 ++++++++++++++++-------
hw/arm/virt-acpi-build.c | 13 ++++++++++++-
hw/i386/acpi-build.c | 2 +-
include/hw/acpi/cpu.h | 5 +++--
4 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 21fe0463b9..867fdd6993 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -335,9 +335,10 @@ const VMStateDescription vmstate_cpu_hotplug = {
#define CPU_EJECT_EVENT "CEJ0"
void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
- hwaddr io_base,
+ hwaddr mmap_io_base,
const char *res_root,
- const char *event_handler_method)
+ const char *event_handler_method,
+ AmlRegionSpace rs)
{
Aml *ifctx;
Aml *field;
@@ -365,13 +366,19 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
aml_append(cpu_ctrl_dev, aml_mutex(CPU_LOCK, 0));
crs = aml_resource_template();
- aml_append(crs, aml_io(AML_DECODE16, io_base, io_base, 1,
+ if (rs == AML_SYSTEM_IO) {
+ aml_append(crs, aml_io(AML_DECODE16, mmap_io_base, mmap_io_base, 1,
ACPI_CPU_HOTPLUG_REG_LEN));
+ } else {
+ aml_append(crs, aml_memory32_fixed(mmap_io_base,
+ ACPI_CPU_HOTPLUG_REG_LEN, AML_READ_WRITE));
+ }
+
aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs));
/* declare CPU hotplug MMIO region with related access fields */
aml_append(cpu_ctrl_dev,
- aml_operation_region("PRST", AML_SYSTEM_IO, aml_int(io_base),
+ aml_operation_region("PRST", rs, aml_int(mmap_io_base),
ACPI_CPU_HOTPLUG_REG_LEN));
field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK,
@@ -593,9 +600,11 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
aml_append(sb_scope, cpus_dev);
aml_append(table, sb_scope);
- method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
- aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
- aml_append(table, method);
+ if (event_handler_method) {
+ method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
+ aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
+ aml_append(table, method);
+ }
g_free(cphp_res_path);
}
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index ca31f70f7f..d40540db61 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -731,7 +731,18 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
* the RTC ACPI device at all when using UEFI.
*/
scope = aml_scope("\\_SB");
- acpi_dsdt_add_cpus(scope, vms->smp_cpus);
+ /* if GED is enabled then cpus AML shall be added as part build_cpus_aml */
+ if (vms->acpi_dev) {
+ CPUHotplugFeatures opts = {
+ .acpi_1_compatible = false,
+ .has_legacy_cphp = false
+ };
+
+ build_cpus_aml(scope, ms, opts, memmap[VIRT_CPUHP_ACPI].base,
+ "\\_SB", NULL, AML_SYSTEM_MEMORY);
+ } else {
+ acpi_dsdt_add_cpus(scope, vms->smp_cpus);
+ }
acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
(irqmap[VIRT_UART] + ARM_SPI_BASE));
acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 473cbdfffd..4b224d80a5 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1668,7 +1668,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
.acpi_1_compatible = true, .has_legacy_cphp = true
};
build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
- "\\_SB.PCI0", "\\_GPE._E02");
+ "\\_SB.PCI0", "\\_GPE._E02", AML_SYSTEM_IO);
}
if (pcms->memhp_io_base && nr_mem) {
diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
index 62f0278ba2..c3a9981dc3 100644
--- a/include/hw/acpi/cpu.h
+++ b/include/hw/acpi/cpu.h
@@ -53,9 +53,10 @@ typedef struct CPUHotplugFeatures {
} CPUHotplugFeatures;
void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
- hwaddr io_base,
+ hwaddr mmap_io_base,
const char *res_root,
- const char *event_handler_method);
+ const char *event_handler_method,
+ AmlRegionSpace rs);
void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list);
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 11/22] arm/cpuhp: Update GED _EVT method AML with cpu scan
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (9 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 10/22] arm/cpuhp: Update CPUs AML with cpu-(ctrl)dev change Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 12/22] arm/cpuhp: MADT Tbl change to size the guest with possible vcpus Salil Mehta
` (14 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
OSPM evaluates _EVT method to map the event. The cpu hotplug event eventually
results in start of the cpu scan. Scan figures out the cpu and the kind of
event(plug/unplug) and notifies it back to the guest.
The change in this patch updates the GED AML _EVT method with the call to
\\_SB.CPUS.CSCN which will do above.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/acpi/generic_event_device.c | 4 ++++
include/hw/acpi/cpu_hotplug.h | 3 +++
2 files changed, 7 insertions(+)
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 0f2c8a959e..79177deda2 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -108,6 +108,10 @@ void build_ged_aml(Aml *table, const char *name, HotplugHandler *hotplug_dev,
aml_append(if_ctx, aml_call0(MEMORY_DEVICES_CONTAINER "."
MEMORY_SLOT_SCAN_METHOD));
break;
+ case ACPI_GED_CPU_HOTPLUG_EVT:
+ aml_append(if_ctx, aml_call0(ACPI_CPU_CONTAINER "."
+ ACPI_CPU_SCAN_METHOD));
+ break;
case ACPI_GED_PWR_DOWN_EVT:
aml_append(if_ctx,
aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE),
diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 48b291e45e..f47fc8e79b 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -20,6 +20,9 @@
#include "hw/acpi/cpu.h"
#define ACPI_CPU_HOTPLUG_REG_LEN 12
+#define ACPI_CPU_SCAN_METHOD "CSCN"
+#define ACPI_CPU_CONTAINER "\\_SB.CPUS"
+
typedef struct AcpiCpuHotplug {
Object *device;
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 12/22] arm/cpuhp: MADT Tbl change to size the guest with possible vcpus
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (10 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 11/22] arm/cpuhp: Update GED _EVT method AML with cpu scan Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 13/22] arm/cpuhp: Add ACPI _MAT entry for Processor object Salil Mehta
` (13 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
Changes required during building of MADT Table by QEMU to accomodate disabled
possible vcpus. This info shall be used by the guest kernel to size up its
resources during boot time. This pre-sizing of the guest kernel done on
possible vcpus will facilitate hotplug of the disabled vcpus.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/arm/virt-acpi-build.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index d40540db61..c654e2c9a3 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -603,6 +603,8 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
AcpiMultipleApicTable *madt;
AcpiMadtGenericDistributor *gicd;
AcpiMadtGenericMsiFrame *gic_msi;
+ MachineState *ms = &vms->parent;
+ CPUArchIdList *possible_cpus = ms->possible_cpus;
int i;
madt = acpi_data_push(table_data, sizeof *madt);
@@ -613,11 +615,10 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
gicd->base_address = cpu_to_le64(memmap[VIRT_GIC_DIST].base);
gicd->version = vms->gic_version;
- for (i = 0; i < vms->smp_cpus; i++) {
+ for (i = 0; i < vms->max_cpus; i++) {
AcpiMadtGenericCpuInterface *gicc = acpi_data_push(table_data,
sizeof(*gicc));
- ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i));
-
+ ARMCPU *cpu = ARM_CPU(qemu_get_possible_cpu(i));
gicc->type = ACPI_APIC_GENERIC_CPU_INTERFACE;
gicc->length = sizeof(*gicc);
if (vms->gic_version == 2) {
@@ -626,11 +627,14 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
gicc->gicv_base_address = cpu_to_le64(memmap[VIRT_GIC_VCPU].base);
}
gicc->cpu_interface_number = cpu_to_le32(i);
- gicc->arm_mpidr = cpu_to_le64(armcpu->mp_affinity);
+ gicc->arm_mpidr = possible_cpus->cpus[i].arch_id;
gicc->uid = cpu_to_le32(i);
- gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED);
-
- if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
+ if ( i < vms->smp_cpus ) {
+ gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED);
+ } else {
+ gicc->flags = cpu_to_le32(0);
+ }
+ if ((cpu && arm_feature(&cpu->env, ARM_FEATURE_PMU)) || vms->pmu) {
gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
}
if (vms->virt) {
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 13/22] arm/cpuhp: Add ACPI _MAT entry for Processor object
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (11 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 12/22] arm/cpuhp: MADT Tbl change to size the guest with possible vcpus Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 14/22] arm/cpuhp: Release objects for *disabled* possible vcpus after init Salil Mehta
` (12 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
Adds a function which builds the ACPI _MAT entry for processor objects. This
shall be called from the cpus AML for all possible vcpus.
The entry is passed to the guest kernel with ACPI_MADT_GICC_ENABLED flag when
it evaluates _MAT object. OSPM evaluates _MAT object in context to the cpu
hotplug event.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/acpi/cpu.c | 5 +++++
hw/arm/virt-acpi-build.c | 20 ++++++++++++++++++++
include/hw/arm/virt.h | 1 +
3 files changed, 26 insertions(+)
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 867fdd6993..a79dc65120 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -565,6 +565,11 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
apic->flags = cpu_to_le32(1);
break;
}
+ case ACPI_APIC_GENERIC_CPU_INTERFACE: {
+ AcpiMadtGenericCpuInterface *gicc = (void *)madt_buf->data;
+ gicc->flags = cpu_to_le32(1);
+ break;
+ }
default:
assert(0);
}
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index c654e2c9a3..354fd775f9 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -593,6 +593,22 @@ build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
}
/* MADT */
+static void
+build_mat_entry(AcpiDeviceIf *adev, int uid, const CPUArchIdList *arch_ids,
+ GArray *entry)
+{
+ AcpiMadtGenericCpuInterface *gicc = acpi_data_push(entry,sizeof(*gicc));
+ MachineState *ms = MACHINE(qdev_get_machine());
+ CPUArchIdList *possible_cpus = ms->possible_cpus;
+
+ /* fill the relevant fields of _MAT entry for GICC */
+ gicc->type = ACPI_APIC_GENERIC_CPU_INTERFACE;
+ gicc->length = sizeof(*gicc);
+ gicc->cpu_interface_number = cpu_to_le32(uid);
+ gicc->arm_mpidr = possible_cpus->cpus[uid].arch_id;
+ gicc->uid = cpu_to_le32(uid);
+}
+
static void
build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
{
@@ -741,6 +757,10 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
.acpi_1_compatible = false,
.has_legacy_cphp = false
};
+ AcpiDeviceIfClass *adevc;
+ /* _MAT entry shall be used within cpus aml */
+ adevc = ACPI_DEVICE_IF_CLASS(DEVICE_GET_CLASS(vms->acpi_dev));
+ adevc->madt_cpu = build_mat_entry;
build_cpus_aml(scope, ms, opts, memmap[VIRT_CPUHP_ACPI].base,
"\\_SB", NULL, AML_SYSTEM_MEMORY);
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index e0bd9df69d..e8468d8cf6 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -37,6 +37,7 @@
#include "hw/block/flash.h"
#include "sysemu/kvm.h"
#include "hw/intc/arm_gicv3_common.h"
+#include "hw/acpi/acpi_dev_interface.h"
#define NUM_GICV2M_SPIS 64
#define NUM_VIRTIO_TRANSPORTS 32
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 14/22] arm/cpuhp: Release objects for *disabled* possible vcpus after init
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (12 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 13/22] arm/cpuhp: Add ACPI _MAT entry for Processor object Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 15/22] arm/cpuhp: Update ACPI GED framework to support vcpu hotplug Salil Mehta
` (11 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
During machvirt_init(), ARMCPU objects are pre-created along with the
corresponding KVM vcpus in the host. Disabled possible KVM vcpus are then
parked at the per-virt-machine list "kvm_parked_vcpus".
Prime purpose to pre-create ARMCPU objects for the disabled vcpus is to
facilitate the GIC initialization (pre-sized with possible vcpus). GIC
requires all vcpus corresponding to its GICC(GIC CPU Interface) to be
initialized and present during its own initialization.
After initialization of the machine is complete we release the ARMCPU objects
for the disabled vcpus(which shall be re-created at the time when vcpu is hot
plugged again. This newly created ARMCPU object is then attached with
corresponding parked KVM VCPU).
We have few options after the machine init where the disabled ARMCPU object
could be released:
1. Release in context to the virt_machine_done() notifier.(This is also our
current approach)
2. Defer the release till a new vcpu object is hot plugged. Then release the
object in context to the pre_plug() phase.
3. Never release and keep on reusing them and release once at VM exit. This
will require some modifications within the interface of qdevice_add() to
get old ARMCPU object instead of creating a new one for the hotplug request.
Each of the above approaches come with their own pros and cons. This prototype
uses the 1st approach.(suggestions are welcome!)
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/arm/virt.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index e9ead0e2dd..0faf54aa8f 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1403,6 +1403,28 @@ static void create_secure_ram(VirtMachineState *vms,
g_free(nodename);
}
+static void virt_remove_disabled_cpus(VirtMachineState *vms)
+{
+ MachineState *ms = MACHINE(vms);
+ int n;
+
+ /*
+ * RFC: Question: Other approach could have been to keep them forever
+ * and release it only once when qemu exits as part o finalize or when
+ * new vcpu is hotplugged. In the later old could be released for the
+ * newly created object for the same vcpu?
+ */
+ for (n = vms->smp_cpus; n < vms->max_cpus; n++) {
+ CPUState *cs = qemu_get_possible_cpu(n);
+ if (!qemu_present_cpu(cs)) {
+ CPUArchId *cpu_slot;
+ cpu_slot = virt_find_cpu_slot(ms, cs->cpu_index);
+ cpu_slot->cpu = NULL;
+ object_unref(OBJECT(cs));
+ }
+ }
+}
+
static bool virt_pmu_init(VirtMachineState *vms)
{
CPUArchIdList *possible_cpus = vms->parent.possible_cpus;
@@ -1500,6 +1522,9 @@ void virt_machine_done(Notifier *notifier, void *data)
virt_acpi_setup(vms);
virt_build_smbios(vms);
+
+ /* release the disabled ARMCPU objects used during init for pre-sizing */
+ virt_remove_disabled_cpus(vms);
}
static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 15/22] arm/cpuhp: Update ACPI GED framework to support vcpu hotplug
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (13 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 14/22] arm/cpuhp: Release objects for *disabled* possible vcpus after init Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 16/22] arm/cpuhp: Add/update basic hot-(un)plug framework Salil Mehta
` (10 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
ACPI GED shall be used to convey to the guest kernel about any cpu hot-(un)plug
events. Therefore, existing ACPI GED framework inside QEMU needs to be enhanced
to support CPU hotplug state and events.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/acpi/generic_event_device.c | 42 ++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 79177deda2..df81e9292a 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -13,7 +13,9 @@
#include "qapi/error.h"
#include "exec/address-spaces.h"
#include "hw/acpi/acpi.h"
+#include "hw/acpi/cpu.h"
#include "hw/acpi/generic_event_device.h"
+#include "hw/arm/virt.h"
#include "hw/irq.h"
#include "hw/mem/pc-dimm.h"
#include "hw/mem/nvdimm.h"
@@ -192,12 +194,47 @@ static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev,
} else {
acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, dev, errp);
}
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+ acpi_cpu_plug_cb(hotplug_dev, &s->cpuhp_state, dev, errp);
} else {
error_setg(errp, "virt: device plug request for unsupported device"
" type: %s", object_get_typename(OBJECT(dev)));
}
}
+static void acpi_ged_device_unplug_request_cb(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+ AcpiGedState *s = ACPI_GED(hotplug_dev);
+
+ if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+ acpi_cpu_unplug_request_cb(hotplug_dev, &s->cpuhp_state, dev, errp);
+ } else {
+ error_setg(errp, "virt: device unplug request for the unsupported device"
+ " type: %s", object_get_typename(OBJECT(dev)));
+ }
+}
+
+static void acpi_ged_device_unplug_cb(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+ AcpiGedState *s = ACPI_GED(hotplug_dev);
+
+ if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+ acpi_cpu_unplug_cb(&s->cpuhp_state, dev, errp);
+ } else {
+ error_setg(errp, "virt: device plug request for unsupported device"
+ " type: %s", object_get_typename(OBJECT(dev)));
+ }
+}
+
+static void acpi_ged_ospm_status(AcpiDeviceIf *adev, ACPIOSTInfoList ***list)
+{
+ AcpiGedState *s = ACPI_GED(adev);
+
+ acpi_cpu_ospm_status(&s->cpuhp_state, list);
+}
+
static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
{
AcpiGedState *s = ACPI_GED(adev);
@@ -210,6 +247,8 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
sel = ACPI_GED_PWR_DOWN_EVT;
} else if (ev & ACPI_NVDIMM_HOTPLUG_STATUS) {
sel = ACPI_GED_NVDIMM_HOTPLUG_EVT;
+ } else if (ev & ACPI_CPU_HOTPLUG_STATUS) {
+ sel = ACPI_GED_CPU_HOTPLUG_EVT;
} else {
/* Unknown event. Return without generating interrupt. */
warn_report("GED: Unsupported event %d. No irq injected", ev);
@@ -330,8 +369,11 @@ static void acpi_ged_class_init(ObjectClass *class, void *data)
dc->vmsd = &vmstate_acpi_ged;
hc->plug = acpi_ged_device_plug_cb;
+ hc->unplug_request = acpi_ged_device_unplug_request_cb;
+ hc->unplug = acpi_ged_device_unplug_cb;
adevc->send_event = acpi_ged_send_event;
+ adevc->ospm_status = acpi_ged_ospm_status;
}
static const TypeInfo acpi_ged_info = {
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 16/22] arm/cpuhp: Add/update basic hot-(un)plug framework
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (14 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 15/22] arm/cpuhp: Update ACPI GED framework to support vcpu hotplug Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 17/22] arm/cpuhp: Changes to (un)wire GICC<->VCPU IRQs during hot-(un)plug Salil Mehta
` (9 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
Adds the new cpu hot-unplug hooks and updates the existing hotplug hooks with
sanity checks.
Note, Functional contents of the hooks(now left with TODO comment) shall be
gradually filled in the subsequent patches in an incremental approach to patch
and logic building which would be roughly as follows:
1. (Un-)wiring of interrupts between vcpu<->gic
2. Sending events to Guest for hot-(un)plug so that guest can take appropriate
actions.
3. Notifying GIC about hot-(un)plug action so that vcpu could be (un-)stitched
to the GIC CPU interface.
4. Updating the Guest with Next boot info for this vcpu in the firmware.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/arm/virt.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 100 insertions(+), 2 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0faf54aa8f..ac2941159a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2324,11 +2324,23 @@ out:
static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
Error **errp)
{
+ VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
MachineState *ms = MACHINE(hotplug_dev);
ARMCPU *cpu = ARM_CPU(dev);
CPUState *cs = CPU(dev);
CPUArchId *cpu_slot;
+ if (dev->hotplugged && !vms->acpi_dev) {
+ error_setg(errp, "GED acpi device does not exists");
+ return;
+ }
+
+ if (dev->hotplugged && (vms->gic_version < VIRT_GIC_VERSION_3)) {
+ error_setg(errp, "CPU hotplug not supported with GICv%d, use GICv3 or "
+ "later", vms->gic_version);
+ return;
+ }
+
/* sanity check the cpu */
if (!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) {
error_setg(errp, "Invalid CPU type, expected cpu type: '%s'",
@@ -2361,6 +2373,10 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
return;
}
virt_cpu_set_properties(OBJECT(cs), cpu_slot);
+
+ if (dev->hotplugged) {
+ /* TODO: update GIC about this hotplug change here */
+ }
}
static void virt_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
@@ -2375,10 +2391,75 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
cpu_slot = virt_find_cpu_slot(ms, cpu->core_id);
cpu_slot->cpu = OBJECT(dev);
+ if (dev->hotplugged) {
+ /* TODO: wire the gic-cpu irqs */
+ /* TODO: update acpi hotplug state and send cpu hotplug event to guest */
+ /* TODO: register this cpu for reset & update F/W info for the next boot */
+ }
+
cs->disabled = false;
return;
}
+static void virt_cpu_unplug_request(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+ VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
+ CPUState *cs = CPU(dev);
+
+ if (!vms->acpi_dev || !dev->realized) {
+ error_setg(errp, "GED does not exists or device is not realized!");
+ return;
+ }
+
+ if (vms->gic_version < VIRT_GIC_VERSION_3) {
+ error_setg(errp, "CPU hot-unplug not supported with GICv%d",
+ vms->gic_version);
+ return;
+ }
+
+ if (cs->cpu_index == first_cpu->cpu_index)
+ {
+ error_setg(errp, "hot-unplug of ARM boot vcpu %d not supported",
+ first_cpu->cpu_index);
+ return;
+ }
+
+ /* TODO: request cpu hotplug from guest */
+
+ return;
+}
+
+static void virt_cpu_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp)
+{
+ VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
+ MachineState *ms = MACHINE(hotplug_dev);
+ CPUState *cs = CPU(dev);
+ CPUArchId *cpu_slot;
+
+ if (!vms->acpi_dev || !dev->realized) {
+ error_setg(errp, "GED does not exists or device is not realized!");
+ return;
+ }
+
+ cpu_slot = virt_find_cpu_slot(ms, ARM_CPU(cs)->core_id);
+
+ /* TODO: update the acpi cpu hotplug state for cpu hot-unplug */
+
+ /* TODO: unwire the gic-cpu irqs here */
+ /* TODO: update the GIC about this hot unplug change */
+
+ /* TODO: unregister this cpu for reset & update F/W info for the next boot */
+
+ qemu_opts_del(dev->opts);
+ dev->opts = NULL;
+
+ cpu_slot->cpu = NULL;
+ cs->disabled = true;
+ return;
+}
+
static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
@@ -2417,8 +2498,23 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
- error_setg(errp, "device unplug request for unsupported device"
- " type: %s", object_get_typename(OBJECT(dev)));
+ if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+ virt_cpu_unplug_request(hotplug_dev, dev, errp);
+ } else {
+ error_setg(errp, "device unplug request for unsupported type: %s",
+ object_get_typename(OBJECT(dev)));
+ }
+}
+
+static void virt_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+ if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+ virt_cpu_unplug(hotplug_dev, dev, errp);
+ } else {
+ error_setg(errp, "device unplug for unsupported type: %s",
+ object_get_typename(OBJECT(dev)));
+ }
}
static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
@@ -2535,11 +2631,13 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
mc->kvm_type = virt_kvm_type;
+ mc->has_hotpluggable_cpus = true;
assert(!mc->get_hotplug_handler);
mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
hc->pre_plug = virt_machine_device_pre_plug_cb;
hc->plug = virt_machine_device_plug_cb;
hc->unplug_request = virt_machine_device_unplug_request_cb;
+ hc->unplug = virt_machine_device_unplug_cb;
mc->numa_mem_supported = true;
mc->nvdimm_supported = true;
mc->auto_enable_numa_with_memhp = true;
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 17/22] arm/cpuhp: Changes to (un)wire GICC<->VCPU IRQs during hot-(un)plug
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (15 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 16/22] arm/cpuhp: Add/update basic hot-(un)plug framework Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 18/22] arm/cpuhp: Changes to update GIC with vcpu hot-plug notification Salil Mehta
` (8 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
Refactors the existing gic create code to extract common code to wire the
vcpu<->gic interrupts. This function could be used with cold-plug case and also
used when vcpu is hot-plugged. It also introduces a new function to unwire the
vcpu>->gic interrupts for the vcpu hot-unplug cases.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/arm/virt.c | 144 +++++++++++++++++++++++++++++------------
hw/core/qdev.c | 2 +-
include/hw/qdev-core.h | 2 +
3 files changed, 104 insertions(+), 44 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ac2941159a..f0295e940e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -615,6 +615,103 @@ static void create_v2m(VirtMachineState *vms)
fdt_add_v2m_gic_node(vms);
}
+static void unwire_gic_cpu_irqs(VirtMachineState *vms, CPUState *cs)
+{
+ unsigned int max_cpus = vms->max_cpus;
+ DeviceState *cpudev = DEVICE(cs);
+ DeviceState *gicdev = vms->gic;
+ int cpu = CPU(cs)->cpu_index;
+ int type = vms->gic_version;
+ int irq;
+
+ /* Mapping from the output timer irq lines from the CPU to the
+ * GIC PPI inputs we use for the virt board.
+ */
+ const int timer_irq[] = {
+ [GTIMER_PHYS] = ARCH_TIMER_NS_EL1_IRQ,
+ [GTIMER_VIRT] = ARCH_TIMER_VIRT_IRQ,
+ [GTIMER_HYP] = ARCH_TIMER_NS_EL2_IRQ,
+ [GTIMER_SEC] = ARCH_TIMER_S_EL1_IRQ,
+ };
+
+ for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
+ qdev_disconnect_gpio_out_named(cpudev, NULL, irq);
+ }
+
+ if (type == 3) {
+ qdev_disconnect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0);
+ } else if (vms->virt) {
+ qdev_disconnect_gpio_out_named(gicdev, SYSBUS_DEVICE_GPIO_IRQ, cpu + 4 * max_cpus);
+ }
+
+ /*
+ * RFC: Question: This currently does not takes care of intimating the devices
+ * which might be sitting on system bus. Do we need a sysbus_disconnect_irq()
+ * which also does the job of notification beside disconnection?
+ */
+ qdev_disconnect_gpio_out_named(cpudev, "pmu-interrupt", 0);
+ qdev_disconnect_gpio_out_named(gicdev, SYSBUS_DEVICE_GPIO_IRQ, cpu);
+ qdev_disconnect_gpio_out_named(gicdev,
+ SYSBUS_DEVICE_GPIO_IRQ, cpu + max_cpus);
+ qdev_disconnect_gpio_out_named(gicdev, SYSBUS_DEVICE_GPIO_IRQ,
+ cpu + 2 * max_cpus);
+ qdev_disconnect_gpio_out_named(gicdev, SYSBUS_DEVICE_GPIO_IRQ,
+ cpu + 3 * max_cpus);
+}
+
+static void wire_gic_cpu_irqs(VirtMachineState *vms, CPUState *cs)
+{
+ unsigned int max_cpus = vms->max_cpus;
+ DeviceState *cpudev = DEVICE(cs);
+ DeviceState *gicdev = vms->gic;
+ int cpu = CPU(cs)->cpu_index;
+ int type = vms->gic_version;
+ SysBusDevice *gicbusdev;
+ int ppibase;
+ int irq;
+
+ ppibase = NUM_IRQS + cpu * GIC_INTERNAL + GIC_NR_SGIS;
+
+ /* Mapping from the output timer irq lines from the CPU to the
+ * GIC PPI inputs we use for the virt board.
+ */
+ const int timer_irq[] = {
+ [GTIMER_PHYS] = ARCH_TIMER_NS_EL1_IRQ,
+ [GTIMER_VIRT] = ARCH_TIMER_VIRT_IRQ,
+ [GTIMER_HYP] = ARCH_TIMER_NS_EL2_IRQ,
+ [GTIMER_SEC] = ARCH_TIMER_S_EL1_IRQ,
+ };
+
+ for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
+ qdev_connect_gpio_out(cpudev, irq,
+ qdev_get_gpio_in(gicdev,
+ ppibase + timer_irq[irq]));
+ }
+
+ gicbusdev = SYS_BUS_DEVICE(gicdev);
+ if (type == 3) {
+ qemu_irq irq = qdev_get_gpio_in(gicdev,
+ ppibase + ARCH_GIC_MAINT_IRQ);
+ qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt",
+ 0, irq);
+ } else if (vms->virt) {
+ qemu_irq irq = qdev_get_gpio_in(gicdev,
+ ppibase + ARCH_GIC_MAINT_IRQ);
+ sysbus_connect_irq(gicbusdev, cpu + 4 * max_cpus, irq);
+ }
+
+ qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0,
+ qdev_get_gpio_in(gicdev,
+ ppibase + VIRTUAL_PMU_IRQ));
+ sysbus_connect_irq(gicbusdev, cpu, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
+ sysbus_connect_irq(gicbusdev, cpu + max_cpus,
+ qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
+ sysbus_connect_irq(gicbusdev, cpu + 2 * max_cpus,
+ qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
+ sysbus_connect_irq(gicbusdev, cpu + 3 * max_cpus,
+ qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
+}
+
static void create_gic(VirtMachineState *vms)
{
/* We create a standalone GIC */
@@ -684,47 +781,7 @@ static void create_gic(VirtMachineState *vms)
* and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs.
*/
for (i = 0; i < smp_cpus; i++) {
- DeviceState *cpudev = DEVICE(qemu_get_cpu(i));
- int ppibase = NUM_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
- int irq;
- /* Mapping from the output timer irq lines from the CPU to the
- * GIC PPI inputs we use for the virt board.
- */
- const int timer_irq[] = {
- [GTIMER_PHYS] = ARCH_TIMER_NS_EL1_IRQ,
- [GTIMER_VIRT] = ARCH_TIMER_VIRT_IRQ,
- [GTIMER_HYP] = ARCH_TIMER_NS_EL2_IRQ,
- [GTIMER_SEC] = ARCH_TIMER_S_EL1_IRQ,
- };
-
- for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
- qdev_connect_gpio_out(cpudev, irq,
- qdev_get_gpio_in(vms->gic,
- ppibase + timer_irq[irq]));
- }
-
- if (type == 3) {
- qemu_irq irq = qdev_get_gpio_in(vms->gic,
- ppibase + ARCH_GIC_MAINT_IRQ);
- qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt",
- 0, irq);
- } else if (vms->virt) {
- qemu_irq irq = qdev_get_gpio_in(vms->gic,
- ppibase + ARCH_GIC_MAINT_IRQ);
- sysbus_connect_irq(gicbusdev, i + 4 * max_cpus, irq);
- }
-
- qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0,
- qdev_get_gpio_in(vms->gic, ppibase
- + VIRTUAL_PMU_IRQ));
-
- sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
- sysbus_connect_irq(gicbusdev, i + max_cpus,
- qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
- sysbus_connect_irq(gicbusdev, i + 2 * max_cpus,
- qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
- sysbus_connect_irq(gicbusdev, i + 3 * max_cpus,
- qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
+ wire_gic_cpu_irqs(vms, qemu_get_cpu(i));
}
fdt_add_gic_node(vms);
@@ -2382,6 +2439,7 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
static void virt_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
Error **errp)
{
+ VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
MachineState *ms = MACHINE(hotplug_dev);
ARMCPU *cpu = ARM_CPU(dev);
CPUState *cs = CPU(dev);
@@ -2392,7 +2450,7 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
cpu_slot->cpu = OBJECT(dev);
if (dev->hotplugged) {
- /* TODO: wire the gic-cpu irqs */
+ wire_gic_cpu_irqs(vms, cs);
/* TODO: update acpi hotplug state and send cpu hotplug event to guest */
/* TODO: register this cpu for reset & update F/W info for the next boot */
}
@@ -2447,7 +2505,7 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
/* TODO: update the acpi cpu hotplug state for cpu hot-unplug */
- /* TODO: unwire the gic-cpu irqs here */
+ unwire_gic_cpu_irqs(vms, cs);
/* TODO: update the GIC about this hot unplug change */
/* TODO: unregister this cpu for reset & update F/W info for the next boot */
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 9e5538aeae..65b3ec7c8f 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -563,7 +563,7 @@ qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n)
/* disconnect a GPIO output, returning the disconnected input (if any) */
-static qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
+qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
const char *name, int n)
{
char *propname = g_strdup_printf("%s[%d]",
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index b870b27966..8434cc5a3e 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -355,6 +355,8 @@ void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n);
qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt,
const char *name, int n);
+qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
+ const char *name, int n);
BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 18/22] arm/cpuhp: Changes to update GIC with vcpu hot-plug notification
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (16 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 17/22] arm/cpuhp: Changes to (un)wire GICC<->VCPU IRQs during hot-(un)plug Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 19/22] arm/cpuhp: Changes required to (re)init the vcpu register info Salil Mehta
` (7 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
Adds the notification support about vcpu hot-(un)plug required to update the
GIC so that it can update its vcpu to GIC cpu interface association.
NOTE: This is using 'struct VirtMachineState' inside the notifier function.
Question: Not sure if it is right to use machine related data structure
inside GIC related files? Its design looks to be pretty much abstracted
from any machine related stuff. @Peter Maydell
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/arm/virt.c | 12 +++++--
hw/intc/arm_gicv3_common.c | 54 ++++++++++++++++++++++++++++++
hw/intc/arm_gicv3_cpuif.c | 5 +++
hw/intc/gicv3_internal.h | 1 +
include/hw/arm/virt.h | 1 +
include/hw/intc/arm_gicv3_common.h | 1 +
6 files changed, 72 insertions(+), 2 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index f0295e940e..b4cfd53a59 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1928,6 +1928,8 @@ static void machvirt_init(MachineState *machine)
create_fdt(vms);
+ notifier_list_init(&vms->cpuhp_notifiers);
+
possible_cpus = mc->possible_cpu_arch_ids(machine);
for (n = 0; n < possible_cpus->len; n++) {
Object *cpuobj;
@@ -2378,6 +2380,12 @@ out:
error_propagate(errp, local_err);
}
+static void virt_update_gic(VirtMachineState *vms, CPUState *cs)
+{
+ /* notify gic to stitch GICC to this new cpu */
+ notifier_list_notify(&vms->cpuhp_notifiers, cs);
+}
+
static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
Error **errp)
{
@@ -2432,7 +2440,7 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
virt_cpu_set_properties(OBJECT(cs), cpu_slot);
if (dev->hotplugged) {
- /* TODO: update GIC about this hotplug change here */
+ virt_update_gic(vms, cs);
}
}
@@ -2506,7 +2514,7 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
/* TODO: update the acpi cpu hotplug state for cpu hot-unplug */
unwire_gic_cpu_irqs(vms, cs);
- /* TODO: update the GIC about this hot unplug change */
+ virt_update_gic(vms, cs);
/* TODO: unregister this cpu for reset & update F/W info for the next boot */
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index bfa514444a..f6b7b359cb 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -31,6 +31,7 @@
#include "gicv3_internal.h"
#include "hw/arm/linux-boot-if.h"
#include "sysemu/kvm.h"
+#include "hw/arm/virt.h"
static void gicv3_gicd_no_migration_shift_bug_post_load(GICv3State *cs)
@@ -305,8 +306,57 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler,
}
}
+static int arm_gicv3_get_proc_num(GICv3State *s, CPUState *cpu)
+{
+ uint64_t mp_affinity;
+ uint64_t gicr_typer;
+ uint64_t cpu_affid;
+ int i;
+
+ mp_affinity = object_property_get_uint(OBJECT(cpu), "mp-affinity", NULL);
+ /* match the cpu mp-affinity to get the gic cpuif number */
+ for (i = 0; i < s->num_cpu; i++) {
+ gicr_typer = s->cpu[i].gicr_typer;
+ cpu_affid = (gicr_typer >> 32) & 0xFFFFFF;
+ if (cpu_affid == mp_affinity) {
+ return i;
+ }
+ }
+
+ return -1;
+}
+
+static void arm_gicv3_cpu_update_notifier(Notifier * notifier, void * data)
+{
+ VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine());
+ GICv3State *s = ARM_GICV3_COMMON(vms->gic);
+ CPUState *cpu = (CPUState *)data;
+ int gic_cpuif_num;
+
+ /* this shall get us mapped gicv3 cpuif corresponding to mpidr */
+ gic_cpuif_num = arm_gicv3_get_proc_num(s, cpu);
+ if (gic_cpuif_num < 0) {
+ error_report("Failed to associate cpu %d with any GIC cpuif",
+ cpu->cpu_index);
+ abort();
+ }
+
+ /* check if update is for vcpu hot-unplug */
+ if (qemu_present_cpu(cpu)) {
+ s->cpu[gic_cpuif_num].cpu = NULL;
+ return;
+ }
+
+ /* re-stitch the gic cpuif to this new cpu */
+ gicv3_set_gicv3state(cpu, &s->cpu[gic_cpuif_num]);
+ gicv3_set_cpustate(&s->cpu[gic_cpuif_num], cpu);
+
+ /* TODO: initialize the registers info for this newly added cpu */
+}
+
static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
{
+ VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine());
GICv3State *s = ARM_GICV3_COMMON(dev);
int i;
@@ -386,12 +436,16 @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
(i << 8) |
(last << 4);
}
+
+ s->cpu_update_notifier.notify = arm_gicv3_cpu_update_notifier;
+ notifier_list_add(&vms->cpuhp_notifiers, &s->cpu_update_notifier);
}
static void arm_gicv3_finalize(Object *obj)
{
GICv3State *s = ARM_GICV3_COMMON(obj);
+ notifier_remove(&s->cpu_update_notifier);
g_free(s->redist_region_count);
}
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index 90d8b0118e..b3aa5979ca 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -20,6 +20,11 @@
#include "hw/irq.h"
#include "cpu.h"
+void gicv3_set_cpustate(GICv3CPUState *s, CPUState *cpu)
+{
+ s->cpu = cpu;
+}
+
void gicv3_set_gicv3state(CPUState *cpu, GICv3CPUState *s)
{
ARMCPU *arm_cpu = ARM_CPU(cpu);
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 05303a55c8..6e14a7a6cd 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -409,5 +409,6 @@ static inline void gicv3_cache_all_target_cpustates(GICv3State *s)
}
void gicv3_set_gicv3state(CPUState *cpu, GICv3CPUState *s);
+void gicv3_set_cpustate(GICv3CPUState *s, CPUState *cpu);
#endif /* QEMU_ARM_GICV3_INTERNAL_H */
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index e8468d8cf6..c287433219 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -158,6 +158,7 @@ typedef struct {
DeviceState *gic;
DeviceState *acpi_dev;
Notifier powerdown_notifier;
+ NotifierList cpuhp_notifiers;
} VirtMachineState;
#define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index 31ec9a1ae4..b51f74c728 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -248,6 +248,7 @@ struct GICv3State {
GICv3CPUState *gicd_irouter_target[GICV3_MAXIRQ];
uint32_t gicd_nsacr[DIV_ROUND_UP(GICV3_MAXIRQ, 16)];
+ Notifier cpu_update_notifier;
GICv3CPUState *cpu;
};
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 19/22] arm/cpuhp: Changes required to (re)init the vcpu register info
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (17 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 18/22] arm/cpuhp: Changes to update GIC with vcpu hot-plug notification Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 20/22] arm/cpuhp: Update the guest(via GED) about cpu hot-(un)plug events Salil Mehta
` (6 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
VCPU register info needs to be re-initialized each time vcpu is hot-plugged.
This has to be done both for emulation/TCG and KVM case. This is done in
context to the GIC update notification for any vcpu hot-(un)plug events. This
change adds that support and re-factors existing to maximize the code re-use.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/intc/arm_gicv3.c | 1 +
hw/intc/arm_gicv3_common.c | 6 +-
hw/intc/arm_gicv3_cpuif.c | 134 ++++++++++++++++-------------
hw/intc/arm_gicv3_kvm.c | 7 +-
hw/intc/gicv3_internal.h | 1 +
include/hw/intc/arm_gicv3_common.h | 1 +
6 files changed, 86 insertions(+), 64 deletions(-)
diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
index 66eaa97198..86f2f4cca7 100644
--- a/hw/intc/arm_gicv3.c
+++ b/hw/intc/arm_gicv3.c
@@ -396,6 +396,7 @@ static void arm_gicv3_class_init(ObjectClass *klass, void *data)
ARMGICv3Class *agc = ARM_GICV3_CLASS(klass);
agcc->post_load = arm_gicv3_post_load;
+ agcc->init_cpu_reginfo = gicv3_init_cpu_reginfo;
device_class_set_parent_realize(dc, arm_gic_realize, &agc->parent_realize);
}
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index f6b7b359cb..01e4aba4f1 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -330,6 +330,7 @@ static void arm_gicv3_cpu_update_notifier(Notifier * notifier, void * data)
{
VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine());
GICv3State *s = ARM_GICV3_COMMON(vms->gic);
+ ARMGICv3CommonClass *c = ARM_GICV3_COMMON_GET_CLASS(s);
CPUState *cpu = (CPUState *)data;
int gic_cpuif_num;
@@ -351,7 +352,10 @@ static void arm_gicv3_cpu_update_notifier(Notifier * notifier, void * data)
gicv3_set_gicv3state(cpu, &s->cpu[gic_cpuif_num]);
gicv3_set_cpustate(&s->cpu[gic_cpuif_num], cpu);
- /* TODO: initialize the registers info for this newly added cpu */
+ /* initialize the registers info for this newly added cpu */
+ if (c->init_cpu_reginfo) {
+ c->init_cpu_reginfo(cpu);
+ }
}
static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index b3aa5979ca..1126fffa55 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -2605,6 +2605,74 @@ static const ARMCPRegInfo gicv3_cpuif_ich_apxr23_reginfo[] = {
REGINFO_SENTINEL
};
+void gicv3_init_cpu_reginfo(CPUState *cs)
+{
+ ARMCPU *cpu = ARM_CPU(cs);
+ GICv3CPUState *gcs = icc_cs_from_env(&cpu->env);
+
+ /* Note that we can't just use the GICv3CPUState as an opaque pointer
+ * in define_arm_cp_regs_with_opaque(), because when we're called back
+ * it might be with code translated by CPU 0 but run by CPU 1, in
+ * which case we'd get the wrong value.
+ * So instead we define the regs with no ri->opaque info, and
+ * get back to the GICv3CPUState from the CPUARMState.
+ */
+ define_arm_cp_regs(cpu, gicv3_cpuif_reginfo);
+ if (arm_feature(&cpu->env, ARM_FEATURE_EL2)
+ && cpu->gic_num_lrs) {
+ int j;
+
+ gcs->maintenance_irq = cpu->gicv3_maintenance_interrupt;
+
+ gcs->num_list_regs = cpu->gic_num_lrs;
+ gcs->vpribits = cpu->gic_vpribits;
+ gcs->vprebits = cpu->gic_vprebits;
+
+ /* Check against architectural constraints: getting these
+ * wrong would be a bug in the CPU code defining these,
+ * and the implementation relies on them holding.
+ */
+ g_assert(gcs->vprebits <= gcs->vpribits);
+ g_assert(gcs->vprebits >= 5 && gcs->vprebits <= 7);
+ g_assert(gcs->vpribits >= 5 && gcs->vpribits <= 8);
+
+ define_arm_cp_regs(cpu, gicv3_cpuif_hcr_reginfo);
+
+ for (j = 0; j < gcs->num_list_regs; j++) {
+ /* Note that the AArch64 LRs are 64-bit; the AArch32 LRs
+ * are split into two cp15 regs, LR (the low part, with the
+ * same encoding as the AArch64 LR) and LRC (the high part).
+ */
+ ARMCPRegInfo lr_regset[] = {
+ { .name = "ICH_LRn_EL2", .state = ARM_CP_STATE_BOTH,
+ .opc0 = 3, .opc1 = 4, .crn = 12,
+ .crm = 12 + (j >> 3), .opc2 = j & 7,
+ .type = ARM_CP_IO | ARM_CP_NO_RAW,
+ .access = PL2_RW,
+ .readfn = ich_lr_read,
+ .writefn = ich_lr_write,
+ },
+ { .name = "ICH_LRCn_EL2", .state = ARM_CP_STATE_AA32,
+ .cp = 15, .opc1 = 4, .crn = 12,
+ .crm = 14 + (j >> 3), .opc2 = j & 7,
+ .type = ARM_CP_IO | ARM_CP_NO_RAW,
+ .access = PL2_RW,
+ .readfn = ich_lr_read,
+ .writefn = ich_lr_write,
+ },
+ REGINFO_SENTINEL
+ };
+ define_arm_cp_regs(cpu, lr_regset);
+ }
+ if (gcs->vprebits >= 6) {
+ define_arm_cp_regs(cpu, gicv3_cpuif_ich_apxr1_reginfo);
+ }
+ if (gcs->vprebits == 7) {
+ define_arm_cp_regs(cpu, gicv3_cpuif_ich_apxr23_reginfo);
+ }
+ }
+}
+
static void gicv3_cpuif_el_change_hook(ARMCPU *cpu, void *opaque)
{
GICv3CPUState *cs = opaque;
@@ -2621,69 +2689,11 @@ void gicv3_init_cpuif(GICv3State *s)
for (i = 0; i < s->num_cpu; i++) {
ARMCPU *cpu = ARM_CPU(qemu_get_cpu(i));
- GICv3CPUState *cs = &s->cpu[i];
-
- /* Note that we can't just use the GICv3CPUState as an opaque pointer
- * in define_arm_cp_regs_with_opaque(), because when we're called back
- * it might be with code translated by CPU 0 but run by CPU 1, in
- * which case we'd get the wrong value.
- * So instead we define the regs with no ri->opaque info, and
- * get back to the GICv3CPUState from the CPUARMState.
- */
- define_arm_cp_regs(cpu, gicv3_cpuif_reginfo);
- if (arm_feature(&cpu->env, ARM_FEATURE_EL2)
- && cpu->gic_num_lrs) {
- int j;
- cs->maintenance_irq = cpu->gicv3_maintenance_interrupt;
-
- cs->num_list_regs = cpu->gic_num_lrs;
- cs->vpribits = cpu->gic_vpribits;
- cs->vprebits = cpu->gic_vprebits;
-
- /* Check against architectural constraints: getting these
- * wrong would be a bug in the CPU code defining these,
- * and the implementation relies on them holding.
- */
- g_assert(cs->vprebits <= cs->vpribits);
- g_assert(cs->vprebits >= 5 && cs->vprebits <= 7);
- g_assert(cs->vpribits >= 5 && cs->vpribits <= 8);
-
- define_arm_cp_regs(cpu, gicv3_cpuif_hcr_reginfo);
-
- for (j = 0; j < cs->num_list_regs; j++) {
- /* Note that the AArch64 LRs are 64-bit; the AArch32 LRs
- * are split into two cp15 regs, LR (the low part, with the
- * same encoding as the AArch64 LR) and LRC (the high part).
- */
- ARMCPRegInfo lr_regset[] = {
- { .name = "ICH_LRn_EL2", .state = ARM_CP_STATE_BOTH,
- .opc0 = 3, .opc1 = 4, .crn = 12,
- .crm = 12 + (j >> 3), .opc2 = j & 7,
- .type = ARM_CP_IO | ARM_CP_NO_RAW,
- .access = PL2_RW,
- .readfn = ich_lr_read,
- .writefn = ich_lr_write,
- },
- { .name = "ICH_LRCn_EL2", .state = ARM_CP_STATE_AA32,
- .cp = 15, .opc1 = 4, .crn = 12,
- .crm = 14 + (j >> 3), .opc2 = j & 7,
- .type = ARM_CP_IO | ARM_CP_NO_RAW,
- .access = PL2_RW,
- .readfn = ich_lr_read,
- .writefn = ich_lr_write,
- },
- REGINFO_SENTINEL
- };
- define_arm_cp_regs(cpu, lr_regset);
- }
- if (cs->vprebits >= 6) {
- define_arm_cp_regs(cpu, gicv3_cpuif_ich_apxr1_reginfo);
- }
- if (cs->vprebits == 7) {
- define_arm_cp_regs(cpu, gicv3_cpuif_ich_apxr23_reginfo);
- }
+ if (qemu_present_cpu(CPU(cpu))) {
+ GICv3CPUState *cs = icc_cs_from_env(&cpu->env);
+ gicv3_init_cpu_reginfo(CPU(cpu));
+ arm_register_el_change_hook(cpu, gicv3_cpuif_el_change_hook, cs);
}
- arm_register_el_change_hook(cpu, gicv3_cpuif_el_change_hook, cs);
}
}
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 7fe000e53c..8d19af3974 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -781,6 +781,10 @@ static void vm_change_state_handler(void *opaque, int running,
}
}
+static void kvm_gicv3_init_cpu_reginfo(CPUState *cs)
+{
+ define_arm_cp_regs(ARM_CPU(cs), gicv3_cpuif_reginfo);
+}
static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
{
@@ -813,7 +817,7 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
for (i = 0; i < s->num_cpu; i++) {
CPUState *cs = qemu_get_cpu(i);
if (qemu_present_cpu(cs))
- define_arm_cp_regs(ARM_CPU(cs), gicv3_cpuif_reginfo);
+ kvm_gicv3_init_cpu_reginfo(cs);
}
/* Try to create the device via the device control API */
@@ -902,6 +906,7 @@ static void kvm_arm_gicv3_class_init(ObjectClass *klass, void *data)
agcc->pre_save = kvm_arm_gicv3_get;
agcc->post_load = kvm_arm_gicv3_put;
+ agcc->init_cpu_reginfo = kvm_gicv3_init_cpu_reginfo;
device_class_set_parent_realize(dc, kvm_arm_gicv3_realize,
&kgc->parent_realize);
device_class_set_parent_reset(dc, kvm_arm_gicv3_reset, &kgc->parent_reset);
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 6e14a7a6cd..66eec3c3fc 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -298,6 +298,7 @@ void gicv3_dist_set_irq(GICv3State *s, int irq, int level);
void gicv3_redist_set_irq(GICv3CPUState *cs, int irq, int level);
void gicv3_redist_send_sgi(GICv3CPUState *cs, int grp, int irq, bool ns);
void gicv3_init_cpuif(GICv3State *s);
+void gicv3_init_cpu_reginfo(CPUState *cs);
/**
* gicv3_cpuif_update:
diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index b51f74c728..4d1cbc66db 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -294,6 +294,7 @@ typedef struct ARMGICv3CommonClass {
void (*pre_save)(GICv3State *s);
void (*post_load)(GICv3State *s);
+ void (*init_cpu_reginfo)(CPUState *cs);
} ARMGICv3CommonClass;
void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler,
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 20/22] arm/cpuhp: Update the guest(via GED) about cpu hot-(un)plug events
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (18 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 19/22] arm/cpuhp: Changes required to (re)init the vcpu register info Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 21/22] arm/cpuhp: Changes required for reset and to support next boot Salil Mehta
` (5 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
During any vcpu hot-(un)plug, running guest VM needs to be intimated about the
new vcpu being added or request the deletion of the vcpu which is already part
of the guest VM. This is done using the ACPI GED event which eventually gets
demultiplexed to a CPU hotplug event and further to specific hot-(un)plug event
of a particular vcpu.
This change adds the ACPI calls to the existing hot-(un)plug hooks to trigger
ACPI GED events from QEMU to guest VM.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/arm/virt.c | 32 +++++++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b4cfd53a59..db7eca1b84 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2451,6 +2451,7 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
MachineState *ms = MACHINE(hotplug_dev);
ARMCPU *cpu = ARM_CPU(dev);
CPUState *cs = CPU(dev);
+ Error *local_err = NULL;
CPUArchId *cpu_slot;
/* insert the cold/hot-plugged vcpu in the slot */
@@ -2458,20 +2459,31 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
cpu_slot->cpu = OBJECT(dev);
if (dev->hotplugged) {
+ HotplugHandlerClass *hhc;
+
wire_gic_cpu_irqs(vms, cs);
- /* TODO: update acpi hotplug state and send cpu hotplug event to guest */
+
+ /* update acpi hotplug state and send cpu hotplug event to guest */
+ hhc = HOTPLUG_HANDLER_GET_CLASS(vms->acpi_dev);
+ hhc->plug(HOTPLUG_HANDLER(vms->acpi_dev), dev, &local_err);
+ if (local_err)
+ goto fail;
/* TODO: register this cpu for reset & update F/W info for the next boot */
}
cs->disabled = false;
return;
+fail:
+ error_propagate(errp, local_err);
}
static void virt_cpu_unplug_request(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
+ HotplugHandlerClass *hhc;
CPUState *cs = CPU(dev);
+ Error *local_err = NULL;
if (!vms->acpi_dev || !dev->realized) {
error_setg(errp, "GED does not exists or device is not realized!");
@@ -2491,9 +2503,15 @@ static void virt_cpu_unplug_request(HotplugHandler *hotplug_dev,
return;
}
- /* TODO: request cpu hotplug from guest */
+ /* request cpu hotplug from guest */
+ hhc = HOTPLUG_HANDLER_GET_CLASS(vms->acpi_dev);
+ hhc->unplug_request(HOTPLUG_HANDLER(vms->acpi_dev), dev, &local_err);
+ if (local_err)
+ goto fail;
return;
+fail:
+ error_propagate(errp, local_err);
}
static void virt_cpu_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
@@ -2501,7 +2519,9 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
{
VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
MachineState *ms = MACHINE(hotplug_dev);
+ HotplugHandlerClass *hhc;
CPUState *cs = CPU(dev);
+ Error *local_err = NULL;
CPUArchId *cpu_slot;
if (!vms->acpi_dev || !dev->realized) {
@@ -2511,7 +2531,11 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
cpu_slot = virt_find_cpu_slot(ms, ARM_CPU(cs)->core_id);
- /* TODO: update the acpi cpu hotplug state for cpu hot-unplug */
+ /* update the acpi cpu hotplug state for cpu hot-unplug */
+ hhc = HOTPLUG_HANDLER_GET_CLASS(vms->acpi_dev);
+ hhc->unplug(HOTPLUG_HANDLER(vms->acpi_dev), dev, &local_err);
+ if (local_err)
+ goto fail;
unwire_gic_cpu_irqs(vms, cs);
virt_update_gic(vms, cs);
@@ -2524,6 +2548,8 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
cpu_slot->cpu = NULL;
cs->disabled = true;
return;
+fail:
+ error_propagate(errp, local_err);
}
static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 21/22] arm/cpuhp: Changes required for reset and to support next boot
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (19 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 20/22] arm/cpuhp: Update the guest(via GED) about cpu hot-(un)plug events Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 22/22] arm/cpuhp: Add support of *unrealize* ARMCPU during vcpu hot-unplug Salil Mehta
` (4 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
Updates the firmware config with the next boot cpus information and also
registers the reset callback to be called when guest reboots to reset the cpu.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
hw/arm/boot.c | 2 +-
hw/arm/virt.c | 18 ++++++++++++++----
include/hw/arm/boot.h | 2 ++
include/hw/arm/virt.h | 1 +
4 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index fef4072db1..05f329c1e1 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -675,7 +675,7 @@ fail:
return -1;
}
-static void do_cpu_reset(void *opaque)
+void do_cpu_reset(void *opaque)
{
ARMCPU *cpu = opaque;
CPUState *cs = CPU(cpu);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index db7eca1b84..55101c0050 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -47,6 +47,7 @@
#include "sysemu/device_tree.h"
#include "sysemu/numa.h"
#include "sysemu/runstate.h"
+#include "sysemu/reset.h"
#include "sysemu/sysemu.h"
#include "sysemu/tpm.h"
#include "sysemu/kvm.h"
@@ -1149,14 +1150,13 @@ static bool virt_firmware_init(VirtMachineState *vms,
static FWCfgState *create_fw_cfg(const VirtMachineState *vms, AddressSpace *as)
{
- MachineState *ms = MACHINE(vms);
hwaddr base = vms->memmap[VIRT_FW_CFG].base;
hwaddr size = vms->memmap[VIRT_FW_CFG].size;
FWCfgState *fw_cfg;
char *nodename;
fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16, as);
- fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)ms->smp.cpus);
+ fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, vms->boot_cpus);
nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
qemu_fdt_add_subnode(vms->fdt, nodename);
@@ -2468,7 +2468,13 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
hhc->plug(HOTPLUG_HANDLER(vms->acpi_dev), dev, &local_err);
if (local_err)
goto fail;
- /* TODO: register this cpu for reset & update F/W info for the next boot */
+
+ /* register this cpu for reset & update F/W info for the next boot */
+ qemu_register_reset(do_cpu_reset, ARM_CPU(cs));
+ vms->boot_cpus++;
+ if (vms->fw_cfg) {
+ fw_cfg_modify_i16(vms->fw_cfg, FW_CFG_NB_CPUS, vms->boot_cpus);
+ }
}
cs->disabled = false;
@@ -2540,7 +2546,11 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
unwire_gic_cpu_irqs(vms, cs);
virt_update_gic(vms, cs);
- /* TODO: unregister this cpu for reset & update F/W info for the next boot */
+ qemu_unregister_reset(do_cpu_reset, ARM_CPU(cs));
+ vms->boot_cpus--;
+ if (vms->fw_cfg) {
+ fw_cfg_modify_i16(vms->fw_cfg, FW_CFG_NB_CPUS, vms->boot_cpus);
+ }
qemu_opts_del(dev->opts);
dev->opts = NULL;
diff --git a/include/hw/arm/boot.h b/include/hw/arm/boot.h
index ce2b48b88b..aa156967af 100644
--- a/include/hw/arm/boot.h
+++ b/include/hw/arm/boot.h
@@ -163,6 +163,8 @@ AddressSpace *arm_boot_address_space(ARMCPU *cpu,
int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
hwaddr addr_limit, AddressSpace *as, MachineState *ms);
+void do_cpu_reset(void *opaque);
+
/* Write a secure board setup routine with a dummy handler for SMCs */
void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
const struct arm_boot_info *info,
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index c287433219..df785ea6ba 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -147,6 +147,7 @@ typedef struct {
const int *irqmap;
int smp_cpus;
int max_cpus;
+ uint16_t boot_cpus;
void *fdt;
int fdt_size;
uint32_t clock_phandle;
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH RFC 22/22] arm/cpuhp: Add support of *unrealize* ARMCPU during vcpu hot-unplug
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (20 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 21/22] arm/cpuhp: Changes required for reset and to support next boot Salil Mehta
@ 2020-06-13 21:36 ` Salil Mehta
2020-06-13 22:24 ` [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch no-reply
` (3 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-13 21:36 UTC (permalink / raw)
To: qemu-devel, qemu-arm
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
maz, zhukeqian1, david, richard.henderson, linuxarm, eric.auger,
james.morse, catalin.marinas, imammedo, Salil Mehta, pbonzini,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng2
During vcpu hot-unplug ARM cpu unrealization shall happen which should do away
with all the vcpu thread creations, allocations, registrations which happened
as part of the realization process of the ARM cpu. This change introduces the
ARM cpu unrealize function taking care of exactly that.
Note, initialized vcpus are not destroyed at host KVM but are rather parked in
the QEMU/KVM layer. These are later reused once vcpu is hotplugged again.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
exec.c | 24 ++++++++++++
gdbstub.c | 13 +++++++
include/exec/exec-all.h | 8 ++++
include/exec/gdbstub.h | 1 +
include/hw/core/cpu.h | 2 +
target/arm/cpu-qom.h | 3 ++
target/arm/cpu.c | 86 +++++++++++++++++++++++++++++++++++++++++
target/arm/cpu.h | 13 +++++++
target/arm/helper.c | 31 +++++++++++++++
target/arm/internals.h | 1 +
target/arm/kvm64.c | 7 +++-
11 files changed, 188 insertions(+), 1 deletion(-)
diff --git a/exec.c b/exec.c
index a0bf9d61c8..7e808affdf 100644
--- a/exec.c
+++ b/exec.c
@@ -869,6 +869,7 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
if (!cpu->cpu_ases) {
cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases);
+ cpu->cpu_ases_ref_count = cpu->num_ases;
}
newas = &cpu->cpu_ases[asidx];
@@ -881,6 +882,29 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
}
}
+void cpu_address_space_destroy(CPUState *cpu, int asidx)
+{
+ CPUAddressSpace *cpuas;
+
+ assert(asidx < cpu->num_ases);
+ assert(asidx == 0 || !kvm_enabled());
+ assert(cpu->cpu_ases);
+
+ cpuas = &cpu->cpu_ases[asidx];
+ if (tcg_enabled()) {
+ memory_listener_unregister(&cpuas->tcg_as_listener);
+ }
+
+ address_space_destroy(cpuas->as);
+
+ if(cpu->cpu_ases_ref_count == 1) {
+ g_free(cpu->cpu_ases);
+ cpu->cpu_ases = NULL;
+ }
+
+ cpu->cpu_ases_ref_count--;
+}
+
AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
{
/* Return the AddressSpace corresponding to the specified index */
diff --git a/gdbstub.c b/gdbstub.c
index 6950fd243f..e960268d15 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -982,6 +982,19 @@ void gdb_register_coprocessor(CPUState *cpu,
}
}
+void gdb_unregister_coprocessor_all(CPUState *cpu)
+{
+ GDBRegisterState *s, *p;
+
+ p = cpu->gdb_regs;
+ while (p) {
+ s = p;
+ p = p->next;
+ g_free(s);
+ }
+ cpu->gdb_regs = NULL;
+}
+
#ifndef CONFIG_USER_ONLY
/* Translate GDB watchpoint type to a flags value for cpu_watchpoint_* */
static inline int xlat_gdb_type(CPUState *cpu, int gdbtype)
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 8792bea07a..44420c144d 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -115,6 +115,14 @@ void cpu_reloading_memory_map(void);
*/
void cpu_address_space_init(CPUState *cpu, int asidx,
const char *prefix, MemoryRegion *mr);
+/**
+ * cpu_address_space_destroy:
+ * @cpu: CPU for which address space needs to be destroyed
+ * @asidx: integer index of this address space
+ *
+ * Note that with KVM only one address space is supported.
+ */
+void cpu_address_space_destroy(CPUState *cpu, int asidx);
#endif
#if !defined(CONFIG_USER_ONLY) && defined(CONFIG_TCG)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 94d8f83e92..db2336d0df 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -73,6 +73,7 @@ typedef int (*gdb_set_reg_cb)(CPUArchState *env, uint8_t *buf, int reg);
void gdb_register_coprocessor(CPUState *cpu,
gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
int num_regs, const char *xml, int g_pos);
+void gdb_unregister_coprocessor_all(CPUState *cpu);
/*
* The GDB remote protocol transfers values in target byte order. As
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index d9cae71ea5..851f03dd83 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -379,6 +379,7 @@ struct CPUState {
struct qemu_work_item *queued_work_first, *queued_work_last;
CPUAddressSpace *cpu_ases;
+ int cpu_ases_ref_count;
int num_ases;
AddressSpace *as;
MemoryRegion *memory;
@@ -410,6 +411,7 @@ struct CPUState {
int kvm_fd;
struct KVMState *kvm_state;
struct kvm_run *kvm_run;
+ VMChangeStateEntry *vmcse;
/* Used for events with 'vcpu' and *without* the 'disabled' properties */
DECLARE_BITMAP(trace_dstate_delayed, CPU_TRACE_DSTATE_MAX_EVENTS);
diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index 56395b87f6..d943683551 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -58,6 +58,7 @@ typedef struct ARMCPUClass {
const ARMCPUInfo *info;
DeviceRealize parent_realize;
+ DeviceUnrealize parent_unrealize;
DeviceReset parent_reset;
} ARMCPUClass;
@@ -76,7 +77,9 @@ typedef struct AArch64CPUClass {
} AArch64CPUClass;
void register_cp_regs_for_features(ARMCPU *cpu);
+void unregister_cp_regs_for_features(ARMCPU *cpu);
void init_cpreg_list(ARMCPU *cpu);
+void destroy_cpreg_list(ARMCPU *cpu);
/* Callback functions for the generic timer's timers. */
void arm_gt_ptimer_cb(void *opaque);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 0c9f5f970e..dac42c418d 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -92,6 +92,16 @@ void arm_register_pre_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook,
QLIST_INSERT_HEAD(&cpu->pre_el_change_hooks, entry, node);
}
+void arm_unregister_pre_el_change_hooks(ARMCPU *cpu)
+{
+ ARMELChangeHook *entry, *next;
+
+ QLIST_FOREACH_SAFE(entry, &cpu->pre_el_change_hooks, node, next) {
+ QLIST_REMOVE(entry, node);
+ g_free(entry);
+ }
+}
+
void arm_register_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook,
void *opaque)
{
@@ -103,6 +113,16 @@ void arm_register_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook,
QLIST_INSERT_HEAD(&cpu->el_change_hooks, entry, node);
}
+void arm_unregister_el_change_hooks(ARMCPU *cpu)
+{
+ ARMELChangeHook *entry, *next;
+
+ QLIST_FOREACH_SAFE(entry, &cpu->el_change_hooks, node, next) {
+ QLIST_REMOVE(entry, node);
+ g_free(entry);
+ }
+}
+
static void cp_reg_reset(gpointer key, gpointer value, gpointer opaque)
{
/* Reset a single ARMCPRegInfo register */
@@ -1765,6 +1785,70 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
acc->parent_realize(dev, errp);
}
+static void arm_cpu_unrealizefn(DeviceState *dev)
+{
+ ARMCPUClass *acc = ARM_CPU_GET_CLASS(dev);
+ ARMCPU *cpu = ARM_CPU(dev);
+ CPUARMState *env = &cpu->env;
+ CPUState *cs = CPU(dev);
+
+ /* rock 'n' un-roll, whatever happened in the arm_cpu_realizefn cleanly */
+ if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {
+ cpu_address_space_destroy(cs, ARMASIdx_S);
+ }
+ cpu_address_space_destroy(cs, ARMASIdx_NS);
+
+ destroy_cpreg_list(cpu);
+ arm_cpu_unregister_gdb_regs(cpu);
+ unregister_cp_regs_for_features(cpu);
+
+ if (cpu->sau_sregion && arm_feature(env, ARM_FEATURE_M_SECURITY)) {
+ g_free(env->sau.rbar);
+ g_free(env->sau.rlar);
+ }
+
+ if (arm_feature(env, ARM_FEATURE_PMSA) &&
+ arm_feature(env, ARM_FEATURE_V7) &&
+ cpu->pmsav7_dregion) {
+ if (arm_feature(env, ARM_FEATURE_V8)) {
+ g_free(env->pmsav8.rbar[M_REG_NS]);
+ g_free(env->pmsav8.rlar[M_REG_NS]);
+ if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
+ g_free(env->pmsav8.rbar[M_REG_S]);
+ g_free(env->pmsav8.rlar[M_REG_S]);
+ }
+ } else {
+ g_free(env->pmsav7.drbar);
+ g_free(env->pmsav7.drsr);
+ g_free(env->pmsav7.dracr);
+ }
+ }
+
+ if (arm_feature(env, ARM_FEATURE_PMU)) {
+ if (!kvm_enabled()) {
+ arm_unregister_pre_el_change_hooks(cpu);
+ arm_unregister_el_change_hooks(cpu);
+ }
+
+#ifndef CONFIG_USER_ONLY
+ if (cpu->pmu_timer) {
+ timer_del(cpu->pmu_timer);
+ }
+#endif
+ }
+
+ cpu_remove_sync(CPU(dev));
+ acc->parent_unrealize(dev);
+
+#ifndef CONFIG_USER_ONLY
+ timer_del(cpu->gt_timer[GTIMER_PHYS]);
+ timer_del(cpu->gt_timer[GTIMER_VIRT]);
+ timer_del(cpu->gt_timer[GTIMER_HYP]);
+ timer_del(cpu->gt_timer[GTIMER_SEC]);
+ timer_del(cpu->gt_timer[GTIMER_HYPVIRT]);
+#endif
+}
+
static ObjectClass *arm_cpu_class_by_name(const char *cpu_model)
{
ObjectClass *oc;
@@ -2145,6 +2229,8 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
device_class_set_parent_realize(dc, arm_cpu_realizefn,
&acc->parent_realize);
+ device_class_set_parent_unrealize(dc, arm_cpu_unrealizefn,
+ &acc->parent_unrealize);
device_class_set_props(dc, arm_cpu_properties);
device_class_set_parent_reset(dc, arm_cpu_reset, &acc->parent_reset);
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 5c4991156e..a8e7cb9fb1 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3337,6 +3337,13 @@ static inline AddressSpace *arm_addressspace(CPUState *cs, MemTxAttrs attrs)
*/
void arm_register_pre_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook,
void *opaque);
+/**
+ * arm_unregister_pre_el_change_hook:
+ * unregister all pre EL change hook functions. Generally called during
+ * unrealize'ing leg
+ */
+void arm_unregister_pre_el_change_hooks(ARMCPU *cpu);
+
/**
* arm_register_el_change_hook:
* Register a hook function which will be called immediately after this
@@ -3349,6 +3356,12 @@ void arm_register_pre_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook,
*/
void arm_register_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook, void
*opaque);
+/**
+ * arm_unregister_el_change_hook:
+ * unregister all EL change hook functions. Generally called during
+ * unrealize'ing leg
+ */
+void arm_unregister_el_change_hooks(ARMCPU *cpu);
/**
* arm_rebuild_hflags:
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 972a766730..dc4100ea89 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -502,6 +502,19 @@ void init_cpreg_list(ARMCPU *cpu)
g_list_free(keys);
}
+void destroy_cpreg_list(ARMCPU *cpu)
+{
+ assert(cpu->cpreg_indexes);
+ assert(cpu->cpreg_values);
+ assert(cpu->cpreg_vmstate_indexes);
+ assert(cpu->cpreg_vmstate_values);
+
+ g_free(cpu->cpreg_indexes);
+ g_free(cpu->cpreg_values);
+ g_free(cpu->cpreg_vmstate_indexes);
+ g_free(cpu->cpreg_vmstate_values);
+}
+
/*
* Some registers are not accessible from AArch32 EL3 if SCR.NS == 0.
*/
@@ -7981,6 +7994,18 @@ void register_cp_regs_for_features(ARMCPU *cpu)
#endif
}
+void unregister_cp_regs_for_features(ARMCPU *cpu)
+{
+ CPUARMState *env = &cpu->env;
+ if (arm_feature(env, ARM_FEATURE_M)) {
+ /* M profile has no coprocessor registers */
+ return;
+ }
+
+ /* empty it all. unregister all the coprocessor registers */
+ g_hash_table_remove_all(cpu->cp_regs);
+}
+
void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
{
CPUState *cs = CPU(cpu);
@@ -8019,6 +8044,12 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
}
+void arm_cpu_unregister_gdb_regs(ARMCPU *cpu)
+{
+ CPUState *cs = CPU(cpu);
+ gdb_unregister_coprocessor_all(cs);
+}
+
/* Sort alphabetically by type name, except for "any". */
static gint arm_cpu_list_compare(gconstpointer a, gconstpointer b)
{
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 4bdbc3a8ac..8ece9e09f5 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -169,6 +169,7 @@ static inline int r14_bank_number(int mode)
}
void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
+void arm_cpu_unregister_gdb_regs(ARMCPU *cpu);
void arm_translate_init(void);
enum arm_fprounding {
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index f09ed9f4df..b6df17912e 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -775,7 +775,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
return -EINVAL;
}
- qemu_add_vm_change_state_handler(kvm_arm_vm_state_change, cs);
+ if (qemu_present_cpu(cs))
+ cs->vmcse = qemu_add_vm_change_state_handler(kvm_arm_vm_state_change,
+ cs);
/* Determine init features for this CPU */
memset(cpu->kvm_init_features, 0, sizeof(cpu->kvm_init_features));
@@ -840,6 +842,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
int kvm_arch_destroy_vcpu(CPUState *cs)
{
+ if (qemu_present_cpu(cs))
+ qemu_del_vm_change_state_handler(cs->vmcse);
+
return 0;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (21 preceding siblings ...)
2020-06-13 21:36 ` [PATCH RFC 22/22] arm/cpuhp: Add support of *unrealize* ARMCPU during vcpu hot-unplug Salil Mehta
@ 2020-06-13 22:24 ` no-reply
2020-06-13 22:26 ` no-reply
` (2 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: no-reply @ 2020-06-13 22:24 UTC (permalink / raw)
To: salil.mehta
Cc: peter.maydell, gshan, mst, catalin.marinas, qemu-devel, will,
wangxiongfeng2, jiakernel2, maz, zhukeqian1, david, drjones,
mehta.salil.lnk, richard.henderson, linuxarm, eric.auger,
qemu-arm, imammedo, salil.mehta, maran.wilson, james.morse,
sudeep.holla, pbonzini
Patchew URL: https://patchew.org/QEMU/20200613213629.21984-1-salil.mehta@huawei.com/
Hi,
This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===
TEST check-unit: tests/test-char
TEST check-qtest-aarch64: tests/qtest/tpm-tis-device-swtpm-test
TEST check-qtest-aarch64: tests/qtest/numa-test
qemu-system-aarch64: warning: For GICv2 max-cpus must be equal to smp-cpus
qemu-system-aarch64: warning: Overriding specified max-cpus(8) with smp-cpus(8)
TEST iotest-qcow2: 003
qemu-system-aarch64: warning: For GICv2 max-cpus must be equal to smp-cpus
qemu-system-aarch64: warning: Overriding specified max-cpus(8) with smp-cpus(8)
qemu-system-aarch64: warning: For GICv2 max-cpus must be equal to smp-cpus
qemu-system-aarch64: warning: Overriding specified max-cpus(8) with smp-cpus(8)
qemu-system-aarch64: warning: For GICv2 max-cpus must be equal to smp-cpus
qemu-system-aarch64: warning: Overriding specified max-cpus(8) with smp-cpus(8)
qemu-system-aarch64: warning: For GICv2 max-cpus must be equal to smp-cpus
qemu-system-aarch64: warning: Overriding specified max-cpus(2) with smp-cpus(2)
TEST check-qtest-aarch64: tests/qtest/boot-serial-test
qemu-system-aarch64: warning: For GICv2 max-cpus must be equal to smp-cpus
qemu-system-aarch64: warning: Overriding specified max-cpus(1) with smp-cpus(1)
TEST check-qtest-aarch64: tests/qtest/migration-test
TEST check-unit: tests/check-qnum
TEST iotest-qcow2: 004
---
qemu-system-aarch64: falling back to tcg
TEST iotest-qcow2: 060
TEST check-qtest-aarch64: tests/qtest/bios-tables-test
qemu-system-aarch64: warning: For GICv2 max-cpus must be equal to smp-cpus
qemu-system-aarch64: warning: Overriding specified max-cpus(1) with smp-cpus(1)
TEST iotest-qcow2: 061
TEST iotest-qcow2: 062
TEST iotest-qcow2: 063
---
acpi-test: Warning! DSDT binary file mismatch. Actual [aml:/tmp/aml-3PFIM0], Expected [aml:tests/data/acpi/virt/DSDT].
See source file tests/qtest/bios-tables-test.c for instructions on how to update expected files.
to see ASL diff between mismatched files install IASL, rebuild QEMU from scratch and re-run tests with V=1 environment variable set**
ERROR:/tmp/qemu-test/src/tests/qtest/bios-tables-test.c:494:test_acpi_asl: assertion failed: (all_tables_match)
ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/bios-tables-test.c:494:test_acpi_asl: assertion failed: (all_tables_match)
make: *** [check-qtest-aarch64] Error 1
make: *** Waiting for unfinished jobs....
TEST iotest-qcow2: 073
TEST iotest-qcow2: 074
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=85b55b455d55449297469bf3a981cabd', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-i8pzxavb/src/docker-src.2020-06-13-18.08.03.29364:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=85b55b455d55449297469bf3a981cabd
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-i8pzxavb/src'
make: *** [docker-run-test-quick@centos7] Error 2
real 16m30.871s
user 0m9.410s
The full log is available at
http://patchew.org/logs/20200613213629.21984-1-salil.mehta@huawei.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (22 preceding siblings ...)
2020-06-13 22:24 ` [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch no-reply
@ 2020-06-13 22:26 ` no-reply
2020-06-14 11:54 ` Marc Zyngier
2020-06-23 9:12 ` Andrew Jones
25 siblings, 0 replies; 33+ messages in thread
From: no-reply @ 2020-06-13 22:26 UTC (permalink / raw)
To: salil.mehta
Cc: peter.maydell, gshan, mst, catalin.marinas, qemu-devel, will,
wangxiongfeng2, jiakernel2, maz, zhukeqian1, david, drjones,
mehta.salil.lnk, richard.henderson, linuxarm, eric.auger,
qemu-arm, imammedo, salil.mehta, maran.wilson, james.morse,
sudeep.holla, pbonzini
Patchew URL: https://patchew.org/QEMU/20200613213629.21984-1-salil.mehta@huawei.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20200613213629.21984-1-salil.mehta@huawei.com
Subject: [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
dc56c53 arm/cpuhp: Add support of *unrealize* ARMCPU during vcpu hot-unplug
988141e arm/cpuhp: Changes required for reset and to support next boot
d5feac6 arm/cpuhp: Update the guest(via GED) about cpu hot-(un)plug events
185a83e arm/cpuhp: Changes required to (re)init the vcpu register info
659f80f arm/cpuhp: Changes to update GIC with vcpu hot-plug notification
bcfc8e7 arm/cpuhp: Changes to (un)wire GICC<->VCPU IRQs during hot-(un)plug
643af9e arm/cpuhp: Add/update basic hot-(un)plug framework
8e17131 arm/cpuhp: Update ACPI GED framework to support vcpu hotplug
a8d5f8e arm/cpuhp: Release objects for *disabled* possible vcpus after init
1f520fd arm/cpuhp: Add ACPI _MAT entry for Processor object
e9543c1 arm/cpuhp: MADT Tbl change to size the guest with possible vcpus
a41164f arm/cpuhp: Update GED _EVT method AML with cpu scan
72b8a98 arm/cpuhp: Update CPUs AML with cpu-(ctrl)dev change
9d81444 arm/cpuhp: Init GED framework with cpu hotplug events
96ef87b arm/cpuhp: Enable ACPI support for vcpu hotplug
be81157 arm/cpuhp: Init PMU at host for all possible vcpus
cb588d9 arm/cpuhp: Changes to pre-size GIC with possible vcpus @machine init
75fe8dd arm/cpuhp: Pre-create disabled possible vcpus @machine init
55adeca arm/cpuhp: Machine init time change common to vcpu {cold|hot}-plug
9c53779 arm/cpuhp: Add common cpu utility for possible vcpus
36a71f4 arm/cpuhp: Add new ARMCPU core-id property
ecab7ce arm/cpuhp: Add QMP vcpu params validation support
=== OUTPUT BEGIN ===
1/22 Checking commit ecab7ce0dc60 (arm/cpuhp: Add QMP vcpu params validation support)
2/22 Checking commit 36a71f45080c (arm/cpuhp: Add new ARMCPU core-id property)
3/22 Checking commit 9c53779a7878 (arm/cpuhp: Add common cpu utility for possible vcpus)
ERROR: return is not a function, parentheses are not required
#51: FILE: cpus-common.c:104:
+ return (cpu && !cpu->disabled);
total: 1 errors, 0 warnings, 65 lines checked
Patch 3/22 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/22 Checking commit 55adeca4f0c2 (arm/cpuhp: Machine init time change common to vcpu {cold|hot}-plug)
ERROR: space prohibited between function name and open parenthesis '('
#206: FILE: hw/arm/virt.c:2143:
+ assert (found_cpu);
ERROR: space required before the open brace '{'
#341: FILE: hw/arm/virt.c:2338:
+ (object_dynamic_cast(OBJECT(dev), TYPE_CPU))){
total: 2 errors, 0 warnings, 375 lines checked
Patch 4/22 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/22 Checking commit 75fe8dd750d3 (arm/cpuhp: Pre-create disabled possible vcpus @machine init)
ERROR: space required after that ',' (ctx:VxV)
#93: FILE: hw/arm/virt.c:1841:
+ qdev_set_id(DEVICE(cpuobj),core_id);
^
ERROR: suspect code indent for conditional statements (12, 15)
#109: FILE: hw/arm/virt.c:1857:
+ if (kvm_enabled()) {
+ kvm_arm_create_host_vcpu(ARM_CPU(cs));
WARNING: line over 80 characters
#113: FILE: hw/arm/virt.c:1861:
+ * Add disabled vcpu to cpu slot during the init phase of the virt machine.
WARNING: line over 80 characters
#119: FILE: hw/arm/virt.c:1867:
+ * 2. Now, after initialization of the virt machine is complete we could use
WARNING: line over 80 characters
#127: FILE: hw/arm/virt.c:1875:
+ * We will use the (ii) approach and release the ARMCPU objects after GIC
total: 2 errors, 3 warnings, 159 lines checked
Patch 5/22 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/22 Checking commit cb588d911d62 (arm/cpuhp: Changes to pre-size GIC with possible vcpus @machine init)
ERROR: braces {} are necessary for all arms of this statement
#105: FILE: hw/intc/arm_gicv3_common.c:355:
+ if (qemu_present_cpu(cpu))
[...]
+ else
[...]
ERROR: braces {} are necessary for all arms of this statement
#121: FILE: hw/intc/arm_gicv3_cpuif.c:782:
+ if (!qemu_present_cpu(cs->cpu))
[...]
ERROR: braces {} are necessary for all arms of this statement
#161: FILE: hw/intc/arm_gicv3_kvm.c:470:
+ if (!qemu_present_cpu(c->cpu))
[...]
ERROR: braces {} are necessary for all arms of this statement
#178: FILE: hw/intc/arm_gicv3_kvm.c:699:
+ if (!qemu_present_cpu(c->cpu))
[...]
ERROR: braces {} are necessary for all arms of this statement
#184: FILE: hw/intc/arm_gicv3_kvm.c:705:
+ if (!qemu_present_cpu(c->cpu))
[...]
ERROR: braces {} are necessary for all arms of this statement
#197: FILE: hw/intc/arm_gicv3_kvm.c:815:
+ if (qemu_present_cpu(cs))
[...]
total: 6 errors, 0 warnings, 160 lines checked
Patch 6/22 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/22 Checking commit be81157f5799 (arm/cpuhp: Init PMU at host for all possible vcpus)
8/22 Checking commit 96ef87bd215d (arm/cpuhp: Enable ACPI support for vcpu hotplug)
ERROR: braces {} are necessary for all arms of this statement
#43: FILE: hw/acpi/cpu.c:222:
+ if (qemu_present_cpu(cpu))
[...]
+ else
[...]
total: 1 errors, 0 warnings, 48 lines checked
Patch 8/22 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/22 Checking commit 9d81444f4907 (arm/cpuhp: Init GED framework with cpu hotplug events)
10/22 Checking commit 72b8a98eb62a (arm/cpuhp: Update CPUs AML with cpu-(ctrl)dev change)
11/22 Checking commit a41164f24088 (arm/cpuhp: Update GED _EVT method AML with cpu scan)
12/22 Checking commit e9543c111d5d (arm/cpuhp: MADT Tbl change to size the guest with possible vcpus)
ERROR: space prohibited after that open parenthesis '('
#53: FILE: hw/arm/virt-acpi-build.c:632:
+ if ( i < vms->smp_cpus ) {
ERROR: space prohibited before that close parenthesis ')'
#53: FILE: hw/arm/virt-acpi-build.c:632:
+ if ( i < vms->smp_cpus ) {
total: 2 errors, 0 warnings, 39 lines checked
Patch 12/22 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/22 Checking commit 1f520fd3c10b (arm/cpuhp: Add ACPI _MAT entry for Processor object)
ERROR: space required after that ',' (ctx:VxV)
#45: FILE: hw/arm/virt-acpi-build.c:600:
+ AcpiMadtGenericCpuInterface *gicc = acpi_data_push(entry,sizeof(*gicc));
^
total: 1 errors, 0 warnings, 50 lines checked
Patch 13/22 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
14/22 Checking commit a8d5f8e441b4 (arm/cpuhp: Release objects for *disabled* possible vcpus after init)
15/22 Checking commit 8e171312cdaf (arm/cpuhp: Update ACPI GED framework to support vcpu hotplug)
WARNING: line over 80 characters
#49: FILE: hw/acpi/generic_event_device.c:213:
+ error_setg(errp, "virt: device unplug request for the unsupported device"
total: 0 errors, 1 warnings, 75 lines checked
Patch 15/22 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
16/22 Checking commit 643af9ec7c7d (arm/cpuhp: Add/update basic hot-(un)plug framework)
WARNING: line over 80 characters
#69: FILE: hw/arm/virt.c:2396:
+ /* TODO: update acpi hotplug state and send cpu hotplug event to guest */
WARNING: line over 80 characters
#70: FILE: hw/arm/virt.c:2397:
+ /* TODO: register this cpu for reset & update F/W info for the next boot */
ERROR: that open brace { should be on the previous line
#94: FILE: hw/arm/virt.c:2421:
+ if (cs->cpu_index == first_cpu->cpu_index)
+ {
WARNING: line over 80 characters
#126: FILE: hw/arm/virt.c:2453:
+ /* TODO: unregister this cpu for reset & update F/W info for the next boot */
total: 1 errors, 3 warnings, 146 lines checked
Patch 16/22 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
17/22 Checking commit bcfc8e7af6d2 (arm/cpuhp: Changes to (un)wire GICC<->VCPU IRQs during hot-(un)plug)
WARNING: Block comments use a leading /* on a separate line
#33: FILE: hw/arm/virt.c:627:
+ /* Mapping from the output timer irq lines from the CPU to the
WARNING: line over 80 characters
#48: FILE: hw/arm/virt.c:642:
+ qdev_disconnect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0);
ERROR: line over 90 characters
#50: FILE: hw/arm/virt.c:644:
+ qdev_disconnect_gpio_out_named(gicdev, SYSBUS_DEVICE_GPIO_IRQ, cpu + 4 * max_cpus);
WARNING: line over 80 characters
#54: FILE: hw/arm/virt.c:648:
+ * RFC: Question: This currently does not takes care of intimating the devices
WARNING: line over 80 characters
#55: FILE: hw/arm/virt.c:649:
+ * which might be sitting on system bus. Do we need a sysbus_disconnect_irq()
WARNING: Block comments use a leading /* on a separate line
#81: FILE: hw/arm/virt.c:675:
+ /* Mapping from the output timer irq lines from the CPU to the
total: 1 errors, 5 warnings, 190 lines checked
Patch 17/22 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
18/22 Checking commit 659f80fdff39 (arm/cpuhp: Changes to update GIC with vcpu hot-plug notification)
ERROR: "foo * bar" should be "foo *bar"
#99: FILE: hw/intc/arm_gicv3_common.c:329:
+static void arm_gicv3_cpu_update_notifier(Notifier * notifier, void * data)
total: 1 errors, 0 warnings, 147 lines checked
Patch 18/22 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
19/22 Checking commit 185a83e914a7 (arm/cpuhp: Changes required to (re)init the vcpu register info)
WARNING: Block comments use a leading /* on a separate line
#65: FILE: hw/intc/arm_gicv3_cpuif.c:2613:
+ /* Note that we can't just use the GICv3CPUState as an opaque pointer
WARNING: Block comments use a leading /* on a separate line
#83: FILE: hw/intc/arm_gicv3_cpuif.c:2631:
+ /* Check against architectural constraints: getting these
WARNING: Block comments use a leading /* on a separate line
#94: FILE: hw/intc/arm_gicv3_cpuif.c:2642:
+ /* Note that the AArch64 LRs are 64-bit; the AArch32 LRs
total: 0 errors, 3 warnings, 211 lines checked
Patch 19/22 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
20/22 Checking commit d5feac63a862 (arm/cpuhp: Update the guest(via GED) about cpu hot-(un)plug events)
ERROR: braces {} are necessary for all arms of this statement
#44: FILE: hw/arm/virt.c:2469:
+ if (local_err)
[...]
ERROR: braces {} are necessary for all arms of this statement
#73: FILE: hw/arm/virt.c:2509:
+ if (local_err)
[...]
ERROR: braces {} are necessary for all arms of this statement
#100: FILE: hw/arm/virt.c:2537:
+ if (local_err)
[...]
total: 3 errors, 0 warnings, 84 lines checked
Patch 20/22 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
21/22 Checking commit 988141e42492 (arm/cpuhp: Changes required for reset and to support next boot)
22/22 Checking commit dc56c5324bae (arm/cpuhp: Add support of *unrealize* ARMCPU during vcpu hot-unplug)
ERROR: space required before the open parenthesis '('
#50: FILE: exec.c:900:
+ if(cpu->cpu_ases_ref_count == 1) {
ERROR: suspect code indent for conditional statements (4, 11)
#217: FILE: target/arm/cpu.c:1805:
+ if (cpu->sau_sregion && arm_feature(env, ARM_FEATURE_M_SECURITY)) {
+ g_free(env->sau.rbar);
ERROR: code indent should never use tabs
#272: FILE: target/arm/cpu.c:2233:
+^I^I^I^I &acc->parent_unrealize);$
ERROR: braces {} are necessary for all arms of this statement
#394: FILE: target/arm/kvm64.c:845:
+ if (qemu_present_cpu(cs))
[...]
total: 4 errors, 0 warnings, 316 lines checked
Patch 22/22 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20200613213629.21984-1-salil.mehta@huawei.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (23 preceding siblings ...)
2020-06-13 22:26 ` no-reply
@ 2020-06-14 11:54 ` Marc Zyngier
2020-06-15 10:19 ` Salil Mehta
2020-06-23 9:12 ` Andrew Jones
25 siblings, 1 reply; 33+ messages in thread
From: Marc Zyngier @ 2020-06-14 11:54 UTC (permalink / raw)
To: Salil Mehta
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
catalin.marinas, zhukeqian1, david, richard.henderson,
qemu-devel, linuxarm, eric.auger, qemu-arm, james.morse,
pbonzini, imammedo, mehta.salil.lnk, maran.wilson, will,
wangxiongfeng2
Hi Salil,
On 2020-06-13 22:36, Salil Mehta wrote:
> This patch-set introduces the virtual cpu hotplug support for ARMv8
> architecture in QEMU. Idea is to be able to hotplug and hot-unplug the
> vcpus
> while guest VM is running and no reboot is required. This does *not*
> makes any
> assumption of the physical cpu hotplug availability within the host
> system but
> rather tries to solve the problem at virtualizer/QEMU layer and by
> introducing
> cpu hotplug hooks and event handling within the guest kernel. No
> changes are
> required within the host kernel/KVM.
>
> Motivation:
> This allows scaling the guest VM compute capacity on-demand which would
> be
> useful for the following example scenarios,
> 1. Vertical Pod Autoscaling[3][4] in the cloud: Part of the
> orchestration
> framework which could adjust resource requests (CPU and Mem
> requests) for
> the containers in a pod, based on usage.
> 2. Pay-as-you-grow Business Model: Infrastructure provider could
> allocate and
> restrict the total number of compute resources available to the
> guest VM
> according to the SLA(Service Level Agreement). VM owner could
> request for
> more compute to be hot-plugged for some cost.
>
> Terminology:
>
> (*) Present cpus: Total cpus with which guest has/will boot and are
> available
> to guest for use and can be onlined. Qemu
> parameter(-smp)
> (*) Disabled cpus: Possible cpus which will not be available for the
> guest to
> use. These can be hotplugged and made present. These
> can be
> thought of as un-plugged vcpus. These will be included as
> part of sizing.
> (*) Posssible cpus: Total vcpus which could ever exist in VM. This
> includes
> booted cpus plus any cpus which could be later
> plugged.
> - Qemu parameter(-maxcpus)
> - Possible vcpus = Present vcpus (+) Disabled vcpus
>
>
> Limitations of ARMv8 Architecture:
>
> A. Physical Limitation to CPU Hotplug:
> 1. ARMv8 architecture does not support the concept of the physical cpu
> hotplug.
> The closest thing which is recomended to achieve the cpu hotplug on
> ARM is
> to bring down power state of the cpu using PSCI.
It isn't so much that the ARMv8 architecture doesn't support CPU
hotplug. It is that CPU hotplug is largely out of the scope of the ARMv8
architecture, which is a CPU architecture and not a system architecture.
Yes, the lack of a comprehensive system architecture is *very* annoying,
but let's put the blame where it belongs... ;-)
> 2. Other ARM components like GIC etc. have not been designed to realize
> physical cpu hotplug capability as of now.
>
> B. Limitations of GIC to Support Virtual CPU Hotplug:
> 1. GIC requires various resources(related to GICR/redistributor,
> GICC/cpu
> interface etc) like memory regions to be fixed at the VM init time
> and these
> could not be changed later on after VM has inited.
> 2. Associations between GICC(GIC cpu interface) and vcpu get fixed at
> the VM
> init time and GIC does not allows to change this association once
> GIC has
> initialized.
There isn't an association, really. the GIC CPU interface is part of the
CPU itself, and not an external entity. KVM doesn't split the two
either. It is the association between the CPU and its redistributor that
is being done. There is no architectural way to set this up this, so KVM
just statically configures these based on the number of vcpus and the
number/size of redistributor ranges.
>
> C. Known Limitation of the KVM:
> 1. As of now KVM allows to create VCPUs but does not allows to delete
> the
> already created vcpus. QEMU already provides an interface to manage
> created
> vcpus at KVM level and then to re-use them.
> 2. Inconsistency in interpretation of the MPIDR generated by KVM for
> vcpus
> vis-a-vis SMT/threads. This does not looks to be compliant to the
> MPIDR
> format(SMT is present) as mentioned in the ARMv8 spec. (Please
> correct my
> understanding if I am wrong here?)
I'm unsure of which part of the architecture KVM doesn't follow when
assigning a MPIDR to a vcpu. By having MPIDR_EL1.MT to 0, KVM tells the
guest OS that vcpus having the same Aff3-1 fields are mostly
independent.
This is because:
- we make no placement guarantee whatsoever (this is userspace's
business)
- ARMv8.2 CPUs designed by ARM (as opposed to architecture licensees)
always set this bit to 1 as they can hypothetically be coupled to SMT
CPUs in a big-little system. This makes MT totally meaningless.
- the one SMT implementation available in the wild (ThunderX-2) is
broken enough that you really should consider turning SMT off (see
CAVIUM_TX2_ERRATUM_219)
- KVM actively prevents the sharing of resources such as TLBs across
vcpus
Given the above, I fail to see the point in setting the SMT bit to
anything but 0.
>
>
> Workaround to the problems mentioned in Section B & C1:
> 1. We pre-size the GIC with possible vcpus at VM init time
> 2. Pre-create all possible vcpus at KVM and associate them with GICC
Again, I'm not sure what you mean here. The CPU interface is by
construction associated with the CPU. Do you mean the redistributor?
> 3. Park the unplugged vcpus (similar to x86)
>
>
> (*) For all of above please refer to Marc's suggestion here[1]
>
>
> Overview of the Approach:
> At the time of machvirt_init() we pre-create all of the possible ARMCPU
> objects along with the corresponding KVM vcpus at the host. Disabled
> KVM vcpu
> (which are *not* "present" vcpus but are part of "possible" vcpu list)
> are
> parked at per VM list "kvm_parked_vcpus" after their initialization.
>
> We create the ARMCPU objects(but these are not *realized* in QOM sense)
> even
> for the disabled vcpus to facilitate the GIC initialization (pre-sized
> with
> possible vcpus). After Initialization of the machine is complete we
> release
> the ARMCPU Objects for the disabled vcpus. These ARMCPU object shall be
> re-created at the time when vcpu is hot plugged. This new object is
> then
> re-attached with the earlier parked KVM vcpu which also gets unparked.
> The
> ARMCPU object gets now "realized" in QEMU, which means creation of the
> corresponding threads, pre_plug/plug phases, and event notification to
> the
> guest using ACPI GED etc. Similarly, hot-unplug leg will lead to the
> "unrealization" of the vcpus and will lead to similar ACPI GED events
> to the
> guest for unplug and cleanup and eventually ARMCPU object shall be
> released and
> KVM vcpus shall be parked again.
>
> During machine init, ACPI MADT Table is sized with *possible* vcpus
> GICC
> entries. The unplugged/disabled vcpus are presented as MADT GICC
> DISABLED
> entries to the guest. This means the guest will have its resources
> pre-sized
> with possible vcpus(=present+disabled)
>
> Other approaches to deal with ARMCPU object release(after machine
> init):
> 1. The ARMCPU objects for the disabled vcpus are released in context to
> the
> virt_machine_done() notifier(approach adopted in this patch-set).
> 2. Defer the release of current ARMCPU object till the new vcpu object
> is
> hot plugged.
> 3. Never release and keep on reusing them and release once at VM exit.
> This
> solves many problems with above 2 approaches but requires change in
> the way
> qdev_device_add() fetches/creates the ARMCPU object for the new
> vcpus being
> hotplugged. For the arm cpu hotplug case we need to figure out way
> how to
> get access to old object and use it to "re-realize" instead of the
> new
> ARMCPU object.
>
> Concerns/Questions:
> 1. In ARM arch a cpu is uniquely represented in hierarchy using various
> affinity levels which could represent thread, core, cluster,
> package. This
> is generally represented by a value in MPIDR register as per the
> format
> mentioned in specification. Now, the way MPIDR value is derived for
> vcpus is
> done using vcpu-index. The concept of thread is not quite as same
> and rather
> gets lost in the derivation of MPIDR for vcpus.
I think we disagree on what a thread is when MPIDR_EL1.MT==0.
> 2. The topology info used to specify the vcpu while hot-plugging might
> not
> match with the MPIDR value given back by the KVM for the vcpu at the
> time of
> init. Concept of SMT bit in MPIDR gets lost as per the derivation
> being done
> in the KVM. Hence, concept of thread-id, core-id, socket-id if used
> as a
> topology info to derive MPIDR value as per ARM specification will
> not match
> with MPIDR actually assigned by the KVM?
The architecture doesn't map the affinity levels on any of these
concepts. They merely imply increased independence as you look at higher
affinity levels. I don't see any value in exposing any meaning to them
unless you start moving some notion of placement into the kernel, or
allow userspace to setup MPIDR_EL1 by itself.
> Perhaps need to carry forward work of Andrew? please check here[2]
> 3. Further if this info is supplied to the guest using PPTT(once
> introduced in
> QEMU) or even derived using MPIDR shall be inconsistent with the
> host vcpu.
> 4. Any possibilities of interrupts(SGI/PPI/LPI/SPI) always remaining in
> *pending* state for the cpus which have been hot-unplugged? IMHO it
> looks
> okay but will need Marc's confirmation on this.
I don't see why interrupts wouldn't be pending if the storage resource
still exists. Given that distributor and redistributors are persistent,
it seems logical that pending bits stick around.
Slightly more selfish question: is there any kernel change associated
with this series? I'd be perfectly happy to hear a "no!"... ;-)
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch
2020-06-14 11:54 ` Marc Zyngier
@ 2020-06-15 10:19 ` Salil Mehta
0 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-15 10:19 UTC (permalink / raw)
To: Marc Zyngier
Cc: peter.maydell, drjones, sudeep.holla, gshan, mst, jiakernel2,
catalin.marinas, zhukeqian, david, richard.henderson, qemu-devel,
Linuxarm, eric.auger, qemu-arm, james.morse, pbonzini, imammedo,
mehta.salil.lnk, maran.wilson, will, wangxiongfeng (C)
Hi Marc,
Thanks for the review.
> From: Marc Zyngier [mailto:maz@kernel.org]
> Sent: Sunday, June 14, 2020 12:55 PM
>
> Hi Salil,
>
> On 2020-06-13 22:36, Salil Mehta wrote:
> > This patch-set introduces the virtual cpu hotplug support for ARMv8
> > architecture in QEMU. Idea is to be able to hotplug and hot-unplug the
> > vcpus
> > while guest VM is running and no reboot is required. This does *not*
> > makes any
> > assumption of the physical cpu hotplug availability within the host
> > system but
> > rather tries to solve the problem at virtualizer/QEMU layer and by
> > introducing
> > cpu hotplug hooks and event handling within the guest kernel. No
> > changes are
> > required within the host kernel/KVM.
> >
> > Motivation:
> > This allows scaling the guest VM compute capacity on-demand which would
> > be
> > useful for the following example scenarios,
> > 1. Vertical Pod Autoscaling[3][4] in the cloud: Part of the
> > orchestration
> > framework which could adjust resource requests (CPU and Mem
> > requests) for
> > the containers in a pod, based on usage.
> > 2. Pay-as-you-grow Business Model: Infrastructure provider could
> > allocate and
> > restrict the total number of compute resources available to the
> > guest VM
> > according to the SLA(Service Level Agreement). VM owner could
> > request for
> > more compute to be hot-plugged for some cost.
> >
> > Terminology:
> >
> > (*) Present cpus: Total cpus with which guest has/will boot and are
> > available
> > to guest for use and can be onlined. Qemu
> > parameter(-smp)
> > (*) Disabled cpus: Possible cpus which will not be available for the
> > guest to
> > use. These can be hotplugged and made present. These
> > can be
> > thought of as un-plugged vcpus. These will be included as
> > part of sizing.
> > (*) Posssible cpus: Total vcpus which could ever exist in VM. This
> > includes
> > booted cpus plus any cpus which could be later
> > plugged.
> > - Qemu parameter(-maxcpus)
> > - Possible vcpus = Present vcpus (+) Disabled vcpus
> >
> >
> > Limitations of ARMv8 Architecture:
> >
> > A. Physical Limitation to CPU Hotplug:
> > 1. ARMv8 architecture does not support the concept of the physical cpu
> > hotplug.
> > The closest thing which is recomended to achieve the cpu hotplug on
> > ARM is
> > to bring down power state of the cpu using PSCI.
>
> It isn't so much that the ARMv8 architecture doesn't support CPU
> hotplug. It is that CPU hotplug is largely out of the scope of the ARMv8
> architecture, which is a CPU architecture and not a system architecture.
> Yes, the lack of a comprehensive system architecture is *very* annoying,
> but let's put the blame where it belongs... ;-)
Sure.
> > 2. Other ARM components like GIC etc. have not been designed to realize
> > physical cpu hotplug capability as of now.
> >
> > B. Limitations of GIC to Support Virtual CPU Hotplug:
> > 1. GIC requires various resources(related to GICR/redistributor,
> > GICC/cpu
> > interface etc) like memory regions to be fixed at the VM init time
> > and these
> > could not be changed later on after VM has inited.
> > 2. Associations between GICC(GIC cpu interface) and vcpu get fixed at
> > the VM
> > init time and GIC does not allows to change this association once
> > GIC has
> > initialized.
>
> There isn't an association, really. the GIC CPU interface is part of the
> CPU itself, and not an external entity. KVM doesn't split the two
> either. It is the association between the CPU and its redistributor that
> is being done. There is no architectural way to set this up this, so KVM
> just statically configures these based on the number of vcpus and the
> number/size of redistributor ranges.
I stand corrected. Sorry for the horrible mix up and I realized that some
how I copied the same terminology at other 2 other places as well, maybe
under sleep duress :(. I will correct it in later versions.
To be frank, I actually meant association of "mp-affinity" and the
"proc number" as given by GICR_TYPER register for the vgic. I guess reading
this register using kvm_gicr_acces/KVM_DEV_ARM_VGIC_GRP_REDIST_REGS from
QEMU lands up in vgic vgic_mmio_read_v3r_typer() which forms the reg value
using "vcpu-id" and "mpidr"(fetched from MPIDR_EL1).
Also, value of the MPIDR for vcpu is set during KVM_ARM_VCPU_INIT IOCTL
from QEMU after the creation of the vcpus(using KVM_CREATE_VCPU). I guess
this is done during reset of all of the system regs SYS_MPIDR_EL1 value
is also reset to default within function reset_mpidr() derived using below
logic:
+----+----+----+----+----+----+----+----+
MPIDR ||| Res | Aff2 | Aff1 | Aff0 |
+----+----+----+----+----+----+----+----+
\ \ \ | |
\ 8bit \ 8bit \ |4bit|
\<------->\<------->\|<-->|
\ \ \| |
+----+----+----+----+----+----+----+----+
VCPU-ID | Byte4 | Byte2 | Byte1 | Byte0 |
+----+----+----+----+----+----+----+----+
Perhaps once the VM is inited the value present in the MPIDR_EL1 for vcpu cannot
be changed and hence the vgic GICR_TYPER will have fixed association between
"vcpu-id" and "mpidr" as well.
> > C. Known Limitation of the KVM:
> > 1. As of now KVM allows to create VCPUs but does not allows to delete
> > the
> > already created vcpus. QEMU already provides an interface to manage
> > created
> > vcpus at KVM level and then to re-use them.
> > 2. Inconsistency in interpretation of the MPIDR generated by KVM for
> > vcpus
> > vis-a-vis SMT/threads. This does not looks to be compliant to the
> > MPIDR
> > format(SMT is present) as mentioned in the ARMv8 spec. (Please
> > correct my
> > understanding if I am wrong here?)
>
> I'm unsure of which part of the architecture KVM doesn't follow when
> assigning a MPIDR to a vcpu. By having MPIDR_EL1.MT to 0, KVM tells the
> guest OS that vcpus having the same Aff3-1 fields are mostly
> independent.
>
> This is because:
> - we make no placement guarantee whatsoever (this is userspace's
> business)
> - ARMv8.2 CPUs designed by ARM (as opposed to architecture licensees)
> always set this bit to 1 as they can hypothetically be coupled to SMT
> CPUs in a big-little system. This makes MT totally meaningless.
> - the one SMT implementation available in the wild (ThunderX-2) is
> broken enough that you really should consider turning SMT off (see
> CAVIUM_TX2_ERRATUM_219)
> - KVM actively prevents the sharing of resources such as TLBs across
> vcpus
>
> Given the above, I fail to see the point in setting the SMT bit to
> anything but 0.
ok. effectively this also means that even in the upcoming SoCs, which
might have SMT capability, the value of MT bit should continue to be 0
at the KVM level for the MPIDR of the vcpus?
> > Workaround to the problems mentioned in Section B & C1:
> > 1. We pre-size the GIC with possible vcpus at VM init time
> > 2. Pre-create all possible vcpus at KVM and associate them with GICC
>
> Again, I'm not sure what you mean here. The CPU interface is by
> construction associated with the CPU. Do you mean the redistributor?
Sorry for this mix-up. Yes, I meant the association between the mp-affinity
and the proc number as given by GICR_TYPER for vgic as explained earlier.
>
> > 3. Park the unplugged vcpus (similar to x86)
> >
> >
> > (*) For all of above please refer to Marc's suggestion here[1]
> >
> >
> > Overview of the Approach:
> > At the time of machvirt_init() we pre-create all of the possible ARMCPU
> > objects along with the corresponding KVM vcpus at the host. Disabled
> > KVM vcpu
> > (which are *not* "present" vcpus but are part of "possible" vcpu list)
> > are
> > parked at per VM list "kvm_parked_vcpus" after their initialization.
> >
> > We create the ARMCPU objects(but these are not *realized* in QOM sense)
> > even
> > for the disabled vcpus to facilitate the GIC initialization (pre-sized
> > with
> > possible vcpus). After Initialization of the machine is complete we
> > release
> > the ARMCPU Objects for the disabled vcpus. These ARMCPU object shall be
> > re-created at the time when vcpu is hot plugged. This new object is
> > then
> > re-attached with the earlier parked KVM vcpu which also gets unparked.
> > The
> > ARMCPU object gets now "realized" in QEMU, which means creation of the
> > corresponding threads, pre_plug/plug phases, and event notification to
> > the
> > guest using ACPI GED etc. Similarly, hot-unplug leg will lead to the
> > "unrealization" of the vcpus and will lead to similar ACPI GED events
> > to the
> > guest for unplug and cleanup and eventually ARMCPU object shall be
> > released and
> > KVM vcpus shall be parked again.
> >
> > During machine init, ACPI MADT Table is sized with *possible* vcpus
> > GICC
> > entries. The unplugged/disabled vcpus are presented as MADT GICC
> > DISABLED
> > entries to the guest. This means the guest will have its resources
> > pre-sized
> > with possible vcpus(=present+disabled)
> >
> > Other approaches to deal with ARMCPU object release(after machine
> > init):
> > 1. The ARMCPU objects for the disabled vcpus are released in context to
> > the
> > virt_machine_done() notifier(approach adopted in this patch-set).
> > 2. Defer the release of current ARMCPU object till the new vcpu object
> > is
> > hot plugged.
> > 3. Never release and keep on reusing them and release once at VM exit.
> > This
> > solves many problems with above 2 approaches but requires change in
> > the way
> > qdev_device_add() fetches/creates the ARMCPU object for the new
> > vcpus being
> > hotplugged. For the arm cpu hotplug case we need to figure out way
> > how to
> > get access to old object and use it to "re-realize" instead of the
> > new
> > ARMCPU object.
> >
> > Concerns/Questions:
> > 1. In ARM arch a cpu is uniquely represented in hierarchy using various
> > affinity levels which could represent thread, core, cluster,
> > package. This
> > is generally represented by a value in MPIDR register as per the
> > format
> > mentioned in specification. Now, the way MPIDR value is derived for
> > vcpus is
> > done using vcpu-index. The concept of thread is not quite as same
> > and rather
> > gets lost in the derivation of MPIDR for vcpus.
>
> I think we disagree on what a thread is when MPIDR_EL1.MT==0.
>
> > 2. The topology info used to specify the vcpu while hot-plugging might
> > not
> > match with the MPIDR value given back by the KVM for the vcpu at the
> > time of
> > init. Concept of SMT bit in MPIDR gets lost as per the derivation
> > being done
> > in the KVM. Hence, concept of thread-id, core-id, socket-id if used
> > as a
> > topology info to derive MPIDR value as per ARM specification will
> > not match
> > with MPIDR actually assigned by the KVM?
>
> The architecture doesn't map the affinity levels on any of these
> concepts. They merely imply increased independence as you look at higher
> affinity levels. I don't see any value in exposing any meaning to them
> unless you start moving some notion of placement into the kernel, or
> allow userspace to setup MPIDR_EL1 by itself.
MPIDR_EL1 of the vcpu is effectively being derived by KVM using the
'vcpu-id' provided by the QEMU during vcpu creation/init time. so isn't it
kind of being indirectly configured by QEMU even now?
Also. I was wondering if it was safe to present hardware threads as cores
in the virtual topology being set at the QEMU level?
> > Perhaps need to carry forward work of Andrew? please check here[2]
> > 3. Further if this info is supplied to the guest using PPTT(once
> > introduced in
> > QEMU) or even derived using MPIDR shall be inconsistent with the
> > host vcpu.
> > 4. Any possibilities of interrupts(SGI/PPI/LPI/SPI) always remaining in
> > *pending* state for the cpus which have been hot-unplugged? IMHO it
> > looks
> > okay but will need Marc's confirmation on this.
>
> I don't see why interrupts wouldn't be pending if the storage resource
> still exists. Given that distributor and redistributors are persistent,
> it seems logical that pending bits stick around.
wouldn't they be migrated to some other vcpu after current vcpu to which
they are associated gets offlined as part of the cpu hot-unplug action?
Like in case of the NIC, it gets notified about this and the affinity is
changed so the interrupts should not remain pending. Is it something
different about storage?
>
> Slightly more selfish question: is there any kernel change associated
> with this series? I'd be perfectly happy to hear a "no!"... ;-)
Requires minimalistic change which amounts to:
1. Allocation of the logical cpu-id and associating with acpi-id/hw-id
coming from qemu/ACPI Tables.
2. During init time parsing the MADT Table entries to get the exact count
of the possible and present cpus. This as you know shall be used to
size up the guest data structures. Most of this infrastructure already
exist in smp.c but still requires some adjustment of the code.
I will be sharing these changes this week.
Many thanks
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH RFC 01/22] arm/cpuhp: Add QMP vcpu params validation support
2020-06-13 21:36 ` [PATCH RFC 01/22] arm/cpuhp: Add QMP vcpu params validation support Salil Mehta
@ 2020-06-23 8:46 ` Andrew Jones
2020-06-23 9:40 ` Salil Mehta
0 siblings, 1 reply; 33+ messages in thread
From: Andrew Jones @ 2020-06-23 8:46 UTC (permalink / raw)
To: Salil Mehta
Cc: peter.maydell, mehta.salil.lnk, gshan, pbonzini, mst, jiakernel2,
maz, david, richard.henderson, qemu-devel, linuxarm, eric.auger,
will, qemu-arm, james.morse, catalin.marinas, sudeep.holla,
imammedo, maran.wilson, zhukeqian1, wangxiongfeng2
On Sat, Jun 13, 2020 at 10:36:08PM +0100, Salil Mehta wrote:
> For now, vcpu hotplug is only supported with single socket single thread,
> single die. NUMA is not supported either and everthing falls into single
> node. Work to properly support these could be taken later once community
> agrees with the base framework changes being presented to support ARM vcpu
> hotplug in QEMU. Hence, these checks.
>
> Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
> hw/arm/virt.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 37462a6f78..5d1afdd031 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2201,6 +2201,46 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
> return NULL;
> }
>
> +static void virt_smp_parse(MachineState *ms, QemuOpts *opts)
> +{
> + if (opts) {
> + unsigned cpus = qemu_opt_get_number(opts, "cpus", 1);
> + unsigned sockets = qemu_opt_get_number(opts, "sockets", 1);
> + unsigned cores = qemu_opt_get_number(opts, "cores", cpus);
> + unsigned threads = qemu_opt_get_number(opts, "threads", 1);
> + unsigned int max_cpus;
> +
> + if (sockets > 1 || threads > 1) {
> + error_report("does not support more than one socket or thread");
> + exit(1);
> + }
> +
> + if (cores != cpus) {
> + error_report("cpu topology: "
> + "sockets (%u) * cores (%u) * threads (%u) < "
> + "smp_cpus (%u)",
> + sockets, cores, threads, cpus);
> + exit(1);
> + }
> +
> + max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
> + if (sockets * cores * threads > max_cpus) {
> + error_report("cpu topology: "
> + "sockets (%u) * cores (%u) * threads (%u) > "
> + "maxcpus (%u)",
> + sockets, cores, threads,
> + max_cpus);
> + exit(1);
> + }
> +
> + ms->smp.max_cpus = max_cpus;
> + ms->smp.sockets = sockets;
> + ms->smp.cpus = cpus;
> + ms->smp.cores = cores;
> + ms->smp.threads = threads;
> + }
> +}
> +
> /*
> * for arm64 kvm_type [7-0] encodes the requested number of bits
> * in the IPA address space
> @@ -2266,6 +2306,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> mc->nvdimm_supported = true;
> mc->auto_enable_numa_with_memhp = true;
> mc->default_ram_id = "mach-virt.ram";
> + mc->smp_parse = virt_smp_parse;
>
> object_class_property_add(oc, "acpi", "OnOffAuto",
> virt_get_acpi, virt_set_acpi,
> --
> 2.17.1
>
>
>
Hi Salil,
This patch and the whole series has inspired me to pick up the vcpu
topology work again. In fact, I think it may be necessary in order
to properly describe a cpu when hot[un]plugging. I'll try to pull
together an RFC soon, at least for TCG. For KVM, we may need to
change KVM in order to allow user-controlled MPIDR. Although I'm
not sure about that anymore, because, as you stated somewhere else,
we already have user-controlled MPIDR to some degree, since KVM simply
transforms the cpu index.
Regarding this patch specifically, I would change this to allow
sockets, but prefer cores (i.e. when only '-smp N' is given, then
N is the number of cores, not sockets). Also I would allow threads,
but only for !kvm_enabled(). Then the function would be similar to
something I think I once posted long ago, or at least wrote and maybe
never posted...
Thanks,
drew
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH RFC 07/22] arm/cpuhp: Init PMU at host for all possible vcpus
2020-06-13 21:36 ` [PATCH RFC 07/22] arm/cpuhp: Init PMU at host for all possible vcpus Salil Mehta
@ 2020-06-23 9:00 ` Andrew Jones
2020-06-23 9:52 ` Salil Mehta
0 siblings, 1 reply; 33+ messages in thread
From: Andrew Jones @ 2020-06-23 9:00 UTC (permalink / raw)
To: Salil Mehta
Cc: peter.maydell, mehta.salil.lnk, gshan, pbonzini, mst, jiakernel2,
maz, david, richard.henderson, qemu-devel, linuxarm, eric.auger,
will, qemu-arm, james.morse, catalin.marinas, sudeep.holla,
imammedo, maran.wilson, zhukeqian1, wangxiongfeng2
On Sat, Jun 13, 2020 at 10:36:14PM +0100, Salil Mehta wrote:
> PMU for all possible vcpus must be initialized at the virt machine
> initialization time. This patch refactors existing code to accomodate possible
> vcpus. This also assumes that all processor being used are identical at least
> for now but does not affect the normal scanarios where they might not be in
> future. This assumption only affects the future hotplug scenarios if ever there
> exists any hetergenous processors. In such a case PMU might not be enabled on
> some vcpus. Is it acceptable and doable tradeoff for now?
>
> This perhaps needs more discussion. please check below link,
> Link: https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg00131.html
>
> Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
> hw/arm/virt.c | 51 ++++++++++++++++++++++++++++++-------------
> include/hw/arm/virt.h | 1 +
> 2 files changed, 37 insertions(+), 15 deletions(-)
>
I have a similar patch to this one in my steal-time dev branch that I just
started last week. I'm creating a new function that must be called after
cpu realization and gic creation that completes cpu setup. There's a few
things that will end up there (including steal-time stuff).
Thanks,
drew
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
` (24 preceding siblings ...)
2020-06-14 11:54 ` Marc Zyngier
@ 2020-06-23 9:12 ` Andrew Jones
2020-06-23 9:56 ` Salil Mehta
25 siblings, 1 reply; 33+ messages in thread
From: Andrew Jones @ 2020-06-23 9:12 UTC (permalink / raw)
To: Salil Mehta
Cc: peter.maydell, mehta.salil.lnk, gshan, pbonzini, mst, jiakernel2,
maz, david, richard.henderson, qemu-devel, linuxarm, eric.auger,
will, qemu-arm, james.morse, catalin.marinas, sudeep.holla,
imammedo, maran.wilson, zhukeqian1, wangxiongfeng2
On Sat, Jun 13, 2020 at 10:36:07PM +0100, Salil Mehta wrote:
> This patch-set introduces the virtual cpu hotplug support for ARMv8
> architecture in QEMU. Idea is to be able to hotplug and hot-unplug the vcpus
> while guest VM is running and no reboot is required. This does *not* makes any
> assumption of the physical cpu hotplug availability within the host system but
> rather tries to solve the problem at virtualizer/QEMU layer and by introducing
> cpu hotplug hooks and event handling within the guest kernel. No changes are
> required within the host kernel/KVM.
>
> Motivation:
> This allows scaling the guest VM compute capacity on-demand which would be
> useful for the following example scenarios,
> 1. Vertical Pod Autoscaling[3][4] in the cloud: Part of the orchestration
> framework which could adjust resource requests (CPU and Mem requests) for
> the containers in a pod, based on usage.
> 2. Pay-as-you-grow Business Model: Infrastructure provider could allocate and
> restrict the total number of compute resources available to the guest VM
> according to the SLA(Service Level Agreement). VM owner could request for
> more compute to be hot-plugged for some cost.
>
> Terminology:
>
> (*) Present cpus: Total cpus with which guest has/will boot and are available
> to guest for use and can be onlined. Qemu parameter(-smp)
> (*) Disabled cpus: Possible cpus which will not be available for the guest to
> use. These can be hotplugged and made present. These can be
> thought of as un-plugged vcpus. These will be included as
> part of sizing.
> (*) Posssible cpus: Total vcpus which could ever exist in VM. This includes
> booted cpus plus any cpus which could be later plugged.
> - Qemu parameter(-maxcpus)
> - Possible vcpus = Present vcpus (+) Disabled vcpus
>
>
> Limitations of ARMv8 Architecture:
>
> A. Physical Limitation to CPU Hotplug:
> 1. ARMv8 architecture does not support the concept of the physical cpu hotplug.
> The closest thing which is recomended to achieve the cpu hotplug on ARM is
> to bring down power state of the cpu using PSCI.
> 2. Other ARM components like GIC etc. have not been designed to realize
> physical cpu hotplug capability as of now.
>
> B. Limitations of GIC to Support Virtual CPU Hotplug:
> 1. GIC requires various resources(related to GICR/redistributor, GICC/cpu
> interface etc) like memory regions to be fixed at the VM init time and these
> could not be changed later on after VM has inited.
> 2. Associations between GICC(GIC cpu interface) and vcpu get fixed at the VM
> init time and GIC does not allows to change this association once GIC has
> initialized.
>
> C. Known Limitation of the KVM:
> 1. As of now KVM allows to create VCPUs but does not allows to delete the
> already created vcpus. QEMU already provides an interface to manage created
> vcpus at KVM level and then to re-use them.
> 2. Inconsistency in interpretation of the MPIDR generated by KVM for vcpus
> vis-a-vis SMT/threads. This does not looks to be compliant to the MPIDR
> format(SMT is present) as mentioned in the ARMv8 spec. (Please correct my
> understanding if I am wrong here?)
>
>
> Workaround to the problems mentioned in Section B & C1:
> 1. We pre-size the GIC with possible vcpus at VM init time
> 2. Pre-create all possible vcpus at KVM and associate them with GICC
> 3. Park the unplugged vcpus (similar to x86)
>
>
> (*) For all of above please refer to Marc's suggestion here[1]
>
>
> Overview of the Approach:
> At the time of machvirt_init() we pre-create all of the possible ARMCPU
> objects along with the corresponding KVM vcpus at the host. Disabled KVM vcpu
> (which are *not* "present" vcpus but are part of "possible" vcpu list) are
> parked at per VM list "kvm_parked_vcpus" after their initialization.
>
> We create the ARMCPU objects(but these are not *realized* in QOM sense) even
> for the disabled vcpus to facilitate the GIC initialization (pre-sized with
> possible vcpus). After Initialization of the machine is complete we release
> the ARMCPU Objects for the disabled vcpus. These ARMCPU object shall be
> re-created at the time when vcpu is hot plugged. This new object is then
> re-attached with the earlier parked KVM vcpu which also gets unparked. The
> ARMCPU object gets now "realized" in QEMU, which means creation of the
> corresponding threads, pre_plug/plug phases, and event notification to the
> guest using ACPI GED etc. Similarly, hot-unplug leg will lead to the
> "unrealization" of the vcpus and will lead to similar ACPI GED events to the
> guest for unplug and cleanup and eventually ARMCPU object shall be released and
> KVM vcpus shall be parked again.
>
> During machine init, ACPI MADT Table is sized with *possible* vcpus GICC
> entries. The unplugged/disabled vcpus are presented as MADT GICC DISABLED
> entries to the guest. This means the guest will have its resources pre-sized
> with possible vcpus(=present+disabled)
>
> Other approaches to deal with ARMCPU object release(after machine init):
> 1. The ARMCPU objects for the disabled vcpus are released in context to the
> virt_machine_done() notifier(approach adopted in this patch-set).
> 2. Defer the release of current ARMCPU object till the new vcpu object is
> hot plugged.
> 3. Never release and keep on reusing them and release once at VM exit. This
> solves many problems with above 2 approaches but requires change in the way
> qdev_device_add() fetches/creates the ARMCPU object for the new vcpus being
> hotplugged. For the arm cpu hotplug case we need to figure out way how to
> get access to old object and use it to "re-realize" instead of the new
> ARMCPU object.
>
> Concerns/Questions:
> 1. In ARM arch a cpu is uniquely represented in hierarchy using various
> affinity levels which could represent thread, core, cluster, package. This
> is generally represented by a value in MPIDR register as per the format
> mentioned in specification. Now, the way MPIDR value is derived for vcpus is
> done using vcpu-index. The concept of thread is not quite as same and rather
> gets lost in the derivation of MPIDR for vcpus.
> 2. The topology info used to specify the vcpu while hot-plugging might not
> match with the MPIDR value given back by the KVM for the vcpu at the time of
> init. Concept of SMT bit in MPIDR gets lost as per the derivation being done
> in the KVM. Hence, concept of thread-id, core-id, socket-id if used as a
> topology info to derive MPIDR value as per ARM specification will not match
> with MPIDR actually assigned by the KVM?
> Perhaps need to carry forward work of Andrew? please check here[2]
> 3. Further if this info is supplied to the guest using PPTT(once introduced in
> QEMU) or even derived using MPIDR shall be inconsistent with the host vcpu.
> 4. Any possibilities of interrupts(SGI/PPI/LPI/SPI) always remaining in
> *pending* state for the cpus which have been hot-unplugged? IMHO it looks
> okay but will need Marc's confirmation on this.
> 5. If the ARMCPU object is released after the machine init, UEFI could call
> back virt_update_table() to re-build the ACPI tables which might need an
> ARMCPU object. Please check the discussion here[5]
>
>
> Commands Used:
>
> A. Qemu launch commands to init the machine
>
> $ qemu-system-aarch64 --enable-kvm -machine virt,gic-version=3 \
> -cpu host -smp cpus=4,maxcpus=6 \
> -m 300M \
> -kernel Image \
> -initrd rootfs.cpio.gz \
> -append "console=ttyAMA0 root=/dev/ram rdinit=/init maxcpus=2 acpi=force" \
> -nographic \
> -bios QEMU_EFI.fd \
>
> B. Hot-(un)plug related commands
>
> # Hotplug a host vcpu(accel=kvm)
> $ device_add host-arm-cpu,id=core4,core-id=4
>
> # Hotplug a vcpu(accel=tcg)
> $ device_add cortex-a57-arm-cpu,id=core4,core-id=4
>
> # Delete the vcpu
> $ device_del core4
>
> NOTE: I have not tested the current solution with '-device' interface. The use
> is suggested by Igor here[6]. I will test this in coming times but looks
> it should work with existing changes.
>
>
> Sample output on guest after boot:
>
> $ cat /sys/devices/system/cpu/possible
> 0-5
> $ cat /sys/devices/system/cpu/present
> 0-3
> $ cat /sys/devices/system/cpu/online
> 0-1
> $ cat /sys/devices/system/cpu/offline
> 2-5
>
>
> Sample output on guest after hotplug of vcpu=4:
>
> $ cat /sys/devices/system/cpu/possible
> 0-5
> $ cat /sys/devices/system/cpu/present
> 0-4
> $ cat /sys/devices/system/cpu/online
> 0-1,4
> $ cat /sys/devices/system/cpu/offline
> 2-3,5
>
> Note: vcpu=4 was explicitly 'onlined' after hot-plug
> $ echo 1 > /sys/devices/system/cpu/cpu4/online
>
>
> Repository:
> (*) QEMU changes for vcpu hotplug could be cloned from below site,
> https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v1
>
> (*) Guest Kernel changes required to co-work with the QEMU shall be posted soon
> and repo made available at above site.
>
>
> THINGS TO DO:
> (*) Migration support
> (*) TCG/Emulation support is not proper right now. Works to a certain extent
> but is not complete. especially the unrealize part in which there is a
> overflow of tcg contexts. The last is due to the fact tcg maintains a
> count on number of context(per thread instance) so as we hotplug the vcpus
> this counter keeps on incrementing. But during hot-unplug the counter is
> not decremented.
> (*) Support of hotplug with NUMA is not proper
> (*) CPU Topology right now is not specified using thread/core/socket but
> rather flatly indexed using core-id. This needs consideration[2].
> (*) Do we need PPTT Support for to specify right topology info to guest about
> hot-plugged or unplugged vcpus?
> (*) Test cases
> (*) Docs need to be updated.
>
>
Hi Salil,
I realize this is just a preliminary posting and the approach hasn't been
finalized, but maybe in a future posting we can put a lot of this
information into a doc patch. I think we'll need good documentation for
this feature to ensure we get it right and keep in maintained correctly.
Thanks,
drew
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH RFC 01/22] arm/cpuhp: Add QMP vcpu params validation support
2020-06-23 8:46 ` Andrew Jones
@ 2020-06-23 9:40 ` Salil Mehta
0 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-23 9:40 UTC (permalink / raw)
To: Andrew Jones
Cc: peter.maydell, mehta.salil.lnk, gshan, pbonzini, mst, jiakernel2,
maz, david, richard.henderson, qemu-devel, Linuxarm, eric.auger,
will, qemu-arm, james.morse, catalin.marinas, sudeep.holla,
imammedo, maran.wilson, zhukeqian, wangxiongfeng (C)
Hi Andrew,
> From: Andrew Jones [mailto:drjones@redhat.com]
> Sent: Tuesday, June 23, 2020 9:46 AM
> To: Salil Mehta <salil.mehta@huawei.com>
> Cc: qemu-devel@nongnu.org; qemu-arm@nongnu.org; peter.maydell@linaro.org;
> sudeep.holla@arm.com; gshan@redhat.com; mst@redhat.com; jiakernel2@gmail.com;
> maz@kernel.org; zhukeqian <zhukeqian1@huawei.com>; david@redhat.com;
> richard.henderson@linaro.org; Linuxarm <linuxarm@huawei.com>;
> eric.auger@redhat.com; james.morse@arm.com; catalin.marinas@arm.com;
> imammedo@redhat.com; pbonzini@redhat.com; mehta.salil.lnk@gmail.com;
> maran.wilson@oracle.com; will@kernel.org; wangxiongfeng (C)
> <wangxiongfeng2@huawei.com>
> Subject: Re: [PATCH RFC 01/22] arm/cpuhp: Add QMP vcpu params validation support
>
> On Sat, Jun 13, 2020 at 10:36:08PM +0100, Salil Mehta wrote:
> > For now, vcpu hotplug is only supported with single socket single thread,
> > single die. NUMA is not supported either and everthing falls into single
> > node. Work to properly support these could be taken later once community
> > agrees with the base framework changes being presented to support ARM vcpu
> > hotplug in QEMU. Hence, these checks.
> >
> > Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > ---
> > hw/arm/virt.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 41 insertions(+)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 37462a6f78..5d1afdd031 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -2201,6 +2201,46 @@ static HotplugHandler
> *virt_machine_get_hotplug_handler(MachineState *machine,
> > return NULL;
> > }
> >
> > +static void virt_smp_parse(MachineState *ms, QemuOpts *opts)
> > +{
> > + if (opts) {
> > + unsigned cpus = qemu_opt_get_number(opts, "cpus", 1);
> > + unsigned sockets = qemu_opt_get_number(opts, "sockets", 1);
> > + unsigned cores = qemu_opt_get_number(opts, "cores", cpus);
> > + unsigned threads = qemu_opt_get_number(opts, "threads", 1);
> > + unsigned int max_cpus;
> > +
> > + if (sockets > 1 || threads > 1) {
> > + error_report("does not support more than one socket or thread");
> > + exit(1);
> > + }
> > +
> > + if (cores != cpus) {
> > + error_report("cpu topology: "
> > + "sockets (%u) * cores (%u) * threads (%u) < "
> > + "smp_cpus (%u)",
> > + sockets, cores, threads, cpus);
> > + exit(1);
> > + }
> > +
> > + max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
> > + if (sockets * cores * threads > max_cpus) {
> > + error_report("cpu topology: "
> > + "sockets (%u) * cores (%u) * threads (%u) > "
> > + "maxcpus (%u)",
> > + sockets, cores, threads,
> > + max_cpus);
> > + exit(1);
> > + }
> > +
> > + ms->smp.max_cpus = max_cpus;
> > + ms->smp.sockets = sockets;
> > + ms->smp.cpus = cpus;
> > + ms->smp.cores = cores;
> > + ms->smp.threads = threads;
> > + }
> > +}
> > +
> > /*
> > * for arm64 kvm_type [7-0] encodes the requested number of bits
> > * in the IPA address space
> > @@ -2266,6 +2306,7 @@ static void virt_machine_class_init(ObjectClass *oc,
> void *data)
> > mc->nvdimm_supported = true;
> > mc->auto_enable_numa_with_memhp = true;
> > mc->default_ram_id = "mach-virt.ram";
> > + mc->smp_parse = virt_smp_parse;
> >
> > object_class_property_add(oc, "acpi", "OnOffAuto",
> > virt_get_acpi, virt_set_acpi,
> > --
> > 2.17.1
> >
> >
> >
>
> Hi Salil,
>
> This patch and the whole series has inspired me to pick up the vcpu
> topology work again.
Thanks for looking into this QEMU series. I am glad to hear that this
work looks sensible at least :)
> In fact, I think it may be necessary in order
> to properly describe a cpu when hot[un]plugging.
Agreed. it is required. Since, MPIDR derived using topo specified by
the user should match that assigned by the KVM/Host(which derives it
using vcpu-id).
? I'll try to pull
> together an RFC soon, at least for TCG. For KVM, we may need to
> change KVM in order to allow user-controlled MPIDR. Although I'm
> not sure about that anymore, because, as you stated somewhere else,
> we already have user-controlled MPIDR to some degree, since KVM simply
> transforms the cpu index.
Yes, kind-of, if you actually see the KVM logic and how it derives the
MPIDR for vcpu in reset_mpidr() you will know that the only variable
it is using is vcpu-id which is specified by QEMU during the vcpu
creation time. This clearly means end MPIDR value is actually dependent
on QEMU and KVM/host just programs it. Of course user of the QEMU does
not have any direct interface and this is as of now done implicitly
at QEMU level.
But the thing which concerned me was the idea of threads. Marc recently
clarified about that aspect as well, that KVM always(with some exception
of thunderX) uses MT=0(a bit in MPIDR register) even for the hardware
which support threads. IMHO there has to be some way to distinguish
between threads and cores at the QEMU level whether QEMU fetches this
info using sysfs interface like lscpu etc does or devise some other way.
But QEMU should ensure that right thread/core info is being facilitated
to the guest kernel. This could either be done wrapped as part of the
MPIDR or PPTT Table.
>
> Regarding this patch specifically, I would change this to allow
> sockets, but prefer cores (i.e. when only '-smp N' is given, then
> N is the number of cores, not sockets). Also I would allow threads,
> but only for !kvm_enabled(). Then the function would be similar to
> something I think I once posted long ago, or at least wrote and maybe
> never posted...
Ok. We need to modify the logic to derive the arch-id/MPIDR using the
topology info instead of just vcpu-id. Hope you will be doing that
as well for TCG? and this will require check to distinguish the KVM and
TCG part there as well. I am specifically talking about {pre|post}_plug
functions. BTW, can 'N' (in -smp N) ever be sockets?
In case you require my help for anything like testing on real ARM64
server hardware or anything else please give a nudge and we can co-work
from here :)
Regarding kernel part, I will be posting the guest kernel changes
today to the community. If you have time please have a look at them
as well.
Many thanks
Salil.
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH RFC 07/22] arm/cpuhp: Init PMU at host for all possible vcpus
2020-06-23 9:00 ` Andrew Jones
@ 2020-06-23 9:52 ` Salil Mehta
0 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-23 9:52 UTC (permalink / raw)
To: Andrew Jones
Cc: peter.maydell, mehta.salil.lnk, gshan, pbonzini, mst, jiakernel2,
maz, david, richard.henderson, qemu-devel, Linuxarm, eric.auger,
will, qemu-arm, james.morse, catalin.marinas, sudeep.holla,
imammedo, maran.wilson, zhukeqian, wangxiongfeng (C)
> From: Andrew Jones [mailto:drjones@redhat.com]
> Sent: Tuesday, June 23, 2020 10:01 AM
> To: Salil Mehta <salil.mehta@huawei.com>
> Cc: qemu-devel@nongnu.org; qemu-arm@nongnu.org; peter.maydell@linaro.org;
> sudeep.holla@arm.com; gshan@redhat.com; mst@redhat.com; jiakernel2@gmail.com;
> maz@kernel.org; zhukeqian <zhukeqian1@huawei.com>; david@redhat.com;
> richard.henderson@linaro.org; Linuxarm <linuxarm@huawei.com>;
> eric.auger@redhat.com; james.morse@arm.com; catalin.marinas@arm.com;
> imammedo@redhat.com; pbonzini@redhat.com; mehta.salil.lnk@gmail.com;
> maran.wilson@oracle.com; will@kernel.org; wangxiongfeng (C)
> <wangxiongfeng2@huawei.com>
> Subject: Re: [PATCH RFC 07/22] arm/cpuhp: Init PMU at host for all possible vcpus
>
> On Sat, Jun 13, 2020 at 10:36:14PM +0100, Salil Mehta wrote:
> > PMU for all possible vcpus must be initialized at the virt machine
> > initialization time. This patch refactors existing code to accomodate possible
> > vcpus. This also assumes that all processor being used are identical at least
> > for now but does not affect the normal scanarios where they might not be in
> > future. This assumption only affects the future hotplug scenarios if ever there
> > exists any hetergenous processors. In such a case PMU might not be enabled
> on
> > some vcpus. Is it acceptable and doable tradeoff for now?
> >
> > This perhaps needs more discussion. please check below link,
> > Link: https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg00131.html
> >
> > Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > ---
> > hw/arm/virt.c | 51 ++++++++++++++++++++++++++++++-------------
> > include/hw/arm/virt.h | 1 +
> > 2 files changed, 37 insertions(+), 15 deletions(-)
> >
>
> I have a similar patch to this one in my steal-time dev branch that I just
> started last week. I'm creating a new function that must be called after
> cpu realization and gic creation that completes cpu setup. There's a few
> things that will end up there (including steal-time stuff).
ok.
>
> Thanks,
> drew
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch
2020-06-23 9:12 ` Andrew Jones
@ 2020-06-23 9:56 ` Salil Mehta
0 siblings, 0 replies; 33+ messages in thread
From: Salil Mehta @ 2020-06-23 9:56 UTC (permalink / raw)
To: Andrew Jones
Cc: peter.maydell, mehta.salil.lnk, gshan, pbonzini, mst, jiakernel2,
maz, david, richard.henderson, qemu-devel, Linuxarm, eric.auger,
will, qemu-arm, james.morse, catalin.marinas, sudeep.holla,
imammedo, maran.wilson, zhukeqian, wangxiongfeng (C)
> From: Andrew Jones [mailto:drjones@redhat.com]
> Sent: Tuesday, June 23, 2020 10:12 AM
> To: Salil Mehta <salil.mehta@huawei.com>
> Cc: qemu-devel@nongnu.org; qemu-arm@nongnu.org; peter.maydell@linaro.org;
> sudeep.holla@arm.com; gshan@redhat.com; mst@redhat.com; jiakernel2@gmail.com;
> maz@kernel.org; zhukeqian <zhukeqian1@huawei.com>; david@redhat.com;
> richard.henderson@linaro.org; Linuxarm <linuxarm@huawei.com>;
> eric.auger@redhat.com; james.morse@arm.com; catalin.marinas@arm.com;
> imammedo@redhat.com; pbonzini@redhat.com; mehta.salil.lnk@gmail.com;
> maran.wilson@oracle.com; will@kernel.org; wangxiongfeng (C)
> <wangxiongfeng2@huawei.com>
> Subject: Re: [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch
>
> On Sat, Jun 13, 2020 at 10:36:07PM +0100, Salil Mehta wrote:
> > This patch-set introduces the virtual cpu hotplug support for ARMv8
> > architecture in QEMU. Idea is to be able to hotplug and hot-unplug the vcpus
> > while guest VM is running and no reboot is required. This does *not* makes
> any
> > assumption of the physical cpu hotplug availability within the host system
> but
> > rather tries to solve the problem at virtualizer/QEMU layer and by introducing
> > cpu hotplug hooks and event handling within the guest kernel. No changes are
> > required within the host kernel/KVM.
> >
> > Motivation:
> > This allows scaling the guest VM compute capacity on-demand which would be
> > useful for the following example scenarios,
> > 1. Vertical Pod Autoscaling[3][4] in the cloud: Part of the orchestration
> > framework which could adjust resource requests (CPU and Mem requests) for
> > the containers in a pod, based on usage.
> > 2. Pay-as-you-grow Business Model: Infrastructure provider could allocate and
> > restrict the total number of compute resources available to the guest VM
> > according to the SLA(Service Level Agreement). VM owner could request for
> > more compute to be hot-plugged for some cost.
> >
> > Terminology:
> >
> > (*) Present cpus: Total cpus with which guest has/will boot and are available
> > to guest for use and can be onlined. Qemu parameter(-smp)
> > (*) Disabled cpus: Possible cpus which will not be available for the guest
> to
> > use. These can be hotplugged and made present. These can be
> > thought of as un-plugged vcpus. These will be included as
> > part of sizing.
> > (*) Posssible cpus: Total vcpus which could ever exist in VM. This includes
> > booted cpus plus any cpus which could be later plugged.
> > - Qemu parameter(-maxcpus)
> > - Possible vcpus = Present vcpus (+) Disabled vcpus
> >
> >
> > Limitations of ARMv8 Architecture:
> >
> > A. Physical Limitation to CPU Hotplug:
> > 1. ARMv8 architecture does not support the concept of the physical cpu hotplug.
> > The closest thing which is recomended to achieve the cpu hotplug on ARM
> is
> > to bring down power state of the cpu using PSCI.
> > 2. Other ARM components like GIC etc. have not been designed to realize
> > physical cpu hotplug capability as of now.
> >
> > B. Limitations of GIC to Support Virtual CPU Hotplug:
> > 1. GIC requires various resources(related to GICR/redistributor, GICC/cpu
> > interface etc) like memory regions to be fixed at the VM init time and these
> > could not be changed later on after VM has inited.
> > 2. Associations between GICC(GIC cpu interface) and vcpu get fixed at the VM
> > init time and GIC does not allows to change this association once GIC has
> > initialized.
> >
> > C. Known Limitation of the KVM:
> > 1. As of now KVM allows to create VCPUs but does not allows to delete the
> > already created vcpus. QEMU already provides an interface to manage created
> > vcpus at KVM level and then to re-use them.
> > 2. Inconsistency in interpretation of the MPIDR generated by KVM for vcpus
> > vis-a-vis SMT/threads. This does not looks to be compliant to the MPIDR
> > format(SMT is present) as mentioned in the ARMv8 spec. (Please correct my
> > understanding if I am wrong here?)
> >
> >
> > Workaround to the problems mentioned in Section B & C1:
> > 1. We pre-size the GIC with possible vcpus at VM init time
> > 2. Pre-create all possible vcpus at KVM and associate them with GICC
> > 3. Park the unplugged vcpus (similar to x86)
> >
> >
> > (*) For all of above please refer to Marc's suggestion here[1]
> >
> >
> > Overview of the Approach:
> > At the time of machvirt_init() we pre-create all of the possible ARMCPU
> > objects along with the corresponding KVM vcpus at the host. Disabled KVM vcpu
> > (which are *not* "present" vcpus but are part of "possible" vcpu list) are
> > parked at per VM list "kvm_parked_vcpus" after their initialization.
> >
> > We create the ARMCPU objects(but these are not *realized* in QOM sense) even
> > for the disabled vcpus to facilitate the GIC initialization (pre-sized with
> > possible vcpus). After Initialization of the machine is complete we release
> > the ARMCPU Objects for the disabled vcpus. These ARMCPU object shall be
> > re-created at the time when vcpu is hot plugged. This new object is then
> > re-attached with the earlier parked KVM vcpu which also gets unparked. The
> > ARMCPU object gets now "realized" in QEMU, which means creation of the
> > corresponding threads, pre_plug/plug phases, and event notification to the
> > guest using ACPI GED etc. Similarly, hot-unplug leg will lead to the
> > "unrealization" of the vcpus and will lead to similar ACPI GED events to the
> > guest for unplug and cleanup and eventually ARMCPU object shall be released
> and
> > KVM vcpus shall be parked again.
> >
> > During machine init, ACPI MADT Table is sized with *possible* vcpus GICC
> > entries. The unplugged/disabled vcpus are presented as MADT GICC DISABLED
> > entries to the guest. This means the guest will have its resources pre-sized
> > with possible vcpus(=present+disabled)
> >
> > Other approaches to deal with ARMCPU object release(after machine init):
> > 1. The ARMCPU objects for the disabled vcpus are released in context to the
> > virt_machine_done() notifier(approach adopted in this patch-set).
> > 2. Defer the release of current ARMCPU object till the new vcpu object is
> > hot plugged.
> > 3. Never release and keep on reusing them and release once at VM exit. This
> > solves many problems with above 2 approaches but requires change in the
> way
> > qdev_device_add() fetches/creates the ARMCPU object for the new vcpus being
> > hotplugged. For the arm cpu hotplug case we need to figure out way how to
> > get access to old object and use it to "re-realize" instead of the new
> > ARMCPU object.
> >
> > Concerns/Questions:
> > 1. In ARM arch a cpu is uniquely represented in hierarchy using various
> > affinity levels which could represent thread, core, cluster, package. This
> > is generally represented by a value in MPIDR register as per the format
> > mentioned in specification. Now, the way MPIDR value is derived for vcpus
> is
> > done using vcpu-index. The concept of thread is not quite as same and rather
> > gets lost in the derivation of MPIDR for vcpus.
> > 2. The topology info used to specify the vcpu while hot-plugging might not
> > match with the MPIDR value given back by the KVM for the vcpu at the time
> of
> > init. Concept of SMT bit in MPIDR gets lost as per the derivation being
> done
> > in the KVM. Hence, concept of thread-id, core-id, socket-id if used as a
> > topology info to derive MPIDR value as per ARM specification will not match
> > with MPIDR actually assigned by the KVM?
> > Perhaps need to carry forward work of Andrew? please check here[2]
> > 3. Further if this info is supplied to the guest using PPTT(once introduced
> in
> > QEMU) or even derived using MPIDR shall be inconsistent with the host vcpu.
> > 4. Any possibilities of interrupts(SGI/PPI/LPI/SPI) always remaining in
> > *pending* state for the cpus which have been hot-unplugged? IMHO it looks
> > okay but will need Marc's confirmation on this.
> > 5. If the ARMCPU object is released after the machine init, UEFI could call
> > back virt_update_table() to re-build the ACPI tables which might need an
> > ARMCPU object. Please check the discussion here[5]
> >
> >
> > Commands Used:
> >
> > A. Qemu launch commands to init the machine
> >
> > $ qemu-system-aarch64 --enable-kvm -machine virt,gic-version=3 \
> > -cpu host -smp cpus=4,maxcpus=6 \
> > -m 300M \
> > -kernel Image \
> > -initrd rootfs.cpio.gz \
> > -append "console=ttyAMA0 root=/dev/ram rdinit=/init maxcpus=2 acpi=force" \
> > -nographic \
> > -bios QEMU_EFI.fd \
> >
> > B. Hot-(un)plug related commands
> >
> > # Hotplug a host vcpu(accel=kvm)
> > $ device_add host-arm-cpu,id=core4,core-id=4
> >
> > # Hotplug a vcpu(accel=tcg)
> > $ device_add cortex-a57-arm-cpu,id=core4,core-id=4
> >
> > # Delete the vcpu
> > $ device_del core4
> >
> > NOTE: I have not tested the current solution with '-device' interface. The
> use
> > is suggested by Igor here[6]. I will test this in coming times but looks
> > it should work with existing changes.
> >
> >
> > Sample output on guest after boot:
> >
> > $ cat /sys/devices/system/cpu/possible
> > 0-5
> > $ cat /sys/devices/system/cpu/present
> > 0-3
> > $ cat /sys/devices/system/cpu/online
> > 0-1
> > $ cat /sys/devices/system/cpu/offline
> > 2-5
> >
> >
> > Sample output on guest after hotplug of vcpu=4:
> >
> > $ cat /sys/devices/system/cpu/possible
> > 0-5
> > $ cat /sys/devices/system/cpu/present
> > 0-4
> > $ cat /sys/devices/system/cpu/online
> > 0-1,4
> > $ cat /sys/devices/system/cpu/offline
> > 2-3,5
> >
> > Note: vcpu=4 was explicitly 'onlined' after hot-plug
> > $ echo 1 > /sys/devices/system/cpu/cpu4/online
> >
> >
> > Repository:
> > (*) QEMU changes for vcpu hotplug could be cloned from below site,
> > https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v1
> >
> > (*) Guest Kernel changes required to co-work with the QEMU shall be posted
> soon
> > and repo made available at above site.
> >
> >
> > THINGS TO DO:
> > (*) Migration support
> > (*) TCG/Emulation support is not proper right now. Works to a certain extent
> > but is not complete. especially the unrealize part in which there is a
> > overflow of tcg contexts. The last is due to the fact tcg maintains a
> > count on number of context(per thread instance) so as we hotplug the vcpus
> > this counter keeps on incrementing. But during hot-unplug the counter
> is
> > not decremented.
> > (*) Support of hotplug with NUMA is not proper
> > (*) CPU Topology right now is not specified using thread/core/socket but
> > rather flatly indexed using core-id. This needs consideration[2].
> > (*) Do we need PPTT Support for to specify right topology info to guest about
> > hot-plugged or unplugged vcpus?
> > (*) Test cases
> > (*) Docs need to be updated.
> >
> >
>
> Hi Salil,
>
> I realize this is just a preliminary posting and the approach hasn't been
> finalized, but maybe in a future posting we can put a lot of this
> information into a doc patch. I think we'll need good documentation for
> this feature to ensure we get it right and keep in maintained correctly.
Sure, let us do it once we converge on the concept.
Thanks
Salil.
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2020-06-23 9:57 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-13 21:36 [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 01/22] arm/cpuhp: Add QMP vcpu params validation support Salil Mehta
2020-06-23 8:46 ` Andrew Jones
2020-06-23 9:40 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 02/22] arm/cpuhp: Add new ARMCPU core-id property Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 03/22] arm/cpuhp: Add common cpu utility for possible vcpus Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 04/22] arm/cpuhp: Machine init time change common to vcpu {cold|hot}-plug Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 05/22] arm/cpuhp: Pre-create disabled possible vcpus @machine init Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 06/22] arm/cpuhp: Changes to pre-size GIC with " Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 07/22] arm/cpuhp: Init PMU at host for all possible vcpus Salil Mehta
2020-06-23 9:00 ` Andrew Jones
2020-06-23 9:52 ` Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 08/22] arm/cpuhp: Enable ACPI support for vcpu hotplug Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 09/22] arm/cpuhp: Init GED framework with cpu hotplug events Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 10/22] arm/cpuhp: Update CPUs AML with cpu-(ctrl)dev change Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 11/22] arm/cpuhp: Update GED _EVT method AML with cpu scan Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 12/22] arm/cpuhp: MADT Tbl change to size the guest with possible vcpus Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 13/22] arm/cpuhp: Add ACPI _MAT entry for Processor object Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 14/22] arm/cpuhp: Release objects for *disabled* possible vcpus after init Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 15/22] arm/cpuhp: Update ACPI GED framework to support vcpu hotplug Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 16/22] arm/cpuhp: Add/update basic hot-(un)plug framework Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 17/22] arm/cpuhp: Changes to (un)wire GICC<->VCPU IRQs during hot-(un)plug Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 18/22] arm/cpuhp: Changes to update GIC with vcpu hot-plug notification Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 19/22] arm/cpuhp: Changes required to (re)init the vcpu register info Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 20/22] arm/cpuhp: Update the guest(via GED) about cpu hot-(un)plug events Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 21/22] arm/cpuhp: Changes required for reset and to support next boot Salil Mehta
2020-06-13 21:36 ` [PATCH RFC 22/22] arm/cpuhp: Add support of *unrealize* ARMCPU during vcpu hot-unplug Salil Mehta
2020-06-13 22:24 ` [PATCH RFC 00/22] Support of Virtual CPU Hotplug for ARMv8 Arch no-reply
2020-06-13 22:26 ` no-reply
2020-06-14 11:54 ` Marc Zyngier
2020-06-15 10:19 ` Salil Mehta
2020-06-23 9:12 ` Andrew Jones
2020-06-23 9:56 ` Salil Mehta
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.