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

Janosch Frank
2020-10-30T14:31:09Z

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