All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Keir Fraser <keirf@google.com>
Cc: kernel-team@android.com, linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org
Subject: Re: [PATCH] virtio: Force DMA restricted devices through DMA API
Date: Tue, 19 Jul 2022 17:31:45 -0400	[thread overview]
Message-ID: <20220719172607-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <Yta6RowkzVbXaSt2@google.com>

On Tue, Jul 19, 2022 at 02:05:58PM +0000, Keir Fraser wrote:
> On Tue, Jul 19, 2022 at 07:56:09AM -0400, Michael S. Tsirkin wrote:
> > On Tue, Jul 19, 2022 at 10:02:56AM +0000, Keir Fraser wrote:
> > > If virtio devices are tagged for "restricted-dma-pool", then that
> > > pool should be used for virtio ring setup, via the DMA API.
> > > 
> > > In particular, this fixes virtio_balloon for ARM PKVM, where the usual
> > > workaround of setting VIRTIO_F_ACCESS_PLATFORM in the virtio device
> > > doesn't work because the virtio_balloon driver clears the flag. This
> > > seems a more robust fix than fiddling the flag again.
> > > 
> > > Signed-off-by: Keir Fraser <keirf@google.com>
> > 
> > 
> > So the reason balloon disables ACCESS_PLATFORM is simply
> > because it passes physical addresses to device and
> > expects device to be able to poke at them.
> > 
> > I worry about modifying DMA semantics yet again - it has as much of a
> > chance to break some legacy configs as it has to fix some.
> > 
> > 
> > And I don't really know much about restricted-dma-pool but
> > I'd like to understand why does it make sense to set it for
> > the balloon since it pokes at all and any system memory.
> 
> So this is set in the device tree by the host, telling it to bounce all DMA
> through a restricted memory window (basically swiotlb). The original reason
> is simply to isolate DMA, to the extent possible, on IOMMU-less systems.
> 
> However it is also useful for PKVM because the host is not trusted to access
> ordinary protected VM memory.

I'll have to read up on pKVM. Will get back to you.

