| Message ID | 59c8f2c8e80d705fb4981bddd9fe703c1205346c.1467904342.git.pkrempa@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 7 Jul 2016 17:17:13 +0200 Peter Krempa <pkrempa@redhat.com> wrote: > Add 'vcpu index' to the output of query hotpluggable cpus. This output > is identical to the linear cpu index taken by the 'cpus' attribute > passed to -numa. The problem is, the vcpu index of what? Each entry in the hotpluggable cpus table could represent more than one vcpu. > This will allow to reliably map the cpu number to a given topology > element without making mgmt apps to reimplement the mapping. > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > --- > hmp.c | 1 + > hw/i386/pc.c | 1 + > hw/ppc/spapr.c | 1 + > qapi-schema.json | 2 ++ > 4 files changed, 5 insertions(+) > > diff --git a/hmp.c b/hmp.c > index 0cf5baa..613601e 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -2450,6 +2450,7 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict) > monitor_printf(mon, " type: \"%s\"\n", l->value->type); > monitor_printf(mon, " vcpus_count: \"%" PRIu64 "\"\n", > l->value->vcpus_count); > + monitor_printf(mon, " vcpu_id: \"%" PRIu64 "\"\n", l->value->vcpu_id); > if (l->value->has_qom_path) { > monitor_printf(mon, " qom_path: \"%s\"\n", l->value->qom_path); > } > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index f293a0c..4ba02c4 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -2131,6 +2131,7 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine) > > cpu_item->type = g_strdup(cpu_type); > cpu_item->vcpus_count = 1; > + cpu_item->vcpu_id = i; > cpu_props->has_socket_id = true; > cpu_props->socket_id = topo.pkg_id; > cpu_props->has_core_id = true; > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 7f33a1b..d1f5195 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -2378,6 +2378,7 @@ static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine) > > cpu_item->type = spapr_get_cpu_core_type(machine->cpu_model); > cpu_item->vcpus_count = smp_threads; > + cpu_item->vcpu_id = i; This is wrong. This is the index of the core. The individual vcpus within the core will have ids starting at core_id and working up. > cpu_props->has_core_id = true; > cpu_props->core_id = i * smt; > /* TODO: add 'has_node/node' here to describe > diff --git a/qapi-schema.json b/qapi-schema.json > index ba3bf14..6db9294 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -4292,6 +4292,7 @@ > # @type: CPU object type for usage with device_add command > # @props: list of properties to be used for hotplugging CPU > # @vcpus-count: number of logical VCPU threads @HotpluggableCPU provides > +# @vcpu-id: linear index of the vcpu > # @qom-path: #optional link to existing CPU object if CPU is present or > # omitted if CPU is not present. > # > @@ -4300,6 +4301,7 @@ > { 'struct': 'HotpluggableCPU', > 'data': { 'type': 'str', > 'vcpus-count': 'int', > + 'vcpu-id': 'int', > 'props': 'CpuInstanceProperties', > '*qom-path': 'str' > } > -- > 2.9.0 >
On Fri, 8 Jul 2016 12:18:55 +1000 David Gibson <dgibson@redhat.com> wrote: > On Thu, 7 Jul 2016 17:17:13 +0200 > Peter Krempa <pkrempa@redhat.com> wrote: > > > Add 'vcpu index' to the output of query hotpluggable cpus. This output > > is identical to the linear cpu index taken by the 'cpus' attribute > > passed to -numa. > > > The problem is, the vcpu index of what? Each entry in the hotpluggable > cpus table could represent more than one vcpu. agreed, -numa cpus should take socket/core/thread-ids instead so that mgmt could do layout at start-up time for example if mgmt specifies -smp cpus=1,sockets=2,cores=2,maxcpus=4 it knows socket/core layout and can assign them as desired -numa nodeid=0,cpu=[socket-id=0,core-id=0] \ -numa nodeid=1,cpu=[socket-id=0,core-id=1] \ -numa nodeid=2,cpu=[socket-id=1] that of cause assuming that QEMU would guarantee IDs are are ranges [0..sockets), ... it's so for x86, can we guarantee it for spapr? > > > This will allow to reliably map the cpu number to a given topology > > element without making mgmt apps to reimplement the mapping. > > > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > > --- > > hmp.c | 1 + > > hw/i386/pc.c | 1 + > > hw/ppc/spapr.c | 1 + > > qapi-schema.json | 2 ++ > > 4 files changed, 5 insertions(+) > > > > diff --git a/hmp.c b/hmp.c > > index 0cf5baa..613601e 100644 > > --- a/hmp.c > > +++ b/hmp.c > > @@ -2450,6 +2450,7 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict) > > monitor_printf(mon, " type: \"%s\"\n", l->value->type); > > monitor_printf(mon, " vcpus_count: \"%" PRIu64 "\"\n", > > l->value->vcpus_count); > > + monitor_printf(mon, " vcpu_id: \"%" PRIu64 "\"\n", l->value->vcpu_id); > > if (l->value->has_qom_path) { > > monitor_printf(mon, " qom_path: \"%s\"\n", l->value->qom_path); > > } > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index f293a0c..4ba02c4 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -2131,6 +2131,7 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine) > > > > cpu_item->type = g_strdup(cpu_type); > > cpu_item->vcpus_count = 1; > > + cpu_item->vcpu_id = i; > > cpu_props->has_socket_id = true; > > cpu_props->socket_id = topo.pkg_id; > > cpu_props->has_core_id = true; > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 7f33a1b..d1f5195 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -2378,6 +2378,7 @@ static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine) > > > > cpu_item->type = spapr_get_cpu_core_type(machine->cpu_model); > > cpu_item->vcpus_count = smp_threads; > > + cpu_item->vcpu_id = i; > > This is wrong. This is the index of the core. The individual vcpus > within the core will have ids starting at core_id and working up. > > > cpu_props->has_core_id = true; > > cpu_props->core_id = i * smt; > > /* TODO: add 'has_node/node' here to describe > > diff --git a/qapi-schema.json b/qapi-schema.json > > index ba3bf14..6db9294 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -4292,6 +4292,7 @@ > > # @type: CPU object type for usage with device_add command > > # @props: list of properties to be used for hotplugging CPU > > # @vcpus-count: number of logical VCPU threads @HotpluggableCPU provides > > +# @vcpu-id: linear index of the vcpu > > # @qom-path: #optional link to existing CPU object if CPU is present or > > # omitted if CPU is not present. > > # > > @@ -4300,6 +4301,7 @@ > > { 'struct': 'HotpluggableCPU', > > 'data': { 'type': 'str', > > 'vcpus-count': 'int', > > + 'vcpu-id': 'int', > > 'props': 'CpuInstanceProperties', > > '*qom-path': 'str' > > } > > -- > > 2.9.0 > > > >
On Fri, 8 Jul 2016 13:40:38 +0200 Igor Mammedov <imammedo@redhat.com> wrote: > On Fri, 8 Jul 2016 12:18:55 +1000 > David Gibson <dgibson@redhat.com> wrote: > > > On Thu, 7 Jul 2016 17:17:13 +0200 > > Peter Krempa <pkrempa@redhat.com> wrote: > > > > > Add 'vcpu index' to the output of query hotpluggable cpus. This output > > > is identical to the linear cpu index taken by the 'cpus' attribute > > > passed to -numa. > > > > > > The problem is, the vcpu index of what? Each entry in the hotpluggable > > cpus table could represent more than one vcpu. > agreed, > -numa cpus should take socket/core/thread-ids instead so that mgmt > could do layout at start-up time > > for example if mgmt specifies > -smp cpus=1,sockets=2,cores=2,maxcpus=4 > it knows socket/core layout and can assign them as desired > -numa nodeid=0,cpu=[socket-id=0,core-id=0] \ > -numa nodeid=1,cpu=[socket-id=0,core-id=1] \ > -numa nodeid=2,cpu=[socket-id=1] > > that of cause assuming that QEMU would guarantee IDs are are ranges [0..sockets), ... > it's so for x86, can we guarantee it for spapr? Urgh.. we could for spapr, but I think it's bad idea to do that in general. For powernv (or others) we might want to use socket ids with a physical meaning (e.g. actual values of chip select lines) and those might not be contiguous. > > > > > > This will allow to reliably map the cpu number to a given topology > > > element without making mgmt apps to reimplement the mapping. > > > > > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > > > --- > > > hmp.c | 1 + > > > hw/i386/pc.c | 1 + > > > hw/ppc/spapr.c | 1 + > > > qapi-schema.json | 2 ++ > > > 4 files changed, 5 insertions(+) > > > > > > diff --git a/hmp.c b/hmp.c > > > index 0cf5baa..613601e 100644 > > > --- a/hmp.c > > > +++ b/hmp.c > > > @@ -2450,6 +2450,7 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict) > > > monitor_printf(mon, " type: \"%s\"\n", l->value->type); > > > monitor_printf(mon, " vcpus_count: \"%" PRIu64 "\"\n", > > > l->value->vcpus_count); > > > + monitor_printf(mon, " vcpu_id: \"%" PRIu64 "\"\n", l->value->vcpu_id); > > > if (l->value->has_qom_path) { > > > monitor_printf(mon, " qom_path: \"%s\"\n", l->value->qom_path); > > > } > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > > index f293a0c..4ba02c4 100644 > > > --- a/hw/i386/pc.c > > > +++ b/hw/i386/pc.c > > > @@ -2131,6 +2131,7 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine) > > > > > > cpu_item->type = g_strdup(cpu_type); > > > cpu_item->vcpus_count = 1; > > > + cpu_item->vcpu_id = i; > > > cpu_props->has_socket_id = true; > > > cpu_props->socket_id = topo.pkg_id; > > > cpu_props->has_core_id = true; > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > index 7f33a1b..d1f5195 100644 > > > --- a/hw/ppc/spapr.c > > > +++ b/hw/ppc/spapr.c > > > @@ -2378,6 +2378,7 @@ static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine) > > > > > > cpu_item->type = spapr_get_cpu_core_type(machine->cpu_model); > > > cpu_item->vcpus_count = smp_threads; > > > + cpu_item->vcpu_id = i; > > > > This is wrong. This is the index of the core. The individual vcpus > > within the core will have ids starting at core_id and working up. > > > > > cpu_props->has_core_id = true; > > > cpu_props->core_id = i * smt; > > > /* TODO: add 'has_node/node' here to describe > > > diff --git a/qapi-schema.json b/qapi-schema.json > > > index ba3bf14..6db9294 100644 > > > --- a/qapi-schema.json > > > +++ b/qapi-schema.json > > > @@ -4292,6 +4292,7 @@ > > > # @type: CPU object type for usage with device_add command > > > # @props: list of properties to be used for hotplugging CPU > > > # @vcpus-count: number of logical VCPU threads @HotpluggableCPU provides > > > +# @vcpu-id: linear index of the vcpu > > > # @qom-path: #optional link to existing CPU object if CPU is present or > > > # omitted if CPU is not present. > > > # > > > @@ -4300,6 +4301,7 @@ > > > { 'struct': 'HotpluggableCPU', > > > 'data': { 'type': 'str', > > > 'vcpus-count': 'int', > > > + 'vcpu-id': 'int', > > > 'props': 'CpuInstanceProperties', > > > '*qom-path': 'str' > > > } > > > -- > > > 2.9.0 > > > > > > > >
On Mon, 11 Jul 2016 13:30:38 +1000 David Gibson <dgibson@redhat.com> wrote: > On Fri, 8 Jul 2016 13:40:38 +0200 > Igor Mammedov <imammedo@redhat.com> wrote: > > > On Fri, 8 Jul 2016 12:18:55 +1000 > > David Gibson <dgibson@redhat.com> wrote: > > > > > On Thu, 7 Jul 2016 17:17:13 +0200 > > > Peter Krempa <pkrempa@redhat.com> wrote: > > > > > > > Add 'vcpu index' to the output of query hotpluggable cpus. This output > > > > is identical to the linear cpu index taken by the 'cpus' attribute > > > > passed to -numa. > > > > > > > > > The problem is, the vcpu index of what? Each entry in the hotpluggable > > > cpus table could represent more than one vcpu. > > agreed, > > -numa cpus should take socket/core/thread-ids instead so that mgmt > > could do layout at start-up time > > > > for example if mgmt specifies > > -smp cpus=1,sockets=2,cores=2,maxcpus=4 > > it knows socket/core layout and can assign them as desired > > -numa nodeid=0,cpu=[socket-id=0,core-id=0] \ > > -numa nodeid=1,cpu=[socket-id=0,core-id=1] \ > > -numa nodeid=2,cpu=[socket-id=1] > > > > that of cause assuming that QEMU would guarantee IDs are are ranges [0..sockets), ... > > it's so for x86, can we guarantee it for spapr? > > Urgh.. we could for spapr, but I think it's bad idea to do that in > general. For powernv (or others) we might want to use socket ids with > a physical meaning (e.g. actual values of chip select lines) and those > might not be contiguous. Are there any other ideas how we can design numa mapping interface, then? > > > > > > > > > > > This will allow to reliably map the cpu number to a given topology > > > > element without making mgmt apps to reimplement the mapping. > > > > > > > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > > > > --- > > > > hmp.c | 1 + > > > > hw/i386/pc.c | 1 + > > > > hw/ppc/spapr.c | 1 + > > > > qapi-schema.json | 2 ++ > > > > 4 files changed, 5 insertions(+) > > > > > > > > diff --git a/hmp.c b/hmp.c > > > > index 0cf5baa..613601e 100644 > > > > --- a/hmp.c > > > > +++ b/hmp.c > > > > @@ -2450,6 +2450,7 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict) > > > > monitor_printf(mon, " type: \"%s\"\n", l->value->type); > > > > monitor_printf(mon, " vcpus_count: \"%" PRIu64 "\"\n", > > > > l->value->vcpus_count); > > > > + monitor_printf(mon, " vcpu_id: \"%" PRIu64 "\"\n", l->value->vcpu_id); > > > > if (l->value->has_qom_path) { > > > > monitor_printf(mon, " qom_path: \"%s\"\n", l->value->qom_path); > > > > } > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > > > index f293a0c..4ba02c4 100644 > > > > --- a/hw/i386/pc.c > > > > +++ b/hw/i386/pc.c > > > > @@ -2131,6 +2131,7 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine) > > > > > > > > cpu_item->type = g_strdup(cpu_type); > > > > cpu_item->vcpus_count = 1; > > > > + cpu_item->vcpu_id = i; > > > > cpu_props->has_socket_id = true; > > > > cpu_props->socket_id = topo.pkg_id; > > > > cpu_props->has_core_id = true; > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > > index 7f33a1b..d1f5195 100644 > > > > --- a/hw/ppc/spapr.c > > > > +++ b/hw/ppc/spapr.c > > > > @@ -2378,6 +2378,7 @@ static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine) > > > > > > > > cpu_item->type = spapr_get_cpu_core_type(machine->cpu_model); > > > > cpu_item->vcpus_count = smp_threads; > > > > + cpu_item->vcpu_id = i; > > > > > > This is wrong. This is the index of the core. The individual vcpus > > > within the core will have ids starting at core_id and working up. > > > > > > > cpu_props->has_core_id = true; > > > > cpu_props->core_id = i * smt; > > > > /* TODO: add 'has_node/node' here to describe > > > > diff --git a/qapi-schema.json b/qapi-schema.json > > > > index ba3bf14..6db9294 100644 > > > > --- a/qapi-schema.json > > > > +++ b/qapi-schema.json > > > > @@ -4292,6 +4292,7 @@ > > > > # @type: CPU object type for usage with device_add command > > > > # @props: list of properties to be used for hotplugging CPU > > > > # @vcpus-count: number of logical VCPU threads @HotpluggableCPU provides > > > > +# @vcpu-id: linear index of the vcpu > > > > # @qom-path: #optional link to existing CPU object if CPU is present or > > > > # omitted if CPU is not present. > > > > # > > > > @@ -4300,6 +4301,7 @@ > > > > { 'struct': 'HotpluggableCPU', > > > > 'data': { 'type': 'str', > > > > 'vcpus-count': 'int', > > > > + 'vcpu-id': 'int', > > > > 'props': 'CpuInstanceProperties', > > > > '*qom-path': 'str' > > > > } > > > > -- > > > > 2.9.0 > > > > > > > > > > > > > >
diff --git a/hmp.c b/hmp.c index 0cf5baa..613601e 100644 --- a/hmp.c +++ b/hmp.c @@ -2450,6 +2450,7 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict) monitor_printf(mon, " type: \"%s\"\n", l->value->type); monitor_printf(mon, " vcpus_count: \"%" PRIu64 "\"\n", l->value->vcpus_count); + monitor_printf(mon, " vcpu_id: \"%" PRIu64 "\"\n", l->value->vcpu_id); if (l->value->has_qom_path) { monitor_printf(mon, " qom_path: \"%s\"\n", l->value->qom_path); } diff --git a/hw/i386/pc.c b/hw/i386/pc.c index f293a0c..4ba02c4 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -2131,6 +2131,7 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine) cpu_item->type = g_strdup(cpu_type); cpu_item->vcpus_count = 1; + cpu_item->vcpu_id = i; cpu_props->has_socket_id = true; cpu_props->socket_id = topo.pkg_id; cpu_props->has_core_id = true; diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 7f33a1b..d1f5195 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2378,6 +2378,7 @@ static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine) cpu_item->type = spapr_get_cpu_core_type(machine->cpu_model); cpu_item->vcpus_count = smp_threads; + 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 diff --git a/qapi-schema.json b/qapi-schema.json index ba3bf14..6db9294 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4292,6 +4292,7 @@ # @type: CPU object type for usage with device_add command # @props: list of properties to be used for hotplugging CPU # @vcpus-count: number of logical VCPU threads @HotpluggableCPU provides +# @vcpu-id: linear index of the vcpu # @qom-path: #optional link to existing CPU object if CPU is present or # omitted if CPU is not present. # @@ -4300,6 +4301,7 @@ { 'struct': 'HotpluggableCPU', 'data': { 'type': 'str', 'vcpus-count': 'int', + 'vcpu-id': 'int', 'props': 'CpuInstanceProperties', '*qom-path': 'str' }
Add 'vcpu index' to the output of query hotpluggable cpus. This output is identical to the linear cpu index taken by the 'cpus' attribute passed to -numa. This will allow to reliably map the cpu number to a given topology element without making mgmt apps to reimplement the mapping. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- hmp.c | 1 + hw/i386/pc.c | 1 + hw/ppc/spapr.c | 1 + qapi-schema.json | 2 ++ 4 files changed, 5 insertions(+)