Re: [RFC PATCH 2/6] drivers core: Add I/O ASID allocator
On 22/11/2018 08:44, Joerg Roedel wrote: > Hi Jean-Philippe, > > On Wed, Nov 21, 2018 at 11:16:07AM +, Jean-Philippe Brucker wrote: >> On 12/11/2018 14:40, Joerg Roedel wrote: >>> What is the intended use-case for this function? >> >> I'm using it in sva_bind(), to see if there already exists an io_mm >> associated to the mm_struct given as argument > > Can we store the pasid in the mm_struct directly? We still need a > reverse mapping for the page-fault path, but storing information in > mm_struct makes at least this path faster. The bind() path doesn't really need to be fast. So on its own, I don't think it justifies changing the mm_struct until we have real-world numbers indicating that it's really too slow (as the worst case of iterating over 2**20 values will be) Thanks, Jean ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 2/6] drivers core: Add I/O ASID allocator
Hi Jean-Philippe,
On Wed, Nov 21, 2018 at 11:16:07AM +, Jean-Philippe Brucker wrote:
> On 12/11/2018 14:40, Joerg Roedel wrote:
> > What is the intended use-case for this function?
>
> I'm using it in sva_bind(), to see if there already exists an io_mm
> associated to the mm_struct given as argument
Can we store the pasid in the mm_struct directly? We still need a
reverse mapping for the page-fault path, but storing information in
mm_struct makes at least this path faster.
> diff --git a/drivers/base/ioasid.c b/drivers/base/ioasid.c
> index 5eb3abbf69ac..fa3b48c9c044 100644
> --- a/drivers/base/ioasid.c
> +++ b/drivers/base/ioasid.c
> @@ -13,6 +13,7 @@ struct ioasid_data {
> int id;
> struct ioasid_set *set;
> void *private;
> + struct rcu_head rcu;
> };
>
> static DEFINE_IDR(ioasid_idr);
> @@ -56,7 +57,8 @@ void ioasid_free(ioasid_t ioasid)
> ioasid_data = idr_remove(_idr, ioasid);
> idr_unlock(_idr);
>
> - kfree(ioasid_data);
> + if (ioasid_data)
> + kfree_rcu(ioasid_data, rcu);
> }
> EXPORT_SYMBOL_GPL(ioasid_free);
>
> @@ -95,9 +97,9 @@ int ioasid_for_each(struct ioasid_set *set,
> ioasid_iter_t func, void *data)
> .data = data,
> };
>
> - idr_lock(_idr);
> + rcu_read_lock();
> ret = idr_for_each(_idr, ioasid_iter, _data);
> - idr_unlock(_idr);
> + rcu_read_unlock();
>
> return ret;
> }
> @@ -111,11 +113,11 @@ void *ioasid_find(struct ioasid_set *set, ioasid_t
> ioasid)
> void *priv = NULL;
> struct ioasid_data *ioasid_data;
>
> - idr_lock(_idr);
> + rcu_read_lock();
> ioasid_data = idr_find(_idr, ioasid);
> if (ioasid_data && ioasid_data->set == set)
> priv = ioasid_data->private;
> - idr_unlock(_idr);
> + rcu_read_unlock();
>
> return priv;
> }
Looks good to me.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 2/6] drivers core: Add I/O ASID allocator
On Wed, Nov 21, 2018 at 07:10:20PM +, Koenig, Christian wrote: > All of that is completely unrelated to IOMMU, but when IOMMU is enabled > you need to use the same allocator because all use cases use the same ID > space. Okay, I see. Is there ever a case where we can have multiple PASIDs for one mm_struct? Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC PATCH 2/6] drivers core: Add I/O ASID allocator
> From: Koenig, Christian > Sent: Thursday, November 22, 2018 3:10 AM > > Am 21.11.18 um 12:16 schrieb Jean-Philippe Brucker: > > On 12/11/2018 14:40, Joerg Roedel wrote: > >> Hi Jean-Philippe, > >> > >> On Fri, Oct 19, 2018 at 07:11:54PM +0100, Jean-Philippe Brucker wrote: > >>> The allocator doesn't really belong in drivers/iommu because some > >>> drivers would like to allocate PASIDs for devices that aren't managed by > >>> an IOMMU, using the same ID space as IOMMU. It doesn't really > belong in > >>> drivers/pci either since platform device also support PASID. Add the > >>> allocator in drivers/base. > >> I don't really buy this argument, in the end it is the IOMMU that > >> enforces the PASID mappings for a device. Why should a device not > behind > >> an IOMMU need to allocate a pasid? Do you have examples of such > devices > >> which also don't have their own iommu built-in? > > I misunderstood that requirement. Reading through the thread again > > (https://www.mail-archive.com/iommu@lists.linux- > foundation.org/msg25640.html) > > it's more about a device using PASIDs as context IDs. Some contexts are > > not bound to processes but they still need their ID in the same PASID > > space as the contexts that are bound to process address spaces. So we > > can keep that code in drivers/iommu > > The problem with that approach is that we also need this allocator when > IOMMU is completely disabled. > > In other words PASIDs are used as contexts IDs by the hardware for > things like signaling which application has caused an interrupt/event > even when they are not used by IOMMU later on. > > Additional to that we have some MMUs which are internal to the devices > (GPUVM on AMD GPUs for example, similar exists for NVidia) which uses > PASIDs for address translation internally in the device. > > All of that is completely unrelated to IOMMU, but when IOMMU is enabled > you need to use the same allocator because all use cases use the same ID > space. > imo context ID is a resource used by device internal while PASID is sth. informational to external world (as defined in PCI bus package). ideally two things are separate in hardware but can be associated (e.g. PASID as an optional parameter per context). That is what I know for Intel GPUs. For your device, is "PASIDs are used as contexts IDs" a software implementation policy or actual hardware requirement (i.e. no dedicated PASID recording)? Thanks Kevin ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 2/6] drivers core: Add I/O ASID allocator
Am 21.11.18 um 12:16 schrieb Jean-Philippe Brucker:
> On 12/11/2018 14:40, Joerg Roedel wrote:
>> Hi Jean-Philippe,
>>
>> On Fri, Oct 19, 2018 at 07:11:54PM +0100, Jean-Philippe Brucker wrote:
>>> The allocator doesn't really belong in drivers/iommu because some
>>> drivers would like to allocate PASIDs for devices that aren't managed by
>>> an IOMMU, using the same ID space as IOMMU. It doesn't really belong in
>>> drivers/pci either since platform device also support PASID. Add the
>>> allocator in drivers/base.
>> I don't really buy this argument, in the end it is the IOMMU that
>> enforces the PASID mappings for a device. Why should a device not behind
>> an IOMMU need to allocate a pasid? Do you have examples of such devices
>> which also don't have their own iommu built-in?
> I misunderstood that requirement. Reading through the thread again
> (https://www.mail-archive.com/iommu@lists.linux-foundation.org/msg25640.html)
> it's more about a device using PASIDs as context IDs. Some contexts are
> not bound to processes but they still need their ID in the same PASID
> space as the contexts that are bound to process address spaces. So we
> can keep that code in drivers/iommu
The problem with that approach is that we also need this allocator when
IOMMU is completely disabled.
In other words PASIDs are used as contexts IDs by the hardware for
things like signaling which application has caused an interrupt/event
even when they are not used by IOMMU later on.
Additional to that we have some MMUs which are internal to the devices
(GPUVM on AMD GPUs for example, similar exists for NVidia) which uses
PASIDs for address translation internally in the device.
All of that is completely unrelated to IOMMU, but when IOMMU is enabled
you need to use the same allocator because all use cases use the same ID
space.
Regards,
Christian.
>
>>> +int ioasid_for_each(struct ioasid_set *set, ioasid_iter_t func, void *data)
>>> +{
>>> + int ret;
>>> + struct ioasid_iter_data iter_data = {
>>> + .set= set,
>>> + .func = func,
>>> + .data = data,
>>> + };
>>> +
>>> + idr_lock(_idr);
>>> + ret = idr_for_each(_idr, ioasid_iter, _data);
>>> + idr_unlock(_idr);
>>> +
>>> + return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(ioasid_for_each);
>> What is the intended use-case for this function?
> I'm using it in sva_bind(), to see if there already exists an io_mm
> associated to the mm_struct given as argument
>
>>> +void *ioasid_find(struct ioasid_set *set, ioasid_t ioasid)
>>> +{
>>> + void *priv = NULL;
>>> + struct ioasid_data *ioasid_data;
>>> +
>>> + idr_lock(_idr);
>>> + ioasid_data = idr_find(_idr, ioasid);
>>> + if (ioasid_data && ioasid_data->set == set)
>>> + priv = ioasid_data->private;
>>> + idr_unlock(_idr);
>>> +
>>> + return priv;
>>> +}
>> I think using the idr_lock here will become a performance problem, as we
>> need this function in the SVA page-fault path to retrieve the mm_struct
>> belonging to a PASID.
>>
>> The head comment in lib/idr.c states that it should be safe to use
>> rcu_readlock() on read-only iterations. If so, this should be used here
>> so that concurrent lookups are possible.
> Agreed. I have a patch to use the RCU which looks simple enough, but
> since I haven't used RCU before I wasn't comfortable squashing it right
> away.
>
> Thanks,
> Jean
>
> diff --git a/drivers/base/ioasid.c b/drivers/base/ioasid.c
> index 5eb3abbf69ac..fa3b48c9c044 100644
> --- a/drivers/base/ioasid.c
> +++ b/drivers/base/ioasid.c
> @@ -13,6 +13,7 @@ struct ioasid_data {
> int id;
> struct ioasid_set *set;
> void *private;
> + struct rcu_head rcu;
> };
>
> static DEFINE_IDR(ioasid_idr);
> @@ -56,7 +57,8 @@ void ioasid_free(ioasid_t ioasid)
> ioasid_data = idr_remove(_idr, ioasid);
> idr_unlock(_idr);
>
> - kfree(ioasid_data);
> + if (ioasid_data)
> + kfree_rcu(ioasid_data, rcu);
> }
> EXPORT_SYMBOL_GPL(ioasid_free);
>
> @@ -95,9 +97,9 @@ int ioasid_for_each(struct ioasid_set *set,
> ioasid_iter_t func, void *data)
> .data = data,
> };
>
> - idr_lock(_idr);
> + rcu_read_lock();
> ret = idr_for_each(_idr, ioasid_iter, _data);
> - idr_unlock(_idr);
> + rcu_read_unlock();
>
> return ret;
> }
> @@ -111,11 +113,11 @@ void *ioasid_find(struct ioasid_set *set, ioasid_t
> ioasid)
> void *priv = NULL;
> struct ioasid_data *ioasid_data;
>
> - idr_lock(_idr);
> + rcu_read_lock();
> ioasid_data = idr_find(_idr, ioasid);
> if (ioasid_data && ioasid_data->set == set)
> priv = ioasid_data->private;
> - idr_unlock(_idr);
> + rcu_read_unlock();
>
> return priv;
> }
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 2/6] drivers core: Add I/O ASID allocator
On 12/11/2018 14:40, Joerg Roedel wrote:
> Hi Jean-Philippe,
>
> On Fri, Oct 19, 2018 at 07:11:54PM +0100, Jean-Philippe Brucker wrote:
>> The allocator doesn't really belong in drivers/iommu because some
>> drivers would like to allocate PASIDs for devices that aren't managed by
>> an IOMMU, using the same ID space as IOMMU. It doesn't really belong in
>> drivers/pci either since platform device also support PASID. Add the
>> allocator in drivers/base.
>
> I don't really buy this argument, in the end it is the IOMMU that
> enforces the PASID mappings for a device. Why should a device not behind
> an IOMMU need to allocate a pasid? Do you have examples of such devices
> which also don't have their own iommu built-in?
I misunderstood that requirement. Reading through the thread again
(https://www.mail-archive.com/iommu@lists.linux-foundation.org/msg25640.html)
it's more about a device using PASIDs as context IDs. Some contexts are
not bound to processes but they still need their ID in the same PASID
space as the contexts that are bound to process address spaces. So we
can keep that code in drivers/iommu
>> +int ioasid_for_each(struct ioasid_set *set, ioasid_iter_t func, void *data)
>> +{
>> +int ret;
>> +struct ioasid_iter_data iter_data = {
>> +.set= set,
>> +.func = func,
>> +.data = data,
>> +};
>> +
>> +idr_lock(_idr);
>> +ret = idr_for_each(_idr, ioasid_iter, _data);
>> +idr_unlock(_idr);
>> +
>> +return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(ioasid_for_each);
>
> What is the intended use-case for this function?
I'm using it in sva_bind(), to see if there already exists an io_mm
associated to the mm_struct given as argument
>> +void *ioasid_find(struct ioasid_set *set, ioasid_t ioasid)
>> +{
>> +void *priv = NULL;
>> +struct ioasid_data *ioasid_data;
>> +
>> +idr_lock(_idr);
>> +ioasid_data = idr_find(_idr, ioasid);
>> +if (ioasid_data && ioasid_data->set == set)
>> +priv = ioasid_data->private;
>> +idr_unlock(_idr);
>> +
>> +return priv;
>> +}
>
> I think using the idr_lock here will become a performance problem, as we
> need this function in the SVA page-fault path to retrieve the mm_struct
> belonging to a PASID.
>
> The head comment in lib/idr.c states that it should be safe to use
> rcu_readlock() on read-only iterations. If so, this should be used here
> so that concurrent lookups are possible.
Agreed. I have a patch to use the RCU which looks simple enough, but
since I haven't used RCU before I wasn't comfortable squashing it right
away.
Thanks,
Jean
diff --git a/drivers/base/ioasid.c b/drivers/base/ioasid.c
index 5eb3abbf69ac..fa3b48c9c044 100644
--- a/drivers/base/ioasid.c
+++ b/drivers/base/ioasid.c
@@ -13,6 +13,7 @@ struct ioasid_data {
int id;
struct ioasid_set *set;
void *private;
+ struct rcu_head rcu;
};
static DEFINE_IDR(ioasid_idr);
@@ -56,7 +57,8 @@ void ioasid_free(ioasid_t ioasid)
ioasid_data = idr_remove(_idr, ioasid);
idr_unlock(_idr);
- kfree(ioasid_data);
+ if (ioasid_data)
+ kfree_rcu(ioasid_data, rcu);
}
EXPORT_SYMBOL_GPL(ioasid_free);
@@ -95,9 +97,9 @@ int ioasid_for_each(struct ioasid_set *set,
ioasid_iter_t func, void *data)
.data = data,
};
- idr_lock(_idr);
+ rcu_read_lock();
ret = idr_for_each(_idr, ioasid_iter, _data);
- idr_unlock(_idr);
+ rcu_read_unlock();
return ret;
}
@@ -111,11 +113,11 @@ void *ioasid_find(struct ioasid_set *set, ioasid_t
ioasid)
void *priv = NULL;
struct ioasid_data *ioasid_data;
- idr_lock(_idr);
+ rcu_read_lock();
ioasid_data = idr_find(_idr, ioasid);
if (ioasid_data && ioasid_data->set == set)
priv = ioasid_data->private;
- idr_unlock(_idr);
+ rcu_read_unlock();
return priv;
}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 2/6] drivers core: Add I/O ASID allocator
Hi Jean-Philippe,
On Fri, Oct 19, 2018 at 07:11:54PM +0100, Jean-Philippe Brucker wrote:
> The allocator doesn't really belong in drivers/iommu because some
> drivers would like to allocate PASIDs for devices that aren't managed by
> an IOMMU, using the same ID space as IOMMU. It doesn't really belong in
> drivers/pci either since platform device also support PASID. Add the
> allocator in drivers/base.
I don't really buy this argument, in the end it is the IOMMU that
enforces the PASID mappings for a device. Why should a device not behind
an IOMMU need to allocate a pasid? Do you have examples of such devices
which also don't have their own iommu built-in?
> +int ioasid_for_each(struct ioasid_set *set, ioasid_iter_t func, void *data)
> +{
> + int ret;
> + struct ioasid_iter_data iter_data = {
> + .set= set,
> + .func = func,
> + .data = data,
> + };
> +
> + idr_lock(_idr);
> + ret = idr_for_each(_idr, ioasid_iter, _data);
> + idr_unlock(_idr);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(ioasid_for_each);
What is the intended use-case for this function?
> +void *ioasid_find(struct ioasid_set *set, ioasid_t ioasid)
> +{
> + void *priv = NULL;
> + struct ioasid_data *ioasid_data;
> +
> + idr_lock(_idr);
> + ioasid_data = idr_find(_idr, ioasid);
> + if (ioasid_data && ioasid_data->set == set)
> + priv = ioasid_data->private;
> + idr_unlock(_idr);
> +
> + return priv;
> +}
I think using the idr_lock here will become a performance problem, as we
need this function in the SVA page-fault path to retrieve the mm_struct
belonging to a PASID.
The head comment in lib/idr.c states that it should be safe to use
rcu_readlock() on read-only iterations. If so, this should be used here
so that concurrent lookups are possible.
Regards,
Joerg
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 2/6] drivers core: Add I/O ASID allocator
On 07/11/2018 04:53, Lu Baolu wrote:
> Hi,
>
> On 10/20/18 2:11 AM, Jean-Philippe Brucker wrote:
>> +static inline void *ioasid_find(struct ioasid_set *set, ioasid_t ioasid)
>> +{
>> +return -ESRCH;
>
> return NULL;
I'll fix it, thanks
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 2/6] drivers core: Add I/O ASID allocator
Hi,
On 10/20/18 2:11 AM, Jean-Philippe Brucker wrote:
+static inline void *ioasid_find(struct ioasid_set *set, ioasid_t ioasid)
+{
+ return -ESRCH;
return NULL;
Best regards,
Lu Baolu
+}
+#endif /* CONFIG_IOASID */
+#endif /* __LINUX_IOASID_H */
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC PATCH 2/6] drivers core: Add I/O ASID allocator
> From: Lu Baolu [mailto:baolu...@linux.intel.com] > Sent: Tuesday, October 23, 2018 2:57 PM > > Hi, > > On 10/22/18 6:22 PM, Raj, Ashok wrote: > > On Mon, Oct 22, 2018 at 12:49:47PM +0800, Lu Baolu wrote: > >> Hi, > >> > >> On 10/20/18 2:11 AM, Jean-Philippe Brucker wrote: > >>> Some devices might support multiple DMA address spaces, in particular > >>> those that have the PCI PASID feature. PASID (Process Address Space ID) > >>> allows to share process address spaces with devices (SVA), partition a > >>> device into VM-assignable entities (VFIO mdev) or simply provide > >>> multiple DMA address space to kernel drivers. Add a global PASID > >>> allocator usable by different drivers at the same time. Name it I/O ASID > >>> to avoid confusion with ASIDs allocated by arch code, which are usually > >>> a separate ID space. > >>> > >>> The IOASID space is global. Each device can have its own PASID space, > >>> but by convention the IOMMU ended up having a global PASID space, > so > >>> that with SVA, each mm_struct is associated to a single PASID. > >>> > >>> The allocator doesn't really belong in drivers/iommu because some > >>> drivers would like to allocate PASIDs for devices that aren't managed by > >>> an IOMMU, using the same ID space as IOMMU. It doesn't really > belong in > >>> drivers/pci either since platform device also support PASID. Add the > >>> allocator in drivers/base. > >> > >> One concern of moving pasid allocator here is about paravirtual > >> allocation of pasid. > >> > >> Since there is only a single set of pasid tables which is controlled by > > > > Minor correction: Single system wide PASID namespace, but PASID tables > > would be created ideally per-bdf for isolation purposes. > > > > I'm sure you meant name space, but didn't want that to be mis- > interpreted. > > Yes, confirmed. > Above essentially means that multiple IOASID allocators may exist. There needs a way for IOMMU driver to choose which one to use, e.g. for current two usages the core allocator will be used on host and also in VM (non-VTd cases), and pv allocator will be used in VM w/ virtual VTd. Thanks Kevin ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 2/6] drivers core: Add I/O ASID allocator
Hi, On 10/22/18 6:22 PM, Raj, Ashok wrote: On Mon, Oct 22, 2018 at 12:49:47PM +0800, Lu Baolu wrote: Hi, On 10/20/18 2:11 AM, Jean-Philippe Brucker wrote: Some devices might support multiple DMA address spaces, in particular those that have the PCI PASID feature. PASID (Process Address Space ID) allows to share process address spaces with devices (SVA), partition a device into VM-assignable entities (VFIO mdev) or simply provide multiple DMA address space to kernel drivers. Add a global PASID allocator usable by different drivers at the same time. Name it I/O ASID to avoid confusion with ASIDs allocated by arch code, which are usually a separate ID space. The IOASID space is global. Each device can have its own PASID space, but by convention the IOMMU ended up having a global PASID space, so that with SVA, each mm_struct is associated to a single PASID. The allocator doesn't really belong in drivers/iommu because some drivers would like to allocate PASIDs for devices that aren't managed by an IOMMU, using the same ID space as IOMMU. It doesn't really belong in drivers/pci either since platform device also support PASID. Add the allocator in drivers/base. One concern of moving pasid allocator here is about paravirtual allocation of pasid. Since there is only a single set of pasid tables which is controlled by Minor correction: Single system wide PASID namespace, but PASID tables would be created ideally per-bdf for isolation purposes. I'm sure you meant name space, but didn't want that to be mis-interpreted. Yes, confirmed. Best regards, Lu Baolu the host, the pasid is a system wide resource. When a driver running in a guest VM wants to consume a pasid, it must be intercepted by the simulation software and routed the allocation to the host via VFIO. Some iommu arch's provide mechanisms to aid this, for example, the virtual command interfaces defined in vt-d 3.0. Any pasid used in guest VM should go through the virtual command interfaces. Cheers, Ashok ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 2/6] drivers core: Add I/O ASID allocator
On Mon, Oct 22, 2018 at 12:49:47PM +0800, Lu Baolu wrote: > Hi, > > On 10/20/18 2:11 AM, Jean-Philippe Brucker wrote: > > Some devices might support multiple DMA address spaces, in particular > > those that have the PCI PASID feature. PASID (Process Address Space ID) > > allows to share process address spaces with devices (SVA), partition a > > device into VM-assignable entities (VFIO mdev) or simply provide > > multiple DMA address space to kernel drivers. Add a global PASID > > allocator usable by different drivers at the same time. Name it I/O ASID > > to avoid confusion with ASIDs allocated by arch code, which are usually > > a separate ID space. > > > > The IOASID space is global. Each device can have its own PASID space, > > but by convention the IOMMU ended up having a global PASID space, so > > that with SVA, each mm_struct is associated to a single PASID. > > > > The allocator doesn't really belong in drivers/iommu because some > > drivers would like to allocate PASIDs for devices that aren't managed by > > an IOMMU, using the same ID space as IOMMU. It doesn't really belong in > > drivers/pci either since platform device also support PASID. Add the > > allocator in drivers/base. > > One concern of moving pasid allocator here is about paravirtual > allocation of pasid. > > Since there is only a single set of pasid tables which is controlled by Minor correction: Single system wide PASID namespace, but PASID tables would be created ideally per-bdf for isolation purposes. I'm sure you meant name space, but didn't want that to be mis-interpreted. > the host, the pasid is a system wide resource. When a driver running in > a guest VM wants to consume a pasid, it must be intercepted by the > simulation software and routed the allocation to the host via VFIO. Some > iommu arch's provide mechanisms to aid this, for example, the virtual > command interfaces defined in vt-d 3.0. Any pasid used in guest VM > should go through the virtual command interfaces. > Cheers, Ashok ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 2/6] drivers core: Add I/O ASID allocator
Hi,
On 10/20/18 2:11 AM, Jean-Philippe Brucker wrote:
Some devices might support multiple DMA address spaces, in particular
those that have the PCI PASID feature. PASID (Process Address Space ID)
allows to share process address spaces with devices (SVA), partition a
device into VM-assignable entities (VFIO mdev) or simply provide
multiple DMA address space to kernel drivers. Add a global PASID
allocator usable by different drivers at the same time. Name it I/O ASID
to avoid confusion with ASIDs allocated by arch code, which are usually
a separate ID space.
The IOASID space is global. Each device can have its own PASID space,
but by convention the IOMMU ended up having a global PASID space, so
that with SVA, each mm_struct is associated to a single PASID.
The allocator doesn't really belong in drivers/iommu because some
drivers would like to allocate PASIDs for devices that aren't managed by
an IOMMU, using the same ID space as IOMMU. It doesn't really belong in
drivers/pci either since platform device also support PASID. Add the
allocator in drivers/base.
One concern of moving pasid allocator here is about paravirtual
allocation of pasid.
Since there is only a single set of pasid tables which is controlled by
the host, the pasid is a system wide resource. When a driver running in
a guest VM wants to consume a pasid, it must be intercepted by the
simulation software and routed the allocation to the host via VFIO. Some
iommu arch's provide mechanisms to aid this, for example, the virtual
command interfaces defined in vt-d 3.0. Any pasid used in guest VM
should go through the virtual command interfaces.
Best regards,
Lu Baolu
Signed-off-by: Jean-Philippe Brucker
---
drivers/base/Kconfig | 7 +++
drivers/base/Makefile | 1 +
drivers/base/ioasid.c | 140 +
include/linux/ioasid.h | 45 +
4 files changed, 193 insertions(+)
create mode 100644 drivers/base/ioasid.c
create mode 100644 include/linux/ioasid.h
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 3e63a900b330..9e41653467d7 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -182,6 +182,13 @@ config DMA_SHARED_BUFFER
APIs extension; the file's descriptor can then be passed on to other
driver.
+config IOASID
+ bool
+ default n
+ help
+ Enable the I/O Address Space ID allocator. A single ID space shared
+ between different users.
+
config DMA_FENCE_TRACE
bool "Enable verbose DMA_FENCE_TRACE messages"
depends on DMA_SHARED_BUFFER
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 704f44295810..79e83a1b344b 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_PINCTRL) += pinctrl.o
obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o
obj-$(CONFIG_GENERIC_MSI_IRQ_DOMAIN) += platform-msi.o
obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
+obj-$(CONFIG_IOASID) += ioasid.o
obj-y += test/
diff --git a/drivers/base/ioasid.c b/drivers/base/ioasid.c
new file mode 100644
index ..71ab7ee73ecb
--- /dev/null
+++ b/drivers/base/ioasid.c
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * I/O Address Space ID allocator. There is one global IOASID space, split into
+ * subsets. Users create a subset with DECLARE_IOASID_SET, then allocate and
+ * free IOASIDs with ioasid_alloc and ioasid_free.
+ */
+#include
+#include
+#include
+#include
+
+struct ioasid_data {
+ ioasid_t id;
+ struct ioasid_set *set;
+ void *private;
+};
+
+static DEFINE_IDR(ioasid_idr);
+
+/**
+ * ioasid_alloc - Allocate an IOASID
+ * @set: the IOASID set
+ * @min: the minimum ID (inclusive)
+ * @max: the maximum ID (exclusive)
+ * @private: data private to the caller
+ *
+ * Allocate an ID between @min and @max (or %0 and %INT_MAX). Return the
+ * allocated ID on success, or INVALID_IOASID on failure. The @private pointer
+ * is stored internally and can be retrieved with ioasid_find().
+ */
+ioasid_t ioasid_alloc(struct ioasid_set *set, ioasid_t min, ioasid_t max,
+ void *private)
+{
+ int id;
+ struct ioasid_data *data;
+
+ data = kzalloc(sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return INVALID_IOASID;
+
+ data->set = set;
+ data->private = private;
+
+ idr_preload(GFP_KERNEL);
+ idr_lock(_idr);
+ data->id = id = idr_alloc(_idr, data, min, max, GFP_ATOMIC);
+ idr_unlock(_idr);
+ idr_preload_end();
+
+ if (id < 0) {
+ kfree(data);
+ return INVALID_IOASID;
+ }
+
+ return data->id;
+}
+EXPORT_SYMBOL_GPL(ioasid_alloc);
+
+/**
+ * ioasid_free - Free an IOASID
+ * @ioasid: the ID to remove
+ */
+void ioasid_free(ioasid_t ioasid)
+{
+ struct ioasid_data *ioasid_data;
+
+ idr_lock(_idr);
+ ioasid_data = idr_remove(_idr, ioasid);
+ idr_unlock(_idr);
+
