thr3ads.net - Linux Virtualization - [PATCH V2 RFC] fixup! virtio: convert to use DMA api [Apr 2016]

If this information is useful, please help other people find it:
Share via:

Michael S. Tsirkin

2016-Apr-21 13:43 UTC

[PATCH V2 RFC] fixup! virtio: convert to use DMA api

This adds a flag to enable/disable bypassing the IOMMU by
virtio devices.

This is on top of patch
http://article.gmane.org/gmane.comp.emulators.qemu/403467
virtio: convert to use DMA api

Tested with patchset
http://article.gmane.org/gmane.linux.kernel.virtualization/27545
virtio-pci: iommu support (note: bit number has been kept at 34
intentionally to match posted guest code. a non-RFC version will
renumber bits to be contigious).

changes from v1:
    drop PASSTHROUGH flag

The interaction between virtio and DMA API is messy.

On most systems with virtio, physical addresses match bus addresses,
and it doesn't particularly matter whether we use the DMA API.

On some systems, including Xen and any system with a physical device
that speaks virtio behind a physical IOMMU, we must use the DMA API
for virtio DMA to work at all.

Add a feature bit to detect that: VIRTIO_F_IOMMU_PLATFORM.

If not there, we preserve historic behavior and bypass the DMA
API unless within Xen guest. This is actually required for
systems, including SPARC and PPC64, where virtio-pci devices are
enumerated as though they are behind an IOMMU, but the virtio host
ignores the IOMMU, so we must either pretend that the IOMMU isn't
there or somehow map everything as the identity.

Re: non-virtio devices.

It turns out that on old QEMU hosts, only emulated devices which were
part of QEMU use the IOMMU.  Should we want to bypass the IOMMU for such
devices *only*, it would be rather easy to detect them by looking at
subsystem vendor and device ID. Thus, no new interfaces are required
except for virtio which always uses the same subsystem vendor and device ID.

Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
---
 include/hw/virtio/virtio-access.h              | 3 ++-
 include/hw/virtio/virtio.h                     | 4 +++-
 include/standard-headers/linux/virtio_config.h | 2 ++
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/hw/virtio/virtio-access.h
b/include/hw/virtio/virtio-access.h
index 967cc75..bb6f34e 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -23,7 +23,8 @@ static inline AddressSpace *virtio_get_dma_as(VirtIODevice
*vdev)
     BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 
-    if (k->get_dma_as) {
+    if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM) &&
+        k->get_dma_as) {
         return k->get_dma_as(qbus->parent);
     }
     return &address_space_memory;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b12faa9..44f3788 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -228,7 +228,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
     DEFINE_PROP_BIT64("notify_on_empty", _state, _field,  \
                       VIRTIO_F_NOTIFY_ON_EMPTY, true), \
     DEFINE_PROP_BIT64("any_layout", _state, _field, \
-                      VIRTIO_F_ANY_LAYOUT, true)
+                      VIRTIO_F_ANY_LAYOUT, true), \
+    DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
+                      VIRTIO_F_IOMMU_PLATFORM, false)
 
 hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
diff --git a/include/standard-headers/linux/virtio_config.h
b/include/standard-headers/linux/virtio_config.h
index bcc445b..3fcfbb1 100644
--- a/include/standard-headers/linux/virtio_config.h
+++ b/include/standard-headers/linux/virtio_config.h
@@ -61,4 +61,6 @@
 /* v1.0 compliant. */
 #define VIRTIO_F_VERSION_1		32
 
+/* Do not bypass the IOMMU (if configured) */
+#define VIRTIO_F_IOMMU_PLATFORM		34
 #endif /* _LINUX_VIRTIO_CONFIG_H */
-- 
MST

Wei Liu

2016-Apr-21 13:54 UTC

head link

[PATCH V2 RFC] fixup! virtio: convert to use DMA api

Add Stefano and Anthony

