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

[Cornelia Huck]

On Wed, 11 Mar 2020 09:21:40 -0400
Janosch Frank <address@hidden> 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 <address@hidden>
> Reviewed-by: David Hildenbrand <address@hidden>
> Reviewed-by: Christian Borntraeger <address@hidden>
> ---
>  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);