* [Qemu-devel] [PATCH qemu v4] RFC: vfio-pci: Allow mmap of MSIX BAR
@ 2018-01-25 4:22 Alexey Kardashevskiy
2018-01-25 13:56 ` Auger Eric
0 siblings, 1 reply; 5+ messages in thread
From: Alexey Kardashevskiy @ 2018-01-25 4:22 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, qemu-ppc, David Gibson, Alex Williamson,
Auger Eric
This makes use of a new VFIO_REGION_INFO_CAP_MSIX_MAPPABLE capability
which tells that a region with MSIX data can be mapped entirely, i.e.
the VFIO PCI driver won't prevent MSIX vectors area from being mapped.
With this change, all BARs are mapped in a single chunk and MSIX vectors
are emulated on top unless the machine requests not to by defining and
enabling a new "vfio-no-msix-emulation" property. At the moment only
sPAPR machine does so - it prohibits MSIX emulation and does not allow
enabling it as it does not define the "set" callback for the new property;
the new property also does not appear in "-machine pseries,help".
If MSIX vectors section is not aligned to the page size, the KVM memory
listener does not register it with the KVM as a memory slot and MSIX is
emulated by QEMU as before. This may create MMIO RAM memory sections with
an address or/and a size not aligned which will make vfio_dma_map() fail;
to address this, this treats such failures as non-fatal and does not print
an error. This adds a return value from vfio_dma_map() to the tracepoint
and moves the latter further down to let the user have a clue why P2P
might not work.
This requires the kernel change - "vfio-pci: Allow mapping MSIX BAR" -
for the new capability: https://www.spinics.net/lists/kvm/msg160282.html
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v4:
* silenced dma map errors if unaligned mapping is attempted - they are going
to fail anyway
v3:
* vfio_listener_region_add() won't make qemu exit if failed on MMIO MR
---
include/hw/vfio/vfio-common.h | 1 +
| 5 +++++
hw/ppc/spapr.c | 7 +++++++
hw/vfio/common.c | 27 +++++++++++++++++++++++++--
hw/vfio/pci.c | 10 ++++++++++
hw/vfio/trace-events | 2 +-
6 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index f3a2ac9..927d600 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -171,6 +171,7 @@ int vfio_get_region_info(VFIODevice *vbasedev, int index,
struct vfio_region_info **info);
int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
uint32_t subtype, struct vfio_region_info **info);
+bool vfio_is_cap_present(VFIODevice *vbasedev, uint16_t cap_type, int region);
#endif
extern const MemoryListener vfio_prereg_listener;
--git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index 4312e96..b45182e 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -301,6 +301,11 @@ struct vfio_region_info_cap_type {
#define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG (2)
#define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG (3)
+/*
+ * The MSIX mappable capability informs that MSIX data of a BAR can be mmapped.
+ */
+#define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE 3
+
/**
* VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
* struct vfio_irq_info)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d1acfe8..5ff43ce 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2789,6 +2789,11 @@ static void spapr_set_modern_hotplug_events(Object *obj, bool value,
spapr->use_hotplug_event_source = value;
}
+static bool spapr_get_msix_emulation(Object *obj, Error **errp)
+{
+ return true;
+}
+
static char *spapr_get_resize_hpt(Object *obj, Error **errp)
{
sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
@@ -2870,6 +2875,8 @@ static void spapr_instance_init(Object *obj)
object_property_set_description(obj, "vsmt",
"Virtual SMT: KVM behaves as if this were"
" the host's SMT mode", &error_abort);
+ object_property_add_bool(obj, "vfio-no-msix-emulation",
+ spapr_get_msix_emulation, NULL, NULL);
}
static void spapr_machine_finalizefn(Object *obj)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 3d652c8..04a8280 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -513,13 +513,21 @@ static void vfio_listener_region_add(MemoryListener *listener,
section->offset_within_region +
(iova - section->offset_within_address_space);
- trace_vfio_listener_region_add_ram(iova, end, vaddr);
-
llsize = int128_sub(llend, int128_make64(iova));
ret = vfio_dma_map(container, iova, int128_get64(llsize),
vaddr, section->readonly);
+ trace_vfio_listener_region_add_ram(iova, end, vaddr, ret);
+
if (ret) {
+ hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
+
+ if (memory_region_is_ram_device(section->mr) &&
+ ((section->offset_within_region & pgmask) ||
+ (int128_getlo(section->size) & pgmask))) {
+ return;
+ }
+
error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
"0x%"HWADDR_PRIx", %p) = %d (%m)",
container, iova, int128_get64(llsize), vaddr, ret);
@@ -1386,6 +1394,21 @@ int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
return -ENODEV;
}
+bool vfio_is_cap_present(VFIODevice *vbasedev, uint16_t cap_type, int region)
+{
+ struct vfio_region_info *info = NULL;
+ bool ret = false;
+
+ if (!vfio_get_region_info(vbasedev, region, &info)) {
+ if (vfio_get_region_info_cap(info, cap_type)) {
+ ret = true;
+ }
+ g_free(info);
+ }
+
+ return ret;
+}
+
/*
* Interfaces for IBM EEH (Enhanced Error Handling)
*/
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 359a8f1..a96ece6 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1289,6 +1289,11 @@ static void vfio_pci_fixup_msix_region(VFIOPCIDevice *vdev)
off_t start, end;
VFIORegion *region = &vdev->bars[vdev->msix->table_bar].region;
+ if (vfio_is_cap_present(&vdev->vbasedev, VFIO_REGION_INFO_CAP_MSIX_MAPPABLE,
+ vdev->msix->table_bar)) {
+ return;
+ }
+
/*
* We expect to find a single mmap covering the whole BAR, anything else
* means it's either unsupported or already setup.
@@ -1569,6 +1574,11 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
*/
memory_region_set_enabled(&vdev->pdev.msix_pba_mmio, false);
+ if (object_property_get_bool(OBJECT(qdev_get_machine()),
+ "vfio-no-msix-emulation", NULL)) {
+ memory_region_set_enabled(&vdev->pdev.msix_table_mmio, false);
+ }
+
return 0;
}
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 437ccdd..d747c11 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -89,7 +89,7 @@ vfio_region_read(char *name, int index, uint64_t addr, unsigned size, uint64_t d
vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t iova_end) "iommu %s @ 0x%"PRIx64" - 0x%"PRIx64
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_ram(uint64_t iova_start, uint64_t iova_end, void *vaddr, int ret) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p] ret=%d"
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.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v4] RFC: vfio-pci: Allow mmap of MSIX BAR
2018-01-25 4:22 [Qemu-devel] [PATCH qemu v4] RFC: vfio-pci: Allow mmap of MSIX BAR Alexey Kardashevskiy
@ 2018-01-25 13:56 ` Auger Eric
2018-01-25 14:30 ` Auger Eric
2018-01-29 3:55 ` Alexey Kardashevskiy
0 siblings, 2 replies; 5+ messages in thread
From: Auger Eric @ 2018-01-25 13:56 UTC (permalink / raw)
To: Alexey Kardashevskiy, qemu-devel; +Cc: Alex Williamson, qemu-ppc, David Gibson
Hi Alexey,
On 25/01/18 05:22, Alexey Kardashevskiy wrote:
> This makes use of a new VFIO_REGION_INFO_CAP_MSIX_MAPPABLE capability
> which tells that a region with MSIX data can be mapped entirely, i.e.
> the VFIO PCI driver won't prevent MSIX vectors area from being mapped.
>
> With this change, all BARs are mapped in a single chunk and MSIX vectors
> are emulated on top unless the machine requests not to by defining and
> enabling a new "vfio-no-msix-emulation" property. At the moment only
> sPAPR machine does so - it prohibits MSIX emulation and does not allow
> enabling it as it does not define the "set" callback for the new property;
> the new property also does not appear in "-machine pseries,help".
>
> If MSIX vectors section is not aligned to the page size, the KVM memory
> listener does not register it with the KVM as a memory slot and MSIX is
> emulated by QEMU as before. This may create MMIO RAM memory sections with
> an address or/and a size not aligned which will make vfio_dma_map() fail;
> to address this, this treats such failures as non-fatal and does not print
> an error. This adds a return value from vfio_dma_map() to the tracepoint
> and moves the latter further down to let the user have a clue why P2P
> might not work.
>
> This requires the kernel change - "vfio-pci: Allow mapping MSIX BAR" -
> for the new capability: https://www.spinics.net/lists/kvm/msg160282.html
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v4:
> * silenced dma map errors if unaligned mapping is attempted - they are going
> to fail anyway
>
> v3:
> * vfio_listener_region_add() won't make qemu exit if failed on MMIO MR
> ---
> include/hw/vfio/vfio-common.h | 1 +
> linux-headers/linux/vfio.h | 5 +++++
> hw/ppc/spapr.c | 7 +++++++
> hw/vfio/common.c | 27 +++++++++++++++++++++++++--
> hw/vfio/pci.c | 10 ++++++++++
> hw/vfio/trace-events | 2 +-
> 6 files changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index f3a2ac9..927d600 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -171,6 +171,7 @@ int vfio_get_region_info(VFIODevice *vbasedev, int index,
> struct vfio_region_info **info);
> int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
> uint32_t subtype, struct vfio_region_info **info);
> +bool vfio_is_cap_present(VFIODevice *vbasedev, uint16_t cap_type, int region);
> #endif
> extern const MemoryListener vfio_prereg_listener;
>
> diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
> index 4312e96..b45182e 100644
> --- a/linux-headers/linux/vfio.h
> +++ b/linux-headers/linux/vfio.h
> @@ -301,6 +301,11 @@ struct vfio_region_info_cap_type {
> #define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG (2)
> #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG (3)
>
> +/*
> + * The MSIX mappable capability informs that MSIX data of a BAR can be mmapped.
> + */
> +#define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE 3
> +
> /**
> * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
> * struct vfio_irq_info)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d1acfe8..5ff43ce 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2789,6 +2789,11 @@ static void spapr_set_modern_hotplug_events(Object *obj, bool value,
> spapr->use_hotplug_event_source = value;
> }
>
> +static bool spapr_get_msix_emulation(Object *obj, Error **errp)
> +{
> + return true;
> +}
> +
> static char *spapr_get_resize_hpt(Object *obj, Error **errp)
> {
> sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
> @@ -2870,6 +2875,8 @@ static void spapr_instance_init(Object *obj)
> object_property_set_description(obj, "vsmt",
> "Virtual SMT: KVM behaves as if this were"
> " the host's SMT mode", &error_abort);
> + object_property_add_bool(obj, "vfio-no-msix-emulation",
> + spapr_get_msix_emulation, NULL, NULL);
> }
>
> static void spapr_machine_finalizefn(Object *obj)
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 3d652c8..04a8280 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -513,13 +513,21 @@ static void vfio_listener_region_add(MemoryListener *listener,
> section->offset_within_region +
> (iova - section->offset_within_address_space);
>
> - trace_vfio_listener_region_add_ram(iova, end, vaddr);
> -
> llsize = int128_sub(llend, int128_make64(iova));
>
> ret = vfio_dma_map(container, iova, int128_get64(llsize),
> vaddr, section->readonly);
> + trace_vfio_listener_region_add_ram(iova, end, vaddr, ret);
> +
> if (ret) {
> + hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
> +
> + if (memory_region_is_ram_device(section->mr) &&
> + ((section->offset_within_region & pgmask) ||
> + (int128_getlo(section->size) & pgmask))) {
> + return;
> + }
> +
Why not avoiding to call vfio_dma_map in this case and print a helpful
message for the end-user?
qemu-system-aarch64: VFIO_UNMAP_DMA: -22
qemu-system-aarch64: vfio_dma_unmap(0xc0dab90, 0x8000000000, 0x2000) =
-22 (Invalid argument)
is not self-explanatory I think.
Thanks
Eric
> error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
> "0x%"HWADDR_PRIx", %p) = %d (%m)",
> container, iova, int128_get64(llsize), vaddr, ret);
> @@ -1386,6 +1394,21 @@ int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
> return -ENODEV;
> }
>
> +bool vfio_is_cap_present(VFIODevice *vbasedev, uint16_t cap_type, int region)
> +{
> + struct vfio_region_info *info = NULL;
> + bool ret = false;
> +
> + if (!vfio_get_region_info(vbasedev, region, &info)) {
> + if (vfio_get_region_info_cap(info, cap_type)) {
> + ret = true;
> + }
> + g_free(info);
> + }
> +
> + return ret;
> +}
> +
> /*
> * Interfaces for IBM EEH (Enhanced Error Handling)
> */
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 359a8f1..a96ece6 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1289,6 +1289,11 @@ static void vfio_pci_fixup_msix_region(VFIOPCIDevice *vdev)
> off_t start, end;
> VFIORegion *region = &vdev->bars[vdev->msix->table_bar].region;
>
> + if (vfio_is_cap_present(&vdev->vbasedev, VFIO_REGION_INFO_CAP_MSIX_MAPPABLE,
> + vdev->msix->table_bar)) {
> + return;
> + }
> +
> /*
> * We expect to find a single mmap covering the whole BAR, anything else
> * means it's either unsupported or already setup.
> @@ -1569,6 +1574,11 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
> */
> memory_region_set_enabled(&vdev->pdev.msix_pba_mmio, false);
>
> + if (object_property_get_bool(OBJECT(qdev_get_machine()),
> + "vfio-no-msix-emulation", NULL)) {
> + memory_region_set_enabled(&vdev->pdev.msix_table_mmio, false);
> + }
> +
> return 0;
> }
>
> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
> index 437ccdd..d747c11 100644
> --- a/hw/vfio/trace-events
> +++ b/hw/vfio/trace-events
> @@ -89,7 +89,7 @@ vfio_region_read(char *name, int index, uint64_t addr, unsigned size, uint64_t d
> vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t iova_end) "iommu %s @ 0x%"PRIx64" - 0x%"PRIx64
> 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_ram(uint64_t iova_start, uint64_t iova_end, void *vaddr, int ret) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p] ret=%d"
> 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] 5+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v4] RFC: vfio-pci: Allow mmap of MSIX BAR
2018-01-25 13:56 ` Auger Eric
@ 2018-01-25 14:30 ` Auger Eric
2018-01-29 3:55 ` Alexey Kardashevskiy
1 sibling, 0 replies; 5+ messages in thread
From: Auger Eric @ 2018-01-25 14:30 UTC (permalink / raw)
To: Alexey Kardashevskiy, qemu-devel; +Cc: Alex Williamson, qemu-ppc, David Gibson
Hi,
On 25/01/18 14:56, Auger Eric wrote:
> Hi Alexey,
>
> On 25/01/18 05:22, Alexey Kardashevskiy wrote:
>> This makes use of a new VFIO_REGION_INFO_CAP_MSIX_MAPPABLE capability
>> which tells that a region with MSIX data can be mapped entirely, i.e.
>> the VFIO PCI driver won't prevent MSIX vectors area from being mapped.
>>
>> With this change, all BARs are mapped in a single chunk and MSIX vectors
>> are emulated on top unless the machine requests not to by defining and
>> enabling a new "vfio-no-msix-emulation" property. At the moment only
>> sPAPR machine does so - it prohibits MSIX emulation and does not allow
>> enabling it as it does not define the "set" callback for the new property;
>> the new property also does not appear in "-machine pseries,help".
>>
>> If MSIX vectors section is not aligned to the page size, the KVM memory
>> listener does not register it with the KVM as a memory slot and MSIX is
>> emulated by QEMU as before. This may create MMIO RAM memory sections with
>> an address or/and a size not aligned which will make vfio_dma_map() fail;
>> to address this, this treats such failures as non-fatal and does not print
>> an error. This adds a return value from vfio_dma_map() to the tracepoint
>> and moves the latter further down to let the user have a clue why P2P
>> might not work.
>>
>> This requires the kernel change - "vfio-pci: Allow mapping MSIX BAR" -
>> for the new capability: https://www.spinics.net/lists/kvm/msg160282.html
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> Changes:
>> v4:
>> * silenced dma map errors if unaligned mapping is attempted - they are going
>> to fail anyway
>>
>> v3:
>> * vfio_listener_region_add() won't make qemu exit if failed on MMIO MR
>> ---
>> include/hw/vfio/vfio-common.h | 1 +
>> linux-headers/linux/vfio.h | 5 +++++
>> hw/ppc/spapr.c | 7 +++++++
>> hw/vfio/common.c | 27 +++++++++++++++++++++++++--
>> hw/vfio/pci.c | 10 ++++++++++
>> hw/vfio/trace-events | 2 +-
>> 6 files changed, 49 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>> index f3a2ac9..927d600 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -171,6 +171,7 @@ int vfio_get_region_info(VFIODevice *vbasedev, int index,
>> struct vfio_region_info **info);
>> int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
>> uint32_t subtype, struct vfio_region_info **info);
>> +bool vfio_is_cap_present(VFIODevice *vbasedev, uint16_t cap_type, int region);
>> #endif
>> extern const MemoryListener vfio_prereg_listener;
>>
>> diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
>> index 4312e96..b45182e 100644
>> --- a/linux-headers/linux/vfio.h
>> +++ b/linux-headers/linux/vfio.h
>> @@ -301,6 +301,11 @@ struct vfio_region_info_cap_type {
>> #define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG (2)
>> #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG (3)
>>
>> +/*
>> + * The MSIX mappable capability informs that MSIX data of a BAR can be mmapped.
>> + */
>> +#define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE 3
>> +
>> /**
>> * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
>> * struct vfio_irq_info)
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index d1acfe8..5ff43ce 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -2789,6 +2789,11 @@ static void spapr_set_modern_hotplug_events(Object *obj, bool value,
>> spapr->use_hotplug_event_source = value;
>> }
>>
>> +static bool spapr_get_msix_emulation(Object *obj, Error **errp)
>> +{
>> + return true;
>> +}
>> +
>> static char *spapr_get_resize_hpt(Object *obj, Error **errp)
>> {
>> sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
>> @@ -2870,6 +2875,8 @@ static void spapr_instance_init(Object *obj)
>> object_property_set_description(obj, "vsmt",
>> "Virtual SMT: KVM behaves as if this were"
>> " the host's SMT mode", &error_abort);
>> + object_property_add_bool(obj, "vfio-no-msix-emulation",
>> + spapr_get_msix_emulation, NULL, NULL);
>> }
>>
>> static void spapr_machine_finalizefn(Object *obj)
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 3d652c8..04a8280 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -513,13 +513,21 @@ static void vfio_listener_region_add(MemoryListener *listener,
>> section->offset_within_region +
>> (iova - section->offset_within_address_space);
>>
>> - trace_vfio_listener_region_add_ram(iova, end, vaddr);
>> -
>> llsize = int128_sub(llend, int128_make64(iova));
>>
>> ret = vfio_dma_map(container, iova, int128_get64(llsize),
>> vaddr, section->readonly);
>> + trace_vfio_listener_region_add_ram(iova, end, vaddr, ret);
>> +
>> if (ret) {
>> + hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
>> +
>> + if (memory_region_is_ram_device(section->mr) &&
>> + ((section->offset_within_region & pgmask) ||
>> + (int128_getlo(section->size) & pgmask))) {
>> + return;
>> + }
>> +
> Why not avoiding to call vfio_dma_map in this case and print a helpful
> message for the end-user?
>
> qemu-system-aarch64: VFIO_UNMAP_DMA: -22
> qemu-system-aarch64: vfio_dma_unmap(0xc0dab90, 0x8000000000, 0x2000) =
> -22 (Invalid argument)
By the way there is also the unmap path in vfio_listener_region_del ;-)
Thanks
Eric
>
> is not self-explanatory I think.
>
> Thanks
>
> Eric
>> error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
>> "0x%"HWADDR_PRIx", %p) = %d (%m)",
>> container, iova, int128_get64(llsize), vaddr, ret);
>> @@ -1386,6 +1394,21 @@ int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
>> return -ENODEV;
>> }
>>
>> +bool vfio_is_cap_present(VFIODevice *vbasedev, uint16_t cap_type, int region)
>> +{
>> + struct vfio_region_info *info = NULL;
>> + bool ret = false;
>> +
>> + if (!vfio_get_region_info(vbasedev, region, &info)) {
>> + if (vfio_get_region_info_cap(info, cap_type)) {
>> + ret = true;
>> + }
>> + g_free(info);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> /*
>> * Interfaces for IBM EEH (Enhanced Error Handling)
>> */
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 359a8f1..a96ece6 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -1289,6 +1289,11 @@ static void vfio_pci_fixup_msix_region(VFIOPCIDevice *vdev)
>> off_t start, end;
>> VFIORegion *region = &vdev->bars[vdev->msix->table_bar].region;
>>
>> + if (vfio_is_cap_present(&vdev->vbasedev, VFIO_REGION_INFO_CAP_MSIX_MAPPABLE,
>> + vdev->msix->table_bar)) {
>> + return;
>> + }
>> +
>> /*
>> * We expect to find a single mmap covering the whole BAR, anything else
>> * means it's either unsupported or already setup.
>> @@ -1569,6 +1574,11 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
>> */
>> memory_region_set_enabled(&vdev->pdev.msix_pba_mmio, false);
>>
>> + if (object_property_get_bool(OBJECT(qdev_get_machine()),
>> + "vfio-no-msix-emulation", NULL)) {
>> + memory_region_set_enabled(&vdev->pdev.msix_table_mmio, false);
>> + }
>> +
>> return 0;
>> }
>>
>> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
>> index 437ccdd..d747c11 100644
>> --- a/hw/vfio/trace-events
>> +++ b/hw/vfio/trace-events
>> @@ -89,7 +89,7 @@ vfio_region_read(char *name, int index, uint64_t addr, unsigned size, uint64_t d
>> vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t iova_end) "iommu %s @ 0x%"PRIx64" - 0x%"PRIx64
>> 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_ram(uint64_t iova_start, uint64_t iova_end, void *vaddr, int ret) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p] ret=%d"
>> 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] 5+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v4] RFC: vfio-pci: Allow mmap of MSIX BAR
2018-01-25 13:56 ` Auger Eric
2018-01-25 14:30 ` Auger Eric
@ 2018-01-29 3:55 ` Alexey Kardashevskiy
2018-01-29 8:58 ` Auger Eric
1 sibling, 1 reply; 5+ messages in thread
From: Alexey Kardashevskiy @ 2018-01-29 3:55 UTC (permalink / raw)
To: Auger Eric, qemu-devel; +Cc: Alex Williamson, qemu-ppc, David Gibson
On 26/01/18 00:56, Auger Eric wrote:
> Hi Alexey,
>
> On 25/01/18 05:22, Alexey Kardashevskiy wrote:
>> This makes use of a new VFIO_REGION_INFO_CAP_MSIX_MAPPABLE capability
>> which tells that a region with MSIX data can be mapped entirely, i.e.
>> the VFIO PCI driver won't prevent MSIX vectors area from being mapped.
>>
>> With this change, all BARs are mapped in a single chunk and MSIX vectors
>> are emulated on top unless the machine requests not to by defining and
>> enabling a new "vfio-no-msix-emulation" property. At the moment only
>> sPAPR machine does so - it prohibits MSIX emulation and does not allow
>> enabling it as it does not define the "set" callback for the new property;
>> the new property also does not appear in "-machine pseries,help".
>>
>> If MSIX vectors section is not aligned to the page size, the KVM memory
>> listener does not register it with the KVM as a memory slot and MSIX is
>> emulated by QEMU as before. This may create MMIO RAM memory sections with
>> an address or/and a size not aligned which will make vfio_dma_map() fail;
>> to address this, this treats such failures as non-fatal and does not print
>> an error. This adds a return value from vfio_dma_map() to the tracepoint
>> and moves the latter further down to let the user have a clue why P2P
>> might not work.
>>
>> This requires the kernel change - "vfio-pci: Allow mapping MSIX BAR" -
>> for the new capability: https://www.spinics.net/lists/kvm/msg160282.html
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> Changes:
>> v4:
>> * silenced dma map errors if unaligned mapping is attempted - they are going
>> to fail anyway
>>
>> v3:
>> * vfio_listener_region_add() won't make qemu exit if failed on MMIO MR
>> ---
>> include/hw/vfio/vfio-common.h | 1 +
>> linux-headers/linux/vfio.h | 5 +++++
>> hw/ppc/spapr.c | 7 +++++++
>> hw/vfio/common.c | 27 +++++++++++++++++++++++++--
>> hw/vfio/pci.c | 10 ++++++++++
>> hw/vfio/trace-events | 2 +-
>> 6 files changed, 49 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>> index f3a2ac9..927d600 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -171,6 +171,7 @@ int vfio_get_region_info(VFIODevice *vbasedev, int index,
>> struct vfio_region_info **info);
>> int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
>> uint32_t subtype, struct vfio_region_info **info);
>> +bool vfio_is_cap_present(VFIODevice *vbasedev, uint16_t cap_type, int region);
>> #endif
>> extern const MemoryListener vfio_prereg_listener;
>>
>> diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
>> index 4312e96..b45182e 100644
>> --- a/linux-headers/linux/vfio.h
>> +++ b/linux-headers/linux/vfio.h
>> @@ -301,6 +301,11 @@ struct vfio_region_info_cap_type {
>> #define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG (2)
>> #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG (3)
>>
>> +/*
>> + * The MSIX mappable capability informs that MSIX data of a BAR can be mmapped.
>> + */
>> +#define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE 3
>> +
>> /**
>> * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
>> * struct vfio_irq_info)
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index d1acfe8..5ff43ce 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -2789,6 +2789,11 @@ static void spapr_set_modern_hotplug_events(Object *obj, bool value,
>> spapr->use_hotplug_event_source = value;
>> }
>>
>> +static bool spapr_get_msix_emulation(Object *obj, Error **errp)
>> +{
>> + return true;
>> +}
>> +
>> static char *spapr_get_resize_hpt(Object *obj, Error **errp)
>> {
>> sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
>> @@ -2870,6 +2875,8 @@ static void spapr_instance_init(Object *obj)
>> object_property_set_description(obj, "vsmt",
>> "Virtual SMT: KVM behaves as if this were"
>> " the host's SMT mode", &error_abort);
>> + object_property_add_bool(obj, "vfio-no-msix-emulation",
>> + spapr_get_msix_emulation, NULL, NULL);
>> }
>>
>> static void spapr_machine_finalizefn(Object *obj)
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 3d652c8..04a8280 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -513,13 +513,21 @@ static void vfio_listener_region_add(MemoryListener *listener,
>> section->offset_within_region +
>> (iova - section->offset_within_address_space);
>>
>> - trace_vfio_listener_region_add_ram(iova, end, vaddr);
>> -
>> llsize = int128_sub(llend, int128_make64(iova));
>>
>> ret = vfio_dma_map(container, iova, int128_get64(llsize),
>> vaddr, section->readonly);
>> + trace_vfio_listener_region_add_ram(iova, end, vaddr, ret);
>> +
>> if (ret) {
>> + hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
>> +
>> + if (memory_region_is_ram_device(section->mr) &&
>> + ((section->offset_within_region & pgmask) ||
>> + (int128_getlo(section->size) & pgmask))) {
>> + return;
>> + }
>> +
> Why not avoiding to call vfio_dma_map in this case and print a helpful
> message for the end-user?
Avoiding is a good point, but I am not sure about the message though. I am
leaning towards disabling these regions from being dma mapped by default
and have a machine option for it, for example. We do not need this in most
cases but if we do, we actually want to see every failed dma map request. I
am just not sure about that property interface.
> qemu-system-aarch64: VFIO_UNMAP_DMA: -22
Ah, missed this.
> qemu-system-aarch64: vfio_dma_unmap(0xc0dab90, 0x8000000000, 0x2000) =
> -22 (Invalid argument)
>
> is not self-explanatory I think.
What is missing?
>
> Thanks
>
> Eric
>> error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
>> "0x%"HWADDR_PRIx", %p) = %d (%m)",
>> container, iova, int128_get64(llsize), vaddr, ret);
>> @@ -1386,6 +1394,21 @@ int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
>> return -ENODEV;
>> }
>>
>> +bool vfio_is_cap_present(VFIODevice *vbasedev, uint16_t cap_type, int region)
>> +{
>> + struct vfio_region_info *info = NULL;
>> + bool ret = false;
>> +
>> + if (!vfio_get_region_info(vbasedev, region, &info)) {
>> + if (vfio_get_region_info_cap(info, cap_type)) {
>> + ret = true;
>> + }
>> + g_free(info);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> /*
>> * Interfaces for IBM EEH (Enhanced Error Handling)
>> */
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 359a8f1..a96ece6 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -1289,6 +1289,11 @@ static void vfio_pci_fixup_msix_region(VFIOPCIDevice *vdev)
>> off_t start, end;
>> VFIORegion *region = &vdev->bars[vdev->msix->table_bar].region;
>>
>> + if (vfio_is_cap_present(&vdev->vbasedev, VFIO_REGION_INFO_CAP_MSIX_MAPPABLE,
>> + vdev->msix->table_bar)) {
>> + return;
>> + }
>> +
>> /*
>> * We expect to find a single mmap covering the whole BAR, anything else
>> * means it's either unsupported or already setup.
>> @@ -1569,6 +1574,11 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
>> */
>> memory_region_set_enabled(&vdev->pdev.msix_pba_mmio, false);
>>
>> + if (object_property_get_bool(OBJECT(qdev_get_machine()),
>> + "vfio-no-msix-emulation", NULL)) {
>> + memory_region_set_enabled(&vdev->pdev.msix_table_mmio, false);
>> + }
>> +
>> return 0;
>> }
>>
>> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
>> index 437ccdd..d747c11 100644
>> --- a/hw/vfio/trace-events
>> +++ b/hw/vfio/trace-events
>> @@ -89,7 +89,7 @@ vfio_region_read(char *name, int index, uint64_t addr, unsigned size, uint64_t d
>> vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t iova_end) "iommu %s @ 0x%"PRIx64" - 0x%"PRIx64
>> 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_ram(uint64_t iova_start, uint64_t iova_end, void *vaddr, int ret) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p] ret=%d"
>> 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] 5+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v4] RFC: vfio-pci: Allow mmap of MSIX BAR
2018-01-29 3:55 ` Alexey Kardashevskiy
@ 2018-01-29 8:58 ` Auger Eric
0 siblings, 0 replies; 5+ messages in thread
From: Auger Eric @ 2018-01-29 8:58 UTC (permalink / raw)
To: Alexey Kardashevskiy, qemu-devel; +Cc: Alex Williamson, qemu-ppc, David Gibson
Hi Alexey,
On 29/01/18 04:55, Alexey Kardashevskiy wrote:
> On 26/01/18 00:56, Auger Eric wrote:
>> Hi Alexey,
>>
>> On 25/01/18 05:22, Alexey Kardashevskiy wrote:
>>> This makes use of a new VFIO_REGION_INFO_CAP_MSIX_MAPPABLE capability
>>> which tells that a region with MSIX data can be mapped entirely, i.e.
>>> the VFIO PCI driver won't prevent MSIX vectors area from being mapped.
>>>
>>> With this change, all BARs are mapped in a single chunk and MSIX vectors
>>> are emulated on top unless the machine requests not to by defining and
>>> enabling a new "vfio-no-msix-emulation" property. At the moment only
>>> sPAPR machine does so - it prohibits MSIX emulation and does not allow
>>> enabling it as it does not define the "set" callback for the new property;
>>> the new property also does not appear in "-machine pseries,help".
>>>
>>> If MSIX vectors section is not aligned to the page size, the KVM memory
>>> listener does not register it with the KVM as a memory slot and MSIX is
>>> emulated by QEMU as before. This may create MMIO RAM memory sections with
>>> an address or/and a size not aligned which will make vfio_dma_map() fail;
>>> to address this, this treats such failures as non-fatal and does not print
>>> an error. This adds a return value from vfio_dma_map() to the tracepoint
>>> and moves the latter further down to let the user have a clue why P2P
>>> might not work.
>>>
>>> This requires the kernel change - "vfio-pci: Allow mapping MSIX BAR" -
>>> for the new capability: https://www.spinics.net/lists/kvm/msg160282.html
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>> Changes:
>>> v4:
>>> * silenced dma map errors if unaligned mapping is attempted - they are going
>>> to fail anyway
>>>
>>> v3:
>>> * vfio_listener_region_add() won't make qemu exit if failed on MMIO MR
>>> ---
>>> include/hw/vfio/vfio-common.h | 1 +
>>> linux-headers/linux/vfio.h | 5 +++++
>>> hw/ppc/spapr.c | 7 +++++++
>>> hw/vfio/common.c | 27 +++++++++++++++++++++++++--
>>> hw/vfio/pci.c | 10 ++++++++++
>>> hw/vfio/trace-events | 2 +-
>>> 6 files changed, 49 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>>> index f3a2ac9..927d600 100644
>>> --- a/include/hw/vfio/vfio-common.h
>>> +++ b/include/hw/vfio/vfio-common.h
>>> @@ -171,6 +171,7 @@ int vfio_get_region_info(VFIODevice *vbasedev, int index,
>>> struct vfio_region_info **info);
>>> int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
>>> uint32_t subtype, struct vfio_region_info **info);
>>> +bool vfio_is_cap_present(VFIODevice *vbasedev, uint16_t cap_type, int region);
>>> #endif
>>> extern const MemoryListener vfio_prereg_listener;
>>>
>>> diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
>>> index 4312e96..b45182e 100644
>>> --- a/linux-headers/linux/vfio.h
>>> +++ b/linux-headers/linux/vfio.h
>>> @@ -301,6 +301,11 @@ struct vfio_region_info_cap_type {
>>> #define VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG (2)
>>> #define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG (3)
>>>
>>> +/*
>>> + * The MSIX mappable capability informs that MSIX data of a BAR can be mmapped.
>>> + */
>>> +#define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE 3
>>> +
>>> /**
>>> * VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
>>> * struct vfio_irq_info)
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index d1acfe8..5ff43ce 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -2789,6 +2789,11 @@ static void spapr_set_modern_hotplug_events(Object *obj, bool value,
>>> spapr->use_hotplug_event_source = value;
>>> }
>>>
>>> +static bool spapr_get_msix_emulation(Object *obj, Error **errp)
>>> +{
>>> + return true;
>>> +}
>>> +
>>> static char *spapr_get_resize_hpt(Object *obj, Error **errp)
>>> {
>>> sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
>>> @@ -2870,6 +2875,8 @@ static void spapr_instance_init(Object *obj)
>>> object_property_set_description(obj, "vsmt",
>>> "Virtual SMT: KVM behaves as if this were"
>>> " the host's SMT mode", &error_abort);
>>> + object_property_add_bool(obj, "vfio-no-msix-emulation",
>>> + spapr_get_msix_emulation, NULL, NULL);
>>> }
>>>
>>> static void spapr_machine_finalizefn(Object *obj)
>>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>>> index 3d652c8..04a8280 100644
>>> --- a/hw/vfio/common.c
>>> +++ b/hw/vfio/common.c
>>> @@ -513,13 +513,21 @@ static void vfio_listener_region_add(MemoryListener *listener,
>>> section->offset_within_region +
>>> (iova - section->offset_within_address_space);
>>>
>>> - trace_vfio_listener_region_add_ram(iova, end, vaddr);
>>> -
>>> llsize = int128_sub(llend, int128_make64(iova));
>>>
>>> ret = vfio_dma_map(container, iova, int128_get64(llsize),
>>> vaddr, section->readonly);
>>> + trace_vfio_listener_region_add_ram(iova, end, vaddr, ret);
>>> +
>>> if (ret) {
>>> + hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
>>> +
>>> + if (memory_region_is_ram_device(section->mr) &&
>>> + ((section->offset_within_region & pgmask) ||
>>> + (int128_getlo(section->size) & pgmask))) {
>>> + return;
>>> + }
>>> +
>> Why not avoiding to call vfio_dma_map in this case and print a helpful
>> message for the end-user?
>
> Avoiding is a good point, but I am not sure about the message though. I am
> leaning towards disabling these regions from being dma mapped by default
> and have a machine option for it, for example. We do not need this in most
> cases but if we do, we actually want to see every failed dma map request. I
> am just not sure about that property interface.
>
>
>> qemu-system-aarch64: VFIO_UNMAP_DMA: -22
>
>
> Ah, missed this.
>
>
>> qemu-system-aarch64: vfio_dma_unmap(0xc0dab90, 0x8000000000, 0x2000) =
>> -22 (Invalid argument)
>>
>> is not self-explanatory I think.
>
> What is missing?
Would something like:
"Region [start, end] cannot be DMA accessed by device <name>: start/size
misaligned with host iommu page size (<size>). Check the region nature
and size. If part of a BAR, check MSI-X table/PBA and think of
relocating it." make sense?
Thanks
Eric
>
>
>
>
>>
>> Thanks
>>
>> Eric
>>> error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
>>> "0x%"HWADDR_PRIx", %p) = %d (%m)",
>>> container, iova, int128_get64(llsize), vaddr, ret);
>>> @@ -1386,6 +1394,21 @@ int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
>>> return -ENODEV;
>>> }
>>>
>>> +bool vfio_is_cap_present(VFIODevice *vbasedev, uint16_t cap_type, int region)
>>> +{
>>> + struct vfio_region_info *info = NULL;
>>> + bool ret = false;
>>> +
>>> + if (!vfio_get_region_info(vbasedev, region, &info)) {
>>> + if (vfio_get_region_info_cap(info, cap_type)) {
>>> + ret = true;
>>> + }
>>> + g_free(info);
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> /*
>>> * Interfaces for IBM EEH (Enhanced Error Handling)
>>> */
>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>>> index 359a8f1..a96ece6 100644
>>> --- a/hw/vfio/pci.c
>>> +++ b/hw/vfio/pci.c
>>> @@ -1289,6 +1289,11 @@ static void vfio_pci_fixup_msix_region(VFIOPCIDevice *vdev)
>>> off_t start, end;
>>> VFIORegion *region = &vdev->bars[vdev->msix->table_bar].region;
>>>
>>> + if (vfio_is_cap_present(&vdev->vbasedev, VFIO_REGION_INFO_CAP_MSIX_MAPPABLE,
>>> + vdev->msix->table_bar)) {
>>> + return;
>>> + }
>>> +
>>> /*
>>> * We expect to find a single mmap covering the whole BAR, anything else
>>> * means it's either unsupported or already setup.
>>> @@ -1569,6 +1574,11 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
>>> */
>>> memory_region_set_enabled(&vdev->pdev.msix_pba_mmio, false);
>>>
>>> + if (object_property_get_bool(OBJECT(qdev_get_machine()),
>>> + "vfio-no-msix-emulation", NULL)) {
>>> + memory_region_set_enabled(&vdev->pdev.msix_table_mmio, false);
>>> + }
>>> +
>>> return 0;
>>> }
>>>
>>> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
>>> index 437ccdd..d747c11 100644
>>> --- a/hw/vfio/trace-events
>>> +++ b/hw/vfio/trace-events
>>> @@ -89,7 +89,7 @@ vfio_region_read(char *name, int index, uint64_t addr, unsigned size, uint64_t d
>>> vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t iova_end) "iommu %s @ 0x%"PRIx64" - 0x%"PRIx64
>>> 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_ram(uint64_t iova_start, uint64_t iova_end, void *vaddr, int ret) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p] ret=%d"
>>> 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] 5+ messages in thread
end of thread, other threads:[~2018-01-29 8:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-25 4:22 [Qemu-devel] [PATCH qemu v4] RFC: vfio-pci: Allow mmap of MSIX BAR Alexey Kardashevskiy
2018-01-25 13:56 ` Auger Eric
2018-01-25 14:30 ` Auger Eric
2018-01-29 3:55 ` Alexey Kardashevskiy
2018-01-29 8:58 ` 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.