Re: [PATCH v3 3/3] xen/pci: solve compilation error on ARM with HAS_PCI enabled.

[Jan Beulich <jbeulich@suse.com>]

On 16.11.2020 13:25, Rahul Singh wrote:
> If mem-sharing, mem-paging, or log-dirty functionality is not enabled
> for non-x86 architecture when HAS_PCI is enabled, the compiler will
> throw an error.
> 
> Move code to x86 specific directory to fix compilation error.

Perhaps rather "file" than "directory"?

> Also, modify the code to use likely() in place of unlikley() for each
> condition to make code more optimized.
> 
> No functional change.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>

In principle I'm okay with this now, but there continue to be a few
nits:

> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -23,6 +23,7 @@
>  #include <asm/hvm/io.h>
>  #include <asm/io_apic.h>
>  #include <asm/setup.h>
> +#include <xen/vm_event.h>

Please insert this alongside the other "#include <xen/...>" higher up.

> @@ -315,6 +316,17 @@ int iommu_update_ire_from_msi(
>             ? iommu_call(&iommu_ops, update_ire_from_msi, msi_desc, msg) : 0;
>  }
>  
> +bool arch_iommu_use_permitted(const struct domain *d)
> +{
> +    /*
> +     * Prevent device assign if mem paging, mem sharing or log-dirty
> +     * have been enabled for this domain.
> +     */
> +    return d == dom_io ||
> +           (likely(!mem_sharing_enabled(d)) &&
> +            likely(!vm_event_check_ring(d->vm_event_paging)) &&
> +            likely(!p2m_get_hostp2m(d)->global_logdirty));
> +}
>  /*
>   * Local variables:
>   * mode: C

Please don't alter stylistic aspects like this trailing comment
being preceded by a blank line.

> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -381,6 +381,8 @@ DECLARE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
>  extern struct spinlock iommu_pt_cleanup_lock;
>  extern struct page_list_head iommu_pt_cleanup_list;
>  
> +bool arch_iommu_use_permitted(const struct domain *d);

Just FTR - this way you effectively preclude an arch from
making this a trivial static inline in one of its headers.

Jan