* [PATCH] kvm: s390: pv: Mark mm as protected after the set secure parameters and improve cleanup
@ 2020-10-30 14:01 Janosch Frank
2020-10-30 14:23 ` Christian Borntraeger
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Janosch Frank @ 2020-10-30 14:01 UTC (permalink / raw)
To: kvm; +Cc: david, thuth, cohuck, linux-s390, borntraeger, imbrenda
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>
---
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
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm: s390: pv: Mark mm as protected after the set secure parameters and improve cleanup
2020-10-30 14:01 [PATCH] kvm: s390: pv: Mark mm as protected after the set secure parameters and improve cleanup Janosch Frank
@ 2020-10-30 14:23 ` Christian Borntraeger
2020-11-11 8:17 ` Janosch Frank
2020-10-30 14:25 ` Christian Borntraeger
2020-11-10 11:31 ` Cornelia Huck
2 siblings, 1 reply; 7+ messages in thread
From: Christian Borntraeger @ 2020-10-30 14:23 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: david, thuth, cohuck, linux-s390, imbrenda
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>
Can we check this into devel to give it some test coverage?
> ---
> 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm: s390: pv: Mark mm as protected after the set secure parameters and improve cleanup
2020-10-30 14:01 [PATCH] kvm: s390: pv: Mark mm as protected after the set secure parameters and improve cleanup Janosch Frank
2020-10-30 14:23 ` Christian Borntraeger
@ 2020-10-30 14:25 ` Christian Borntraeger
2020-10-30 14:31 ` Janosch Frank
2020-11-10 11:31 ` Cornelia Huck
2 siblings, 1 reply; 7+ messages in thread
From: Christian Borntraeger @ 2020-10-30 14:25 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: david, thuth, cohuck, linux-s390, imbrenda
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>
As said in my other mail lets give it some days for the CI to test this.
> ---
> 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm: s390: pv: Mark mm as protected after the set secure parameters and improve cleanup
2020-10-30 14:25 ` Christian Borntraeger
@ 2020-10-30 14:31 ` Janosch Frank
0 siblings, 0 replies; 7+ messages in thread
From: Janosch Frank @ 2020-10-30 14:31 UTC (permalink / raw)
To: Christian Borntraeger, kvm; +Cc: david, thuth, cohuck, linux-s390, imbrenda
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
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm: s390: pv: Mark mm as protected after the set secure parameters and improve cleanup
2020-10-30 14:01 [PATCH] kvm: s390: pv: Mark mm as protected after the set secure parameters and improve cleanup Janosch Frank
2020-10-30 14:23 ` Christian Borntraeger
2020-10-30 14:25 ` Christian Borntraeger
@ 2020-11-10 11:31 ` Cornelia Huck
2 siblings, 0 replies; 7+ messages in thread
From: Cornelia Huck @ 2020-11-10 11:31 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, david, thuth, linux-s390, borntraeger, imbrenda
On Fri, 30 Oct 2020 10:01:41 -0400
Janosch Frank <frankja@linux.ibm.com> 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>
> ---
> 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(-)
Seems reasonable to me. I assume it has been given some coverage by now?
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm: s390: pv: Mark mm as protected after the set secure parameters and improve cleanup
2020-10-30 14:23 ` Christian Borntraeger
@ 2020-11-11 8:17 ` Janosch Frank
2020-11-11 8:26 ` Christian Borntraeger
0 siblings, 1 reply; 7+ messages in thread
From: Janosch Frank @ 2020-11-11 8:17 UTC (permalink / raw)
To: Christian Borntraeger, kvm; +Cc: david, thuth, cohuck, linux-s390, imbrenda
[-- Attachment #1.1.1: Type: text/plain, Size: 2528 bytes --]
On 10/30/20 3:23 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>
>
> Can we check this into devel to give it some test coverage?
I think this also lacks:
Fixes: 29b40f105ec8 ("KVM: s390: protvirt: Add initial vm and cpu
lifecycle handling")
>
>> ---
>> 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
>>
[-- Attachment #1.1.2: OpenPGP_0xE354E6B8E238B9F8.asc --]
[-- Type: application/pgp-keys, Size: 7995 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm: s390: pv: Mark mm as protected after the set secure parameters and improve cleanup
2020-11-11 8:17 ` Janosch Frank
@ 2020-11-11 8:26 ` Christian Borntraeger
0 siblings, 0 replies; 7+ messages in thread
From: Christian Borntraeger @ 2020-11-11 8:26 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: david, thuth, cohuck, linux-s390, imbrenda
On 11.11.20 09:17, Janosch Frank wrote:
> On 10/30/20 3:23 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>
>>
>> Can we check this into devel to give it some test coverage?
>
> I think this also lacks:
> Fixes: 29b40f105ec8 ("KVM: s390: protvirt: Add initial vm and cpu
> lifecycle handling")
Yes, it does. I will schedule for kvm/master.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-11-11 8:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-30 14:01 [PATCH] kvm: s390: pv: Mark mm as protected after the set secure parameters and improve cleanup Janosch Frank
2020-10-30 14:23 ` Christian Borntraeger
2020-11-11 8:17 ` Janosch Frank
2020-11-11 8:26 ` Christian Borntraeger
2020-10-30 14:25 ` Christian Borntraeger
2020-10-30 14:31 ` Janosch Frank
2020-11-10 11:31 ` Cornelia Huck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox