Re: [PATCH 1/1] arm/xen: don't use xen DMA ops when the device is protected by an IOMMU

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 22 Dec 2021, Sergiy Kibrik wrote:
> Only Xen is able to know if a device can safely avoid to use xen-swiotlb.
> However since Xen links FDT nodes of protected devices to special dummy
> xen-iommu node we can use that information to decide whether
> xen-swiotlb is needed.
> 
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
> ---
>  arch/arm/mm/dma-mapping.c   | 2 +-
>  arch/arm/xen/enlighten.c    | 9 +++++++++
>  arch/arm64/mm/dma-mapping.c | 2 +-
>  include/xen/swiotlb-xen.h   | 1 +
>  4 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index c4b8df2ad328..fc875dd16e0e 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -2280,7 +2280,7 @@ void arch_setup_dma_ops(struct device *dev, u64 
> dma_base, u64 size,
>       set_dma_ops(dev, dma_ops);
>  
>  #ifdef CONFIG_XEN
> -     if (xen_initial_domain())
> +     if (xen_initial_domain() && !xen_is_protected_device(dev))
>               dev->dma_ops = &xen_swiotlb_dma_ops;
>  #endif
>       dev->archdata.dma_ops_setup = true;
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 49f566ad9acb..b36659238db3 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -66,6 +66,15 @@ static __read_mostly unsigned int xen_events_irq;
>  uint32_t xen_start_flags;
>  EXPORT_SYMBOL(xen_start_flags);
>  
> +bool xen_is_protected_device(struct device *dev)
> +{
> +     struct fwnode_handle *fwnode =
> +             fwnode_find_reference(dev_fwnode(dev), "iommus", 0) ;
> +     if (IS_ERR(fwnode))
> +             return false;
> +     return of_device_is_compatible(to_of_node(fwnode), "xen,iommu-el2-v1");
> +}

We need to add a description of the "xen,iommu-el2-v1" compatible node
under Documentation/devicetree/bindings. Maybe it could be added to
Documentation/devicetree/bindings/arm/xen.txt, but it could also be its
own new file.


>  int xen_unmap_domain_gfn_range(struct vm_area_struct *vma,
>                              int nr, struct page **pages)
>  {
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 93e87b287556..68248e72e052 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -53,7 +53,7 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, 
> u64 size,
>               iommu_setup_dma_ops(dev, dma_base, size);
>  
>  #ifdef CONFIG_XEN
> -     if (xen_initial_domain())
> +     if (xen_initial_domain() && !xen_is_protected_device(dev))
>               dev->dma_ops = &xen_swiotlb_dma_ops;

This patch needs to be rebased on the latest master. You'll see that now
we have a more sophisticated xen_swiotlb_detect(), instead of the simple
xen_initial_domain() we used to have. Still, xen_swiotlb_detect() is
global, not per device, so I think this change would still apply as is,
resulting in:

if (xen_swiotlb_detect() && !xen_is_protected_device(dev))
    dev->dma_ops = &xen_swiotlb_dma_ops;


>  #endif
>  }
> diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
> index d5eaf9d682b8..00b2782430fb 100644
> --- a/include/xen/swiotlb-xen.h
> +++ b/include/xen/swiotlb-xen.h
> @@ -8,6 +8,7 @@ void xen_dma_sync_for_cpu(struct device *dev, dma_addr_t 
> handle,
>                         size_t size, enum dma_data_direction dir);
>  void xen_dma_sync_for_device(struct device *dev, dma_addr_t handle,
>                            size_t size, enum dma_data_direction dir);
> +bool xen_is_protected_device(struct device *dev);
>  
>  extern int xen_swiotlb_init(int verbose, bool early);
>  extern const struct dma_map_ops xen_swiotlb_dma_ops;