Linux Virtualization - Apr 2016 - [PATCH 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

Signed-off-by: Michael S. Tsirkin <mst at redhat.com>

---
 include/hw/virtio/virtio-access.h              | 3 ++-
 include/hw/virtio/virtio.h                     | 6 +++++-
 include/standard-headers/linux/virtio_config.h | 8 ++++++++
 3 files changed, 15 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 ((vdev->host_features & (0x1ULL <<
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..34d3041 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -228,7 +228,11 @@ 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_passthrough", _state, _field, \
+                      VIRTIO_F_IOMMU_PASSTHROUGH, false), \
+    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..5564dab 100644
--- a/include/standard-headers/linux/virtio_config.h
+++ b/include/standard-headers/linux/virtio_config.h
@@ -61,4 +61,12 @@
 /* v1.0 compliant. */
 #define VIRTIO_F_VERSION_1		32
 
+/* Request IOMMU passthrough (if available)
+ * Without VIRTIO_F_IOMMU_PLATFORM: bypass the IOMMU even if enabled.
+ * With VIRTIO_F_IOMMU_PLATFORM: suggest disabling IOMMU.
+ */
+#define VIRTIO_F_IOMMU_PASSTHROUGH	33
+
+/* Do not bypass the IOMMU (if configured) */
+#define VIRTIO_F_IOMMU_PLATFORM		34
 #endif /* _LINUX_VIRTIO_CONFIG_H */
-- 
MST

On Mon, 2016-04-18 at 14:47 +0300, Michael S. Tsirkin
wrote:
> This adds a flag to enable/disable bypassing the IOMMU by
> virtio devices.
I'm still deeply unhappy with having this kind of hack in the virtio
code at all, as you know. Drivers should just use the DMA API and if
the *platform* wants to make it a no-op for a specific device, then it
can.

Remember, this isn't just virtio either. Don't we have *precisely* the
same issue with assigned PCI devices on a system with an emulated Intel
IOMMU? The assigned PCI devices aren't covered by the emulated IOMMU,
and the platform needs to know to bypass *those* too.

Now, we've had this conversation, and we accepted the hack in virtio
for now until the platforms (especially SPARC and Power IIRC) can get
their act together and make their DMA API implementations not broken.

But now you're adding this hack to the public API where we have to
support it for ever. Please, can't we avoid that?

-- 
dwmw2


-------------- 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/20160418/18eb0d20/attachment.bin>

On Mon, Apr 18, 2016 at 07:58:37AM -0400, David Woodhouse
wrote:
> On Mon, 2016-04-18 at 14:47 +0300, Michael S. Tsirkin wrote:
> > This adds a flag to enable/disable bypassing the IOMMU by
> > virtio devices.
> 
> I'm still deeply unhappy with having this kind of hack in the virtio
> code at all, as you know. Drivers should just use the DMA API and if
> the *platform* wants to make it a no-op for a specific device, then it
> can.
> 
> Remember, this isn't just virtio either. Don't we have *precisely*
the
> same issue with assigned PCI devices on a system with an emulated Intel
> IOMMU? The assigned PCI devices aren't covered by the emulated IOMMU,
> and the platform needs to know to bypass *those* too.
> 
> Now, we've had this conversation, and we accepted the hack in virtio
> for now until the platforms (especially SPARC and Power IIRC) can get
> their act together and make their DMA API implementations not broken.
> 
> But now you're adding this hack to the public API where we have to
> support it for ever. Please, can't we avoid that?
I'm not sure I understand the issue.  The public API is not about how
the driver works.  It doesn't say "don't use DMA API"
anywhere, does it?
It's about telling device whether to obey the IOMMU and
about discovering whether a device is in fact under the IOMMU.

Once DMA API allows bypassing IOMMU per device we'll be
able to drop the ugly hack from virtio drivers, simply keying it
off the given flag.

> -- 
> dwmw2
> 
>