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
>>
>
--
Thanks,
David