On Thu, Apr 21, 2016 at 04:43:45PM +0300, Michael S. Tsirkin
wrote:> This adds a flag to enable/disable bypassing the IOMMU by
> virtio devices.
> 
> This is on top of patch
> http://article.gmane.org/gmane.comp.emulators.qemu/403467
> virtio: convert to use DMA api
> 
> Tested with patchset
> http://article.gmane.org/gmane.linux.kernel.virtualization/27545
> virtio-pci: iommu support (note: bit number has been kept at 34
> intentionally to match posted guest code. a non-RFC version will
> renumber bits to be contigious).
> 
> changes from v1:
>     drop PASSTHROUGH flag
> 
> The interaction between virtio and DMA API is messy.
> 
> On most systems with virtio, physical addresses match bus addresses,
> and it doesn't particularly matter whether we use the DMA API.
> 
> On some systems, including Xen and any system with a physical device
> that speaks virtio behind a physical IOMMU, we must use the DMA API
> for virtio DMA to work at all.
> 
> Add a feature bit to detect that: VIRTIO_F_IOMMU_PLATFORM.
> 
> If not there, we preserve historic behavior and bypass the DMA
> API unless within Xen guest. This is actually required for
> systems, including SPARC and PPC64, where virtio-pci devices are
> enumerated as though they are behind an IOMMU, but the virtio host
> ignores the IOMMU, so we must either pretend that the IOMMU isn't
> there or somehow map everything as the identity.
> 
> Re: non-virtio devices.
> 
> It turns out that on old QEMU hosts, only emulated devices which were
> part of QEMU use the IOMMU.  Should we want to bypass the IOMMU for such
> devices *only*, it would be rather easy to detect them by looking at
> subsystem vendor and device ID. Thus, no new interfaces are required
> except for virtio which always uses the same subsystem vendor and device
ID.
> 
> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> ---
>  include/hw/virtio/virtio-access.h              | 3 ++-
>  include/hw/virtio/virtio.h                     | 4 +++-
>  include/standard-headers/linux/virtio_config.h | 2 ++
>  3 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio-access.h
b/include/hw/virtio/virtio-access.h
> index 967cc75..bb6f34e 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -23,7 +23,8 @@ static inline AddressSpace
*virtio_get_dma_as(VirtIODevice *vdev)
>      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>  
> -    if (k->get_dma_as) {
> +    if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM) &&
> +        k->get_dma_as) {
>          return k->get_dma_as(qbus->parent);
>      }
>      return &address_space_memory;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b12faa9..44f3788 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -228,7 +228,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
>      DEFINE_PROP_BIT64("notify_on_empty", _state, _field,  \
>                        VIRTIO_F_NOTIFY_ON_EMPTY, true), \
>      DEFINE_PROP_BIT64("any_layout", _state, _field, \
> -                      VIRTIO_F_ANY_LAYOUT, true)
> +                      VIRTIO_F_ANY_LAYOUT, true), \
> +    DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
> +                      VIRTIO_F_IOMMU_PLATFORM, false)
>  
>  hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
>  hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
> diff --git a/include/standard-headers/linux/virtio_config.h
b/include/standard-headers/linux/virtio_config.h
> index bcc445b..3fcfbb1 100644
> --- a/include/standard-headers/linux/virtio_config.h
> +++ b/include/standard-headers/linux/virtio_config.h
> @@ -61,4 +61,6 @@
>  /* v1.0 compliant. */
>  #define VIRTIO_F_VERSION_1		32
>  
> +/* Do not bypass the IOMMU (if configured) */
> +#define VIRTIO_F_IOMMU_PLATFORM		34
>  #endif /* _LINUX_VIRTIO_CONFIG_H */
> -- 
> MST

Stefan Hajnoczi

2016-Apr-21 14:56 UTC

head link

[PATCH V2 RFC] fixup! virtio: convert to use DMA api

On Thu, Apr 21, 2016 at 04:43:45PM +0300, Michael S. Tsirkin
wrote:> This adds a flag to enable/disable bypassing the IOMMU by
> virtio devices.
> 
> This is on top of patch
> http://article.gmane.org/gmane.comp.emulators.qemu/403467
> virtio: convert to use DMA api
> 
> Tested with patchset
> http://article.gmane.org/gmane.linux.kernel.virtualization/27545
> virtio-pci: iommu support (note: bit number has been kept at 34
> intentionally to match posted guest code. a non-RFC version will
> renumber bits to be contigious).
> 
> changes from v1:
>     drop PASSTHROUGH flag
> 
> The interaction between virtio and DMA API is messy.
> 
> On most systems with virtio, physical addresses match bus addresses,
> and it doesn't particularly matter whether we use the DMA API.
> 
> On some systems, including Xen and any system with a physical device
> that speaks virtio behind a physical IOMMU, we must use the DMA API
> for virtio DMA to work at all.
> 
> Add a feature bit to detect that: VIRTIO_F_IOMMU_PLATFORM.
> 
> If not there, we preserve historic behavior and bypass the DMA
> API unless within Xen guest. This is actually required for
> systems, including SPARC and PPC64, where virtio-pci devices are
> enumerated as though they are behind an IOMMU, but the virtio host
> ignores the IOMMU, so we must either pretend that the IOMMU isn't
> there or somehow map everything as the identity.
> 
> Re: non-virtio devices.
> 
> It turns out that on old QEMU hosts, only emulated devices which were
> part of QEMU use the IOMMU.  Should we want to bypass the IOMMU for such
> devices *only*, it would be rather easy to detect them by looking at
> subsystem vendor and device ID. Thus, no new interfaces are required
> except for virtio which always uses the same subsystem vendor and device
ID.
> 
> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> ---
>  include/hw/virtio/virtio-access.h              | 3 ++-
>  include/hw/virtio/virtio.h                     | 4 +++-
>  include/standard-headers/linux/virtio_config.h | 2 ++
>  3 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio-access.h
b/include/hw/virtio/virtio-access.h
> index 967cc75..bb6f34e 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -23,7 +23,8 @@ static inline AddressSpace
*virtio_get_dma_as(VirtIODevice *vdev)
>      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>  
> -    if (k->get_dma_as) {
> +    if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM) &&
> +        k->get_dma_as) {
>          return k->get_dma_as(qbus->parent);
>      }
>      return &address_space_memory;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b12faa9..44f3788 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -228,7 +228,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
>      DEFINE_PROP_BIT64("notify_on_empty", _state, _field,  \
>                        VIRTIO_F_NOTIFY_ON_EMPTY, true), \
>      DEFINE_PROP_BIT64("any_layout", _state, _field, \
> -                      VIRTIO_F_ANY_LAYOUT, true)
> +                      VIRTIO_F_ANY_LAYOUT, true), \
> +    DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
> +                      VIRTIO_F_IOMMU_PLATFORM, false)
Looks like the impact of this patch is that users who relied on
k->get_dma_as today may now have to explicitly add iommu_platform=on.
Are there any such users (e.g. Xen)?

