* Re: [Qemu-devel] [RFC PATCH 2/2] numa: Add node_id data in query-hotpluggable-cpus
2016-07-07 15:17 ` [Qemu-devel] [RFC PATCH 2/2] numa: Add node_id data in query-hotpluggable-cpus Peter Krempa
@ 2016-07-07 16:10 ` Andrew Jones
2016-07-08 2:23 ` David Gibson
2016-07-08 11:54 ` Igor Mammedov
2 siblings, 0 replies; 18+ messages in thread
From: Andrew Jones @ 2016-07-07 16:10 UTC (permalink / raw)
To: Peter Krempa; +Cc: qemu-devel, dgibson, imammedo
On Thu, Jul 07, 2016 at 05:17:14PM +0200, Peter Krempa wrote:
> Add a helper that looks up the NUMA node for a given CPU and use it to
> fill the node_id in the PPC and X86 impls of query-hotpluggable-cpus.
>
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> hw/i386/pc.c | 7 +++++++
> hw/ppc/spapr.c | 8 ++++++--
> include/sysemu/numa.h | 1 +
> numa.c | 13 +++++++++++++
> 4 files changed, 27 insertions(+), 2 deletions(-)
The helper function should be a separate patch, and Igor beat you
to it, see
https://www.mail-archive.com/qemu-devel@nongnu.org/msg383461.html
drew
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 4ba02c4..a0b9507 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2115,6 +2115,7 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
> HotpluggableCPUList *head = NULL;
> PCMachineState *pcms = PC_MACHINE(machine);
> const char *cpu_type;
> + int node_id;
>
> cpu = pcms->possible_cpus->cpus[0].cpu;
> assert(cpu); /* BSP is always present */
> @@ -2138,6 +2139,12 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
> cpu_props->core_id = topo.core_id;
> cpu_props->has_thread_id = true;
> cpu_props->thread_id = topo.smt_id;
> +
> + if ((node_id = numa_node_get_by_cpu_index(i)) >= 0) {
> + cpu_props->has_node_id = true;
> + cpu_props->node_id = node_id;
> + }
> +
> cpu_item->props = cpu_props;
>
> cpu = pcms->possible_cpus->cpus[i].cpu;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d1f5195..06ba7fc 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2370,6 +2370,7 @@ static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
> sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
> int spapr_max_cores = max_cpus / smp_threads;
> int smt = kvmppc_smt_threads();
> + int node_id;
>
> for (i = 0; i < spapr_max_cores; i++) {
> HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> @@ -2381,8 +2382,11 @@ static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
> cpu_item->vcpu_id = i;
> cpu_props->has_core_id = true;
> cpu_props->core_id = i * smt;
> - /* TODO: add 'has_node/node' here to describe
> - to which node core belongs */
> +
> + if ((node_id = numa_node_get_by_cpu_index(i)) >= 0) {
> + cpu_props->has_node_id = true;
> + cpu_props->node_id = node_id;
> + }
>
> cpu_item->props = cpu_props;
> if (spapr->cores[i]) {
> diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
> index bb184c9..04d7097 100644
> --- a/include/sysemu/numa.h
> +++ b/include/sysemu/numa.h
> @@ -31,5 +31,6 @@ extern QemuOptsList qemu_numa_opts;
> void numa_set_mem_node_id(ram_addr_t addr, uint64_t size, uint32_t node);
> void numa_unset_mem_node_id(ram_addr_t addr, uint64_t size, uint32_t node);
> uint32_t numa_get_node(ram_addr_t addr, Error **errp);
> +int numa_node_get_by_cpu_index(int cpu_index);
>
> #endif
> diff --git a/numa.c b/numa.c
> index cbae430..365738a 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -506,6 +506,19 @@ void query_numa_node_mem(uint64_t node_mem[])
> }
> }
>
> +int numa_node_get_by_cpu_index(int cpu_index)
> +{
> + int i;
> +
> + for (i = 0; i < nb_numa_nodes; i++) {
> + if (test_bit(cpu_index, numa_info[i].node_cpu)) {
> + return i;
> + }
> + }
> +
> + return -1;
> +}
> +
> static int query_memdev(Object *obj, void *opaque)
> {
> MemdevList **list = opaque;
> --
> 2.9.0
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [RFC PATCH 2/2] numa: Add node_id data in query-hotpluggable-cpus
2016-07-07 15:17 ` [Qemu-devel] [RFC PATCH 2/2] numa: Add node_id data in query-hotpluggable-cpus Peter Krempa
2016-07-07 16:10 ` Andrew Jones
@ 2016-07-08 2:23 ` David Gibson
2016-07-08 7:46 ` Peter Krempa
2016-07-08 11:54 ` Igor Mammedov
2 siblings, 1 reply; 18+ messages in thread
From: David Gibson @ 2016-07-08 2:23 UTC (permalink / raw)
To: Peter Krempa; +Cc: qemu-devel, imammedo
[-- Attachment #1: Type: text/plain, Size: 4501 bytes --]
On Thu, 7 Jul 2016 17:17:14 +0200
Peter Krempa <pkrempa@redhat.com> wrote:
> Add a helper that looks up the NUMA node for a given CPU and use it to
> fill the node_id in the PPC and X86 impls of query-hotpluggable-cpus.
IIUC how the query thing works this means that the node id issued by
query-hotpluggable-cpus will be echoed back to device add by libvirt.
I'm not sure we actually process that information in the core at
present, so I don't know that that's right.
We need to be clear on which direction information is flowing here.
Does query-hotpluggable-cpus *define* the NUMA node allocation which is
then passed to the core device which implements it. Or is the NUMA
allocation defined elsewhere, and query-hotpluggable-cpus just reports
it.
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> hw/i386/pc.c | 7 +++++++
> hw/ppc/spapr.c | 8 ++++++--
> include/sysemu/numa.h | 1 +
> numa.c | 13 +++++++++++++
> 4 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 4ba02c4..a0b9507 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2115,6 +2115,7 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
> HotpluggableCPUList *head = NULL;
> PCMachineState *pcms = PC_MACHINE(machine);
> const char *cpu_type;
> + int node_id;
>
> cpu = pcms->possible_cpus->cpus[0].cpu;
> assert(cpu); /* BSP is always present */
> @@ -2138,6 +2139,12 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
> cpu_props->core_id = topo.core_id;
> cpu_props->has_thread_id = true;
> cpu_props->thread_id = topo.smt_id;
> +
> + if ((node_id = numa_node_get_by_cpu_index(i)) >= 0) {
> + cpu_props->has_node_id = true;
> + cpu_props->node_id = node_id;
> + }
> +
> cpu_item->props = cpu_props;
>
> cpu = pcms->possible_cpus->cpus[i].cpu;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d1f5195..06ba7fc 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2370,6 +2370,7 @@ static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
> sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
> int spapr_max_cores = max_cpus / smp_threads;
> int smt = kvmppc_smt_threads();
> + int node_id;
>
> for (i = 0; i < spapr_max_cores; i++) {
> HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> @@ -2381,8 +2382,11 @@ static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
> cpu_item->vcpu_id = i;
> cpu_props->has_core_id = true;
> cpu_props->core_id = i * smt;
> - /* TODO: add 'has_node/node' here to describe
> - to which node core belongs */
> +
> + if ((node_id = numa_node_get_by_cpu_index(i)) >= 0) {
As with the previous patch this is incorrect, becauyse
numa_node_get_by_cpu_index() is working from a vcpu (i.e. thread)
index, but you're passing a core index.
> + cpu_props->has_node_id = true;
> + cpu_props->node_id = node_id;
> + }
>
> cpu_item->props = cpu_props;
> if (spapr->cores[i]) {
> diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
> index bb184c9..04d7097 100644
> --- a/include/sysemu/numa.h
> +++ b/include/sysemu/numa.h
> @@ -31,5 +31,6 @@ extern QemuOptsList qemu_numa_opts;
> void numa_set_mem_node_id(ram_addr_t addr, uint64_t size, uint32_t node);
> void numa_unset_mem_node_id(ram_addr_t addr, uint64_t size, uint32_t node);
> uint32_t numa_get_node(ram_addr_t addr, Error **errp);
> +int numa_node_get_by_cpu_index(int cpu_index);
>
> #endif
> diff --git a/numa.c b/numa.c
> index cbae430..365738a 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -506,6 +506,19 @@ void query_numa_node_mem(uint64_t node_mem[])
> }
> }
>
> +int numa_node_get_by_cpu_index(int cpu_index)
> +{
> + int i;
> +
> + for (i = 0; i < nb_numa_nodes; i++) {
> + if (test_bit(cpu_index, numa_info[i].node_cpu)) {
> + return i;
> + }
> + }
> +
> + return -1;
> +}
> +
> static int query_memdev(Object *obj, void *opaque)
> {
> MemdevList **list = opaque;
> --
> 2.9.0
>
--
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [RFC PATCH 2/2] numa: Add node_id data in query-hotpluggable-cpus
2016-07-08 2:23 ` David Gibson
@ 2016-07-08 7:46 ` Peter Krempa
2016-07-08 12:06 ` Igor Mammedov
2016-07-12 3:27 ` David Gibson
0 siblings, 2 replies; 18+ messages in thread
From: Peter Krempa @ 2016-07-08 7:46 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-devel, imammedo
On Fri, Jul 08, 2016 at 12:23:08 +1000, David Gibson wrote:
> On Thu, 7 Jul 2016 17:17:14 +0200
> Peter Krempa <pkrempa@redhat.com> wrote:
>
> > Add a helper that looks up the NUMA node for a given CPU and use it to
> > fill the node_id in the PPC and X86 impls of query-hotpluggable-cpus.
>
>
> IIUC how the query thing works this means that the node id issued by
> query-hotpluggable-cpus will be echoed back to device add by libvirt.
It will be echoed back, but the problem is that it's configured
separately ...
> I'm not sure we actually process that information in the core at
> present, so I don't know that that's right.
>
> We need to be clear on which direction information is flowing here.
> Does query-hotpluggable-cpus *define* the NUMA node allocation which is
> then passed to the core device which implements it. Or is the NUMA
> allocation defined elsewhere, and query-hotpluggable-cpus just reports
> it.
Currently (in the pre-hotplug era) the NUMA topology is defined by
specifying a CPU numbers (see previous patch) on the commandline:
-numa node=1,cpus=1-5,cpus=8,cpus=11...
This is then reported to the guest.
For a machine started with:
-smp 5,maxcpus=8,sockets=2,cores=2,threads=2
-numa node,nodeid=0,cpus=0,cpus=2,cpus=4,cpus=6,mem=500
-numa node,nodeid=1,cpus=1,cpus=3,cpus=5,cpus=7,mem=500
you get the following topology that is not really possible with
hardware:
# lscpu
Architecture: x86_64
CPU op-mode(s): 32-bit, 64-bit
Byte Order: Little Endian
CPU(s): 5
On-line CPU(s) list: 0-4
Thread(s) per core: 1
Core(s) per socket: 2
Socket(s): 2
NUMA node(s): 2
Vendor ID: GenuineIntel
CPU family: 6
Model: 6
Model name: QEMU Virtual CPU version 2.5+
Stepping: 3
CPU MHz: 3504.318
BogoMIPS: 7008.63
Hypervisor vendor: KVM
Virtualization type: full
L1d cache: 32K
L1i cache: 32K
L2 cache: 4096K
NUMA node0 CPU(s): 0,2,4
NUMA node1 CPU(s): 1,3
Note that the count of cpus per numa node does not need to be identical.
As of the above 'query-hotpluggable-cpus' will need to report the data
that was configured above even if it doesn't make much sense in a real
world.
I did not try the above on a PPC host and thus I'm not sure whether the
config above is allowed or not.
While for the hotplug cpus it would be possible to plug in the correct
one according to the requested use the queried data but with the current
approach it's impossible to set the initial vcpus differently.
Peter
Note: For libvirt it's a no-go to start a throwaway qemu process just to
query the information and thus it's desired to have a way to configure
all this without the need to query with a specific machine
type/topology.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 2/2] numa: Add node_id data in query-hotpluggable-cpus
2016-07-08 7:46 ` Peter Krempa
@ 2016-07-08 12:06 ` Igor Mammedov
2016-07-08 12:26 ` Peter Krempa
2016-07-12 3:27 ` David Gibson
1 sibling, 1 reply; 18+ messages in thread
From: Igor Mammedov @ 2016-07-08 12:06 UTC (permalink / raw)
To: Peter Krempa; +Cc: David Gibson, qemu-devel
On Fri, 8 Jul 2016 09:46:00 +0200
Peter Krempa <pkrempa@redhat.com> wrote:
[...]
> Note: For libvirt it's a no-go to start a throwaway qemu process just to
> query the information and thus it's desired to have a way to configure
> all this without the need to query with a specific machine
> type/topology.
Is it no-go to start throwaway qemu on the new domain creation time?
i.e. user create a new domain, with specific -machine/-smp layout
libvirt stores results of query-hotpluggable-cpus and then allow user
in (eg)virt-manager to pick which CPUs are enabled and optionally to
which numa nodes they are mapped.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 2/2] numa: Add node_id data in query-hotpluggable-cpus
2016-07-08 12:06 ` Igor Mammedov
@ 2016-07-08 12:26 ` Peter Krempa
0 siblings, 0 replies; 18+ messages in thread
From: Peter Krempa @ 2016-07-08 12:26 UTC (permalink / raw)
To: Igor Mammedov; +Cc: David Gibson, qemu-devel
On Fri, Jul 08, 2016 at 14:06:31 +0200, Igor Mammedov wrote:
> On Fri, 8 Jul 2016 09:46:00 +0200
> Peter Krempa <pkrempa@redhat.com> wrote:
>
> [...]
> > Note: For libvirt it's a no-go to start a throwaway qemu process just to
> > query the information and thus it's desired to have a way to configure
> > all this without the need to query with a specific machine
> > type/topology.
> Is it no-go to start throwaway qemu on the new domain creation time?
Yes. The policy is that once we are starting the VM the qemu process
that we create is the one running the VM later. We can tweak stuff using
QMP and/or kill it if the configuration requested by the user can't be
achieved, but starting a different qemu process would not be accepted
upstream.
Capability querying is done prior to that with a qemu process with -M
none and the results are cached for the given combination of qemu binary
and libvirtd binary (even across restarts of the libvirtd binary).
> i.e. user create a new domain, with specific -machine/-smp layout
> libvirt stores results of query-hotpluggable-cpus and then allow user
> in (eg)virt-manager to pick which CPUs are enabled and optionally to
> which numa nodes they are mapped.
We can update certain stuff that was not explicitly configured by the
user to the state that will then be used by qemu.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 2/2] numa: Add node_id data in query-hotpluggable-cpus
2016-07-08 7:46 ` Peter Krempa
2016-07-08 12:06 ` Igor Mammedov
@ 2016-07-12 3:27 ` David Gibson
1 sibling, 0 replies; 18+ messages in thread
From: David Gibson @ 2016-07-12 3:27 UTC (permalink / raw)
To: Peter Krempa; +Cc: qemu-devel, imammedo
[-- Attachment #1: Type: text/plain, Size: 3494 bytes --]
On Fri, 8 Jul 2016 09:46:00 +0200
Peter Krempa <pkrempa@redhat.com> wrote:
> On Fri, Jul 08, 2016 at 12:23:08 +1000, David Gibson wrote:
> > On Thu, 7 Jul 2016 17:17:14 +0200
> > Peter Krempa <pkrempa@redhat.com> wrote:
> >
> > > Add a helper that looks up the NUMA node for a given CPU and use it to
> > > fill the node_id in the PPC and X86 impls of query-hotpluggable-cpus.
> >
> >
> > IIUC how the query thing works this means that the node id issued by
> > query-hotpluggable-cpus will be echoed back to device add by libvirt.
>
> It will be echoed back, but the problem is that it's configured
> separately ...
>
> > I'm not sure we actually process that information in the core at
> > present, so I don't know that that's right.
> >
> > We need to be clear on which direction information is flowing here.
> > Does query-hotpluggable-cpus *define* the NUMA node allocation which is
> > then passed to the core device which implements it. Or is the NUMA
> > allocation defined elsewhere, and query-hotpluggable-cpus just reports
> > it.
>
> Currently (in the pre-hotplug era) the NUMA topology is defined by
> specifying a CPU numbers (see previous patch) on the commandline:
>
> -numa node=1,cpus=1-5,cpus=8,cpus=11...
>
> This is then reported to the guest.
>
> For a machine started with:
>
> -smp 5,maxcpus=8,sockets=2,cores=2,threads=2
> -numa node,nodeid=0,cpus=0,cpus=2,cpus=4,cpus=6,mem=500
> -numa node,nodeid=1,cpus=1,cpus=3,cpus=5,cpus=7,mem=500
>
> you get the following topology that is not really possible with
> hardware:
>
> # lscpu
> Architecture: x86_64
> CPU op-mode(s): 32-bit, 64-bit
> Byte Order: Little Endian
> CPU(s): 5
> On-line CPU(s) list: 0-4
> Thread(s) per core: 1
> Core(s) per socket: 2
> Socket(s): 2
> NUMA node(s): 2
> Vendor ID: GenuineIntel
> CPU family: 6
> Model: 6
> Model name: QEMU Virtual CPU version 2.5+
> Stepping: 3
> CPU MHz: 3504.318
> BogoMIPS: 7008.63
> Hypervisor vendor: KVM
> Virtualization type: full
> L1d cache: 32K
> L1i cache: 32K
> L2 cache: 4096K
> NUMA node0 CPU(s): 0,2,4
> NUMA node1 CPU(s): 1,3
>
> Note that the count of cpus per numa node does not need to be identical.
>
> As of the above 'query-hotpluggable-cpus' will need to report the data
> that was configured above even if it doesn't make much sense in a real
> world.
>
> I did not try the above on a PPC host and thus I'm not sure whether the
> config above is allowed or not.
It's not - although I'm not sure that we actually have something
enforcing this.
However, single cores *must* be in the same NUMA node - there's no way
of reporting to the guest anything finer grained.
> While for the hotplug cpus it would be possible to plug in the correct
> one according to the requested use the queried data but with the current
> approach it's impossible to set the initial vcpus differently.
>
> Peter
>
> Note: For libvirt it's a no-go to start a throwaway qemu process just to
> query the information and thus it's desired to have a way to configure
> all this without the need to query with a specific machine
> type/topology.
--
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 2/2] numa: Add node_id data in query-hotpluggable-cpus
2016-07-07 15:17 ` [Qemu-devel] [RFC PATCH 2/2] numa: Add node_id data in query-hotpluggable-cpus Peter Krempa
2016-07-07 16:10 ` Andrew Jones
2016-07-08 2:23 ` David Gibson
@ 2016-07-08 11:54 ` Igor Mammedov
2016-07-08 12:04 ` Peter Krempa
2 siblings, 1 reply; 18+ messages in thread
From: Igor Mammedov @ 2016-07-08 11:54 UTC (permalink / raw)
To: Peter Krempa; +Cc: qemu-devel, dgibson
On Thu, 7 Jul 2016 17:17:14 +0200
Peter Krempa <pkrempa@redhat.com> wrote:
> Add a helper that looks up the NUMA node for a given CPU and use it to
> fill the node_id in the PPC and X86 impls of query-hotpluggable-cpus.
>
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> hw/i386/pc.c | 7 +++++++
> hw/ppc/spapr.c | 8 ++++++--
> include/sysemu/numa.h | 1 +
> numa.c | 13 +++++++++++++
> 4 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 4ba02c4..a0b9507 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2115,6 +2115,7 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
> HotpluggableCPUList *head = NULL;
> PCMachineState *pcms = PC_MACHINE(machine);
> const char *cpu_type;
> + int node_id;
>
> cpu = pcms->possible_cpus->cpus[0].cpu;
> assert(cpu); /* BSP is always present */
> @@ -2138,6 +2139,12 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
> cpu_props->core_id = topo.core_id;
> cpu_props->has_thread_id = true;
> cpu_props->thread_id = topo.smt_id;
> +
> + if ((node_id = numa_node_get_by_cpu_index(i)) >= 0) {
> + cpu_props->has_node_id = true;
> + cpu_props->node_id = node_id;
> + }
I've not included node_id for a reason,
"-numa cpus=1,2,3..." looks to me hopelessly broken now but
I've not came up with an idea how to redo it in nice and clean way yet.
Alternative could be CLI-less numa configuration, where QEMU is started
without "-numa cpus" but with "-S" then mgmt could call
query_hotpluggable_cpus() to get possible CPUs and then
map them to numa nodes with a new QMP command using attributes
it got from query_hotpluggable_cpus().
it's along the way start QEMU -smp 1,maxcpus=X and then add
remaining CPUs with device_add after getting properties from
query_hotpluggable_cpus().
then at machine_done time we can adjust DT/ACPI data to reflect
configured mapping.
> cpu_item->props = cpu_props;
>
> cpu = pcms->possible_cpus->cpus[i].cpu;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d1f5195..06ba7fc 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2370,6 +2370,7 @@ static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
> sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
> int spapr_max_cores = max_cpus / smp_threads;
> int smt = kvmppc_smt_threads();
> + int node_id;
>
> for (i = 0; i < spapr_max_cores; i++) {
> HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> @@ -2381,8 +2382,11 @@ static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
> cpu_item->vcpu_id = i;
> cpu_props->has_core_id = true;
> cpu_props->core_id = i * smt;
> - /* TODO: add 'has_node/node' here to describe
> - to which node core belongs */
> +
> + if ((node_id = numa_node_get_by_cpu_index(i)) >= 0) {
> + cpu_props->has_node_id = true;
> + cpu_props->node_id = node_id;
> + }
>
> cpu_item->props = cpu_props;
> if (spapr->cores[i]) {
> diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
> index bb184c9..04d7097 100644
> --- a/include/sysemu/numa.h
> +++ b/include/sysemu/numa.h
> @@ -31,5 +31,6 @@ extern QemuOptsList qemu_numa_opts;
> void numa_set_mem_node_id(ram_addr_t addr, uint64_t size, uint32_t node);
> void numa_unset_mem_node_id(ram_addr_t addr, uint64_t size, uint32_t node);
> uint32_t numa_get_node(ram_addr_t addr, Error **errp);
> +int numa_node_get_by_cpu_index(int cpu_index);
>
> #endif
> diff --git a/numa.c b/numa.c
> index cbae430..365738a 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -506,6 +506,19 @@ void query_numa_node_mem(uint64_t node_mem[])
> }
> }
>
> +int numa_node_get_by_cpu_index(int cpu_index)
> +{
> + int i;
> +
> + for (i = 0; i < nb_numa_nodes; i++) {
> + if (test_bit(cpu_index, numa_info[i].node_cpu)) {
> + return i;
> + }
> + }
> +
> + return -1;
> +}
> +
> static int query_memdev(Object *obj, void *opaque)
> {
> MemdevList **list = opaque;
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [RFC PATCH 2/2] numa: Add node_id data in query-hotpluggable-cpus
2016-07-08 11:54 ` Igor Mammedov
@ 2016-07-08 12:04 ` Peter Krempa
2016-07-08 12:10 ` Igor Mammedov
0 siblings, 1 reply; 18+ messages in thread
From: Peter Krempa @ 2016-07-08 12:04 UTC (permalink / raw)
To: Igor Mammedov; +Cc: qemu-devel, dgibson
On Fri, Jul 08, 2016 at 13:54:58 +0200, Igor Mammedov wrote:
> On Thu, 7 Jul 2016 17:17:14 +0200
> Peter Krempa <pkrempa@redhat.com> wrote:
>
> > Add a helper that looks up the NUMA node for a given CPU and use it to
> > fill the node_id in the PPC and X86 impls of query-hotpluggable-cpus.
> >
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> > hw/i386/pc.c | 7 +++++++
> > hw/ppc/spapr.c | 8 ++++++--
> > include/sysemu/numa.h | 1 +
> > numa.c | 13 +++++++++++++
> > 4 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 4ba02c4..a0b9507 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -2115,6 +2115,7 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
> > HotpluggableCPUList *head = NULL;
> > PCMachineState *pcms = PC_MACHINE(machine);
> > const char *cpu_type;
> > + int node_id;
> >
> > cpu = pcms->possible_cpus->cpus[0].cpu;
> > assert(cpu); /* BSP is always present */
> > @@ -2138,6 +2139,12 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
> > cpu_props->core_id = topo.core_id;
> > cpu_props->has_thread_id = true;
> > cpu_props->thread_id = topo.smt_id;
> > +
> > + if ((node_id = numa_node_get_by_cpu_index(i)) >= 0) {
> > + cpu_props->has_node_id = true;
> > + cpu_props->node_id = node_id;
> > + }
> I've not included node_id for a reason,
> "-numa cpus=1,2,3..." looks to me hopelessly broken now but
> I've not came up with an idea how to redo it in nice and clean way yet.
>
> Alternative could be CLI-less numa configuration, where QEMU is started
> without "-numa cpus" but with "-S" then mgmt could call
> query_hotpluggable_cpus() to get possible CPUs and then
> map them to numa nodes with a new QMP command using attributes
> it got from query_hotpluggable_cpus().
I think this could work for libvirt. The CPU index we currently expose
in the XML would become just a libvirt internal detail and the new QMP
command would be used to do the setup. Adding some QMP calls during VM
startup is okay.
> it's along the way start QEMU -smp 1,maxcpus=X and then add
> remaining CPUs with device_add after getting properties from
> query_hotpluggable_cpus().
I'm going to use a similar approach even for the hotpluggable cpus so I
can query the data for a new VM. On the other hand I can't make libvirt
use the approach with -smp 1,... all the time since we guarantee that a
XML that worked on a older version will be migratable back to the older
version.
> then at machine_done time we can adjust DT/ACPI data to reflect
> configured mapping.
In such case this series can be dropped since it provides what I need
differently.
Thanks,
Peter
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [RFC PATCH 2/2] numa: Add node_id data in query-hotpluggable-cpus
2016-07-08 12:04 ` Peter Krempa
@ 2016-07-08 12:10 ` Igor Mammedov
2016-07-08 12:53 ` Peter Krempa
0 siblings, 1 reply; 18+ messages in thread
From: Igor Mammedov @ 2016-07-08 12:10 UTC (permalink / raw)
To: Peter Krempa; +Cc: dgibson, qemu-devel
On Fri, 8 Jul 2016 14:04:23 +0200
Peter Krempa <pkrempa@redhat.com> wrote:
> On Fri, Jul 08, 2016 at 13:54:58 +0200, Igor Mammedov wrote:
> > On Thu, 7 Jul 2016 17:17:14 +0200
> > Peter Krempa <pkrempa@redhat.com> wrote:
> >
> > > Add a helper that looks up the NUMA node for a given CPU and use it to
> > > fill the node_id in the PPC and X86 impls of query-hotpluggable-cpus.
> > >
> > > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > > ---
> > > hw/i386/pc.c | 7 +++++++
> > > hw/ppc/spapr.c | 8 ++++++--
> > > include/sysemu/numa.h | 1 +
> > > numa.c | 13 +++++++++++++
> > > 4 files changed, 27 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > index 4ba02c4..a0b9507 100644
> > > --- a/hw/i386/pc.c
> > > +++ b/hw/i386/pc.c
> > > @@ -2115,6 +2115,7 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
> > > HotpluggableCPUList *head = NULL;
> > > PCMachineState *pcms = PC_MACHINE(machine);
> > > const char *cpu_type;
> > > + int node_id;
> > >
> > > cpu = pcms->possible_cpus->cpus[0].cpu;
> > > assert(cpu); /* BSP is always present */
> > > @@ -2138,6 +2139,12 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
> > > cpu_props->core_id = topo.core_id;
> > > cpu_props->has_thread_id = true;
> > > cpu_props->thread_id = topo.smt_id;
> > > +
> > > + if ((node_id = numa_node_get_by_cpu_index(i)) >= 0) {
> > > + cpu_props->has_node_id = true;
> > > + cpu_props->node_id = node_id;
> > > + }
> > I've not included node_id for a reason,
> > "-numa cpus=1,2,3..." looks to me hopelessly broken now but
> > I've not came up with an idea how to redo it in nice and clean way yet.
> >
> > Alternative could be CLI-less numa configuration, where QEMU is started
> > without "-numa cpus" but with "-S" then mgmt could call
> > query_hotpluggable_cpus() to get possible CPUs and then
> > map them to numa nodes with a new QMP command using attributes
> > it got from query_hotpluggable_cpus().
>
> I think this could work for libvirt. The CPU index we currently expose
> in the XML would become just a libvirt internal detail and the new QMP
> command would be used to do the setup. Adding some QMP calls during VM
> startup is okay.
>
> > it's along the way start QEMU -smp 1,maxcpus=X and then add
> > remaining CPUs with device_add after getting properties from
> > query_hotpluggable_cpus().
>
> I'm going to use a similar approach even for the hotpluggable cpus so I
> can query the data for a new VM. On the other hand I can't make libvirt
> use the approach with -smp 1,... all the time since we guarantee that a
> XML that worked on a older version will be migratable back to the older
> version.
Could you explain a little bit more about issue?
>
> > then at machine_done time we can adjust DT/ACPI data to reflect
> > configured mapping.
>
> In such case this series can be dropped since it provides what I need
> differently.
>
> Thanks,
>
> Peter
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [RFC PATCH 2/2] numa: Add node_id data in query-hotpluggable-cpus
2016-07-08 12:10 ` Igor Mammedov
@ 2016-07-08 12:53 ` Peter Krempa
0 siblings, 0 replies; 18+ messages in thread
From: Peter Krempa @ 2016-07-08 12:53 UTC (permalink / raw)
To: Igor Mammedov; +Cc: dgibson, qemu-devel
On Fri, Jul 08, 2016 at 14:10:59 +0200, Igor Mammedov wrote:
> On Fri, 8 Jul 2016 14:04:23 +0200
> Peter Krempa <pkrempa@redhat.com> wrote:
>
> > On Fri, Jul 08, 2016 at 13:54:58 +0200, Igor Mammedov wrote:
> > > On Thu, 7 Jul 2016 17:17:14 +0200
> > > Peter Krempa <pkrempa@redhat.com> wrote:
[...]
> > > it's along the way start QEMU -smp 1,maxcpus=X and then add
> > > remaining CPUs with device_add after getting properties from
> > > query_hotpluggable_cpus().
> >
> > I'm going to use a similar approach even for the hotpluggable cpus so I
> > can query the data for a new VM. On the other hand I can't make libvirt
> > use the approach with -smp 1,... all the time since we guarantee that a
> > XML that worked on a older version will be migratable back to the older
> > version.
> Could you explain a little bit more about issue?
Libvirt attempts to maintain compatibility of the XML definition file
even for migrations to older versions.
If you are able to start a VM with a XML on libvirt version 'A' then
when you use the same XML to start it on a newer version 'B' we still
need to be able to migrate the VM back to 'A'. This means that vCPUs
added via -device/device_add can be used only for configurations that
will explicitly have configuration enabled.
This shouldn't be a problem generally, but this means that we still
either the 'old' way to work properly or a approach that is compatible
with migration.
For the specific case of vCPU hotplug this means that if you don't
hotplug any CPU it needs to work with older libvirt including the numa
topology and all other possible stuff.
^ permalink raw reply [flat|nested] 18+ messages in thread