Re: [PATCH v5 07/18] s390x: protvirt: Inhibit balloon when switching to protected mode

[Christian Borntraeger]

On 26.02.20 19:24, Cornelia Huck wrote:
> On Wed, 26 Feb 2020 16:30:32 +0100
> Janosch Frank <address@hidden> wrote:
> 
>> On 2/26/20 4:16 PM, David Hildenbrand wrote:
>>> On 26.02.20 16:06, Christian Borntraeger wrote:  
>>>>
>>>>
>>>> On 26.02.20 15:59, David Hildenbrand wrote:  
>>>>> On 26.02.20 13:20, Janosch Frank wrote:  
>>>>>> Ballooning in protected VMs can only be done when the guest shares the
>>>>>> pages it gives to the host. Hence, until we have a solution for this
>>>>>> in the guest kernel, we inhibit ballooning when switching into
>>>>>> protected mode and reverse that once we move out of it.  
>>>>>
>>>>> I don't understand what you mean here, sorry. zapping a page will mean
>>>>> that a fresh one will be faulted in when accessed. And AFAIK, that means
>>>>> it will be encrypted again when needed.
>>>>>
>>>>> Is that more like the UV will detect this as an integrity issue and
>>>>> crash the VM?  
>>>>
>>>> yes, the UV will detect a fresh page as an integrity issue.
>>>> Only if the page was defined to be shared by the guest, we would avoid the
>>>> integrity check.
>>>>  
>>>
>>> Please make that clearer in the patch description. With that
>>>
>>> Reviewed-by: David Hildenbrand <address@hidden>
>>>   
>>
>> How about:
>> s390x: protvirt: Inhibit balloon when switching to protected mode
>>
>> 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.
> 
> This makes sense to me...
> 
>>
>> Hence, until we have a solution for this in the guest kernel, we
>> inhibit ballooning when switching into protected mode and reverse that
>> once we move out of it.
> 
> ...however, I'm scratching my head here.
> 
> If we have a future guest that knows how to handle this, how do we
> know? We know that the current Linux driver clears
> VIRTIO_F_IOMMU_PLATFORM during feature negotiation, and presumably a
> guest that knows how to handle this will not do that. But it still
> won't work as expected, as we inhibit ballooning...
> 
> So, either
> - we don't inhibit ballooning now; any guest that clears the (required)
>   virtio feature bit won't be able to drive the virtio-balloon device
>   anyway, but a future guest that negotiates the bit would work; or
> - we inhibit ballooning now; no guest can therefore use ballooning,
>   regardless what they are doing or not doing (this includes guests
>   that negotiate the feature bit, but fail to handle pages properly).
> 
> Or is there some other reason why we need to inhibit ballooning for
> protected vms?


So here is my proposal.
1. we block ballooning now in QEMU (take this patch now)
2. Later Halil 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
(see 
https://lore.kernel.org/qemu-devel/address@hidden/)
3. later we can fix the guest balloon driver to do the right thing for this
case (e.g. do the make shared call)