| Message ID | 20230201132051.126868-11-pmorel@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x: CPU Topology | expand |
On Wed, 2023-02-01 at 14:20 +0100, Pierre Morel wrote: > When the guest asks to change the polarity this change > is forwarded to the admin using QAPI. > The admin is supposed to take according decisions concerning > CPU provisioning. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > qapi/machine-target.json | 30 ++++++++++++++++++++++++++++++ > hw/s390x/cpu-topology.c | 2 ++ > 2 files changed, 32 insertions(+) > > diff --git a/qapi/machine-target.json b/qapi/machine-target.json > index 58df0f5061..5883c3b020 100644 > --- a/qapi/machine-target.json > +++ b/qapi/machine-target.json > @@ -371,3 +371,33 @@ > }, > 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] } > } > + > +## > +# @CPU_POLARITY_CHANGE: > +# > +# Emitted when the guest asks to change the polarity. > +# > +# @polarity: polarity specified by the guest > +# > +# The guest can tell the host (via the PTF instruction) whether the > +# CPUs should be provisioned using horizontal or vertical polarity. > +# > +# On horizontal polarity the host is expected to provision all vCPUs > +# equally. > +# On vertical polarity the host can provision each vCPU differently. > +# The guest will get information on the details of the provisioning > +# the next time it uses the STSI(15) instruction. > +# > +# Since: 8.0 > +# > +# Example: > +# > +# <- { "event": "CPU_POLARITY_CHANGE", > +# "data": { "polarity": 0 }, > +# "timestamp": { "seconds": 1401385907, "microseconds": 422329 } } > +# > +## > +{ 'event': 'CPU_POLARITY_CHANGE', > + 'data': { 'polarity': 'int' }, > + 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM'] } I wonder if you should depend on CONFIG_KVM or not. If tcg gets topology support it will use the same event and right now it would just never be emitted. On the other hand it's more conservative this way. I also wonder if you should add 'feature' : [ 'unstable' ]. On the upside, it would mark the event as unstable, but I don't know what the consequences are exactly. Also I guess one can remove qemu events without breaking backwards compatibility, since they just won't be emitted? Unless I guess you specify that a event must occur under certain situations and the client waits on it? Patch looks good. > +} > diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c > index 6c50050991..2f8e1b60cf 100644 > --- a/hw/s390x/cpu-topology.c > +++ b/hw/s390x/cpu-topology.c > @@ -19,6 +19,7 @@ > #include "hw/s390x/s390-virtio-ccw.h" > #include "hw/s390x/cpu-topology.h" > #include "qapi/qapi-commands-machine-target.h" > +#include "qapi/qapi-events-machine-target.h" > #include "qapi/qmp/qdict.h" > #include "monitor/hmp.h" > #include "monitor/monitor.h" > @@ -163,6 +164,7 @@ void s390_handle_ptf(S390CPU *cpu, uint8_t r1, uintptr_t ra) > s390_topology.polarity = fc; > s390_cpu_topology_set_modified(); > s390_topology_set_cpus_polarity(fc); > + qapi_event_send_cpu_polarity_change(fc); > setcc(cpu, 0); > } > break;
Nina Schoetterl-Glausch <nsg@linux.ibm.com> writes: > On Wed, 2023-02-01 at 14:20 +0100, Pierre Morel wrote: >> When the guest asks to change the polarity this change >> is forwarded to the admin using QAPI. >> The admin is supposed to take according decisions concerning >> CPU provisioning. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> qapi/machine-target.json | 30 ++++++++++++++++++++++++++++++ >> hw/s390x/cpu-topology.c | 2 ++ >> 2 files changed, 32 insertions(+) >> >> diff --git a/qapi/machine-target.json b/qapi/machine-target.json >> index 58df0f5061..5883c3b020 100644 >> --- a/qapi/machine-target.json >> +++ b/qapi/machine-target.json >> @@ -371,3 +371,33 @@ >> }, >> 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] } >> } >> + >> +## >> +# @CPU_POLARITY_CHANGE: >> +# >> +# Emitted when the guest asks to change the polarity. >> +# >> +# @polarity: polarity specified by the guest >> +# >> +# The guest can tell the host (via the PTF instruction) whether the >> +# CPUs should be provisioned using horizontal or vertical polarity. >> +# >> +# On horizontal polarity the host is expected to provision all vCPUs >> +# equally. >> +# On vertical polarity the host can provision each vCPU differently. >> +# The guest will get information on the details of the provisioning >> +# the next time it uses the STSI(15) instruction. >> +# >> +# Since: 8.0 >> +# >> +# Example: >> +# >> +# <- { "event": "CPU_POLARITY_CHANGE", >> +# "data": { "polarity": 0 }, >> +# "timestamp": { "seconds": 1401385907, "microseconds": 422329 } } >> +# >> +## >> +{ 'event': 'CPU_POLARITY_CHANGE', >> + 'data': { 'polarity': 'int' }, >> + 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM'] } > > I wonder if you should depend on CONFIG_KVM or not. If tcg gets topology > support it will use the same event and right now it would just never be emitted. > On the other hand it's more conservative this way. TCG vs. KVM should be as transparent as we can make it. If only KVM can get into the state where the event is emitted, say because the state is only possible with features only KVM supports, then making the event conditional on KVM makes sense. Of course, when another accelerator acquires these features, we need to emit the event there as well, which will involve adjusting the condition. > I also wonder if you should add 'feature' : [ 'unstable' ]. > On the upside, it would mark the event as unstable, but I don't know what the > consequences are exactly. docs/devel/qapi-code-gen.rst: Special features ~~~~~~~~~~~~~~~~ Feature "deprecated" marks a command, event, enum value, or struct member as deprecated. It is not supported elsewhere so far. Interfaces so marked may be withdrawn in future releases in accordance with QEMU's deprecation policy. Feature "unstable" marks a command, event, enum value, or struct member as unstable. It is not supported elsewhere so far. Interfaces so marked may be withdrawn or changed incompatibly in future releases. See also -compat parameters unstable-input, unstable-output, both intended for "testing the future". > Also I guess one can remove qemu events without breaking backwards compatibility, > since they just won't be emitted? Unless I guess you specify that a event must > occur under certain situations and the client waits on it? Events are part of the interface just like command returns are. Not emitting an event in a situation where it was emitted before can easily break things. Only when the situation is no longer possible, the event can be removed safely. Questions? [...]
On Wed, Feb 08, 2023 at 06:35:39PM +0100, Nina Schoetterl-Glausch wrote: > On Wed, 2023-02-01 at 14:20 +0100, Pierre Morel wrote: > > When the guest asks to change the polarity this change > > is forwarded to the admin using QAPI. > > The admin is supposed to take according decisions concerning > > CPU provisioning. > > > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > > --- > > qapi/machine-target.json | 30 ++++++++++++++++++++++++++++++ > > hw/s390x/cpu-topology.c | 2 ++ > > 2 files changed, 32 insertions(+) > > > > diff --git a/qapi/machine-target.json b/qapi/machine-target.json > > index 58df0f5061..5883c3b020 100644 > > --- a/qapi/machine-target.json > > +++ b/qapi/machine-target.json > > @@ -371,3 +371,33 @@ > > }, > > 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] } > > } > > + > > +## > > +# @CPU_POLARITY_CHANGE: > > +# > > +# Emitted when the guest asks to change the polarity. > > +# > > +# @polarity: polarity specified by the guest > > +# > > +# The guest can tell the host (via the PTF instruction) whether the > > +# CPUs should be provisioned using horizontal or vertical polarity. > > +# > > +# On horizontal polarity the host is expected to provision all vCPUs > > +# equally. > > +# On vertical polarity the host can provision each vCPU differently. > > +# The guest will get information on the details of the provisioning > > +# the next time it uses the STSI(15) instruction. > > +# > > +# Since: 8.0 > > +# > > +# Example: > > +# > > +# <- { "event": "CPU_POLARITY_CHANGE", > > +# "data": { "polarity": 0 }, > > +# "timestamp": { "seconds": 1401385907, "microseconds": 422329 } } > > +# > > +## > > +{ 'event': 'CPU_POLARITY_CHANGE', > > + 'data': { 'polarity': 'int' }, > > + 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM'] } > > I wonder if you should depend on CONFIG_KVM or not. If tcg gets topology > support it will use the same event and right now it would just never be emitted. > On the other hand it's more conservative this way. > > I also wonder if you should add 'feature' : [ 'unstable' ]. > On the upside, it would mark the event as unstable, but I don't know what the > consequences are exactly. The intention of this flag is to allow mgmt apps to make a usage policy decision. Libvirt's policy is that we'll never use features marked unstable. IOW, the consequence of marking it unstable is that it'll likely go unused until the unstable marker gets removed. Using 'unstable' is useful if you want to get complex code merged before you're quite happy with the design, and then iterate on the impl in-tree. This is OK if there's no urgent need for apps to consume the feature. If you want the feature to be used for real though, the unstable flag is not desirable and you need to finalize the design. > Also I guess one can remove qemu events without breaking backwards compatibility, > since they just won't be emitted? Unless I guess you specify that a event must > occur under certain situations and the client waits on it? As Markus says, that's not a safe assumption. If a mgmt app is expecting to receive an event, ceasing to emit it would likely be considered a regression. With regards, Daniel
Daniel P. Berrangé <berrange@redhat.com> writes: > On Wed, Feb 08, 2023 at 06:35:39PM +0100, Nina Schoetterl-Glausch wrote: [...] >> I also wonder if you should add 'feature' : [ 'unstable' ]. >> On the upside, it would mark the event as unstable, but I don't know what the >> consequences are exactly. > > The intention of this flag is to allow mgmt apps to make a usage policy > decision. > > Libvirt's policy is that we'll never use features marked unstable. > > IOW, the consequence of marking it unstable is that it'll likely > go unused until the unstable marker gets removed. > > Using 'unstable' is useful if you want to get complex code merged > before you're quite happy with the design, and then iterate on the > impl in-tree. This is OK if there's no urgent need for apps to > consume the feature. If you want the feature to be used for real > though, the unstable flag is not desirable and you need to finalize > the design. Another use of 'unstable' is debugging aids. Making these stable can be plenty of pain for precious little gain. [...]
On Thu, 2023-02-09 at 10:04 +0000, Daniel P. Berrangé wrote: > On Wed, Feb 08, 2023 at 06:35:39PM +0100, Nina Schoetterl-Glausch wrote: > > On Wed, 2023-02-01 at 14:20 +0100, Pierre Morel wrote: > > > When the guest asks to change the polarity this change > > > is forwarded to the admin using QAPI. > > > The admin is supposed to take according decisions concerning > > > CPU provisioning. > > > > > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > > > --- > > > qapi/machine-target.json | 30 ++++++++++++++++++++++++++++++ > > > hw/s390x/cpu-topology.c | 2 ++ > > > 2 files changed, 32 insertions(+) > > > > > > diff --git a/qapi/machine-target.json b/qapi/machine-target.json > > > index 58df0f5061..5883c3b020 100644 > > > --- a/qapi/machine-target.json > > > +++ b/qapi/machine-target.json > > > @@ -371,3 +371,33 @@ > > > }, > > > 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] } > > > } > > > + > > > +## > > > +# @CPU_POLARITY_CHANGE: > > > +# > > > +# Emitted when the guest asks to change the polarity. > > > +# > > > +# @polarity: polarity specified by the guest > > > +# > > > +# The guest can tell the host (via the PTF instruction) whether the > > > +# CPUs should be provisioned using horizontal or vertical polarity. > > > +# > > > +# On horizontal polarity the host is expected to provision all vCPUs > > > +# equally. > > > +# On vertical polarity the host can provision each vCPU differently. > > > +# The guest will get information on the details of the provisioning > > > +# the next time it uses the STSI(15) instruction. > > > +# > > > +# Since: 8.0 > > > +# > > > +# Example: > > > +# > > > +# <- { "event": "CPU_POLARITY_CHANGE", > > > +# "data": { "polarity": 0 }, > > > +# "timestamp": { "seconds": 1401385907, "microseconds": 422329 } } > > > +# > > > +## > > > +{ 'event': 'CPU_POLARITY_CHANGE', > > > + 'data': { 'polarity': 'int' }, > > > + 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM'] } > > > > I wonder if you should depend on CONFIG_KVM or not. If tcg gets topology > > support it will use the same event and right now it would just never be emitted. > > On the other hand it's more conservative this way. > > > > I also wonder if you should add 'feature' : [ 'unstable' ]. > > On the upside, it would mark the event as unstable, but I don't know what the > > consequences are exactly. > > The intention of this flag is to allow mgmt apps to make a usage policy > decision. > > Libvirt's policy is that we'll never use features marked unstable. Does it enforce that, e.g via compat policies? If so, I assume there is some way to allow use of unstable features in libvirt for development? If for example you're prototyping a new mgmt feature that uses unstable commands. > > IOW, the consequence of marking it unstable is that it'll likely > go unused until the unstable marker gets removed. > > Using 'unstable' is useful if you want to get complex code merged > before you're quite happy with the design, and then iterate on the > impl in-tree. This is OK if there's no urgent need for apps to > consume the feature. If you want the feature to be used for real > though, the unstable flag is not desirable and you need to finalize > the design. > > > Also I guess one can remove qemu events without breaking backwards compatibility, > > since they just won't be emitted? Unless I guess you specify that a event must > > occur under certain situations and the client waits on it? > > As Markus says, that's not a safe assumption. If a mgmt app is expecting > to receive an event, ceasing to emit it would likely be considered a > regression. > > > With regards, > Daniel
On Thu, Feb 09, 2023 at 01:12:17PM +0100, Nina Schoetterl-Glausch wrote: > On Thu, 2023-02-09 at 10:04 +0000, Daniel P. Berrangé wrote: > > On Wed, Feb 08, 2023 at 06:35:39PM +0100, Nina Schoetterl-Glausch wrote: > > > On Wed, 2023-02-01 at 14:20 +0100, Pierre Morel wrote: > > > > When the guest asks to change the polarity this change > > > > is forwarded to the admin using QAPI. > > > > The admin is supposed to take according decisions concerning > > > > CPU provisioning. > > > > > > > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > > > > --- > > > > qapi/machine-target.json | 30 ++++++++++++++++++++++++++++++ > > > > hw/s390x/cpu-topology.c | 2 ++ > > > > 2 files changed, 32 insertions(+) > > > > > > > > diff --git a/qapi/machine-target.json b/qapi/machine-target.json > > > > index 58df0f5061..5883c3b020 100644 > > > > --- a/qapi/machine-target.json > > > > +++ b/qapi/machine-target.json > > > > @@ -371,3 +371,33 @@ > > > > }, > > > > 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] } > > > > } > > > > + > > > > +## > > > > +# @CPU_POLARITY_CHANGE: > > > > +# > > > > +# Emitted when the guest asks to change the polarity. > > > > +# > > > > +# @polarity: polarity specified by the guest > > > > +# > > > > +# The guest can tell the host (via the PTF instruction) whether the > > > > +# CPUs should be provisioned using horizontal or vertical polarity. > > > > +# > > > > +# On horizontal polarity the host is expected to provision all vCPUs > > > > +# equally. > > > > +# On vertical polarity the host can provision each vCPU differently. > > > > +# The guest will get information on the details of the provisioning > > > > +# the next time it uses the STSI(15) instruction. > > > > +# > > > > +# Since: 8.0 > > > > +# > > > > +# Example: > > > > +# > > > > +# <- { "event": "CPU_POLARITY_CHANGE", > > > > +# "data": { "polarity": 0 }, > > > > +# "timestamp": { "seconds": 1401385907, "microseconds": 422329 } } > > > > +# > > > > +## > > > > +{ 'event': 'CPU_POLARITY_CHANGE', > > > > + 'data': { 'polarity': 'int' }, > > > > + 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM'] } > > > > > > I wonder if you should depend on CONFIG_KVM or not. If tcg gets topology > > > support it will use the same event and right now it would just never be emitted. > > > On the other hand it's more conservative this way. > > > > > > I also wonder if you should add 'feature' : [ 'unstable' ]. > > > On the upside, it would mark the event as unstable, but I don't know what the > > > consequences are exactly. > > > > The intention of this flag is to allow mgmt apps to make a usage policy > > decision. > > > > Libvirt's policy is that we'll never use features marked unstable. > > Does it enforce that, e.g via compat policies? The policy is applied at time of code review, in that we'll not merge patches that use features marked unstable. > If so, I assume there is some way to allow use of unstable features in libvirt for development? > If for example you're prototyping a new mgmt feature that uses unstable commands. You can prototype usage in libvirt in a fork of course, but we won't take patches into the libvirt upstream repo. Alternatively in some cases the the libvirt QMP passthrough can be used for experiemnts (eg virsh qemu-monitor-command ) in a non-production envionrment. With regards, Daniel
On Wed, 2023-02-08 at 20:23 +0100, Markus Armbruster wrote: > Nina Schoetterl-Glausch <nsg@linux.ibm.com> writes: > > > On Wed, 2023-02-01 at 14:20 +0100, Pierre Morel wrote: > > > When the guest asks to change the polarity this change > > > is forwarded to the admin using QAPI. > > > The admin is supposed to take according decisions concerning > > > CPU provisioning. > > > > > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > > > --- > > > qapi/machine-target.json | 30 ++++++++++++++++++++++++++++++ > > > hw/s390x/cpu-topology.c | 2 ++ > > > 2 files changed, 32 insertions(+) > > > > > > diff --git a/qapi/machine-target.json b/qapi/machine-target.json > > > index 58df0f5061..5883c3b020 100644 > > > --- a/qapi/machine-target.json > > > +++ b/qapi/machine-target.json > > > @@ -371,3 +371,33 @@ > > > }, > > > 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] } > > > } > > > + > > > +## > > > +# @CPU_POLARITY_CHANGE: > > > +# > > > +# Emitted when the guest asks to change the polarity. > > > +# > > > +# @polarity: polarity specified by the guest > > > +# > > > +# The guest can tell the host (via the PTF instruction) whether the > > > +# CPUs should be provisioned using horizontal or vertical polarity. > > > +# > > > +# On horizontal polarity the host is expected to provision all vCPUs > > > +# equally. > > > +# On vertical polarity the host can provision each vCPU differently. > > > +# The guest will get information on the details of the provisioning > > > +# the next time it uses the STSI(15) instruction. > > > +# > > > +# Since: 8.0 > > > +# > > > +# Example: > > > +# > > > +# <- { "event": "CPU_POLARITY_CHANGE", > > > +# "data": { "polarity": 0 }, > > > +# "timestamp": { "seconds": 1401385907, "microseconds": 422329 } } > > > +# > > > +## > > > +{ 'event': 'CPU_POLARITY_CHANGE', > > > + 'data': { 'polarity': 'int' }, > > > + 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM'] } > > > > I wonder if you should depend on CONFIG_KVM or not. If tcg gets topology > > support it will use the same event and right now it would just never be emitted. > > On the other hand it's more conservative this way. > > TCG vs. KVM should be as transparent as we can make it. > > If only KVM can get into the state where the event is emitted, say > because the state is only possible with features only KVM supports, then > making the event conditional on KVM makes sense. Of course, when > another accelerator acquires these features, we need to emit the event > there as well, which will involve adjusting the condition. That's the case here, KVM supports the feature, TCG doesn't, although there is no reason it couldn't in the future. > > > I also wonder if you should add 'feature' : [ 'unstable' ]. > > On the upside, it would mark the event as unstable, but I don't know what the > > consequences are exactly. > > docs/devel/qapi-code-gen.rst: > > Special features > ~~~~~~~~~~~~~~~~ > > Feature "deprecated" marks a command, event, enum value, or struct > member as deprecated. It is not supported elsewhere so far. > Interfaces so marked may be withdrawn in future releases in accordance > with QEMU's deprecation policy. > > Feature "unstable" marks a command, event, enum value, or struct > member as unstable. It is not supported elsewhere so far. Interfaces > so marked may be withdrawn or changed incompatibly in future releases. Yeah, I saw that, but wasn't aware of -compat, thanks. > > See also -compat parameters unstable-input, unstable-output, both > intended for "testing the future". > > > Also I guess one can remove qemu events without breaking backwards compatibility, > > since they just won't be emitted? Unless I guess you specify that a event must > > occur under certain situations and the client waits on it? > > Events are part of the interface just like command returns are. Not > emitting an event in a situation where it was emitted before can easily > break things. Only when the situation is no longer possible, the event > can be removed safely. @Pierre, seems it would be a good idea to mark all changes to qmp unstable, not just set-cpu-topology, can just remove it later after all. > > Questions? > > [...] >
On 2/9/23 13:28, Nina Schoetterl-Glausch wrote: > On Wed, 2023-02-08 at 20:23 +0100, Markus Armbruster wrote: >> Nina Schoetterl-Glausch <nsg@linux.ibm.com> writes: >> ... > >> >>> I also wonder if you should add 'feature' : [ 'unstable' ]. >>> On the upside, it would mark the event as unstable, but I don't know what the >>> consequences are exactly. >> >> docs/devel/qapi-code-gen.rst: >> >> Special features >> ~~~~~~~~~~~~~~~~ >> >> Feature "deprecated" marks a command, event, enum value, or struct >> member as deprecated. It is not supported elsewhere so far. >> Interfaces so marked may be withdrawn in future releases in accordance >> with QEMU's deprecation policy. >> >> Feature "unstable" marks a command, event, enum value, or struct >> member as unstable. It is not supported elsewhere so far. Interfaces >> so marked may be withdrawn or changed incompatibly in future releases. > > Yeah, I saw that, but wasn't aware of -compat, thanks. > >> >> See also -compat parameters unstable-input, unstable-output, both >> intended for "testing the future". >> >>> Also I guess one can remove qemu events without breaking backwards compatibility, >>> since they just won't be emitted? Unless I guess you specify that a event must >>> occur under certain situations and the client waits on it? >> >> Events are part of the interface just like command returns are. Not >> emitting an event in a situation where it was emitted before can easily >> break things. Only when the situation is no longer possible, the event >> can be removed safely. > > @Pierre, seems it would be a good idea to mark all changes to qmp unstable, not just > set-cpu-topology, can just remove it later after all. OK. Just curious: how do you think this simple event matching the guest request 1 on 1 may evolve? Regards, Pierre
On Thu, 2023-02-09 at 14:00 +0100, Pierre Morel wrote: > > On 2/9/23 13:28, Nina Schoetterl-Glausch wrote: > > On Wed, 2023-02-08 at 20:23 +0100, Markus Armbruster wrote: > > > Nina Schoetterl-Glausch <nsg@linux.ibm.com> writes: > > > > > ... > > > > > > > > > > I also wonder if you should add 'feature' : [ 'unstable' ]. > > > > On the upside, it would mark the event as unstable, but I don't know what the > > > > consequences are exactly. > > > > > > docs/devel/qapi-code-gen.rst: > > > > > > Special features > > > ~~~~~~~~~~~~~~~~ > > > > > > Feature "deprecated" marks a command, event, enum value, or struct > > > member as deprecated. It is not supported elsewhere so far. > > > Interfaces so marked may be withdrawn in future releases in accordance > > > with QEMU's deprecation policy. > > > > > > Feature "unstable" marks a command, event, enum value, or struct > > > member as unstable. It is not supported elsewhere so far. Interfaces > > > so marked may be withdrawn or changed incompatibly in future releases. > > > > Yeah, I saw that, but wasn't aware of -compat, thanks. > > > > > > > > See also -compat parameters unstable-input, unstable-output, both > > > intended for "testing the future". > > > > > > > Also I guess one can remove qemu events without breaking backwards compatibility, > > > > since they just won't be emitted? Unless I guess you specify that a event must > > > > occur under certain situations and the client waits on it? > > > > > > Events are part of the interface just like command returns are. Not > > > emitting an event in a situation where it was emitted before can easily > > > break things. Only when the situation is no longer possible, the event > > > can be removed safely. > > > > @Pierre, seems it would be a good idea to mark all changes to qmp unstable, not just > > set-cpu-topology, can just remove it later after all. > > OK. > > Just curious: how do you think this simple event matching the guest > request 1 on 1 may evolve? Well, I don't know really, making it unstable is just more conservative for now. But this way you can prototype/implement support in libvirt for topology and then make the interface stable one you've confirmed that everything works as intended. Here is something to think about: The architecture allows rejection of the PTF polarization change request, if a request is currently in process. A possible design to allow the same semantics for qemu/libvirt would be to set a polarization_change_in_progess bit when a PTF request occurs and refuse subsequent requests until this bit was reset via qmp. This wouldn't result in the event data structure being changed, but its semantics, since it isn't fired for every request anymore. > > Regards, > Pierre >
diff --git a/qapi/machine-target.json b/qapi/machine-target.json index 58df0f5061..5883c3b020 100644 --- a/qapi/machine-target.json +++ b/qapi/machine-target.json @@ -371,3 +371,33 @@ }, 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] } } + +## +# @CPU_POLARITY_CHANGE: +# +# Emitted when the guest asks to change the polarity. +# +# @polarity: polarity specified by the guest +# +# The guest can tell the host (via the PTF instruction) whether the +# CPUs should be provisioned using horizontal or vertical polarity. +# +# On horizontal polarity the host is expected to provision all vCPUs +# equally. +# On vertical polarity the host can provision each vCPU differently. +# The guest will get information on the details of the provisioning +# the next time it uses the STSI(15) instruction. +# +# Since: 8.0 +# +# Example: +# +# <- { "event": "CPU_POLARITY_CHANGE", +# "data": { "polarity": 0 }, +# "timestamp": { "seconds": 1401385907, "microseconds": 422329 } } +# +## +{ 'event': 'CPU_POLARITY_CHANGE', + 'data': { 'polarity': 'int' }, + 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM'] } +} diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c index 6c50050991..2f8e1b60cf 100644 --- a/hw/s390x/cpu-topology.c +++ b/hw/s390x/cpu-topology.c @@ -19,6 +19,7 @@ #include "hw/s390x/s390-virtio-ccw.h" #include "hw/s390x/cpu-topology.h" #include "qapi/qapi-commands-machine-target.h" +#include "qapi/qapi-events-machine-target.h" #include "qapi/qmp/qdict.h" #include "monitor/hmp.h" #include "monitor/monitor.h" @@ -163,6 +164,7 @@ void s390_handle_ptf(S390CPU *cpu, uint8_t r1, uintptr_t ra) s390_topology.polarity = fc; s390_cpu_topology_set_modified(); s390_topology_set_cpus_polarity(fc); + qapi_event_send_cpu_polarity_change(fc); setcc(cpu, 0); } break;
When the guest asks to change the polarity this change is forwarded to the admin using QAPI. The admin is supposed to take according decisions concerning CPU provisioning. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- qapi/machine-target.json | 30 ++++++++++++++++++++++++++++++ hw/s390x/cpu-topology.c | 2 ++ 2 files changed, 32 insertions(+)