Re: [PATCH guest kernel] vfio/powerpc/spapr_tce: Enforce IOMMU type compatibility check
From: Alex Williamson <hidden>
Date: 2017-03-24 20:29:08
Also in:
kvm
On Fri, 24 Mar 2017 17:44:06 +1100 Alexey Kardashevskiy [off-list ref] wrote:
The existing SPAPR TCE driver advertises both VFIO_SPAPR_TCE_IOMMU and VFIO_SPAPR_TCE_v2_IOMMU types to the userspace and the userspace usually picks the v2. Normally the userspace would create a container, attach an IOMMU group to it and only then set the IOMMU type (which would normally be v2). However a specific IOMMU group may not support v2, in other words it may not implement set_window/unset_window/take_ownership/ release_ownership and such a group should not be attached to a v2 container. This adds extra checks that a new group can do what the selected IOMMU type suggests. The userspace can then test the return value from ioctl(VFIO_SET_IOMMU, VFIO_SPAPR_TCE_v2_IOMMU) and try VFIO_SPAPR_TCE_IOMMU. Signed-off-by: Alexey Kardashevskiy <redacted> --- This is one of the patches needed to do nested VFIO - for either second level guest or DPDK running in a guest. --- drivers/vfio/vfio_iommu_spapr_tce.c | 8 ++++++++ 1 file changed, 8 insertions(+)
I'm not sure I understand why you're labeling this "guest kernel", is a VM the only case where we can have combinations that only a subset of the groups might support v2? What terrible things happen when such a combination is created? The fix itself seems sane, but I'm trying to figure out whether it should be marked for stable, should go in for v4.11, or be queued for v4.12. Thanks, Alex
quoted hunk ↗ jump to hunk
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c index cf3de91fbfe7..a7d811524092 100644 --- a/drivers/vfio/vfio_iommu_spapr_tce.c +++ b/drivers/vfio/vfio_iommu_spapr_tce.c@@ -1335,8 +1335,16 @@ static int tce_iommu_attach_group(void *iommu_data, if (!table_group->ops || !table_group->ops->take_ownership || !table_group->ops->release_ownership) { + if (container->v2) { + ret = -EPERM; + goto unlock_exit; + } ret = tce_iommu_take_ownership(container, table_group); } else { + if (!container->v2) { + ret = -EPERM; + goto unlock_exit; + } ret = tce_iommu_take_ownership_ddw(container, table_group); if (!tce_groups_attached(container) && !container->tables[0]) container->def_window_pending = true;