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

[Keir Fraser <keirf@google.com>]

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.

> >> 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.

> >> +
> >>  \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.

> >> +
> >>  \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.

 Regards,
 Keir