| Message ID | 20211117152303.627969-1-walling@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] s390: kvm: adjust diag318 resets to retain data | expand |
Am 17.11.21 um 16:23 schrieb Collin Walling: > The CPNC portion of the diag318 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 diag318 instruction > has been invoked. > > The s390_machine_reset code already takes care of zeroing the diag318 > data on VM resets, which also cover resets caused by diag308. > > Signed-off-by: Collin Walling <walling@linux.ibm.com> > Fixes: fabdada9357b ("s390: guest support for diagnose 0x318") > Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> > Reviewed-by: Janosch Frank <frankja@linux.ibm.com> > Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com> Thomas, I think this is good to go. Will you take it via your tree? > --- > > Changelog: > > v4 > - fixed up commit message and added r-b's > > v3 > - reverted changes from previous versions > - simply relocate diag318_info in CPU State struct > - add comment in set_diag318 to explain resets > > v2 > - handler uses run_on_cpu again > - reworded commit message slightly > - added fixes and reported-by tags > > v3 > - nixed code reduction changes > - added a comment to diag318 handler to briefly describe > when relevent data is zeroed > > --- > target/s390x/cpu.h | 4 ++-- > target/s390x/kvm/kvm.c | 4 ++++ > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h > index 3153d053e9..88aace36ff 100644 > --- a/target/s390x/cpu.h > +++ b/target/s390x/cpu.h > @@ -63,6 +63,8 @@ struct CPUS390XState { > uint64_t etoken; /* etoken */ > uint64_t etoken_extension; /* etoken extension */ > > + uint64_t diag318_info; > + > /* Fields up to this point are not cleared by initial CPU reset */ > struct {} start_initial_reset_fields; > > @@ -118,8 +120,6 @@ struct CPUS390XState { > uint16_t external_call_addr; > DECLARE_BITMAP(emergency_signals, S390_MAX_CPUS); > > - uint64_t diag318_info; > - > #if !defined(CONFIG_USER_ONLY) > uint64_t tlb_fill_tec; /* translation exception code during tlb_fill */ > int tlb_fill_exc; /* exception number seen during tlb_fill */ > diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c > index 5b1fdb55c4..6acf14d5ec 100644 > --- a/target/s390x/kvm/kvm.c > +++ b/target/s390x/kvm/kvm.c > @@ -1585,6 +1585,10 @@ void kvm_s390_set_diag318(CPUState *cs, uint64_t diag318_info) > env->diag318_info = diag318_info; > cs->kvm_run->s.regs.diag318 = diag318_info; > cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318; > + /* > + * diag 318 info is zeroed during a clear reset and > + * diag 308 IPL subcodes. > + */ > } > } > >
On 23/11/2021 10.28, Christian Borntraeger wrote: > Am 17.11.21 um 16:23 schrieb Collin Walling: >> The CPNC portion of the diag318 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 diag318 instruction >> has been invoked. >> >> The s390_machine_reset code already takes care of zeroing the diag318 >> data on VM resets, which also cover resets caused by diag308. >> >> Signed-off-by: Collin Walling <walling@linux.ibm.com> >> Fixes: fabdada9357b ("s390: guest support for diagnose 0x318") >> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> >> Reviewed-by: Janosch Frank <frankja@linux.ibm.com> >> Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com> > > Thomas, I think this is good to go. Will you take it via your tree? Yes, queued to s390x-next now: https://gitlab.com/thuth/qemu/-/commits/s390x-next/ Thomas
Polite ping. I may have missed if this patch was picked already. Thanks!
On 01/12/2021 19.45, Collin Walling wrote:
> Polite ping. I may have missed if this patch was picked already. Thanks!
I've already queued it to my s390x-next branch:
https://gitlab.com/thuth/qemu/-/commits/s390x-next/
It just came in very late for 6.2, and it didn't seem too critical to me, so
I didn't sent a separate pull request for this one. Thus it will get merged
once the hard freeze period of QEMU is over.
Thomas
On 12/2/21 04:23, Thomas Huth wrote: > On 01/12/2021 19.45, Collin Walling wrote: >> Polite ping. I may have missed if this patch was picked already. Thanks! > > I've already queued it to my s390x-next branch: > > https://gitlab.com/thuth/qemu/-/commits/s390x-next/ > > It just came in very late for 6.2, and it didn't seem too critical to > me, so I didn't sent a separate pull request for this one. Thus it will > get merged once the hard freeze period of QEMU is over. > > Thomas > Apologies, I missed this. My mail filters are sometimes messed up and I didn't see this thread. All is good :)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index 3153d053e9..88aace36ff 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -63,6 +63,8 @@ struct CPUS390XState { uint64_t etoken; /* etoken */ uint64_t etoken_extension; /* etoken extension */ + uint64_t diag318_info; + /* Fields up to this point are not cleared by initial CPU reset */ struct {} start_initial_reset_fields; @@ -118,8 +120,6 @@ struct CPUS390XState { uint16_t external_call_addr; DECLARE_BITMAP(emergency_signals, S390_MAX_CPUS); - uint64_t diag318_info; - #if !defined(CONFIG_USER_ONLY) uint64_t tlb_fill_tec; /* translation exception code during tlb_fill */ int tlb_fill_exc; /* exception number seen during tlb_fill */ diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c index 5b1fdb55c4..6acf14d5ec 100644 --- a/target/s390x/kvm/kvm.c +++ b/target/s390x/kvm/kvm.c @@ -1585,6 +1585,10 @@ void kvm_s390_set_diag318(CPUState *cs, uint64_t diag318_info) env->diag318_info = diag318_info; cs->kvm_run->s.regs.diag318 = diag318_info; cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318; + /* + * diag 318 info is zeroed during a clear reset and + * diag 308 IPL subcodes. + */ } }