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
