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

[David Hildenbrand <david@xxxxxxxxxx>]

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

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


-- 

Thanks,

David / dhildenb