[PATCH v2 1/2] virtio_mmio: Set DMA masks appropriately

[PATCH v2 1/2] virtio_mmio: Set DMA masks appropriately

[Robin Murphy]

Once DMA API usage is enabled, it becomes apparent that virtio-mmio is
inadvertently relying on the default 32-bit DMA mask, which leads to
problems like rapidly exhausting SWIOTLB bounce buffers.

Ensure that we set the appropriate 64-bit DMA mask whenever possible,
with the coherent mask suitably limited for the legacy vring as per
a0be1db4304f ("virtio_pci: Limit DMA mask to 44 bits for legacy virtio
devices").

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Michael S. Tsirkin <mst@redhat.com>
Reported-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Fixes: b42111382f0e ("virtio_mmio: Use the DMA API if enabled")
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/virtio/virtio_mmio.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index d47a2fcef818..c71fde5fe835 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -59,6 +59,7 @@
 #define pr_fmt(fmt) "virtio-mmio: " fmt
 
 #include <linux/acpi.h>
+#include <linux/dma-mapping.h>
 #include <linux/highmem.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -498,6 +499,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
 	struct virtio_mmio_device *vm_dev;
 	struct resource *mem;
 	unsigned long magic;
+	int rc;
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!mem)
@@ -547,9 +549,25 @@ static int virtio_mmio_probe(struct platform_device *pdev)
 	}
 	vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID);
 
-	if (vm_dev->version == 1)
+	if (vm_dev->version == 1) {
 		writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
 
+		rc = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
+		/*
+		 * In the legacy case, ensure our coherently-allocated virtio
+		 * ring will be at an address expressable as a 32-bit PFN.
+		 */
+		if (!rc)
+			dma_set_coherent_mask(&pdev->dev,
+					      DMA_BIT_MASK(32 + PAGE_SHIFT));
+	} else {
+		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+	}
+	if (rc)
+		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (rc)
+		dev_warn(&pdev->dev, "Failed to enable 64-bit or 32-bit DMA.  Trying to continue, but this might not work.\n");
+
 	platform_set_drvdata(pdev, vm_dev);
 
 	return register_virtio_device(&vm_dev->vdev);
-- 
2.10.2.dirty

[PATCH v2 1/2] virtio_mmio: Set DMA masks appropriately

[Robin Murphy]

Once DMA API usage is enabled, it becomes apparent that virtio-mmio is
inadvertently relying on the default 32-bit DMA mask, which leads to
problems like rapidly exhausting SWIOTLB bounce buffers.

Ensure that we set the appropriate 64-bit DMA mask whenever possible,
with the coherent mask suitably limited for the legacy vring as per
a0be1db4304f ("virtio_pci: Limit DMA mask to 44 bits for legacy virtio
devices").

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Michael S. Tsirkin <mst@redhat.com>
Reported-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Fixes: b42111382f0e ("virtio_mmio: Use the DMA API if enabled")
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/virtio/virtio_mmio.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index d47a2fcef818..c71fde5fe835 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -59,6 +59,7 @@
 #define pr_fmt(fmt) "virtio-mmio: " fmt
 
 #include <linux/acpi.h>
+#include <linux/dma-mapping.h>
 #include <linux/highmem.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -498,6 +499,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
 	struct virtio_mmio_device *vm_dev;
 	struct resource *mem;
 	unsigned long magic;
+	int rc;
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!mem)
@@ -547,9 +549,25 @@ static int virtio_mmio_probe(struct platform_device *pdev)
 	}
 	vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID);
 
-	if (vm_dev->version == 1)
+	if (vm_dev->version == 1) {
 		writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
 
+		rc = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
+		/*
+		 * In the legacy case, ensure our coherently-allocated virtio
+		 * ring will be at an address expressable as a 32-bit PFN.
+		 */
+		if (!rc)
+			dma_set_coherent_mask(&pdev->dev,
+					      DMA_BIT_MASK(32 + PAGE_SHIFT));
+	} else {
+		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+	}
+	if (rc)
+		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (rc)
+		dev_warn(&pdev->dev, "Failed to enable 64-bit or 32-bit DMA.  Trying to continue, but this might not work.\n");
+
 	platform_set_drvdata(pdev, vm_dev);
 
 	return register_virtio_device(&vm_dev->vdev);
-- 
2.10.2.dirty

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Robin Murphy]

From: Will Deacon <will.deacon@arm.com>

Booting Linux on an ARM fastmodel containing an SMMU emulation results
in an unexpected I/O page fault from the legacy virtio-blk PCI device:

