* [Qemu-devel] [PULL 01/11] memory/iommu: Add get_attr()
2018-02-06 18:42 [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06 Alex Williamson
@ 2018-02-06 18:42 ` Alex Williamson
2018-02-06 18:43 ` [Qemu-devel] [PULL 02/11] vfio/spapr: Use iommu memory region's get_attr() Alex Williamson
` (10 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Alex Williamson @ 2018-02-06 18:42 UTC (permalink / raw)
To: qemu-devel
From: Alexey Kardashevskiy <aik@ozlabs.ru>
This adds get_attr() to IOMMUMemoryRegionClass, like
iommu_ops::domain_get_attr in the Linux kernel.
This defines the first attribute - IOMMU_ATTR_SPAPR_TCE_FD - which
will be used between the pSeries machine and VFIO-PCI.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
include/exec/memory.h | 22 ++++++++++++++++++++++
memory.c | 13 +++++++++++++
2 files changed, 35 insertions(+)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 07c5d6d59796..3ef83991e99a 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -190,6 +190,10 @@ struct MemoryRegionOps {
const MemoryRegionMmio old_mmio;
};
+enum IOMMUMemoryRegionAttr {
+ IOMMU_ATTR_SPAPR_TCE_FD
+};
+
typedef struct IOMMUMemoryRegionClass {
/* private */
struct DeviceClass parent_class;
@@ -210,6 +214,10 @@ typedef struct IOMMUMemoryRegionClass {
IOMMUNotifierFlag new_flags);
/* Set this up to provide customized IOMMU replay function */
void (*replay)(IOMMUMemoryRegion *iommu, IOMMUNotifier *notifier);
+
+ /* Get IOMMU misc attributes */
+ int (*get_attr)(IOMMUMemoryRegion *iommu, enum IOMMUMemoryRegionAttr,
+ void *data);
} IOMMUMemoryRegionClass;
typedef struct CoalescedMemoryRange CoalescedMemoryRange;
@@ -926,6 +934,20 @@ void memory_region_iommu_replay_all(IOMMUMemoryRegion *iommu_mr);
void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
IOMMUNotifier *n);
+/**
+ * memory_region_iommu_get_attr: return an IOMMU attr if get_attr() is
+ * defined on the IOMMU.
+ *
+ * Returns 0 if succeded, error code otherwise.
+ *
+ * @iommu_mr: the memory region
+ * @attr: the requested attribute
+ * @data: a pointer to the requested attribute data
+ */
+int memory_region_iommu_get_attr(IOMMUMemoryRegion *iommu_mr,
+ enum IOMMUMemoryRegionAttr attr,
+ void *data);
+
/**
* memory_region_name: get a memory region's name
*
diff --git a/memory.c b/memory.c
index 449a1429b946..a4c603730a8c 100644
--- a/memory.c
+++ b/memory.c
@@ -1922,6 +1922,19 @@ void memory_region_notify_iommu(IOMMUMemoryRegion *iommu_mr,
}
}
+int memory_region_iommu_get_attr(IOMMUMemoryRegion *iommu_mr,
+ enum IOMMUMemoryRegionAttr attr,
+ void *data)
+{
+ IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_GET_CLASS(iommu_mr);
+
+ if (!imrc->get_attr) {
+ return -EINVAL;
+ }
+
+ return imrc->get_attr(iommu_mr, attr, data);
+}
+
void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
{
uint8_t mask = 1 << client;
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 02/11] vfio/spapr: Use iommu memory region's get_attr()
2018-02-06 18:42 [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06 Alex Williamson
2018-02-06 18:42 ` [Qemu-devel] [PULL 01/11] memory/iommu: Add get_attr() Alex Williamson
@ 2018-02-06 18:43 ` Alex Williamson
2018-02-06 18:43 ` [Qemu-devel] [PULL 03/11] spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device Alex Williamson
` (9 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Alex Williamson @ 2018-02-06 18:43 UTC (permalink / raw)
To: qemu-devel
From: Alexey Kardashevskiy <aik@ozlabs.ru>
In order to enable TCE operations support in KVM, we have to inform
the KVM about VFIO groups being attached to specific LIOBNs. The KVM
already knows about VFIO groups, the only bit missing is which
in-kernel TCE table (the one with user visible TCEs) should update
the attached broups. There is an KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE
attribute of the VFIO KVM device which receives a groupfd/tablefd couple.
This uses a new memory_region_iommu_get_attr() helper to get the IOMMU fd
and calls KVM to establish the link.
As get_attr() is not implemented yet, this should cause no behavioural
change.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
hw/vfio/common.c | 27 +++++++++++++++++++++++++++
hw/vfio/trace-events | 1 +
2 files changed, 28 insertions(+)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index b77be3a8b36b..ec41f93b7479 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -457,6 +457,33 @@ static void vfio_listener_region_add(MemoryListener *listener,
vfio_host_win_add(container, section->offset_within_address_space,
section->offset_within_address_space +
int128_get64(section->size) - 1, pgsize);
+#ifdef CONFIG_KVM
+ if (kvm_enabled()) {
+ VFIOGroup *group;
+ IOMMUMemoryRegion *iommu_mr = IOMMU_MEMORY_REGION(section->mr);
+ struct kvm_vfio_spapr_tce param;
+ struct kvm_device_attr attr = {
+ .group = KVM_DEV_VFIO_GROUP,
+ .attr = KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE,
+ .addr = (uint64_t)(unsigned long)¶m,
+ };
+
+ if (!memory_region_iommu_get_attr(iommu_mr, IOMMU_ATTR_SPAPR_TCE_FD,
+ ¶m.tablefd)) {
+ QLIST_FOREACH(group, &container->group_list, container_next) {
+ param.groupfd = group->fd;
+ if (ioctl(vfio_kvm_device_fd, KVM_SET_DEVICE_ATTR, &attr)) {
+ error_report("vfio: failed to setup fd %d "
+ "for a group with fd %d: %s",
+ param.tablefd, param.groupfd,
+ strerror(errno));
+ return;
+ }
+ trace_vfio_spapr_group_attach(param.groupfd, param.tablefd);
+ }
+ }
+ }
+#endif
}
hostwin_found = false;
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index fae096c0724f..3d34fe8a93f1 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -123,3 +123,4 @@ vfio_prereg_register(uint64_t va, uint64_t size, int ret) "va=0x%"PRIx64" size=0
vfio_prereg_unregister(uint64_t va, uint64_t size, int ret) "va=0x%"PRIx64" size=0x%"PRIx64" ret=%d"
vfio_spapr_create_window(int ps, uint64_t ws, uint64_t off) "pageshift=0x%x winsize=0x%"PRIx64" offset=0x%"PRIx64
vfio_spapr_remove_window(uint64_t off) "offset=0x%"PRIx64
+vfio_spapr_group_attach(int groupfd, int tablefd) "Attached groupfd %d to liobn fd %d"
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 03/11] spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
2018-02-06 18:42 [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06 Alex Williamson
2018-02-06 18:42 ` [Qemu-devel] [PULL 01/11] memory/iommu: Add get_attr() Alex Williamson
2018-02-06 18:43 ` [Qemu-devel] [PULL 02/11] vfio/spapr: Use iommu memory region's get_attr() Alex Williamson
@ 2018-02-06 18:43 ` Alex Williamson
2018-02-06 18:43 ` [Qemu-devel] [PULL 04/11] vfio/pci: Fixup VFIOMSIXInfo comment Alex Williamson
` (8 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Alex Williamson @ 2018-02-06 18:43 UTC (permalink / raw)
To: qemu-devel
From: Alexey Kardashevskiy <aik@ozlabs.ru>
In order to enable TCE operations support in KVM, we have to inform
the KVM about VFIO groups being attached to specific LIOBNs;
the necessary bits are implemented already by IOMMU MR and VFIO.
This defines get_attr() for the SPAPR TCE IOMMU MR which makes VFIO
call the KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE ioctl and establish
LIOBN-to-IOMMU link.
This changes spapr_tce_set_need_vfio() to avoid TCE table reallocation
if the kernel supports the TCE acceleration.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
[aw - remove unnecessary sys/ioctl.h include]
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
hw/ppc/spapr_iommu.c | 18 ++++++++++++++++++
target/ppc/kvm.c | 7 ++++++-
target/ppc/kvm_ppc.h | 6 ++++++
3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index 5ccd785d5a06..aaa6010d5c97 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -160,6 +160,19 @@ static uint64_t spapr_tce_get_min_page_size(IOMMUMemoryRegion *iommu)
return 1ULL << tcet->page_shift;
}
+static int spapr_tce_get_attr(IOMMUMemoryRegion *iommu,
+ enum IOMMUMemoryRegionAttr attr, void *data)
+{
+ sPAPRTCETable *tcet = container_of(iommu, sPAPRTCETable, iommu);
+
+ if (attr == IOMMU_ATTR_SPAPR_TCE_FD && kvmppc_has_cap_spapr_vfio()) {
+ *(int *) data = tcet->fd;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
static void spapr_tce_notify_flag_changed(IOMMUMemoryRegion *iommu,
IOMMUNotifierFlag old,
IOMMUNotifierFlag new)
@@ -284,6 +297,10 @@ void spapr_tce_set_need_vfio(sPAPRTCETable *tcet, bool need_vfio)
tcet->need_vfio = need_vfio;
+ if (!need_vfio || (tcet->fd != -1 && kvmppc_has_cap_spapr_vfio())) {
+ return;
+ }
+
oldtable = tcet->table;
tcet->table = spapr_tce_alloc_table(tcet->liobn,
@@ -643,6 +660,7 @@ static void spapr_iommu_memory_region_class_init(ObjectClass *klass, void *data)
imrc->translate = spapr_tce_translate_iommu;
imrc->get_min_page_size = spapr_tce_get_min_page_size;
imrc->notify_flag_changed = spapr_tce_notify_flag_changed;
+ imrc->get_attr = spapr_tce_get_attr;
}
static const TypeInfo spapr_iommu_memory_region_info = {
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 84284d5957c5..9842b3bb124e 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -137,7 +137,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
cap_spapr_tce = kvm_check_extension(s, KVM_CAP_SPAPR_TCE);
cap_spapr_tce_64 = kvm_check_extension(s, KVM_CAP_SPAPR_TCE_64);
cap_spapr_multitce = kvm_check_extension(s, KVM_CAP_SPAPR_MULTITCE);
- cap_spapr_vfio = false;
+ cap_spapr_vfio = kvm_vm_check_extension(s, KVM_CAP_SPAPR_TCE_VFIO);
cap_one_reg = kvm_check_extension(s, KVM_CAP_ONE_REG);
cap_hior = kvm_check_extension(s, KVM_CAP_PPC_HIOR);
cap_epr = kvm_check_extension(s, KVM_CAP_PPC_EPR);
@@ -2514,6 +2514,11 @@ int kvmppc_get_cap_safe_indirect_branch(void)
return cap_ppc_safe_indirect_branch;
}
+bool kvmppc_has_cap_spapr_vfio(void)
+{
+ return cap_spapr_vfio;
+}
+
PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void)
{
uint32_t host_pvr = mfpvr();
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 39830baa77b9..4d2789eef6ef 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -46,6 +46,7 @@ void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t page_shift,
int kvmppc_remove_spapr_tce(void *table, int pfd, uint32_t window_size);
int kvmppc_reset_htab(int shift_hint);
uint64_t kvmppc_rma_size(uint64_t current_size, unsigned int hash_shift);
+bool kvmppc_has_cap_spapr_vfio(void);
#endif /* !CONFIG_USER_ONLY */
bool kvmppc_has_cap_epr(void);
int kvmppc_define_rtas_kernel_token(uint32_t token, const char *function);
@@ -232,6 +233,11 @@ static inline bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
return true;
}
+static inline bool kvmppc_has_cap_spapr_vfio(void)
+{
+ return false;
+}
+
#endif /* !CONFIG_USER_ONLY */
static inline bool kvmppc_has_cap_epr(void)
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 04/11] vfio/pci: Fixup VFIOMSIXInfo comment
2018-02-06 18:42 [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06 Alex Williamson
` (2 preceding siblings ...)
2018-02-06 18:43 ` [Qemu-devel] [PULL 03/11] spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device Alex Williamson
@ 2018-02-06 18:43 ` Alex Williamson
2018-02-06 18:43 ` [Qemu-devel] [PULL 05/11] vfio/pci: Add base BAR MemoryRegion Alex Williamson
` (7 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Alex Williamson @ 2018-02-06 18:43 UTC (permalink / raw)
To: qemu-devel
The fields were removed in the referenced commit, but the comment
still mentions them.
Fixes: 2fb9636ebf24 ("vfio-pci: Remove unused fields from VFIOMSIXInfo")
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
hw/vfio/pci.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index a8fb3b34222c..3d753222ca4c 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -86,7 +86,7 @@ enum {
VFIO_INT_MSIX = 3,
};
-/* Cache of MSI-X setup plus extra mmap and memory region for split BAR map */
+/* Cache of MSI-X setup */
typedef struct VFIOMSIXInfo {
uint8_t table_bar;
uint8_t pba_bar;
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 05/11] vfio/pci: Add base BAR MemoryRegion
2018-02-06 18:42 [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06 Alex Williamson
` (3 preceding siblings ...)
2018-02-06 18:43 ` [Qemu-devel] [PULL 04/11] vfio/pci: Fixup VFIOMSIXInfo comment Alex Williamson
@ 2018-02-06 18:43 ` Alex Williamson
2018-02-06 18:43 ` [Qemu-devel] [PULL 06/11] vfio/pci: Emulate BARs Alex Williamson
` (6 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Alex Williamson @ 2018-02-06 18:43 UTC (permalink / raw)
To: qemu-devel
Add one more layer to our stack of MemoryRegions, this base region
allows us to register BARs independently of the vfio region or to
extend the size of BARs which do map to a region. This will be
useful when we want hypervisor defined BARs or sections of BARs,
for purposes such as relocating MSI-X emulation. We therefore call
msix_init() based on this new base MemoryRegion, while the quirks,
which only modify regions still operate on those sub-MemoryRegions.
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
hw/vfio/pci.c | 94 ++++++++++++++++++++++++++++++++++++++++++---------------
hw/vfio/pci.h | 3 ++
2 files changed, 72 insertions(+), 25 deletions(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 2c7129512563..908b8dffca2b 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1087,7 +1087,7 @@ static void vfio_sub_page_bar_update_mapping(PCIDevice *pdev, int bar)
{
VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
VFIORegion *region = &vdev->bars[bar].region;
- MemoryRegion *mmap_mr, *mr;
+ MemoryRegion *mmap_mr, *region_mr, *base_mr;
PCIIORegion *r;
pcibus_t bar_addr;
uint64_t size = region->size;
@@ -1100,7 +1100,8 @@ static void vfio_sub_page_bar_update_mapping(PCIDevice *pdev, int bar)
r = &pdev->io_regions[bar];
bar_addr = r->addr;
- mr = region->mem;
+ base_mr = vdev->bars[bar].mr;
+ region_mr = region->mem;
mmap_mr = ®ion->mmaps[0].mem;
/* If BAR is mapped and page aligned, update to fill PAGE_SIZE */
@@ -1111,12 +1112,15 @@ static void vfio_sub_page_bar_update_mapping(PCIDevice *pdev, int bar)
memory_region_transaction_begin();
- memory_region_set_size(mr, size);
+ if (vdev->bars[bar].size < size) {
+ memory_region_set_size(base_mr, size);
+ }
+ memory_region_set_size(region_mr, size);
memory_region_set_size(mmap_mr, size);
- if (size != region->size && memory_region_is_mapped(mr)) {
- memory_region_del_subregion(r->address_space, mr);
+ if (size != vdev->bars[bar].size && memory_region_is_mapped(base_mr)) {
+ memory_region_del_subregion(r->address_space, base_mr);
memory_region_add_subregion_overlap(r->address_space,
- bar_addr, mr, 0);
+ bar_addr, base_mr, 0);
}
memory_region_transaction_commit();
@@ -1218,8 +1222,8 @@ void vfio_pci_write_config(PCIDevice *pdev,
for (bar = 0; bar < PCI_ROM_SLOT; bar++) {
if (old_addr[bar] != pdev->io_regions[bar].addr &&
- pdev->io_regions[bar].size > 0 &&
- pdev->io_regions[bar].size < qemu_real_host_page_size) {
+ vdev->bars[bar].region.size > 0 &&
+ vdev->bars[bar].region.size < qemu_real_host_page_size) {
vfio_sub_page_bar_update_mapping(pdev, bar);
}
}
@@ -1440,9 +1444,9 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
vdev->msix->pending = g_malloc0(BITS_TO_LONGS(vdev->msix->entries) *
sizeof(unsigned long));
ret = msix_init(&vdev->pdev, vdev->msix->entries,
- vdev->bars[vdev->msix->table_bar].region.mem,
+ vdev->bars[vdev->msix->table_bar].mr,
vdev->msix->table_bar, vdev->msix->table_offset,
- vdev->bars[vdev->msix->pba_bar].region.mem,
+ vdev->bars[vdev->msix->pba_bar].mr,
vdev->msix->pba_bar, vdev->msix->pba_offset, pos,
&err);
if (ret < 0) {
@@ -1482,8 +1486,8 @@ static void vfio_teardown_msi(VFIOPCIDevice *vdev)
if (vdev->msix) {
msix_uninit(&vdev->pdev,
- vdev->bars[vdev->msix->table_bar].region.mem,
- vdev->bars[vdev->msix->pba_bar].region.mem);
+ vdev->bars[vdev->msix->table_bar].mr,
+ vdev->bars[vdev->msix->pba_bar].mr);
g_free(vdev->msix->pending);
}
}
@@ -1500,12 +1504,11 @@ static void vfio_mmap_set_enabled(VFIOPCIDevice *vdev, bool enabled)
}
}
-static void vfio_bar_setup(VFIOPCIDevice *vdev, int nr)
+static void vfio_bar_prepare(VFIOPCIDevice *vdev, int nr)
{
VFIOBAR *bar = &vdev->bars[nr];
uint32_t pci_bar;
- uint8_t type;
int ret;
/* Skip both unimplemented BARs and the upper half of 64bit BARS. */
@@ -1524,23 +1527,52 @@ static void vfio_bar_setup(VFIOPCIDevice *vdev, int nr)
pci_bar = le32_to_cpu(pci_bar);
bar->ioport = (pci_bar & PCI_BASE_ADDRESS_SPACE_IO);
bar->mem64 = bar->ioport ? 0 : (pci_bar & PCI_BASE_ADDRESS_MEM_TYPE_64);
- type = pci_bar & (bar->ioport ? ~PCI_BASE_ADDRESS_IO_MASK :
- ~PCI_BASE_ADDRESS_MEM_MASK);
+ bar->type = pci_bar & (bar->ioport ? ~PCI_BASE_ADDRESS_IO_MASK :
+ ~PCI_BASE_ADDRESS_MEM_MASK);
+ bar->size = bar->region.size;
+}
+
+static void vfio_bars_prepare(VFIOPCIDevice *vdev)
+{
+ int i;
+
+ for (i = 0; i < PCI_ROM_SLOT; i++) {
+ vfio_bar_prepare(vdev, i);
+ }
+}
+
+static void vfio_bar_register(VFIOPCIDevice *vdev, int nr)
+{
+ VFIOBAR *bar = &vdev->bars[nr];
+ char *name;
- if (vfio_region_mmap(&bar->region)) {
- error_report("Failed to mmap %s BAR %d. Performance may be slow",
- vdev->vbasedev.name, nr);
+ if (!bar->size) {
+ return;
}
- pci_register_bar(&vdev->pdev, nr, type, bar->region.mem);
+ bar->mr = g_new0(MemoryRegion, 1);
+ name = g_strdup_printf("%s base BAR %d", vdev->vbasedev.name, nr);
+ memory_region_init_io(bar->mr, OBJECT(vdev), NULL, NULL, name, bar->size);
+ g_free(name);
+
+ if (bar->region.size) {
+ memory_region_add_subregion(bar->mr, 0, bar->region.mem);
+
+ if (vfio_region_mmap(&bar->region)) {
+ error_report("Failed to mmap %s BAR %d. Performance may be slow",
+ vdev->vbasedev.name, nr);
+ }
+ }
+
+ pci_register_bar(&vdev->pdev, nr, bar->type, bar->mr);
}
-static void vfio_bars_setup(VFIOPCIDevice *vdev)
+static void vfio_bars_register(VFIOPCIDevice *vdev)
{
int i;
for (i = 0; i < PCI_ROM_SLOT; i++) {
- vfio_bar_setup(vdev, i);
+ vfio_bar_register(vdev, i);
}
}
@@ -1549,8 +1581,13 @@ static void vfio_bars_exit(VFIOPCIDevice *vdev)
int i;
for (i = 0; i < PCI_ROM_SLOT; i++) {
+ VFIOBAR *bar = &vdev->bars[i];
+
vfio_bar_quirk_exit(vdev, i);
- vfio_region_exit(&vdev->bars[i].region);
+ vfio_region_exit(&bar->region);
+ if (bar->region.size) {
+ memory_region_del_subregion(bar->mr, bar->region.mem);
+ }
}
if (vdev->vga) {
@@ -1564,8 +1601,14 @@ static void vfio_bars_finalize(VFIOPCIDevice *vdev)
int i;
for (i = 0; i < PCI_ROM_SLOT; i++) {
+ VFIOBAR *bar = &vdev->bars[i];
+
vfio_bar_quirk_finalize(vdev, i);
- vfio_region_finalize(&vdev->bars[i].region);
+ vfio_region_finalize(&bar->region);
+ if (bar->size) {
+ object_unparent(OBJECT(bar->mr));
+ g_free(bar->mr);
+ }
}
if (vdev->vga) {
@@ -2810,7 +2853,8 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
goto error;
}
- vfio_bars_setup(vdev);
+ vfio_bars_prepare(vdev);
+ vfio_bars_register(vdev);
ret = vfio_add_capabilities(vdev, errp);
if (ret) {
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 3d753222ca4c..dcdb1a806769 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -33,6 +33,9 @@ typedef struct VFIOQuirk {
typedef struct VFIOBAR {
VFIORegion region;
+ MemoryRegion *mr;
+ size_t size;
+ uint8_t type;
bool ioport;
bool mem64;
QLIST_HEAD(, VFIOQuirk) quirks;
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 06/11] vfio/pci: Emulate BARs
2018-02-06 18:42 [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06 Alex Williamson
` (4 preceding siblings ...)
2018-02-06 18:43 ` [Qemu-devel] [PULL 05/11] vfio/pci: Add base BAR MemoryRegion Alex Williamson
@ 2018-02-06 18:43 ` Alex Williamson
2018-02-06 18:43 ` [Qemu-devel] [PULL 07/11] qapi: Create DEFINE_PROP_OFF_AUTO_PCIBAR Alex Williamson
` (5 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Alex Williamson @ 2018-02-06 18:43 UTC (permalink / raw)
To: qemu-devel
The kernel provides similar emulation of PCI BAR register access to
QEMU, so up until now we've used that for things like BAR sizing and
storing the BAR address. However, if we intend to resize BARs or add
BARs that don't exist on the physical device, we need to switch to the
pure QEMU emulation of the BAR.
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
hw/vfio/pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 908b8dffca2b..9436ac5cc9ae 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2777,6 +2777,8 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
/* QEMU can choose to expose the ROM or not */
memset(vdev->emulated_config_bits + PCI_ROM_ADDRESS, 0xff, 4);
+ /* QEMU can also add or extend BARs */
+ memset(vdev->emulated_config_bits + PCI_BASE_ADDRESS_0, 0xff, 6 * 4);
/*
* The PCI spec reserves vendor ID 0xffff as an invalid value. The
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 07/11] qapi: Create DEFINE_PROP_OFF_AUTO_PCIBAR
2018-02-06 18:42 [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06 Alex Williamson
` (5 preceding siblings ...)
2018-02-06 18:43 ` [Qemu-devel] [PULL 06/11] vfio/pci: Emulate BARs Alex Williamson
@ 2018-02-06 18:43 ` Alex Williamson
2018-02-06 18:43 ` [Qemu-devel] [PULL 08/11] vfio/pci: Allow relocating MSI-X MMIO Alex Williamson
` (4 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Alex Williamson @ 2018-02-06 18:43 UTC (permalink / raw)
To: qemu-devel
Add an option which allows the user to specify a PCI BAR number,
including an 'off' and 'auto' selection.
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
hw/core/qdev-properties.c | 11 +++++++++++
include/hw/qdev-properties.h | 4 ++++
qapi/common.json | 26 ++++++++++++++++++++++++++
3 files changed, 41 insertions(+)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 24c17800e3f1..5bbc2d98b5a8 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1317,3 +1317,14 @@ const PropertyInfo qdev_prop_link = {
.name = "link",
.create = create_link_property,
};
+
+/* --- OffAutoPCIBAR off/auto/bar0/bar1/bar2/bar3/bar4/bar5 --- */
+
+const PropertyInfo qdev_prop_off_auto_pcibar = {
+ .name = "OffAutoPCIBAR",
+ .description = "off/auto/bar0/bar1/bar2/bar3/bar4/bar5",
+ .enum_table = &OffAutoPCIBAR_lookup,
+ .get = get_enum,
+ .set = set_enum,
+ .set_default_value = set_default_value_enum,
+};
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 5bbfec634b29..1d61a351086d 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -34,6 +34,7 @@ extern const PropertyInfo qdev_prop_pci_host_devaddr;
extern const PropertyInfo qdev_prop_uuid;
extern const PropertyInfo qdev_prop_arraylen;
extern const PropertyInfo qdev_prop_link;
+extern const PropertyInfo qdev_prop_off_auto_pcibar;
#define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
.name = (_name), \
@@ -214,6 +215,9 @@ extern const PropertyInfo qdev_prop_link;
DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
#define DEFINE_PROP_MEMORY_REGION(_n, _s, _f) \
DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, MemoryRegion *)
+#define DEFINE_PROP_OFF_AUTO_PCIBAR(_n, _s, _f, _d) \
+ DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_off_auto_pcibar, \
+ OffAutoPCIBAR)
#define DEFINE_PROP_UUID(_name, _state, _field) { \
.name = (_name), \
diff --git a/qapi/common.json b/qapi/common.json
index 6eb01821ef59..d9b14dd429f3 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -100,3 +100,29 @@
{ 'alternate': 'StrOrNull',
'data': { 's': 'str',
'n': 'null' } }
+
+##
+# @OffAutoPCIBAR:
+#
+# An enumeration of options for specifying a PCI BAR
+#
+# @off: The specified feature is disabled
+#
+# @auto: The PCI BAR for the feature is automatically selected
+#
+# @bar0: PCI BAR0 is used for the feature
+#
+# @bar1: PCI BAR1 is used for the feature
+#
+# @bar2: PCI BAR2 is used for the feature
+#
+# @bar3: PCI BAR3 is used for the feature
+#
+# @bar4: PCI BAR4 is used for the feature
+#
+# @bar5: PCI BAR5 is used for the feature
+#
+# Since: 2.12
+##
+{ 'enum': 'OffAutoPCIBAR',
+ 'data': [ 'off', 'auto', 'bar0', 'bar1', 'bar2', 'bar3', 'bar4', 'bar5' ] }
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 08/11] vfio/pci: Allow relocating MSI-X MMIO
2018-02-06 18:42 [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06 Alex Williamson
` (6 preceding siblings ...)
2018-02-06 18:43 ` [Qemu-devel] [PULL 07/11] qapi: Create DEFINE_PROP_OFF_AUTO_PCIBAR Alex Williamson
@ 2018-02-06 18:43 ` Alex Williamson
2018-02-06 18:43 ` [Qemu-devel] [PULL 09/11] hw/vfio/platform: Init the interrupt mutex Alex Williamson
` (3 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Alex Williamson @ 2018-02-06 18:43 UTC (permalink / raw)
To: qemu-devel
Recently proposed vfio-pci kernel changes (v4.16) remove the
restriction preventing userspace from mmap'ing PCI BARs in areas
overlapping the MSI-X vector table. This change is primarily intended
to benefit host platforms which make use of system page sizes larger
than the PCI spec recommendation for alignment of MSI-X data
structures (ie. not x86_64). In the case of POWER systems, the SPAPR
spec requires the VM to program MSI-X using hypercalls, rendering the
MSI-X vector table unused in the VM view of the device. However,
ARM64 platforms also support 64KB pages and rely on QEMU emulation of
MSI-X. Regardless of the kernel driver allowing mmaps overlapping
the MSI-X vector table, emulation of the MSI-X vector table also
prevents direct mapping of device MMIO spaces overlapping this page.
Thanks to the fact that PCI devices have a standard self discovery
mechanism, we can try to resolve this by relocating the MSI-X data
structures, either by creating a new PCI BAR or extending an existing
BAR and updating the MSI-X capability for the new location. There's
even a very slim chance that this could benefit devices which do not
adhere to the PCI spec alignment guidelines on x86_64 systems.
This new x-msix-relocation option accepts the following choices:
off: Disable MSI-X relocation, use native device config (default)
auto: Use a known good combination for the platform/device (none yet)
bar0..bar5: Specify the target BAR for MSI-X data structures
If compatible, the target BAR will either be created or extended and
the new portion will be used for MSI-X emulation.
The first obvious user question with this option is how to determine
whether a given platform and device might benefit from this option.
In most cases, the answer is that it won't, especially on x86_64.
Devices often dedicate an entire BAR to MSI-X and therefore no
performance sensitive registers overlap the MSI-X area. Take for
example:
# lspci -vvvs 0a:00.0
0a:00.0 Ethernet controller: Intel Corporation I350 Gigabit Network Connection
...
Region 0: Memory at db680000 (32-bit, non-prefetchable) [size=512K]
Region 3: Memory at db7f8000 (32-bit, non-prefetchable) [size=16K]
...
Capabilities: [70] MSI-X: Enable+ Count=10 Masked-
Vector table: BAR=3 offset=00000000
PBA: BAR=3 offset=00002000
This device uses the 16K bar3 for MSI-X with the vector table at
offset zero and the pending bits arrary at offset 8K, fully honoring
the PCI spec alignment guidance. The data sheet specifically refers
to this as an MSI-X BAR. This device would not see a benefit from
MSI-X relocation regardless of the platform, regardless of the page
size.
However, here's another example:
# lspci -vvvs 02:00.0
02:00.0 Serial Attached SCSI controller: xxxxxxxx
...
Region 0: I/O ports at c000 [size=256]
Region 1: Memory at ef640000 (64-bit, non-prefetchable) [size=64K]
Region 3: Memory at ef600000 (64-bit, non-prefetchable) [size=256K]
...
Capabilities: [c0] MSI-X: Enable+ Count=16 Masked-
Vector table: BAR=1 offset=0000e000
PBA: BAR=1 offset=0000f000
Here the MSI-X data structures are placed on separate 4K pages at the
end of a 64KB BAR. If our host page size is 4K, we're likely fine,
but at 64KB page size, MSI-X emulation at that location prevents the
entire BAR from being directly mapped into the VM address space.
Overlapping performance sensitive registers then starts to be a very
likely scenario on such a platform. At this point, the user could
enable tracing on vfio_region_read and vfio_region_write to determine
more conclusively if device accesses are being trapped through QEMU.
Upon finding a device and platform in need of MSI-X relocation, the
next problem is how to choose target PCI BAR to host the MSI-X data
structures. A few key rules to keep in mind for this selection
include:
* There are only 6 BAR slots, bar0..bar5
* 64-bit BARs occupy two BAR slots, 'lspci -vvv' lists the first slot
* PCI BARs are always a power of 2 in size, extending == doubling
* The maximum size of a 32-bit BAR is 2GB
* MSI-X data structures must reside in an MMIO BAR
Using these rules, we can evaluate each BAR of the second example
device above as follows:
bar0: I/O port BAR, incompatible with MSI-X tables
bar1: BAR could be extended, incurring another 64KB of MMIO
bar2: Unavailable, bar1 is 64-bit, this register is used by bar1
bar3: BAR could be extended, incurring another 256KB of MMIO
bar4: Unavailable, bar3 is 64bit, this register is used by bar3
bar5: Available, empty BAR, minimum additional MMIO
A secondary optimization we might wish to make in relocating MSI-X
is to minimize the additional MMIO required for the device, therefore
we might test the available choices in order of preference as bar5,
bar1, and finally bar3. The original proposal for this feature
included an 'auto' option which would choose bar5 in this case, but
various drivers have been found that make assumptions about the
properties of the "first" BAR or the size of BARs such that there
appears to be no foolproof automatic selection available, requiring
known good combinations to be sourced from users. This patch is
pre-enabled for an 'auto' selection making use of a validated lookup
table, but no entries are yet identified.
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
hw/vfio/pci.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++-
hw/vfio/pci.h | 1 +
hw/vfio/trace-events | 2 +
3 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 9436ac5cc9ae..cab2aecb8094 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1356,6 +1356,98 @@ static void vfio_pci_fixup_msix_region(VFIOPCIDevice *vdev)
}
}
+static void vfio_pci_relocate_msix(VFIOPCIDevice *vdev, Error **errp)
+{
+ int target_bar = -1;
+ size_t msix_sz;
+
+ if (!vdev->msix || vdev->msix_relo == OFF_AUTOPCIBAR_OFF) {
+ return;
+ }
+
+ /* The actual minimum size of MSI-X structures */
+ msix_sz = (vdev->msix->entries * PCI_MSIX_ENTRY_SIZE) +
+ (QEMU_ALIGN_UP(vdev->msix->entries, 64) / 8);
+ /* Round up to host pages, we don't want to share a page */
+ msix_sz = REAL_HOST_PAGE_ALIGN(msix_sz);
+ /* PCI BARs must be a power of 2 */
+ msix_sz = pow2ceil(msix_sz);
+
+ if (vdev->msix_relo == OFF_AUTOPCIBAR_AUTO) {
+ /*
+ * TODO: Lookup table for known devices.
+ *
+ * Logically we might use an algorithm here to select the BAR adding
+ * the least additional MMIO space, but we cannot programatically
+ * predict the driver dependency on BAR ordering or sizing, therefore
+ * 'auto' becomes a lookup for combinations reported to work.
+ */
+ if (target_bar < 0) {
+ error_setg(errp, "No automatic MSI-X relocation available for "
+ "device %04x:%04x", vdev->vendor_id, vdev->device_id);
+ return;
+ }
+ } else {
+ target_bar = (int)(vdev->msix_relo - OFF_AUTOPCIBAR_BAR0);
+ }
+
+ /* I/O port BARs cannot host MSI-X structures */
+ if (vdev->bars[target_bar].ioport) {
+ error_setg(errp, "Invalid MSI-X relocation BAR %d, "
+ "I/O port BAR", target_bar);
+ return;
+ }
+
+ /* Cannot use a BAR in the "shadow" of a 64-bit BAR */
+ if (!vdev->bars[target_bar].size &&
+ target_bar > 0 && vdev->bars[target_bar - 1].mem64) {
+ error_setg(errp, "Invalid MSI-X relocation BAR %d, "
+ "consumed by 64-bit BAR %d", target_bar, target_bar - 1);
+ return;
+ }
+
+ /* 2GB max size for 32-bit BARs, cannot double if already > 1G */
+ if (vdev->bars[target_bar].size > (1 * 1024 * 1024 * 1024) &&
+ !vdev->bars[target_bar].mem64) {
+ error_setg(errp, "Invalid MSI-X relocation BAR %d, "
+ "no space to extend 32-bit BAR", target_bar);
+ return;
+ }
+
+ /*
+ * If adding a new BAR, test if we can make it 64bit. We make it
+ * prefetchable since QEMU MSI-X emulation has no read side effects
+ * and doing so makes mapping more flexible.
+ */
+ if (!vdev->bars[target_bar].size) {
+ if (target_bar < (PCI_ROM_SLOT - 1) &&
+ !vdev->bars[target_bar + 1].size) {
+ vdev->bars[target_bar].mem64 = true;
+ vdev->bars[target_bar].type = PCI_BASE_ADDRESS_MEM_TYPE_64;
+ }
+ vdev->bars[target_bar].type |= PCI_BASE_ADDRESS_MEM_PREFETCH;
+ vdev->bars[target_bar].size = msix_sz;
+ vdev->msix->table_offset = 0;
+ } else {
+ vdev->bars[target_bar].size = MAX(vdev->bars[target_bar].size * 2,
+ msix_sz * 2);
+ /*
+ * Due to above size calc, MSI-X always starts halfway into the BAR,
+ * which will always be a separate host page.
+ */
+ vdev->msix->table_offset = vdev->bars[target_bar].size / 2;
+ }
+
+ vdev->msix->table_bar = target_bar;
+ vdev->msix->pba_bar = target_bar;
+ /* Requires 8-byte alignment, but PCI_MSIX_ENTRY_SIZE guarantees that */
+ vdev->msix->pba_offset = vdev->msix->table_offset +
+ (vdev->msix->entries * PCI_MSIX_ENTRY_SIZE);
+
+ trace_vfio_msix_relo(vdev->vbasedev.name,
+ vdev->msix->table_bar, vdev->msix->table_offset);
+}
+
/*
* We don't have any control over how pci_add_capability() inserts
* capabilities into the chain. In order to setup MSI-X we need a
@@ -1434,6 +1526,8 @@ static void vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
vdev->msix = msix;
vfio_pci_fixup_msix_region(vdev);
+
+ vfio_pci_relocate_msix(vdev, errp);
}
static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
@@ -2849,13 +2943,14 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
vfio_pci_size_rom(vdev);
+ vfio_bars_prepare(vdev);
+
vfio_msix_early_setup(vdev, &err);
if (err) {
error_propagate(errp, err);
goto error;
}
- vfio_bars_prepare(vdev);
vfio_bars_register(vdev);
ret = vfio_add_capabilities(vdev, errp);
@@ -3045,6 +3140,8 @@ static Property vfio_pci_dev_properties[] = {
DEFINE_PROP_UNSIGNED_NODEFAULT("x-nv-gpudirect-clique", VFIOPCIDevice,
nv_gpudirect_clique,
qdev_prop_nv_gpudirect_clique, uint8_t),
+ DEFINE_PROP_OFF_AUTO_PCIBAR("x-msix-relocation", VFIOPCIDevice, msix_relo,
+ OFF_AUTOPCIBAR_OFF),
/*
* TODO - support passed fds... is this necessary?
* DEFINE_PROP_STRING("vfiofd", VFIOPCIDevice, vfiofd_name),
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index dcdb1a806769..588381f201b4 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -135,6 +135,7 @@ typedef struct VFIOPCIDevice {
(1 << VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT)
int32_t bootindex;
uint32_t igd_gms;
+ OffAutoPCIBAR msix_relo;
uint8_t pm_cap;
uint8_t nv_gpudirect_clique;
bool pci_aer;
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 3d34fe8a93f1..79f63a2ff6f7 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -16,6 +16,8 @@ vfio_msix_pba_disable(const char *name) " (%s)"
vfio_msix_pba_enable(const char *name) " (%s)"
vfio_msix_disable(const char *name) " (%s)"
vfio_msix_fixup(const char *name, int bar, uint64_t start, uint64_t end) " (%s) MSI-X region %d mmap fixup [0x%"PRIx64" - 0x%"PRIx64"]"
+vfio_msix_relo_cost(const char *name, int bar, uint64_t cost) " (%s) BAR %d cost 0x%"PRIx64""
+vfio_msix_relo(const char *name, int bar, uint64_t offset) " (%s) BAR %d offset 0x%"PRIx64""
vfio_msi_enable(const char *name, int nr_vectors) " (%s) Enabled %d MSI vectors"
vfio_msi_disable(const char *name) " (%s)"
vfio_pci_load_rom(const char *name, unsigned long size, unsigned long offset, unsigned long flags) "Device %s ROM:\n size: 0x%lx, offset: 0x%lx, flags: 0x%lx"
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 09/11] hw/vfio/platform: Init the interrupt mutex
2018-02-06 18:42 [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06 Alex Williamson
` (7 preceding siblings ...)
2018-02-06 18:43 ` [Qemu-devel] [PULL 08/11] vfio/pci: Allow relocating MSI-X MMIO Alex Williamson
@ 2018-02-06 18:43 ` Alex Williamson
2018-02-06 18:44 ` [Qemu-devel] [PULL 10/11] vfio/common: Remove redundant copy of local variable Alex Williamson
` (2 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Alex Williamson @ 2018-02-06 18:43 UTC (permalink / raw)
To: qemu-devel
From: Eric Auger <eric.auger@redhat.com>
Add the initialization of the mutex protecting the interrupt list.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
hw/vfio/platform.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index da84abf4fc4c..0d4bc0aae889 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -643,6 +643,8 @@ static void vfio_platform_realize(DeviceState *dev, Error **errp)
vbasedev->dev = dev;
vbasedev->ops = &vfio_platform_ops;
+ qemu_mutex_init(&vdev->intp_mutex);
+
trace_vfio_platform_realize(vbasedev->sysfsdev ?
vbasedev->sysfsdev : vbasedev->name,
vdev->compat);
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 10/11] vfio/common: Remove redundant copy of local variable
2018-02-06 18:42 [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06 Alex Williamson
` (8 preceding siblings ...)
2018-02-06 18:43 ` [Qemu-devel] [PULL 09/11] hw/vfio/platform: Init the interrupt mutex Alex Williamson
@ 2018-02-06 18:44 ` Alex Williamson
2018-02-06 18:44 ` [Qemu-devel] [PULL 11/11] vfio/pci: Add option to disable GeForce quirks Alex Williamson
2018-02-07 16:25 ` [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06 Peter Maydell
11 siblings, 0 replies; 14+ messages in thread
From: Alex Williamson @ 2018-02-06 18:44 UTC (permalink / raw)
To: qemu-devel
From: Alexey Kardashevskiy <aik@ozlabs.ru>
There is already @hostwin in vfio_listener_region_add() so there is no
point in having the other one.
Fixes: 2e4109de8e58 ("vfio/spapr: Create DMA window dynamically (SPAPR IOMMU v2)")
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
hw/vfio/common.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index ec41f93b7479..ee9240daeb5d 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -435,7 +435,6 @@ static void vfio_listener_region_add(MemoryListener *listener,
end = int128_get64(int128_sub(llend, int128_one()));
if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
- VFIOHostDMAWindow *hostwin;
hwaddr pgsize = 0;
/* For now intersections are not allowed, we may relax this later */
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 11/11] vfio/pci: Add option to disable GeForce quirks
2018-02-06 18:42 [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06 Alex Williamson
` (9 preceding siblings ...)
2018-02-06 18:44 ` [Qemu-devel] [PULL 10/11] vfio/common: Remove redundant copy of local variable Alex Williamson
@ 2018-02-06 18:44 ` Alex Williamson
2018-02-07 16:25 ` [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06 Peter Maydell
11 siblings, 0 replies; 14+ messages in thread
From: Alex Williamson @ 2018-02-06 18:44 UTC (permalink / raw)
To: qemu-devel
These quirks are necessary for GeForce, but not for Quadro/GRID/Tesla
assignment. Leaving them enabled is fully functional and provides the
most compatibility, but due to the unique NVIDIA MSI ACK behavior[1],
it also introduces latency in re-triggering the MSI interrupt. This
overhead is typically negligible, but has been shown to adversely
affect some (very) high interrupt rate applications. This adds the
vfio-pci device option "x-no-geforce-quirks=" which can be set to
"on" to disable this additional overhead.
A follow-on optimization for GeForce might be to make use of an
ioeventfd to allow KVM to trigger an irqfd in the kernel vfio-pci
driver, avoiding the bounce through userspace to handle this device
write.
[1] Background: the NVIDIA driver has been observed to issue a write
to the MMIO mirror of PCI config space in BAR0 in order to allow the
MSI interrupt for the device to retrigger. Older reports indicated a
write of 0xff to the (read-only) MSI capability ID register, while
more recently a write of 0x0 is observed at config space offset 0x704,
non-architected, extended config space of the device (BAR0 offset
0x88704). Virtualization of this range is only required for GeForce.
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
hw/vfio/pci-quirks.c | 9 ++++++---
hw/vfio/pci.c | 2 ++
hw/vfio/pci.h | 1 +
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 60ad5fb91a83..e5779a7ad35b 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -542,7 +542,8 @@ static void vfio_vga_probe_nvidia_3d0_quirk(VFIOPCIDevice *vdev)
VFIOQuirk *quirk;
VFIONvidia3d0Quirk *data;
- if (!vfio_pci_is(vdev, PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID) ||
+ if (vdev->no_geforce_quirks ||
+ !vfio_pci_is(vdev, PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID) ||
!vdev->bars[1].region.size) {
return;
}
@@ -660,7 +661,8 @@ static void vfio_probe_nvidia_bar5_quirk(VFIOPCIDevice *vdev, int nr)
VFIONvidiaBAR5Quirk *bar5;
VFIOConfigWindowQuirk *window;
- if (!vfio_pci_is(vdev, PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID) ||
+ if (vdev->no_geforce_quirks ||
+ !vfio_pci_is(vdev, PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID) ||
!vdev->vga || nr != 5 || !vdev->bars[5].ioport) {
return;
}
@@ -754,7 +756,8 @@ static void vfio_probe_nvidia_bar0_quirk(VFIOPCIDevice *vdev, int nr)
VFIOQuirk *quirk;
VFIOConfigMirrorQuirk *mirror;
- if (!vfio_pci_is(vdev, PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID) ||
+ if (vdev->no_geforce_quirks ||
+ !vfio_pci_is(vdev, PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID) ||
!vfio_is_vga(vdev) || nr != 0) {
return;
}
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index cab2aecb8094..879510c046cd 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3130,6 +3130,8 @@ static Property vfio_pci_dev_properties[] = {
DEFINE_PROP_BOOL("x-no-kvm-intx", VFIOPCIDevice, no_kvm_intx, false),
DEFINE_PROP_BOOL("x-no-kvm-msi", VFIOPCIDevice, no_kvm_msi, false),
DEFINE_PROP_BOOL("x-no-kvm-msix", VFIOPCIDevice, no_kvm_msix, false),
+ DEFINE_PROP_BOOL("x-no-geforce-quirks", VFIOPCIDevice,
+ no_geforce_quirks, false),
DEFINE_PROP_UINT32("x-pci-vendor-id", VFIOPCIDevice, vendor_id, PCI_ANY_ID),
DEFINE_PROP_UINT32("x-pci-device-id", VFIOPCIDevice, device_id, PCI_ANY_ID),
DEFINE_PROP_UINT32("x-pci-sub-vendor-id", VFIOPCIDevice,
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 588381f201b4..f4aa13e021fa 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -146,6 +146,7 @@ typedef struct VFIOPCIDevice {
bool no_kvm_intx;
bool no_kvm_msi;
bool no_kvm_msix;
+ bool no_geforce_quirks;
} VFIOPCIDevice;
uint32_t vfio_pci_read_config(PCIDevice *pdev, uint32_t addr, int len);
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06
2018-02-06 18:42 [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-06 Alex Williamson
` (10 preceding siblings ...)
2018-02-06 18:44 ` [Qemu-devel] [PULL 11/11] vfio/pci: Add option to disable GeForce quirks Alex Williamson
@ 2018-02-07 16:25 ` Peter Maydell
11 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2018-02-07 16:25 UTC (permalink / raw)
To: Alex Williamson; +Cc: QEMU Developers
On 6 February 2018 at 18:42, Alex Williamson <alex.williamson@redhat.com> wrote:
> The following changes since commit 20e0d439a6ded635ec89f6135c08cd5541c68962:
>
> Merge remote-tracking branch 'remotes/rth/tags/pull-hppa-20180204' into staging (2018-02-06 14:21:41 +0000)
>
> are available in the Git repository at:
>
> git://github.com/awilliam/qemu-vfio.git tags/vfio-update-20180206.0
>
> for you to fetch changes up to db32d0f43839627f54a1a7f8eee17baa770f52d2:
>
> vfio/pci: Add option to disable GeForce quirks (2018-02-06 11:08:27 -0700)
>
> ----------------------------------------------------------------
> VFIO updates 2018-02-06
>
> - SPAPR in-kernel TCE accleration (Alexey Kardashevskiy)
>
> - MSI-X relocation (Alex Williamson)
>
> - Add missing platform mutex init (Eric Auger)
>
> - Redundant variable cleanup (Alexey Kardashevskiy)
>
> - Option to disable GeForce quirks (Alex Williamson)
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PULL 08/11] vfio/pci: Allow relocating MSI-X MMIO
2018-02-05 18:56 [Qemu-devel] [PULL 00/11] VFIO updates 2018-02-05 Alex Williamson
@ 2018-02-05 18:57 ` Alex Williamson
0 siblings, 0 replies; 14+ messages in thread
From: Alex Williamson @ 2018-02-05 18:57 UTC (permalink / raw)
To: qemu-devel
Recently proposed vfio-pci kernel changes (v4.16) remove the
restriction preventing userspace from mmap'ing PCI BARs in areas
overlapping the MSI-X vector table. This change is primarily intended
to benefit host platforms which make use of system page sizes larger
than the PCI spec recommendation for alignment of MSI-X data
structures (ie. not x86_64). In the case of POWER systems, the SPAPR
spec requires the VM to program MSI-X using hypercalls, rendering the
MSI-X vector table unused in the VM view of the device. However,
ARM64 platforms also support 64KB pages and rely on QEMU emulation of
MSI-X. Regardless of the kernel driver allowing mmaps overlapping
the MSI-X vector table, emulation of the MSI-X vector table also
prevents direct mapping of device MMIO spaces overlapping this page.
Thanks to the fact that PCI devices have a standard self discovery
mechanism, we can try to resolve this by relocating the MSI-X data
structures, either by creating a new PCI BAR or extending an existing
BAR and updating the MSI-X capability for the new location. There's
even a very slim chance that this could benefit devices which do not
adhere to the PCI spec alignment guidelines on x86_64 systems.
This new x-msix-relocation option accepts the following choices:
off: Disable MSI-X relocation, use native device config (default)
auto: Use a known good combination for the platform/device (none yet)
bar0..bar5: Specify the target BAR for MSI-X data structures
If compatible, the target BAR will either be created or extended and
the new portion will be used for MSI-X emulation.
The first obvious user question with this option is how to determine
whether a given platform and device might benefit from this option.
In most cases, the answer is that it won't, especially on x86_64.
Devices often dedicate an entire BAR to MSI-X and therefore no
performance sensitive registers overlap the MSI-X area. Take for
example:
# lspci -vvvs 0a:00.0
0a:00.0 Ethernet controller: Intel Corporation I350 Gigabit Network Connection
...
Region 0: Memory at db680000 (32-bit, non-prefetchable) [size=512K]
Region 3: Memory at db7f8000 (32-bit, non-prefetchable) [size=16K]
...
Capabilities: [70] MSI-X: Enable+ Count=10 Masked-
Vector table: BAR=3 offset=00000000
PBA: BAR=3 offset=00002000
This device uses the 16K bar3 for MSI-X with the vector table at
offset zero and the pending bits arrary at offset 8K, fully honoring
the PCI spec alignment guidance. The data sheet specifically refers
to this as an MSI-X BAR. This device would not see a benefit from
MSI-X relocation regardless of the platform, regardless of the page
size.
However, here's another example:
# lspci -vvvs 02:00.0
02:00.0 Serial Attached SCSI controller: xxxxxxxx
...
Region 0: I/O ports at c000 [size=256]
Region 1: Memory at ef640000 (64-bit, non-prefetchable) [size=64K]
Region 3: Memory at ef600000 (64-bit, non-prefetchable) [size=256K]
...
Capabilities: [c0] MSI-X: Enable+ Count=16 Masked-
Vector table: BAR=1 offset=0000e000
PBA: BAR=1 offset=0000f000
Here the MSI-X data structures are placed on separate 4K pages at the
end of a 64KB BAR. If our host page size is 4K, we're likely fine,
but at 64KB page size, MSI-X emulation at that location prevents the
entire BAR from being directly mapped into the VM address space.
Overlapping performance sensitive registers then starts to be a very
likely scenario on such a platform. At this point, the user could
enable tracing on vfio_region_read and vfio_region_write to determine
more conclusively if device accesses are being trapped through QEMU.
Upon finding a device and platform in need of MSI-X relocation, the
next problem is how to choose target PCI BAR to host the MSI-X data
structures. A few key rules to keep in mind for this selection
include:
* There are only 6 BAR slots, bar0..bar5
* 64-bit BARs occupy two BAR slots, 'lspci -vvv' lists the first slot
* PCI BARs are always a power of 2 in size, extending == doubling
* The maximum size of a 32-bit BAR is 2GB
* MSI-X data structures must reside in an MMIO BAR
Using these rules, we can evaluate each BAR of the second example
device above as follows:
bar0: I/O port BAR, incompatible with MSI-X tables
bar1: BAR could be extended, incurring another 64KB of MMIO
bar2: Unavailable, bar1 is 64-bit, this register is used by bar1
bar3: BAR could be extended, incurring another 256KB of MMIO
bar4: Unavailable, bar3 is 64bit, this register is used by bar3
bar5: Available, empty BAR, minimum additional MMIO
A secondary optimization we might wish to make in relocating MSI-X
is to minimize the additional MMIO required for the device, therefore
we might test the available choices in order of preference as bar5,
bar1, and finally bar3. The original proposal for this feature
included an 'auto' option which would choose bar5 in this case, but
various drivers have been found that make assumptions about the
properties of the "first" BAR or the size of BARs such that there
appears to be no foolproof automatic selection available, requiring
known good combinations to be sourced from users. This patch is
pre-enabled for an 'auto' selection making use of a validated lookup
table, but no entries are yet identified.
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
hw/vfio/pci.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++-
hw/vfio/pci.h | 1 +
hw/vfio/trace-events | 2 +
3 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 9436ac5cc9ae..cab2aecb8094 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1356,6 +1356,98 @@ static void vfio_pci_fixup_msix_region(VFIOPCIDevice *vdev)
}
}
+static void vfio_pci_relocate_msix(VFIOPCIDevice *vdev, Error **errp)
+{
+ int target_bar = -1;
+ size_t msix_sz;
+
+ if (!vdev->msix || vdev->msix_relo == OFF_AUTOPCIBAR_OFF) {
+ return;
+ }
+
+ /* The actual minimum size of MSI-X structures */
+ msix_sz = (vdev->msix->entries * PCI_MSIX_ENTRY_SIZE) +
+ (QEMU_ALIGN_UP(vdev->msix->entries, 64) / 8);
+ /* Round up to host pages, we don't want to share a page */
+ msix_sz = REAL_HOST_PAGE_ALIGN(msix_sz);
+ /* PCI BARs must be a power of 2 */
+ msix_sz = pow2ceil(msix_sz);
+
+ if (vdev->msix_relo == OFF_AUTOPCIBAR_AUTO) {
+ /*
+ * TODO: Lookup table for known devices.
+ *
+ * Logically we might use an algorithm here to select the BAR adding
+ * the least additional MMIO space, but we cannot programatically
+ * predict the driver dependency on BAR ordering or sizing, therefore
+ * 'auto' becomes a lookup for combinations reported to work.
+ */
+ if (target_bar < 0) {
+ error_setg(errp, "No automatic MSI-X relocation available for "
+ "device %04x:%04x", vdev->vendor_id, vdev->device_id);
+ return;
+ }
+ } else {
+ target_bar = (int)(vdev->msix_relo - OFF_AUTOPCIBAR_BAR0);
+ }
+
+ /* I/O port BARs cannot host MSI-X structures */
+ if (vdev->bars[target_bar].ioport) {
+ error_setg(errp, "Invalid MSI-X relocation BAR %d, "
+ "I/O port BAR", target_bar);
+ return;
+ }
+
+ /* Cannot use a BAR in the "shadow" of a 64-bit BAR */
+ if (!vdev->bars[target_bar].size &&
+ target_bar > 0 && vdev->bars[target_bar - 1].mem64) {
+ error_setg(errp, "Invalid MSI-X relocation BAR %d, "
+ "consumed by 64-bit BAR %d", target_bar, target_bar - 1);
+ return;
+ }
+
+ /* 2GB max size for 32-bit BARs, cannot double if already > 1G */
+ if (vdev->bars[target_bar].size > (1 * 1024 * 1024 * 1024) &&
+ !vdev->bars[target_bar].mem64) {
+ error_setg(errp, "Invalid MSI-X relocation BAR %d, "
+ "no space to extend 32-bit BAR", target_bar);
+ return;
+ }
+
+ /*
+ * If adding a new BAR, test if we can make it 64bit. We make it
+ * prefetchable since QEMU MSI-X emulation has no read side effects
+ * and doing so makes mapping more flexible.
+ */
+ if (!vdev->bars[target_bar].size) {
+ if (target_bar < (PCI_ROM_SLOT - 1) &&
+ !vdev->bars[target_bar + 1].size) {
+ vdev->bars[target_bar].mem64 = true;
+ vdev->bars[target_bar].type = PCI_BASE_ADDRESS_MEM_TYPE_64;
+ }
+ vdev->bars[target_bar].type |= PCI_BASE_ADDRESS_MEM_PREFETCH;
+ vdev->bars[target_bar].size = msix_sz;
+ vdev->msix->table_offset = 0;
+ } else {
+ vdev->bars[target_bar].size = MAX(vdev->bars[target_bar].size * 2,
+ msix_sz * 2);
+ /*
+ * Due to above size calc, MSI-X always starts halfway into the BAR,
+ * which will always be a separate host page.
+ */
+ vdev->msix->table_offset = vdev->bars[target_bar].size / 2;
+ }
+
+ vdev->msix->table_bar = target_bar;
+ vdev->msix->pba_bar = target_bar;
+ /* Requires 8-byte alignment, but PCI_MSIX_ENTRY_SIZE guarantees that */
+ vdev->msix->pba_offset = vdev->msix->table_offset +
+ (vdev->msix->entries * PCI_MSIX_ENTRY_SIZE);
+
+ trace_vfio_msix_relo(vdev->vbasedev.name,
+ vdev->msix->table_bar, vdev->msix->table_offset);
+}
+
/*
* We don't have any control over how pci_add_capability() inserts
* capabilities into the chain. In order to setup MSI-X we need a
@@ -1434,6 +1526,8 @@ static void vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
vdev->msix = msix;
vfio_pci_fixup_msix_region(vdev);
+
+ vfio_pci_relocate_msix(vdev, errp);
}
static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
@@ -2849,13 +2943,14 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
vfio_pci_size_rom(vdev);
+ vfio_bars_prepare(vdev);
+
vfio_msix_early_setup(vdev, &err);
if (err) {
error_propagate(errp, err);
goto error;
}
- vfio_bars_prepare(vdev);
vfio_bars_register(vdev);
ret = vfio_add_capabilities(vdev, errp);
@@ -3045,6 +3140,8 @@ static Property vfio_pci_dev_properties[] = {
DEFINE_PROP_UNSIGNED_NODEFAULT("x-nv-gpudirect-clique", VFIOPCIDevice,
nv_gpudirect_clique,
qdev_prop_nv_gpudirect_clique, uint8_t),
+ DEFINE_PROP_OFF_AUTO_PCIBAR("x-msix-relocation", VFIOPCIDevice, msix_relo,
+ OFF_AUTOPCIBAR_OFF),
/*
* TODO - support passed fds... is this necessary?
* DEFINE_PROP_STRING("vfiofd", VFIOPCIDevice, vfiofd_name),
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index dcdb1a806769..588381f201b4 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -135,6 +135,7 @@ typedef struct VFIOPCIDevice {
(1 << VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT)
int32_t bootindex;
uint32_t igd_gms;
+ OffAutoPCIBAR msix_relo;
uint8_t pm_cap;
uint8_t nv_gpudirect_clique;
bool pci_aer;
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 3d34fe8a93f1..79f63a2ff6f7 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -16,6 +16,8 @@ vfio_msix_pba_disable(const char *name) " (%s)"
vfio_msix_pba_enable(const char *name) " (%s)"
vfio_msix_disable(const char *name) " (%s)"
vfio_msix_fixup(const char *name, int bar, uint64_t start, uint64_t end) " (%s) MSI-X region %d mmap fixup [0x%"PRIx64" - 0x%"PRIx64"]"
+vfio_msix_relo_cost(const char *name, int bar, uint64_t cost) " (%s) BAR %d cost 0x%"PRIx64""
+vfio_msix_relo(const char *name, int bar, uint64_t offset) " (%s) BAR %d offset 0x%"PRIx64""
vfio_msi_enable(const char *name, int nr_vectors) " (%s) Enabled %d MSI vectors"
vfio_msi_disable(const char *name) " (%s)"
vfio_pci_load_rom(const char *name, unsigned long size, unsigned long offset, unsigned long flags) "Device %s ROM:\n size: 0x%lx, offset: 0x%lx, flags: 0x%lx"
^ permalink raw reply related [flat|nested] 14+ messages in thread