linux-kernel - Re: [PATCH v13 1/6] KVM: s390: pv: asynchronous destroy for reboot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <166030300049.24812.17231340197534047162@localhost.localdomain>
Date:   Fri, 12 Aug 2022 13:16:40 +0200
From:   Nico Boehr <nrb@...ux.ibm.com>
To:     Claudio Imbrenda <imbrenda@...ux.ibm.com>, kvm@...r.kernel.org
Cc:     borntraeger@...ibm.com, frankja@...ux.ibm.com, thuth@...hat.com,
        pasic@...ux.ibm.com, david@...hat.com, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org, scgl@...ux.ibm.com,
        seiden@...ux.ibm.com
Subject: Re: [PATCH v13 1/6] KVM: s390: pv: asynchronous destroy for reboot

Quoting Claudio Imbrenda (2022-08-10 14:56:20)
> Until now, destroying a protected guest was an entirely synchronous
> operation that could potentially take a very long time, depending on
> the size of the guest, due to the time needed to clean up the address
> space from protected pages.
> 
> This patch implements an asynchronous destroy mechanism, that allows a
> protected guest to reboot significantly faster than previously.
> 
> This is achieved by clearing the pages of the old guest in background.
> In case of reboot, the new guest will be able to run in the same
> address space almost immediately.
> 
> The old protected guest is then only destroyed when all of its memory has
> been destroyed or otherwise made non protected.
> 
> Two new PV commands are added for the KVM_S390_PV_COMMAND ioctl:
> 
> KVM_PV_ASYNC_CLEANUP_PREPARE: prepares the current protected VM for
> asynchronous teardown. 

I would add:

A protected VM which has been prepared for asynchronous teardown is called a *set aside VM*. A set a side VM never has any CPUs associated with it, but is still registered with the Ultravisor and may have memory.

> The current VM will then continue immediately
> as non-protected. If a protected VM had already been set aside without
> starting the teardown process, this call will fail. There can be at
> most one prepared VM at any time.
> 
> KVM_PV_ASYNC_CLEANUP_PERFORM: tears down the protected VM previously
> set aside for asynchronous teardown. This PV command should ideally be
> issued by userspace from a separate thread. If a fatal signal is
> received (or the process terminates naturally), the command will
> terminate immediately without completing. 

I would add:

A set aside VM where cleanup started but was interrupted is called a *leftover VM*. There can be multiple leftovers per VM, which are tracked in the need_cleanup list.

I would like a more consistent language. We have three different names for the leftover in the patch below:
- leftover,
- the struct describing a leftover is called pv_vm_to_be_destroyed,
- the list of leftover vms is called need_cleanup.

It would be nice if this were a little more consistent, e.g. by renaming pv_vm_to_be_destroyed to pvm_vm_leftover and maybe need_cleanup to leftovers.

Otherwise, this looks good. I would be glad if you pick up my naming suggestions, but feel free to add:

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