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

Joao Martins
2023-10-17T15:44:48Z

On 17/10/2023 16:23, Jason Gunthorpe wrote:
> On Tue, Oct 17, 2023 at 04:20:22PM +0100, Joao Martins wrote:
>> 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?
>
> It is our standard, if you want to make two patches in this series that is OK too
>

This is what I have now (as a separate patch).

It is a little more intrusive as I need to change exist module users
(mlx5-vfio-pci, pds, vfio).

--->8---

From: Joao Martins <joao.m.martins@oracle.com>
Date: Tue, 17 Oct 2023 11:12:28 -0400
Subject: [PATCH v4 03/20] iommufd/iova_bitmap: Move symbols to IOMMUFD namespace

The IOVA bitmap helpers were not using any namespaces, so to adhere with
IOMMUFD symbol export convention, use the IOMMUFD and import in the right
places. This today means to self-import in iommufd/main.c, VFIO and the
vfio-pci drivers that use iova_bitmap_set().

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
drivers/iommu/iommufd/iova_bitmap.c | 8 ++++----
drivers/iommu/iommufd/main.c | 1 +
drivers/vfio/pci/mlx5/main.c | 1 +
drivers/vfio/pci/pds/pci_drv.c | 1 +
drivers/vfio/vfio_main.c | 1 +
5 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/iommufd/iova_bitmap.c
b/drivers/iommu/iommufd/iova_bitmap.c
index f54b56388e00..0a92c9eeaf7f 100644
--- a/drivers/iommu/iommufd/iova_bitmap.c
+++ b/drivers/iommu/iommufd/iova_bitmap.c
@@ -268,7 +268,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 +290,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 +389,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
@@ -423,4 +423,4 @@ void iova_bitmap_set(struct iova_bitmap *bitmap,
cur_bit += nbits;
} while (cur_bit <= last_bit);
}
-EXPORT_SYMBOL_GPL(iova_bitmap_set);
+EXPORT_SYMBOL_NS_GPL(iova_bitmap_set, IOMMUFD);
diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c
index e71523cbd0de..9b2c18d7af1e 100644
--- a/drivers/iommu/iommufd/main.c
+++ b/drivers/iommu/iommufd/main.c
@@ -552,5 +552,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/pci/mlx5/main.c b/drivers/vfio/pci/mlx5/main.c
index 42ec574a8622..5cf2b491d15a 100644
--- a/drivers/vfio/pci/mlx5/main.c
+++ b/drivers/vfio/pci/mlx5/main.c
@@ -1376,6 +1376,7 @@ static struct pci_driver mlx5vf_pci_driver = {

module_pci_driver(mlx5vf_pci_driver);

+MODULE_IMPORT_NS(IOMMUFD);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Max Gurtovoy <mgurtovoy@nvidia.com>");
MODULE_AUTHOR("Yishai Hadas <yishaih@nvidia.com>");
diff --git a/drivers/vfio/pci/pds/pci_drv.c b/drivers/vfio/pci/pds/pci_drv.c
index ab4b5958e413..dd8c00c895a2 100644
--- a/drivers/vfio/pci/pds/pci_drv.c
+++ b/drivers/vfio/pci/pds/pci_drv.c
@@ -204,6 +204,7 @@ static struct pci_driver pds_vfio_pci_driver = {

module_pci_driver(pds_vfio_pci_driver);

+MODULE_IMPORT_NS(IOMMUFD);
MODULE_DESCRIPTION(PDS_VFIO_DRV_DESCRIPTION);
MODULE_AUTHOR("Brett Creeley <brett.creeley@amd.com>");
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);
--
2.17.2