On 10/22/20 3:43 PM, Collin Walling wrote:
> On 10/16/20 3:44 AM, Christian Borntraeger wrote:
>>
>>
>> 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.
>>
> 
> A few questions regarding how resets for diag318 should work here:
> 
> The AR states that any copies retained by the diag318 should be set to 0
> on a clear reset and load normal -- I thought both of those resets were
> implicitly called by diag308 as well?
> 
> Should the register used to store diag318 info not be set to 0 *by KVM*
> then? Should the values be set *by QEMU* and a subsequent sync_regs will
> ensure things are sane on the KVM side?

Just a FYI for the non-IBMers reading in:

I have spoken to the author of the architecture and cleared up our way
forward.

* We need to clear on diag 308 subcodes 0,1,3,4
* SIGP does not alter diag318 data in any way
* We need to set the cpnc on all VCPUs of the VM


> 
>>>
>>>>
>>>> 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)
>>>>
>>>
>>>
> 
>