Instead of breaking the command-line for these users you could invert
the flag's meaning ("iommu_bypass=on") and set it in the SPARC/PPC
machine types.

Stefan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20160421/7cf0bb19/attachment-0001.sig>

Michael S. Tsirkin

2016-Apr-21 15:11 UTC

head link

[PATCH V2 RFC] fixup! virtio: convert to use DMA api

On Thu, Apr 21, 2016 at 03:56:53PM +0100, Stefan Hajnoczi
wrote:> On Thu, Apr 21, 2016 at 04:43:45PM +0300, Michael S. Tsirkin wrote:
> > This adds a flag to enable/disable bypassing the IOMMU by
> > virtio devices.
> > 
> > This is on top of patch
> > http://article.gmane.org/gmane.comp.emulators.qemu/403467
> > virtio: convert to use DMA api
> > 
> > Tested with patchset
> > http://article.gmane.org/gmane.linux.kernel.virtualization/27545
> > virtio-pci: iommu support (note: bit number has been kept at 34
> > intentionally to match posted guest code. a non-RFC version will
> > renumber bits to be contigious).
> > 
> > changes from v1:
> >     drop PASSTHROUGH flag
> > 
> > The interaction between virtio and DMA API is messy.
> > 
> > On most systems with virtio, physical addresses match bus addresses,
> > and it doesn't particularly matter whether we use the DMA API.
> > 
> > On some systems, including Xen and any system with a physical device
> > that speaks virtio behind a physical IOMMU, we must use the DMA API
> > for virtio DMA to work at all.
> > 
> > Add a feature bit to detect that: VIRTIO_F_IOMMU_PLATFORM.
> > 
> > If not there, we preserve historic behavior and bypass the DMA
> > API unless within Xen guest. This is actually required for
> > systems, including SPARC and PPC64, where virtio-pci devices are
> > enumerated as though they are behind an IOMMU, but the virtio host
> > ignores the IOMMU, so we must either pretend that the IOMMU isn't
> > there or somehow map everything as the identity.
> > 
> > Re: non-virtio devices.
> > 
> > It turns out that on old QEMU hosts, only emulated devices which were
> > part of QEMU use the IOMMU.  Should we want to bypass the IOMMU for
such
> > devices *only*, it would be rather easy to detect them by looking at
> > subsystem vendor and device ID. Thus, no new interfaces are required
> > except for virtio which always uses the same subsystem vendor and
device ID.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> > ---
> >  include/hw/virtio/virtio-access.h              | 3 ++-
> >  include/hw/virtio/virtio.h                     | 4 +++-
> >  include/standard-headers/linux/virtio_config.h | 2 ++
> >  3 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/hw/virtio/virtio-access.h
b/include/hw/virtio/virtio-access.h
> > index 967cc75..bb6f34e 100644
> > --- a/include/hw/virtio/virtio-access.h
> > +++ b/include/hw/virtio/virtio-access.h
> > @@ -23,7 +23,8 @@ static inline AddressSpace
*virtio_get_dma_as(VirtIODevice *vdev)
> >      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> >      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> >  
> > -    if (k->get_dma_as) {
> > +    if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)
&&
> > +        k->get_dma_as) {
> >          return k->get_dma_as(qbus->parent);
> >      }
> >      return &address_space_memory;
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index b12faa9..44f3788 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -228,7 +228,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
> >      DEFINE_PROP_BIT64("notify_on_empty", _state, _field,  \
> >                        VIRTIO_F_NOTIFY_ON_EMPTY, true), \
> >      DEFINE_PROP_BIT64("any_layout", _state, _field, \
> > -                      VIRTIO_F_ANY_LAYOUT, true)
> > +                      VIRTIO_F_ANY_LAYOUT, true), \
> > +    DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
> > +                      VIRTIO_F_IOMMU_PLATFORM, false)
> 
> Looks like the impact of this patch is that users who relied on
> k->get_dma_as today may now have to explicitly add iommu_platform=on.
> Are there any such users (e.g. Xen)?
No because upstream this is ignored. This is an incremental patch
on top of Jason's one.
> Instead of breaking the command-line for these users you could invert
> the flag's meaning ("iommu_bypass=on") and set it in the
SPARC/PPC
> machine types.
> 
> Stefan
I hope I made it clear that there are no such users.

