Re: [PATCH v3] s390: kvm: adjust diag318 resets to retain data

[Collin Walling <walling@linux.ibm.com>]

On 11/17/21 02:43, Christian Borntraeger wrote:
> Am 09.11.21 um 21:56 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.
>>
>> 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: Christian Borntraeger <borntraeger@linux.ibm.com>
> 
> maybe add cc stable just in case there will be one.
> Can you resend with the final patch description and add Thomas as TO
> (not cc)
> as this should probably go via Thomas tree.

Done. Thank you.

>> ---
>>
>> Changelog:
>>
>>      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.
>> +         */
>>       }
>>   }
>>  
> 


-- 
Regards,
Collin

Stay safe and stay healthy