Hi Xiang,

On Mon, Jul 06, 2020 at 08:40:27PM +0800, Xiang Zheng wrote:
> > @@ -1836,7 +1880,20 @@ static struct arm_smmu_ctx_desc 
> > *arm_smmu_alloc_shared_cd(struct mm_struct *mm)
> >  
> >     arm_smmu_init_cd(cd);
> >  
> > +   /*
> > +    * Serialize against arm_smmu_domain_finalise_s1() and
> > +    * arm_smmu_domain_free() as we might need to replace the private ASID
> > +    * from an existing CD.
> > +    */
> > +   mutex_lock(&asid_lock);
> >     old_cd = arm_smmu_share_asid(asid);
> > +   if (!old_cd) {
> > +           ret = xa_insert(&asid_xa, asid, cd, GFP_KERNEL);
> 
> Should we use "xa_store" here? If "asid" has already been used for private, 
> old_cd would be NULL and
> the entry indexed by "asid" in the asid_xa remains.

Great catch, that's a bug introduced in v7. arm_smmu_share_asid() would
allocate a new asid for the private context but does not remove the old
entry. For the fix I think it looks clearer if arm_smmu_share_asid()
erases the old entry before returning.

Thanks,
Jean

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to