Re: [Xen-devel] [PATCH v1 05/10] iommu/arm: Re-define iommu_use_hap_pt(d) as iommu_hap_pt_share

[Julien Grall <julien.grall@xxxxxxx>]

Hi Oleksandr,

On 10/05/17 15:03, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>
> Not every integrated into ARM SoCs IOMMU can share page tables
> with the CPU and as result the iommu_use_hap_pt(d) is not always true.
> Reuse x86's iommu_hap_pt_share flag to indicate whether the IOMMU
> page table is shared or not.
>
> Now all IOMMU drivers on ARM are able to change this flag
> according to their possibilities like x86-variants do.
> Therefore set iommu_hap_pt_share flag for SMMU because it always shares
> page table with the CPU.
>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> ---
>  xen/drivers/passthrough/arm/smmu.c | 3 +++
>  xen/include/asm-arm/iommu.h        | 7 +++++--
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/xen/drivers/passthrough/arm/smmu.c 
> b/xen/drivers/passthrough/arm/smmu.c
> index 527a592..86ee12a 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -2870,6 +2870,9 @@ static __init int arm_smmu_dt_init(struct dt_device_node 
> *dev,
>
>         platform_features &= smmu->features;
>
> +       /* Always share P2M table between the CPU and the SMMU */
> +       iommu_hap_pt_share = true;
> +
I would prefer to bail-out if someone try to unshare the page-table 
rather than overriding. This would help us to know if someone are try to 
do that.
So I would do:

if ( !iommu_hap_pt_share )
{
        printk(....)
        return -EINVAL;
}

>         return 0;
>  }
>
> diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
> index 57d9b1e..10a6f23 100644
> --- a/xen/include/asm-arm/iommu.h
> +++ b/xen/include/asm-arm/iommu.h
> @@ -20,8 +20,11 @@ struct arch_iommu
>      void *priv;
>  };
>
> -/* Always share P2M Table between the CPU and the IOMMU */
> -#define iommu_use_hap_pt(d) (1)
> +/*
> + * The ARM domain always has a P2M table, but not every integrated into
> + * ARM SoCs IOMMU can use it as page table.
The first part: "ARM domain has a P2M table" is pretty obvious. I would 
instead say: "Not every ARM SoCs IOMMU use the same page-table format as 
the processor.".
> + */
> +#define iommu_use_hap_pt(d) (iommu_hap_pt_share)
>
>  const struct iommu_ops *iommu_get_ops(void);
>  void __init iommu_set_ops(const struct iommu_ops *ops);
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel