diff mbox series

[v9,04/15] s390x: protvirt: Inhibit balloon when switching to protected mode

Message ID 20200311132151.172389-5-frankja@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: Protected Virtualization support | expand

Commit Message

Janosch Frank March 11, 2020, 1:21 p.m. UTC
Ballooning in protected VMs can only be done when the guest shares the
pages it gives to the host. If pages are not shared, the integrity
checks will fail once those pages have been altered and are given back
to the guest.

As we currently do not yet have a solution for this we will continue
like this:

1. We block ballooning now in QEMU (with this patch)

2. Later we will provide a change to virtio that removes the blocker
and adds VIRTIO_F_IOMMU_PLATFORM automatically by QEMU when doing the
protvirt switch. This is ok as the guest balloon driver will reject to
work with the IOMMU change

3. Later we can fix the guest balloon driver to accept the IOMMU
feature bit and correctly exercise sharing and unsharing of balloon
pages

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Claudio Imbrenda March 13, 2020, 12:57 p.m. UTC | #1
On Wed, 11 Mar 2020 09:21:40 -0400
Janosch Frank <frankja@linux.ibm.com> wrote:

> Ballooning in protected VMs can only be done when the guest shares the
> pages it gives to the host. If pages are not shared, the integrity
> checks will fail once those pages have been altered and are given back
> to the guest.
> 
> As we currently do not yet have a solution for this we will continue
> like this:
> 
> 1. We block ballooning now in QEMU (with this patch)
> 
> 2. Later we will provide a change to virtio that removes the blocker
> and adds VIRTIO_F_IOMMU_PLATFORM automatically by QEMU when doing the
> protvirt switch. This is ok as the guest balloon driver will reject to
> work with the IOMMU change
> 
> 3. Later we can fix the guest balloon driver to accept the IOMMU
> feature bit and correctly exercise sharing and unsharing of balloon
> pages
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index deb31e060052d279..066e01f303c35671 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/balloon.h"
>  #include "hw/s390x/pv.h"
>  #include "migration/blocker.h"
>  
> @@ -326,6 +327,7 @@ static void
> s390_machine_unprotect(S390CcwMachineState *ms) ms->pv = false;
>      migrate_del_blocker(pv_mig_blocker);
>      error_free_or_abort(&pv_mig_blocker);
> +    qemu_balloon_inhibit(false);
>  }
>  
>  static int s390_machine_protect(S390CcwMachineState *ms)
> @@ -333,10 +335,12 @@ static int
> s390_machine_protect(S390CcwMachineState *ms) Error *local_err = NULL;
>      int rc;
>  
> +    qemu_balloon_inhibit(true);
>      error_setg(&pv_mig_blocker,
>                 "protected VMs are currently not migrateable.");
>      rc = migrate_add_blocker(pv_mig_blocker, &local_err);
>      if (local_err) {
> +        qemu_balloon_inhibit(false);
>          error_report_err(local_err);
>          error_free_or_abort(&pv_mig_blocker);
>          return rc;
> @@ -345,6 +349,7 @@ static int
> s390_machine_protect(S390CcwMachineState *ms) /* Create SE VM */
>      rc = s390_pv_vm_enable();
>      if (rc) {
> +        qemu_balloon_inhibit(false);
>          error_report_err(local_err);
>          migrate_del_blocker(pv_mig_blocker);
>          error_free_or_abort(&pv_mig_blocker);

looks straightforward

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Cornelia Huck March 18, 2020, 11:42 a.m. UTC | #2
On Wed, 11 Mar 2020 09:21:40 -0400
Janosch Frank <frankja@linux.ibm.com> wrote:

> Ballooning in protected VMs can only be done when the guest shares the
> pages it gives to the host. If pages are not shared, the integrity
> checks will fail once those pages have been altered and are given back
> to the guest.
> 
> As we currently do not yet have a solution for this we will continue
> like this:
> 
> 1. We block ballooning now in QEMU (with this patch)

add trailing '.'

> 
> 2. Later we will provide a change to virtio that removes the blocker

s/Later/Later,/

> and adds VIRTIO_F_IOMMU_PLATFORM automatically by QEMU when doing the
> protvirt switch. This is ok as the guest balloon driver will reject to
> work with the IOMMU change

"This is OK, as the balloon driver in Linux (the only supported guest)
will refuse to work with the IOMMU_PLATFORM feature bit set."

?

> 
> 3. Later we can fix the guest balloon driver to accept the IOMMU

"Later, we can fix the Linux guest balloon driver..."

> feature bit and correctly exercise sharing and unsharing of balloon
> pages

again, missing trailing '.'

> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index deb31e060052d279..066e01f303c35671 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/balloon.h"
>  #include "hw/s390x/pv.h"
>  #include "migration/blocker.h"
>  
> @@ -326,6 +327,7 @@ static void s390_machine_unprotect(S390CcwMachineState *ms)
>      ms->pv = false;
>      migrate_del_blocker(pv_mig_blocker);
>      error_free_or_abort(&pv_mig_blocker);
> +    qemu_balloon_inhibit(false);
>  }
>  
>  static int s390_machine_protect(S390CcwMachineState *ms)
> @@ -333,10 +335,12 @@ static int s390_machine_protect(S390CcwMachineState *ms)
>      Error *local_err = NULL;
>      int rc;
>

Maybe add a comment?

/*
 * Ballooning on protected VMs needs support in the guest for
 * sharing and unsharing balloon pages. Block ballooning for
 * now, until we have a solution to make at least Linux guests
 * either support it or fail gracefully.
 */
  
> +    qemu_balloon_inhibit(true);
>      error_setg(&pv_mig_blocker,
>                 "protected VMs are currently not migrateable.");
>      rc = migrate_add_blocker(pv_mig_blocker, &local_err);
>      if (local_err) {
> +        qemu_balloon_inhibit(false);
>          error_report_err(local_err);
>          error_free_or_abort(&pv_mig_blocker);
>          return rc;
> @@ -345,6 +349,7 @@ static int s390_machine_protect(S390CcwMachineState *ms)
>      /* Create SE VM */
>      rc = s390_pv_vm_enable();
>      if (rc) {
> +        qemu_balloon_inhibit(false);
>          error_report_err(local_err);
>          migrate_del_blocker(pv_mig_blocker);
>          error_free_or_abort(&pv_mig_blocker);
Janosch Frank March 18, 2020, 12:30 p.m. UTC | #3
On 3/18/20 12:42 PM, Cornelia Huck wrote:
> On Wed, 11 Mar 2020 09:21:40 -0400
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Ballooning in protected VMs can only be done when the guest shares the
>> pages it gives to the host. If pages are not shared, the integrity
>> checks will fail once those pages have been altered and are given back
>> to the guest.
>>
>> As we currently do not yet have a solution for this we will continue
>> like this:
>>
>> 1. We block ballooning now in QEMU (with this patch)
> 
> add trailing '.'
> 
>>
>> 2. Later we will provide a change to virtio that removes the blocker
> 
> s/Later/Later,/
> 
>> and adds VIRTIO_F_IOMMU_PLATFORM automatically by QEMU when doing the
>> protvirt switch. This is ok as the guest balloon driver will reject to
>> work with the IOMMU change
> 
> "This is OK, as the balloon driver in Linux (the only supported guest)
> will refuse to work with the IOMMU_PLATFORM feature bit set."
> 
> ?
> 
>>
>> 3. Later we can fix the guest balloon driver to accept the IOMMU
> 
> "Later, we can fix the Linux guest balloon driver..."
> 
>> feature bit and correctly exercise sharing and unsharing of balloon
>> pages
> 
> again, missing trailing '.'

All nits fixed

> 
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  hw/s390x/s390-virtio-ccw.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index deb31e060052d279..066e01f303c35671 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/balloon.h"
>>  #include "hw/s390x/pv.h"
>>  #include "migration/blocker.h"
>>  
>> @@ -326,6 +327,7 @@ static void s390_machine_unprotect(S390CcwMachineState *ms)
>>      ms->pv = false;
>>      migrate_del_blocker(pv_mig_blocker);
>>      error_free_or_abort(&pv_mig_blocker);
>> +    qemu_balloon_inhibit(false);
>>  }
>>  
>>  static int s390_machine_protect(S390CcwMachineState *ms)
>> @@ -333,10 +335,12 @@ static int s390_machine_protect(S390CcwMachineState *ms)
>>      Error *local_err = NULL;
>>      int rc;
>>
> 
> Maybe add a comment?
> 
> /*
>  * Ballooning on protected VMs needs support in the guest for
>  * sharing and unsharing balloon pages. Block ballooning for
>  * now, until we have a solution to make at least Linux guests
>  * either support it or fail gracefully.
>  */
>   
>> +    qemu_balloon_inhibit(true);
>>      error_setg(&pv_mig_blocker,
>>                 "protected VMs are currently not migrateable.");
>>      rc = migrate_add_blocker(pv_mig_blocker, &local_err);
>>      if (local_err) {
>> +        qemu_balloon_inhibit(false);
>>          error_report_err(local_err);
>>          error_free_or_abort(&pv_mig_blocker);
>>          return rc;
>> @@ -345,6 +349,7 @@ static int s390_machine_protect(S390CcwMachineState *ms)
>>      /* Create SE VM */
>>      rc = s390_pv_vm_enable();
>>      if (rc) {
>> +        qemu_balloon_inhibit(false);
>>          error_report_err(local_err);
>>          migrate_del_blocker(pv_mig_blocker);
>>          error_free_or_abort(&pv_mig_blocker);
>
diff mbox series

Patch

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index deb31e060052d279..066e01f303c35671 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/balloon.h"
 #include "hw/s390x/pv.h"
 #include "migration/blocker.h"
 
@@ -326,6 +327,7 @@  static void s390_machine_unprotect(S390CcwMachineState *ms)
     ms->pv = false;
     migrate_del_blocker(pv_mig_blocker);
     error_free_or_abort(&pv_mig_blocker);
+    qemu_balloon_inhibit(false);
 }
 
 static int s390_machine_protect(S390CcwMachineState *ms)
@@ -333,10 +335,12 @@  static int s390_machine_protect(S390CcwMachineState *ms)
     Error *local_err = NULL;
     int rc;
 
+    qemu_balloon_inhibit(true);
     error_setg(&pv_mig_blocker,
                "protected VMs are currently not migrateable.");
     rc = migrate_add_blocker(pv_mig_blocker, &local_err);
     if (local_err) {
+        qemu_balloon_inhibit(false);
         error_report_err(local_err);
         error_free_or_abort(&pv_mig_blocker);
         return rc;
@@ -345,6 +349,7 @@  static int s390_machine_protect(S390CcwMachineState *ms)
     /* Create SE VM */
     rc = s390_pv_vm_enable();
     if (rc) {
+        qemu_balloon_inhibit(false);
         error_report_err(local_err);
         migrate_del_blocker(pv_mig_blocker);
         error_free_or_abort(&pv_mig_blocker);