On 5/4/18 9:09 am, Alex Williamson wrote: > On Wed, 4 Apr 2018 22:30:50 +0200 > Eric Auger <eric.au...@redhat.com> 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 <a...@ozlabs.ru> >> Signed-off-by: Eric Auger <eric.au...@redhat.com> >> >> --- >> >> v1 -> v2: >> - use a trace point >> - add some details in the commit message > > I think this is v2 with v1 being > Message-Id:<20180322081837.21460-1-...@ozlabs.ru> but this is > substantially different from Alexey's posting so I'd like to verify the > Sign-off. Alexey?
My understanding it is not "sign-off" any more (I am fine either way, it just does not seem fully correct), but Reviewed-by: Alexey Kardashevskiy <a...@ozlabs.ru> > > I agree that this seems to be the right thing to do for 2.12, nothing > has changed for these mappings and the error reports are difficult even > for experts to decipher and explain to users. Thanks, > > Alex > >> --- >> 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" > -- Alexey