Robin Murphy
2017-Feb-02 16:36 UTC
[PATCH v2 1/2] virtio: Make ARM SMMU workaround more specific
Whilst always using the DMA API is OK on ARM systems in most cases,
there can be a problem if a hypervisor fails to tell its guest that a
virtio device is cache-coherent. In that case, the guest will end up
making non-cacheable mappings for DMA buffers (i.e. the vring), which,
if the host is using a cacheable view of the same buffer on the other
end, is not a recipe for success.
It turns out that current kvmtool, and probably QEMU as well, runs into
this exact problem, and a guest using a virtio console can be seen to
hang pretty quickly after writing a few characters as host data in cache
and guest data directly in RAM go out of sync.
In order to fix this, narrow the scope of the original workaround from
all legacy devices to just those behind IOMMUs, which was really the
only thing we were trying to deal with in the first place.
Fixes: c7070619f340 ("vring: Force use of DMA API for ARM-based systems
with legacy devices")
Signed-off-by: Robin Murphy <robin.murphy at arm.com>
---
drivers/virtio/virtio_ring.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 7e38ed79c3fc..03e824c77d61 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -20,6 +20,7 @@
#include <linux/virtio_ring.h>
#include <linux/virtio_config.h>
#include <linux/device.h>
+#include <linux/iommu.h>
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/hrtimer.h>
@@ -117,6 +118,27 @@ struct vring_virtqueue {
#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
/*
+ * ARM Fast Models are hopefully unique in implementing "hardware"
legacy
+ * virtio block devices, which can be placed behind a "real" IOMMU,
but are
+ * unaware of VIRTIO_F_IOMMU_PLATFORM. Fortunately, we can detect whether
+ * an IOMMU is present and in use by checking whether an IOMMU driver has
+ * assigned the DMA master device a group.
+ */
+static bool vring_arm_legacy_dma_quirk(struct virtio_device *vdev)
+{
+ struct iommu_group *group;
+
+ if (!(IS_ENABLED(CONFIG_ARM) || IS_ENABLED(CONFIG_ARM64)) ||
+ virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
+ return false;
+
+ group = iommu_group_get(vdev->dev.parent);
+ iommu_group_put(group);
+
+ return group != NULL;
+}
+
+/*
* Modern virtio devices have feature bits to specify whether they need a
* quirk and bypass the IOMMU. If not there, just use the DMA API.
*
@@ -159,12 +181,8 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
if (xen_domain())
return true;
- /*
- * On ARM-based machines, the DMA ops will do the right thing,
- * so always use them with legacy devices.
- */
- if (IS_ENABLED(CONFIG_ARM) || IS_ENABLED(CONFIG_ARM64))
- return !virtio_has_feature(vdev, VIRTIO_F_VERSION_1);
+ if (vring_arm_legacy_dma_quirk(vdev))
+ return true;
return false;
}
--
2.11.0.dirty
Since making use of the DMA API will require the architecture code to have the correct notion of device cache-coherency on architectures like ARM, explicitly call this out in the virtio-mmio DT binding. The ship has sailed for legacy virtio, but let's hope that we can head off any future firmware mishaps. Signed-off-by: Robin Murphy <robin.murphy at arm.com> --- Documentation/devicetree/bindings/virtio/mmio.txt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Documentation/devicetree/bindings/virtio/mmio.txt b/Documentation/devicetree/bindings/virtio/mmio.txt index 5069c1b8e193..999a93faa67c 100644 --- a/Documentation/devicetree/bindings/virtio/mmio.txt +++ b/Documentation/devicetree/bindings/virtio/mmio.txt @@ -7,6 +7,16 @@ Required properties: - compatible: "virtio,mmio" compatibility string - reg: control registers base address and size including configuration space - interrupts: interrupt generated by the device +- dma-coherent: required if the device (or host emulation) accesses memory + cache-coherently, absent otherwise + +Linux implementation note: + +virtio devices not advertising the VIRTIO_F_IOMMU_PLATFORM flag have been +implicitly assumed to be cache-coherent by Linux, and for legacy reasons this +behaviour is likely to remain. If VIRTIO_F_IOMMU_PLATFORM is advertised, then +such assumptions cannot be relied upon and the "dma-coherent" property must +accurately reflect the coherency of the device. Example: @@ -14,4 +24,5 @@ Example: compatible = "virtio,mmio"; reg = <0x3000 0x100>; interrupts = <41>; + dma-coherent; } -- 2.11.0.dirty
On Thu, Feb 02, 2017 at 04:36:21PM +0000, Robin Murphy wrote:> Since making use of the DMA API will require the architecture code to > have the correct notion of device cache-coherency on architectures like > ARM, explicitly call this out in the virtio-mmio DT binding. The ship > has sailed for legacy virtio, but let's hope that we can head off any > future firmware mishaps. > > Signed-off-by: Robin Murphy <robin.murphy at arm.com> > --- > Documentation/devicetree/bindings/virtio/mmio.txt | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/virtio/mmio.txt b/Documentation/devicetree/bindings/virtio/mmio.txt > index 5069c1b8e193..999a93faa67c 100644 > --- a/Documentation/devicetree/bindings/virtio/mmio.txt > +++ b/Documentation/devicetree/bindings/virtio/mmio.txt > @@ -7,6 +7,16 @@ Required properties: > - compatible: "virtio,mmio" compatibility string > - reg: control registers base address and size including configuration space > - interrupts: interrupt generated by the device > +- dma-coherent: required if the device (or host emulation) accesses memory > + cache-coherently, absent otherwise > + > +Linux implementation note: > + > +virtio devices not advertising the VIRTIO_F_IOMMU_PLATFORM flag have been > +implicitly assumed to be cache-coherent by Linux, and for legacy reasons this > +behaviour is likely to remain. If VIRTIO_F_IOMMU_PLATFORM is advertised, then > +such assumptions cannot be relied upon and the "dma-coherent" property must > +accurately reflect the coherency of the device. > > Example: > > @@ -14,4 +24,5 @@ Example: > compatible = "virtio,mmio"; > reg = <0x3000 0x100>; > interrupts = <41>; > + dma-coherent;I think this is a sensible update to the binding and is independent of whatever we decide to do for IOMMUs and DMA on legacy devices. Acked-by: Will Deacon <will.deacon at arm.com> Will
On Thu, Feb 02, 2017 at 04:36:21PM +0000, Robin Murphy wrote:> Since making use of the DMA API will require the architecture code to > have the correct notion of device cache-coherency on architectures like > ARM, explicitly call this out in the virtio-mmio DT binding. The ship > has sailed for legacy virtio, but let's hope that we can head off any > future firmware mishaps. > > Signed-off-by: Robin Murphy <robin.murphy at arm.com> > --- > Documentation/devicetree/bindings/virtio/mmio.txt | 11 +++++++++++ > 1 file changed, 11 insertions(+)Acked-by: Rob Herring <robh at kernel.org>