On Mon, Nov 09, 2020 at 11:01:48AM +0000, Salil Mehta wrote:
> > From: fangying
> > Sent: Monday, November 9, 2020 3:05 AM
> > To: peter.mayd...@linaro.org
> > Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org; drjo...@redhat.com;
> > imamm...@redhat.com; shannon.zha...@gmail.com; alistair.fran...@wdc.com;
> > Zhanghailiang <zhang.zhanghaili...@huawei.com>; Salil Mehta
> > <salil.me...@huawei.com>
> > Subject: [RFC PATCH v3 03/13] hw/arm/virt: Replace smp_parse with one that
> > prefers cores
> >
> > From: Andrew Jones <drjo...@redhat.com>
> >
> > The virt machine type has never used the CPU topology parameters, other
> > than number of online CPUs and max CPUs. When choosing how to allocate
> > those CPUs the default has been to assume cores. In preparation for
> > using the other CPU topology parameters let's use an smp_parse that
> > prefers cores over sockets. We can also enforce the topology matches
> > max_cpus check because we have no legacy to preserve.
>
>
> Hi Andrew,
> I am wondering if we need to take care of other levels of processor
> hierarchy as well in ARM64 like 'clusters'/'Dies'?
I don't know, but so far in Linux only x86 considers dies. Also, Linux
doesn't define the concept of a cluster in its arch-neutral cpu topology
API. With that in mind, I think we should just focus on sockets, cores,
and threads until we have a guest OS that can be used to test other
levels.
Thanks,
drew
>
> Thanks
>
>
> >
> > Signed-off-by: Andrew Jones <drjo...@redhat.com>
> > ---
> > hw/arm/virt.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 76 insertions(+)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index ea24b576c6..ba902b53ba 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -78,6 +78,8 @@
> > #include "hw/virtio/virtio-iommu.h"
> > #include "hw/char/pl011.h"
> > #include "qemu/guest-random.h"
> > +#include "qapi/qmp/qerror.h"
> > +#include "sysemu/replay.h"
> >
> > #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
> > static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
> > @@ -2444,6 +2446,79 @@ static int virt_kvm_type(MachineState *ms, const char
> > *type_str)
> > return requested_pa_size > 40 ? requested_pa_size : 0;
> > }
> >
> > +/*
> > + * Unlike smp_parse() in hw/core/machine.c, we prefer cores over sockets,
> > + * e.g. '-smp 8' creates 1 socket with 8 cores. Whereas '-smp 8' with
> > + * hw/core/machine.c's smp_parse() creates 8 sockets, each with 1 core.
> > + * Additionally, we can enforce the topology matches max_cpus check,
> > + * because we have no legacy to preserve.
> > + */
> > +static void virt_smp_parse(MachineState *ms, QemuOpts *opts)
> > +{
> > + if (opts) {
> > + unsigned cpus = qemu_opt_get_number(opts, "cpus", 0);
> > + unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
> > + unsigned cores = qemu_opt_get_number(opts, "cores", 0);
> > + unsigned threads = qemu_opt_get_number(opts, "threads", 0);
> > +
> > + /*
> > + * Compute missing values; prefer cores over sockets and
> > + * sockets over threads.
> > + */
> > + if (cpus == 0 || cores == 0) {
> > + sockets = sockets > 0 ? sockets : 1;
> > + threads = threads > 0 ? threads : 1;
> > + if (cpus == 0) {
> > + cores = cores > 0 ? cores : 1;
> > + cpus = cores * threads * sockets;
> > + } else {
> > + ms->smp.max_cpus = qemu_opt_get_number(opts, "maxcpus",
> > cpus);
> > + cores = ms->smp.max_cpus / (sockets * threads);
> > + }
> > + } else if (sockets == 0) {
> > + threads = threads > 0 ? threads : 1;
> > + sockets = cpus / (cores * threads);
> > + sockets = sockets > 0 ? sockets : 1;
> > + } else if (threads == 0) {
> > + threads = cpus / (cores * sockets);
> > + threads = threads > 0 ? threads : 1;
> > + } else if (sockets * cores * threads < cpus) {
> > + error_report("cpu topology: "
> > + "sockets (%u) * cores (%u) * threads (%u) < "
> > + "smp_cpus (%u)",
> > + sockets, cores, threads, cpus);
> > + exit(1);
> > + }
> > +
> > + ms->smp.max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
> > +
> > + if (ms->smp.max_cpus < cpus) {
> > + error_report("maxcpus must be equal to or greater than smp");
> > + exit(1);
> > + }
> > +
> > + if (sockets * cores * threads != ms->smp.max_cpus) {
> > + error_report("cpu topology: "
> > + "sockets (%u) * cores (%u) * threads (%u)"
> > + "!= maxcpus (%u)",
> > + sockets, cores, threads,
> > + ms->smp.max_cpus);
> > + exit(1);
> > + }
> > +
> > + ms->smp.cpus = cpus;
> > + ms->smp.cores = cores;
> > + ms->smp.threads = threads;
> > + ms->smp.sockets = sockets;
> > + }
> > +
> > + if (ms->smp.cpus > 1) {
> > + Error *blocker = NULL;
> > + error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
> > + replay_add_blocker(blocker);
> > + }
> > +}
> > +
> > static void virt_machine_class_init(ObjectClass *oc, void *data)
> > {
> > MachineClass *mc = MACHINE_CLASS(oc);
> > @@ -2469,6 +2544,7 @@ static void virt_machine_class_init(ObjectClass *oc,
> > void
> > *data)
> > mc->cpu_index_to_instance_props = virt_cpu_index_to_props;
> > mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
> > mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
> > + mc->smp_parse = virt_smp_parse;
> > mc->kvm_type = virt_kvm_type;
> > assert(!mc->get_hotplug_handler);
> > mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
> > --
> > 2.23.0
>
>
