On 18/10/2023 13:03, Jason Gunthorpe wrote:
> On Wed, Oct 18, 2023 at 11:19:07AM +0100, Joao Martins wrote:
>> On 16/10/2023 19:05, Jason Gunthorpe wrote:
>>> 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:
>>>>>> 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..
>>>
>> Making it tristate will break build bisection in this module with errors like this:
>>
>> [I say bisection, because aftewards when we put IOMMU drivers in the mix, these
>> are always builtin, so it ends up selecting IOMMU_DRIVER=y.]
>>
>> ERROR: modpost: missing MODULE_LICENSE() in drivers/iommu/iommufd/iova_bitmap.o
>>
>> iova_bitmap is no module, and making it tristate allows to build it as a module
>> as long as one of the selectors of is a module. 'bool' is actually more accurate
>> to what it is builtin or not.
>
> It is a module if you make it tristate, add the MODULE_LICENSE
It's not just that. It can't work as a module when CONFIG_VFIO=y and another
user is CONFIG_MLX5_VFIO_PCI=m. CONFIG_VFIO uses the API so this is that case
where IS_ENABLED(CONFIG_IOMMUFD_DRIVER) evaluates to true but it is only
technically used by a module so it doesn't link it in. You could have the header
file test for its presence of being a module instead of just IS_ENABLED() . But
then this is useless as CONFIG_VFIO code is what drives the whole VFIO driver
dirty tracking so it must override it as =y.
This extra kconfig change at the end should fix it. But I am a bit skeptical of
these last minute module-user changes, as it is getting a bit too nuanced from
what was a relatively non invasive change.
I would like to reiterate that there's no actual module user, making a bool is a
bit more clear on its usage on what it actually is (you would need IOMMU drivers
to be modules, which I think is a big gamble that is happening anytime soon?)
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
help
VFIO provides a framework for secure userspace device drivers.
See Documentation/driver-api/vfio.rst for more details.
diff --git a/drivers/iommu/iommufd/iova_bitmap.c
b/drivers/iommu/iommufd/iova_bitmap.c
index f54b56388e00..350f6b615e91 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)
@@ -424,3 +425,5 @@ void iova_bitmap_set(struct iova_bitmap *bitmap,
} while (cur_bit <= last_bit);
}
EXPORT_SYMBOL_GPL(iova_bitmap_set);
+
+MODULE_LICENSE("GPL v2");
