Re: [PATCH v2 5/9] iommu/ioasid: Introduce ioasid_set private ID
Hi Jacob,
On 9/9/20 12:40 AM, Jacob Pan wrote:
> On Tue, 1 Sep 2020 17:38:44 +0200
> Auger Eric wrote:
>
>> Hi Jacob,
>> On 8/22/20 6:35 AM, Jacob Pan wrote:
>>> When an IOASID set is used for guest SVA, each VM will acquire its
>>> ioasid_set for IOASID allocations. IOASIDs within the VM must have a
>>> host/physical IOASID backing, mapping between guest and host
>>> IOASIDs can be non-identical. IOASID set private ID (SPID) is
>>> introduced in this patch to be used as guest IOASID. However, the
>>> concept of ioasid_set specific namespace is generic, thus named
>>> SPID.
>>>
>>> As SPID namespace is within the IOASID set, the IOASID core can
>>> provide lookup services at both directions. SPIDs may not be
>>> allocated when its IOASID is allocated, the mapping between SPID
>>> and IOASID is usually established when a guest page table is bound
>>> to a host PASID.
>>>
>>> Signed-off-by: Jacob Pan
>>> ---
>>> drivers/iommu/ioasid.c | 54
>>> ++
>>> include/linux/ioasid.h | 12 +++ 2 files changed, 66
>>> insertions(+)
>>>
>>> diff --git a/drivers/iommu/ioasid.c b/drivers/iommu/ioasid.c
>>> index 5f31d63c75b1..c0aef38a4fde 100644
>>> --- a/drivers/iommu/ioasid.c
>>> +++ b/drivers/iommu/ioasid.c
>>> @@ -21,6 +21,7 @@ enum ioasid_state {
>>> * struct ioasid_data - Meta data about ioasid
>>> *
>>> * @id:Unique ID
>>> + * @spid: Private ID unique within a set
>>> * @users Number of active users
>>> * @state Track state of the IOASID
>>> * @setMeta data of the set this IOASID belongs to
>>> @@ -29,6 +30,7 @@ enum ioasid_state {
>>> */
>>> struct ioasid_data {
>>> ioasid_t id;
>>> + ioasid_t spid;
>>> struct ioasid_set *set;
>>> refcount_t users;
>>> enum ioasid_state state;
>>> @@ -326,6 +328,58 @@ int ioasid_attach_data(ioasid_t ioasid, void
>>> *data) EXPORT_SYMBOL_GPL(ioasid_attach_data);
>>>
>>> /**
>>> + * ioasid_attach_spid - Attach ioasid_set private ID to an IOASID
>>> + *
>>> + * @ioasid: the ID to attach
>>> + * @spid: the ioasid_set private ID of @ioasid
>>> + *
>>> + * For IOASID that is already allocated, private ID within the set
>>> can be
>>> + * attached via this API. Future lookup can be done via
>>> ioasid_find.
>> I would remove "For IOASID that is already allocated, private ID
>> within the set can be attached via this API"
> I guess it is implied. Will remove.
>
>>> + */
>>> +int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
>>> +{
>>> + struct ioasid_data *ioasid_data;
>>> + int ret = 0;
>>> +
>>> + spin_lock(_allocator_lock);
>> We keep on saying the SPID is local to an IOASID set but we don't
>> check any IOASID set contains this ioasid. It looks a bit weird to me.
> We store ioasid_set inside ioasid_data when an IOASID is allocated, so
> we don't need to search all the ioasid_sets. Perhaps I missed your
> point?
No I think it became clearer ;-)
>
>>> + ioasid_data = xa_load(_allocator->xa, ioasid);
>>> +
>>> + if (!ioasid_data) {
>>> + pr_err("No IOASID entry %d to attach SPID %d\n",
>>> + ioasid, spid);
>>> + ret = -ENOENT;
>>> + goto done_unlock;
>>> + }
>>> + ioasid_data->spid = spid;
>> is there any way/need to remove an spid binding?
> For guest SVA, we attach SPID as a guest PASID during bind guest page
> table. Unbind does the opposite, ioasid_attach_spid() with
> spid=INVALID_IOASID clears the binding.
>
> Perhaps add more symmetric functions? i.e.
> ioasid_detach_spid(ioasid_t ioasid)
> ioasid_attach_spid(struct ioasid_set *set, ioasid_t ioasid)
yep make sense
Thanks
Eric
>
>>> +
>>> +done_unlock:
>>> + spin_unlock(_allocator_lock);
>>> + return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(ioasid_attach_spid);
>>> +
>>> +ioasid_t ioasid_find_by_spid(struct ioasid_set *set, ioasid_t spid)
>>> +{
>>> + struct ioasid_data *entry;
>>> + unsigned long index;
>>> +
>>> + if (!xa_load(_sets, set->sid)) {
>>> + pr_warn("Invalid set\n");
>>> + return INVALID_IOASID;
>>> + }
>>> +
>>> + xa_for_each(>xa, index, entry) {
>>> + if (spid == entry->spid) {
>>> + pr_debug("Found ioasid %lu by spid %u\n",
>>> index, spid);
>>> + refcount_inc(>users);
>>> + return index;
>>> + }
>>> + }
>>> + return INVALID_IOASID;
>>> +}
>>> +EXPORT_SYMBOL_GPL(ioasid_find_by_spid);
>>> +
>>> +/**
>>> * ioasid_alloc - Allocate an IOASID
>>> * @set: the IOASID set
>>> * @min: the minimum ID (inclusive)
>>> diff --git a/include/linux/ioasid.h b/include/linux/ioasid.h
>>> index 310abe4187a3..d4b3e83672f6 100644
>>> --- a/include/linux/ioasid.h
>>> +++ b/include/linux/ioasid.h
>>> @@ -73,6 +73,8 @@ bool ioasid_is_active(ioasid_t ioasid);
>>>
>>> void *ioasid_find(struct ioasid_set *set, ioasid_t ioasid, bool
>>> (*getter)(void *)); int ioasid_attach_data(ioasid_t ioasid, void
>>> *data); +int
Re: [PATCH v2 5/9] iommu/ioasid: Introduce ioasid_set private ID
On Tue, 1 Sep 2020 17:38:44 +0200
Auger Eric wrote:
> Hi Jacob,
> On 8/22/20 6:35 AM, Jacob Pan wrote:
> > When an IOASID set is used for guest SVA, each VM will acquire its
> > ioasid_set for IOASID allocations. IOASIDs within the VM must have a
> > host/physical IOASID backing, mapping between guest and host
> > IOASIDs can be non-identical. IOASID set private ID (SPID) is
> > introduced in this patch to be used as guest IOASID. However, the
> > concept of ioasid_set specific namespace is generic, thus named
> > SPID.
> >
> > As SPID namespace is within the IOASID set, the IOASID core can
> > provide lookup services at both directions. SPIDs may not be
> > allocated when its IOASID is allocated, the mapping between SPID
> > and IOASID is usually established when a guest page table is bound
> > to a host PASID.
> >
> > Signed-off-by: Jacob Pan
> > ---
> > drivers/iommu/ioasid.c | 54
> > ++
> > include/linux/ioasid.h | 12 +++ 2 files changed, 66
> > insertions(+)
> >
> > diff --git a/drivers/iommu/ioasid.c b/drivers/iommu/ioasid.c
> > index 5f31d63c75b1..c0aef38a4fde 100644
> > --- a/drivers/iommu/ioasid.c
> > +++ b/drivers/iommu/ioasid.c
> > @@ -21,6 +21,7 @@ enum ioasid_state {
> > * struct ioasid_data - Meta data about ioasid
> > *
> > * @id:Unique ID
> > + * @spid: Private ID unique within a set
> > * @users Number of active users
> > * @state Track state of the IOASID
> > * @setMeta data of the set this IOASID belongs to
> > @@ -29,6 +30,7 @@ enum ioasid_state {
> > */
> > struct ioasid_data {
> > ioasid_t id;
> > + ioasid_t spid;
> > struct ioasid_set *set;
> > refcount_t users;
> > enum ioasid_state state;
> > @@ -326,6 +328,58 @@ int ioasid_attach_data(ioasid_t ioasid, void
> > *data) EXPORT_SYMBOL_GPL(ioasid_attach_data);
> >
> > /**
> > + * ioasid_attach_spid - Attach ioasid_set private ID to an IOASID
> > + *
> > + * @ioasid: the ID to attach
> > + * @spid: the ioasid_set private ID of @ioasid
> > + *
> > + * For IOASID that is already allocated, private ID within the set
> > can be
> > + * attached via this API. Future lookup can be done via
> > ioasid_find.
> I would remove "For IOASID that is already allocated, private ID
> within the set can be attached via this API"
I guess it is implied. Will remove.
> > + */
> > +int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
> > +{
> > + struct ioasid_data *ioasid_data;
> > + int ret = 0;
> > +
> > + spin_lock(_allocator_lock);
> We keep on saying the SPID is local to an IOASID set but we don't
> check any IOASID set contains this ioasid. It looks a bit weird to me.
We store ioasid_set inside ioasid_data when an IOASID is allocated, so
we don't need to search all the ioasid_sets. Perhaps I missed your
point?
> > + ioasid_data = xa_load(_allocator->xa, ioasid);
> > +
> > + if (!ioasid_data) {
> > + pr_err("No IOASID entry %d to attach SPID %d\n",
> > + ioasid, spid);
> > + ret = -ENOENT;
> > + goto done_unlock;
> > + }
> > + ioasid_data->spid = spid;
> is there any way/need to remove an spid binding?
For guest SVA, we attach SPID as a guest PASID during bind guest page
table. Unbind does the opposite, ioasid_attach_spid() with
spid=INVALID_IOASID clears the binding.
Perhaps add more symmetric functions? i.e.
ioasid_detach_spid(ioasid_t ioasid)
ioasid_attach_spid(struct ioasid_set *set, ioasid_t ioasid)
> > +
> > +done_unlock:
> > + spin_unlock(_allocator_lock);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(ioasid_attach_spid);
> > +
> > +ioasid_t ioasid_find_by_spid(struct ioasid_set *set, ioasid_t spid)
> > +{
> > + struct ioasid_data *entry;
> > + unsigned long index;
> > +
> > + if (!xa_load(_sets, set->sid)) {
> > + pr_warn("Invalid set\n");
> > + return INVALID_IOASID;
> > + }
> > +
> > + xa_for_each(>xa, index, entry) {
> > + if (spid == entry->spid) {
> > + pr_debug("Found ioasid %lu by spid %u\n",
> > index, spid);
> > + refcount_inc(>users);
> > + return index;
> > + }
> > + }
> > + return INVALID_IOASID;
> > +}
> > +EXPORT_SYMBOL_GPL(ioasid_find_by_spid);
> > +
> > +/**
> > * ioasid_alloc - Allocate an IOASID
> > * @set: the IOASID set
> > * @min: the minimum ID (inclusive)
> > diff --git a/include/linux/ioasid.h b/include/linux/ioasid.h
> > index 310abe4187a3..d4b3e83672f6 100644
> > --- a/include/linux/ioasid.h
> > +++ b/include/linux/ioasid.h
> > @@ -73,6 +73,8 @@ bool ioasid_is_active(ioasid_t ioasid);
> >
> > void *ioasid_find(struct ioasid_set *set, ioasid_t ioasid, bool
> > (*getter)(void *)); int ioasid_attach_data(ioasid_t ioasid, void
> > *data); +int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid);
> > +ioasid_t ioasid_find_by_spid(struct ioasid_set *set, ioasid_t
> > spid); int
Re: [PATCH v2 5/9] iommu/ioasid: Introduce ioasid_set private ID
On Tue, 25 Aug 2020 12:22:09 +0200
Jean-Philippe Brucker wrote:
> On Fri, Aug 21, 2020 at 09:35:14PM -0700, Jacob Pan wrote:
> > When an IOASID set is used for guest SVA, each VM will acquire its
> > ioasid_set for IOASID allocations. IOASIDs within the VM must have a
> > host/physical IOASID backing, mapping between guest and host IOASIDs can
> > be non-identical. IOASID set private ID (SPID) is introduced in this
> > patch to be used as guest IOASID. However, the concept of ioasid_set
> > specific namespace is generic, thus named SPID.
> >
> > As SPID namespace is within the IOASID set, the IOASID core can provide
> > lookup services at both directions. SPIDs may not be allocated when its
> > IOASID is allocated, the mapping between SPID and IOASID is usually
> > established when a guest page table is bound to a host PASID.
> >
> > Signed-off-by: Jacob Pan
> > ---
> > drivers/iommu/ioasid.c | 54
> > ++
> > include/linux/ioasid.h | 12 +++
> > 2 files changed, 66 insertions(+)
> >
> > diff --git a/drivers/iommu/ioasid.c b/drivers/iommu/ioasid.c
> > index 5f31d63c75b1..c0aef38a4fde 100644
> > --- a/drivers/iommu/ioasid.c
> > +++ b/drivers/iommu/ioasid.c
> > @@ -21,6 +21,7 @@ enum ioasid_state {
> > * struct ioasid_data - Meta data about ioasid
> > *
> > * @id:Unique ID
> > + * @spid: Private ID unique within a set
> > * @users Number of active users
> > * @state Track state of the IOASID
> > * @setMeta data of the set this IOASID belongs to
> > @@ -29,6 +30,7 @@ enum ioasid_state {
> > */
> > struct ioasid_data {
> > ioasid_t id;
> > + ioasid_t spid;
> > struct ioasid_set *set;
> > refcount_t users;
> > enum ioasid_state state;
> > @@ -326,6 +328,58 @@ int ioasid_attach_data(ioasid_t ioasid, void *data)
> > EXPORT_SYMBOL_GPL(ioasid_attach_data);
> >
> > /**
> > + * ioasid_attach_spid - Attach ioasid_set private ID to an IOASID
> > + *
> > + * @ioasid: the ID to attach
> > + * @spid: the ioasid_set private ID of @ioasid
> > + *
> > + * For IOASID that is already allocated, private ID within the set can be
> > + * attached via this API. Future lookup can be done via ioasid_find.
>
> via ioasid_find_by_spid()?
>
yes, will update.
> > + */
> > +int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
> > +{
> > + struct ioasid_data *ioasid_data;
> > + int ret = 0;
> > +
> > + spin_lock(_allocator_lock);
> > + ioasid_data = xa_load(_allocator->xa, ioasid);
> > +
> > + if (!ioasid_data) {
> > + pr_err("No IOASID entry %d to attach SPID %d\n",
> > + ioasid, spid);
> > + ret = -ENOENT;
> > + goto done_unlock;
> > + }
> > + ioasid_data->spid = spid;
> > +
> > +done_unlock:
> > + spin_unlock(_allocator_lock);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(ioasid_attach_spid);
> > +
> > +ioasid_t ioasid_find_by_spid(struct ioasid_set *set, ioasid_t spid)
>
> Maybe add a bit of documentation as this is public-facing.
>
Good point, I will add
/**
* ioasid_find_by_spid - Find the system-wide IOASID by a set private ID and
* its set.
*
* @set:the ioasid_set to search within
* @spid: the set private ID
*
* Given a set private ID and its IOASID set, find the system-wide IOASID. Take
* a reference upon finding the matching IOASID. Return INVALID_IOASID if the
* IOASID is not found in the set or the set is not valid.
*/
> > +{
> > + struct ioasid_data *entry;
> > + unsigned long index;
> > +
> > + if (!xa_load(_sets, set->sid)) {
> > + pr_warn("Invalid set\n");
> > + return INVALID_IOASID;
> > + }
> > +
> > + xa_for_each(>xa, index, entry) {
> > + if (spid == entry->spid) {
> > + pr_debug("Found ioasid %lu by spid %u\n", index, spid);
> > + refcount_inc(>users);
>
> Nothing prevents ioasid_free() from concurrently dropping the refcount to
> zero and calling ioasid_do_free(). The caller will later call ioasid_put()
> on a stale/reallocated index.
>
right, need to add spin_lock(_allocator_lock);
> > + return index;
> > + }
> > + }
> > + return INVALID_IOASID;
> > +}
> > +EXPORT_SYMBOL_GPL(ioasid_find_by_spid);
> > +
> > +/**
> > * ioasid_alloc - Allocate an IOASID
> > * @set: the IOASID set
> > * @min: the minimum ID (inclusive)
> > diff --git a/include/linux/ioasid.h b/include/linux/ioasid.h
> > index 310abe4187a3..d4b3e83672f6 100644
> > --- a/include/linux/ioasid.h
> > +++ b/include/linux/ioasid.h
> > @@ -73,6 +73,8 @@ bool ioasid_is_active(ioasid_t ioasid);
> >
> > void *ioasid_find(struct ioasid_set *set, ioasid_t ioasid, bool
> > (*getter)(void *));
> > int ioasid_attach_data(ioasid_t ioasid, void *data);
> > +int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid);
> > +ioasid_t ioasid_find_by_spid(struct ioasid_set *set, ioasid_t spid);
> > int
Re: [PATCH v2 5/9] iommu/ioasid: Introduce ioasid_set private ID
Hi Jacob,
On 8/22/20 6:35 AM, Jacob Pan wrote:
> When an IOASID set is used for guest SVA, each VM will acquire its
> ioasid_set for IOASID allocations. IOASIDs within the VM must have a
> host/physical IOASID backing, mapping between guest and host IOASIDs can
> be non-identical. IOASID set private ID (SPID) is introduced in this
> patch to be used as guest IOASID. However, the concept of ioasid_set
> specific namespace is generic, thus named SPID.
>
> As SPID namespace is within the IOASID set, the IOASID core can provide
> lookup services at both directions. SPIDs may not be allocated when its
> IOASID is allocated, the mapping between SPID and IOASID is usually
> established when a guest page table is bound to a host PASID.
>
> Signed-off-by: Jacob Pan
> ---
> drivers/iommu/ioasid.c | 54
> ++
> include/linux/ioasid.h | 12 +++
> 2 files changed, 66 insertions(+)
>
> diff --git a/drivers/iommu/ioasid.c b/drivers/iommu/ioasid.c
> index 5f31d63c75b1..c0aef38a4fde 100644
> --- a/drivers/iommu/ioasid.c
> +++ b/drivers/iommu/ioasid.c
> @@ -21,6 +21,7 @@ enum ioasid_state {
> * struct ioasid_data - Meta data about ioasid
> *
> * @id: Unique ID
> + * @spid:Private ID unique within a set
> * @usersNumber of active users
> * @stateTrack state of the IOASID
> * @set Meta data of the set this IOASID belongs to
> @@ -29,6 +30,7 @@ enum ioasid_state {
> */
> struct ioasid_data {
> ioasid_t id;
> + ioasid_t spid;
> struct ioasid_set *set;
> refcount_t users;
> enum ioasid_state state;
> @@ -326,6 +328,58 @@ int ioasid_attach_data(ioasid_t ioasid, void *data)
> EXPORT_SYMBOL_GPL(ioasid_attach_data);
>
> /**
> + * ioasid_attach_spid - Attach ioasid_set private ID to an IOASID
> + *
> + * @ioasid: the ID to attach
> + * @spid: the ioasid_set private ID of @ioasid
> + *
> + * For IOASID that is already allocated, private ID within the set can be
> + * attached via this API. Future lookup can be done via ioasid_find.
I would remove "For IOASID that is already allocated, private ID within
the set can be attached via this API"
> + */
> +int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
> +{
> + struct ioasid_data *ioasid_data;
> + int ret = 0;
> +
> + spin_lock(_allocator_lock);
We keep on saying the SPID is local to an IOASID set but we don't check
any IOASID set contains this ioasid. It looks a bit weird to me.
> + ioasid_data = xa_load(_allocator->xa, ioasid);
> +
> + if (!ioasid_data) {
> + pr_err("No IOASID entry %d to attach SPID %d\n",
> + ioasid, spid);
> + ret = -ENOENT;
> + goto done_unlock;
> + }
> + ioasid_data->spid = spid;
is there any way/need to remove an spid binding?
> +
> +done_unlock:
> + spin_unlock(_allocator_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(ioasid_attach_spid);
> +
> +ioasid_t ioasid_find_by_spid(struct ioasid_set *set, ioasid_t spid)
> +{
> + struct ioasid_data *entry;
> + unsigned long index;
> +
> + if (!xa_load(_sets, set->sid)) {
> + pr_warn("Invalid set\n");
> + return INVALID_IOASID;
> + }
> +
> + xa_for_each(>xa, index, entry) {
> + if (spid == entry->spid) {
> + pr_debug("Found ioasid %lu by spid %u\n", index, spid);
> + refcount_inc(>users);
> + return index;
> + }
> + }
> + return INVALID_IOASID;
> +}
> +EXPORT_SYMBOL_GPL(ioasid_find_by_spid);
> +
> +/**
> * ioasid_alloc - Allocate an IOASID
> * @set: the IOASID set
> * @min: the minimum ID (inclusive)
> diff --git a/include/linux/ioasid.h b/include/linux/ioasid.h
> index 310abe4187a3..d4b3e83672f6 100644
> --- a/include/linux/ioasid.h
> +++ b/include/linux/ioasid.h
> @@ -73,6 +73,8 @@ bool ioasid_is_active(ioasid_t ioasid);
>
> void *ioasid_find(struct ioasid_set *set, ioasid_t ioasid, bool
> (*getter)(void *));
> int ioasid_attach_data(ioasid_t ioasid, void *data);
> +int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid);
> +ioasid_t ioasid_find_by_spid(struct ioasid_set *set, ioasid_t spid);
> int ioasid_register_allocator(struct ioasid_allocator_ops *allocator);
> void ioasid_unregister_allocator(struct ioasid_allocator_ops *allocator);
> void ioasid_is_in_set(struct ioasid_set *set, ioasid_t ioasid);
> @@ -136,5 +138,15 @@ static inline int ioasid_attach_data(ioasid_t ioasid,
> void *data)
> return -ENOTSUPP;
> }
>
> +staic inline int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline ioasid_t ioasid_find_by_spid(struct ioasid_set *set, ioasid_t
> spid)
> +{
> + return -ENOTSUPP;
> +}
> +
> #endif /* CONFIG_IOASID */
> #endif /* __LINUX_IOASID_H */
>
Thanks
Eric
___
iommu mailing list
Re: [PATCH v2 5/9] iommu/ioasid: Introduce ioasid_set private ID
On Fri, Aug 21, 2020 at 09:35:14PM -0700, Jacob Pan wrote:
> When an IOASID set is used for guest SVA, each VM will acquire its
> ioasid_set for IOASID allocations. IOASIDs within the VM must have a
> host/physical IOASID backing, mapping between guest and host IOASIDs can
> be non-identical. IOASID set private ID (SPID) is introduced in this
> patch to be used as guest IOASID. However, the concept of ioasid_set
> specific namespace is generic, thus named SPID.
>
> As SPID namespace is within the IOASID set, the IOASID core can provide
> lookup services at both directions. SPIDs may not be allocated when its
> IOASID is allocated, the mapping between SPID and IOASID is usually
> established when a guest page table is bound to a host PASID.
>
> Signed-off-by: Jacob Pan
> ---
> drivers/iommu/ioasid.c | 54
> ++
> include/linux/ioasid.h | 12 +++
> 2 files changed, 66 insertions(+)
>
> diff --git a/drivers/iommu/ioasid.c b/drivers/iommu/ioasid.c
> index 5f31d63c75b1..c0aef38a4fde 100644
> --- a/drivers/iommu/ioasid.c
> +++ b/drivers/iommu/ioasid.c
> @@ -21,6 +21,7 @@ enum ioasid_state {
> * struct ioasid_data - Meta data about ioasid
> *
> * @id: Unique ID
> + * @spid:Private ID unique within a set
> * @usersNumber of active users
> * @stateTrack state of the IOASID
> * @set Meta data of the set this IOASID belongs to
> @@ -29,6 +30,7 @@ enum ioasid_state {
> */
> struct ioasid_data {
> ioasid_t id;
> + ioasid_t spid;
> struct ioasid_set *set;
> refcount_t users;
> enum ioasid_state state;
> @@ -326,6 +328,58 @@ int ioasid_attach_data(ioasid_t ioasid, void *data)
> EXPORT_SYMBOL_GPL(ioasid_attach_data);
>
> /**
> + * ioasid_attach_spid - Attach ioasid_set private ID to an IOASID
> + *
> + * @ioasid: the ID to attach
> + * @spid: the ioasid_set private ID of @ioasid
> + *
> + * For IOASID that is already allocated, private ID within the set can be
> + * attached via this API. Future lookup can be done via ioasid_find.
via ioasid_find_by_spid()?
> + */
> +int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
> +{
> + struct ioasid_data *ioasid_data;
> + int ret = 0;
> +
> + spin_lock(_allocator_lock);
> + ioasid_data = xa_load(_allocator->xa, ioasid);
> +
> + if (!ioasid_data) {
> + pr_err("No IOASID entry %d to attach SPID %d\n",
> + ioasid, spid);
> + ret = -ENOENT;
> + goto done_unlock;
> + }
> + ioasid_data->spid = spid;
> +
> +done_unlock:
> + spin_unlock(_allocator_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(ioasid_attach_spid);
> +
> +ioasid_t ioasid_find_by_spid(struct ioasid_set *set, ioasid_t spid)
Maybe add a bit of documentation as this is public-facing.
> +{
> + struct ioasid_data *entry;
> + unsigned long index;
> +
> + if (!xa_load(_sets, set->sid)) {
> + pr_warn("Invalid set\n");
> + return INVALID_IOASID;
> + }
> +
> + xa_for_each(>xa, index, entry) {
> + if (spid == entry->spid) {
> + pr_debug("Found ioasid %lu by spid %u\n", index, spid);
> + refcount_inc(>users);
Nothing prevents ioasid_free() from concurrently dropping the refcount to
zero and calling ioasid_do_free(). The caller will later call ioasid_put()
on a stale/reallocated index.
> + return index;
> + }
> + }
> + return INVALID_IOASID;
> +}
> +EXPORT_SYMBOL_GPL(ioasid_find_by_spid);
> +
> +/**
> * ioasid_alloc - Allocate an IOASID
> * @set: the IOASID set
> * @min: the minimum ID (inclusive)
> diff --git a/include/linux/ioasid.h b/include/linux/ioasid.h
> index 310abe4187a3..d4b3e83672f6 100644
> --- a/include/linux/ioasid.h
> +++ b/include/linux/ioasid.h
> @@ -73,6 +73,8 @@ bool ioasid_is_active(ioasid_t ioasid);
>
> void *ioasid_find(struct ioasid_set *set, ioasid_t ioasid, bool
> (*getter)(void *));
> int ioasid_attach_data(ioasid_t ioasid, void *data);
> +int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid);
> +ioasid_t ioasid_find_by_spid(struct ioasid_set *set, ioasid_t spid);
> int ioasid_register_allocator(struct ioasid_allocator_ops *allocator);
> void ioasid_unregister_allocator(struct ioasid_allocator_ops *allocator);
> void ioasid_is_in_set(struct ioasid_set *set, ioasid_t ioasid);
> @@ -136,5 +138,15 @@ static inline int ioasid_attach_data(ioasid_t ioasid,
> void *data)
> return -ENOTSUPP;
> }
>
> +staic inline int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline ioasid_t ioasid_find_by_spid(struct ioasid_set *set, ioasid_t
> spid)
> +{
> + return -ENOTSUPP;
INVALID_IOASID
Thanks,
Jean
> +}
> +
> #endif /* CONFIG_IOASID */
> #endif /* __LINUX_IOASID_H */
> --
> 2.7.4
>
___
iommu mailing list
Re: [PATCH v2 5/9] iommu/ioasid: Introduce ioasid_set private ID
Hi Jacob,
I love your patch! Yet something to improve:
[auto build test ERROR on iommu/next]
[also build test ERROR on linux/master linus/master v5.9-rc1 next-20200821]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Jacob-Pan/IOASID-extensions-for-guest-SVA/20200822-123111
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: mips-randconfig-r015-20200822 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project
b587ca93be114d07ec3bf654add97d7872325281)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
In file included from drivers/of/device.c:7:
In file included from include/linux/of_iommu.h:6:
In file included from include/linux/iommu.h:16:
>> include/linux/ioasid.h:141:1: error: unknown type name 'staic'; did you mean
>> 'static'?
staic inline int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
^
static
In file included from drivers/of/device.c:8:
include/linux/dma-mapping.h:824:9: warning: implicit conversion from
'unsigned long long' to 'unsigned long' changes value from 18446744073709551615
to 4294967295 [-Wconstant-conversion]
return DMA_BIT_MASK(32);
~~ ^~~~
include/linux/dma-mapping.h:139:40: note: expanded from macro 'DMA_BIT_MASK'
#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
^
1 warning and 1 error generated.
--
In file included from drivers/of/platform.c:20:
In file included from include/linux/of_iommu.h:6:
In file included from include/linux/iommu.h:16:
>> include/linux/ioasid.h:141:1: error: unknown type name 'staic'; did you mean
>> 'static'?
staic inline int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
^
static
1 error generated.
#
https://github.com/0day-ci/linux/commit/09f31e901946399a274ce954bdefa4108e895b33
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Jacob-Pan/IOASID-extensions-for-guest-SVA/20200822-123111
git checkout 09f31e901946399a274ce954bdefa4108e895b33
vim +141 include/linux/ioasid.h
140
> 141 staic inline int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
142 {
143 return -ENOTSUPP;
144 }
145
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 5/9] iommu/ioasid: Introduce ioasid_set private ID
Hi Jacob,
I love your patch! Yet something to improve:
[auto build test ERROR on iommu/next]
[also build test ERROR on linux/master linus/master v5.9-rc1 next-20200821]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Jacob-Pan/IOASID-extensions-for-guest-SVA/20200822-123111
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: ia64-randconfig-r003-20200822 (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=ia64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
In file included from include/linux/iommu.h:16,
from include/linux/of_iommu.h:6,
from drivers/of/device.c:7:
>> include/linux/ioasid.h:141:6: error: expected ';' before 'inline'
141 | staic inline int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
| ^
| ;
--
In file included from include/linux/iommu.h:16,
from drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:34:
>> include/linux/ioasid.h:141:6: error: expected ';' before 'inline'
141 | staic inline int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
| ^
| ;
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:1094:5: warning: no previous
prototype for 'amdgpu_ttm_gart_bind' [-Wmissing-prototypes]
1094 | int amdgpu_ttm_gart_bind(struct amdgpu_device *adev,
| ^~~~
In file included from drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:55:
drivers/gpu/drm/amd/amdgpu/amdgpu.h:190:18: warning: 'sched_policy' defined
but not used [-Wunused-const-variable=]
190 | static const int sched_policy = KFD_SCHED_POLICY_HWS;
| ^~~~
In file included from drivers/gpu/drm/amd/amdgpu/../display/dc/dc_types.h:33,
from
drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:30,
from
drivers/gpu/drm/amd/amdgpu/../include/dm_pp_interface.h:26,
from drivers/gpu/drm/amd/amdgpu/amdgpu.h:65,
from drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:55:
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:76:32: warning:
'dc_fixpt_ln2_div_2' defined but not used [-Wunused-const-variable=]
76 | static const struct fixed31_32 dc_fixpt_ln2_div_2 = { 1488522236LL };
|^~
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:75:32: warning:
'dc_fixpt_ln2' defined but not used [-Wunused-const-variable=]
75 | static const struct fixed31_32 dc_fixpt_ln2 = { 2977044471LL };
|^~~~
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:74:32: warning:
'dc_fixpt_e' defined but not used [-Wunused-const-variable=]
74 | static const struct fixed31_32 dc_fixpt_e = { 11674931555LL };
|^~
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:73:32: warning:
'dc_fixpt_two_pi' defined but not used [-Wunused-const-variable=]
73 | static const struct fixed31_32 dc_fixpt_two_pi = { 26986075409LL };
|^~~
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:72:32: warning:
'dc_fixpt_pi' defined but not used [-Wunused-const-variable=]
72 | static const struct fixed31_32 dc_fixpt_pi = { 13493037705LL };
|^~~
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:67:32: warning:
'dc_fixpt_zero' defined but not used [-Wunused-const-variable=]
67 | static const struct fixed31_32 dc_fixpt_zero = { 0 };
|^
--
In file included from include/linux/iommu.h:16,
from drivers/gpu/drm/nouveau/include/nvif/os.h:30,
from drivers/gpu/drm/nouveau/include/nvkm/core/os.h:4,
from drivers/gpu/drm/nouveau/include/nvkm/core/oclass.h:3,
from drivers/gpu/drm/nouveau/include/nvkm/core/device.h:4,
from drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h:4,
from drivers/gpu/drm/nouveau/nvkm/nvfw/acr.c:22:
>> include/linux/ioasid.h:141:6: error: expected ';' before 'inline'
141 | staic inline int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
| ^
| ;
[PATCH v2 5/9] iommu/ioasid: Introduce ioasid_set private ID
When an IOASID set is used for guest SVA, each VM will acquire its
ioasid_set for IOASID allocations. IOASIDs within the VM must have a
host/physical IOASID backing, mapping between guest and host IOASIDs can
be non-identical. IOASID set private ID (SPID) is introduced in this
patch to be used as guest IOASID. However, the concept of ioasid_set
specific namespace is generic, thus named SPID.
As SPID namespace is within the IOASID set, the IOASID core can provide
lookup services at both directions. SPIDs may not be allocated when its
IOASID is allocated, the mapping between SPID and IOASID is usually
established when a guest page table is bound to a host PASID.
Signed-off-by: Jacob Pan
---
drivers/iommu/ioasid.c | 54 ++
include/linux/ioasid.h | 12 +++
2 files changed, 66 insertions(+)
diff --git a/drivers/iommu/ioasid.c b/drivers/iommu/ioasid.c
index 5f31d63c75b1..c0aef38a4fde 100644
--- a/drivers/iommu/ioasid.c
+++ b/drivers/iommu/ioasid.c
@@ -21,6 +21,7 @@ enum ioasid_state {
* struct ioasid_data - Meta data about ioasid
*
* @id:Unique ID
+ * @spid: Private ID unique within a set
* @users Number of active users
* @state Track state of the IOASID
* @setMeta data of the set this IOASID belongs to
@@ -29,6 +30,7 @@ enum ioasid_state {
*/
struct ioasid_data {
ioasid_t id;
+ ioasid_t spid;
struct ioasid_set *set;
refcount_t users;
enum ioasid_state state;
@@ -326,6 +328,58 @@ int ioasid_attach_data(ioasid_t ioasid, void *data)
EXPORT_SYMBOL_GPL(ioasid_attach_data);
/**
+ * ioasid_attach_spid - Attach ioasid_set private ID to an IOASID
+ *
+ * @ioasid: the ID to attach
+ * @spid: the ioasid_set private ID of @ioasid
+ *
+ * For IOASID that is already allocated, private ID within the set can be
+ * attached via this API. Future lookup can be done via ioasid_find.
+ */
+int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
+{
+ struct ioasid_data *ioasid_data;
+ int ret = 0;
+
+ spin_lock(_allocator_lock);
+ ioasid_data = xa_load(_allocator->xa, ioasid);
+
+ if (!ioasid_data) {
+ pr_err("No IOASID entry %d to attach SPID %d\n",
+ ioasid, spid);
+ ret = -ENOENT;
+ goto done_unlock;
+ }
+ ioasid_data->spid = spid;
+
+done_unlock:
+ spin_unlock(_allocator_lock);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ioasid_attach_spid);
+
+ioasid_t ioasid_find_by_spid(struct ioasid_set *set, ioasid_t spid)
+{
+ struct ioasid_data *entry;
+ unsigned long index;
+
+ if (!xa_load(_sets, set->sid)) {
+ pr_warn("Invalid set\n");
+ return INVALID_IOASID;
+ }
+
+ xa_for_each(>xa, index, entry) {
+ if (spid == entry->spid) {
+ pr_debug("Found ioasid %lu by spid %u\n", index, spid);
+ refcount_inc(>users);
+ return index;
+ }
+ }
+ return INVALID_IOASID;
+}
+EXPORT_SYMBOL_GPL(ioasid_find_by_spid);
+
+/**
* ioasid_alloc - Allocate an IOASID
* @set: the IOASID set
* @min: the minimum ID (inclusive)
diff --git a/include/linux/ioasid.h b/include/linux/ioasid.h
index 310abe4187a3..d4b3e83672f6 100644
--- a/include/linux/ioasid.h
+++ b/include/linux/ioasid.h
@@ -73,6 +73,8 @@ bool ioasid_is_active(ioasid_t ioasid);
void *ioasid_find(struct ioasid_set *set, ioasid_t ioasid, bool (*getter)(void
*));
int ioasid_attach_data(ioasid_t ioasid, void *data);
+int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid);
+ioasid_t ioasid_find_by_spid(struct ioasid_set *set, ioasid_t spid);
int ioasid_register_allocator(struct ioasid_allocator_ops *allocator);
void ioasid_unregister_allocator(struct ioasid_allocator_ops *allocator);
void ioasid_is_in_set(struct ioasid_set *set, ioasid_t ioasid);
@@ -136,5 +138,15 @@ static inline int ioasid_attach_data(ioasid_t ioasid, void
*data)
return -ENOTSUPP;
}
+staic inline int ioasid_attach_spid(ioasid_t ioasid, ioasid_t spid)
+{
+ return -ENOTSUPP;
+}
+
+static inline ioasid_t ioasid_find_by_spid(struct ioasid_set *set, ioasid_t
spid)
+{
+ return -ENOTSUPP;
+}
+
#endif /* CONFIG_IOASID */
#endif /* __LINUX_IOASID_H */
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
