On Wed, 16 Nov 2022 17:05:29 -0400
Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:
> This legacy module knob has become uAPI, when set on the vfio_iommu_type1
> it disables some security protections in the iommu drivers. Move the
> storage for this knob to vfio_main.c so that iommufd can access it too.
>
> The may need enhancing as we learn more about how necessary
> allow_unsafe_interrupts is in the current state of the world. If vfio
> container is disabled then this option will not be available to the user.
>
> Tested-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
> Tested-by: Yi Liu <yi.l.liu@xxxxxxxxx>
> Tested-by: Lixiao Yang <lixiao.yang@xxxxxxxxx>
> Tested-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
> Tested-by: Yu He <yu.he@xxxxxxxxx>
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> ---
> drivers/vfio/vfio.h | 2 ++
> drivers/vfio/vfio_iommu_type1.c | 5 ++---
> drivers/vfio/vfio_main.c | 3 +++
> 3 files changed, 7 insertions(+), 3 deletions(-)
It's really quite trivial to convert to a vfio_iommu.ko module to host
a separate option for this. Half of the patch below is undoing what's
done here. Is your only concern with this approach that we use a few
KB more memory for the separate module?
I don't think a per-device sysfs setting makes a lot of sense, if we're
on a host w/o MSI isolation, all devices are affected. Thanks,
Alex
Makefile | 4 +++-
iommufd.c | 12 +++++++++++-
vfio.h | 2 --
vfio_iommu_type1.c | 5 +++--
vfio_main.c | 6 +++---
5 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index b953517dc70f..34b7d91112e5 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -5,9 +5,11 @@ obj-$(CONFIG_VFIO) += vfio.o
vfio-y += vfio_main.o \
iova_bitmap.o
-vfio-$(CONFIG_IOMMUFD) += iommufd.o
vfio-$(CONFIG_VFIO_CONTAINER) += container.o
+obj-$(CONFIG_IOMMUFD) += vfio_iommufd.o
+vfio_iommufd-y += iommufd.o
+
obj-$(CONFIG_VFIO_VIRQFD) += vfio_virqfd.o
obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c
index dad8935d71f7..b6b4038ba036 100644
--- a/drivers/vfio/iommufd.c
+++ b/drivers/vfio/iommufd.c
@@ -10,6 +10,12 @@
MODULE_IMPORT_NS(IOMMUFD);
MODULE_IMPORT_NS(IOMMUFD_VFIO);
+static bool allow_unsafe_interrupts;
+module_param_named(allow_unsafe_interrupts,
+ allow_unsafe_interrupts, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(allow_unsafe_interrupts,
+ "Enable VFIO IOMMUFD support on platforms without MSI/X isolation facilities.");
+
int vfio_iommufd_bind(struct vfio_device *vdev, struct iommufd_ctx *ictx)
{
u32 ioas_id;
@@ -47,6 +53,7 @@ int vfio_iommufd_bind(struct vfio_device *vdev, struct iommufd_ctx *ictx)
vdev->ops->unbind_iommufd(vdev);
return ret;
}
+EXPORT_SYMBOL_GPL(vfio_iommufd_bind);
void vfio_iommufd_unbind(struct vfio_device *vdev)
{
@@ -55,6 +62,7 @@ void vfio_iommufd_unbind(struct vfio_device *vdev)
if (vdev->ops->unbind_iommufd)
vdev->ops->unbind_iommufd(vdev);
}
+EXPORT_SYMBOL_GPL(vfio_iommufd_unbind);
/*
* The physical standard ops mean that the iommufd_device is bound to the
@@ -92,7 +100,7 @@ int vfio_iommufd_physical_attach_ioas(struct vfio_device *vdev, u32 *pt_id)
unsigned int flags = 0;
int rc;
- if (vfio_allow_unsafe_interrupts)
+ if (allow_unsafe_interrupts)
flags |= IOMMUFD_ATTACH_FLAGS_ALLOW_UNSAFE_INTERRUPT;
rc = iommufd_device_attach(vdev->iommufd_device, pt_id, flags);
if (rc)
@@ -159,3 +167,5 @@ int vfio_iommufd_emulated_attach_ioas(struct vfio_device *vdev, u32 *pt_id)
return 0;
}
EXPORT_SYMBOL_GPL(vfio_iommufd_emulated_attach_ioas);
+
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
index 3378714a7462..ce5fe3fc493b 100644
--- a/drivers/vfio/vfio.h
+++ b/drivers/vfio/vfio.h
@@ -216,6 +216,4 @@ extern bool vfio_noiommu __read_mostly;
enum { vfio_noiommu = false };
#endif
-extern bool vfio_allow_unsafe_interrupts;
-
#endif
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 186e33a006d3..23c24fe98c00 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -44,8 +44,9 @@
#define DRIVER_AUTHOR "Alex Williamson <alex.williamson@xxxxxxxxxx>"
#define DRIVER_DESC "Type1 IOMMU driver for VFIO"
+static bool allow_unsafe_interrupts;
module_param_named(allow_unsafe_interrupts,
- vfio_allow_unsafe_interrupts, bool, S_IRUGO | S_IWUSR);
+ allow_unsafe_interrupts, bool, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(allow_unsafe_interrupts,
"Enable VFIO IOMMU support for on platforms without interrupt remapping support.");
@@ -2281,7 +2282,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
iommu_group_for_each_dev(iommu_group, (void *)IOMMU_CAP_INTR_REMAP,
vfio_iommu_device_capable);
- if (!vfio_allow_unsafe_interrupts && !msi_remap) {
+ if (!allow_unsafe_interrupts && !msi_remap) {
pr_warn("%s: No interrupt remapping support. Use the module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this platform\n",
__func__);
ret = -EPERM;
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index f3c48b8c4562..48b3aa8582aa 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -42,6 +42,9 @@
#define DRIVER_AUTHOR "Alex Williamson <alex.williamson@xxxxxxxxxx>"
#define DRIVER_DESC "VFIO - User Level meta-driver"
+MODULE_IMPORT_NS(IOMMUFD);
+MODULE_IMPORT_NS(IOMMUFD_VFIO);
+
static struct vfio {
struct class *class;
struct list_head group_list;
@@ -52,9 +55,6 @@ static struct vfio {
struct ida device_ida;
} vfio;
-bool vfio_allow_unsafe_interrupts;
-EXPORT_SYMBOL_GPL(vfio_allow_unsafe_interrupts);
-
static DEFINE_XARRAY(vfio_device_set_xa);
static const struct file_operations vfio_group_fops;
