| Message ID | 20191219163033.2608177-10-jean-philippe@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,01/13] iommu/arm-smmu-v3: Drop __GFP_ZERO flag from DMA allocation | expand |
On Thu, Dec 19, 2019 at 05:30:29PM +0100, Jean-Philippe Brucker wrote: > Second-level context descriptor tables will be allocated lazily in > arm_smmu_write_ctx_desc(). Help with handling allocation failure by > moving the CD write into arm_smmu_domain_finalise_s1(). > > Reviewed-by: Eric Auger <eric.auger@redhat.com> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > --- > drivers/iommu/arm-smmu-v3.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index e147087198ef..b825a5639afc 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -2301,8 +2301,15 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain, > cfg->cd.ttbr = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0]; > cfg->cd.tcr = pgtbl_cfg->arm_lpae_s1_cfg.tcr; > cfg->cd.mair = pgtbl_cfg->arm_lpae_s1_cfg.mair; > + > + ret = arm_smmu_write_ctx_desc(smmu_domain, 0, &cfg->cd); Hmm. This ends up calling arm_smmu_sync_cd() but I think that happens before we've added the master to the devices list of the domain. Does that mean we miss the new SSID during the invalidation? > + if (ret) > + goto out_free_tables; > + > return 0; > > +out_free_tables: nit: We have more tables in this driver than you can shake a stick at, so please rename the label "out_free_cd_tables" or something like that. Will
On Tue, Jan 14, 2020 at 12:42:47PM +0000, Will Deacon wrote: > On Thu, Dec 19, 2019 at 05:30:29PM +0100, Jean-Philippe Brucker wrote: > > Second-level context descriptor tables will be allocated lazily in > > arm_smmu_write_ctx_desc(). Help with handling allocation failure by > > moving the CD write into arm_smmu_domain_finalise_s1(). > > > > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > --- > > drivers/iommu/arm-smmu-v3.c | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > > index e147087198ef..b825a5639afc 100644 > > --- a/drivers/iommu/arm-smmu-v3.c > > +++ b/drivers/iommu/arm-smmu-v3.c > > @@ -2301,8 +2301,15 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain, > > cfg->cd.ttbr = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0]; > > cfg->cd.tcr = pgtbl_cfg->arm_lpae_s1_cfg.tcr; > > cfg->cd.mair = pgtbl_cfg->arm_lpae_s1_cfg.mair; > > + > > + ret = arm_smmu_write_ctx_desc(smmu_domain, 0, &cfg->cd); > > Hmm. This ends up calling arm_smmu_sync_cd() but I think that happens before > we've added the master to the devices list of the domain. Does that mean we > miss the new SSID during the invalidation? Yes, the arm_smmu_sync_cd() isn't useful in this case, it's only needed when the STE is live and arm_smmu_write_ctx_desc() is called for a ssid!=0. On this path, the CD cache is invalidated by a CFGI_STE executed later, when arm_smmu_attach_dev() installs the STE. I didn't want to add a special case that avoids the sync when ssid==0 in because a spurious sync probably doesn't impact performance here and arm_smmu_write_ctx_desc() is quite fiddly already. Thanks, Jean
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index e147087198ef..b825a5639afc 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -2301,8 +2301,15 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain, cfg->cd.ttbr = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0]; cfg->cd.tcr = pgtbl_cfg->arm_lpae_s1_cfg.tcr; cfg->cd.mair = pgtbl_cfg->arm_lpae_s1_cfg.mair; + + ret = arm_smmu_write_ctx_desc(smmu_domain, 0, &cfg->cd); + if (ret) + goto out_free_tables; + return 0; +out_free_tables: + arm_smmu_free_cd_tables(smmu_domain); out_free_asid: arm_smmu_bitmap_free(smmu->asid_map, asid); return ret; @@ -2569,10 +2576,6 @@ 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) - arm_smmu_write_ctx_desc(smmu_domain, 0, - &smmu_domain->s1_cfg.cd); - arm_smmu_install_ste_for_dev(master); spin_lock_irqsave(&smmu_domain->devices_lock, flags);