On Mon, Oct 23, 2023, at 16:19, Jason Gunthorpe wrote:
> On Mon, Oct 23, 2023 at 04:02:11PM +0200, Arnd Bergmann wrote:
>> On Mon, Oct 23, 2023, at 15:23, Jason Gunthorpe wrote:
>> >
>> > I think the right thing is to combine IOMMU_SUPPORT and IOMMU_API into
>> > the same thing.
>>
>> I've had a closer look now and I think the way it is currently
>> designed to be used makes some sense: IOMMU implementations almost
>> universally depend on both a CPU architecture and CONFIG_IOMMU_SUPPORT,
>> but select IOMMU_API. So if you enable IOMMU_SUPPORT on an
>> architecture that has no IOMMU implementations, none of the drivers
>> are visible and nothing happens. Similarly, almost all drivers
>> using the IOMMU interface depend on IOMMU_API, so they can only
>> be built if at least one IOMMU driver is configured.
>
> Maybe, but I don't think we need such micro-optimization.
>
> If someone selects 'enable IOMMU support' and doesn't turn on any
> drivers then they should still get the core API. That is how a lot of
> the kconfig stuff typically works in the kernel.
Agreed, that would be fine as well, and I agree it is less confusing.
A similar approach as in iommu is used in a couple of other subsystems
(regmap, gpiolib, sound, mfd, phy, virtio) where each provider selects
the subsystem as a library, but I'm not a huge fan of this either.
It's usually just easier to not make fundamental changes like this.
In this case, we can just use 'depends on' for one of the two
symbols everywhere and use to control both the providers and
consumers.
> Similarly, if they don't select 'enable IOMMU support' then they
> definitely shouldn't quitely get the core API turned on!
...
>> diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
>> index 4a79704b164f7..2902b89a48f17 100644
>> --- a/drivers/gpu/drm/nouveau/Kconfig
>> +++ b/drivers/gpu/drm/nouveau/Kconfig
>> @@ -4,7 +4,7 @@ config DRM_NOUVEAU
>> depends on DRM && PCI && MMU
>> depends on (ACPI_VIDEO && ACPI_WMI && MXM_WMI) || !(ACPI && X86)
>> depends on BACKLIGHT_CLASS_DEVICE
>> - select IOMMU_API
>> + depends on IOMMU_API
>> select FW_LOADER
>> select DRM_DISPLAY_DP_HELPER
>> select DRM_DISPLAY_HDMI_HELPER
>
> Like here, nouveau should still be compilable even if no iommu driver
> was selected, and it should compile on arches without iommu drivers at
> all.
Right, so with my draft patch, we can't build nouveau without
having an IOMMU driver, which matches the original intention
behind the Kconfig logic, while your suggestion would add the
same dependency here but still allow it to be compile tested
on target systems with no IOMMU. A minor downside of your
approach is that you end up building drivers (without
CONFIG_COMPILE_TEST) that currently exclude because we know
they will never work.
Arnd
