On 04.01.2023 09:44, Xenia Ragiadakou wrote:
> The functions acpi_dmar_init() and acpi_dmar_zap/reinstate() are
> VT-d specific while the function acpi_ivrs_init() is AMD-Vi specific.
> To eliminate dead code, they need to be guarded under CONFIG_INTEL_IOMMU
> and CONFIG_AMD_IOMMU, respectively.
>
> Instead of adding #ifdef guards around the function calls, implement them
> as empty static inline functions.
>
> Take the opportunity to move the declarations of acpi_dmar_zap/reinstate() to
> the arch specific header.
>
> No functional change intended.
>
> Signed-off-by: Xenia Ragiadakou <[email protected]>
While I'm not opposed to ack the change in this form, I have a question
first:
> --- a/xen/arch/x86/include/asm/acpi.h
> +++ b/xen/arch/x86/include/asm/acpi.h
> @@ -140,8 +140,22 @@ extern u32 pmtmr_ioport;
> extern unsigned int pmtmr_width;
>
> void acpi_iommu_init(void);
> +
> +#ifdef CONFIG_INTEL_IOMMU
> int acpi_dmar_init(void);
> +void acpi_dmar_zap(void);
> +void acpi_dmar_reinstate(void);
> +#else
> +static inline int acpi_dmar_init(void) { return -ENODEV; }
> +static inline void acpi_dmar_zap(void) {}
> +static inline void acpi_dmar_reinstate(void) {}
> +#endif
Leaving aside my request to drop that part of patch 3, you've kept
declarations for VT-d in the common header there. Which I consider
correct, knowing that VT-d was also used on IA-64 at the time. As
a result I would suppose movement might better be done in the other
direction here.
> +#ifdef CONFIG_AMD_IOMMU
> int acpi_ivrs_init(void);
> +#else
> +static inline int acpi_ivrs_init(void) { return -ENODEV; }
> +#endif
For AMD, otoh, without there being a 2nd architecture re-using
their IOMMU, moving into the x86-specific header is certainly fine,
no matter that there's a slim chance that this may need moving the
other direction down the road.
Jan
