Re: [Xen-devel] [PATCH v3 13/13] xen/iommu: smmu: Advertise when the SMMU support coherent table walk

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Fri, 2015-02-20 at 14:07 +0000, Julien Grall wrote:
> On 20/02/15 13:34, Ian Campbell wrote:
> > On Fri, 2015-01-30 at 18:49 +0000, Julien Grall wrote:
> >> @@ -2896,6 +2911,16 @@ static __init int arm_smmu_dt_init(struct 
> >> dt_device_node *dev,
> >>    if ( !rc )
> >>            iommu_set_ops(&arm_smmu_iommu_ops);
> >>  
> >> +  /*
> >> +   * The last added SMMU is the first element of arm_smmu_devices.
> >> +   * It's not necessary to take the lock because only the boot CPU is
> >> +   * initialized the SMMU devices.
> > 
> > Why is only the last added SMMU of interest? Do we not need to take the
> > union and/or intersection of them all?
> 
> It's already the case. The function arm_smmu_dt_init is called on every
> SMMU. So the last added SMMU is the one we are currently added.

Why do we not just have it in our hand and have to go scrobbling round
in a list then?

> > Perhaps the code which calls iommu_set_feature should gain an else which
> > calls iommu_clear_feature, and between them they can ensure that
> > platform_features is correctly updated?
> 
> iommu_{set,clear}_feature is a generic code and per-domain.

Ah, ok.

> The main issue is if we add device protected by SMMU which doesn't have
> coherent walk, we have to browse the page table and clean the cache.
> 
> So we have to know at boot time that one of the SMMU doesn't have
> coherent walk.

Rather than making assumptions about the list ordering and if we can't
just get hold of the smmu pointer directly from arm_smmu_dt_init then
I'd prefer an explicit walk of the list at some appropriate point after
everything has been registered up.

Ian.


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