Messages in this thread | | | From | Michael Shavit <> | Date | Tue, 15 Aug 2023 20:10:31 +0800 | Subject | Re: [PATCH v5 6/9] iommu/arm-smmu-v3: Move CD table to arm_smmu_master |
| |
On Wed, Aug 9, 2023 at 1:15 AM Michael Shavit <mshavit@google.com> wrote: >
> -static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain, > +static int arm_smmu_domain_finalise_cd(struct arm_smmu_domain *smmu_domain, > struct arm_smmu_master *master, > struct io_pgtable_cfg *pgtbl_cfg) > { > @@ -2115,10 +2110,6 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain, > if (ret) > goto out_unlock; > > - ret = arm_smmu_alloc_cd_tables(smmu_domain, master); > - if (ret) > - goto out_free_asid; > - > cd->asid = (u16)asid; > cd->ttbr = pgtbl_cfg->arm_lpae_s1_cfg.ttbr; > cd->tcr = FIELD_PREP(CTXDESC_CD_0_TCR_T0SZ, tcr->tsz) | > @@ -2130,17 +2121,9 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain, > CTXDESC_CD_0_TCR_EPD1 | CTXDESC_CD_0_AA64; > cd->mair = pgtbl_cfg->arm_lpae_s1_cfg.mair; > > - ret = arm_smmu_write_ctx_desc(master, 0, cd); > - if (ret) > - goto out_free_cd_tables; > - > mutex_unlock(&arm_smmu_asid_lock); > return 0; > > -out_free_cd_tables: > - arm_smmu_free_cd_tables(smmu_domain); > -out_free_asid: > - arm_smmu_free_asid(cd); > out_unlock: > mutex_unlock(&arm_smmu_asid_lock); > return ret; ... > @@ -2465,6 +2450,22 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) > if (smmu_domain->stage != ARM_SMMU_DOMAIN_BYPASS) > master->ats_enabled = arm_smmu_ats_supported(master); > > + if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) { > + if (!master->cd_table.cdtab) { > + ret = arm_smmu_alloc_cd_tables(master); > + if (ret) { > + master->domain = NULL; > + return ret; > + } > + } > + > + ret = arm_smmu_write_ctx_desc(master, 0, &smmu_domain->cd); > + if (ret) { > + master->domain = NULL; > + return ret; > + } > + } > + > arm_smmu_install_ste_for_dev(master); > > spin_lock_irqsave(&smmu_domain->devices_lock, flags); > @@ -2472,10 +2473,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) > spin_unlock_irqrestore(&smmu_domain->devices_lock, flags); > > arm_smmu_enable_ats(master);
All this talk of locking on the other thread made me realize there's an issue here. We are no longer holding the arm_smmu_asid_lock while arm_smmu_write_ctx_desc is called due to its move out of arm_smmu_domain_finalise_s1. This can race with arm_smmu_share_asid which may modify the asid after we've written it, but before we've updated the CD's smmu_domain->devices list.
| |