Message ID | 20170918160012.4317-1-david@redhat.com |
---|---|
Headers | show |
Series | s390x: SMP for TCG (+ cleanups) | expand |
David, can you outline how much you tested KVM after these changes? I assume we should review/test the whole series properly? On 09/18/2017 05:59 PM, David Hildenbrand wrote: > This series contains: > - properly implement local external interrupts for TCG > - factor out KVM SIGP handling code into common code > - implement missing SIGP orders for TCG handled by the kernel for KVM > (including STOP and RESTART interrupts) > - make TCG use the new SIGP code - experimental SMP support for s390x TCG > - refactor STFL(E) implementation for TCG > - bunch of cleanups > > Basically all SIGP instructions are fully supported. > > Thanks to Aurelien Jarno for the initital prototype and showcasing that > supporting experimental SMP code can be implemented quite easily. > > TCG SMP on s390x - what works? > - "-smp X,maxcpus=X" with both, single and multi threaded TCG > - "-smp ... -device qemu-s390-cpu,id=cpuX,core-id=X" > - system_powerdown, system_reset, shutdown, reboot, NMI > - online/offline of CPUs from inside the guest > > TCG SMP on s390x - what does not work? > - Floating interrupts all target CPU 0. Don't offline it. > - CPU hotplug after the machine/main loop has been fully setup > -- the new CPU comes up, answers and sends emergency signals, but suddenly > the VM gets stuck. This is strange, as ordinary online/offline works > just fine. > -- can be triggered by "cpu-add 1" + "system_reset". The system will hang > when trying to online CPUs. (note: in Linux code they are fully up and > running and already executed code) > -- also if hotplugging with "-S", before anything has run. This is strange, > as "-device qemu-s390-cpu" works just fine. > -- does not seem to be related to CPU setup/reset code, I checked that > -- seems to be related to some TCG internals (as broken for single and > multi threaded TCG). > -- common code seems to be somehow broken, not sure if this is even > expected to work (e.g. for single threaded TCG, hotplugged CPUs will > never get set "cpu->created = true". But doesn't seem to be related to > this) > > > Based on: https://github.com/cohuck/qemu.git s390-next > Available on: git@github.com:davidhildenbrand/qemu.git s390x-queue > > > David Hildenbrand (27): > s390x: raise CPU hotplug irq after really hotplugged > s390x/cpumodel: fix max STFL(E) bit number > target/s390x: get rid of next_core_id > s390x: introduce and use S390_MAX_CPUS > s390/tcg: turn INTERRUPT_EXT into a mask > s390x/tcg: injection of emergency signals and extarnal calls > s390x/tcg: STOPPED cpus can never wake up > s390x/tcg: a CPU cannot switch state due to an interrupt > target/s390x: factor out handling of WAIT PSW into handle_wait() > s390x/kvm: pass ipb directly into handle_sigp() > s390x/kvm: generalize SIGP stop and restart interrupt injection > s390x/kvm: factor out storing of CPU status > target/s390x: proper cpu->be convertion in s390_store_status() > s390x/kvm: factor out storing of adtl CPU status > s390x/kvm: drop two debug prints > s390x/kvm: factor out SIGP code into sigp.c > s390x/kvm: factor out actual handling of STOP interrupts > s390x/tcg: implement SIGP SENSE RUNNING STATUS > s390x/tcg: implement SIGP SENSE > s390x/tcg: implement SIGP EXTERNAL CALL > s390x/tcg: implement SIGP EMERGENCY SIGNAL > s390x/tcg: implement SIGP CONDITIONAL EMERGENCY SIGNAL > s390x/tcg: implement STOP and RESET interrupts for TCG > s390x/tcg: flush the tlb on SIGP SET PREFIX > s390x/tcg: switch to new SIGP handling code > s390x/tcg: unlock NMI > s390x/tcg: refactor stfl(e) to use s390_get_feat_block() > > hw/s390x/s390-virtio-ccw.c | 17 +- > target/s390x/Makefile.objs | 1 + > target/s390x/cpu-qom.h | 2 - > target/s390x/cpu.c | 40 ++-- > target/s390x/cpu.h | 36 +++- > target/s390x/cpu_features.c | 2 +- > target/s390x/cpu_models.c | 2 + > target/s390x/excp_helper.c | 98 ++++++--- > target/s390x/helper.c | 115 ++++++++-- > target/s390x/helper.h | 4 +- > target/s390x/internal.h | 15 ++ > target/s390x/interrupt.c | 70 +++++- > target/s390x/kvm-stub.c | 13 +- > target/s390x/kvm.c | 470 +++-------------------------------------- > target/s390x/kvm_s390x.h | 3 +- > target/s390x/misc_helper.c | 114 ++++------ > target/s390x/sigp.c | 504 ++++++++++++++++++++++++++++++++++++++++++++ > target/s390x/trace-events | 4 +- > target/s390x/translate.c | 6 +- > 19 files changed, 896 insertions(+), 620 deletions(-) > create mode 100644 target/s390x/sigp.c >
On 18.09.2017 19:31, Christian Borntraeger wrote: > David, > > can you outline how much you tested KVM after these changes? I assume we should > review/test the whole series properly? Related to my tests: I did the usual shutdown/reboot/system_reset/online-offline test with KVM. I remember also playing with CPU hotplug again. However, no kvm-unit-tests so far (also not in my pipeline). Related to the patches: I tried to modify as little KVM code as possible. Most stuff really is just moving code around. But still, subtle errors can happen. E.g. for all new SIGP instructions, I keep existing behavior for KVM by checking for tcg/kvm directly at the start at the handlers. So most s390x/tcg patches should have no influence. So reviewing all s390x/kvm patches and !tcg patches would be great. I will certainly not complain if some IBM folks would also review the TCG patches ;) Special care should need: - "target/s390x: proper cpu->be convertion in s390_store_status()" -- to make sure I haven't done any stupid mistakes while converting (just realized s/convertion/conversion/ :) ) - "s390x/kvm: factor out SIGP code into sigp.c" -- to make sure the new kvm_s390_handle_sigp() and handle_sigp() do the same thing as handle_sigp() did before. "s390x/tcg: STOPPED cpus can never wake up" and "s390x/tcg: implement STOP and RESET interrupts for TCG" modify a function called also for kvm (s390_cpu_has_work()). It is expected to return always "false" for kvm. This behavior should still be that way. Thanks! > > On 09/18/2017 05:59 PM, David Hildenbrand wrote: >> This series contains: >> - properly implement local external interrupts for TCG >> - factor out KVM SIGP handling code into common code >> - implement missing SIGP orders for TCG handled by the kernel for KVM >> (including STOP and RESTART interrupts) >> - make TCG use the new SIGP code - experimental SMP support for s390x TCG >> - refactor STFL(E) implementation for TCG >> - bunch of cleanups >> >> Basically all SIGP instructions are fully supported. >> >> Thanks to Aurelien Jarno for the initital prototype and showcasing that >> supporting experimental SMP code can be implemented quite easily. >> >> TCG SMP on s390x - what works? >> - "-smp X,maxcpus=X" with both, single and multi threaded TCG >> - "-smp ... -device qemu-s390-cpu,id=cpuX,core-id=X" >> - system_powerdown, system_reset, shutdown, reboot, NMI >> - online/offline of CPUs from inside the guest >> >> TCG SMP on s390x - what does not work? >> - Floating interrupts all target CPU 0. Don't offline it. >> - CPU hotplug after the machine/main loop has been fully setup >> -- the new CPU comes up, answers and sends emergency signals, but suddenly >> the VM gets stuck. This is strange, as ordinary online/offline works >> just fine. >> -- can be triggered by "cpu-add 1" + "system_reset". The system will hang >> when trying to online CPUs. (note: in Linux code they are fully up and >> running and already executed code) >> -- also if hotplugging with "-S", before anything has run. This is strange, >> as "-device qemu-s390-cpu" works just fine. >> -- does not seem to be related to CPU setup/reset code, I checked that >> -- seems to be related to some TCG internals (as broken for single and >> multi threaded TCG). >> -- common code seems to be somehow broken, not sure if this is even >> expected to work (e.g. for single threaded TCG, hotplugged CPUs will >> never get set "cpu->created = true". But doesn't seem to be related to >> this) >> >> >> Based on: https://github.com/cohuck/qemu.git s390-next >> Available on: git@github.com:davidhildenbrand/qemu.git s390x-queue >> >> >> David Hildenbrand (27): >> s390x: raise CPU hotplug irq after really hotplugged >> s390x/cpumodel: fix max STFL(E) bit number >> target/s390x: get rid of next_core_id >> s390x: introduce and use S390_MAX_CPUS >> s390/tcg: turn INTERRUPT_EXT into a mask >> s390x/tcg: injection of emergency signals and extarnal calls >> s390x/tcg: STOPPED cpus can never wake up >> s390x/tcg: a CPU cannot switch state due to an interrupt >> target/s390x: factor out handling of WAIT PSW into handle_wait() >> s390x/kvm: pass ipb directly into handle_sigp() >> s390x/kvm: generalize SIGP stop and restart interrupt injection >> s390x/kvm: factor out storing of CPU status >> target/s390x: proper cpu->be convertion in s390_store_status() >> s390x/kvm: factor out storing of adtl CPU status >> s390x/kvm: drop two debug prints >> s390x/kvm: factor out SIGP code into sigp.c >> s390x/kvm: factor out actual handling of STOP interrupts >> s390x/tcg: implement SIGP SENSE RUNNING STATUS >> s390x/tcg: implement SIGP SENSE >> s390x/tcg: implement SIGP EXTERNAL CALL >> s390x/tcg: implement SIGP EMERGENCY SIGNAL >> s390x/tcg: implement SIGP CONDITIONAL EMERGENCY SIGNAL >> s390x/tcg: implement STOP and RESET interrupts for TCG >> s390x/tcg: flush the tlb on SIGP SET PREFIX >> s390x/tcg: switch to new SIGP handling code >> s390x/tcg: unlock NMI >> s390x/tcg: refactor stfl(e) to use s390_get_feat_block() >> >> hw/s390x/s390-virtio-ccw.c | 17 +- >> target/s390x/Makefile.objs | 1 + >> target/s390x/cpu-qom.h | 2 - >> target/s390x/cpu.c | 40 ++-- >> target/s390x/cpu.h | 36 +++- >> target/s390x/cpu_features.c | 2 +- >> target/s390x/cpu_models.c | 2 + >> target/s390x/excp_helper.c | 98 ++++++--- >> target/s390x/helper.c | 115 ++++++++-- >> target/s390x/helper.h | 4 +- >> target/s390x/internal.h | 15 ++ >> target/s390x/interrupt.c | 70 +++++- >> target/s390x/kvm-stub.c | 13 +- >> target/s390x/kvm.c | 470 +++-------------------------------------- >> target/s390x/kvm_s390x.h | 3 +- >> target/s390x/misc_helper.c | 114 ++++------ >> target/s390x/sigp.c | 504 ++++++++++++++++++++++++++++++++++++++++++++ >> target/s390x/trace-events | 4 +- >> target/s390x/translate.c | 6 +- >> 19 files changed, 896 insertions(+), 620 deletions(-) >> create mode 100644 target/s390x/sigp.c >> >
> David Hildenbrand (27): > s390x: raise CPU hotplug irq after really hotplugged > s390x/cpumodel: fix max STFL(E) bit number > target/s390x: get rid of next_core_id > s390x: introduce and use S390_MAX_CPUS It probably makes sense to apply 1-3/4 before the SMP support. Igor, can you take a look at Patch nr. 3? > s390/tcg: turn INTERRUPT_EXT into a mask > s390x/tcg: injection of emergency signals and extarnal calls > s390x/tcg: STOPPED cpus can never wake up > s390x/tcg: a CPU cannot switch state due to an interrupt > target/s390x: factor out handling of WAIT PSW into handle_wait() > s390x/kvm: pass ipb directly into handle_sigp() > s390x/kvm: generalize SIGP stop and restart interrupt injection > s390x/kvm: factor out storing of CPU status > target/s390x: proper cpu->be convertion in s390_store_status() > s390x/kvm: factor out storing of adtl CPU status > s390x/kvm: drop two debug prints > s390x/kvm: factor out SIGP code into sigp.c > s390x/kvm: factor out actual handling of STOP interrupts > s390x/tcg: implement SIGP SENSE RUNNING STATUS > s390x/tcg: implement SIGP SENSE > s390x/tcg: implement SIGP EXTERNAL CALL > s390x/tcg: implement SIGP EMERGENCY SIGNAL > s390x/tcg: implement SIGP CONDITIONAL EMERGENCY SIGNAL > s390x/tcg: implement STOP and RESET interrupts for TCG > s390x/tcg: flush the tlb on SIGP SET PREFIX > s390x/tcg: switch to new SIGP handling code > s390x/tcg: unlock NMI > s390x/tcg: refactor stfl(e) to use s390_get_feat_block()