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(&ioasid_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(&active_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(&ioasid_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(&ioasid_sets, set->sid)) { >>> + pr_warn("Invalid set\n"); >>> + return INVALID_IOASID; >>> + } >>> + >>> + xa_for_each(&set->xa, index, entry) { >>> + if (spid == entry->spid) { >>> + pr_debug("Found ioasid %lu by spid %u\n", >>> index, spid); >>> + refcount_inc(&entry->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 ioas
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(&ioasid_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(&active_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(&ioasid_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(&ioasid_sets, set->sid)) { > > + pr_warn("Invalid set\n"); > > + return INVALID_IOASID; > > + } > > + > > + xa_for_each(&set->xa, index, entry) { > > + if (spid == entry->spid) { > > + pr_debug("Found ioasid %lu by spid %u\n", > > index, spid); > > + refcount_inc(&entry->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 > >
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(&ioasid_allocator_lock); > > + ioasid_data = xa_load(&active_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(&ioasid_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(&ioasid_sets, set->sid)) { > > + pr_warn("Invalid set\n"); > > + return INVALID_IOASID; > > + } > > + > > + xa_for_each(&set->xa, index, entry) { > > + if (spid == entry->spid) { > > + pr_debug("Found ioasid %lu by spid %u\n", index, spid); > > + refcount_inc(&entry->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(&ioasid_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,
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(&ioasid_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(&active_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(&ioasid_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(&ioasid_sets, set->sid)) { > + pr_warn("Invalid set\n"); > + return INVALID_IOASID; > + } > + > + xa_for_each(&set->xa, index, entry) { > + if (spid == entry->spid) { > + pr_debug("Found ioasid %lu by spid %u\n", index, spid); > + refcount_inc(&entry->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
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(&ioasid_allocator_lock); > + ioasid_data = xa_load(&active_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(&ioasid_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(&ioasid_sets, set->sid)) { > + pr_warn("Invalid set\n"); > + return INVALID_IOASID; > + } > + > + xa_for_each(&set->xa, index, entry) { > + if (spid == entry->spid) { > + pr_debug("Found ioasid %lu by spid %u\n", index, spid); > + refcount_inc(&entry->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 > ___
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) | ^ | ; drivers/gp