| Message ID | 20170913132417.24384-19-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x cleanups and CPU hotplug via device_add | expand |
On Wed, 13 Sep 2017 15:24:13 +0200 David Hildenbrand <david@redhat.com> wrote: > CPU hotplug is only possible on a per core basis on s390x. So let's > add possible_cpus and properly wire everything up. > > Signed-off-by: David Hildenbrand <david@redhat.com> looks good to me from cpu hotplug infrastructure pov, Acked-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/s390x/s390-virtio-ccw.c | 41 +++++++++++++++++++++++++++++++++++++++++ > qapi-schema.json | 16 ++++++++++++++++ > 2 files changed, 57 insertions(+) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 3be81d96af..cc64a81321 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -50,6 +50,7 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) > > static void s390_init_cpus(MachineState *machine) > { > + MachineClass *mc = MACHINE_GET_CLASS(machine); > int i; > gchar *name; > > @@ -74,6 +75,9 @@ static void s390_init_cpus(MachineState *machine) > g_free(name); > } > > + /* initialize possible_cpus */ > + mc->possible_cpu_arch_ids(machine); > + > for (i = 0; i < smp_cpus; i++) { > s390x_new_cpu(machine->cpu_model, i, &error_fatal); > } > @@ -307,6 +311,7 @@ static void s390_cpu_plug(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp) > { > gchar *name; > + MachineState *ms = MACHINE(hotplug_dev); > S390CPU *cpu = S390_CPU(dev); > CPUState *cs = CPU(dev); > > @@ -314,6 +319,9 @@ static void s390_cpu_plug(HotplugHandler *hotplug_dev, > object_property_set_link(OBJECT(hotplug_dev), OBJECT(cs), name, > errp); > g_free(name); > + > + g_assert(!ms->possible_cpus->cpus[cpu->env.core_id].cpu); > + ms->possible_cpus->cpus[cpu->env.core_id].cpu = OBJECT(dev); > } > > static void s390_machine_reset(void) > @@ -346,6 +354,36 @@ static void s390_machine_device_unplug_request(HotplugHandler *hotplug_dev, > } > } > > +static CpuInstanceProperties s390_cpu_index_to_props(MachineState *machine, > + unsigned cpu_index) > +{ > + g_assert(machine->possible_cpus && cpu_index < machine->possible_cpus->len); > + > + return machine->possible_cpus->cpus[cpu_index].props; > +} > + > +static const CPUArchIdList *s390_possible_cpu_arch_ids(MachineState *ms) > +{ > + int i; > + > + if (ms->possible_cpus) { > + g_assert(ms->possible_cpus && ms->possible_cpus->len == max_cpus); > + return ms->possible_cpus; > + } > + > + ms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) + > + sizeof(CPUArchId) * max_cpus); > + ms->possible_cpus->len = max_cpus; > + for (i = 0; i < ms->possible_cpus->len; i++) { > + ms->possible_cpus->cpus[i].vcpus_count = 1; > + ms->possible_cpus->cpus[i].arch_id = i; > + ms->possible_cpus->cpus[i].props.has_core_id = true; > + ms->possible_cpus->cpus[i].props.core_id = i; > + } > + > + return ms->possible_cpus; > +} > + > static HotplugHandler *s390_get_hotplug_handler(MachineState *machine, > DeviceState *dev) > { > @@ -393,7 +431,10 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) > mc->no_sdcard = 1; > mc->use_sclp = 1; > mc->max_cpus = 248; > + mc->has_hotpluggable_cpus = true; > mc->get_hotplug_handler = s390_get_hotplug_handler; > + mc->cpu_index_to_instance_props = s390_cpu_index_to_props; > + mc->possible_cpu_arch_ids = s390_possible_cpu_arch_ids; > hc->plug = s390_machine_device_plug; > hc->unplug_request = s390_machine_device_unplug_request; > nc->nmi_monitor_handler = s390_nmi; > diff --git a/qapi-schema.json b/qapi-schema.json > index f3af2cb851..79e9f85404 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -3121,6 +3121,22 @@ > # } > # ]} > # > +# For s390x-virtio-ccw machine type started with -smp 1,maxcpus=2 -cpu qemu > +# (Since: 2.11): > +# > +# -> { "execute": "query-hotpluggable-cpus" } > +# <- {"return": [ > +# { > +# "type": "qemu-s390-cpu", "vcpus-count": 1, > +# "props": { "core-id": 1 } > +# }, > +# { > +# "qom-path": "/machine/unattached/device[0]", > +# "type": "qemu-s390-cpu", "vcpus-count": 1, > +# "props": { "core-id": 0 } > +# } > +# ]} > +# > ## > { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] } >
On Wed, 13 Sep 2017 15:24:13 +0200 David Hildenbrand <david@redhat.com> wrote: > CPU hotplug is only possible on a per core basis on s390x. So let's > add possible_cpus and properly wire everything up. s/properly wire everything up/wire everything up properly/ ? (fixing while... you get the idea) > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > hw/s390x/s390-virtio-ccw.c | 41 +++++++++++++++++++++++++++++++++++++++++ > qapi-schema.json | 16 ++++++++++++++++ > 2 files changed, 57 insertions(+)
On 13.09.2017 17:49, Cornelia Huck wrote: > On Wed, 13 Sep 2017 15:24:13 +0200 > David Hildenbrand <david@redhat.com> wrote: > >> CPU hotplug is only possible on a per core basis on s390x. So let's >> add possible_cpus and properly wire everything up. > > s/properly wire everything up/wire everything up properly/ > > ? > > (fixing while... you get the idea) :) Sure!
On Mon, 02 Oct 2017 09:46:41 +0200 Markus Armbruster <armbru@redhat.com> wrote: > David Hildenbrand <david@redhat.com> writes: > > > CPU hotplug is only possible on a per core basis on s390x. So let's > > add possible_cpus and properly wire everything up. > > > > Signed-off-by: David Hildenbrand <david@redhat.com> > [...] > > diff --git a/qapi-schema.json b/qapi-schema.json > > index f3af2cb851..79e9f85404 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -3121,6 +3121,22 @@ > > # } > > # ]} > > # > > +# For s390x-virtio-ccw machine type started with -smp 1,maxcpus=2 -cpu qemu > > +# (Since: 2.11): > > +# > > +# -> { "execute": "query-hotpluggable-cpus" } > > +# <- {"return": [ > > +# { > > +# "type": "qemu-s390-cpu", "vcpus-count": 1, > > +# "props": { "core-id": 1 } > > +# }, > > +# { > > +# "qom-path": "/machine/unattached/device[0]", > > +# "type": "qemu-s390-cpu", "vcpus-count": 1, > > +# "props": { "core-id": 0 } > > +# } > > +# ]} > > +# > > ## > > { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] } > > Please stick in '# Example:' like we do in query-hotpluggable-cpus's doc > comment. I expect the generated documentation to be illegible[*] > without it. Should '# Example:' go before each of the examples (instead of the beginning of the example block), then? > > > [*] If you want to perform a quick eye-over, try running "make > tests/qapi-schema/doc-good.test.txt" and examine the result. Hmm... I might be missing something in my setup: [cohuck@gondolin build]$ make tests/qapi-schema/doc-good.test.txt GEN tests/qapi-schema/doc-good.test.txt tests/qapi-schema/doc-good.test.texi:79: @subsection seen before @end deftp tests/qapi-schema/doc-good.test.texi:90: unmatched `@end deftp' make: *** [Makefile:695: tests/qapi-schema/doc-good.test.txt] Error 1
Cornelia Huck <cohuck@redhat.com> writes: > On Mon, 02 Oct 2017 09:46:41 +0200 > Markus Armbruster <armbru@redhat.com> wrote: > >> David Hildenbrand <david@redhat.com> writes: >> >> > CPU hotplug is only possible on a per core basis on s390x. So let's >> > add possible_cpus and properly wire everything up. >> > >> > Signed-off-by: David Hildenbrand <david@redhat.com> >> [...] >> > diff --git a/qapi-schema.json b/qapi-schema.json >> > index f3af2cb851..79e9f85404 100644 >> > --- a/qapi-schema.json >> > +++ b/qapi-schema.json >> > @@ -3121,6 +3121,22 @@ >> > # } >> > # ]} >> > # >> > +# For s390x-virtio-ccw machine type started with -smp 1,maxcpus=2 -cpu qemu >> > +# (Since: 2.11): >> > +# >> > +# -> { "execute": "query-hotpluggable-cpus" } >> > +# <- {"return": [ >> > +# { >> > +# "type": "qemu-s390-cpu", "vcpus-count": 1, >> > +# "props": { "core-id": 1 } >> > +# }, >> > +# { >> > +# "qom-path": "/machine/unattached/device[0]", >> > +# "type": "qemu-s390-cpu", "vcpus-count": 1, >> > +# "props": { "core-id": 0 } >> > +# } >> > +# ]} >> > +# >> > ## >> > { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] } >> >> Please stick in '# Example:' like we do in query-hotpluggable-cpus's doc >> comment. I expect the generated documentation to be illegible[*] >> without it. > > Should '# Example:' go before each of the examples (instead of the > beginning of the example block), then? Whatever makes the generated documentation look better. >> [*] If you want to perform a quick eye-over, try running "make >> tests/qapi-schema/doc-good.test.txt" and examine the result. > > Hmm... I might be missing something in my setup: > > [cohuck@gondolin build]$ make tests/qapi-schema/doc-good.test.txt > GEN tests/qapi-schema/doc-good.test.txt > tests/qapi-schema/doc-good.test.texi:79: @subsection seen before @end deftp > tests/qapi-schema/doc-good.test.texi:90: unmatched `@end deftp' > make: *** [Makefile:695: tests/qapi-schema/doc-good.test.txt] Error 1 Pasto, sorry! Try "make docs/interop/qemu-qmp-ref.txt". .pdf and .html also work.
On Wed, 04 Oct 2017 14:42:55 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Cornelia Huck <cohuck@redhat.com> writes: > > > On Mon, 02 Oct 2017 09:46:41 +0200 > > Markus Armbruster <armbru@redhat.com> wrote: > > > >> David Hildenbrand <david@redhat.com> writes: > >> > >> > CPU hotplug is only possible on a per core basis on s390x. So let's > >> > add possible_cpus and properly wire everything up. > >> > > >> > Signed-off-by: David Hildenbrand <david@redhat.com> > >> [...] > >> > diff --git a/qapi-schema.json b/qapi-schema.json > >> > index f3af2cb851..79e9f85404 100644 > >> > --- a/qapi-schema.json > >> > +++ b/qapi-schema.json > >> > @@ -3121,6 +3121,22 @@ > >> > # } > >> > # ]} > >> > # > >> > +# For s390x-virtio-ccw machine type started with -smp 1,maxcpus=2 -cpu qemu > >> > +# (Since: 2.11): > >> > +# > >> > +# -> { "execute": "query-hotpluggable-cpus" } > >> > +# <- {"return": [ > >> > +# { > >> > +# "type": "qemu-s390-cpu", "vcpus-count": 1, > >> > +# "props": { "core-id": 1 } > >> > +# }, > >> > +# { > >> > +# "qom-path": "/machine/unattached/device[0]", > >> > +# "type": "qemu-s390-cpu", "vcpus-count": 1, > >> > +# "props": { "core-id": 0 } > >> > +# } > >> > +# ]} > >> > +# > >> > ## > >> > { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] } > >> > >> Please stick in '# Example:' like we do in query-hotpluggable-cpus's doc > >> comment. I expect the generated documentation to be illegible[*] > >> without it. > > > > Should '# Example:' go before each of the examples (instead of the > > beginning of the example block), then? > > Whatever makes the generated documentation look better. The pdf indeed looks ugly, but that's a preexisting problem, and adding more '# Example:' lines does not really help. The basic issue is that the code expects just one example and no additional text for the '# Example:' tag -- the explaining text for the various statements ends up looking like the qmp examples. That should be improved before doing a patch on top for this command (and I don't really have the skills or time to improve the output, sorry.)
Cornelia Huck <cohuck@redhat.com> writes: > On Wed, 04 Oct 2017 14:42:55 +0200 > Markus Armbruster <armbru@redhat.com> wrote: > >> Cornelia Huck <cohuck@redhat.com> writes: >> >> > On Mon, 02 Oct 2017 09:46:41 +0200 >> > Markus Armbruster <armbru@redhat.com> wrote: >> > >> >> David Hildenbrand <david@redhat.com> writes: >> >> >> >> > CPU hotplug is only possible on a per core basis on s390x. So let's >> >> > add possible_cpus and properly wire everything up. >> >> > >> >> > Signed-off-by: David Hildenbrand <david@redhat.com> >> >> [...] >> >> > diff --git a/qapi-schema.json b/qapi-schema.json >> >> > index f3af2cb851..79e9f85404 100644 >> >> > --- a/qapi-schema.json >> >> > +++ b/qapi-schema.json >> >> > @@ -3121,6 +3121,22 @@ >> >> > # } >> >> > # ]} >> >> > # >> >> > +# For s390x-virtio-ccw machine type started with -smp 1,maxcpus=2 -cpu qemu >> >> > +# (Since: 2.11): >> >> > +# >> >> > +# -> { "execute": "query-hotpluggable-cpus" } >> >> > +# <- {"return": [ >> >> > +# { >> >> > +# "type": "qemu-s390-cpu", "vcpus-count": 1, >> >> > +# "props": { "core-id": 1 } >> >> > +# }, >> >> > +# { >> >> > +# "qom-path": "/machine/unattached/device[0]", >> >> > +# "type": "qemu-s390-cpu", "vcpus-count": 1, >> >> > +# "props": { "core-id": 0 } >> >> > +# } >> >> > +# ]} >> >> > +# >> >> > ## >> >> > { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] } >> >> >> >> Please stick in '# Example:' like we do in query-hotpluggable-cpus's doc >> >> comment. I expect the generated documentation to be illegible[*] >> >> without it. >> > >> > Should '# Example:' go before each of the examples (instead of the >> > beginning of the example block), then? >> >> Whatever makes the generated documentation look better. > > The pdf indeed looks ugly, but that's a preexisting problem, and adding > more '# Example:' lines does not really help. > > The basic issue is that the code expects just one example and no > additional text for the '# Example:' tag -- the explaining text for the > various statements ends up looking like the qmp examples. That should > be improved before doing a patch on top for this command (and I don't > really have the skills or time to improve the output, sorry.) Marc-André, any ideas?
Hi ----- Original Message ----- > Cornelia Huck <cohuck@redhat.com> writes: > > > On Wed, 04 Oct 2017 14:42:55 +0200 > > Markus Armbruster <armbru@redhat.com> wrote: > > > >> Cornelia Huck <cohuck@redhat.com> writes: > >> > >> > On Mon, 02 Oct 2017 09:46:41 +0200 > >> > Markus Armbruster <armbru@redhat.com> wrote: > >> > > >> >> David Hildenbrand <david@redhat.com> writes: > >> >> > >> >> > CPU hotplug is only possible on a per core basis on s390x. So let's > >> >> > add possible_cpus and properly wire everything up. > >> >> > > >> >> > Signed-off-by: David Hildenbrand <david@redhat.com> > >> >> [...] > >> >> > diff --git a/qapi-schema.json b/qapi-schema.json > >> >> > index f3af2cb851..79e9f85404 100644 > >> >> > --- a/qapi-schema.json > >> >> > +++ b/qapi-schema.json > >> >> > @@ -3121,6 +3121,22 @@ > >> >> > # } > >> >> > # ]} > >> >> > # > >> >> > +# For s390x-virtio-ccw machine type started with -smp 1,maxcpus=2 > >> >> > -cpu qemu > >> >> > +# (Since: 2.11): > >> >> > +# > >> >> > +# -> { "execute": "query-hotpluggable-cpus" } > >> >> > +# <- {"return": [ > >> >> > +# { > >> >> > +# "type": "qemu-s390-cpu", "vcpus-count": 1, > >> >> > +# "props": { "core-id": 1 } > >> >> > +# }, > >> >> > +# { > >> >> > +# "qom-path": "/machine/unattached/device[0]", > >> >> > +# "type": "qemu-s390-cpu", "vcpus-count": 1, > >> >> > +# "props": { "core-id": 0 } > >> >> > +# } > >> >> > +# ]} > >> >> > +# > >> >> > ## > >> >> > { 'command': 'query-hotpluggable-cpus', 'returns': > >> >> > ['HotpluggableCPU'] } > >> >> > >> >> Please stick in '# Example:' like we do in query-hotpluggable-cpus's > >> >> doc > >> >> comment. I expect the generated documentation to be illegible[*] > >> >> without it. > >> > > >> > Should '# Example:' go before each of the examples (instead of the > >> > beginning of the example block), then? > >> > >> Whatever makes the generated documentation look better. > > > > The pdf indeed looks ugly, but that's a preexisting problem, and adding > > more '# Example:' lines does not really help. > > > > The basic issue is that the code expects just one example and no > > additional text for the '# Example:' tag -- the explaining text for the > > various statements ends up looking like the qmp examples. That should > > be improved before doing a patch on top for this command (and I don't > > really have the skills or time to improve the output, sorry.) > > Marc-André, any ideas? > It's not supported atm. The ideas to fix this are pretty much a matter of taste. I'd suggest to keep the code below the Example: section as @example/verbatim by default. And introduce a new section syntax, like Example:: to do "manual" formatting. Example:: This text would be in regular format. 1. and allow list, *strong* etc. | -> { then have some code } | <- ... And regulat text again.
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 3be81d96af..cc64a81321 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -50,6 +50,7 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) static void s390_init_cpus(MachineState *machine) { + MachineClass *mc = MACHINE_GET_CLASS(machine); int i; gchar *name; @@ -74,6 +75,9 @@ static void s390_init_cpus(MachineState *machine) g_free(name); } + /* initialize possible_cpus */ + mc->possible_cpu_arch_ids(machine); + for (i = 0; i < smp_cpus; i++) { s390x_new_cpu(machine->cpu_model, i, &error_fatal); } @@ -307,6 +311,7 @@ static void s390_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { gchar *name; + MachineState *ms = MACHINE(hotplug_dev); S390CPU *cpu = S390_CPU(dev); CPUState *cs = CPU(dev); @@ -314,6 +319,9 @@ static void s390_cpu_plug(HotplugHandler *hotplug_dev, object_property_set_link(OBJECT(hotplug_dev), OBJECT(cs), name, errp); g_free(name); + + g_assert(!ms->possible_cpus->cpus[cpu->env.core_id].cpu); + ms->possible_cpus->cpus[cpu->env.core_id].cpu = OBJECT(dev); } static void s390_machine_reset(void) @@ -346,6 +354,36 @@ static void s390_machine_device_unplug_request(HotplugHandler *hotplug_dev, } } +static CpuInstanceProperties s390_cpu_index_to_props(MachineState *machine, + unsigned cpu_index) +{ + g_assert(machine->possible_cpus && cpu_index < machine->possible_cpus->len); + + return machine->possible_cpus->cpus[cpu_index].props; +} + +static const CPUArchIdList *s390_possible_cpu_arch_ids(MachineState *ms) +{ + int i; + + if (ms->possible_cpus) { + g_assert(ms->possible_cpus && ms->possible_cpus->len == max_cpus); + return ms->possible_cpus; + } + + ms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) + + sizeof(CPUArchId) * max_cpus); + ms->possible_cpus->len = max_cpus; + for (i = 0; i < ms->possible_cpus->len; i++) { + ms->possible_cpus->cpus[i].vcpus_count = 1; + ms->possible_cpus->cpus[i].arch_id = i; + ms->possible_cpus->cpus[i].props.has_core_id = true; + ms->possible_cpus->cpus[i].props.core_id = i; + } + + return ms->possible_cpus; +} + static HotplugHandler *s390_get_hotplug_handler(MachineState *machine, DeviceState *dev) { @@ -393,7 +431,10 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) mc->no_sdcard = 1; mc->use_sclp = 1; mc->max_cpus = 248; + mc->has_hotpluggable_cpus = true; mc->get_hotplug_handler = s390_get_hotplug_handler; + mc->cpu_index_to_instance_props = s390_cpu_index_to_props; + mc->possible_cpu_arch_ids = s390_possible_cpu_arch_ids; hc->plug = s390_machine_device_plug; hc->unplug_request = s390_machine_device_unplug_request; nc->nmi_monitor_handler = s390_nmi; diff --git a/qapi-schema.json b/qapi-schema.json index f3af2cb851..79e9f85404 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3121,6 +3121,22 @@ # } # ]} # +# For s390x-virtio-ccw machine type started with -smp 1,maxcpus=2 -cpu qemu +# (Since: 2.11): +# +# -> { "execute": "query-hotpluggable-cpus" } +# <- {"return": [ +# { +# "type": "qemu-s390-cpu", "vcpus-count": 1, +# "props": { "core-id": 1 } +# }, +# { +# "qom-path": "/machine/unattached/device[0]", +# "type": "qemu-s390-cpu", "vcpus-count": 1, +# "props": { "core-id": 0 } +# } +# ]} +# ## { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
CPU hotplug is only possible on a per core basis on s390x. So let's add possible_cpus and properly wire everything up. Signed-off-by: David Hildenbrand <david@redhat.com> --- hw/s390x/s390-virtio-ccw.c | 41 +++++++++++++++++++++++++++++++++++++++++ qapi-schema.json | 16 ++++++++++++++++ 2 files changed, 57 insertions(+)