[PATCH] iommu: always select INTEL_IOMMU for IRQ_REMAP

[PATCH] iommu: always select INTEL_IOMMU for IRQ_REMAP

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

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.

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@arndb.de>
---
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
-- 
2.39.2

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@arndb.de>
> 
> 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@arndb.de>
> ---
> 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

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

[Baolu Lu]

On 3/7/24 11:48 PM, Robin Murphy wrote:
> On 07/03/2024 2:05 pm, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> 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.

IOMMU in passthrough mode + interrupt remapping might be a reasonable
alternative to no IOMMU API + interrupt remapping.

Best regards,
baolu