[    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
[    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
[    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
[    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
[    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
[    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
[    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
[    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
[    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
[    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000

<system hangs failing to read partition table>

This is because the virtio-blk is behind an SMMU, so we have consequently
swizzled its DMA ops and configured the SMMU to translate accesses. This
then requires the vring code to use the DMA API to establish translations,
otherwise all transactions will result in fatal faults and termination.

Given that ARM-based systems only see an SMMU if one is really present
(the topology is all described by firmware tables such as device-tree or
IORT), then we can safely use the DMA API for all virtio devices.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 drivers/virtio/virtio_ring.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 409aeaa49246..447245f2c813 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -159,6 +159,10 @@ 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 */
+	if (IS_ENABLED(CONFIG_ARM) || IS_ENABLED(CONFIG_ARM64))
+		return true;
+
 	return false;
 }
 
-- 
2.10.2.dirty

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Robin Murphy]

From: Will Deacon <will.deacon@arm.com>

Booting Linux on an ARM fastmodel containing an SMMU emulation results
in an unexpected I/O page fault from the legacy virtio-blk PCI device:

[    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
[    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
[    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
[    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
[    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
[    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
[    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
[    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
[    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
[    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000

<system hangs failing to read partition table>

This is because the virtio-blk is behind an SMMU, so we have consequently
swizzled its DMA ops and configured the SMMU to translate accesses. This
then requires the vring code to use the DMA API to establish translations,
otherwise all transactions will result in fatal faults and termination.

Given that ARM-based systems only see an SMMU if one is really present
(the topology is all described by firmware tables such as device-tree or
IORT), then we can safely use the DMA API for all virtio devices.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 drivers/virtio/virtio_ring.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 409aeaa49246..447245f2c813 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -159,6 +159,10 @@ 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 */
+	if (IS_ENABLED(CONFIG_ARM) || IS_ENABLED(CONFIG_ARM64))
+		return true;
+
 	return false;
 }
 
-- 
2.10.2.dirty

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> From: Will Deacon <will.deacon@arm.com>
> 
> Booting Linux on an ARM fastmodel containing an SMMU emulation results
> in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> 
> [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> 
> <system hangs failing to read partition table>
> 
> This is because the virtio-blk is behind an SMMU, so we have consequently
> swizzled its DMA ops and configured the SMMU to translate accesses. This
> then requires the vring code to use the DMA API to establish translations,
> otherwise all transactions will result in fatal faults and termination.
> 
> Given that ARM-based systems only see an SMMU if one is really present
> (the topology is all described by firmware tables such as device-tree or
> IORT), then we can safely use the DMA API for all virtio devices.
> 
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

I'd like to better understand then need for this one.
Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?

I'd rather we avoided need for more hacks and just
have everyone switch to that.


> ---
>  drivers/virtio/virtio_ring.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 409aeaa49246..447245f2c813 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -159,6 +159,10 @@ 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 */
> +	if (IS_ENABLED(CONFIG_ARM) || IS_ENABLED(CONFIG_ARM64))
> +		return true;
> +
>  	return false;
>  }
>  
> -- 
> 2.10.2.dirty

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> From: Will Deacon <will.deacon@arm.com>
> 
> Booting Linux on an ARM fastmodel containing an SMMU emulation results
> in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> 
> [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> 
> <system hangs failing to read partition table>
> 
> This is because the virtio-blk is behind an SMMU, so we have consequently
> swizzled its DMA ops and configured the SMMU to translate accesses. This
> then requires the vring code to use the DMA API to establish translations,
> otherwise all transactions will result in fatal faults and termination.
> 
> Given that ARM-based systems only see an SMMU if one is really present
> (the topology is all described by firmware tables such as device-tree or
> IORT), then we can safely use the DMA API for all virtio devices.
> 
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

I'd like to better understand then need for this one.
Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?

I'd rather we avoided need for more hacks and just
have everyone switch to that.


> ---
>  drivers/virtio/virtio_ring.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 409aeaa49246..447245f2c813 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -159,6 +159,10 @@ 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 */
> +	if (IS_ENABLED(CONFIG_ARM) || IS_ENABLED(CONFIG_ARM64))
> +		return true;
> +
>  	return false;
>  }
>  
> -- 
> 2.10.2.dirty

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> > From: Will Deacon <will.deacon@arm.com>
> > 
> > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> > 
> > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> > [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> > [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > 
> > <system hangs failing to read partition table>
> > 
> > This is because the virtio-blk is behind an SMMU, so we have consequently
> > swizzled its DMA ops and configured the SMMU to translate accesses. This
> > then requires the vring code to use the DMA API to establish translations,
> > otherwise all transactions will result in fatal faults and termination.
> > 
> > Given that ARM-based systems only see an SMMU if one is really present
> > (the topology is all described by firmware tables such as device-tree or
> > IORT), then we can safely use the DMA API for all virtio devices.
> > 
> > Cc: Andy Lutomirski <luto@kernel.org>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> 
> I'd like to better understand then need for this one.
> Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> 
> I'd rather we avoided need for more hacks and just
> have everyone switch to that.

There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:

1. It doesn't exist for legacy devices, which are all we have on the
   platform in question.

2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
   my patch ;). Thanks.

In which case, for non-legacy devices we should definitely be using
VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
world of flying cars, could we unconditionally set the DMA ops on ARM
for legacy devices? The alternative is disabling the SMMU altogether,
but that's less than ideal because there are non-virtio devices on the
same PCI bus.

Will

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> > From: Will Deacon <will.deacon@arm.com>
> > 
> > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> > 
> > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> > [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> > [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > 
> > <system hangs failing to read partition table>
> > 
> > This is because the virtio-blk is behind an SMMU, so we have consequently
> > swizzled its DMA ops and configured the SMMU to translate accesses. This
> > then requires the vring code to use the DMA API to establish translations,
> > otherwise all transactions will result in fatal faults and termination.
> > 
> > Given that ARM-based systems only see an SMMU if one is really present
> > (the topology is all described by firmware tables such as device-tree or
> > IORT), then we can safely use the DMA API for all virtio devices.
> > 
> > Cc: Andy Lutomirski <luto@kernel.org>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> 
> I'd like to better understand then need for this one.
> Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> 
> I'd rather we avoided need for more hacks and just
> have everyone switch to that.

There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:

1. It doesn't exist for legacy devices, which are all we have on the
   platform in question.

2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
   my patch ;). Thanks.

In which case, for non-legacy devices we should definitely be using
VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
world of flying cars, could we unconditionally set the DMA ops on ARM
for legacy devices? The alternative is disabling the SMMU altogether,
but that's less than ideal because there are non-virtio devices on the
same PCI bus.

Will

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Andy Lutomirski]

On Wed, Jan 11, 2017 at 2:01 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
>> On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
>> > From: Will Deacon <will.deacon@arm.com>
>> >
>> > Booting Linux on an ARM fastmodel containing an SMMU emulation results
>> > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
>> >
>> > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
>> > [    1.211800] arm-smmu-v3 2b400000.smmu:   0x00000000fffff010
>> > [    1.211880] arm-smmu-v3 2b400000.smmu:   0x0000020800000000
>> > [    1.211959] arm-smmu-v3 2b400000.smmu:   0x00000008fa081002
>> > [    1.212075] arm-smmu-v3 2b400000.smmu:   0x0000000000000000
>> > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
>> > [    1.212234] arm-smmu-v3 2b400000.smmu:   0x00000000fffff010
>> > [    1.212314] arm-smmu-v3 2b400000.smmu:   0x0000020800000000
>> > [    1.212394] arm-smmu-v3 2b400000.smmu:   0x00000008fa081000
>> > [    1.212471] arm-smmu-v3 2b400000.smmu:   0x0000000000000000
>> >
>> > <system hangs failing to read partition table>
>> >
>> > This is because the virtio-blk is behind an SMMU, so we have consequently
>> > swizzled its DMA ops and configured the SMMU to translate accesses. This
>> > then requires the vring code to use the DMA API to establish translations,
>> > otherwise all transactions will result in fatal faults and termination.
>> >
>> > Given that ARM-based systems only see an SMMU if one is really present
>> > (the topology is all described by firmware tables such as device-tree or
>> > IORT), then we can safely use the DMA API for all virtio devices.
>> >
>> > Cc: Andy Lutomirski <luto@kernel.org>
>> > Cc: Michael S. Tsirkin <mst@redhat.com>
>> > Signed-off-by: Will Deacon <will.deacon@arm.com>
>>
>> I'd like to better understand then need for this one.
>> Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
>>
>> I'd rather we avoided need for more hacks and just
>> have everyone switch to that.
>
> There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
>
> 1. It doesn't exist for legacy devices, which are all we have on the
>    platform in question.
>
> 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
>    my patch ;). Thanks.
>
> In which case, for non-legacy devices we should definitely be using
> VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> world of flying cars, could we unconditionally set the DMA ops on ARM
> for legacy devices? The alternative is disabling the SMMU altogether,
> but that's less than ideal because there are non-virtio devices on the
> same PCI bus.

Also, on ARM, using the DMA API appears to *always* be the correct
approach.  Why not do it all the time, then?  The non-DMA-API path is
a legacy thing that is needed because a few platforms incorrectly
enumerate their IOMMUs.  ARM gets it right, so I don't see why ARM
should be subject to the legacy mess.

Even on x86, it should be possible to get the code into a state where
using DMA ops is always correct.

--Andy

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Andy Lutomirski]

On Wed, Jan 11, 2017 at 2:01 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
>> On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
>> > From: Will Deacon <will.deacon@arm.com>
>> >
>> > Booting Linux on an ARM fastmodel containing an SMMU emulation results
>> > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
>> >
>> > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
>> > [    1.211800] arm-smmu-v3 2b400000.smmu:   0x00000000fffff010
>> > [    1.211880] arm-smmu-v3 2b400000.smmu:   0x0000020800000000
>> > [    1.211959] arm-smmu-v3 2b400000.smmu:   0x00000008fa081002
>> > [    1.212075] arm-smmu-v3 2b400000.smmu:   0x0000000000000000
>> > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
>> > [    1.212234] arm-smmu-v3 2b400000.smmu:   0x00000000fffff010
>> > [    1.212314] arm-smmu-v3 2b400000.smmu:   0x0000020800000000
>> > [    1.212394] arm-smmu-v3 2b400000.smmu:   0x00000008fa081000
>> > [    1.212471] arm-smmu-v3 2b400000.smmu:   0x0000000000000000
>> >
>> > <system hangs failing to read partition table>
>> >
>> > This is because the virtio-blk is behind an SMMU, so we have consequently
>> > swizzled its DMA ops and configured the SMMU to translate accesses. This
>> > then requires the vring code to use the DMA API to establish translations,
>> > otherwise all transactions will result in fatal faults and termination.
>> >
>> > Given that ARM-based systems only see an SMMU if one is really present
>> > (the topology is all described by firmware tables such as device-tree or
>> > IORT), then we can safely use the DMA API for all virtio devices.
>> >
>> > Cc: Andy Lutomirski <luto@kernel.org>
>> > Cc: Michael S. Tsirkin <mst@redhat.com>
>> > Signed-off-by: Will Deacon <will.deacon@arm.com>
>>
>> I'd like to better understand then need for this one.
>> Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
>>
>> I'd rather we avoided need for more hacks and just
>> have everyone switch to that.
>
> There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
>
> 1. It doesn't exist for legacy devices, which are all we have on the
>    platform in question.
>
> 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
>    my patch ;). Thanks.
>
> In which case, for non-legacy devices we should definitely be using
> VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> world of flying cars, could we unconditionally set the DMA ops on ARM
> for legacy devices? The alternative is disabling the SMMU altogether,
> but that's less than ideal because there are non-virtio devices on the
> same PCI bus.

Also, on ARM, using the DMA API appears to *always* be the correct
approach.  Why not do it all the time, then?  The non-DMA-API path is
a legacy thing that is needed because a few platforms incorrectly
enumerate their IOMMUs.  ARM gets it right, so I don't see why ARM
should be subject to the legacy mess.

Even on x86, it should be possible to get the code into a state where
using DMA ops is always correct.

--Andy

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Wed, Jan 11, 2017 at 10:12:36AM -0800, Andy Lutomirski wrote:
> On Wed, Jan 11, 2017 at 2:01 AM, Will Deacon <will.deacon@arm.com> wrote:
> > On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> >> On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> >> > From: Will Deacon <will.deacon@arm.com>
> >> >
> >> > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> >> > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> >> >
> >> > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> >> > [    1.211800] arm-smmu-v3 2b400000.smmu:   0x00000000fffff010
> >> > [    1.211880] arm-smmu-v3 2b400000.smmu:   0x0000020800000000
> >> > [    1.211959] arm-smmu-v3 2b400000.smmu:   0x00000008fa081002
> >> > [    1.212075] arm-smmu-v3 2b400000.smmu:   0x0000000000000000
> >> > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> >> > [    1.212234] arm-smmu-v3 2b400000.smmu:   0x00000000fffff010
> >> > [    1.212314] arm-smmu-v3 2b400000.smmu:   0x0000020800000000
> >> > [    1.212394] arm-smmu-v3 2b400000.smmu:   0x00000008fa081000
> >> > [    1.212471] arm-smmu-v3 2b400000.smmu:   0x0000000000000000
> >> >
> >> > <system hangs failing to read partition table>
> >> >
> >> > This is because the virtio-blk is behind an SMMU, so we have consequently
> >> > swizzled its DMA ops and configured the SMMU to translate accesses. This
> >> > then requires the vring code to use the DMA API to establish translations,
> >> > otherwise all transactions will result in fatal faults and termination.
> >> >
> >> > Given that ARM-based systems only see an SMMU if one is really present
> >> > (the topology is all described by firmware tables such as device-tree or
> >> > IORT), then we can safely use the DMA API for all virtio devices.
> >> >
> >> > Cc: Andy Lutomirski <luto@kernel.org>
> >> > Cc: Michael S. Tsirkin <mst@redhat.com>
> >> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> >>
> >> I'd like to better understand then need for this one.
> >> Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> >>
> >> I'd rather we avoided need for more hacks and just
> >> have everyone switch to that.
> >
> > There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
> >
> > 1. It doesn't exist for legacy devices, which are all we have on the
> >    platform in question.
> >
> > 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
> >    my patch ;). Thanks.
> >
> > In which case, for non-legacy devices we should definitely be using
> > VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> > world of flying cars, could we unconditionally set the DMA ops on ARM
> > for legacy devices? The alternative is disabling the SMMU altogether,
> > but that's less than ideal because there are non-virtio devices on the
> > same PCI bus.
> 
> Also, on ARM, using the DMA API appears to *always* be the correct
> approach.  Why not do it all the time, then?  The non-DMA-API path is
> a legacy thing that is needed because a few platforms incorrectly
> enumerate their IOMMUs.  ARM gets it right, so I don't see why ARM
> should be subject to the legacy mess.

That's what my patch does, but since modern virtio has the
VIRTIO_F_IOMMU_PLATFORM flag (and the spec says if it's not present then
the driver must pass physical addresses), it feels to me like we should
just honour that so that different architectures all have the same
behaviour. For legacy devices, the horse has bolted and we need
arch-specific behaviours to keep things working.

Will

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Wed, Jan 11, 2017 at 10:12:36AM -0800, Andy Lutomirski wrote:
> On Wed, Jan 11, 2017 at 2:01 AM, Will Deacon <will.deacon@arm.com> wrote:
> > On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> >> On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> >> > From: Will Deacon <will.deacon@arm.com>
> >> >
> >> > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> >> > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> >> >
> >> > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> >> > [    1.211800] arm-smmu-v3 2b400000.smmu:   0x00000000fffff010
> >> > [    1.211880] arm-smmu-v3 2b400000.smmu:   0x0000020800000000
> >> > [    1.211959] arm-smmu-v3 2b400000.smmu:   0x00000008fa081002
> >> > [    1.212075] arm-smmu-v3 2b400000.smmu:   0x0000000000000000
> >> > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> >> > [    1.212234] arm-smmu-v3 2b400000.smmu:   0x00000000fffff010
> >> > [    1.212314] arm-smmu-v3 2b400000.smmu:   0x0000020800000000
> >> > [    1.212394] arm-smmu-v3 2b400000.smmu:   0x00000008fa081000
> >> > [    1.212471] arm-smmu-v3 2b400000.smmu:   0x0000000000000000
> >> >
> >> > <system hangs failing to read partition table>
> >> >
> >> > This is because the virtio-blk is behind an SMMU, so we have consequently
> >> > swizzled its DMA ops and configured the SMMU to translate accesses. This
> >> > then requires the vring code to use the DMA API to establish translations,
> >> > otherwise all transactions will result in fatal faults and termination.
> >> >
> >> > Given that ARM-based systems only see an SMMU if one is really present
> >> > (the topology is all described by firmware tables such as device-tree or
> >> > IORT), then we can safely use the DMA API for all virtio devices.
> >> >
> >> > Cc: Andy Lutomirski <luto@kernel.org>
> >> > Cc: Michael S. Tsirkin <mst@redhat.com>
> >> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> >>
> >> I'd like to better understand then need for this one.
> >> Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> >>
> >> I'd rather we avoided need for more hacks and just
> >> have everyone switch to that.
> >
> > There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
> >
> > 1. It doesn't exist for legacy devices, which are all we have on the
> >    platform in question.
> >
> > 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
> >    my patch ;). Thanks.
> >
> > In which case, for non-legacy devices we should definitely be using
> > VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> > world of flying cars, could we unconditionally set the DMA ops on ARM
> > for legacy devices? The alternative is disabling the SMMU altogether,
> > but that's less than ideal because there are non-virtio devices on the
> > same PCI bus.
> 
> Also, on ARM, using the DMA API appears to *always* be the correct
> approach.  Why not do it all the time, then?  The non-DMA-API path is
> a legacy thing that is needed because a few platforms incorrectly
> enumerate their IOMMUs.  ARM gets it right, so I don't see why ARM
> should be subject to the legacy mess.

That's what my patch does, but since modern virtio has the
VIRTIO_F_IOMMU_PLATFORM flag (and the spec says if it's not present then
the driver must pass physical addresses), it feels to me like we should
just honour that so that different architectures all have the same
behaviour. For legacy devices, the horse has bolted and we need
arch-specific behaviours to keep things working.

Will

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Wed, Jan 11, 2017 at 10:12:36AM -0800, Andy Lutomirski wrote:
> On Wed, Jan 11, 2017 at 2:01 AM, Will Deacon <will.deacon@arm.com> wrote:
> > On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> >> On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> >> > From: Will Deacon <will.deacon@arm.com>
> >> >
> >> > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> >> > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> >> >
> >> > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> >> > [    1.211800] arm-smmu-v3 2b400000.smmu:   0x00000000fffff010
> >> > [    1.211880] arm-smmu-v3 2b400000.smmu:   0x0000020800000000
> >> > [    1.211959] arm-smmu-v3 2b400000.smmu:   0x00000008fa081002
> >> > [    1.212075] arm-smmu-v3 2b400000.smmu:   0x0000000000000000
> >> > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> >> > [    1.212234] arm-smmu-v3 2b400000.smmu:   0x00000000fffff010
> >> > [    1.212314] arm-smmu-v3 2b400000.smmu:   0x0000020800000000
> >> > [    1.212394] arm-smmu-v3 2b400000.smmu:   0x00000008fa081000
> >> > [    1.212471] arm-smmu-v3 2b400000.smmu:   0x0000000000000000
> >> >
> >> > <system hangs failing to read partition table>
> >> >
> >> > This is because the virtio-blk is behind an SMMU, so we have consequently
> >> > swizzled its DMA ops and configured the SMMU to translate accesses. This
> >> > then requires the vring code to use the DMA API to establish translations,
> >> > otherwise all transactions will result in fatal faults and termination.
> >> >
> >> > Given that ARM-based systems only see an SMMU if one is really present
> >> > (the topology is all described by firmware tables such as device-tree or
> >> > IORT), then we can safely use the DMA API for all virtio devices.
> >> >
> >> > Cc: Andy Lutomirski <luto@kernel.org>
> >> > Cc: Michael S. Tsirkin <mst@redhat.com>
> >> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> >>
> >> I'd like to better understand then need for this one.
> >> Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> >>
> >> I'd rather we avoided need for more hacks and just
> >> have everyone switch to that.
> >
> > There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
> >
> > 1. It doesn't exist for legacy devices, which are all we have on the
> >    platform in question.
> >
> > 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
> >    my patch ;). Thanks.
> >
> > In which case, for non-legacy devices we should definitely be using
> > VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> > world of flying cars, could we unconditionally set the DMA ops on ARM
> > for legacy devices? The alternative is disabling the SMMU altogether,
> > but that's less than ideal because there are non-virtio devices on the
> > same PCI bus.
> 
> Also, on ARM, using the DMA API appears to *always* be the correct
> approach.  Why not do it all the time, then?  The non-DMA-API path is
> a legacy thing that is needed because a few platforms incorrectly
> enumerate their IOMMUs.  ARM gets it right, so I don't see why ARM
> should be subject to the legacy mess.

I didn't realize ARM gets this right. QEMU still pokes at
physical addresses directly in legacy mode so I wonder how could
that be the case. I'll try to find out.

> Even on x86, it should be possible to get the code into a state where
> using DMA ops is always correct.
> 
> --Andy

This I could totally get behind. A driver would install some per device
flag to make it figure out IOMMU does not apply, and in a portable way
since at least x86 and PPC need this, maybe more. This would/should
also handle the bug that admin can bind vfio to legacy virtio
devices even without the noiommu mode.

-- 
MST

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Wed, Jan 11, 2017 at 10:12:36AM -0800, Andy Lutomirski wrote:
> On Wed, Jan 11, 2017 at 2:01 AM, Will Deacon <will.deacon@arm.com> wrote:
> > On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> >> On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> >> > From: Will Deacon <will.deacon@arm.com>
> >> >
> >> > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> >> > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> >> >
> >> > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> >> > [    1.211800] arm-smmu-v3 2b400000.smmu:   0x00000000fffff010
> >> > [    1.211880] arm-smmu-v3 2b400000.smmu:   0x0000020800000000
> >> > [    1.211959] arm-smmu-v3 2b400000.smmu:   0x00000008fa081002
> >> > [    1.212075] arm-smmu-v3 2b400000.smmu:   0x0000000000000000
> >> > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> >> > [    1.212234] arm-smmu-v3 2b400000.smmu:   0x00000000fffff010
> >> > [    1.212314] arm-smmu-v3 2b400000.smmu:   0x0000020800000000
> >> > [    1.212394] arm-smmu-v3 2b400000.smmu:   0x00000008fa081000
> >> > [    1.212471] arm-smmu-v3 2b400000.smmu:   0x0000000000000000
> >> >
> >> > <system hangs failing to read partition table>
> >> >
> >> > This is because the virtio-blk is behind an SMMU, so we have consequently
> >> > swizzled its DMA ops and configured the SMMU to translate accesses. This
> >> > then requires the vring code to use the DMA API to establish translations,
> >> > otherwise all transactions will result in fatal faults and termination.
> >> >
> >> > Given that ARM-based systems only see an SMMU if one is really present
> >> > (the topology is all described by firmware tables such as device-tree or
> >> > IORT), then we can safely use the DMA API for all virtio devices.
> >> >
> >> > Cc: Andy Lutomirski <luto@kernel.org>
> >> > Cc: Michael S. Tsirkin <mst@redhat.com>
> >> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> >>
> >> I'd like to better understand then need for this one.
> >> Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> >>
> >> I'd rather we avoided need for more hacks and just
> >> have everyone switch to that.
> >
> > There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
> >
> > 1. It doesn't exist for legacy devices, which are all we have on the
> >    platform in question.
> >
> > 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
> >    my patch ;). Thanks.
> >
> > In which case, for non-legacy devices we should definitely be using
> > VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> > world of flying cars, could we unconditionally set the DMA ops on ARM
> > for legacy devices? The alternative is disabling the SMMU altogether,
> > but that's less than ideal because there are non-virtio devices on the
> > same PCI bus.
> 
> Also, on ARM, using the DMA API appears to *always* be the correct
> approach.  Why not do it all the time, then?  The non-DMA-API path is
> a legacy thing that is needed because a few platforms incorrectly
> enumerate their IOMMUs.  ARM gets it right, so I don't see why ARM
> should be subject to the legacy mess.

I didn't realize ARM gets this right. QEMU still pokes at
physical addresses directly in legacy mode so I wonder how could
that be the case. I'll try to find out.

> Even on x86, it should be possible to get the code into a state where
> using DMA ops is always correct.
> 
> --Andy

This I could totally get behind. A driver would install some per device
flag to make it figure out IOMMU does not apply, and in a portable way
since at least x86 and PPC need this, maybe more. This would/should
also handle the bug that admin can bind vfio to legacy virtio
devices even without the noiommu mode.

-- 
MST

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Wed, Jan 11, 2017 at 10:01:39AM +0000, Will Deacon wrote:
> On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> > On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> > > From: Will Deacon <will.deacon@arm.com>
> > > 
> > > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> > > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> > > 
> > > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> > > [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> > > [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > 
> > > <system hangs failing to read partition table>
> > > 
> > > This is because the virtio-blk is behind an SMMU, so we have consequently
> > > swizzled its DMA ops and configured the SMMU to translate accesses. This
> > > then requires the vring code to use the DMA API to establish translations,
> > > otherwise all transactions will result in fatal faults and termination.
> > > 
> > > Given that ARM-based systems only see an SMMU if one is really present
> > > (the topology is all described by firmware tables such as device-tree or
> > > IORT), then we can safely use the DMA API for all virtio devices.
> > > 
> > > Cc: Andy Lutomirski <luto@kernel.org>
> > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > 
> > I'd like to better understand then need for this one.
> > Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> > 
> > I'd rather we avoided need for more hacks and just
> > have everyone switch to that.
> 
> There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
> 
> 1. It doesn't exist for legacy devices, which are all we have on the
>    platform in question.
> 
> 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
>    my patch ;). Thanks.
> 
> In which case, for non-legacy devices we should definitely be using
> VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> world of flying cars, could we unconditionally set the DMA ops on ARM
> for legacy devices? The alternative is disabling the SMMU altogether,
> but that's less than ideal because there are non-virtio devices on the
> same PCI bus.
> 
> Will

I'd rather people didn't use SMMU with legacy devices.
Can't you guys just code up the virtio 1 layout in QEMU?
I took a look and it's not a big deal now that two other
transports converted, except mmio code in QEMU doesn't
use linux header to it's a bit messy.
I'll send a patch to clean that up.

-- 
MST

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Wed, Jan 11, 2017 at 10:01:39AM +0000, Will Deacon wrote:
> On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> > On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> > > From: Will Deacon <will.deacon@arm.com>
> > > 
> > > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> > > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> > > 
> > > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> > > [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> > > [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > 
> > > <system hangs failing to read partition table>
> > > 
> > > This is because the virtio-blk is behind an SMMU, so we have consequently
> > > swizzled its DMA ops and configured the SMMU to translate accesses. This
> > > then requires the vring code to use the DMA API to establish translations,
> > > otherwise all transactions will result in fatal faults and termination.
> > > 
> > > Given that ARM-based systems only see an SMMU if one is really present
> > > (the topology is all described by firmware tables such as device-tree or
> > > IORT), then we can safely use the DMA API for all virtio devices.
> > > 
> > > Cc: Andy Lutomirski <luto@kernel.org>
> > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > 
> > I'd like to better understand then need for this one.
> > Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> > 
> > I'd rather we avoided need for more hacks and just
> > have everyone switch to that.
> 
> There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
> 
> 1. It doesn't exist for legacy devices, which are all we have on the
>    platform in question.
> 
> 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
>    my patch ;). Thanks.
> 
> In which case, for non-legacy devices we should definitely be using
> VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> world of flying cars, could we unconditionally set the DMA ops on ARM
> for legacy devices? The alternative is disabling the SMMU altogether,
> but that's less than ideal because there are non-virtio devices on the
> same PCI bus.
> 
> Will

I'd rather people didn't use SMMU with legacy devices.
Can't you guys just code up the virtio 1 layout in QEMU?
I took a look and it's not a big deal now that two other
transports converted, except mmio code in QEMU doesn't
use linux header to it's a bit messy.
I'll send a patch to clean that up.

-- 
MST

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> On Wed, Jan 11, 2017 at 10:01:39AM +0000, Will Deacon wrote:
> > On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> > > On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> > > > From: Will Deacon <will.deacon@arm.com>
> > > > 
> > > > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> > > > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> > > > 
> > > > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> > > > [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> > > > [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > 
> > > > <system hangs failing to read partition table>
> > > > 
> > > > This is because the virtio-blk is behind an SMMU, so we have consequently
> > > > swizzled its DMA ops and configured the SMMU to translate accesses. This
> > > > then requires the vring code to use the DMA API to establish translations,
> > > > otherwise all transactions will result in fatal faults and termination.
> > > > 
> > > > Given that ARM-based systems only see an SMMU if one is really present
> > > > (the topology is all described by firmware tables such as device-tree or
> > > > IORT), then we can safely use the DMA API for all virtio devices.
> > > > 
> > > > Cc: Andy Lutomirski <luto@kernel.org>
> > > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > 
> > > I'd like to better understand then need for this one.
> > > Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> > > 
> > > I'd rather we avoided need for more hacks and just
> > > have everyone switch to that.
> > 
> > There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
> > 
> > 1. It doesn't exist for legacy devices, which are all we have on the
> >    platform in question.
> > 
> > 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
> >    my patch ;). Thanks.
> > 
> > In which case, for non-legacy devices we should definitely be using
> > VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> > world of flying cars, could we unconditionally set the DMA ops on ARM
> > for legacy devices? The alternative is disabling the SMMU altogether,
> > but that's less than ideal because there are non-virtio devices on the
> > same PCI bus.
> > 
> 
> I'd rather people didn't use SMMU with legacy devices.

I'm afraid we've been doing that for two years and the model already
exists in a mature state, being actively used for development and
validation by ARM and our partners. One of the big things its used for
is to develop SMMU and GIC (our interrupt controller) code with PCI, so
dropping the SMMU from the picture isn't an option.

> Can't you guys just code up the virtio 1 layout in QEMU?
> I took a look and it's not a big deal now that two other
> transports converted, except mmio code in QEMU doesn't
> use linux header to it's a bit messy.
> I'll send a patch to clean that up.

If the model ever changes in this area (which isn't planned atm), the
right thing to do would be to move to modern virtio. However, we're stuck
with what we have for the forseeable future and it works just fine if we
use the DMA API. If we don't use it, Linux no longer boots because it
installs the SMMU-backed DMA ops for the virtio devices and everything
faults.

I really don't understand why this is controversial.

Will

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> On Wed, Jan 11, 2017 at 10:01:39AM +0000, Will Deacon wrote:
> > On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> > > On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> > > > From: Will Deacon <will.deacon@arm.com>
> > > > 
> > > > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> > > > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> > > > 
> > > > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> > > > [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> > > > [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > 
> > > > <system hangs failing to read partition table>
> > > > 
> > > > This is because the virtio-blk is behind an SMMU, so we have consequently
> > > > swizzled its DMA ops and configured the SMMU to translate accesses. This
> > > > then requires the vring code to use the DMA API to establish translations,
> > > > otherwise all transactions will result in fatal faults and termination.
> > > > 
> > > > Given that ARM-based systems only see an SMMU if one is really present
> > > > (the topology is all described by firmware tables such as device-tree or
> > > > IORT), then we can safely use the DMA API for all virtio devices.
> > > > 
> > > > Cc: Andy Lutomirski <luto@kernel.org>
> > > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > 
> > > I'd like to better understand then need for this one.
> > > Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> > > 
> > > I'd rather we avoided need for more hacks and just
> > > have everyone switch to that.
> > 
> > There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
> > 
> > 1. It doesn't exist for legacy devices, which are all we have on the
> >    platform in question.
> > 
> > 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
> >    my patch ;). Thanks.
> > 
> > In which case, for non-legacy devices we should definitely be using
> > VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> > world of flying cars, could we unconditionally set the DMA ops on ARM
> > for legacy devices? The alternative is disabling the SMMU altogether,
> > but that's less than ideal because there are non-virtio devices on the
> > same PCI bus.
> > 
> 
> I'd rather people didn't use SMMU with legacy devices.

I'm afraid we've been doing that for two years and the model already
exists in a mature state, being actively used for development and
validation by ARM and our partners. One of the big things its used for
is to develop SMMU and GIC (our interrupt controller) code with PCI, so
dropping the SMMU from the picture isn't an option.

> Can't you guys just code up the virtio 1 layout in QEMU?
> I took a look and it's not a big deal now that two other
> transports converted, except mmio code in QEMU doesn't
> use linux header to it's a bit messy.
> I'll send a patch to clean that up.

If the model ever changes in this area (which isn't planned atm), the
right thing to do would be to move to modern virtio. However, we're stuck
with what we have for the forseeable future and it works just fine if we
use the DMA API. If we don't use it, Linux no longer boots because it
installs the SMMU-backed DMA ops for the virtio devices and everything
faults.

I really don't understand why this is controversial.

Will

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > On Wed, Jan 11, 2017 at 10:01:39AM +0000, Will Deacon wrote:
> > > On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> > > > On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> > > > > From: Will Deacon <will.deacon@arm.com>
> > > > > 
> > > > > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> > > > > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> > > > > 
> > > > > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > > [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > > [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > > [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> > > > > [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > > [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > > [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > > [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> > > > > [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > > 
> > > > > <system hangs failing to read partition table>
> > > > > 
> > > > > This is because the virtio-blk is behind an SMMU, so we have consequently
> > > > > swizzled its DMA ops and configured the SMMU to translate accesses. This
> > > > > then requires the vring code to use the DMA API to establish translations,
> > > > > otherwise all transactions will result in fatal faults and termination.
> > > > > 
> > > > > Given that ARM-based systems only see an SMMU if one is really present
> > > > > (the topology is all described by firmware tables such as device-tree or
> > > > > IORT), then we can safely use the DMA API for all virtio devices.
> > > > > 
> > > > > Cc: Andy Lutomirski <luto@kernel.org>
> > > > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > > 
> > > > I'd like to better understand then need for this one.
> > > > Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> > > > 
> > > > I'd rather we avoided need for more hacks and just
> > > > have everyone switch to that.
> > > 
> > > There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
> > > 
> > > 1. It doesn't exist for legacy devices, which are all we have on the
> > >    platform in question.
> > > 
> > > 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
> > >    my patch ;). Thanks.
> > > 
> > > In which case, for non-legacy devices we should definitely be using
> > > VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> > > world of flying cars, could we unconditionally set the DMA ops on ARM
> > > for legacy devices? The alternative is disabling the SMMU altogether,
> > > but that's less than ideal because there are non-virtio devices on the
> > > same PCI bus.
> > > 
> > 
> > I'd rather people didn't use SMMU with legacy devices.
> 
> I'm afraid we've been doing that for two years and the model already
> exists in a mature state, being actively used for development and
> validation by ARM and our partners. One of the big things its used for
> is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> dropping the SMMU from the picture isn't an option.

Oh so this fixes a regression?  This is something I didn't realize.
A "Fixes:" tag can't hurt here.  I then wonder
might DMA ops ever use a DMA address which isn't a physical address
from QEMU point of view? If that happens, this hack breaks
because in legacy mode QEMU still uses the GPA.

> > Can't you guys just code up the virtio 1 layout in QEMU?
> > I took a look and it's not a big deal now that two other
> > transports converted, except mmio code in QEMU doesn't
> > use linux header to it's a bit messy.
> > I'll send a patch to clean that up.
> 
> If the model ever changes in this area (which isn't planned atm), the
> right thing to do would be to move to modern virtio. However, we're stuck
> with what we have for the forseeable future and it works just fine if we
> use the DMA API. If we don't use it, Linux no longer boots because it
> installs the SMMU-backed DMA ops for the virtio devices and everything
> faults.
> 
> I really don't understand why this is controversial.
> 
> Will

I agree we need to keep working things working. I just don't yet
understand what change broke things and how did it happen - legacy
devices used to just poke at guest PA from QEMU, what does it matter
that there are changes in DMA ops if neither host nor guest use them for
legacy setups?

I guess that's just me now knowing about how DMA ops work on SMMU
or missing some other basic point about SMMU.

I take your word that it's the right thing to do, but I'd like to figure
it out before merging this.  I'll read up in the coming days to make
sure I understand what the patch does, any pointers welcome.

-- 
MST

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > On Wed, Jan 11, 2017 at 10:01:39AM +0000, Will Deacon wrote:
> > > On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> > > > On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> > > > > From: Will Deacon <will.deacon@arm.com>
> > > > > 
> > > > > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> > > > > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> > > > > 
> > > > > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > > [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > > [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > > [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> > > > > [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > > [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > > [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > > [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> > > > > [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > > 
> > > > > <system hangs failing to read partition table>
> > > > > 
> > > > > This is because the virtio-blk is behind an SMMU, so we have consequently
> > > > > swizzled its DMA ops and configured the SMMU to translate accesses. This
> > > > > then requires the vring code to use the DMA API to establish translations,
> > > > > otherwise all transactions will result in fatal faults and termination.
> > > > > 
> > > > > Given that ARM-based systems only see an SMMU if one is really present
> > > > > (the topology is all described by firmware tables such as device-tree or
> > > > > IORT), then we can safely use the DMA API for all virtio devices.
> > > > > 
> > > > > Cc: Andy Lutomirski <luto@kernel.org>
> > > > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > > 
> > > > I'd like to better understand then need for this one.
> > > > Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> > > > 
> > > > I'd rather we avoided need for more hacks and just
> > > > have everyone switch to that.
> > > 
> > > There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
> > > 
> > > 1. It doesn't exist for legacy devices, which are all we have on the
> > >    platform in question.
> > > 
> > > 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
> > >    my patch ;). Thanks.
> > > 
> > > In which case, for non-legacy devices we should definitely be using
> > > VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> > > world of flying cars, could we unconditionally set the DMA ops on ARM
> > > for legacy devices? The alternative is disabling the SMMU altogether,
> > > but that's less than ideal because there are non-virtio devices on the
> > > same PCI bus.
> > > 
> > 
> > I'd rather people didn't use SMMU with legacy devices.
> 
> I'm afraid we've been doing that for two years and the model already
> exists in a mature state, being actively used for development and
> validation by ARM and our partners. One of the big things its used for
> is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> dropping the SMMU from the picture isn't an option.

Oh so this fixes a regression?  This is something I didn't realize.
A "Fixes:" tag can't hurt here.  I then wonder
might DMA ops ever use a DMA address which isn't a physical address
from QEMU point of view? If that happens, this hack breaks
because in legacy mode QEMU still uses the GPA.

> > Can't you guys just code up the virtio 1 layout in QEMU?
> > I took a look and it's not a big deal now that two other
> > transports converted, except mmio code in QEMU doesn't
> > use linux header to it's a bit messy.
> > I'll send a patch to clean that up.
> 
> If the model ever changes in this area (which isn't planned atm), the
> right thing to do would be to move to modern virtio. However, we're stuck
> with what we have for the forseeable future and it works just fine if we
> use the DMA API. If we don't use it, Linux no longer boots because it
> installs the SMMU-backed DMA ops for the virtio devices and everything
> faults.
> 
> I really don't understand why this is controversial.
> 
> Will

I agree we need to keep working things working. I just don't yet
understand what change broke things and how did it happen - legacy
devices used to just poke at guest PA from QEMU, what does it matter
that there are changes in DMA ops if neither host nor guest use them for
legacy setups?

I guess that's just me now knowing about how DMA ops work on SMMU
or missing some other basic point about SMMU.

I take your word that it's the right thing to do, but I'd like to figure
it out before merging this.  I'll read up in the coming days to make
sure I understand what the patch does, any pointers welcome.

-- 
MST

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > On Wed, Jan 11, 2017 at 10:01:39AM +0000, Will Deacon wrote:
> > > > On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> > > > > On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> > > > > > From: Will Deacon <will.deacon@arm.com>
> > > > > > 
> > > > > > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> > > > > > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> > > > > > 
> > > > > > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > > > [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > > > [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > > > [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> > > > > > [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > > > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > > > [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > > > [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > > > [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> > > > > > [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > > > 
> > > > > > <system hangs failing to read partition table>
> > > > > > 
> > > > > > This is because the virtio-blk is behind an SMMU, so we have consequently
> > > > > > swizzled its DMA ops and configured the SMMU to translate accesses. This
> > > > > > then requires the vring code to use the DMA API to establish translations,
> > > > > > otherwise all transactions will result in fatal faults and termination.
> > > > > > 
> > > > > > Given that ARM-based systems only see an SMMU if one is really present
> > > > > > (the topology is all described by firmware tables such as device-tree or
> > > > > > IORT), then we can safely use the DMA API for all virtio devices.
> > > > > > 
> > > > > > Cc: Andy Lutomirski <luto@kernel.org>
> > > > > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > > > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > > > 
> > > > > I'd like to better understand then need for this one.
> > > > > Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> > > > > 
> > > > > I'd rather we avoided need for more hacks and just
> > > > > have everyone switch to that.
> > > > 
> > > > There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
> > > > 
> > > > 1. It doesn't exist for legacy devices, which are all we have on the
> > > >    platform in question.
> > > > 
> > > > 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
> > > >    my patch ;). Thanks.
> > > > 
> > > > In which case, for non-legacy devices we should definitely be using
> > > > VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> > > > world of flying cars, could we unconditionally set the DMA ops on ARM
> > > > for legacy devices? The alternative is disabling the SMMU altogether,
> > > > but that's less than ideal because there are non-virtio devices on the
> > > > same PCI bus.
> > > > 
> > > 
> > > I'd rather people didn't use SMMU with legacy devices.
> > 
> > I'm afraid we've been doing that for two years and the model already
> > exists in a mature state, being actively used for development and
> > validation by ARM and our partners. One of the big things its used for
> > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > dropping the SMMU from the picture isn't an option.
> 
> Oh so this fixes a regression?  This is something I didn't realize.

Yes, thanks. The regression came about because we implemented SMMU-backed
DMA ops and only then was it apparent that the virtio stuff was bypassing
even with translation enabled (because it wasn't using the DMA API).

> A "Fixes:" tag can't hurt here.  I then wonder
> might DMA ops ever use a DMA address which isn't a physical address
> from QEMU point of view? If that happens, this hack breaks
> because in legacy mode QEMU still uses the GPA.

If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
because we won't swizzle the DMA ops for the master device. If QEMU does
advertise an SMMU, then we'll allocate DMA addresses to fit within the
the intersection of the SMMU aperture and device's DMA mask.

> > > Can't you guys just code up the virtio 1 layout in QEMU?
> > > I took a look and it's not a big deal now that two other
> > > transports converted, except mmio code in QEMU doesn't
> > > use linux header to it's a bit messy.
> > > I'll send a patch to clean that up.
> > 
> > If the model ever changes in this area (which isn't planned atm), the
> > right thing to do would be to move to modern virtio. However, we're stuck
> > with what we have for the forseeable future and it works just fine if we
> > use the DMA API. If we don't use it, Linux no longer boots because it
> > installs the SMMU-backed DMA ops for the virtio devices and everything
> > faults.
> > 
> > I really don't understand why this is controversial.
> > 
> I agree we need to keep working things working. I just don't yet
> understand what change broke things and how did it happen - legacy
> devices used to just poke at guest PA from QEMU, what does it matter
> that there are changes in DMA ops if neither host nor guest use them for
> legacy setups?

The problem is that platforms which advertise an SMMU downstream of the
(virtual) PCI-RC now automatically have their PCI devices attached to the
SMMU, so if they don't use the DMA ops then they will fault.

> I guess that's just me now knowing about how DMA ops work on SMMU
> or missing some other basic point about SMMU.
> 
> I take your word that it's the right thing to do, but I'd like to figure
> it out before merging this.  I'll read up in the coming days to make
> sure I understand what the patch does, any pointers welcome.

Ok, thanks. Just shout if you have more questions. I'm also happy with
only doing this for legacy devices, given that modern virtio has the
IOMMU flag.

Will

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > On Wed, Jan 11, 2017 at 10:01:39AM +0000, Will Deacon wrote:
> > > > On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> > > > > On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> > > > > > From: Will Deacon <will.deacon@arm.com>
> > > > > > 
> > > > > > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> > > > > > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> > > > > > 
> > > > > > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > > > [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > > > [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > > > [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> > > > > > [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > > > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > > > [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > > > [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > > > [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> > > > > > [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > > > 
> > > > > > <system hangs failing to read partition table>
> > > > > > 
> > > > > > This is because the virtio-blk is behind an SMMU, so we have consequently
> > > > > > swizzled its DMA ops and configured the SMMU to translate accesses. This
> > > > > > then requires the vring code to use the DMA API to establish translations,
> > > > > > otherwise all transactions will result in fatal faults and termination.
> > > > > > 
> > > > > > Given that ARM-based systems only see an SMMU if one is really present
> > > > > > (the topology is all described by firmware tables such as device-tree or
> > > > > > IORT), then we can safely use the DMA API for all virtio devices.
> > > > > > 
> > > > > > Cc: Andy Lutomirski <luto@kernel.org>
> > > > > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > > > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > > > 
> > > > > I'd like to better understand then need for this one.
> > > > > Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> > > > > 
> > > > > I'd rather we avoided need for more hacks and just
> > > > > have everyone switch to that.
> > > > 
> > > > There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
> > > > 
> > > > 1. It doesn't exist for legacy devices, which are all we have on the
> > > >    platform in question.
> > > > 
> > > > 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
> > > >    my patch ;). Thanks.
> > > > 
> > > > In which case, for non-legacy devices we should definitely be using
> > > > VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> > > > world of flying cars, could we unconditionally set the DMA ops on ARM
> > > > for legacy devices? The alternative is disabling the SMMU altogether,
> > > > but that's less than ideal because there are non-virtio devices on the
> > > > same PCI bus.
> > > > 
> > > 
> > > I'd rather people didn't use SMMU with legacy devices.
> > 
> > I'm afraid we've been doing that for two years and the model already
> > exists in a mature state, being actively used for development and
> > validation by ARM and our partners. One of the big things its used for
> > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > dropping the SMMU from the picture isn't an option.
> 
> Oh so this fixes a regression?  This is something I didn't realize.

Yes, thanks. The regression came about because we implemented SMMU-backed
DMA ops and only then was it apparent that the virtio stuff was bypassing
even with translation enabled (because it wasn't using the DMA API).

> A "Fixes:" tag can't hurt here.  I then wonder
> might DMA ops ever use a DMA address which isn't a physical address
> from QEMU point of view? If that happens, this hack breaks
> because in legacy mode QEMU still uses the GPA.

If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
because we won't swizzle the DMA ops for the master device. If QEMU does
advertise an SMMU, then we'll allocate DMA addresses to fit within the
the intersection of the SMMU aperture and device's DMA mask.

> > > Can't you guys just code up the virtio 1 layout in QEMU?
> > > I took a look and it's not a big deal now that two other
> > > transports converted, except mmio code in QEMU doesn't
> > > use linux header to it's a bit messy.
> > > I'll send a patch to clean that up.
> > 
> > If the model ever changes in this area (which isn't planned atm), the
> > right thing to do would be to move to modern virtio. However, we're stuck
> > with what we have for the forseeable future and it works just fine if we
> > use the DMA API. If we don't use it, Linux no longer boots because it
> > installs the SMMU-backed DMA ops for the virtio devices and everything
> > faults.
> > 
> > I really don't understand why this is controversial.
> > 
> I agree we need to keep working things working. I just don't yet
> understand what change broke things and how did it happen - legacy
> devices used to just poke at guest PA from QEMU, what does it matter
> that there are changes in DMA ops if neither host nor guest use them for
> legacy setups?

The problem is that platforms which advertise an SMMU downstream of the
(virtual) PCI-RC now automatically have their PCI devices attached to the
SMMU, so if they don't use the DMA ops then they will fault.

> I guess that's just me now knowing about how DMA ops work on SMMU
> or missing some other basic point about SMMU.
> 
> I take your word that it's the right thing to do, but I'd like to figure
> it out before merging this.  I'll read up in the coming days to make
> sure I understand what the patch does, any pointers welcome.

Ok, thanks. Just shout if you have more questions. I'm also happy with
only doing this for legacy devices, given that modern virtio has the
IOMMU flag.

Will

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > On Wed, Jan 11, 2017 at 10:01:39AM +0000, Will Deacon wrote:
> > > > > On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> > > > > > On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> > > > > > > From: Will Deacon <will.deacon@arm.com>
> > > > > > > 
> > > > > > > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> > > > > > > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> > > > > > > 
> > > > > > > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > > > > [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > > > > [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > > > > [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> > > > > > > [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > > > > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > > > > [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > > > > [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > > > > [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> > > > > > > [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > > > > 
> > > > > > > <system hangs failing to read partition table>
> > > > > > > 
> > > > > > > This is because the virtio-blk is behind an SMMU, so we have consequently
> > > > > > > swizzled its DMA ops and configured the SMMU to translate accesses. This
> > > > > > > then requires the vring code to use the DMA API to establish translations,
> > > > > > > otherwise all transactions will result in fatal faults and termination.
> > > > > > > 
> > > > > > > Given that ARM-based systems only see an SMMU if one is really present
> > > > > > > (the topology is all described by firmware tables such as device-tree or
> > > > > > > IORT), then we can safely use the DMA API for all virtio devices.
> > > > > > > 
> > > > > > > Cc: Andy Lutomirski <luto@kernel.org>
> > > > > > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > > > > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > > > > 
> > > > > > I'd like to better understand then need for this one.
> > > > > > Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> > > > > > 
> > > > > > I'd rather we avoided need for more hacks and just
> > > > > > have everyone switch to that.
> > > > > 
> > > > > There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
> > > > > 
> > > > > 1. It doesn't exist for legacy devices, which are all we have on the
> > > > >    platform in question.
> > > > > 
> > > > > 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
> > > > >    my patch ;). Thanks.
> > > > > 
> > > > > In which case, for non-legacy devices we should definitely be using
> > > > > VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> > > > > world of flying cars, could we unconditionally set the DMA ops on ARM
> > > > > for legacy devices? The alternative is disabling the SMMU altogether,
> > > > > but that's less than ideal because there are non-virtio devices on the
> > > > > same PCI bus.
> > > > > 
> > > > 
> > > > I'd rather people didn't use SMMU with legacy devices.
> > > 
> > > I'm afraid we've been doing that for two years and the model already
> > > exists in a mature state, being actively used for development and
> > > validation by ARM and our partners. One of the big things its used for
> > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > dropping the SMMU from the picture isn't an option.
> > 
> > Oh so this fixes a regression?  This is something I didn't realize.
> 
> Yes, thanks. The regression came about because we implemented SMMU-backed
> DMA ops and only then was it apparent that the virtio stuff was bypassing
> even with translation enabled (because it wasn't using the DMA API).

Could you point out a commit ID?


> > A "Fixes:" tag can't hurt here.  I then wonder
> > might DMA ops ever use a DMA address which isn't a physical address
> > from QEMU point of view? If that happens, this hack breaks
> > because in legacy mode QEMU still uses the GPA.
> 
> If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> because we won't swizzle the DMA ops for the master device. If QEMU does
> advertise an SMMU, then we'll allocate DMA addresses to fit within the
> the intersection of the SMMU aperture and device's DMA mask.


Right but doesn't just poking from qemu into phys addresses work
anymore? It used to ...

> > > > Can't you guys just code up the virtio 1 layout in QEMU?
> > > > I took a look and it's not a big deal now that two other
> > > > transports converted, except mmio code in QEMU doesn't
> > > > use linux header to it's a bit messy.
> > > > I'll send a patch to clean that up.
> > > 
> > > If the model ever changes in this area (which isn't planned atm), the
> > > right thing to do would be to move to modern virtio. However, we're stuck
> > > with what we have for the forseeable future and it works just fine if we
> > > use the DMA API. If we don't use it, Linux no longer boots because it
> > > installs the SMMU-backed DMA ops for the virtio devices and everything
> > > faults.
> > > 
> > > I really don't understand why this is controversial.
> > > 
> > I agree we need to keep working things working. I just don't yet
> > understand what change broke things and how did it happen - legacy
> > devices used to just poke at guest PA from QEMU, what does it matter
> > that there are changes in DMA ops if neither host nor guest use them for
> > legacy setups?
> 
> The problem is that platforms which advertise an SMMU downstream of the
> (virtual) PCI-RC now automatically have their PCI devices attached to the
> SMMU, so if they don't use the DMA ops then they will fault.
> 
> > I guess that's just me now knowing about how DMA ops work on SMMU
> > or missing some other basic point about SMMU.
> > 
> > I take your word that it's the right thing to do, but I'd like to figure
> > it out before merging this.  I'll read up in the coming days to make
> > sure I understand what the patch does, any pointers welcome.
> 
> Ok, thanks. Just shout if you have more questions. I'm also happy with
> only doing this for legacy devices, given that modern virtio has the
> IOMMU flag.
> 
> Will

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > On Wed, Jan 11, 2017 at 10:01:39AM +0000, Will Deacon wrote:
> > > > > On Wed, Jan 11, 2017 at 01:33:31AM +0200, Michael S. Tsirkin wrote:
> > > > > > On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy wrote:
> > > > > > > From: Will Deacon <will.deacon@arm.com>
> > > > > > > 
> > > > > > > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> > > > > > > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> > > > > > > 
> > > > > > > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > > > > [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > > > > [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > > > > [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> > > > > > > [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > > > > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > > > > > > [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > > > > > > [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > > > > > > [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> > > > > > > [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > > > > > > 
> > > > > > > <system hangs failing to read partition table>
> > > > > > > 
> > > > > > > This is because the virtio-blk is behind an SMMU, so we have consequently
> > > > > > > swizzled its DMA ops and configured the SMMU to translate accesses. This
> > > > > > > then requires the vring code to use the DMA API to establish translations,
> > > > > > > otherwise all transactions will result in fatal faults and termination.
> > > > > > > 
> > > > > > > Given that ARM-based systems only see an SMMU if one is really present
> > > > > > > (the topology is all described by firmware tables such as device-tree or
> > > > > > > IORT), then we can safely use the DMA API for all virtio devices.
> > > > > > > 
> > > > > > > Cc: Andy Lutomirski <luto@kernel.org>
> > > > > > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > > > > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > > > > 
> > > > > > I'd like to better understand then need for this one.
> > > > > > Can't the device in question just set VIRTIO_F_IOMMU_PLATFORM ?
> > > > > > 
> > > > > > I'd rather we avoided need for more hacks and just
> > > > > > have everyone switch to that.
> > > > > 
> > > > > There are a couple of problems with VIRTIO_F_IOMMU_PLATFORM:
> > > > > 
> > > > > 1. It doesn't exist for legacy devices, which are all we have on the
> > > > >    platform in question.
> > > > > 
> > > > > 2. It's not documented in the virtio sp^H^HSTOP PRESS. I see you applied
> > > > >    my patch ;). Thanks.
> > > > > 
> > > > > In which case, for non-legacy devices we should definitely be using
> > > > > VIRTIO_F_IOMMU_PLATFORM, but since this platform hasn't yet moved to the
> > > > > world of flying cars, could we unconditionally set the DMA ops on ARM
> > > > > for legacy devices? The alternative is disabling the SMMU altogether,
> > > > > but that's less than ideal because there are non-virtio devices on the
> > > > > same PCI bus.
> > > > > 
> > > > 
> > > > I'd rather people didn't use SMMU with legacy devices.
> > > 
> > > I'm afraid we've been doing that for two years and the model already
> > > exists in a mature state, being actively used for development and
> > > validation by ARM and our partners. One of the big things its used for
> > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > dropping the SMMU from the picture isn't an option.
> > 
> > Oh so this fixes a regression?  This is something I didn't realize.
> 
> Yes, thanks. The regression came about because we implemented SMMU-backed
> DMA ops and only then was it apparent that the virtio stuff was bypassing
> even with translation enabled (because it wasn't using the DMA API).

Could you point out a commit ID?


> > A "Fixes:" tag can't hurt here.  I then wonder
> > might DMA ops ever use a DMA address which isn't a physical address
> > from QEMU point of view? If that happens, this hack breaks
> > because in legacy mode QEMU still uses the GPA.
> 
> If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> because we won't swizzle the DMA ops for the master device. If QEMU does
> advertise an SMMU, then we'll allocate DMA addresses to fit within the
> the intersection of the SMMU aperture and device's DMA mask.


Right but doesn't just poking from qemu into phys addresses work
anymore? It used to ...

> > > > Can't you guys just code up the virtio 1 layout in QEMU?
> > > > I took a look and it's not a big deal now that two other
> > > > transports converted, except mmio code in QEMU doesn't
> > > > use linux header to it's a bit messy.
> > > > I'll send a patch to clean that up.
> > > 
> > > If the model ever changes in this area (which isn't planned atm), the
> > > right thing to do would be to move to modern virtio. However, we're stuck
> > > with what we have for the forseeable future and it works just fine if we
> > > use the DMA API. If we don't use it, Linux no longer boots because it
> > > installs the SMMU-backed DMA ops for the virtio devices and everything
> > > faults.
> > > 
> > > I really don't understand why this is controversial.
> > > 
> > I agree we need to keep working things working. I just don't yet
> > understand what change broke things and how did it happen - legacy
> > devices used to just poke at guest PA from QEMU, what does it matter
> > that there are changes in DMA ops if neither host nor guest use them for
> > legacy setups?
> 
> The problem is that platforms which advertise an SMMU downstream of the
> (virtual) PCI-RC now automatically have their PCI devices attached to the
> SMMU, so if they don't use the DMA ops then they will fault.
> 
> > I guess that's just me now knowing about how DMA ops work on SMMU
> > or missing some other basic point about SMMU.
> > 
> > I take your word that it's the right thing to do, but I'd like to figure
> > it out before merging this.  I'll read up in the coming days to make
> > sure I understand what the patch does, any pointers welcome.
> 
> Ok, thanks. Just shout if you have more questions. I'm also happy with
> only doing this for legacy devices, given that modern virtio has the
> IOMMU flag.
> 
> Will

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> > On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > > I'd rather people didn't use SMMU with legacy devices.
> > > > 
> > > > I'm afraid we've been doing that for two years and the model already
> > > > exists in a mature state, being actively used for development and
> > > > validation by ARM and our partners. One of the big things its used for
> > > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > > dropping the SMMU from the picture isn't an option.
> > > 
> > > Oh so this fixes a regression?  This is something I didn't realize.
> > 
> > Yes, thanks. The regression came about because we implemented SMMU-backed
> > DMA ops and only then was it apparent that the virtio stuff was bypassing
> > even with translation enabled (because it wasn't using the DMA API).
> 
> Could you point out a commit ID?

There has been a fair amount of work in this area recently, but you're
probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
as the culprit, which is the point at which we started to swizzle DMA
ops for devices upstream of an SMMU automatically.

> > > A "Fixes:" tag can't hurt here.  I then wonder
> > > might DMA ops ever use a DMA address which isn't a physical address
> > > from QEMU point of view? If that happens, this hack breaks
> > > because in legacy mode QEMU still uses the GPA.
> > 
> > If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> > because we won't swizzle the DMA ops for the master device. If QEMU does
> > advertise an SMMU, then we'll allocate DMA addresses to fit within the
> > the intersection of the SMMU aperture and device's DMA mask.
> 
> 
> Right but doesn't just poking from qemu into phys addresses work
> anymore? It used to ...

Provided that there's no SMMU, then it will continue to work. and my
understanding (from talking to Peter Maydell) is that qemu doesn't model
an SMMU for ARM-based machines.

Will

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> > On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > > I'd rather people didn't use SMMU with legacy devices.
> > > > 
> > > > I'm afraid we've been doing that for two years and the model already
> > > > exists in a mature state, being actively used for development and
> > > > validation by ARM and our partners. One of the big things its used for
> > > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > > dropping the SMMU from the picture isn't an option.
> > > 
> > > Oh so this fixes a regression?  This is something I didn't realize.
> > 
> > Yes, thanks. The regression came about because we implemented SMMU-backed
> > DMA ops and only then was it apparent that the virtio stuff was bypassing
> > even with translation enabled (because it wasn't using the DMA API).
> 
> Could you point out a commit ID?

There has been a fair amount of work in this area recently, but you're
probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
as the culprit, which is the point at which we started to swizzle DMA
ops for devices upstream of an SMMU automatically.

> > > A "Fixes:" tag can't hurt here.  I then wonder
> > > might DMA ops ever use a DMA address which isn't a physical address
> > > from QEMU point of view? If that happens, this hack breaks
> > > because in legacy mode QEMU still uses the GPA.
> > 
> > If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> > because we won't swizzle the DMA ops for the master device. If QEMU does
> > advertise an SMMU, then we'll allocate DMA addresses to fit within the
> > the intersection of the SMMU aperture and device's DMA mask.
> 
> 
> Right but doesn't just poking from qemu into phys addresses work
> anymore? It used to ...

Provided that there's no SMMU, then it will continue to work. and my
understanding (from talking to Peter Maydell) is that qemu doesn't model
an SMMU for ARM-based machines.

Will

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
> On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> > On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> > > On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > > > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > > > I'd rather people didn't use SMMU with legacy devices.
> > > > > 
> > > > > I'm afraid we've been doing that for two years and the model already
> > > > > exists in a mature state, being actively used for development and
> > > > > validation by ARM and our partners. One of the big things its used for
> > > > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > > > dropping the SMMU from the picture isn't an option.
> > > > 
> > > > Oh so this fixes a regression?  This is something I didn't realize.
> > > 
> > > Yes, thanks. The regression came about because we implemented SMMU-backed
> > > DMA ops and only then was it apparent that the virtio stuff was bypassing
> > > even with translation enabled (because it wasn't using the DMA API).
> > 
> > Could you point out a commit ID?
> 
> There has been a fair amount of work in this area recently, but you're
> probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> as the culprit, which is the point at which we started to swizzle DMA
> ops for devices upstream of an SMMU automatically.
> 
> > > > A "Fixes:" tag can't hurt here.  I then wonder
> > > > might DMA ops ever use a DMA address which isn't a physical address
> > > > from QEMU point of view? If that happens, this hack breaks
> > > > because in legacy mode QEMU still uses the GPA.
> > > 
> > > If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> > > because we won't swizzle the DMA ops for the master device. If QEMU does
> > > advertise an SMMU, then we'll allocate DMA addresses to fit within the
> > > the intersection of the SMMU aperture and device's DMA mask.
> > 
> > 
> > Right but doesn't just poking from qemu into phys addresses work
> > anymore? It used to ...
> 
> Provided that there's no SMMU, then it will continue to work. and my
> understanding (from talking to Peter Maydell) is that qemu doesn't model
> an SMMU for ARM-based machines.
> 
> Will

So how come people report failures due to presence of SMMU?
Using some other hypervisor?

-- 
MST

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
> On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> > On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> > > On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > > > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > > > I'd rather people didn't use SMMU with legacy devices.
> > > > > 
> > > > > I'm afraid we've been doing that for two years and the model already
> > > > > exists in a mature state, being actively used for development and
> > > > > validation by ARM and our partners. One of the big things its used for
> > > > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > > > dropping the SMMU from the picture isn't an option.
> > > > 
> > > > Oh so this fixes a regression?  This is something I didn't realize.
> > > 
> > > Yes, thanks. The regression came about because we implemented SMMU-backed
> > > DMA ops and only then was it apparent that the virtio stuff was bypassing
> > > even with translation enabled (because it wasn't using the DMA API).
> > 
> > Could you point out a commit ID?
> 
> There has been a fair amount of work in this area recently, but you're
> probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> as the culprit, which is the point at which we started to swizzle DMA
> ops for devices upstream of an SMMU automatically.
> 
> > > > A "Fixes:" tag can't hurt here.  I then wonder
> > > > might DMA ops ever use a DMA address which isn't a physical address
> > > > from QEMU point of view? If that happens, this hack breaks
> > > > because in legacy mode QEMU still uses the GPA.
> > > 
> > > If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> > > because we won't swizzle the DMA ops for the master device. If QEMU does
> > > advertise an SMMU, then we'll allocate DMA addresses to fit within the
> > > the intersection of the SMMU aperture and device's DMA mask.
> > 
> > 
> > Right but doesn't just poking from qemu into phys addresses work
> > anymore? It used to ...
> 
> Provided that there's no SMMU, then it will continue to work. and my
> understanding (from talking to Peter Maydell) is that qemu doesn't model
> an SMMU for ARM-based machines.
> 
> Will

So how come people report failures due to presence of SMMU?
Using some other hypervisor?

-- 
MST

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Mon, Jan 16, 2017 at 04:18:03PM +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
> > On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> > > On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> > > > On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > > > > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > > > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > > > > I'd rather people didn't use SMMU with legacy devices.
> > > > > > 
> > > > > > I'm afraid we've been doing that for two years and the model already
> > > > > > exists in a mature state, being actively used for development and
> > > > > > validation by ARM and our partners. One of the big things its used for
> > > > > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > > > > dropping the SMMU from the picture isn't an option.
> > > > > 
> > > > > Oh so this fixes a regression?  This is something I didn't realize.
> > > > 
> > > > Yes, thanks. The regression came about because we implemented SMMU-backed
> > > > DMA ops and only then was it apparent that the virtio stuff was bypassing
> > > > even with translation enabled (because it wasn't using the DMA API).
> > > 
> > > Could you point out a commit ID?
> > 
> > There has been a fair amount of work in this area recently, but you're
> > probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> > as the culprit, which is the point at which we started to swizzle DMA
> > ops for devices upstream of an SMMU automatically.
> > 
> > > > > A "Fixes:" tag can't hurt here.  I then wonder
> > > > > might DMA ops ever use a DMA address which isn't a physical address
> > > > > from QEMU point of view? If that happens, this hack breaks
> > > > > because in legacy mode QEMU still uses the GPA.
> > > > 
> > > > If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> > > > because we won't swizzle the DMA ops for the master device. If QEMU does
> > > > advertise an SMMU, then we'll allocate DMA addresses to fit within the
> > > > the intersection of the SMMU aperture and device's DMA mask.
> > > 
> > > 
> > > Right but doesn't just poking from qemu into phys addresses work
> > > anymore? It used to ...
> > 
> > Provided that there's no SMMU, then it will continue to work. and my
> > understanding (from talking to Peter Maydell) is that qemu doesn't model
> > an SMMU for ARM-based machines.
> > 
> 
> So how come people report failures due to presence of SMMU?
> Using some other hypervisor?

The failures are reported on the ARM fastmodel (a complete system
emulation that runs on an x86 box), where an SMMU *is* present
downstream of the virtio-pci masters. There's no qemu involved there.

Will

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Mon, Jan 16, 2017 at 04:18:03PM +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
> > On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> > > On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> > > > On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > > > > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > > > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > > > > I'd rather people didn't use SMMU with legacy devices.
> > > > > > 
> > > > > > I'm afraid we've been doing that for two years and the model already
> > > > > > exists in a mature state, being actively used for development and
> > > > > > validation by ARM and our partners. One of the big things its used for
> > > > > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > > > > dropping the SMMU from the picture isn't an option.
> > > > > 
> > > > > Oh so this fixes a regression?  This is something I didn't realize.
> > > > 
> > > > Yes, thanks. The regression came about because we implemented SMMU-backed
> > > > DMA ops and only then was it apparent that the virtio stuff was bypassing
> > > > even with translation enabled (because it wasn't using the DMA API).
> > > 
> > > Could you point out a commit ID?
> > 
> > There has been a fair amount of work in this area recently, but you're
> > probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> > as the culprit, which is the point at which we started to swizzle DMA
> > ops for devices upstream of an SMMU automatically.
> > 
> > > > > A "Fixes:" tag can't hurt here.  I then wonder
> > > > > might DMA ops ever use a DMA address which isn't a physical address
> > > > > from QEMU point of view? If that happens, this hack breaks
> > > > > because in legacy mode QEMU still uses the GPA.
> > > > 
> > > > If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> > > > because we won't swizzle the DMA ops for the master device. If QEMU does
> > > > advertise an SMMU, then we'll allocate DMA addresses to fit within the
> > > > the intersection of the SMMU aperture and device's DMA mask.
> > > 
> > > 
> > > Right but doesn't just poking from qemu into phys addresses work
> > > anymore? It used to ...
> > 
> > Provided that there's no SMMU, then it will continue to work. and my
> > understanding (from talking to Peter Maydell) is that qemu doesn't model
> > an SMMU for ARM-based machines.
> > 
> 
> So how come people report failures due to presence of SMMU?
> Using some other hypervisor?

The failures are reported on the ARM fastmodel (a complete system
emulation that runs on an x86 box), where an SMMU *is* present
downstream of the virtio-pci masters. There's no qemu involved there.

Will

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Mon, Jan 16, 2017 at 02:21:03PM +0000, Will Deacon wrote:
> On Mon, Jan 16, 2017 at 04:18:03PM +0200, Michael S. Tsirkin wrote:
> > On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
> > > On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> > > > On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> > > > > On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > > > > > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > > > > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > > > > > I'd rather people didn't use SMMU with legacy devices.
> > > > > > > 
> > > > > > > I'm afraid we've been doing that for two years and the model already
> > > > > > > exists in a mature state, being actively used for development and
> > > > > > > validation by ARM and our partners. One of the big things its used for
> > > > > > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > > > > > dropping the SMMU from the picture isn't an option.
> > > > > > 
> > > > > > Oh so this fixes a regression?  This is something I didn't realize.
> > > > > 
> > > > > Yes, thanks. The regression came about because we implemented SMMU-backed
> > > > > DMA ops and only then was it apparent that the virtio stuff was bypassing
> > > > > even with translation enabled (because it wasn't using the DMA API).
> > > > 
> > > > Could you point out a commit ID?
> > > 
> > > There has been a fair amount of work in this area recently, but you're
> > > probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> > > as the culprit, which is the point at which we started to swizzle DMA
> > > ops for devices upstream of an SMMU automatically.
> > > 
> > > > > > A "Fixes:" tag can't hurt here.  I then wonder
> > > > > > might DMA ops ever use a DMA address which isn't a physical address
> > > > > > from QEMU point of view? If that happens, this hack breaks
> > > > > > because in legacy mode QEMU still uses the GPA.
> > > > > 
> > > > > If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> > > > > because we won't swizzle the DMA ops for the master device. If QEMU does
> > > > > advertise an SMMU, then we'll allocate DMA addresses to fit within the
> > > > > the intersection of the SMMU aperture and device's DMA mask.
> > > > 
> > > > 
> > > > Right but doesn't just poking from qemu into phys addresses work
> > > > anymore? It used to ...
> > > 
> > > Provided that there's no SMMU, then it will continue to work. and my
> > > understanding (from talking to Peter Maydell) is that qemu doesn't model
> > > an SMMU for ARM-based machines.
> > > 
> > 
> > So how come people report failures due to presence of SMMU?
> > Using some other hypervisor?
> 
> The failures are reported on the ARM fastmodel (a complete system
> emulation that runs on an x86 box), where an SMMU *is* present
> downstream of the virtio-pci masters. There's no qemu involved there.
> 
> Will

I see. And this hypervisor actually coded up looking up
translations in the SMMU unconditionally for legacy devices,
and this worked as long as guest didn't touch the SMMU?

-- 
MST

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Mon, Jan 16, 2017 at 02:21:03PM +0000, Will Deacon wrote:
> On Mon, Jan 16, 2017 at 04:18:03PM +0200, Michael S. Tsirkin wrote:
> > On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
> > > On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> > > > On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> > > > > On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > > > > > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > > > > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > > > > > I'd rather people didn't use SMMU with legacy devices.
> > > > > > > 
> > > > > > > I'm afraid we've been doing that for two years and the model already
> > > > > > > exists in a mature state, being actively used for development and
> > > > > > > validation by ARM and our partners. One of the big things its used for
> > > > > > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > > > > > dropping the SMMU from the picture isn't an option.
> > > > > > 
> > > > > > Oh so this fixes a regression?  This is something I didn't realize.
> > > > > 
> > > > > Yes, thanks. The regression came about because we implemented SMMU-backed
> > > > > DMA ops and only then was it apparent that the virtio stuff was bypassing
> > > > > even with translation enabled (because it wasn't using the DMA API).
> > > > 
> > > > Could you point out a commit ID?
> > > 
> > > There has been a fair amount of work in this area recently, but you're
> > > probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> > > as the culprit, which is the point at which we started to swizzle DMA
> > > ops for devices upstream of an SMMU automatically.
> > > 
> > > > > > A "Fixes:" tag can't hurt here.  I then wonder
> > > > > > might DMA ops ever use a DMA address which isn't a physical address
> > > > > > from QEMU point of view? If that happens, this hack breaks
> > > > > > because in legacy mode QEMU still uses the GPA.
> > > > > 
> > > > > If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> > > > > because we won't swizzle the DMA ops for the master device. If QEMU does
> > > > > advertise an SMMU, then we'll allocate DMA addresses to fit within the
> > > > > the intersection of the SMMU aperture and device's DMA mask.
> > > > 
> > > > 
> > > > Right but doesn't just poking from qemu into phys addresses work
> > > > anymore? It used to ...
> > > 
> > > Provided that there's no SMMU, then it will continue to work. and my
> > > understanding (from talking to Peter Maydell) is that qemu doesn't model
> > > an SMMU for ARM-based machines.
> > > 
> > 
> > So how come people report failures due to presence of SMMU?
> > Using some other hypervisor?
> 
> The failures are reported on the ARM fastmodel (a complete system
> emulation that runs on an x86 box), where an SMMU *is* present
> downstream of the virtio-pci masters. There's no qemu involved there.
> 
> Will

I see. And this hypervisor actually coded up looking up
translations in the SMMU unconditionally for legacy devices,
and this worked as long as guest didn't touch the SMMU?

-- 
MST

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Mon, Jan 16, 2017 at 04:27:28PM +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 16, 2017 at 02:21:03PM +0000, Will Deacon wrote:
> > On Mon, Jan 16, 2017 at 04:18:03PM +0200, Michael S. Tsirkin wrote:
> > > On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
> > > > On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> > > > > On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> > > > > > On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > > > > > > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > > > > > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > > > > > > I'd rather people didn't use SMMU with legacy devices.
> > > > > > > > 
> > > > > > > > I'm afraid we've been doing that for two years and the model already
> > > > > > > > exists in a mature state, being actively used for development and
> > > > > > > > validation by ARM and our partners. One of the big things its used for
> > > > > > > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > > > > > > dropping the SMMU from the picture isn't an option.
> > > > > > > 
> > > > > > > Oh so this fixes a regression?  This is something I didn't realize.
> > > > > > 
> > > > > > Yes, thanks. The regression came about because we implemented SMMU-backed
> > > > > > DMA ops and only then was it apparent that the virtio stuff was bypassing
> > > > > > even with translation enabled (because it wasn't using the DMA API).
> > > > > 
> > > > > Could you point out a commit ID?
> > > > 
> > > > There has been a fair amount of work in this area recently, but you're
> > > > probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> > > > as the culprit, which is the point at which we started to swizzle DMA
> > > > ops for devices upstream of an SMMU automatically.
> > > > 
> > > > > > > A "Fixes:" tag can't hurt here.  I then wonder
> > > > > > > might DMA ops ever use a DMA address which isn't a physical address
> > > > > > > from QEMU point of view? If that happens, this hack breaks
> > > > > > > because in legacy mode QEMU still uses the GPA.
> > > > > > 
> > > > > > If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> > > > > > because we won't swizzle the DMA ops for the master device. If QEMU does
> > > > > > advertise an SMMU, then we'll allocate DMA addresses to fit within the
> > > > > > the intersection of the SMMU aperture and device's DMA mask.
> > > > > 
> > > > > 
> > > > > Right but doesn't just poking from qemu into phys addresses work
> > > > > anymore? It used to ...
> > > > 
> > > > Provided that there's no SMMU, then it will continue to work. and my
> > > > understanding (from talking to Peter Maydell) is that qemu doesn't model
> > > > an SMMU for ARM-based machines.
> > > > 
> > > 
> > > So how come people report failures due to presence of SMMU?
> > > Using some other hypervisor?
> > 
> > The failures are reported on the ARM fastmodel (a complete system
> > emulation that runs on an x86 box), where an SMMU *is* present
> > downstream of the virtio-pci masters. There's no qemu involved there.
> > 
> I see. And this hypervisor actually coded up looking up
> translations in the SMMU unconditionally for legacy devices,
> and this worked as long as guest didn't touch the SMMU?

Well, the fastmodel isn't a hypervisor really. It's a full system emulation,
so it's better to think of it like a piece of hardware. For example, you
could run KVM on the fastmodel. But yes, when Linux didn't swizzle the
DMA ops to point at the SMMU, then everything defaults to bypass (because
that's the default behaviour of the SMMU driver -- this is configurable
on the command line) which is why things used to work.

Will

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Mon, Jan 16, 2017 at 04:27:28PM +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 16, 2017 at 02:21:03PM +0000, Will Deacon wrote:
> > On Mon, Jan 16, 2017 at 04:18:03PM +0200, Michael S. Tsirkin wrote:
> > > On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
> > > > On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> > > > > On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> > > > > > On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > > > > > > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > > > > > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > > > > > > I'd rather people didn't use SMMU with legacy devices.
> > > > > > > > 
> > > > > > > > I'm afraid we've been doing that for two years and the model already
> > > > > > > > exists in a mature state, being actively used for development and
> > > > > > > > validation by ARM and our partners. One of the big things its used for
> > > > > > > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > > > > > > dropping the SMMU from the picture isn't an option.
> > > > > > > 
> > > > > > > Oh so this fixes a regression?  This is something I didn't realize.
> > > > > > 
> > > > > > Yes, thanks. The regression came about because we implemented SMMU-backed
> > > > > > DMA ops and only then was it apparent that the virtio stuff was bypassing
> > > > > > even with translation enabled (because it wasn't using the DMA API).
> > > > > 
> > > > > Could you point out a commit ID?
> > > > 
> > > > There has been a fair amount of work in this area recently, but you're
> > > > probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> > > > as the culprit, which is the point at which we started to swizzle DMA
> > > > ops for devices upstream of an SMMU automatically.
> > > > 
> > > > > > > A "Fixes:" tag can't hurt here.  I then wonder
> > > > > > > might DMA ops ever use a DMA address which isn't a physical address
> > > > > > > from QEMU point of view? If that happens, this hack breaks
> > > > > > > because in legacy mode QEMU still uses the GPA.
> > > > > > 
> > > > > > If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> > > > > > because we won't swizzle the DMA ops for the master device. If QEMU does
> > > > > > advertise an SMMU, then we'll allocate DMA addresses to fit within the
> > > > > > the intersection of the SMMU aperture and device's DMA mask.
> > > > > 
> > > > > 
> > > > > Right but doesn't just poking from qemu into phys addresses work
> > > > > anymore? It used to ...
> > > > 
> > > > Provided that there's no SMMU, then it will continue to work. and my
> > > > understanding (from talking to Peter Maydell) is that qemu doesn't model
> > > > an SMMU for ARM-based machines.
> > > > 
> > > 
> > > So how come people report failures due to presence of SMMU?
> > > Using some other hypervisor?
> > 
> > The failures are reported on the ARM fastmodel (a complete system
> > emulation that runs on an x86 box), where an SMMU *is* present
> > downstream of the virtio-pci masters. There's no qemu involved there.
> > 
> I see. And this hypervisor actually coded up looking up
> translations in the SMMU unconditionally for legacy devices,
> and this worked as long as guest didn't touch the SMMU?

Well, the fastmodel isn't a hypervisor really. It's a full system emulation,
so it's better to think of it like a piece of hardware. For example, you
could run KVM on the fastmodel. But yes, when Linux didn't swizzle the
DMA ops to point at the SMMU, then everything defaults to bypass (because
that's the default behaviour of the SMMU driver -- this is configurable
on the command line) which is why things used to work.

Will

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Mon, Jan 16, 2017 at 02:34:08PM +0000, Will Deacon wrote:
> On Mon, Jan 16, 2017 at 04:27:28PM +0200, Michael S. Tsirkin wrote:
> > On Mon, Jan 16, 2017 at 02:21:03PM +0000, Will Deacon wrote:
> > > On Mon, Jan 16, 2017 at 04:18:03PM +0200, Michael S. Tsirkin wrote:
> > > > On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
> > > > > On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> > > > > > On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> > > > > > > On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > > > > > > > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > > > > > > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > > > > > > > I'd rather people didn't use SMMU with legacy devices.
> > > > > > > > > 
> > > > > > > > > I'm afraid we've been doing that for two years and the model already
> > > > > > > > > exists in a mature state, being actively used for development and
> > > > > > > > > validation by ARM and our partners. One of the big things its used for
> > > > > > > > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > > > > > > > dropping the SMMU from the picture isn't an option.
> > > > > > > > 
> > > > > > > > Oh so this fixes a regression?  This is something I didn't realize.
> > > > > > > 
> > > > > > > Yes, thanks. The regression came about because we implemented SMMU-backed
> > > > > > > DMA ops and only then was it apparent that the virtio stuff was bypassing
> > > > > > > even with translation enabled (because it wasn't using the DMA API).
> > > > > > 
> > > > > > Could you point out a commit ID?
> > > > > 
> > > > > There has been a fair amount of work in this area recently, but you're
> > > > > probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> > > > > as the culprit, which is the point at which we started to swizzle DMA
> > > > > ops for devices upstream of an SMMU automatically.
> > > > > 
> > > > > > > > A "Fixes:" tag can't hurt here.  I then wonder
> > > > > > > > might DMA ops ever use a DMA address which isn't a physical address
> > > > > > > > from QEMU point of view? If that happens, this hack breaks
> > > > > > > > because in legacy mode QEMU still uses the GPA.
> > > > > > > 
> > > > > > > If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> > > > > > > because we won't swizzle the DMA ops for the master device. If QEMU does
> > > > > > > advertise an SMMU, then we'll allocate DMA addresses to fit within the
> > > > > > > the intersection of the SMMU aperture and device's DMA mask.
> > > > > > 
> > > > > > 
> > > > > > Right but doesn't just poking from qemu into phys addresses work
> > > > > > anymore? It used to ...
> > > > > 
> > > > > Provided that there's no SMMU, then it will continue to work. and my
> > > > > understanding (from talking to Peter Maydell) is that qemu doesn't model
> > > > > an SMMU for ARM-based machines.
> > > > > 
> > > > 
> > > > So how come people report failures due to presence of SMMU?
> > > > Using some other hypervisor?
> > > 
> > > The failures are reported on the ARM fastmodel (a complete system
> > > emulation that runs on an x86 box), where an SMMU *is* present
> > > downstream of the virtio-pci masters. There's no qemu involved there.
> > > 
> > I see. And this hypervisor actually coded up looking up
> > translations in the SMMU unconditionally for legacy devices,
> > and this worked as long as guest didn't touch the SMMU?
> 
> Well, the fastmodel isn't a hypervisor really. It's a full system emulation,
> so it's better to think of it like a piece of hardware. For example, you
> could run KVM on the fastmodel. But yes, when Linux didn't swizzle the
> DMA ops to point at the SMMU, then everything defaults to bypass (because
> that's the default behaviour of the SMMU driver -- this is configurable
> on the command line) which is why things used to work.
> 
> Will

I would be a bit happier if Linux checked virtio iommu quirk and skipped
the DMA ops thing then. It's a bit ugly but at least it's consistently
ugly.  To get clean emulation you would then use a modern device.

-- 
MST

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Mon, Jan 16, 2017 at 02:34:08PM +0000, Will Deacon wrote:
> On Mon, Jan 16, 2017 at 04:27:28PM +0200, Michael S. Tsirkin wrote:
> > On Mon, Jan 16, 2017 at 02:21:03PM +0000, Will Deacon wrote:
> > > On Mon, Jan 16, 2017 at 04:18:03PM +0200, Michael S. Tsirkin wrote:
> > > > On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
> > > > > On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> > > > > > On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> > > > > > > On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > > > > > > > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > > > > > > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > > > > > > > I'd rather people didn't use SMMU with legacy devices.
> > > > > > > > > 
> > > > > > > > > I'm afraid we've been doing that for two years and the model already
> > > > > > > > > exists in a mature state, being actively used for development and
> > > > > > > > > validation by ARM and our partners. One of the big things its used for
> > > > > > > > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > > > > > > > dropping the SMMU from the picture isn't an option.
> > > > > > > > 
> > > > > > > > Oh so this fixes a regression?  This is something I didn't realize.
> > > > > > > 
> > > > > > > Yes, thanks. The regression came about because we implemented SMMU-backed
> > > > > > > DMA ops and only then was it apparent that the virtio stuff was bypassing
> > > > > > > even with translation enabled (because it wasn't using the DMA API).
> > > > > > 
> > > > > > Could you point out a commit ID?
> > > > > 
> > > > > There has been a fair amount of work in this area recently, but you're
> > > > > probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> > > > > as the culprit, which is the point at which we started to swizzle DMA
> > > > > ops for devices upstream of an SMMU automatically.
> > > > > 
> > > > > > > > A "Fixes:" tag can't hurt here.  I then wonder
> > > > > > > > might DMA ops ever use a DMA address which isn't a physical address
> > > > > > > > from QEMU point of view? If that happens, this hack breaks
> > > > > > > > because in legacy mode QEMU still uses the GPA.
> > > > > > > 
> > > > > > > If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> > > > > > > because we won't swizzle the DMA ops for the master device. If QEMU does
> > > > > > > advertise an SMMU, then we'll allocate DMA addresses to fit within the
> > > > > > > the intersection of the SMMU aperture and device's DMA mask.
> > > > > > 
> > > > > > 
> > > > > > Right but doesn't just poking from qemu into phys addresses work
> > > > > > anymore? It used to ...
> > > > > 
> > > > > Provided that there's no SMMU, then it will continue to work. and my
> > > > > understanding (from talking to Peter Maydell) is that qemu doesn't model
> > > > > an SMMU for ARM-based machines.
> > > > > 
> > > > 
> > > > So how come people report failures due to presence of SMMU?
> > > > Using some other hypervisor?
> > > 
> > > The failures are reported on the ARM fastmodel (a complete system
> > > emulation that runs on an x86 box), where an SMMU *is* present
> > > downstream of the virtio-pci masters. There's no qemu involved there.
> > > 
> > I see. And this hypervisor actually coded up looking up
> > translations in the SMMU unconditionally for legacy devices,
> > and this worked as long as guest didn't touch the SMMU?
> 
> Well, the fastmodel isn't a hypervisor really. It's a full system emulation,
> so it's better to think of it like a piece of hardware. For example, you
> could run KVM on the fastmodel. But yes, when Linux didn't swizzle the
> DMA ops to point at the SMMU, then everything defaults to bypass (because
> that's the default behaviour of the SMMU driver -- this is configurable
> on the command line) which is why things used to work.
> 
> Will

I would be a bit happier if Linux checked virtio iommu quirk and skipped
the DMA ops thing then. It's a bit ugly but at least it's consistently
ugly.  To get clean emulation you would then use a modern device.

-- 
MST

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Thu, Jan 19, 2017 at 11:51:06PM +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 16, 2017 at 02:34:08PM +0000, Will Deacon wrote:
> > On Mon, Jan 16, 2017 at 04:27:28PM +0200, Michael S. Tsirkin wrote:
> > > On Mon, Jan 16, 2017 at 02:21:03PM +0000, Will Deacon wrote:
> > > > On Mon, Jan 16, 2017 at 04:18:03PM +0200, Michael S. Tsirkin wrote:
> > > > > On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
> > > > > > On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> > > > > > > On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> > > > > > > > On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > > > > > > > > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > > > > > > > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > > > > > > > > I'd rather people didn't use SMMU with legacy devices.
> > > > > > > > > > 
> > > > > > > > > > I'm afraid we've been doing that for two years and the model already
> > > > > > > > > > exists in a mature state, being actively used for development and
> > > > > > > > > > validation by ARM and our partners. One of the big things its used for
> > > > > > > > > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > > > > > > > > dropping the SMMU from the picture isn't an option.
> > > > > > > > > 
> > > > > > > > > Oh so this fixes a regression?  This is something I didn't realize.
> > > > > > > > 
> > > > > > > > Yes, thanks. The regression came about because we implemented SMMU-backed
> > > > > > > > DMA ops and only then was it apparent that the virtio stuff was bypassing
> > > > > > > > even with translation enabled (because it wasn't using the DMA API).
> > > > > > > 
> > > > > > > Could you point out a commit ID?
> > > > > > 
> > > > > > There has been a fair amount of work in this area recently, but you're
> > > > > > probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> > > > > > as the culprit, which is the point at which we started to swizzle DMA
> > > > > > ops for devices upstream of an SMMU automatically.
> > > > > > 
> > > > > > > > > A "Fixes:" tag can't hurt here.  I then wonder
> > > > > > > > > might DMA ops ever use a DMA address which isn't a physical address
> > > > > > > > > from QEMU point of view? If that happens, this hack breaks
> > > > > > > > > because in legacy mode QEMU still uses the GPA.
> > > > > > > > 
> > > > > > > > If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> > > > > > > > because we won't swizzle the DMA ops for the master device. If QEMU does
> > > > > > > > advertise an SMMU, then we'll allocate DMA addresses to fit within the
> > > > > > > > the intersection of the SMMU aperture and device's DMA mask.
> > > > > > > 
> > > > > > > 
> > > > > > > Right but doesn't just poking from qemu into phys addresses work
> > > > > > > anymore? It used to ...
> > > > > > 
> > > > > > Provided that there's no SMMU, then it will continue to work. and my
> > > > > > understanding (from talking to Peter Maydell) is that qemu doesn't model
> > > > > > an SMMU for ARM-based machines.
> > > > > > 
> > > > > 
> > > > > So how come people report failures due to presence of SMMU?
> > > > > Using some other hypervisor?
> > > > 
> > > > The failures are reported on the ARM fastmodel (a complete system
> > > > emulation that runs on an x86 box), where an SMMU *is* present
> > > > downstream of the virtio-pci masters. There's no qemu involved there.
> > > > 
> > > I see. And this hypervisor actually coded up looking up
> > > translations in the SMMU unconditionally for legacy devices,
> > > and this worked as long as guest didn't touch the SMMU?
> > 
> > Well, the fastmodel isn't a hypervisor really. It's a full system emulation,
> > so it's better to think of it like a piece of hardware. For example, you
> > could run KVM on the fastmodel. But yes, when Linux didn't swizzle the
> > DMA ops to point at the SMMU, then everything defaults to bypass (because
> > that's the default behaviour of the SMMU driver -- this is configurable
> > on the command line) which is why things used to work.
> > 
> I would be a bit happier if Linux checked virtio iommu quirk and skipped
> the DMA ops thing then. It's a bit ugly but at least it's consistently
> ugly.  To get clean emulation you would then use a modern device.

Sorry, but I'm not sure I follow your suggestion here. If you're talking
about the DMA ops themselves, they are assigned in arch_setup_dma_ops long
before the virtio layer gets involved, so we really can't figure out
whether the device has a virtio iommu quirk at that point.

I've reworked the patch (see below) so that we unconditionally set the
DMA ops for arm-based legacy devices, but I can't really tell what you're
after here.

Will

--->8

From 213bad7fdb8e4f45a7724be169cda292bbb50d2b Mon Sep 17 00:00:00 2001
From: Will Deacon <will.deacon@arm.com>
Date: Tue, 20 Dec 2016 12:12:49 +0000
Subject: [PATCH] vring: Force use of DMA API for ARM-based systems with legacy
 devices

Booting Linux on an ARM fastmodel containing an SMMU emulation results
in an unexpected I/O page fault from the legacy virtio-blk PCI device:

[    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
[    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
[    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
[    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
[    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
[    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
[    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
[    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
[    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
[    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000

<system hangs failing to read partition table>

This is because the legacy virtio-blk device is behind an SMMU, so we
have consequently swizzled its DMA ops and configured the SMMU to
translate accesses. This then requires the vring code to use the DMA API
to establish translations, otherwise all transactions will result in
fatal faults and termination.

Given that ARM-based systems only see an SMMU if one is really present
(the topology is all described by firmware tables such as device-tree or
IORT), then we can safely use the DMA API for all legacy virtio devices.
Modern devices can advertise the prescense of an IOMMU using the
VIRTIO_F_IOMMU_PLATFORM feature flag.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: <stable@vger.kernel.org>
Fixes: 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 drivers/virtio/virtio_ring.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 409aeaa49246..7e38ed79c3fc 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -159,6 +159,13 @@ 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);
+
 	return false;
 }
 
-- 
2.1.4

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Will Deacon]

On Thu, Jan 19, 2017 at 11:51:06PM +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 16, 2017 at 02:34:08PM +0000, Will Deacon wrote:
> > On Mon, Jan 16, 2017 at 04:27:28PM +0200, Michael S. Tsirkin wrote:
> > > On Mon, Jan 16, 2017 at 02:21:03PM +0000, Will Deacon wrote:
> > > > On Mon, Jan 16, 2017 at 04:18:03PM +0200, Michael S. Tsirkin wrote:
> > > > > On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
> > > > > > On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> > > > > > > On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> > > > > > > > On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> > > > > > > > > On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> > > > > > > > > > On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> > > > > > > > > > > I'd rather people didn't use SMMU with legacy devices.
> > > > > > > > > > 
> > > > > > > > > > I'm afraid we've been doing that for two years and the model already
> > > > > > > > > > exists in a mature state, being actively used for development and
> > > > > > > > > > validation by ARM and our partners. One of the big things its used for
> > > > > > > > > > is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> > > > > > > > > > dropping the SMMU from the picture isn't an option.
> > > > > > > > > 
> > > > > > > > > Oh so this fixes a regression?  This is something I didn't realize.
> > > > > > > > 
> > > > > > > > Yes, thanks. The regression came about because we implemented SMMU-backed
> > > > > > > > DMA ops and only then was it apparent that the virtio stuff was bypassing
> > > > > > > > even with translation enabled (because it wasn't using the DMA API).
> > > > > > > 
> > > > > > > Could you point out a commit ID?
> > > > > > 
> > > > > > There has been a fair amount of work in this area recently, but you're
> > > > > > probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> > > > > > as the culprit, which is the point at which we started to swizzle DMA
> > > > > > ops for devices upstream of an SMMU automatically.
> > > > > > 
> > > > > > > > > A "Fixes:" tag can't hurt here.  I then wonder
> > > > > > > > > might DMA ops ever use a DMA address which isn't a physical address
> > > > > > > > > from QEMU point of view? If that happens, this hack breaks
> > > > > > > > > because in legacy mode QEMU still uses the GPA.
> > > > > > > > 
> > > > > > > > If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> > > > > > > > because we won't swizzle the DMA ops for the master device. If QEMU does
> > > > > > > > advertise an SMMU, then we'll allocate DMA addresses to fit within the
> > > > > > > > the intersection of the SMMU aperture and device's DMA mask.
> > > > > > > 
> > > > > > > 
> > > > > > > Right but doesn't just poking from qemu into phys addresses work
> > > > > > > anymore? It used to ...
> > > > > > 
> > > > > > Provided that there's no SMMU, then it will continue to work. and my
> > > > > > understanding (from talking to Peter Maydell) is that qemu doesn't model
> > > > > > an SMMU for ARM-based machines.
> > > > > > 
> > > > > 
> > > > > So how come people report failures due to presence of SMMU?
> > > > > Using some other hypervisor?
> > > > 
> > > > The failures are reported on the ARM fastmodel (a complete system
> > > > emulation that runs on an x86 box), where an SMMU *is* present
> > > > downstream of the virtio-pci masters. There's no qemu involved there.
> > > > 
> > > I see. And this hypervisor actually coded up looking up
> > > translations in the SMMU unconditionally for legacy devices,
> > > and this worked as long as guest didn't touch the SMMU?
> > 
> > Well, the fastmodel isn't a hypervisor really. It's a full system emulation,
> > so it's better to think of it like a piece of hardware. For example, you
> > could run KVM on the fastmodel. But yes, when Linux didn't swizzle the
> > DMA ops to point at the SMMU, then everything defaults to bypass (because
> > that's the default behaviour of the SMMU driver -- this is configurable
> > on the command line) which is why things used to work.
> > 
> I would be a bit happier if Linux checked virtio iommu quirk and skipped
> the DMA ops thing then. It's a bit ugly but at least it's consistently
> ugly.  To get clean emulation you would then use a modern device.

Sorry, but I'm not sure I follow your suggestion here. If you're talking
about the DMA ops themselves, they are assigned in arch_setup_dma_ops long
before the virtio layer gets involved, so we really can't figure out
whether the device has a virtio iommu quirk at that point.

I've reworked the patch (see below) so that we unconditionally set the
DMA ops for arm-based legacy devices, but I can't really tell what you're
after here.

Will

--->8

>From 213bad7fdb8e4f45a7724be169cda292bbb50d2b Mon Sep 17 00:00:00 2001
From: Will Deacon <will.deacon@arm.com>
Date: Tue, 20 Dec 2016 12:12:49 +0000
Subject: [PATCH] vring: Force use of DMA API for ARM-based systems with legacy
 devices

Booting Linux on an ARM fastmodel containing an SMMU emulation results
in an unexpected I/O page fault from the legacy virtio-blk PCI device:

[    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
[    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
[    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
[    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
[    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
[    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
[    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
[    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
[    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
[    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000

<system hangs failing to read partition table>

This is because the legacy virtio-blk device is behind an SMMU, so we
have consequently swizzled its DMA ops and configured the SMMU to
translate accesses. This then requires the vring code to use the DMA API
to establish translations, otherwise all transactions will result in
fatal faults and termination.

Given that ARM-based systems only see an SMMU if one is really present
(the topology is all described by firmware tables such as device-tree or
IORT), then we can safely use the DMA API for all legacy virtio devices.
Modern devices can advertise the prescense of an IOMMU using the
VIRTIO_F_IOMMU_PLATFORM feature flag.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: <stable@vger.kernel.org>
Fixes: 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 drivers/virtio/virtio_ring.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 409aeaa49246..7e38ed79c3fc 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -159,6 +159,13 @@ 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);
+
 	return false;
 }
 
-- 
2.1.4

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Marc Zyngier]

On 20/01/17 10:33, Will Deacon wrote:
> On Thu, Jan 19, 2017 at 11:51:06PM +0200, Michael S. Tsirkin wrote:
>> On Mon, Jan 16, 2017 at 02:34:08PM +0000, Will Deacon wrote:
>>> On Mon, Jan 16, 2017 at 04:27:28PM +0200, Michael S. Tsirkin wrote:
>>>> On Mon, Jan 16, 2017 at 02:21:03PM +0000, Will Deacon wrote:
>>>>> On Mon, Jan 16, 2017 at 04:18:03PM +0200, Michael S. Tsirkin wrote:
>>>>>> On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
>>>>>>> On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
>>>>>>>> On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
>>>>>>>>> On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
>>>>>>>>>> On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
>>>>>>>>>>> On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
>>>>>>>>>>>> I'd rather people didn't use SMMU with legacy devices.
>>>>>>>>>>>
>>>>>>>>>>> I'm afraid we've been doing that for two years and the model already
>>>>>>>>>>> exists in a mature state, being actively used for development and
>>>>>>>>>>> validation by ARM and our partners. One of the big things its used for
>>>>>>>>>>> is to develop SMMU and GIC (our interrupt controller) code with PCI, so
>>>>>>>>>>> dropping the SMMU from the picture isn't an option.
>>>>>>>>>>
>>>>>>>>>> Oh so this fixes a regression?  This is something I didn't realize.
>>>>>>>>>
>>>>>>>>> Yes, thanks. The regression came about because we implemented SMMU-backed
>>>>>>>>> DMA ops and only then was it apparent that the virtio stuff was bypassing
>>>>>>>>> even with translation enabled (because it wasn't using the DMA API).
>>>>>>>>
>>>>>>>> Could you point out a commit ID?
>>>>>>>
>>>>>>> There has been a fair amount of work in this area recently, but you're
>>>>>>> probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
>>>>>>> as the culprit, which is the point at which we started to swizzle DMA
>>>>>>> ops for devices upstream of an SMMU automatically.
>>>>>>>
>>>>>>>>>> A "Fixes:" tag can't hurt here.  I then wonder
>>>>>>>>>> might DMA ops ever use a DMA address which isn't a physical address
>>>>>>>>>> from QEMU point of view? If that happens, this hack breaks
>>>>>>>>>> because in legacy mode QEMU still uses the GPA.
>>>>>>>>>
>>>>>>>>> If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
>>>>>>>>> because we won't swizzle the DMA ops for the master device. If QEMU does
>>>>>>>>> advertise an SMMU, then we'll allocate DMA addresses to fit within the
>>>>>>>>> the intersection of the SMMU aperture and device's DMA mask.
>>>>>>>>
>>>>>>>>
>>>>>>>> Right but doesn't just poking from qemu into phys addresses work
>>>>>>>> anymore? It used to ...
>>>>>>>
>>>>>>> Provided that there's no SMMU, then it will continue to work. and my
>>>>>>> understanding (from talking to Peter Maydell) is that qemu doesn't model
>>>>>>> an SMMU for ARM-based machines.
>>>>>>>
>>>>>>
>>>>>> So how come people report failures due to presence of SMMU?
>>>>>> Using some other hypervisor?
>>>>>
>>>>> The failures are reported on the ARM fastmodel (a complete system
>>>>> emulation that runs on an x86 box), where an SMMU *is* present
>>>>> downstream of the virtio-pci masters. There's no qemu involved there.
>>>>>
>>>> I see. And this hypervisor actually coded up looking up
>>>> translations in the SMMU unconditionally for legacy devices,
>>>> and this worked as long as guest didn't touch the SMMU?
>>>
>>> Well, the fastmodel isn't a hypervisor really. It's a full system emulation,
>>> so it's better to think of it like a piece of hardware. For example, you
>>> could run KVM on the fastmodel. But yes, when Linux didn't swizzle the
>>> DMA ops to point at the SMMU, then everything defaults to bypass (because
>>> that's the default behaviour of the SMMU driver -- this is configurable
>>> on the command line) which is why things used to work.
>>>
>> I would be a bit happier if Linux checked virtio iommu quirk and skipped
>> the DMA ops thing then. It's a bit ugly but at least it's consistently
>> ugly.  To get clean emulation you would then use a modern device.
> 
> Sorry, but I'm not sure I follow your suggestion here. If you're talking
> about the DMA ops themselves, they are assigned in arch_setup_dma_ops long
> before the virtio layer gets involved, so we really can't figure out
> whether the device has a virtio iommu quirk at that point.
> 
> I've reworked the patch (see below) so that we unconditionally set the
> DMA ops for arm-based legacy devices, but I can't really tell what you're
> after here.
> 
> Will
> 
> --->8
> 
> From 213bad7fdb8e4f45a7724be169cda292bbb50d2b Mon Sep 17 00:00:00 2001
> From: Will Deacon <will.deacon@arm.com>
> Date: Tue, 20 Dec 2016 12:12:49 +0000
> Subject: [PATCH] vring: Force use of DMA API for ARM-based systems with legacy
>  devices
> 
> Booting Linux on an ARM fastmodel containing an SMMU emulation results
> in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> 
> [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> 
> <system hangs failing to read partition table>
> 
> This is because the legacy virtio-blk device is behind an SMMU, so we
> have consequently swizzled its DMA ops and configured the SMMU to
> translate accesses. This then requires the vring code to use the DMA API
> to establish translations, otherwise all transactions will result in
> fatal faults and termination.
> 
> Given that ARM-based systems only see an SMMU if one is really present
> (the topology is all described by firmware tables such as device-tree or
> IORT), then we can safely use the DMA API for all legacy virtio devices.
> Modern devices can advertise the prescense of an IOMMU using the
> VIRTIO_F_IOMMU_PLATFORM feature flag.
> 
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: <stable@vger.kernel.org>
> Fixes: 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  drivers/virtio/virtio_ring.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 409aeaa49246..7e38ed79c3fc 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -159,6 +159,13 @@ 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);
> +
>  	return false;
>  }
>  
> 

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

Any chance this fix (or anything with similar effects) gets applied
sometime soon? I cannot use the model without using a similar workaround:

http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm64/gicv4-wip&id=622ff1190890c0ae60d57e76a7c2f3e6fb27e25d

and I suspect that other users of the same system are carrying their own
version of the fix. Something in mainline would be infinitely better.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Tue, Jan 24, 2017 at 04:04:11PM +0000, Marc Zyngier wrote:
> On 20/01/17 10:33, Will Deacon wrote:
> > On Thu, Jan 19, 2017 at 11:51:06PM +0200, Michael S. Tsirkin wrote:
> >> On Mon, Jan 16, 2017 at 02:34:08PM +0000, Will Deacon wrote:
> >>> On Mon, Jan 16, 2017 at 04:27:28PM +0200, Michael S. Tsirkin wrote:
> >>>> On Mon, Jan 16, 2017 at 02:21:03PM +0000, Will Deacon wrote:
> >>>>> On Mon, Jan 16, 2017 at 04:18:03PM +0200, Michael S. Tsirkin wrote:
> >>>>>> On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
> >>>>>>> On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> >>>>>>>> On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> >>>>>>>>> On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> >>>>>>>>>> On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> >>>>>>>>>>> On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> >>>>>>>>>>>> I'd rather people didn't use SMMU with legacy devices.
> >>>>>>>>>>>
> >>>>>>>>>>> I'm afraid we've been doing that for two years and the model already
> >>>>>>>>>>> exists in a mature state, being actively used for development and
> >>>>>>>>>>> validation by ARM and our partners. One of the big things its used for
> >>>>>>>>>>> is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> >>>>>>>>>>> dropping the SMMU from the picture isn't an option.
> >>>>>>>>>>
> >>>>>>>>>> Oh so this fixes a regression?  This is something I didn't realize.
> >>>>>>>>>
> >>>>>>>>> Yes, thanks. The regression came about because we implemented SMMU-backed
> >>>>>>>>> DMA ops and only then was it apparent that the virtio stuff was bypassing
> >>>>>>>>> even with translation enabled (because it wasn't using the DMA API).
> >>>>>>>>
> >>>>>>>> Could you point out a commit ID?
> >>>>>>>
> >>>>>>> There has been a fair amount of work in this area recently, but you're
> >>>>>>> probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> >>>>>>> as the culprit, which is the point at which we started to swizzle DMA
> >>>>>>> ops for devices upstream of an SMMU automatically.
> >>>>>>>
> >>>>>>>>>> A "Fixes:" tag can't hurt here.  I then wonder
> >>>>>>>>>> might DMA ops ever use a DMA address which isn't a physical address
> >>>>>>>>>> from QEMU point of view? If that happens, this hack breaks
> >>>>>>>>>> because in legacy mode QEMU still uses the GPA.
> >>>>>>>>>
> >>>>>>>>> If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> >>>>>>>>> because we won't swizzle the DMA ops for the master device. If QEMU does
> >>>>>>>>> advertise an SMMU, then we'll allocate DMA addresses to fit within the
> >>>>>>>>> the intersection of the SMMU aperture and device's DMA mask.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Right but doesn't just poking from qemu into phys addresses work
> >>>>>>>> anymore? It used to ...
> >>>>>>>
> >>>>>>> Provided that there's no SMMU, then it will continue to work. and my
> >>>>>>> understanding (from talking to Peter Maydell) is that qemu doesn't model
> >>>>>>> an SMMU for ARM-based machines.
> >>>>>>>
> >>>>>>
> >>>>>> So how come people report failures due to presence of SMMU?
> >>>>>> Using some other hypervisor?
> >>>>>
> >>>>> The failures are reported on the ARM fastmodel (a complete system
> >>>>> emulation that runs on an x86 box), where an SMMU *is* present
> >>>>> downstream of the virtio-pci masters. There's no qemu involved there.
> >>>>>
> >>>> I see. And this hypervisor actually coded up looking up
> >>>> translations in the SMMU unconditionally for legacy devices,
> >>>> and this worked as long as guest didn't touch the SMMU?
> >>>
> >>> Well, the fastmodel isn't a hypervisor really. It's a full system emulation,
> >>> so it's better to think of it like a piece of hardware. For example, you
> >>> could run KVM on the fastmodel. But yes, when Linux didn't swizzle the
> >>> DMA ops to point at the SMMU, then everything defaults to bypass (because
> >>> that's the default behaviour of the SMMU driver -- this is configurable
> >>> on the command line) which is why things used to work.
> >>>
> >> I would be a bit happier if Linux checked virtio iommu quirk and skipped
> >> the DMA ops thing then. It's a bit ugly but at least it's consistently
> >> ugly.  To get clean emulation you would then use a modern device.
> > 
> > Sorry, but I'm not sure I follow your suggestion here. If you're talking
> > about the DMA ops themselves, they are assigned in arch_setup_dma_ops long
> > before the virtio layer gets involved, so we really can't figure out
> > whether the device has a virtio iommu quirk at that point.
> > 
> > I've reworked the patch (see below) so that we unconditionally set the
> > DMA ops for arm-based legacy devices, but I can't really tell what you're
> > after here.
> > 
> > Will
> > 
> > --->8
> > 
> > From 213bad7fdb8e4f45a7724be169cda292bbb50d2b Mon Sep 17 00:00:00 2001
> > From: Will Deacon <will.deacon@arm.com>
> > Date: Tue, 20 Dec 2016 12:12:49 +0000
> > Subject: [PATCH] vring: Force use of DMA API for ARM-based systems with legacy
> >  devices
> > 
> > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> > 
> > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> > [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> > [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > 
> > <system hangs failing to read partition table>
> > 
> > This is because the legacy virtio-blk device is behind an SMMU, so we
> > have consequently swizzled its DMA ops and configured the SMMU to
> > translate accesses. This then requires the vring code to use the DMA API
> > to establish translations, otherwise all transactions will result in
> > fatal faults and termination.
> > 
> > Given that ARM-based systems only see an SMMU if one is really present
> > (the topology is all described by firmware tables such as device-tree or
> > IORT), then we can safely use the DMA API for all legacy virtio devices.
> > Modern devices can advertise the prescense of an IOMMU using the
> > VIRTIO_F_IOMMU_PLATFORM feature flag.
> > 
> > Cc: Andy Lutomirski <luto@kernel.org>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: <stable@vger.kernel.org>
> > Fixes: 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >  drivers/virtio/virtio_ring.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index 409aeaa49246..7e38ed79c3fc 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -159,6 +159,13 @@ 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);
> > +
> >  	return false;
> >  }
> >  
> > 
> 
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> 
> Any chance this fix (or anything with similar effects) gets applied
> sometime soon? I cannot use the model without using a similar
> workaround:
> 
> http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm64/gicv4-wip&id=622ff1190890c0ae60d57e76a7c2f3e6fb27e25d
> 
> and I suspect that other users of the same system are carrying their own
> version of the fix. Something in mainline would be infinitely better.
> 
> Thanks,
> 
> 	M.

I'll merge this in the next pull.

> -- 
> Jazz is not dead. It just smells funny...

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Michael S. Tsirkin]

On Tue, Jan 24, 2017 at 04:04:11PM +0000, Marc Zyngier wrote:
> On 20/01/17 10:33, Will Deacon wrote:
> > On Thu, Jan 19, 2017 at 11:51:06PM +0200, Michael S. Tsirkin wrote:
> >> On Mon, Jan 16, 2017 at 02:34:08PM +0000, Will Deacon wrote:
> >>> On Mon, Jan 16, 2017 at 04:27:28PM +0200, Michael S. Tsirkin wrote:
> >>>> On Mon, Jan 16, 2017 at 02:21:03PM +0000, Will Deacon wrote:
> >>>>> On Mon, Jan 16, 2017 at 04:18:03PM +0200, Michael S. Tsirkin wrote:
> >>>>>> On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
> >>>>>>> On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
> >>>>>>>> On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
> >>>>>>>>> On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
> >>>>>>>>>> On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
> >>>>>>>>>>> On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
> >>>>>>>>>>>> I'd rather people didn't use SMMU with legacy devices.
> >>>>>>>>>>>
> >>>>>>>>>>> I'm afraid we've been doing that for two years and the model already
> >>>>>>>>>>> exists in a mature state, being actively used for development and
> >>>>>>>>>>> validation by ARM and our partners. One of the big things its used for
> >>>>>>>>>>> is to develop SMMU and GIC (our interrupt controller) code with PCI, so
> >>>>>>>>>>> dropping the SMMU from the picture isn't an option.
> >>>>>>>>>>
> >>>>>>>>>> Oh so this fixes a regression?  This is something I didn't realize.
> >>>>>>>>>
> >>>>>>>>> Yes, thanks. The regression came about because we implemented SMMU-backed
> >>>>>>>>> DMA ops and only then was it apparent that the virtio stuff was bypassing
> >>>>>>>>> even with translation enabled (because it wasn't using the DMA API).
> >>>>>>>>
> >>>>>>>> Could you point out a commit ID?
> >>>>>>>
> >>>>>>> There has been a fair amount of work in this area recently, but you're
> >>>>>>> probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> >>>>>>> as the culprit, which is the point at which we started to swizzle DMA
> >>>>>>> ops for devices upstream of an SMMU automatically.
> >>>>>>>
> >>>>>>>>>> A "Fixes:" tag can't hurt here.  I then wonder
> >>>>>>>>>> might DMA ops ever use a DMA address which isn't a physical address
> >>>>>>>>>> from QEMU point of view? If that happens, this hack breaks
> >>>>>>>>>> because in legacy mode QEMU still uses the GPA.
> >>>>>>>>>
> >>>>>>>>> If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
> >>>>>>>>> because we won't swizzle the DMA ops for the master device. If QEMU does
> >>>>>>>>> advertise an SMMU, then we'll allocate DMA addresses to fit within the
> >>>>>>>>> the intersection of the SMMU aperture and device's DMA mask.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Right but doesn't just poking from qemu into phys addresses work
> >>>>>>>> anymore? It used to ...
> >>>>>>>
> >>>>>>> Provided that there's no SMMU, then it will continue to work. and my
> >>>>>>> understanding (from talking to Peter Maydell) is that qemu doesn't model
> >>>>>>> an SMMU for ARM-based machines.
> >>>>>>>
> >>>>>>
> >>>>>> So how come people report failures due to presence of SMMU?
> >>>>>> Using some other hypervisor?
> >>>>>
> >>>>> The failures are reported on the ARM fastmodel (a complete system
> >>>>> emulation that runs on an x86 box), where an SMMU *is* present
> >>>>> downstream of the virtio-pci masters. There's no qemu involved there.
> >>>>>
> >>>> I see. And this hypervisor actually coded up looking up
> >>>> translations in the SMMU unconditionally for legacy devices,
> >>>> and this worked as long as guest didn't touch the SMMU?
> >>>
> >>> Well, the fastmodel isn't a hypervisor really. It's a full system emulation,
> >>> so it's better to think of it like a piece of hardware. For example, you
> >>> could run KVM on the fastmodel. But yes, when Linux didn't swizzle the
> >>> DMA ops to point at the SMMU, then everything defaults to bypass (because
> >>> that's the default behaviour of the SMMU driver -- this is configurable
> >>> on the command line) which is why things used to work.
> >>>
> >> I would be a bit happier if Linux checked virtio iommu quirk and skipped
> >> the DMA ops thing then. It's a bit ugly but at least it's consistently
> >> ugly.  To get clean emulation you would then use a modern device.
> > 
> > Sorry, but I'm not sure I follow your suggestion here. If you're talking
> > about the DMA ops themselves, they are assigned in arch_setup_dma_ops long
> > before the virtio layer gets involved, so we really can't figure out
> > whether the device has a virtio iommu quirk at that point.
> > 
> > I've reworked the patch (see below) so that we unconditionally set the
> > DMA ops for arm-based legacy devices, but I can't really tell what you're
> > after here.
> > 
> > Will
> > 
> > --->8
> > 
> > From 213bad7fdb8e4f45a7724be169cda292bbb50d2b Mon Sep 17 00:00:00 2001
> > From: Will Deacon <will.deacon@arm.com>
> > Date: Tue, 20 Dec 2016 12:12:49 +0000
> > Subject: [PATCH] vring: Force use of DMA API for ARM-based systems with legacy
> >  devices
> > 
> > Booting Linux on an ARM fastmodel containing an SMMU emulation results
> > in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> > 
> > [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> > [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> > [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> > [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> > [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> > [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> > 
> > <system hangs failing to read partition table>
> > 
> > This is because the legacy virtio-blk device is behind an SMMU, so we
> > have consequently swizzled its DMA ops and configured the SMMU to
> > translate accesses. This then requires the vring code to use the DMA API
> > to establish translations, otherwise all transactions will result in
> > fatal faults and termination.
> > 
> > Given that ARM-based systems only see an SMMU if one is really present
> > (the topology is all described by firmware tables such as device-tree or
> > IORT), then we can safely use the DMA API for all legacy virtio devices.
> > Modern devices can advertise the prescense of an IOMMU using the
> > VIRTIO_F_IOMMU_PLATFORM feature flag.
> > 
> > Cc: Andy Lutomirski <luto@kernel.org>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: <stable@vger.kernel.org>
> > Fixes: 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >  drivers/virtio/virtio_ring.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index 409aeaa49246..7e38ed79c3fc 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -159,6 +159,13 @@ 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);
> > +
> >  	return false;
> >  }
> >  
> > 
> 
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> 
> Any chance this fix (or anything with similar effects) gets applied
> sometime soon? I cannot use the model without using a similar
> workaround:
> 
> http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm64/gicv4-wip&id=622ff1190890c0ae60d57e76a7c2f3e6fb27e25d
> 
> and I suspect that other users of the same system are carrying their own
> version of the fix. Something in mainline would be infinitely better.
> 
> Thanks,
> 
> 	M.

I'll merge this in the next pull.

> -- 
> Jazz is not dead. It just smells funny...

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Marc Zyngier]

On 24/01/17 16:06, Michael S. Tsirkin wrote:
> On Tue, Jan 24, 2017 at 04:04:11PM +0000, Marc Zyngier wrote:
>> On 20/01/17 10:33, Will Deacon wrote:
>>> From 213bad7fdb8e4f45a7724be169cda292bbb50d2b Mon Sep 17 00:00:00 2001
>>> From: Will Deacon <will.deacon@arm.com>
>>> Date: Tue, 20 Dec 2016 12:12:49 +0000
>>> Subject: [PATCH] vring: Force use of DMA API for ARM-based systems with legacy
>>>  devices
>>>
>>> Booting Linux on an ARM fastmodel containing an SMMU emulation results
>>> in an unexpected I/O page fault from the legacy virtio-blk PCI device:
>>>
>>> [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
>>> [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
>>> [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
>>> [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
>>> [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
>>> [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
>>> [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
>>> [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
>>> [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
>>> [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
>>>
>>> <system hangs failing to read partition table>
>>>
>>> This is because the legacy virtio-blk device is behind an SMMU, so we
>>> have consequently swizzled its DMA ops and configured the SMMU to
>>> translate accesses. This then requires the vring code to use the DMA API
>>> to establish translations, otherwise all transactions will result in
>>> fatal faults and termination.
>>>
>>> Given that ARM-based systems only see an SMMU if one is really present
>>> (the topology is all described by firmware tables such as device-tree or
>>> IORT), then we can safely use the DMA API for all legacy virtio devices.
>>> Modern devices can advertise the prescense of an IOMMU using the
>>> VIRTIO_F_IOMMU_PLATFORM feature flag.
>>>
>>> Cc: Andy Lutomirski <luto@kernel.org>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Cc: <stable@vger.kernel.org>
>>> Fixes: 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
>>> Signed-off-by: Will Deacon <will.deacon@arm.com>
>>> ---
>>>  drivers/virtio/virtio_ring.c | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>> index 409aeaa49246..7e38ed79c3fc 100644
>>> --- a/drivers/virtio/virtio_ring.c
>>> +++ b/drivers/virtio/virtio_ring.c
>>> @@ -159,6 +159,13 @@ 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);
>>> +
>>>  	return false;
>>>  }
>>>  
>>>
>>
>> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>>
>> Any chance this fix (or anything with similar effects) gets applied
>> sometime soon? I cannot use the model without using a similar
>> workaround:
>>
>> http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm64/gicv4-wip&id=622ff1190890c0ae60d57e76a7c2f3e6fb27e25d
>>
>> and I suspect that other users of the same system are carrying their own
>> version of the fix. Something in mainline would be infinitely better.
>>
>> Thanks,
>>
>> 	M.
> 
> I'll merge this in the next pull.

Awesome, thanks a lot.

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Marc Zyngier]

On 24/01/17 16:06, Michael S. Tsirkin wrote:
> On Tue, Jan 24, 2017 at 04:04:11PM +0000, Marc Zyngier wrote:
>> On 20/01/17 10:33, Will Deacon wrote:
>>> From 213bad7fdb8e4f45a7724be169cda292bbb50d2b Mon Sep 17 00:00:00 2001
>>> From: Will Deacon <will.deacon@arm.com>
>>> Date: Tue, 20 Dec 2016 12:12:49 +0000
>>> Subject: [PATCH] vring: Force use of DMA API for ARM-based systems with legacy
>>>  devices
>>>
>>> Booting Linux on an ARM fastmodel containing an SMMU emulation results
>>> in an unexpected I/O page fault from the legacy virtio-blk PCI device:
>>>
>>> [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
>>> [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
>>> [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
>>> [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
>>> [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
>>> [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
>>> [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
>>> [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
>>> [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
>>> [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
>>>
>>> <system hangs failing to read partition table>
>>>
>>> This is because the legacy virtio-blk device is behind an SMMU, so we
>>> have consequently swizzled its DMA ops and configured the SMMU to
>>> translate accesses. This then requires the vring code to use the DMA API
>>> to establish translations, otherwise all transactions will result in
>>> fatal faults and termination.
>>>
>>> Given that ARM-based systems only see an SMMU if one is really present
>>> (the topology is all described by firmware tables such as device-tree or
>>> IORT), then we can safely use the DMA API for all legacy virtio devices.
>>> Modern devices can advertise the prescense of an IOMMU using the
>>> VIRTIO_F_IOMMU_PLATFORM feature flag.
>>>
>>> Cc: Andy Lutomirski <luto@kernel.org>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Cc: <stable@vger.kernel.org>
>>> Fixes: 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
>>> Signed-off-by: Will Deacon <will.deacon@arm.com>
>>> ---
>>>  drivers/virtio/virtio_ring.c | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>> index 409aeaa49246..7e38ed79c3fc 100644
>>> --- a/drivers/virtio/virtio_ring.c
>>> +++ b/drivers/virtio/virtio_ring.c
>>> @@ -159,6 +159,13 @@ 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);
>>> +
>>>  	return false;
>>>  }
>>>  
>>>
>>
>> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>>
>> Any chance this fix (or anything with similar effects) gets applied
>> sometime soon? I cannot use the model without using a similar
>> workaround:
>>
>> http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm64/gicv4-wip&id=622ff1190890c0ae60d57e76a7c2f3e6fb27e25d
>>
>> and I suspect that other users of the same system are carrying their own
>> version of the fix. Something in mainline would be infinitely better.
>>
>> Thanks,
>>
>> 	M.
> 
> I'll merge this in the next pull.

Awesome, thanks a lot.

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2 2/2] vring: Force use of DMA API for ARM-based systems

[Marc Zyngier]

On 20/01/17 10:33, Will Deacon wrote:
> On Thu, Jan 19, 2017 at 11:51:06PM +0200, Michael S. Tsirkin wrote:
>> On Mon, Jan 16, 2017 at 02:34:08PM +0000, Will Deacon wrote:
>>> On Mon, Jan 16, 2017 at 04:27:28PM +0200, Michael S. Tsirkin wrote:
>>>> On Mon, Jan 16, 2017 at 02:21:03PM +0000, Will Deacon wrote:
>>>>> On Mon, Jan 16, 2017 at 04:18:03PM +0200, Michael S. Tsirkin wrote:
>>>>>> On Mon, Jan 16, 2017 at 10:40:28AM +0000, Will Deacon wrote:
>>>>>>> On Fri, Jan 13, 2017 at 08:23:35PM +0200, Michael S. Tsirkin wrote:
>>>>>>>> On Fri, Jan 13, 2017 at 05:21:54PM +0000, Will Deacon wrote:
>>>>>>>>> On Fri, Jan 13, 2017 at 06:46:32PM +0200, Michael S. Tsirkin wrote:
>>>>>>>>>> On Fri, Jan 13, 2017 at 09:25:22AM +0000, Will Deacon wrote:
>>>>>>>>>>> On Fri, Jan 13, 2017 at 12:12:56AM +0200, Michael S. Tsirkin wrote:
>>>>>>>>>>>> I'd rather people didn't use SMMU with legacy devices.
>>>>>>>>>>>
>>>>>>>>>>> I'm afraid we've been doing that for two years and the model already
>>>>>>>>>>> exists in a mature state, being actively used for development and
>>>>>>>>>>> validation by ARM and our partners. One of the big things its used for
>>>>>>>>>>> is to develop SMMU and GIC (our interrupt controller) code with PCI, so
>>>>>>>>>>> dropping the SMMU from the picture isn't an option.
>>>>>>>>>>
>>>>>>>>>> Oh so this fixes a regression?  This is something I didn't realize.
>>>>>>>>>
>>>>>>>>> Yes, thanks. The regression came about because we implemented SMMU-backed
>>>>>>>>> DMA ops and only then was it apparent that the virtio stuff was bypassing
>>>>>>>>> even with translation enabled (because it wasn't using the DMA API).
>>>>>>>>
>>>>>>>> Could you point out a commit ID?
>>>>>>>
>>>>>>> There has been a fair amount of work in this area recently, but you're
>>>>>>> probably after something like 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
>>>>>>> as the culprit, which is the point at which we started to swizzle DMA
>>>>>>> ops for devices upstream of an SMMU automatically.
>>>>>>>
>>>>>>>>>> A "Fixes:" tag can't hurt here.  I then wonder
>>>>>>>>>> might DMA ops ever use a DMA address which isn't a physical address
>>>>>>>>>> from QEMU point of view? If that happens, this hack breaks
>>>>>>>>>> because in legacy mode QEMU still uses the GPA.
>>>>>>>>>
>>>>>>>>> If QEMU doesn't advertise an SMMU, then it will work fine with the GPA,
>>>>>>>>> because we won't swizzle the DMA ops for the master device. If QEMU does
>>>>>>>>> advertise an SMMU, then we'll allocate DMA addresses to fit within the
>>>>>>>>> the intersection of the SMMU aperture and device's DMA mask.
>>>>>>>>
>>>>>>>>
>>>>>>>> Right but doesn't just poking from qemu into phys addresses work
>>>>>>>> anymore? It used to ...
>>>>>>>
>>>>>>> Provided that there's no SMMU, then it will continue to work. and my
>>>>>>> understanding (from talking to Peter Maydell) is that qemu doesn't model
>>>>>>> an SMMU for ARM-based machines.
>>>>>>>
>>>>>>
>>>>>> So how come people report failures due to presence of SMMU?
>>>>>> Using some other hypervisor?
>>>>>
>>>>> The failures are reported on the ARM fastmodel (a complete system
>>>>> emulation that runs on an x86 box), where an SMMU *is* present
>>>>> downstream of the virtio-pci masters. There's no qemu involved there.
>>>>>
>>>> I see. And this hypervisor actually coded up looking up
>>>> translations in the SMMU unconditionally for legacy devices,
>>>> and this worked as long as guest didn't touch the SMMU?
>>>
>>> Well, the fastmodel isn't a hypervisor really. It's a full system emulation,
>>> so it's better to think of it like a piece of hardware. For example, you
>>> could run KVM on the fastmodel. But yes, when Linux didn't swizzle the
>>> DMA ops to point at the SMMU, then everything defaults to bypass (because
>>> that's the default behaviour of the SMMU driver -- this is configurable
>>> on the command line) which is why things used to work.
>>>
>> I would be a bit happier if Linux checked virtio iommu quirk and skipped
>> the DMA ops thing then. It's a bit ugly but at least it's consistently
>> ugly.  To get clean emulation you would then use a modern device.
> 
> Sorry, but I'm not sure I follow your suggestion here. If you're talking
> about the DMA ops themselves, they are assigned in arch_setup_dma_ops long
> before the virtio layer gets involved, so we really can't figure out
> whether the device has a virtio iommu quirk at that point.
> 
> I've reworked the patch (see below) so that we unconditionally set the
> DMA ops for arm-based legacy devices, but I can't really tell what you're
> after here.
> 
> Will
> 
> --->8
> 
> From 213bad7fdb8e4f45a7724be169cda292bbb50d2b Mon Sep 17 00:00:00 2001
> From: Will Deacon <will.deacon@arm.com>
> Date: Tue, 20 Dec 2016 12:12:49 +0000
> Subject: [PATCH] vring: Force use of DMA API for ARM-based systems with legacy
>  devices
> 
> Booting Linux on an ARM fastmodel containing an SMMU emulation results
> in an unexpected I/O page fault from the legacy virtio-blk PCI device:
> 
> [    1.211721] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> [    1.211800] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> [    1.211880] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> [    1.211959] arm-smmu-v3 2b400000.smmu:	0x00000008fa081002
> [    1.212075] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> [    1.212155] arm-smmu-v3 2b400000.smmu: event 0x10 received:
> [    1.212234] arm-smmu-v3 2b400000.smmu:	0x00000000fffff010
> [    1.212314] arm-smmu-v3 2b400000.smmu:	0x0000020800000000
> [    1.212394] arm-smmu-v3 2b400000.smmu:	0x00000008fa081000
> [    1.212471] arm-smmu-v3 2b400000.smmu:	0x0000000000000000
> 
> <system hangs failing to read partition table>
> 
> This is because the legacy virtio-blk device is behind an SMMU, so we
> have consequently swizzled its DMA ops and configured the SMMU to
> translate accesses. This then requires the vring code to use the DMA API
> to establish translations, otherwise all transactions will result in
> fatal faults and termination.
> 
> Given that ARM-based systems only see an SMMU if one is really present
> (the topology is all described by firmware tables such as device-tree or
> IORT), then we can safely use the DMA API for all legacy virtio devices.
> Modern devices can advertise the prescense of an IOMMU using the
> VIRTIO_F_IOMMU_PLATFORM feature flag.
> 
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: <stable@vger.kernel.org>
> Fixes: 876945dbf649 ("arm64: Hook up IOMMU dma_ops")
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  drivers/virtio/virtio_ring.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 409aeaa49246..7e38ed79c3fc 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -159,6 +159,13 @@ 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);
> +
>  	return false;
>  }
>  
> 

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

Any chance this fix (or anything with similar effects) gets applied
sometime soon? I cannot use the model without using a similar workaround:

http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm64/gicv4-wip&id=622ff1190890c0ae60d57e76a7c2f3e6fb27e25d

and I suspect that other users of the same system are carrying their own
version of the fix. Something in mainline would be infinitely better.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2 1/2] virtio_mmio: Set DMA masks appropriately

[Michael S. Tsirkin]

On Tue, Jan 10, 2017 at 05:51:17PM +0000, Robin Murphy wrote:
> Once DMA API usage is enabled, it becomes apparent that virtio-mmio is
> inadvertently relying on the default 32-bit DMA mask, which leads to
> problems like rapidly exhausting SWIOTLB bounce buffers.
> 
> Ensure that we set the appropriate 64-bit DMA mask whenever possible,
> with the coherent mask suitably limited for the legacy vring as per
> a0be1db4304f ("virtio_pci: Limit DMA mask to 44 bits for legacy virtio
> devices").
> 
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Reported-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> Fixes: b42111382f0e ("virtio_mmio: Use the DMA API if enabled")
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

I'mm merge this soon.

> ---
>  drivers/virtio/virtio_mmio.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index d47a2fcef818..c71fde5fe835 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -59,6 +59,7 @@
>  #define pr_fmt(fmt) "virtio-mmio: " fmt
>  
>  #include <linux/acpi.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/highmem.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -498,6 +499,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>  	struct virtio_mmio_device *vm_dev;
>  	struct resource *mem;
>  	unsigned long magic;
> +	int rc;
>  
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!mem)
> @@ -547,9 +549,25 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>  	}
>  	vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID);
>  
> -	if (vm_dev->version == 1)
> +	if (vm_dev->version == 1) {
>  		writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
>  
> +		rc = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
> +		/*
> +		 * In the legacy case, ensure our coherently-allocated virtio
> +		 * ring will be at an address expressable as a 32-bit PFN.
> +		 */
> +		if (!rc)
> +			dma_set_coherent_mask(&pdev->dev,
> +					      DMA_BIT_MASK(32 + PAGE_SHIFT));
> +	} else {
> +		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> +	}
> +	if (rc)
> +		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +	if (rc)
> +		dev_warn(&pdev->dev, "Failed to enable 64-bit or 32-bit DMA.  Trying to continue, but this might not work.\n");
> +
>  	platform_set_drvdata(pdev, vm_dev);
>  
>  	return register_virtio_device(&vm_dev->vdev);
> -- 
> 2.10.2.dirty

[PATCH v2 1/2] virtio_mmio: Set DMA masks appropriately

[Michael S. Tsirkin]

On Tue, Jan 10, 2017 at 05:51:17PM +0000, Robin Murphy wrote:
> Once DMA API usage is enabled, it becomes apparent that virtio-mmio is
> inadvertently relying on the default 32-bit DMA mask, which leads to
> problems like rapidly exhausting SWIOTLB bounce buffers.
> 
> Ensure that we set the appropriate 64-bit DMA mask whenever possible,
> with the coherent mask suitably limited for the legacy vring as per
> a0be1db4304f ("virtio_pci: Limit DMA mask to 44 bits for legacy virtio
> devices").
> 
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Reported-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> Fixes: b42111382f0e ("virtio_mmio: Use the DMA API if enabled")
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

I'mm merge this soon.

> ---
>  drivers/virtio/virtio_mmio.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index d47a2fcef818..c71fde5fe835 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -59,6 +59,7 @@
>  #define pr_fmt(fmt) "virtio-mmio: " fmt
>  
>  #include <linux/acpi.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/highmem.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -498,6 +499,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>  	struct virtio_mmio_device *vm_dev;
>  	struct resource *mem;
>  	unsigned long magic;
> +	int rc;
>  
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!mem)
> @@ -547,9 +549,25 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>  	}
>  	vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID);
>  
> -	if (vm_dev->version == 1)
> +	if (vm_dev->version == 1) {
>  		writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
>  
> +		rc = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
> +		/*
> +		 * In the legacy case, ensure our coherently-allocated virtio
> +		 * ring will be at an address expressable as a 32-bit PFN.
> +		 */
> +		if (!rc)
> +			dma_set_coherent_mask(&pdev->dev,
> +					      DMA_BIT_MASK(32 + PAGE_SHIFT));
> +	} else {
> +		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> +	}
> +	if (rc)
> +		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +	if (rc)
> +		dev_warn(&pdev->dev, "Failed to enable 64-bit or 32-bit DMA.  Trying to continue, but this might not work.\n");
> +
>  	platform_set_drvdata(pdev, vm_dev);
>  
>  	return register_virtio_device(&vm_dev->vdev);
> -- 
> 2.10.2.dirty

Re: [PATCH v2 1/2] virtio_mmio: Set DMA masks appropriately

[Jean-Philippe Brucker]

On 10/01/17 17:51, Robin Murphy wrote:
> Once DMA API usage is enabled, it becomes apparent that virtio-mmio is
> inadvertently relying on the default 32-bit DMA mask, which leads to
> problems like rapidly exhausting SWIOTLB bounce buffers.
> 
> Ensure that we set the appropriate 64-bit DMA mask whenever possible,
> with the coherent mask suitably limited for the legacy vring as per
> a0be1db4304f ("virtio_pci: Limit DMA mask to 44 bits for legacy virtio
> devices").

Thanks, and for what it's worth:

Tested-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

> 
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Reported-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> Fixes: b42111382f0e ("virtio_mmio: Use the DMA API if enabled")
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  drivers/virtio/virtio_mmio.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index d47a2fcef818..c71fde5fe835 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -59,6 +59,7 @@
>  #define pr_fmt(fmt) "virtio-mmio: " fmt
>  
>  #include <linux/acpi.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/highmem.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -498,6 +499,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>  	struct virtio_mmio_device *vm_dev;
>  	struct resource *mem;
>  	unsigned long magic;
> +	int rc;
>  
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!mem)
> @@ -547,9 +549,25 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>  	}
>  	vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID);
>  
> -	if (vm_dev->version == 1)
> +	if (vm_dev->version == 1) {
>  		writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
>  
> +		rc = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
> +		/*
> +		 * In the legacy case, ensure our coherently-allocated virtio
> +		 * ring will be at an address expressable as a 32-bit PFN.
> +		 */
> +		if (!rc)
> +			dma_set_coherent_mask(&pdev->dev,
> +					      DMA_BIT_MASK(32 + PAGE_SHIFT));
> +	} else {
> +		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> +	}
> +	if (rc)
> +		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +	if (rc)
> +		dev_warn(&pdev->dev, "Failed to enable 64-bit or 32-bit DMA.  Trying to continue, but this might not work.\n");
> +
>  	platform_set_drvdata(pdev, vm_dev);
>  
>  	return register_virtio_device(&vm_dev->vdev);
> 

[PATCH v2 1/2] virtio_mmio: Set DMA masks appropriately

[Jean-Philippe Brucker]

On 10/01/17 17:51, Robin Murphy wrote:
> Once DMA API usage is enabled, it becomes apparent that virtio-mmio is
> inadvertently relying on the default 32-bit DMA mask, which leads to
> problems like rapidly exhausting SWIOTLB bounce buffers.
> 
> Ensure that we set the appropriate 64-bit DMA mask whenever possible,
> with the coherent mask suitably limited for the legacy vring as per
> a0be1db4304f ("virtio_pci: Limit DMA mask to 44 bits for legacy virtio
> devices").

Thanks, and for what it's worth:

Tested-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

> 
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Reported-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> Fixes: b42111382f0e ("virtio_mmio: Use the DMA API if enabled")
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  drivers/virtio/virtio_mmio.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index d47a2fcef818..c71fde5fe835 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -59,6 +59,7 @@
>  #define pr_fmt(fmt) "virtio-mmio: " fmt
>  
>  #include <linux/acpi.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/highmem.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -498,6 +499,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>  	struct virtio_mmio_device *vm_dev;
>  	struct resource *mem;
>  	unsigned long magic;
> +	int rc;
>  
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!mem)
> @@ -547,9 +549,25 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>  	}
>  	vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID);
>  
> -	if (vm_dev->version == 1)
> +	if (vm_dev->version == 1) {
>  		writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
>  
> +		rc = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
> +		/*
> +		 * In the legacy case, ensure our coherently-allocated virtio
> +		 * ring will be at an address expressable as a 32-bit PFN.
> +		 */
> +		if (!rc)
> +			dma_set_coherent_mask(&pdev->dev,
> +					      DMA_BIT_MASK(32 + PAGE_SHIFT));
> +	} else {
> +		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> +	}
> +	if (rc)
> +		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +	if (rc)
> +		dev_warn(&pdev->dev, "Failed to enable 64-bit or 32-bit DMA.  Trying to continue, but this might not work.\n");
> +
>  	platform_set_drvdata(pdev, vm_dev);
>  
>  	return register_virtio_device(&vm_dev->vdev);
>