Re: [Xen-devel] [RFC] arm/p2m: call iommu iotlb flush if iommu exists and enabled

[Julien Grall <julien.grall@xxxxxxx>]

(+ Paul)

Hello,

On 23/01/2019 10:12, Andrii Anisov wrote:
> From: Andrii Anisov <andrii_anisov@xxxxxxxx>
>
> Taking decission by `need_iommu_pt_sync()` make us never kicking
s/decission/decision/

> `iommu_iotlb_flush()` for IOMMUs which do share TLB with CPU.
I am not aware of platform where we share the TLB with the CPU. Do you mean 
sharing the P2M?
> So check `has_iommu_pt()` instead.
>
> Signed-off-by: Andrii Anisov <andrii_anisov@xxxxxxxx>
>
> ---
>
> Julien,
>
> Could you please look at this, IMO there is a mistake here.
> x86 uses `need_iommu_pt_sync()` to make decission if iommu's map/unmap should 
> be additionally called.
> But ARM has no non-shared pt support in the mainline, so using 
> `need_iommu_pt_sync()` seems to be odd.
>
>   xen/arch/arm/p2m.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 2394f97..059a391 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1019,7 +1019,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
>            !mfn_eq(lpae_get_mfn(*entry), lpae_get_mfn(orig_pte)) )
>           p2m_free_entry(p2m, orig_pte, level);
>   
> -    if ( need_iommu_pt_sync(p2m->domain) &&
> +    if ( has_iommu_pt(p2m->domain) &&
I think this makes sense because we want to flush the TLB when the P2M is 
shared. Although, I would like to hear Paul's opinion here.
>            (lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) )
>       {
>           unsigned int flush_flags = 0; >
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel