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

[Janosch Frank]

On 3/18/20 12:42 PM, Cornelia Huck wrote:
> 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 '.'

All nits fixed

> 
>>
>> 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);
>

signature.asc
Description: OpenPGP digital signature