Re: [PATCH v2] KVM: s390: Fix pfmf and conditional skey emulation

[David Hildenbrand <david@xxxxxxxxxx>]

On 30.08.2018 12:11, Christian Borntraeger wrote:
> Please re-add the "Fixes" tag. While not relevant for stable, it might be
> relevant for backport.

Isn't that a potential endless loop in the pfmf handler? (!PFMF_SK)

If so, this should be stable no?

> 
> On 08/30/2018 10:21 AM, Janosch Frank wrote:
>> On 30.08.2018 10:13, Janosch Frank wrote:
>>> We should not return with a lock.
>>> We also have to increase the address when we do page clearing.
>>>
>>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>>
>> It's time for a vacation, patch will be renamed to:
>> KVM: s390: Fix skey emulation fault error handling
>>
>>> ---
>>>
>>> I knew there were more instances...
>>>
>>> ---
>>>  arch/s390/kvm/priv.c | 30 ++++++++++++++++++------------
>>>  1 file changed, 18 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
>>> index d68f10441a16..8679bd74d337 100644
>>> --- a/arch/s390/kvm/priv.c
>>> +++ b/arch/s390/kvm/priv.c
>>> @@ -280,9 +280,11 @@ static int handle_iske(struct kvm_vcpu *vcpu)
>>>  			goto retry;
>>>  		}
>>>  	}
>>> -	if (rc)
>>> -		return kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
>>>  	up_read(&current->mm->mmap_sem);
>>> +	if (rc == -EFAULT)
>>> +		return kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
>>> +	if (rc < 0)
>>> +		return rc;
>>>  	vcpu->run->s.regs.gprs[reg1] &= ~0xff;
>>>  	vcpu->run->s.regs.gprs[reg1] |= key;
>>>  	return 0;
>>> @@ -324,9 +326,11 @@ static int handle_rrbe(struct kvm_vcpu *vcpu)
>>>  			goto retry;
>>>  		}
>>>  	}
>>> -	if (rc < 0)
>>> -		return kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
>>>  	up_read(&current->mm->mmap_sem);
>>> +	if (rc == -EFAULT)
>>> +		return kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
>>> +	if (rc < 0)
>>> +		return rc;
>>>  	kvm_s390_set_psw_cc(vcpu, rc);
>>>  	return 0;
>>>  }
>>> @@ -390,12 +394,12 @@ static int handle_sske(struct kvm_vcpu *vcpu)
>>>  					      FAULT_FLAG_WRITE, &unlocked);
>>>  			rc = !rc ? -EAGAIN : rc;
>>>  		}
>>> +		up_read(&current->mm->mmap_sem);
>>>  		if (rc == -EFAULT)
>>>  			return kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
>>> -
>>> -		up_read(&current->mm->mmap_sem);
>>> -		if (rc >= 0)
>>> -			start += PAGE_SIZE;
>>> +		if (rc < 0)
>>> +			return rc;
>>> +		start += PAGE_SIZE;
>>>  	}
>>>  
>>>  	if (m3 & (SSKE_MC | SSKE_MR)) {
>>> @@ -1002,13 +1006,15 @@ static int handle_pfmf(struct kvm_vcpu *vcpu)
>>>  						      FAULT_FLAG_WRITE, &unlocked);
>>>  				rc = !rc ? -EAGAIN : rc;
>>>  			}
>>> +			up_read(&current->mm->mmap_sem);
>>>  			if (rc == -EFAULT)
>>>  				return kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
>>> -
>>> -			up_read(&current->mm->mmap_sem);
>>> -			if (rc >= 0)
>>> -				start += PAGE_SIZE;
>>> +			if (rc == -EAGAIN)
>>> +				continue;
>>> +			if (rc < 0)
>>> +				return rc;
>>>  		}
>>> +		start += PAGE_SIZE;
>>>  	}
>>>  	if (vcpu->run->s.regs.gprs[reg1] & PFMF_FSC) {
>>>  		if (psw_bits(vcpu->arch.sie_block->gpsw).eaba == PSW_BITS_AMODE_64BIT) {
>>>
>>
>>
> 


-- 

Thanks,

David / dhildenb