| Message ID | 20200613213629.21984-2-salil.mehta@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support of Virtual CPU Hotplug for ARMv8 Arch | expand |
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
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.
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,
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(+) -- 2.17.1