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

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 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.
> > > > > 
> > > > 
> > > > 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

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

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