David Woodhouse

2016-Apr-27 12:18 UTC

head link

[PATCH V2 RFC] fixup! virtio: convert to use DMA api

> > On some systems, including Xen and any system with a physical device
> > that speaks virtio behind a physical IOMMU, we must use the DMA API
> > for virtio DMA to work at all.
> >?
> > Add a feature bit to detect that: VIRTIO_F_IOMMU_PLATFORM.
> >?
> > If not there, we preserve historic behavior and bypass the DMA
> > API unless within Xen guest. This is actually required for
> > systems, including SPARC and PPC64, where virtio-pci devices are
> > enumerated as though they are behind an IOMMU, but the virtio host
> > ignores the IOMMU, so we must either pretend that the IOMMU isn't
> > there or somehow map everything as the identity.
> >?
> > Re: non-virtio devices.
> >?
> > It turns out that on old QEMU hosts, only emulated devices which were
> > part of QEMU use the IOMMU.? Should we want to bypass the IOMMU for
such
> > devices *only*, it would be rather easy to detect them by looking at
> > subsystem vendor and device ID. Thus, no new interfaces are required
> > except for virtio which always uses the same subsystem vendor and
device ID.
Apologies for dropping this thread; I've been travelling.

But seriously, NO!

I understand why you want to see this as a virtio-specific issue, but
it isn't. And we don't *want* it to be.

In the guest, drivers SHALL use the DMA API. And the DMA API SHALL do
the right thing for each device according to its needs.

So any information passed from qemu to the guest should be directed at
the platform IOMMU code (or handled by qemu-detection quirks in the
guest, if we must).

It is *not* acceptable for the virtio drivers in the guest to just
eschew the DMA API completely, triggered by some device-specific flag.

The qemu implementation is, of course, monolithic. In qemu the fact
that virtio doesn't get translated by the emulated IOMMU *is* actually
down to code in the virtio implementation. I get that.

But then again, it's not just virtio. *Any* device which we emulate for
the guest could have that same issue, and appear as untranslated. (And
assigned PCI devices currently do).

Let's think about the parallel with a system-on-chip. Let's say we have
a peripheral which got included, but which was wired up such that it
bypasses the IOMMU and gets to do direct physical DMA. Is that a
feature of that specific peripheral? Do we hack its drivers to make the
distinction between this incarnation, and a normal discrete version of
the same device? No! It's a feature of the *system* and needs to be
conveyed to the OS IOMMU code to do the right thing. Not to the driver.

In my opinion, adding the VIRTIO_F_IOMMU_PLATFORM feature bit is
absolutely the wrong thing to do.

What we *should* do is a patchset in the guest which both fixes virtio
drivers to *always* use the DMA API, and fixes the DMA API to DTRT at
the same time ? by detecting qemu and installing no-op DMA ops for the
appropriate devices, perhaps.

Then we can look at giving qemu a way to properly indicate which
devices it actually does DMA mapping for, so we can remove those
heuristic assumptions.

But that flag does *not* live in the virtio host??guest ABI.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse at intel.com                              Intel Corporation

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5691 bytes
Desc: not available
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20160427/8c13228b/attachment.bin>

Possibly Parallel Threads

Search for more possibly parallel threads

Linux Virtualization - Apr 2016 - [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[PATCH V2 RFC] fixup! virtio: convert to use DMA api

[PATCH V2 RFC] fixup! virtio: convert to use DMA api

[PATCH V2 RFC] fixup! virtio: convert to use DMA api

[PATCH V2 RFC] fixup! virtio: convert to use DMA api

[PATCH V2 RFC] fixup! virtio: convert to use DMA api

Possibly Parallel Threads