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

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

On Thu, May 11, 2017 at 2:28 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
> Hi Oleksandr,
Hi Julien

>
> 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;
> }
I got it for SMMU.

But, for IPMMU we will override since iommu_hap_pt_share is true by
default. Right?

/*
* The IPMMU can't reuse P2M table since it only supports
* stage-1 page tables.
*/
iommu_hap_pt_share = false;

>
>>         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.".
Agree.

>
>> + */
>> +#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


-- 
Regards,

Oleksandr Tyshchenko

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