On 06.04.2018 11:40, Thomas Huth wrote: > On 06.04.2018 11:35, David Hildenbrand wrote: >> Manually having to use cpu_synchronize_state() is error prone. And as >> Christian Borntraeger discovered, e.g. handle_diag() is currently >> missing a cpu_synchronize_state(), as decode_basedisp_s() uses a >> general purpose register value internally. >> >> So let's do an overall cpu_synchronize_state(), which fixes at least the >> one mentioned BUG. We will clean up the superfluous cpu_synchronize_state() >> calls later. >> >> We now also call it (although maybe not neded) for >> - KVM_EXIT_S390_RESET -> s390_reipl_request() >> - KVM_EXIT_DEBUG -> kvm_arch_handle_debug_exit() >> - unmanagable/unimplemented intercepts >> - ICPT_CPU_STOP -> do_stop_interrupt() -> cpu gets halted >> - Scenarios where we inject an operation exception >> - handle_stsi() >> >> I don't think any of these are performance critical. Especially as we >> have all information directly contained in kvm_run, there are no >> additional IOCTLs to issue on modern kernels. >> >> Signed-off-by: David Hildenbrand <da...@redhat.com> >> --- >> target/s390x/kvm.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c >> index f570896dc1..fb59d92def 100644 >> --- a/target/s390x/kvm.c >> +++ b/target/s390x/kvm.c >> @@ -1778,6 +1778,8 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run >> *run) >> >> qemu_mutex_lock_iothread(); >> >> + cpu_synchronize_state(cs); > > Since we're in kvm.c here, maybe rather call kvm_cpu_synchronize_state() > directly to avoid the wrapper function? > > Thomas >
No strong opinion. I can see that kvm_cpu_synchronize_state() - is not used in target/s390x/kvm.c yet - is very rarely used in kvm code in general -- Thanks, David / dhildenb
