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

On Tue, Jan 10, 2017 at 05:51:18PM +0000, Robin Murphy
wrote:
> From: Will Deacon <will.deacon at 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 at kernel.org>
> Cc: Michael S. Tsirkin <mst at redhat.com>
> Signed-off-by: Will Deacon <will.deacon at 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

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 at 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 at kernel.org>
> > Cc: Michael S. Tsirkin <mst at redhat.com>
> > Signed-off-by: Will Deacon <will.deacon at 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

On Wed, Jan 11, 2017 at 2:01 AM, Will Deacon <will.deacon at 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 at 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 at kernel.org>
>> > Cc: Michael S. Tsirkin <mst at redhat.com>
>> > Signed-off-by: Will Deacon <will.deacon at 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

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 at 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 at kernel.org>
> > > Cc: Michael S. Tsirkin <mst at redhat.com>
> > > Signed-off-by: Will Deacon <will.deacon at 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