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