* [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped
@ 2018-04-04 20:30 Eric Auger
2018-04-04 22:47 ` Philippe Mathieu-Daudé
2018-04-04 23:09 ` Alex Williamson
0 siblings, 2 replies; 6+ messages in thread
From: Eric Auger @ 2018-04-04 20:30 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, alex.williamson; +Cc: aik
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 <aik@ozlabs.ru>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
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"
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped
2018-04-04 20:30 [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped Eric Auger
@ 2018-04-04 22:47 ` Philippe Mathieu-Daudé
2018-04-04 23:09 ` Alex Williamson
1 sibling, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-04-04 22:47 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, qemu-devel, alex.williamson; +Cc: aik
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 <aik@ozlabs.ru>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>
> 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"
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped
2018-04-04 20:30 [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped Eric Auger
2018-04-04 22:47 ` Philippe Mathieu-Daudé
@ 2018-04-04 23:09 ` Alex Williamson
2018-04-05 0:53 ` Alexey Kardashevskiy
1 sibling, 1 reply; 6+ messages in thread
From: Alex Williamson @ 2018-04-04 23:09 UTC (permalink / raw)
To: Eric Auger; +Cc: eric.auger.pro, qemu-devel, aik
On Wed, 4 Apr 2018 22:30:50 +0200
Eric Auger <eric.auger@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 <aik@ozlabs.ru>
> Signed-off-by: Eric Auger <eric.auger@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-aik@ozlabs.ru> but this is
substantially different from Alexey's posting so I'd like to verify the
Sign-off. Alexey?
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"
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped
2018-04-04 23:09 ` Alex Williamson
@ 2018-04-05 0:53 ` Alexey Kardashevskiy
2018-04-05 2:17 ` Alex Williamson
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Kardashevskiy @ 2018-04-05 0:53 UTC (permalink / raw)
To: Alex Williamson, Eric Auger; +Cc: eric.auger.pro, qemu-devel
On 5/4/18 9:09 am, Alex Williamson wrote:
> On Wed, 4 Apr 2018 22:30:50 +0200
> Eric Auger <eric.auger@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 <aik@ozlabs.ru>
>> Signed-off-by: Eric Auger <eric.auger@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-aik@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 <aik@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped
2018-04-05 0:53 ` Alexey Kardashevskiy
@ 2018-04-05 2:17 ` Alex Williamson
2018-04-05 6:20 ` Auger Eric
0 siblings, 1 reply; 6+ messages in thread
From: Alex Williamson @ 2018-04-05 2:17 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: Eric Auger, eric.auger.pro, qemu-devel
On Thu, 5 Apr 2018 10:53:38 +1000
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> On 5/4/18 9:09 am, Alex Williamson wrote:
> > On Wed, 4 Apr 2018 22:30:50 +0200
> > Eric Auger <eric.auger@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 <aik@ozlabs.ru>
> >> Signed-off-by: Eric Auger <eric.auger@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-aik@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 <aik@ozlabs.ru>
Yep, I think Eric was intending to give you credit since this patch
builds on your patch. I'll include the R-b.
There's still work to do, I'd like to see a single, concise message as
we're parsing the device, but there's no benefit in alarming users when
nothing has changed and it's too late into the release cycle to do more
than just silence this for now. Patches welcome towards a better
solution, to be considered for 2.13. Thanks,
Alex
> >
> > 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"
> >
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped
2018-04-05 2:17 ` Alex Williamson
@ 2018-04-05 6:20 ` Auger Eric
0 siblings, 0 replies; 6+ messages in thread
From: Auger Eric @ 2018-04-05 6:20 UTC (permalink / raw)
To: Alex Williamson, Alexey Kardashevskiy; +Cc: qemu-devel, eric.auger.pro
Hi,
On 05/04/18 04:17, Alex Williamson wrote:
> On Thu, 5 Apr 2018 10:53:38 +1000
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
>> On 5/4/18 9:09 am, Alex Williamson wrote:
>>> On Wed, 4 Apr 2018 22:30:50 +0200
>>> Eric Auger <eric.auger@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 <aik@ozlabs.ru>
>>>> Signed-off-by: Eric Auger <eric.auger@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-aik@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 <aik@ozlabs.ru>
>
> Yep, I think Eric was intending to give you credit since this patch
> builds on your patch. I'll include the R-b.
Yes this was my intent, sorry for the confusion. I was eager to get this
for 2.12, hence this follow-up.
Thanks
Eric
>
> There's still work to do, I'd like to see a single, concise message as
> we're parsing the device, but there's no benefit in alarming users when
> nothing has changed and it's too late into the release cycle to do more
> than just silence this for now. Patches welcome towards a better
> solution, to be considered for 2.13. Thanks,
>
> Alex
>
>>>
>>> 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"
>>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-04-05 6:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-04 20:30 [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped Eric Auger
2018-04-04 22:47 ` Philippe Mathieu-Daudé
2018-04-04 23:09 ` Alex Williamson
2018-04-05 0:53 ` Alexey Kardashevskiy
2018-04-05 2:17 ` Alex Williamson
2018-04-05 6:20 ` Auger Eric
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.