Thread (45 messages) 45 messages, 3 authors, 2023-08-15

Re: [PATCH v5 6/9] iommu/arm-smmu-v3: Move CD table to arm_smmu_master

From: Michael Shavit <hidden>
Date: 2023-08-15 12:11:45
Also in: linux-iommu, lkml

On Wed, Aug 9, 2023 at 1:15 AM Michael Shavit [off-list ref] wrote:
quoted hunk ↗ jump to hunk
-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;
...
quoted hunk ↗ jump to hunk
@@ -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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help