Re: [PATCH v2 2/2] s390x/pv: Add support for asynchronous teardown for reboot

[Thomas Huth]

On 05/01/2023 16.50, Claudio Imbrenda wrote:
> This patch adds support for the asynchronous teardown for reboot for
> protected VMs.
>
> When attempting to tear down a protected VM, try to use the new
> asynchronous interface first. If that fails, fall back to the classic
> synchronous one.
>
> The asynchronous interface involves invoking the new
> KVM_PV_ASYNC_DISABLE_PREPARE command for the KVM_S390_PV_COMMAND ioctl.
>
> This will prepare the current protected VM for asynchronous teardown.
> Once the protected VM is prepared for teardown, execution can continue
> immediately.
>
> Once the protected VM has been prepared, a new thread is started to
> actually perform the teardown. The new thread uses the new
> KVM_PV_ASYNC_DISABLE command for the KVM_S390_PV_COMMAND ioctl. The
> previously prepared protected VM is torn down in the new thread.
>
> Once KVM_PV_ASYNC_DISABLE is invoked, it is possible to use
> KVM_PV_ASYNC_DISABLE_PREPARE again. If a protected VM has already been
> prepared and its cleanup has not started, it will not be possible to
> prepare a new VM. In that case the classic synchronous teardown has to
> be performed.
>
> The synchronous teardown will now also clean up any prepared VMs whose
> asynchronous teardown has not been initiated yet.
>
> This considerably speeds up the reboot of a protected VM; for large VMs
> especially, it could take a long time to perform a reboot with the
> traditional synchronous teardown, while with this patch it is almost
> immediate.
>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>   hw/s390x/pv.c              | 25 +++++++++++++++++++++++++
>   hw/s390x/s390-virtio-ccw.c |  5 ++++-
>   include/hw/s390x/pv.h      |  2 ++
>   3 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/hw/s390x/pv.c b/hw/s390x/pv.c
> index 8dfe92d8df..3d1c529eb5 100644
> --- a/hw/s390x/pv.c
> +++ b/hw/s390x/pv.c
> @@ -16,6 +16,7 @@
>   #include "qapi/error.h"
>   #include "qemu/error-report.h"
>   #include "sysemu/kvm.h"
> +#include "sysemu/cpus.h"
>   #include "qom/object_interfaces.h"
>   #include "exec/confidential-guest-support.h"
>   #include "hw/s390x/ipl.h"
> @@ -107,6 +108,30 @@ void s390_pv_vm_disable(void)
>        s390_pv_cmd_exit(KVM_PV_DISABLE, NULL);
>   }
>   
> +static void *s390_pv_do_unprot_async_fn(void *p)
> +{
> +     s390_pv_cmd_exit(KVM_PV_ASYNC_CLEANUP_PERFORM, NULL);
> +     return NULL;
> +}
> +
> +bool s390_pv_vm_try_disable_async(void)
> +{
> +    QemuThread *t;
> +
> +    if (!kvm_check_extension(kvm_state, KVM_CAP_S390_PROTECTED_ASYNC_DISABLE)) 
> {
> +        return false;
> +    }
> +    if (s390_pv_cmd(KVM_PV_ASYNC_CLEANUP_PREPARE, NULL) != 0) {
> +        return false;
> +    }
> +
> +    t = g_malloc0(sizeof(QemuThread));

Sorry for not noticing it in v1 already ... but isn't this leaking memory? 
Who's supposed to free "t" again?

 Thomas


> +    qemu_thread_create(t, "async_cleanup", s390_pv_do_unprot_async_fn, NULL,
> +                       QEMU_THREAD_DETACHED);
> +
> +    return true;
> +}