| Message ID | 20230822185632.RFC.v2.4.I326c62dc062aed8d901d319aa665dbe983c7904c@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Install domain onto multiple smmus | expand |
On Tue, Aug 22, 2023 at 06:57:00PM +0800, Michael Shavit wrote: > Pick an ASID that is within the supported range of all SMMUs that the > domain is installed to. > > Signed-off-by: Michael Shavit <mshavit@google.com> > --- > > (no changes since v1) > > .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 23 +++++++++++++++---- > 1 file changed, 19 insertions(+), 4 deletions(-) > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > index fe88a7880ad57..92d2f8c4e90a8 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > @@ -66,6 +66,20 @@ static int arm_smmu_write_ctx_desc_devices(struct arm_smmu_domain *smmu_domain, > return ret; > } > > +static u32 arm_smmu_domain_max_asid_bits(struct arm_smmu_domain *smmu_domain) > +{ > + struct arm_smmu_master *master; > + unsigned long flags; > + u32 asid_bits = 16; > + > + spin_lock_irqsave(&smmu_domain->devices_lock, flags); > + list_for_each_entry(master, &smmu_domain->devices, > + domain_head) > + asid_bits = min(asid_bits, master->smmu->asid_bits); > + spin_unlock_irqrestore(&smmu_domain->devices_lock, flags); > + return asid_bits; > +} I still don't like this, it is not locked properly. You release the devices_lock which means the max_asid could change before we get to arm_smmu_write_ctx_desc() If you want to take this shortcut temporarily then a global max_asid is probably a better plan. Change it to a per-master allocation later to remove that. Jason
On Tue, Aug 22, 2023 at 9:19 PM Jason Gunthorpe <jgg@nvidia.com> wrote: > > On Tue, Aug 22, 2023 at 06:57:00PM +0800, Michael Shavit wrote: > > Pick an ASID that is within the supported range of all SMMUs that the > > domain is installed to. > > > > Signed-off-by: Michael Shavit <mshavit@google.com> > > --- > > > > (no changes since v1) > > > > .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 23 +++++++++++++++---- > > 1 file changed, 19 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > > index fe88a7880ad57..92d2f8c4e90a8 100644 > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > > @@ -66,6 +66,20 @@ static int arm_smmu_write_ctx_desc_devices(struct arm_smmu_domain *smmu_domain, > > return ret; > > } > > > > +static u32 arm_smmu_domain_max_asid_bits(struct arm_smmu_domain *smmu_domain) > > +{ > > + struct arm_smmu_master *master; > > + unsigned long flags; > > + u32 asid_bits = 16; > > + > > + spin_lock_irqsave(&smmu_domain->devices_lock, flags); > > + list_for_each_entry(master, &smmu_domain->devices, > > + domain_head) > > + asid_bits = min(asid_bits, master->smmu->asid_bits); > > + spin_unlock_irqrestore(&smmu_domain->devices_lock, flags); > > + return asid_bits; > > +} > > I still don't like this, it is not locked properly. You release the > devices_lock which means the max_asid could change before we get to > arm_smmu_write_ctx_desc() Good point. > If you want to take this shortcut temporarily then a global max_asid > is probably a better plan. Change it to a per-master allocation later > to remove that. Two options there: 1. When allocating a new ASID in arm_smmu_share_asid, limit ourselves to 8-bit-wide ASIDs regardless of whether all the installed SMMUs support 16bit ASIDs. 2. In addition, also use a maximum 8-bit-wide ASID when allocating asids in arm_smmu_domain_finalise_s1. The first one has minimal impact since arm_smmu_share_asid is supposedly rare, and is a simple replacement for this patch. The second one is more intrusive since we'd be limiting the number of dma/unmanaged domains to a fairly small number, but it has the advantage of allowing those domains to always successfully attach to masters belonging to SMMUs with different asid_bits values (without having to re-allocate a new ASID for the domain arm_smmu_share_asid style). Where-as this series simply fails to attach the domain in such scenarios.
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c index fe88a7880ad57..92d2f8c4e90a8 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c @@ -66,6 +66,20 @@ static int arm_smmu_write_ctx_desc_devices(struct arm_smmu_domain *smmu_domain, return ret; } +static u32 arm_smmu_domain_max_asid_bits(struct arm_smmu_domain *smmu_domain) +{ + struct arm_smmu_master *master; + unsigned long flags; + u32 asid_bits = 16; + + spin_lock_irqsave(&smmu_domain->devices_lock, flags); + list_for_each_entry(master, &smmu_domain->devices, + domain_head) + asid_bits = min(asid_bits, master->smmu->asid_bits); + spin_unlock_irqrestore(&smmu_domain->devices_lock, flags); + return asid_bits; +} + /* * Check if the CPU ASID is available on the SMMU side. If a private context * descriptor is using it, try to replace it. @@ -76,7 +90,6 @@ arm_smmu_share_asid(struct mm_struct *mm, u16 asid) int ret; u32 new_asid; struct arm_smmu_ctx_desc *cd; - struct arm_smmu_device *smmu; struct arm_smmu_domain *smmu_domain; cd = xa_load(&arm_smmu_asid_xa, asid); @@ -92,10 +105,12 @@ arm_smmu_share_asid(struct mm_struct *mm, u16 asid) } smmu_domain = container_of(cd, struct arm_smmu_domain, cd); - smmu = smmu_domain->smmu; - ret = xa_alloc(&arm_smmu_asid_xa, &new_asid, cd, - XA_LIMIT(1, (1 << smmu->asid_bits) - 1), GFP_KERNEL); + ret = xa_alloc( + &arm_smmu_asid_xa, &new_asid, cd, + XA_LIMIT(1, + (1 << arm_smmu_domain_max_asid_bits(smmu_domain)) - 1), + GFP_KERNEL); if (ret) return ERR_PTR(-ENOSPC); /*
Pick an ASID that is within the supported range of all SMMUs that the domain is installed to. Signed-off-by: Michael Shavit <mshavit@google.com> --- (no changes since v1) .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)