Re: [PATCH] kvm: s390: pv: Mark mm as protected after the set secure parameters and improve cleanup

[Janosch Frank <frankja@linux.ibm.com>]

On 10/30/20 3:25 PM, Christian Borntraeger wrote:
> On 30.10.20 15:01, Janosch Frank wrote:
>> We can only have protected guest pages after a successful set secure
>> parameters call as only then the UV allows imports and unpacks.
>>
>> By moving the test we can now also check for it in s390_reset_acc()
>> and do an early return if it is 0.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> 
> Looks sane.
> 
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>

Thanks

> 
> As said in my other mail lets give it some days for the CI to test this.

Sure, I'll push it in a minute

> 
>> ---
>>  arch/s390/kvm/kvm-s390.c | 2 +-
>>  arch/s390/kvm/pv.c       | 3 ++-
>>  arch/s390/mm/gmap.c      | 2 ++
>>  3 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 6b74b92c1a58..08ea6c4735cd 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -2312,7 +2312,7 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>>  		struct kvm_s390_pv_unp unp = {};
>>
>>  		r = -EINVAL;
>> -		if (!kvm_s390_pv_is_protected(kvm))
>> +		if (!kvm_s390_pv_is_protected(kvm) || !mm_is_protected(kvm->mm))
>>  			break;
>>
>>  		r = -EFAULT;
>> diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
>> index eb99e2f95ebe..f5847f9dec7c 100644
>> --- a/arch/s390/kvm/pv.c
>> +++ b/arch/s390/kvm/pv.c
>> @@ -208,7 +208,6 @@ int kvm_s390_pv_init_vm(struct kvm *kvm, u16 *rc, u16 *rrc)
>>  		return -EIO;
>>  	}
>>  	kvm->arch.gmap->guest_handle = uvcb.guest_handle;
>> -	atomic_set(&kvm->mm->context.is_protected, 1);
>>  	return 0;
>>  }
>>
>> @@ -228,6 +227,8 @@ int kvm_s390_pv_set_sec_parms(struct kvm *kvm, void *hdr, u64 length, u16 *rc,
>>  	*rrc = uvcb.header.rrc;
>>  	KVM_UV_EVENT(kvm, 3, "PROTVIRT VM SET PARMS: rc %x rrc %x",
>>  		     *rc, *rrc);
>> +	if (!cc)
>> +		atomic_set(&kvm->mm->context.is_protected, 1);
>>  	return cc ? -EINVAL : 0;
>>  }
>>
>> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c
>> index cfb0017f33a7..64795d034926 100644
>> --- a/arch/s390/mm/gmap.c
>> +++ b/arch/s390/mm/gmap.c
>> @@ -2690,6 +2690,8 @@ static const struct mm_walk_ops reset_acc_walk_ops = {
>>  #include <linux/sched/mm.h>
>>  void s390_reset_acc(struct mm_struct *mm)
>>  {
>> +	if (!mm_is_protected(mm))
>> +		return;
>>  	/*
>>  	 * we might be called during
>>  	 * reset:                             we walk the pages and clear
>>