> To allow I/O via the host, restricted-dma-pool
> is used to cause a bounce aperture to be allocated during VM boot, which is
> then explicitly shared with the host. For correct PKVM virtio operation, all
> data *and metadata* (virtio rings and descriptors) must be allocated in or
> bounced through this aperture.
>
> Insofar as virtio device accesses to virtio rings in guest memory essentially
> *are* DMA (from the pov of the guest), I think it makes sense to respect the
> bounce buffer for those rings, if so configured by the device tree.
>
> > > ---
> > >  drivers/virtio/virtio_ring.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > index a5ec724c01d8..12be2607c648 100644
> > > --- a/drivers/virtio/virtio_ring.c
> > > +++ b/drivers/virtio/virtio_ring.c
> > > @@ -12,6 +12,7 @@
> > >  #include <linux/hrtimer.h>
> > >  #include <linux/dma-mapping.h>
> > >  #include <linux/spinlock.h>
> > > +#include <linux/swiotlb.h>
> > >  #include <xen/xen.h>
> > >  
> > >  #ifdef DEBUG
> > > @@ -248,6 +249,13 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> > >  	if (!virtio_has_dma_quirk(vdev))
> > >  		return true;
> > >  
> > > +	/* If the device is configured to use a DMA restricted pool,
> > > +	 * we had better use it.
> > > +	 */
> > > +	if (IS_ENABLED(CONFIG_DMA_RESTRICTED_POOL) &&
> > > +	    is_swiotlb_for_alloc(vdev->dev.parent))
> > > +		return true;
> > > +
> > >  	/* Otherwise, we are left to guess. */
> > >  	/*
> > >  	 * In theory, it's possible to have a buggy QEMU-supposed
> > > -- 
> > > 2.37.0.170.g444d1eabd0-goog
> > 
> > 

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Keir Fraser <keirf@google.com>
Cc: 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
Date: Tue, 19 Jul 2022 17:31:45 -0400	[thread overview]
Message-ID: <20220719172607-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <Yta6RowkzVbXaSt2@google.com>

On Tue, Jul 19, 2022 at 02:05:58PM +0000, Keir Fraser wrote:
> On Tue, Jul 19, 2022 at 07:56:09AM -0400, Michael S. Tsirkin wrote:
> > On Tue, Jul 19, 2022 at 10:02:56AM +0000, Keir Fraser wrote:
> > > If virtio devices are tagged for "restricted-dma-pool", then that
> > > pool should be used for virtio ring setup, via the DMA API.
> > > 
> > > In particular, this fixes virtio_balloon for ARM PKVM, where the usual
> > > workaround of setting VIRTIO_F_ACCESS_PLATFORM in the virtio device
> > > doesn't work because the virtio_balloon driver clears the flag. This
> > > seems a more robust fix than fiddling the flag again.
> > > 
> > > Signed-off-by: Keir Fraser <keirf@google.com>
> > 
> > 
> > So the reason balloon disables ACCESS_PLATFORM is simply
> > because it passes physical addresses to device and
> > expects device to be able to poke at them.
> > 
> > I worry about modifying DMA semantics yet again - it has as much of a
> > chance to break some legacy configs as it has to fix some.
> > 
> > 
> > And I don't really know much about restricted-dma-pool but
> > I'd like to understand why does it make sense to set it for
> > the balloon since it pokes at all and any system memory.
> 
> So this is set in the device tree by the host, telling it to bounce all DMA
> through a restricted memory window (basically swiotlb). The original reason
> is simply to isolate DMA, to the extent possible, on IOMMU-less systems.
> 
> However it is also useful for PKVM because the host is not trusted to access
> ordinary protected VM memory.

I'll have to read up on pKVM. Will get back to you.

> To allow I/O via the host, restricted-dma-pool
> is used to cause a bounce aperture to be allocated during VM boot, which is
> then explicitly shared with the host. For correct PKVM virtio operation, all
> data *and metadata* (virtio rings and descriptors) must be allocated in or
> bounced through this aperture.
>
> Insofar as virtio device accesses to virtio rings in guest memory essentially
> *are* DMA (from the pov of the guest), I think it makes sense to respect the
> bounce buffer for those rings, if so configured by the device tree.
>
> > > ---
> > >  drivers/virtio/virtio_ring.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > index a5ec724c01d8..12be2607c648 100644
> > > --- a/drivers/virtio/virtio_ring.c
> > > +++ b/drivers/virtio/virtio_ring.c
> > > @@ -12,6 +12,7 @@
> > >  #include <linux/hrtimer.h>
> > >  #include <linux/dma-mapping.h>
> > >  #include <linux/spinlock.h>
> > > +#include <linux/swiotlb.h>
> > >  #include <xen/xen.h>
> > >  
> > >  #ifdef DEBUG
> > > @@ -248,6 +249,13 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> > >  	if (!virtio_has_dma_quirk(vdev))
> > >  		return true;
> > >  
> > > +	/* If the device is configured to use a DMA restricted pool,
> > > +	 * we had better use it.
> > > +	 */
> > > +	if (IS_ENABLED(CONFIG_DMA_RESTRICTED_POOL) &&
> > > +	    is_swiotlb_for_alloc(vdev->dev.parent))
> > > +		return true;
> > > +
> > >  	/* Otherwise, we are left to guess. */
> > >  	/*
> > >  	 * In theory, it's possible to have a buggy QEMU-supposed
> > > -- 
> > > 2.37.0.170.g444d1eabd0-goog
> > 
> > 


  reply	other threads:[~2022-07-19 21:31 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-19 10:02 [PATCH] virtio: Force DMA restricted devices through DMA API Keir Fraser
2022-07-19 11:56 ` Michael S. Tsirkin
2022-07-19 11:56   ` Michael S. Tsirkin
2022-07-19 14:05   ` Keir Fraser
2022-07-19 21:31     ` Michael S. Tsirkin [this message]
2022-07-19 21:31       ` Michael S. Tsirkin
2022-07-19 15:23 ` Christoph Hellwig
2022-07-19 15:23   ` Christoph Hellwig
2022-07-19 15:46   ` Keir Fraser
2022-07-19 15:51     ` Christoph Hellwig
2022-07-19 15:51       ` Christoph Hellwig
2022-07-19 16:11       ` Keir Fraser
2022-07-20  5:16         ` Christoph Hellwig
2022-07-20  5:16           ` Christoph Hellwig
2022-07-20  6:59         ` Michael S. Tsirkin
2022-07-20  6:59           ` Michael S. Tsirkin
2022-07-20  8:27           ` Keir Fraser
2022-07-20  9:58             ` Michael S. Tsirkin
2022-07-20  9:58               ` Michael S. Tsirkin
2022-07-21  7:37               ` Keir Fraser

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220719172607-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=keirf@google.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.