Hi Jacob, On Thu, Jul 02, 2020 at 06:48:25AM -0700, Jacob Pan wrote: > Hi Jean, > > Just realized I should send this to your Linaro account instead of ARM. > So Hi again :) > > On Wed, 1 Jul 2020 23:29:16 -0700 > Jacob Pan <jacob.jun....@linux.intel.com> wrote: > > > Hi Jean, > > > > Have a question for you on whether we can have a fixed token type for > > ioasid_set. > > > > Currently, ioasid_set has an arbitrary token. For VT-d vSVA usage, we > > choose mm as ioasid_set token to identify PASIDs within a guest. We > > have multiple in-kernel users of PASIDs such as VFIO, KVM, and VDCM. > > When an IOASID set is created, there is not a good way to communicate > > about the token choices. So we have to let VDCM and KVM *assume* mm > > is used as token, then retrieve ioasid_set based on the token. > > > > This assumption of "mm as token" is not a reliable SW architecture.
I don't see this as a problem. The token type is tied to the IOASID set, so users that pass those IOASID sets to ioasid_find() can safely assume that the returned pointer is an mm_struct. That said I'm not opposed to consolidating the API with explicit types, it could definitely be more elegant. > > So > > we are thinking if we can have an explicit ioasid_set token type where > > mm is used. After all, PASID and mm are closely related. > > > > The code change might be the following: > > 1. add a flag to indicate token type when ioasid_set is allocated, > > e.g. IOASID_SET_TYPE_MM > > IOASID_SET_TYPE_ANY > > 2. other users of the ioasid_set can query if an mm token exists based > > on the flag IOASID_SET_TYPE_MM, then retrieve the ioasid_set. > > > > Existing ioasid_set user can still use arbitrary token under the flag > > IOASID_SET_TYPE_ANY > > > > Would this be an issue for ARM usage? In my current implementation of auxiliary domains for Arm SMMU (which might never be useful enough to go upstream) I don't even use a token for the private IOASID set. However I still think we should leave the option to use a type different than mm_struct as token for some IOASID sets because device drivers (e.g. AMD kfd) may also want to dip into the IOASID space and use their own token type. For the moment, though, we could actually specialize the IOASID API to only take an mm_struct as token. For example the functions exported by the IOASID lib would be: ioasid_t ioasid_alloc_mm(set, min, max, struct mm_struct *mm) struct mm_struct *ioasid_find_mm(set, ioasid) ... And ioasid_alloc(), ioasid_find(), etc would be internal to ioasid.c and deal with IOASID_SET_TYPE_MM (or even be removed entirely for now). New users that need different token types could then introduce their own IOASID_SET_TYPE_* and use the lower-level functions. Thanks, Jean _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
