On 11/8/21 03:07, Christian Borntraeger wrote: > > > Am 05.11.21 um 23:46 schrieb Collin Walling: >> The CPNC portion of the diag 318 data is erroneously reset during an >> initial CPU reset caused by SIGP. Let's go ahead and relocate the >> diag318_info field within the CPUS390XState struct such that it is >> only zeroed during a clear reset. This way, the CPNC will be retained >> for each VCPU in the configuration after the diag 318 instruction >> has been invoked by the kernel. >> >> Additionally, the diag 318 data reset is handled via the CPU reset >> code. The set_diag318 code can be merged into the handler function >> and the helper functions can consequently be removed. >> >> Signed-off-by: Collin Walling <wall...@linux.ibm.com>
[...] >> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c >> index 5b1fdb55c4..ed9c477b6f 100644 >> --- a/target/s390x/kvm/kvm.c >> +++ b/target/s390x/kvm/kvm.c >> @@ -1576,18 +1576,6 @@ static int handle_sw_breakpoint(S390CPU *cpu, >> struct kvm_run *run) >> return -ENOENT; >> } >> -void kvm_s390_set_diag318(CPUState *cs, uint64_t diag318_info) >> -{ >> - CPUS390XState *env = &S390_CPU(cs)->env; >> - >> - /* Feat bit is set only if KVM supports sync for diag318 */ >> - if (s390_has_feat(S390_FEAT_DIAG_318)) { >> - env->diag318_info = diag318_info; >> - cs->kvm_run->s.regs.diag318 = diag318_info; >> - cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318; >> - } >> -} >> - >> static void handle_diag_318(S390CPU *cpu, struct kvm_run *run) >> { >> uint64_t reg = (run->s390_sieic.ipa & 0x00f0) >> 4; >> @@ -1604,8 +1592,11 @@ static void handle_diag_318(S390CPU *cpu, >> struct kvm_run *run) >> } >> CPU_FOREACH(t) { >> - run_on_cpu(t, s390_do_cpu_set_diag318, >> - RUN_ON_CPU_HOST_ULONG(diag318_info)); >> + CPUS390XState *env = &S390_CPU(t)->env; >> + >> + env->diag318_info = diag318_info; >> + t->kvm_run->s.regs.diag318 = diag318_info; >> + t->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318; > > I am not sure if this part works fine. What happens if > another CPU is currently in SIE (not stopped). > Then this change will be not visible in that CPU and in > fact this change will be overwritten when the CPU exits to QEMU. > Ah, I should've paid more attention to what run_on_cpu does. I now see that it makes CPU changes atomic. I'll reintroduce the helper as a static function and use the run_on_cpu again. -- Regards, Collin Stay safe and stay healthy