| Message ID | 20220902075531.188916-7-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: CPU Topology | expand |
Quoting Pierre Morel (2022-09-02 09:55:27) > During a subsystem reset the Topology-Change-Report is cleared > by the machine. > Let's ask KVM to clear the Modified Topology Change Report (MTCR) > bit of the SCA in the case of a subsystem reset. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
On Fri, 2022-09-02 at 09:55 +0200, Pierre Morel wrote: > During a subsystem reset the Topology-Change-Report is cleared > by the machine. > Let's ask KVM to clear the Modified Topology Change Report (MTCR) > bit of the SCA in the case of a subsystem reset. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > hw/s390x/cpu-topology.c | 12 ++++++++++++ > hw/s390x/s390-virtio-ccw.c | 1 + > target/s390x/cpu-sysemu.c | 7 +++++++ > target/s390x/cpu.h | 1 + > target/s390x/kvm/kvm.c | 23 +++++++++++++++++++++++ > target/s390x/kvm/kvm_s390x.h | 1 + > 6 files changed, 45 insertions(+) [...] > diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c > index f96630440b..9c994d27d5 100644 > --- a/target/s390x/kvm/kvm.c > +++ b/target/s390x/kvm/kvm.c > @@ -2585,3 +2585,26 @@ int kvm_s390_get_zpci_op(void) > { > return cap_zpci_op; > } > + > +int kvm_s390_topology_set_mtcr(uint64_t attr) > +{ > + struct kvm_device_attr attribute = { > + .group = KVM_S390_VM_CPU_TOPOLOGY, > + .attr = attr, > + }; > + int ret; > + > + if (!s390_has_feat(S390_FEAT_CONFIGURATION_TOPOLOGY)) { > + return -EFAULT; Why EFAULT? The return value is just ignored when resetting, isn't it? I wonder if it would be better not to. Is it necessary because you're detecting the feature after you've already created the S390Topology instance? And you're doing that because that's just the order in which QEMU does things? So the machine class is inited before the cpu model? I wonder if there is a nice way to create the S390Topology only if the feature is selected. Anyway: Reviewed-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> > + } > + if (!kvm_vm_check_attr(kvm_state, KVM_S390_VM_CPU_TOPOLOGY, attr)) { > + return -ENOENT; > + } > + > + ret = kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attribute); > + if (ret) { > + error_report("Failed to set cpu topology attribute %lu: %s", > + attr, strerror(-ret)); > + } > + return ret; > +} > [...]
On 9/6/22 10:27, Nico Boehr wrote: > Quoting Pierre Morel (2022-09-02 09:55:27) >> During a subsystem reset the Topology-Change-Report is cleared >> by the machine. >> Let's ask KVM to clear the Modified Topology Change Report (MTCR) >> bit of the SCA in the case of a subsystem reset. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > > Reviewed-by: Nico Boehr <nrb@linux.ibm.com> Thanks, Pierre
On 9/8/22 09:57, Janis Schoetterl-Glausch wrote: > On Fri, 2022-09-02 at 09:55 +0200, Pierre Morel wrote: >> During a subsystem reset the Topology-Change-Report is cleared >> by the machine. >> Let's ask KVM to clear the Modified Topology Change Report (MTCR) >> bit of the SCA in the case of a subsystem reset. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> hw/s390x/cpu-topology.c | 12 ++++++++++++ >> hw/s390x/s390-virtio-ccw.c | 1 + >> target/s390x/cpu-sysemu.c | 7 +++++++ >> target/s390x/cpu.h | 1 + >> target/s390x/kvm/kvm.c | 23 +++++++++++++++++++++++ >> target/s390x/kvm/kvm_s390x.h | 1 + >> 6 files changed, 45 insertions(+) > > [...] > >> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c >> index f96630440b..9c994d27d5 100644 >> --- a/target/s390x/kvm/kvm.c >> +++ b/target/s390x/kvm/kvm.c >> @@ -2585,3 +2585,26 @@ int kvm_s390_get_zpci_op(void) >> { >> return cap_zpci_op; >> } >> + >> +int kvm_s390_topology_set_mtcr(uint64_t attr) >> +{ >> + struct kvm_device_attr attribute = { >> + .group = KVM_S390_VM_CPU_TOPOLOGY, >> + .attr = attr, >> + }; >> + int ret; >> + >> + if (!s390_has_feat(S390_FEAT_CONFIGURATION_TOPOLOGY)) { >> + return -EFAULT; > > Why EFAULT? any proposition? > The return value is just ignored when resetting, isn't it? In migration the same function is used and we need to return an error there. But if we would not use it, after the comments you did for migration, we may indeed not need it, then I guess using error_report and a void function may be better. > I wonder if it would be better not to. > Is it necessary because you're detecting the feature after you've > already created the S390Topology instance? > And you're doing that because that's just the order in which QEMU does > things? So the machine class is inited before the cpu model? > I wonder if there is a nice way to create the S390Topology only if the > feature is selected. I do not know if it is possible. I will look. > > Anyway: > Reviewed-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> Thanks, Pierre > >> + } >> + if (!kvm_vm_check_attr(kvm_state, KVM_S390_VM_CPU_TOPOLOGY, attr)) { >> + return -ENOENT; >> + } >> + >> + ret = kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attribute); >> + if (ret) { >> + error_report("Failed to set cpu topology attribute %lu: %s", >> + attr, strerror(-ret)); >> + } >> + return ret; >> +} >> > [...] >
diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c index bb9ae63483..6098d6ea1f 100644 --- a/hw/s390x/cpu-topology.c +++ b/hw/s390x/cpu-topology.c @@ -121,6 +121,17 @@ static void s390_topology_realize(DeviceState *dev, Error **errp) qemu_mutex_init(&topo->topo_mutex); } +/** + * s390_topology_reset: + * @dev: the device + * + * Calls the sysemu topology reset + */ +static void s390_topology_reset(DeviceState *dev) +{ + s390_cpu_topology_reset(); +} + /** * topology_class_init: * @oc: Object class @@ -134,6 +145,7 @@ static void topology_class_init(ObjectClass *oc, void *data) dc->realize = s390_topology_realize; set_bit(DEVICE_CATEGORY_MISC, dc->categories); + dc->reset = s390_topology_reset; } static const TypeInfo cpu_topology_info = { diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 3f28e28d47..1fa98740de 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -104,6 +104,7 @@ static const char *const reset_dev_types[] = { "s390-flic", "diag288", TYPE_S390_PCI_HOST_BRIDGE, + TYPE_S390_CPU_TOPOLOGY, }; static void subsystem_reset(void) diff --git a/target/s390x/cpu-sysemu.c b/target/s390x/cpu-sysemu.c index 948e4bd3e0..707c0b658c 100644 --- a/target/s390x/cpu-sysemu.c +++ b/target/s390x/cpu-sysemu.c @@ -306,3 +306,10 @@ void s390_do_cpu_set_diag318(CPUState *cs, run_on_cpu_data arg) kvm_s390_set_diag318(cs, arg.host_ulong); } } + +void s390_cpu_topology_reset(void) +{ + if (kvm_enabled()) { + kvm_s390_topology_set_mtcr(0); + } +} diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index c61fe9b563..cff5cc0415 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -826,6 +826,7 @@ void s390_enable_css_support(S390CPU *cpu); void s390_do_cpu_set_diag318(CPUState *cs, run_on_cpu_data arg); int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id, int vq, bool assign); +void s390_cpu_topology_reset(void); #ifndef CONFIG_USER_ONLY unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu); #else diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c index f96630440b..9c994d27d5 100644 --- a/target/s390x/kvm/kvm.c +++ b/target/s390x/kvm/kvm.c @@ -2585,3 +2585,26 @@ int kvm_s390_get_zpci_op(void) { return cap_zpci_op; } + +int kvm_s390_topology_set_mtcr(uint64_t attr) +{ + struct kvm_device_attr attribute = { + .group = KVM_S390_VM_CPU_TOPOLOGY, + .attr = attr, + }; + int ret; + + if (!s390_has_feat(S390_FEAT_CONFIGURATION_TOPOLOGY)) { + return -EFAULT; + } + if (!kvm_vm_check_attr(kvm_state, KVM_S390_VM_CPU_TOPOLOGY, attr)) { + return -ENOENT; + } + + ret = kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attribute); + if (ret) { + error_report("Failed to set cpu topology attribute %lu: %s", + attr, strerror(-ret)); + } + return ret; +} diff --git a/target/s390x/kvm/kvm_s390x.h b/target/s390x/kvm/kvm_s390x.h index aaae8570de..a13c8fb9a3 100644 --- a/target/s390x/kvm/kvm_s390x.h +++ b/target/s390x/kvm/kvm_s390x.h @@ -46,5 +46,6 @@ void kvm_s390_crypto_reset(void); void kvm_s390_restart_interrupt(S390CPU *cpu); void kvm_s390_stop_interrupt(S390CPU *cpu); void kvm_s390_set_diag318(CPUState *cs, uint64_t diag318_info); +int kvm_s390_topology_set_mtcr(uint64_t attr); #endif /* KVM_S390X_H */
During a subsystem reset the Topology-Change-Report is cleared by the machine. Let's ask KVM to clear the Modified Topology Change Report (MTCR) bit of the SCA in the case of a subsystem reset. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- hw/s390x/cpu-topology.c | 12 ++++++++++++ hw/s390x/s390-virtio-ccw.c | 1 + target/s390x/cpu-sysemu.c | 7 +++++++ target/s390x/cpu.h | 1 + target/s390x/kvm/kvm.c | 23 +++++++++++++++++++++++ target/s390x/kvm/kvm_s390x.h | 1 + 6 files changed, 45 insertions(+)