Re: [Qemu-devel] [PATCH qemu] vfio: Print address space address when cannot map MMIO for DMA

[Alex Williamson]

On Wed, 28 Mar 2018 23:03:23 +0200
Auger Eric <address@hidden> wrote:

> Hi Alexey, Alex,
> On 22/03/18 09:18, Alexey Kardashevskiy wrote:
> > The 567b5b309abe ("vfio/pci: Relax DMA map errors for MMIO regions") added
> > an error message if a passed memory section address or size is not aligned
> > to the minimal IOMMU page size. However although it checks
> > offset_within_address_space for the alignment, offset_within_region is
> > printed instead which makes it harder to find out what device caused
> > the message so this replaces offset_within_region with
> > offset_within_address_space.
> > 
> > While we are here, this replaces '..' with 'size=' (as the second number
> > is a size, not an end offset) and adds a memory region name.
> > 
> > Fixes: 567b5b309abe "vfio/pci: Relax DMA map errors for MMIO regions"
> > Signed-off-by: Alexey Kardashevskiy <address@hidden>  
> The patch indeed fixes the reported format issues.
> 
> However I have some other concerns with the info that is reported to the
> end-user. See below.
> 
> Assigning an e1000e device with a 64kB host, here are the traces I get:
> 
> Region XXX is not aligned to 0x10000 and cannot be mapped for DMA
> 
> "0004:01:00.0 BAR 3 mmaps[0]" 0x100a0050 size=0x3fb0
> "0004:01:00.0 BAR 3 mmaps[0]" 0x100a0050 size=0xffb0
> "0004:01:00.0 BAR 3 mmaps[0]" 0x100a0050 size=0x3fb0
> "0004:01:00.0 BAR 3 mmaps[0]" 0x100a0050 size=0xffb0
> "0004:01:00.0 BAR 3 mmaps[0]" 0x100a0050 size=0x3fb0
> "0004:01:00.0 BAR 3 mmaps[0]" 0x100a4808 size=0xb7f8
> "0004:01:00.0 BAR 3 mmaps[0]" 0x100e0050 size=0x3fb0
> "0004:01:00.0 BAR 3 mmaps[0]" 0x100e4808 size=0xb7f8
> 
> It took me some time to understand what happens but here is now my
> understanding:
> 
> 1) When looking at vfio_pci_write_config() pdev->io_regions[bar].addr =
> bar_addr in vfio_sub_page_bar_update_mapping() I see the following values:
> 
> UNMAPPED -> 0x0 ->UNMAPPED -> 0x100a0000 -> UNMAPPED -> 0x100a0000 ->
> UNMAPPED -> 0x100e0000
> 
> vfio_sub_page_bar_update_mapping() create mrs with base bar at
> 0x100a0000 and 0x100e0000 successively, hence the
> vfio_listener_region_add on 0x100axxxx. Indeed, 0x0-0x50 msix-table mmio
> region induces some memory section at 0x100a0050 and 0x100e50 successively.
> 
> However this is confusing for the end-user who only has access to the
> final mapping (0x100e0000) through lspi [1].
> 
> 2) The changes in the size (0x3fb0 <-> 0xffb0) relate to the extension
> of the 16kB bar to 64kB in vfio_sub_page_bar_update_mapping
> 
> 3) Also it happens that I have a virtio-scsi-pci device that is put just
> after the BAR3 at 0x100a4000 and 0x100e4000 successively. The device has
> its own msi-table and pba mmio regions[2]. As mmaps[0] is extended to
> 64kB (with prio 0), we have those MMIO regions which result in new
> memory sections, which cause vfio_listener_region_add calls. This
> typically explains why we get a warning on 0x100e4808 (0xb7f8). By the
> way I don't get why we don't have a trace for "0004:01:00.0 BAR 3
> mmaps[0]" 0x100e4040 size=0x7c0, ie. mmaps[0] space between
> virtio-scsi-pci msic-table and pba.
> 
> So at the end of the day, my fear is all those info become really
> frightening and confusing for the end-user and even not relevant
> (0x100a0000 stuff). So I would rather simply remove the trace in 2.12
> until we find a place where we could generate a clear hint for the
> end-user, suggesting to relocate the msix bar.
> 
> Thoughts?

Yep, I think that's probably the right approach.  Everything works as
it should and nothing has worse access in 2.12 than it did in 2.11,
there's only the opportunity to make things better with msi-x
relocation and I don't think we want to error on the side of reporting
too many errors that users cannot understand in an attempt to advise
them of an unsupported option that might be better.  Let's remove the
error report for 2.12 and think about how we could make a concise
suggestion, once, while initializing the device.  Who's posting the
patch?  Thanks,

Alex

PS - Why isn't the firmware/kernel on aarch64 making an attempt to
align PCI resources on page boundaries?  Does
pci=realloc,resource_alignment=pci:ffffffff:ffffffff change it? (I'm
not sure if PCI_ANY_ID works for that option)