diff mbox series

[v10,13/19] KVM: s390: pv: destroy the configuration before its memory

Message ID 20220414080311.1084834-14-imbrenda@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series KVM: s390: pv: implement lazy destroy for reboot | expand

Commit Message

Claudio Imbrenda April 14, 2022, 8:03 a.m. UTC
Move the Destroy Secure Configuration UVC before the loop to destroy
the memory. If the protected VM has memory, it will be cleaned up and
made accessible by the Destroy Secure Configuraion UVC. The struct
page for the relevant pages will still have the protected bit set, so
the loop is still needed to clean that up.

Switching the order of those two operations does not change the
outcome, but it is significantly faster.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/pv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Nico Boehr May 30, 2022, 7:37 a.m. UTC | #1
On Thu, 2022-04-14 at 10:03 +0200, Claudio Imbrenda wrote:
> Move the Destroy Secure Configuration UVC before the loop to destroy
> the memory. If the protected VM has memory, it will be cleaned up and
> made accessible by the Destroy Secure Configuraion UVC. The struct
> page for the relevant pages will still have the protected bit set, so
> the loop is still needed to clean that up.
> 
> Switching the order of those two operations does not change the
> outcome, but it is significantly faster.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Reviewed-by: Nico Boehr <nrb@linux.ibm.com>

See one tiny thing below.

> diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
> index be3b467f8feb..bd850be08c86 100644
> --- a/arch/s390/kvm/pv.c
> +++ b/arch/s390/kvm/pv.c
[...]
> -       cc = uv_cmd_nodata(kvm_s390_pv_get_handle(kvm),
> -                          UVC_CMD_DESTROY_SEC_CONF, rc, rrc);
>         WRITE_ONCE(kvm->arch.gmap->guest_handle, 0);

Maybe it makes sense to also move the WRITE_ONCE up.
Claudio Imbrenda May 30, 2022, 12:05 p.m. UTC | #2
On Mon, 30 May 2022 09:37:37 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:

> On Thu, 2022-04-14 at 10:03 +0200, Claudio Imbrenda wrote:
> > Move the Destroy Secure Configuration UVC before the loop to destroy
> > the memory. If the protected VM has memory, it will be cleaned up and
> > made accessible by the Destroy Secure Configuraion UVC. The struct
> > page for the relevant pages will still have the protected bit set, so
> > the loop is still needed to clean that up.
> > 
> > Switching the order of those two operations does not change the
> > outcome, but it is significantly faster.
> > 
> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>  
> 
> Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
> 
> See one tiny thing below.
> 
> > diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
> > index be3b467f8feb..bd850be08c86 100644
> > --- a/arch/s390/kvm/pv.c
> > +++ b/arch/s390/kvm/pv.c  
> [...]
> > -       cc = uv_cmd_nodata(kvm_s390_pv_get_handle(kvm),
> > -                          UVC_CMD_DESTROY_SEC_CONF, rc, rrc);
> >         WRITE_ONCE(kvm->arch.gmap->guest_handle, 0);  
> 
> Maybe it makes sense to also move the WRITE_ONCE up.

yes, I'll move it up, so that it is right after the UVC again
diff mbox series

Patch

diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
index be3b467f8feb..bd850be08c86 100644
--- a/arch/s390/kvm/pv.c
+++ b/arch/s390/kvm/pv.c
@@ -168,6 +168,8 @@  int kvm_s390_pv_deinit_vm(struct kvm *kvm, u16 *rc, u16 *rrc)
 {
 	int cc;
 
+	cc = uv_cmd_nodata(kvm_s390_pv_get_handle(kvm),
+			   UVC_CMD_DESTROY_SEC_CONF, rc, rrc);
 	/*
 	 * if the mm still has a mapping, make all its pages accessible
 	 * before destroying the guest
@@ -177,8 +179,6 @@  int kvm_s390_pv_deinit_vm(struct kvm *kvm, u16 *rc, u16 *rrc)
 		mmput(kvm->mm);
 	}
 
-	cc = uv_cmd_nodata(kvm_s390_pv_get_handle(kvm),
-			   UVC_CMD_DESTROY_SEC_CONF, rc, rrc);
 	WRITE_ONCE(kvm->arch.gmap->guest_handle, 0);
 	if (!cc) {
 		atomic_dec(&kvm->mm->context.protected_count);