Re: [PATCH] iommu: always select INTEL_IOMMU for IRQ_REMAP

[Robin Murphy]

On 07/03/2024 2:05 pm, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@xxxxxxxx>
>
> CONFIG_INTR_REMAP was originally split out of the intel iommu code to be
> shared by IRQ_REMAP. This recently broke again because the IRQ_REMAP
> code calls the global device_rbtree_find() function that is unavailable
> for builds without INTEL_IOMMU:
>
> x86_64-linux-ld: vmlinux.o: in function `qi_submit_sync':
> (.text+0x10771e0): undefined reference to `device_rbtree_find'
>
> It seems that the intel iommu code now contains a lot of generic helper
> functions that are not specific to intel, such as alloc_pgtable_page(),
> iommu_flush_write_buffer(), domain_attach_iommu() etc, so presumably
> it is not x86 specific any more.

No, it's still all very much Intel-specific, which in fact means it is 
just recently now x86-specific since IA-64 has departed.

Historically it's always been the case that building IRQ remapping 
support on its own without IOMMU_API was supported, and for a while we 
even had the awkward iommu_device_set_ops() wrapper and various other 
indirections and stubs in the core API solely to make it work. IMO the 
underlying issue here is that there have never been very clear lines of 
separation between the ACPI DMAR code, the IOMMU API driver, and the IRQ 
remapping driver, so unless the whole design could be improved to make 
it harder to break, it probably is time to start asking the question of 
whether anyone actually cares about this config combination any more.

Thanks,
Robin.

> Fix the build failure for now by just selectin INTEL_IOMMU by the
> code that relies on it. It might be helpful to split out all the
> functions without an intel_iommu_* prefix into a helper library
> to avoid including the x86 specific bits on non-x86, but that could
> be a follow-up.
>
> Fixes: d3f138106b4b ("iommu: Rename the DMAR and INTR_REMAP config options")
> Fixes: 80a9b50c0b9e ("iommu/vt-d: Improve ITE fault handling if target device isn't present")
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---
> Not sure about this one, I just ran across the build regression and
> wasn't sure if the intel-iommu functions are meant to be generic
> or just misnamed. The patch description assumes the former, if that
> is wrong, it needs a different explanation or a different fix.
> ---
>   drivers/iommu/Kconfig | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index df156f0a1a17..da5339bdb7e7 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -196,6 +196,7 @@ config IRQ_REMAP
>   	bool "Support for Interrupt Remapping"
>   	depends on X86_64 && X86_IO_APIC && PCI_MSI && ACPI
>   	select DMAR_TABLE
> +	select INTEL_IOMMU
>   	help
>   	  Supports Interrupt remapping for IO-APIC and MSI devices.
>   	  To use x2apic mode in the CPU's which support x2APIC enhancements or