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

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

[Claudio Imbrenda]

The first patch is just a minimal header update to compile the second
patch; it can be safely discarded once the Linux headers are updated to
6.2.

The second patch adds support for asynchronous teardown of protected
guests when rebooting. First the existing guest is prepared for
asynchronous teardown, the rebooted guest will be able to continue
immediately, while a background thread actually performs the necessary
cleanup.

v1->v2:
* remove useless snprintf and pass the name of the thread directly
* make the name of the thread more understandable

Claudio Imbrenda (2):
  Linux header update
  s390x/pv: Add support for asynchronous teardown for reboot

 hw/s390x/pv.c              | 25 +++++++++++++++++++++++++
 hw/s390x/s390-virtio-ccw.c |  5 ++++-
 include/hw/s390x/pv.h      |  2 ++
 linux-headers/linux/kvm.h  |  3 +++
 4 files changed, 34 insertions(+), 1 deletion(-)

-- 
2.39.0

[PATCH v2 1/2] Linux header update

[Claudio Imbrenda]

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 linux-headers/linux/kvm.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index ebdafa576d..122b273433 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1175,6 +1175,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_VM_DISABLE_NX_HUGE_PAGES 220
 #define KVM_CAP_S390_ZPCI_OP 221
 #define KVM_CAP_S390_CPU_TOPOLOGY 222
+#define KVM_CAP_S390_PROTECTED_ASYNC_DISABLE 224
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -1737,6 +1738,8 @@ enum pv_cmd_id {
 	KVM_PV_UNSHARE_ALL,
 	KVM_PV_INFO,
 	KVM_PV_DUMP,
+	KVM_PV_ASYNC_CLEANUP_PREPARE,
+	KVM_PV_ASYNC_CLEANUP_PERFORM,
 };
 
 struct kvm_pv_cmd {
-- 
2.39.0

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

[Claudio Imbrenda]

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));
+    qemu_thread_create(t, "async_cleanup", s390_pv_do_unprot_async_fn, NULL,
+                       QEMU_THREAD_DETACHED);
+
+    return true;
+}
+
 int s390_pv_set_sec_parms(uint64_t origin, uint64_t length)
 {
     struct kvm_s390_pv_sec_parm args = {
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index f22f61b8b6..503f212a31 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -41,6 +41,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/s390x/tod.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/cpus.h"
 #include "hw/s390x/pv.h"
 #include "migration/blocker.h"
 #include "qapi/visitor.h"
@@ -329,7 +330,9 @@ static inline void s390_do_cpu_ipl(CPUState *cs, run_on_cpu_data arg)
 
 static void s390_machine_unprotect(S390CcwMachineState *ms)
 {
-    s390_pv_vm_disable();
+    if (!s390_pv_vm_try_disable_async()) {
+        s390_pv_vm_disable();
+    }
     ms->pv = false;
     migrate_del_blocker(pv_mig_blocker);
     error_free_or_abort(&pv_mig_blocker);
diff --git a/include/hw/s390x/pv.h b/include/hw/s390x/pv.h
index 9360aa1091..966306a9db 100644
--- a/include/hw/s390x/pv.h
+++ b/include/hw/s390x/pv.h
@@ -41,6 +41,7 @@ static inline bool s390_is_pv(void)
 int s390_pv_query_info(void);
 int s390_pv_vm_enable(void);
 void s390_pv_vm_disable(void);
+bool s390_pv_vm_try_disable_async(void);
 int s390_pv_set_sec_parms(uint64_t origin, uint64_t length);
 int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak);
 void s390_pv_prep_reset(void);
@@ -60,6 +61,7 @@ static inline bool s390_is_pv(void) { return false; }
 static inline int s390_pv_query_info(void) { return 0; }
 static inline int s390_pv_vm_enable(void) { return 0; }
 static inline void s390_pv_vm_disable(void) {}
+static inline bool s390_pv_vm_try_disable_async(void) { return false; }
 static inline int s390_pv_set_sec_parms(uint64_t origin, uint64_t length) { return 0; }
 static inline int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak) { return 0; }
 static inline void s390_pv_prep_reset(void) {}
-- 
2.39.0

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;
> +}

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

[Claudio Imbrenda]

On Tue, 17 Jan 2023 09:53:46 +0100
Thomas Huth <thuth@redhat.com> wrote:

[...]

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

I assumed that QEMU_THREAD_DETACHED took care of that; it seems like I
was mistaken (oops).

I'll find a way to fix this

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