* IOASID set token @ 2020-07-02 6:29 Jacob Pan 2020-07-02 13:48 ` Jacob Pan 0 siblings, 1 reply; 6+ messages in thread From: Jacob Pan @ 2020-07-02 6:29 UTC (permalink / raw) To: Jean-Philippe Brucker, Raj, Ashok; +Cc: Tian, Kevin, Lu, Baolu, iommu, Wu, Hao 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. 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? Thanks, Jacob _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: IOASID set token 2020-07-02 6:29 IOASID set token Jacob Pan @ 2020-07-02 13:48 ` Jacob Pan 2020-07-06 10:30 ` Jean-Philippe Brucker 0 siblings, 1 reply; 6+ messages in thread From: Jacob Pan @ 2020-07-02 13:48 UTC (permalink / raw) To: Jean-Philippe Brucker, Raj, Ashok, jean-philippe Cc: Tian, Kevin, Lu, Baolu, iommu, Wu, Hao 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.pan@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. 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? > > Thanks, > > Jacob [Jacob Pan] _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: IOASID set token 2020-07-02 13:48 ` Jacob Pan @ 2020-07-06 10:30 ` Jean-Philippe Brucker 2020-07-06 20:51 ` Jacob Pan 0 siblings, 1 reply; 6+ messages in thread From: Jean-Philippe Brucker @ 2020-07-06 10:30 UTC (permalink / raw) To: Jacob Pan Cc: Tian, Kevin, Raj, Ashok, Jean-Philippe Brucker, Lu, Baolu, iommu, Wu, Hao 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.pan@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: IOASID set token 2020-07-06 10:30 ` Jean-Philippe Brucker @ 2020-07-06 20:51 ` Jacob Pan 2020-07-07 10:07 ` Jean-Philippe Brucker 0 siblings, 1 reply; 6+ messages in thread From: Jacob Pan @ 2020-07-06 20:51 UTC (permalink / raw) To: Jean-Philippe Brucker Cc: Tian, Kevin, Raj, Ashok, Jean-Philippe Brucker, Lu, Baolu, iommu, Wu, Hao Hi Jean, Thanks for the feedback, please see replies inline. On Mon, 6 Jul 2020 12:30:41 +0200 Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > 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.pan@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. > In our use case, we need token to ioasid_set lookup. e.g. VFIO creates an ioasid_set with a token. KVM instance needs to know the ioasid_set based on its mm. So ioasid_find() does not know the set until it finds it based on the token and token type. > > > 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. That would be fine with VT-d. We can use init_mm for host PASID set, process mm for VM set. > For example the functions exported > by the IOASID lib would be: > > ioasid_t ioasid_alloc_mm(set, min, max, struct mm_struct *mm) what is the set argument used for? In my view, ioasid_set and mm are 1:1 mapped. struct ioasid_set *ioasid_alloc_set(void *token, ioasid_t quota) I was thinking we still have APIs for IOASID set alloc/free since we want to embed ioasid_set info in driver data structures for ownership check. > struct mm_struct *ioasid_find_mm(set, ioasid) I don't get why we need ioasid to find the set token. If we put mm_struct* inside ioasid_set, then we can get the token from the set directly. > ... > > 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. > I will keep that in mind in my next set. I think it would be much easier to explain with code. My takeaway is that we have a high-level agreement to have explicit mm as token in IOASID APIs. I think we can work out the details with patches. Jacob > Thanks, > Jean [Jacob Pan] _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: IOASID set token 2020-07-06 20:51 ` Jacob Pan @ 2020-07-07 10:07 ` Jean-Philippe Brucker 2020-07-07 15:38 ` Jacob Pan 0 siblings, 1 reply; 6+ messages in thread From: Jean-Philippe Brucker @ 2020-07-07 10:07 UTC (permalink / raw) To: Jacob Pan Cc: Tian, Kevin, Raj, Ashok, Jean-Philippe Brucker, Lu, Baolu, iommu, Wu, Hao On Mon, Jul 06, 2020 at 01:51:37PM -0700, Jacob Pan wrote: > Hi Jean, > Thanks for the feedback, please see replies inline. > > On Mon, 6 Jul 2020 12:30:41 +0200 > Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > > > 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.pan@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. > > > In our use case, we need token to ioasid_set lookup. e.g. VFIO creates > an ioasid_set with a token. KVM instance needs to know the > ioasid_set based on its mm. So ioasid_find() does not know the set > until it finds it based on the token and token type. Right, I mixed up "token" with the private data associated with each IOASID. I had mostly forgotten about the principles you introduced in the "IOASID extensions for guest SVA" series, sorry for the confusion. > > > > 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. > That would be fine with VT-d. We can use init_mm for host PASID set, > process mm for VM set. I'm not fond of using init_mm for the host PASID set. Does it need a token at all? > > > For example the functions exported > > by the IOASID lib would be: > > > > ioasid_t ioasid_alloc_mm(set, min, max, struct mm_struct *mm) > what is the set argument used for? In my view, ioasid_set and mm are > 1:1 mapped. Please disregard this, it was replacing the void* argument of ioasid_alloc() with an mm_struct. > struct ioasid_set *ioasid_alloc_set(void *token, ioasid_t quota) > > > I was thinking we still have APIs for IOASID set alloc/free since we > want to embed ioasid_set info in driver data structures for ownership > check. > > > struct mm_struct *ioasid_find_mm(set, ioasid) > I don't get why we need ioasid to find the set token. If we put > mm_struct* inside ioasid_set, then we can get the token from the set > directly. Same here, this was replacing the void* returned by ioasid_find() with an mm_struct. > > > ... > > > > 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. > > > I will keep that in mind in my next set. I think it would be much > easier to explain with code. > > My takeaway is that we have a high-level agreement to have explicit mm > as token in IOASID APIs. I think we can work out the details with > patches. Yes I think it would be easier to discuss with code. Thanks, Jean _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: IOASID set token 2020-07-07 10:07 ` Jean-Philippe Brucker @ 2020-07-07 15:38 ` Jacob Pan 0 siblings, 0 replies; 6+ messages in thread From: Jacob Pan @ 2020-07-07 15:38 UTC (permalink / raw) To: Jean-Philippe Brucker Cc: Tian, Kevin, Raj, Ashok, Jean-Philippe Brucker, Lu, Baolu, iommu, Wu, Hao Hi Jean, All other points agreed. On Tue, 7 Jul 2020 12:07:18 +0200 Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > > > For the moment, though, we could actually specialize the IOASID > > > API to only take an mm_struct as token. > > That would be fine with VT-d. We can use init_mm for host PASID set, > > process mm for VM set. > > I'm not fond of using init_mm for the host PASID set. Does it need a > token at all? No need to use init_mm at all, probably can do without a token as well. Just need to allocate a set for the native usage. I will give it a try. Thanks, Jacob _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-07 15:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-02 6:29 IOASID set token Jacob Pan 2020-07-02 13:48 ` Jacob Pan 2020-07-06 10:30 ` Jean-Philippe Brucker 2020-07-06 20:51 ` Jacob Pan 2020-07-07 10:07 ` Jean-Philippe Brucker 2020-07-07 15:38 ` Jacob Pan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).