Re: [PATCH v4 18/21] xen/arm: p2m: Clean cache PT when the IOMMU doesn't support coherent walk

[Julien Grall <julien.grall@linaro.org>]

I forgot to cc Jan and Xiantao on this patch.

On 04/22/2014 02:14 PM, Julien Grall wrote:
> Some IOMMU doesn't suppport coherent PT walk. When the p2m is shared with
> the CPU, Xen has to make sure the PT changes have reached the memory.
> 
> Introduce new IOMMU callback that will retrieve the IOMMU feature for a
> specified domain.
> 
> On ARM, the platform can contain multiple IOMMUs. Each of them may not
> have the same set of feature. The domain parameter will be used to get the
> set of features for IOMMUs used by this domain.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> 
> ---
>     Changes in v4:
>         - Patch added
> ---
>  xen/arch/arm/p2m.c              |   24 ++++++++++++++++++------
>  xen/drivers/passthrough/iommu.c |   11 +++++++++++
>  xen/include/xen/iommu.h         |    5 +++++
>  3 files changed, 34 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 21219de..996d2bd 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -274,6 +274,18 @@ enum p2m_operation {
>      CACHEFLUSH,
>  };
>  
> +static void unmap_coherent_domain_page(struct domain *d, const void *va)
> +{
> +    /* Some IOMMU doesn't support coherent PT walk. When the p2m is
> +     * shared with the CPU, Xen has to make sure that the PT changes have
> +     * reached the memory
> +     */
> +    if ( need_iommu(d) && !iommu_has_feature(d, IOMMU_FEAT_COHERENT_WALK) )
> +        clean_xen_dcache_va_range(va, PAGE_SIZE);
> +
> +    unmap_domain_page(va);
> +}
> +
>  static int apply_p2m_changes(struct domain *d,
>                       enum p2m_operation op,
>                       paddr_t start_gpaddr,
> @@ -301,7 +313,7 @@ static int apply_p2m_changes(struct domain *d,
>      {
>          if ( cur_first_page != p2m_first_level_index(addr) )
>          {
> -            if ( first ) unmap_domain_page(first);
> +            if ( first ) unmap_coherent_domain_page(d, first);
>              first = p2m_map_first(p2m, addr);
>              if ( !first )
>              {
> @@ -331,7 +343,7 @@ static int apply_p2m_changes(struct domain *d,
>  
>          if ( cur_first_offset != first_table_offset(addr) )
>          {
> -            if (second) unmap_domain_page(second);
> +            if (second) unmap_coherent_domain_page(d, second);
>              second = map_domain_page(first[first_table_offset(addr)].p2m.base);
>              cur_first_offset = first_table_offset(addr);
>          }
> @@ -357,7 +369,7 @@ static int apply_p2m_changes(struct domain *d,
>          if ( cur_second_offset != second_table_offset(addr) )
>          {
>              /* map third level */
> -            if (third) unmap_domain_page(third);
> +            if (third) unmap_coherent_domain_page(d, third);
>              third = map_domain_page(second[second_table_offset(addr)].p2m.base);
>              cur_second_offset = second_table_offset(addr);
>          }
> @@ -480,9 +492,9 @@ static int apply_p2m_changes(struct domain *d,
>      rc = 0;
>  
>  out:
> -    if (third) unmap_domain_page(third);
> -    if (second) unmap_domain_page(second);
> -    if (first) unmap_domain_page(first);
> +    if (third) unmap_coherent_domain_page(d, third);
> +    if (second) unmap_coherent_domain_page(d, second);
> +    if (first) unmap_coherent_domain_page(d, first);
>  
>      spin_unlock(&p2m->lock);
>  
> diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
> index f93dc79..f24fb46 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -344,6 +344,17 @@ void iommu_crash_shutdown(void)
>      iommu_enabled = iommu_intremap = 0;
>  }
>  
> +bool_t iommu_has_feature(struct domain *d, uint32_t feature)
> +{
> +    const struct iommu_ops *ops = domain_hvm_iommu(d)->platform_ops;
> +    uint32_t features = 0;
> +
> +    if ( iommu_enabled && ops && ops->features )
> +        features = ops->features(d);
> +
> +    return !!(features & feature);
> +}
> +
>  static void iommu_dump_p2m_table(unsigned char key)
>  {
>      struct domain *d;
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index e119379..9ad909f 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -67,6 +67,10 @@ int iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
>                     unsigned int flags);
>  int iommu_unmap_page(struct domain *d, unsigned long gfn);
>  
> +#define IOMMU_FEAT_COHERENT_WALK (1U<<0)
> +bool_t iommu_has_feature(struct domain *d, uint32_t feature);
> +
> +
>  #ifdef HAS_PCI
>  void pt_pci_init(void);
>  
> @@ -139,6 +143,7 @@ struct iommu_ops {
>      void (*iotlb_flush)(struct domain *d, unsigned long gfn, unsigned int page_count);
>      void (*iotlb_flush_all)(struct domain *d);
>      void (*dump_p2m_table)(struct domain *d);
> +    uint32_t (*features)(struct domain *d);
>  };
>  
>  void iommu_suspend(void);
> 


-- 
Julien Grall