Re: [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped

[Philippe Mathieu-Daudé]

On 04/04/2018 05:30 PM, Eric Auger 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 page size and thus cannot be DMA mapped.
> 
> This patch fixes the trace by printing the region name and the
> memory region section offset within the address space (instead of
> offset_within_region).
> 
> We also turn the error_report into a trace event. Indeed, In some
> cases, the traces can be confusing to non expert end-users and
> let think the use case does not work (whereas it
> works as before).
> 
> This is the case where a BAR is successively mapped at different
> GPAs and its sections are not compatible with dma map. The listener
> is called several times and traces are issued for each intermediate
> mapping.  The end-user cannot easily match those GPAs against the
> final GPA output by lscpi. So let's keep those information to
> informed users. In mid term, the plan is to advise the user about
> BAR relocation relevance.
> 
> Fixes: 567b5b309abe "vfio/pci: Relax DMA map errors for MMIO regions"
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> Signed-off-by: Eric Auger <address@hidden>

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> ---
> 
> v1 -> v2:
> - use a trace point
> - add some details in the commit message
> ---
>  hw/vfio/common.c     | 11 +++++------
>  hw/vfio/trace-events |  1 +
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 5e84716..07ffa0b 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -548,12 +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
> -                         " is not aligned to 0x%"HWADDR_PRIx
> -                         " and cannot be mapped for DMA",
> -                         section->offset_within_region,
> -                         int128_getlo(section->size),
> -                         pgmask + 1);
> +            trace_vfio_listener_region_add_no_dma_map(
> +                memory_region_name(section->mr),
> +                section->offset_within_address_space,
> +                int128_getlo(section->size),
> +                pgmask + 1);
>              return;
>          }
>      }
> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
> index 79f63a2..20109cb 100644
> --- a/hw/vfio/trace-events
> +++ b/hw/vfio/trace-events
> @@ -90,6 +90,7 @@ vfio_iommu_map_notify(const char *op, uint64_t iova_start, 
> uint64_t iova_end) "i
>  vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING 
> region_add 0x%"PRIx64" - 0x%"PRIx64
>  vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add 
> [iommu] 0x%"PRIx64" - 0x%"PRIx64
>  vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void 
> *vaddr) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p]"
> +vfio_listener_region_add_no_dma_map(const char *name, uint64_t iova, 
> uint64_t size, uint64_t page_size) "Region \"%s\" 0x%"PRIx64" 
> size=0x%"PRIx64" is not aligned to 0x%"PRIx64" and cannot be mapped for DMA"
>  vfio_listener_region_del_skip(uint64_t start, uint64_t end) "SKIPPING 
> region_del 0x%"PRIx64" - 0x%"PRIx64
>  vfio_listener_region_del(uint64_t start, uint64_t end) "region_del 
> 0x%"PRIx64" - 0x%"PRIx64
>  vfio_disconnect_container(int fd) "close container->fd=%d"
>