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

[Janosch Frank <frankja@xxxxxxxxxxxxx>]

On 29.08.2018 15:31, David Hildenbrand wrote:
> On 29.08.2018 15:19, Janosch Frank wrote:
>> We should not return with a lock.
>> We also have to increase the address when we do page clearing.
>>
>> Fixes: bd096f644319 ("KVM: s390: Add skey emulation fault handling")
>> Fixes: 0230cae75df6 ("KVM: s390: Replace clear_user with kvm_clear_guest")

These will will be removed, KVM: s390: Add skey emulation fault handling
is only in rc1.

>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>> ---
>>  arch/s390/kvm/priv.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
>> index d68f10441a16..7d61dccfd034 100644
>> --- a/arch/s390/kvm/priv.c
>> +++ b/arch/s390/kvm/priv.c
>> @@ -390,10 +390,10 @@ 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);
> 
> Indeed.
> 
>>  		if (rc >= 0)
>>  			start += PAGE_SIZE;
>>  	}
>> @@ -1002,13 +1002,14 @@ static int handle_pfmf(struct kvm_vcpu *vcpu)
>>  						      FAULT_FLAG_WRITE, &unlocked);
>>  				rc = !rc ? -EAGAIN : rc;
>>  			}
>> +			up_read(&current->mm->mmap_sem);
> 
> that's certainly correct
> 
>>  			if (rc == -EFAULT)
>>  				return kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
>> +			if (rc == -EAGAIN)
> 
> I would prefer "rc < 0" instead I guess.

No, I do not want to continue on any other rc.
But we could add this afterwards:
if (rc < 0)
	return rc;



> 
>> +				continue;
>>  
>> -			up_read(&current->mm->mmap_sem);
>> -			if (rc >= 0)
>> -				start += PAGE_SIZE;
>>  		}
>> +		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) {
>>
> 
>

Attachment: signature.asc
Description: OpenPGP digital signature