Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 21 Jul 2022 07:37:38 +0000
From:   Keir Fraser <keirf@google.com>
To:     "Michael S. Tsirkin" <mst@redhat.com>
Cc:     Christoph Hellwig <hch@infradead.org>,
        Jason Wang <jasowang@redhat.com>, kernel-team@android.com,
        virtualization@lists.linux-foundation.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] virtio: Force DMA restricted devices through DMA API
Message-ID: <YtkCQsSvE9AZyrys@google.com>
References: <20220719100256.419780-1-keirf@google.com>
 <YtbMcBw4l0LAFn9+@infradead.org>
 <YtbRwO40CmIRWOUR@google.com>
 <YtbTGjRw65QEKkQA@infradead.org>
 <YtbXxl8STUtQkacb@google.com>
 <20220720024756-mutt-send-email-mst@kernel.org>
 <Yte8h0wtBZqiBky8@google.com>
 <20220720051351-mutt-send-email-mst@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220720051351-mutt-send-email-mst@kernel.org>
Precedence: bulk

On Wed, Jul 20, 2022 at 05:58:28AM -0400, Michael S. Tsirkin wrote:
> On Wed, Jul 20, 2022 at 08:27:51AM +0000, Keir Fraser wrote:
> > The short answer is that there will be more patches forthcoming,
> > because the balloon driver will need to tell the hypervisor (EL2 Hyp
> > in the ARM PKVM case) that is is willingly relinquishing memory
> > pages. So, there will be a patch to add a new HVC to PKVM Hyp, and a
> > patch to detect and use the new HVC via a new API that hooks into
> > Linux's balloon infrastructure.
> > 
> > So the use case is that virtio_balloon needs to set up its rings and
> > descriptors in a shared memory region, as requested via
> > dma-restricted-pool and the VIRTIO_F_PALTFORM_ACCESS flag. This is
> > required because the host device has no access to regular guest
> > memory.
> > 
> > However, in PKVM, page notifications will notify both the (trusted)
> > Hypervisor, via hypercall, and the (untrusted) VMM, via virtio. Guest
> > physical addresses are fine here. The VMM understands guest PAs
> > perfectly well, it's just not normally allowed to access their
> > contents or otherwise map or use those pages itself.
> 
> OK ... and I wonder whether this extends the balloon device
> in some way? Is there or can there be a new feature bit for this
> functionality?

To my mind it is implementation dependent whether the balloon device
needs to be extended. In my current implementation it is not, and
probably it will continue to be entirely handled host-side by the host
kernel and hypervisor. But we should consider the possibility of
requiring knowledge/extension in the device for sure.

Currently there is no feature flag for the new Hyp-notification path
on the driver side. The notification hypercall is hidden behind a new
API, and there is an init/probe call on that API by which the driver
unilaterally decides the extended path including Hyp notification is to
be used. One downside of this is that the device cannot detect a
legacy driver that lacks knowledge of the extended PKVM path. Any
pages returned by a legacy driver will simply crash the VMM, because
the hypervisor is still protecting those pages. A rather inelegant
failure mode!

I can envision a new feature flag that:

1. Is advertised by the device (makes sense: the VMM surely knows that
it is managing a protected VM).

2. Is Ack'ed by an aware driver, and which switches on the extended
notification path.

3. Is not negotiated by a legacy driver, causing the device to clear
FEATURES_OK, and the balloon is unavailable.

A balloon-specific flag called perhaps VIRTIO_F_BALLOON_UNTRUSTED_HOST,
or VIRTIO_F_BALLOON_NOTIFY_HYP, or somesuch? In some senses it's not a
balloon-specific piece of information, but it's only a pertinent feature
for balloon (at least as of now).

My understanding is that the first step in upstreaming such a new flag
would be to get it accepted into the virtio specification? If so and
this sounds agreeable, I'll rework my private patches, and cook up an
extension to the virtio spec. If an RFC posting of the patches here is
preferred before posting to the virtio-spec list, I can do that too.

> > Perhaps it would make more sense to re-submit this patch as part of
> > a larger series that includes the rest of the mechanism needed to
> > support virtio_balloon on PKVM?
> > 
> > Thanks,
> > Keir
> 
> I suspect so, yes.

Thanks for your review feedback. I will submit a full patch series in
due course.

Regards,
Keir