Re: [PATCH v2 1/2] s390/kvm: fix diag318 reset

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 16.10.20 09:34, Janosch Frank wrote:
> On 10/15/20 9:59 PM, Collin Walling wrote:
>> The DIAGNOSE 0x0318 instruction must be reset on a normal and clear
>> reset. However, this was missed for the clear reset case.
>>
>> Let's fix this by resetting the information during a normal reset. 
>> Since clear reset is a superset of normal reset, the info will
>> still reset on a clear reset.
> 
> The architecture really confuses me here but I think we don't want this
> in the kernel VCPU reset handlers at all.
> 
> This needs to be reset per VM *NOT* per VCPU.
> Hence the resets are bound to diag308 and not SIGP.
> 
> I.e. we need to clear it in QEMU's VM reset handler.
> It's still early and I have yet to consume my first coffee, am I missing
> something?

I agree with Janosch. architecture indicates that this should only be reset
for VM-wide resets, e.g. sigp orders 11 and 12 are explicitly mentioned
to NOT reset the value.

> 
>>
>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
>> ---
>>  arch/s390/kvm/kvm-s390.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 6b74b92c1a58..b0cf8367e261 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -3516,6 +3516,7 @@ static void kvm_arch_vcpu_ioctl_normal_reset(struct kvm_vcpu *vcpu)
>>  	vcpu->arch.sie_block->gpsw.mask &= ~PSW_MASK_RI;
>>  	vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID;
>>  	memset(vcpu->run->s.regs.riccb, 0, sizeof(vcpu->run->s.regs.riccb));
>> +	vcpu->run->s.regs.diag318 = 0;
>>  
>>  	kvm_clear_async_pf_completion_queue(vcpu);
>>  	if (!kvm_s390_user_cpu_state_ctrl(vcpu->kvm))
>> @@ -3582,7 +3583,6 @@ static void kvm_arch_vcpu_ioctl_clear_reset(struct kvm_vcpu *vcpu)
>>  
>>  	regs->etoken = 0;
>>  	regs->etoken_extension = 0;
>> -	regs->diag318 = 0;
>>  }
>>  
>>  int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
>>
> 
>