Re: [PATCH v3 02/19] vfio: Move iova_bitmap into iommu core

Jason Gunthorpe
2023-10-16T16:44:48Z

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..

Perhaps (at least for ARM) they should even be coded

select IOMMUFD_DRIVER if IOMMUFD

And then #ifdef out the dirty tracking bits so embedded systems don't
get the bloat with !IOMMUFD

> 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

ie the only way to get it is to build a driver that will consume it.

> diff --git a/drivers/iommu/iommufd/Makefile b/drivers/iommu/iommufd/Makefile
> index 8aeba81800c5..34b446146961 100644
> --- a/drivers/iommu/iommufd/Makefile
> +++ b/drivers/iommu/iommufd/Makefile
> @@ -11,3 +11,4 @@ iommufd-y := \
> iommufd-$(CONFIG_IOMMUFD_TEST) += selftest.o
>
> obj-$(CONFIG_IOMMUFD) += iommufd.o
> +obj-$(CONFIG_IOMMUFD_DRIVER) += iova_bitmap.o

Right..

> diff --git a/drivers/vfio/iova_bitmap.c b/drivers/iommu/iommufd/iova_bitmap.c
> similarity index 100%
> rename from drivers/vfio/iova_bitmap.c
> rename to drivers/iommu/iommufd/iova_bitmap.c
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 6bda6dbb4878..1db519cce815 100644
> --- 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.

VFIO isn't a consumer of it

The question you are asking is on the driver side implementing it, and
it should be conditional if IOMMUFD is turned on.

Jason