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 <a...@ozlabs.ru>
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?
Thanks
Eric
[1] lspci
Region 3: Memory at 104e0000 (32-bit, non-prefetchable) [size=16K]
Expansion ROM at 10480000 [disabled] [size=256K]
../..
Capabilities: [a0] MSI-X: Enable+ Count=5 Masked-
Vector table: BAR=3 offset=00000000
PBA: BAR=3 offset=00002000
[2] info mtree (final)
100e0000-00000000100effff (prio 0, i/o): 0004:01:00.0 base BAR 3
100e0000-00000000100e004f (prio 0, i/o): msix-table
100e0000-00000000100effff (prio 0, i/o): 0004:01:00.0 BAR 3
100e0000-00000000100effff (prio 0, ramd): 0004:01:00.0 BAR 3 mmaps[0]
100e2000-00000000100e2007 (prio 0, i/o): msix-pba [disabled]
100e4000-00000000100e4fff (prio 1, i/o): virtio-scsi-pci-msix
00000000100e4000-00000000100e403f (prio 0, i/o): msix-table
00000000100e4800-00000000100e4807 (prio 0, i/o): msix-pba
> ---
>
> Message on slightly hacked QEMU (iommu pagesize=8K) looks now like this:
>
> qemu-system-x86_64: Region "0000:00:1a.0 BAR 0 mmaps[0]" 0xfebc0000
> size=0x1000 is not aligned to 0x2000 and cannot be mapped for DMA
> ---
> hw/vfio/common.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 5e84716..e2db596 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -548,10 +548,11 @@ static void vfio_listener_region_add(MemoryListener
> *listener,
> hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
>
> if ((iova & pgmask) || (int128_get64(llsize) & pgmask)) {
> - error_report("Region 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx
> + error_report("Region \"%s\" 0x%"HWADDR_PRIx" size=0x%"HWADDR_PRIx
> " is not aligned to 0x%"HWADDR_PRIx
> " and cannot be mapped for DMA",
> - section->offset_within_region,
> + memory_region_name(section->mr),
> + section->offset_within_address_space,
> int128_getlo(section->size),
> pgmask + 1);
> return;
>
