On Thu, Nov 17, 2022 at 01:14:51PM -0700, Alex Williamson wrote: > 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? My main dislike is that it just seems arbitary to shunt iommufd support to a module when it is always required by vfio.ko. In general if you have a module that is only ever used by 1 other module, you should probably just combine them. It saves memory and simplifies operation (eg you don't have to unload a zoo of modules during development testing) If it wasn't for the module option ABI problem I would propse to merge vfio_type1/spapr into vfio.ko too - vfio.ko doesn't work without them and the module soft dependencies are hint something is weird here. An alternative suggestion is to just retain a stub vfio_iommu_type1.ko which only exposes the module option if iommufd is enabled. At least this would preserve the semi-ABI. However, if you think this fits your vision for VFIO better I will take it. > 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."); Except this, I think we still should have iommufd compat with the current module ABI, so this should still get moved into vfio.ko and both vfio_iommu_type1.ko and vfio_iommufd.ko should jointly manipulate the same memory with their module options. Thanks, Jason
