Messages in this thread | | | Date | Fri, 14 Jul 2023 17:35:30 -0700 | From | Nicolin Chen <> | Subject | Re: [PATCH v4 07/13] iommu/arm-smmu-v3: Keep track of attached ssids |
| |
On Fri, Jul 14, 2023 at 05:30:42PM +0800, Michael Shavit wrote:
> > And I don't quite get this part. Prior to this change, it issues > > one ATC_INV command covering all ATC entries per comments inside > > arm_smmu_atc_inv_to_cmd(). But now we replace that single command > > with all attached subdomains in the list? Any reason for such a > > change here? > > Because we don't necessarily want to invalidate all PASID-domains > attached to a master. If arm_smmu_atc_inv_domain() is called on a > domain that is only attached with Pasid, we can restrict the > invalidations to those specific PASID by looping over them. > > But yeah, > you're right that we could potentially optimize this? > * Skip the per-pasid invalidations if the domain is also attached to > this master without PASID as we have to invalidate all its pasids in > that case anyways. It's hard to imagine clients attaching a domain > both with pasid and without pasid to the same device but could be > possible. > * Always invalidate all pasids by issuing atc invalidations on SSID 0. > This sounds like the wrong trade-off??
Well, firstly it's kinda odd to have this optimization hidden in a big rework patch. And I am not sure if it alone would work for all use cases, as it impacts the arm_smmu_atc_inv_domain() that passes in a zero ssid, in which case the affected function is not used by a pasid case all the time: /* * ATS and PASID: ... * When using STRTAB_STE_1_S1DSS_SSID0 (reserving CD 0 for non-PASID * traffic), translation requests without PASID create ATC entries * without PASID, which must be invalidated with substream_valid clear. * This has the unpleasant side-effect of invalidating all PASID-tagged * ATC entries within the address range. */
Thanks Nicolin
| |