Re: [XEN PATCH] x86/iommu: Conditionally compile platform-specific union entries

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, May 23, 2024 at 09:19:53AM +0000, Teddy Astie wrote:
> If some platform driver isn't compiled in, remove its related union
> entries as they are not used.
> 
> Signed-off-by Teddy Astie <teddy.astie@xxxxxxxxxx>
> ---
>  xen/arch/x86/include/asm/iommu.h | 4 ++++
>  xen/arch/x86/include/asm/pci.h   | 4 ++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/xen/arch/x86/include/asm/iommu.h 
> b/xen/arch/x86/include/asm/iommu.h
> index 8dc464fbd3..99180940c4 100644
> --- a/xen/arch/x86/include/asm/iommu.h
> +++ b/xen/arch/x86/include/asm/iommu.h
> @@ -42,17 +42,21 @@ struct arch_iommu
>      struct list_head identity_maps;
>  
>      union {
> +        #ifdef CONFIG_INTEL_IOMMU
>          /* Intel VT-d */
>          struct {
>              uint64_t pgd_maddr; /* io page directory machine address */
>              unsigned int agaw; /* adjusted guest address width, 0 is level 2 
> 30-bit */
>              unsigned long *iommu_bitmap; /* bitmap of iommu(s) that the 
> domain uses */
>          } vtd;
> +        #endif
> +        #ifdef CONFIG_AMD_IOMMU
>          /* AMD IOMMU */
>          struct {
>              unsigned int paging_mode;
>              struct page_info *root_table;
>          } amd;
> +        #endif
>      };
>  };
>  
> diff --git a/xen/arch/x86/include/asm/pci.h b/xen/arch/x86/include/asm/pci.h
> index fd5480d67d..842710f0dc 100644
> --- a/xen/arch/x86/include/asm/pci.h
> +++ b/xen/arch/x86/include/asm/pci.h
> @@ -22,12 +22,16 @@ struct arch_pci_dev {
>       */
>      union {
>          /* Subset of struct arch_iommu's fields, to be used in dom_io. */
> +        #ifdef CONFIG_INTEL_IOMMU
>          struct {
>              uint64_t pgd_maddr;
>          } vtd;
> +        #endif
> +        #ifdef CONFIG_AMD_IOMMU
>          struct {
>              struct page_info *root_table;
>          } amd;
> +        #endif
>      };

The #ifdef and #endif processor directives shouldn't be indented.

Would you mind adding /* CONFIG_{AMD,INTEL}_IOMMU */ comments in the
#endif directives?

I wonder if we could move the definitions of those structures to the
vendor specific headers, but that's more convoluted, and would require
including the iommu headers in pci.h

Thanks, Roger.