On 04/08/2015 12:01 PM, David Gibson wrote:
On Tue, Mar 31, 2015 at 04:28:39PM +1100, Alexey Kardashevskiy wrote:
This enables multiple IOMMU groups in one VFIO container which means
that multiple devices from different groups can share the same IOMMU
table (or tables if DDW).
This removes a group id from vfio_container_ioctl(). The kernel support
is required for this; if the host kernel does not have the support,
it will allow only one group per container. The PHB's "iommuid" property
is ignored.
This adds a sanity check that there is just one VFIO container per
PHB address space.
Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
[snip]
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index b012620..99e1900 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -915,21 +915,23 @@ void vfio_put_base_device(VFIODevice *vbasedev)
close(vbasedev->fd);
}
-static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
+static int vfio_container_do_ioctl(AddressSpace *as,
int req, void *param)
{
- VFIOGroup *group;
VFIOContainer *container;
- int ret = -1;
+ int ret;
+ VFIOAddressSpace *space;
- group = vfio_get_group(groupid, as);
- if (!group) {
- error_report("vfio: group %d not registered", groupid);
- return ret;
- }
+ space = vfio_get_address_space(as);
+ container = QLIST_FIRST(&space->containers);
So getting the container handle from the address space, rather than
the group id certainly makes more sense to me.
- container = group->container;
- if (group->container) {
+ if (!container) {
+ error_report("vfio: container is not set");
+ return -1;
+ } else if (QLIST_NEXT(container, next)) {
+ error_report("vfio: multiple containers per PHB are not supported");
+ return -1;
But if only one PHB per address space is possible, why is the
containers field a list in the first place?
Historically the list was added in 3df3e0a5872 (the patch of yours :) ).
In theory we could implement spapr-pci-bridge (derived from pci-bridge)
with isolation capability (i.e. its own LIOBN/DMA window), in this case
there could be multiple containers per PHB address space. Other archs could
want multiple containers for some other reason. It would help me a lot if
you remembered why you kept the list at the first place :)
For now I guess I'll move the next patch ("vfio: spapr: Move SPAPR-related
code to a separate file") before this one, do s/vfio_container_do_ioctl/
vfio_spapr_container_do_ioctl/ and move it to hw/vfio/spapr.c. Makes sense?
+ } else {
ret = ioctl(container->fd, req, param);
if (ret < 0) {
error_report("vfio: failed to ioctl %d to container: ret=%d, %s",
@@ -937,12 +939,10 @@ static int vfio_container_do_ioctl(AddressSpace *as,
int32_t groupid,
}
}
- vfio_put_group(group);
-
return ret;
}
-int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
+int vfio_container_ioctl(AddressSpace *as,
int req, void *param)
{
/* We allow only certain ioctls to the container */
@@ -957,5 +957,5 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
return -1;
}
- return vfio_container_do_ioctl(as, groupid, req, param);
+ return vfio_container_do_ioctl(as, req, param);
}
diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h
index 0b26cd8..76b5744 100644
--- a/include/hw/vfio/vfio.h
+++ b/include/hw/vfio/vfio.h
@@ -3,7 +3,7 @@
#include "qemu/typedefs.h"
-extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
+extern int vfio_container_ioctl(AddressSpace *as,
int req, void *param);
#endif
--
Alexey
