On Mon, Oct 16, 2023 at 06:52:50PM +0100, Joao Martins wrote: > On 16/10/2023 17:34, Jason Gunthorpe wrote: > > On Mon, Oct 16, 2023 at 05:25:16PM +0100, Joao Martins wrote: > >>>> I think Jason is describing this would eventually be in a built-in > >>>> portion of IOMMUFD, but I think currently that built-in portion is > >>>> IOMMU. So until we have this IOMMUFD_DRIVER that enables that built-in > >>>> portion, it seems unnecessarily disruptive to make VFIO select IOMMUFD > >>>> to get this iova bitmap support. Thanks, > >>> > >>> Right, I'm saying Joao may as well make IOMMUFD_DRIVER right now for > >>> this > >> > >> So far I have this snip at the end. > >> > >> Though given that there are struct iommu_domain changes that set a dirty_ops > >> (which require iova-bitmap). > > > > Drivers which set those ops need to select IOMMUFD_DRIVER.. > > > > My problem is more of the generic/vfio side (headers and structures of iommu > core) not really IOMMU driver nor IOMMUFD. As I said, just don't compile that stuff. If nothing else selects IOMMFD_DRIVER then the core code has nothing to do. > >> diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig > >> index 99d4b075df49..96ec013d1192 100644 > >> --- a/drivers/iommu/iommufd/Kconfig > >> +++ b/drivers/iommu/iommufd/Kconfig > >> @@ -11,6 +11,13 @@ config IOMMUFD > >> > >> If you don't know what to do here, say N. > >> > >> +config IOMMUFD_DRIVER > >> + bool "IOMMUFD provides iommu drivers supporting functions" > >> + default IOMMU_API > >> + help > >> + IOMMUFD will provides supporting data structures and helpers to IOMMU > >> + drivers. > > > > It is not a 'user selectable' kconfig, just make it > > > > config IOMMUFD_DRIVER > > tristate > > default n > > > tristate? More like a bool as IOMMU drivers aren't modloadable tristate, who knows what people will select. If the modular drivers use it then it is forced to a Y not a M. It is the right way to use kconfig.. > >> --- a/drivers/vfio/Kconfig > >> +++ b/drivers/vfio/Kconfig > >> @@ -7,6 +7,7 @@ menuconfig VFIO > >> select VFIO_GROUP if SPAPR_TCE_IOMMU || IOMMUFD=n > >> select VFIO_DEVICE_CDEV if !VFIO_GROUP > >> select VFIO_CONTAINER if IOMMUFD=n > >> + select IOMMUFD_DRIVER > > > > As discussed use a if (IS_ENABLED) here and just disable the > > bitmap code if something else didn't enable it. > > > > I'm adding this to vfio_main: > > if (!IS_ENABLED(CONFIG_IOMMUFD_DRIVER)) > return -EOPNOTSUPP; Seems right > > VFIO isn't a consumer of it > > > > (...) The select IOMMUFD_DRIVER was there because of VFIO PCI vendor drivers not > VFIO core. Those driver should individually select IOMMUFD_DRIVER for the 'disable bitmap code' I can add ifdef-ry in iova_bitmap.h to > add scalfold definitions to error-out/nop if CONFIG_IOMMUFD_DRIVER=n when moving > to iommufd/ Yes that could also be a good approach > For your suggested scheme to work VFIO PCI drivers still need to select > IOMMUFD_DRIVER as they require it Yes, of course Jason
