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
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
[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
[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. > > StefanI hope I made it clear that there are no such users.
David Woodhouse
2016-Apr-27 12:18 UTC
[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>