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

[Joao Martins <joao.m.martins@xxxxxxxxxx>]

On 17/10/2023 13:58, Jason Gunthorpe wrote:
> On Mon, Oct 16, 2023 at 07:50:25PM +0100, Joao Martins wrote:
>> On 16/10/2023 19:37, Joao Martins wrote:
>>> On 16/10/2023 19:20, Jason Gunthorpe wrote:
>>>> On Mon, Oct 16, 2023 at 07:15:10PM +0100, Joao Martins wrote:
>>>>
>>>>> Here's a diff, naturally AMD/Intel kconfigs would get a select IOMMUFD_DRIVER as
>>>>> well later in the series
>>>>
>>>> It looks OK, the IS_ENABLES are probably overkill once you have
>>>> changed the .h file, just saves a few code bytes, not sure we care?
>>>
>>> I can remove them
>>
>> Additionally, I don't think I can use the symbol namespace for IOMMUFD, as
>> iova-bitmap can be build builtin with a module iommufd, otherwise we get into
>> errors like this:
>>
>> ERROR: modpost: module iommufd uses symbol iova_bitmap_for_each from namespace
>> IOMMUFD, but does not import it.
>> ERROR: modpost: module iommufd uses symbol iova_bitmap_free from namespace
>> IOMMUFD, but does not import it.
>> ERROR: modpost: module iommufd uses symbol iova_bitmap_alloc from namespace
>> IOMMUFD, but does not import it.
> 
> You cannot self-import the namespace? I'm not that familiar with this stuff

Neither do I. But self-importing looks to work. An alternative is to have an
alternative namespace (e.g. IOMMUFD_DRIVER) in similar fashion to IOMMUFD_INTERNAL.

But I fear this patch is already doing too much at the late stage. Are you keen
on getting this moved with namespaces right now, or it can be a post-merge cleanup?

diff --git a/drivers/iommu/iommufd/iova_bitmap.c
b/drivers/iommu/iommufd/iova_bitmap.c
index f54b56388e00..0aaf2218bf61 100644
--- a/drivers/iommu/iommufd/iova_bitmap.c
+++ b/drivers/iommu/iommufd/iova_bitmap.c
@@ -7,6 +7,7 @@
 #include <linux/mm.h>
 #include <linux/slab.h>
 #include <linux/highmem.h>
+#include <linux/module.h>

 #define BITS_PER_PAGE (PAGE_SIZE * BITS_PER_BYTE)

@@ -268,7 +269,7 @@ struct iova_bitmap *iova_bitmap_alloc(unsigned long iova,
size_t length,
        iova_bitmap_free(bitmap);
        return ERR_PTR(rc);
 }
-EXPORT_SYMBOL_GPL(iova_bitmap_alloc);
+EXPORT_SYMBOL_NS_GPL(iova_bitmap_alloc, IOMMUFD);

 /**
  * iova_bitmap_free() - Frees an IOVA bitmap object
@@ -290,7 +291,7 @@ void iova_bitmap_free(struct iova_bitmap *bitmap)

        kfree(bitmap);
 }
-EXPORT_SYMBOL_GPL(iova_bitmap_free);
+EXPORT_SYMBOL_NS_GPL(iova_bitmap_free, IOMMUFD);

 /*
  * Returns the remaining bitmap indexes from mapped_total_index to process for
@@ -389,7 +390,7 @@ int iova_bitmap_for_each(struct iova_bitmap *bitmap, void
*opaque,

        return ret;
 }
-EXPORT_SYMBOL_GPL(iova_bitmap_for_each);
+EXPORT_SYMBOL_NS_GPL(iova_bitmap_for_each, IOMMUFD);

 /**
  * iova_bitmap_set() - Records an IOVA range in bitmap
diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c
index 25cfbf67031b..30f1656ac5da 100644
--- a/drivers/iommu/iommufd/main.c
+++ b/drivers/iommu/iommufd/main.c
@@ -558,5 +558,6 @@ MODULE_ALIAS_MISCDEV(VFIO_MINOR);
 MODULE_ALIAS("devname:vfio/vfio");
 #endif
 MODULE_IMPORT_NS(IOMMUFD_INTERNAL);
+MODULE_IMPORT_NS(IOMMUFD);
 MODULE_DESCRIPTION("I/O Address Space Management for passthrough devices");
 MODULE_LICENSE("GPL");
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 40732e8ed4c6..a96d97da367d 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -1693,6 +1693,7 @@ static void __exit vfio_cleanup(void)
 module_init(vfio_init);
 module_exit(vfio_cleanup);

+MODULE_IMPORT_NS(IOMMUFD);
 MODULE_VERSION(DRIVER_VERSION);
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR(DRIVER_AUTHOR);