Re: [virtio-comment] Re: [PATCH] virtio-balloon: add an untrusted device feature

[Cornelia Huck <cohuck@redhat.com>]

On Thu, Aug 04 2022, Keir Fraser <keirf@google.com> wrote:

> On Wed, Aug 03, 2022 at 01:59:28PM +0200, Cornelia Huck wrote:
>> On Wed, Aug 03 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> 
>> > On Wed, Aug 03, 2022 at 08:44:41AM +0000, Keir Fraser wrote:
>> 
>> [hmm... I don't see the original mail on virtio-comment]
>
> Sorry about that. My email bounced even after I subscribed to the
> list. MST replied to my Cc. My list membership seems to have settled
> now.

Yes, seems to work now.

>
>> >> the device must offer the
>> >
>> > MUST
>> >
>> >> +VIRTIO_BALLOON_F_UNTRUSTED_DEVICE feature bit. If the driver does
>> >> +not accept this feature bit, the device MUST signal failure by failing
>> >> +to set FEATURES_OK \field{device status} bit when the driver writes
>> >> +it.
>> 
>> So, the bit is mandatory if offered?
>
> In ARM PKVM the host is untrusted, and so the device cannot regain
> ownership of memory pages. That has to be done via the TCB
> (Hypervisor). If the driver isn't aware of that, it is impossible for
> memory to be transferred back to the host, and ballooning cannot work.

Nod. I see that the Linux driver also explicitly clears the
ACCESS_PLATFORM feature bit; I'm wondering what the overlap between the
two features would be.

>
>> >> +
>> >>  \subparagraph{Legacy Interface: Feature bits}\label{sec:Device
>> >>  Types / Memory Balloon Device / Feature bits / Legacy Interface:
>> >>  Feature bits}
>> >> @@ -5573,6 +5587,9 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
>> >>  allow guest to use memory before notifying host if
>> >>  VIRTIO_BALLOON_F_MUST_TELL_HOST is not negotiated.
>> >>  
>> >> +The legacy interface cannot support VIRTIO_BALLOON_F_UNTRUSTED_DEVICE
>> >> +since there is no way to gracefully report feature negotiation failure.
>> 
>> UNTRUSTED_DEVICE requires VERSION_1, then?
>
> I think so. I don't see any benefit to supporting it on legacy
> interfaces.

This dependency should be made explicit then, as balloon also supports
legacy.

>
>> >> +
>> >>  \subsection{Device configuration layout}\label{sec:Device Types / Memory Balloon Device / Device configuration layout}
>> >>    \field{num_pages} and \field{actual} are always available.
>> >>  
>> >> @@ -5647,6 +5664,10 @@ \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / D
>> >>      pages. These addresses are divided by 4096\footnote{This is historical, and independent of the guest page size.
>> >>  } and the descriptor
>> >>      describing the resulting 32-bit array is added to the inflateq.
>> >> +  \item If the VIRTIO_BALLOON_F_UNTRUSTED_DEVICE feature has been
>> >> +  negotiated, the driver MUST relinquish memory ownership via the TCB
>> >> +  before adding it to the inflateq.
>> 
>> Do we need a definition of "relinquish memory ownership"?
>
> Maybe. Though I would say that the mechanism is platform specific and
> out of scope for virtio. But perhaps a short description of what this
> term means, in more detail, would make sense? And then tighten up
> usage of this term throughout the spec.

I guess other protected memory technologies have similar concepts; would
it make sense to define "memory ownership" as "exclusive access"?
I.e. the device cannot access the memory, and the driver has to give up
exclusive access before adding it to the inflateq.