Messages in this thread | | | From | Michael Shavit <> | Date | Mon, 21 Aug 2023 22:39:14 +0800 | Subject | Re: [RFC PATCH v1 3/8] iommu/arm-smmu-v3-sva: Allocate new ASID from installed_smmus |
| |
On Mon, Aug 21, 2023 at 10:26 PM Jason Gunthorpe <jgg@nvidia.com> wrote: > > On Mon, Aug 21, 2023 at 10:16:54PM +0800, Michael Shavit wrote: > > On Mon, Aug 21, 2023 at 9:50 PM Jason Gunthorpe <jgg@nvidia.com> wrote: > > > > > > On Mon, Aug 21, 2023 at 09:38:40PM +0800, Michael Shavit wrote: > > > > On Mon, Aug 21, 2023 at 7:54 PM Jason Gunthorpe <jgg@nvidia.com> wrote: > > > > > > > > > > On Mon, Aug 21, 2023 at 05:31:23PM +0800, Michael Shavit wrote: > > > > > > On Fri, Aug 18, 2023 at 2:38 AM Jason Gunthorpe <jgg@nvidia.com> wrote: > > > > > > > > > > > > > > On Fri, Aug 18, 2023 at 02:16:25AM +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> > > > > > > > > --- > > > > > > > > > > > > > > This seems like a pretty niche scenario, maybe we should just keep a > > > > > > > global for the max ASID? > > > > > > > > > > > > > > Otherwise we need a code to change the ASID, even for non-SVA domains, > > > > > > > when the domain is installed in different devices if the current ASID > > > > > > > is over the instance max.. > > > > > > > > > > > > This RFC took the other easy way out for this problem by rejecting > > > > > > attaching a domain if its currently assigned ASID/VMID > > > > > > is out of range when attaching to a new SMMU. But I'm not sure > > > > > > which of the two options is the right trade-off. > > > > > > Especially if we move VMID to a global allocator (which I plan to add > > > > > > for v2), setting a global maximum for VMID of 256 sounds small. > > > > > > > > > > IMHO the simplest and best thing is to make both vmid and asid as > > > > > local allocators. Then alot of these problems disappear > > > > > > > > Well that does sound like the most flexible, but IMO quite a lot more > > > > complicated. > > > > > > > > I'll post a v2 RFC that removes the `iommu/arm-smmu-v3: Add list of > > > > installed_smmus` patch and uses a flat master list in smmu_domain as > > > > suggested by Robin, for comparison with the v1. But at a glance using a > > > > local allocator would require: > > > > > > > 1. Keeping that patch so we can track the asid/vmid for a domain on a > > > > per smmu instance > > > > > > You'd have to store the cache tag in the per-master struct on that > > > list and take it out of the domain struct. > > > > > > Ie the list of attached masters contains the per-master cache tag > > > instead of a global cache tag. > > > > > > The only place you need the cache tag is when iterating over the list > > > of masters, so it is OK. > > > > > > If the list of masters is sorted by smmu then the first master of each > > > smmu can be used to perform the cache tag invalidation, then the rest > > > of the list is the ATC invalidation. > > > > > > The looping code will be a bit ugly. > > > > I suppose that could work.... but I'm worried it's gonna be messy, > > especially if we think about how the PASID feature would interact. > > With PASID, there could be multiple domains attached to a master. So > > we won't be able to store a single cache tag/asid for the currently > > attached domain on the arm_smmu_master. > > I wasn't suggesting to store it in the arm_smmu_master, I was > suggesting to store it in the same place you store the per-master > PASID. > > eg I expect that on attach the domain will allocate new memory to > store the pasid/cache tag/master/domain and thread that memory on a > list of attached masters.
Gotcha.
> > > > (on a loop over every smmu the domain in arm_smmu_mmu_notifier_get is > > > > attached to, which just at a glance looks headache inducing because of > > > > sva's piggybacking on the rid domain.) > > > > > > Not every smmu, just the one you are *currently* attaching to. We > > > don't care if the *other* smmu's have different ASIDs, maybe they are > > > not using BTM, or won't use SVA. > > > > I mean because the domain in arm_smmu_mmu_notifier_get is the RID > > domain (not the SVA domain, same issue we discussed in previous > > thread) , which can be attached to multiple SMMUs. > > Oh that is totally nonsensical. I expect you will need to fix that > sooner than later. Once the CD table is moved and there is a proper > way to track the PASID it should not be needed. It shouldn't fall into > the decision making about where to put the ASID xarray.
Right I got a bit of a chicken and egg problem with all these series.
Can we keep the simpler solutions where ASID/VMID across SMMUs has non-optimal constraints and re-consider this after all the other changes land (this series, set_dev_pasid series, fixing sva)?
| |