[PATCH v2 1/2] virtio: Make ARM SMMU workaround more specific

[PATCH v2 1/2] virtio: Make ARM SMMU workaround more specific

[Robin Murphy]

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-5wv7dgnIgG8@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] virtio: Document DMA coherency

[Robin Murphy]

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-5wv7dgnIgG8@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] virtio: Document DMA coherency

[Will Deacon]

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@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@arm.com>

Will

Re: [PATCH v2 2/2] virtio: Document DMA coherency

[Rob Herring]

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@arm.com>
> ---
>  Documentation/devicetree/bindings/virtio/mmio.txt | 11 +++++++++++
>  1 file changed, 11 insertions(+)

Acked-by: Rob Herring <robh@kernel.org>