2013-03-21 07:48:59

by Alexey Kardashevskiy

[permalink] [raw]
Subject: [PATCH] iommu: add a function to find an iommu group by id

As IOMMU groups are exposed to the user space by their numbers,
the user space can use them in various kernel APIs so the kernel
might need an API to find a group by its ID.

As an example, QEMU VFIO on PPC64 platform needs it to associate
a logical bus number (LIOBN) with a specific IOMMU group in order
to support in-kernel handling of DMA map/unmap requests.

The patch adds the iommu_group_find(id) function which performs
such search.

Signed-off-by: Alexey Kardashevskiy <[email protected]>
---
drivers/iommu/iommu.c | 26 ++++++++++++++++++++++++++
include/linux/iommu.h | 1 +
2 files changed, 27 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b0afd3d..6340cac 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -205,6 +205,32 @@ printk("%s %u grp %d\n", __func__, __LINE__, iommu_group_id(group));
}
EXPORT_SYMBOL_GPL(iommu_group_alloc);

+struct iommu_group *iommu_group_find(int id)
+{
+ struct kobject *group_kobj;
+ struct iommu_group *grp;
+ const char *name;
+
+ if (!iommu_group_kset)
+ return NULL;
+
+ name = kasprintf(GFP_KERNEL, "%d", id);
+ if (!name)
+ return NULL;
+
+ group_kobj = kset_find_obj(iommu_group_kset, name);
+ kfree(name);
+
+ if (!group_kobj)
+ return NULL;
+
+ grp = container_of(group_kobj, struct iommu_group, kobj);
+ BUG_ON(grp->id != id);
+
+ return grp;
+}
+EXPORT_SYMBOL_GPL(iommu_group_find);
+
/**
* iommu_group_get_iommudata - retrieve iommu_data registered for a group
* @group: the group
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f3b99e1..20281d5 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -113,6 +113,7 @@ struct iommu_ops {
extern int bus_set_iommu(struct bus_type *bus, struct iommu_ops *ops);
extern bool iommu_present(struct bus_type *bus);
extern struct iommu_domain *iommu_domain_alloc(struct bus_type *bus);
+extern struct iommu_group *iommu_group_find(int id);
extern void iommu_domain_free(struct iommu_domain *domain);
extern int iommu_attach_device(struct iommu_domain *domain,
struct device *dev);
--
1.7.10.4


2013-03-21 16:35:32

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH] iommu: add a function to find an iommu group by id

On Thu, 2013-03-21 at 18:48 +1100, Alexey Kardashevskiy wrote:
> As IOMMU groups are exposed to the user space by their numbers,
> the user space can use them in various kernel APIs so the kernel
> might need an API to find a group by its ID.
>
> As an example, QEMU VFIO on PPC64 platform needs it to associate
> a logical bus number (LIOBN) with a specific IOMMU group in order
> to support in-kernel handling of DMA map/unmap requests.
>
> The patch adds the iommu_group_find(id) function which performs
> such search.
>
> Signed-off-by: Alexey Kardashevskiy <[email protected]>
> ---
> drivers/iommu/iommu.c | 26 ++++++++++++++++++++++++++
> include/linux/iommu.h | 1 +
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index b0afd3d..6340cac 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -205,6 +205,32 @@ printk("%s %u grp %d\n", __func__, __LINE__, iommu_group_id(group));
> }
> EXPORT_SYMBOL_GPL(iommu_group_alloc);
>
> +struct iommu_group *iommu_group_find(int id)
> +{
> + struct kobject *group_kobj;
> + struct iommu_group *grp;
> + const char *name;
> +
> + if (!iommu_group_kset)
> + return NULL;
> +
> + name = kasprintf(GFP_KERNEL, "%d", id);
> + if (!name)
> + return NULL;
> +
> + group_kobj = kset_find_obj(iommu_group_kset, name);
> + kfree(name);
> +
> + if (!group_kobj)
> + return NULL;
> +
> + grp = container_of(group_kobj, struct iommu_group, kobj);
> + BUG_ON(grp->id != id);
> +
> + return grp;
> +}
> +EXPORT_SYMBOL_GPL(iommu_group_find);

Don't you need to do some reference counting here? Otherwise there's no
guarantee the returned pointer is still valid by the time it's used.
The interface should probably be iommu_group_get_by_id(). Thanks,

Alex

> +
> /**
> * iommu_group_get_iommudata - retrieve iommu_data registered for a group
> * @group: the group
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index f3b99e1..20281d5 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -113,6 +113,7 @@ struct iommu_ops {
> extern int bus_set_iommu(struct bus_type *bus, struct iommu_ops *ops);
> extern bool iommu_present(struct bus_type *bus);
> extern struct iommu_domain *iommu_domain_alloc(struct bus_type *bus);
> +extern struct iommu_group *iommu_group_find(int id);
> extern void iommu_domain_free(struct iommu_domain *domain);
> extern int iommu_attach_device(struct iommu_domain *domain,
> struct device *dev);


2013-03-22 06:49:18

by Alexey Kardashevskiy

[permalink] [raw]
Subject: [PATCH] iommu: add a function to find an iommu group by id

As IOMMU groups are exposed to the user space by their numbers,
the user space can use them in various kernel APIs so the kernel
might need an API to find a group by its ID.

As an example, QEMU VFIO on PPC64 platform needs it to associate
a logical bus number (LIOBN) with a specific IOMMU group in order
to support in-kernel handling of DMA map/unmap requests.

The patch adds the iommu_group_get_by_id(id) function which performs
such search.

Signed-off-by: Alexey Kardashevskiy <[email protected]>
---
drivers/iommu/iommu.c | 28 ++++++++++++++++++++++++++++
include/linux/iommu.h | 1 +
2 files changed, 29 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 1065a1a..531cbb5 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -204,6 +204,34 @@ again:
}
EXPORT_SYMBOL_GPL(iommu_group_alloc);

+struct iommu_group *iommu_group_get_by_id(int id)
+{
+ struct kobject *group_kobj;
+ struct iommu_group *grp;
+ const char *name;
+
+ if (!iommu_group_kset)
+ return NULL;
+
+ name = kasprintf(GFP_KERNEL, "%d", id);
+ if (!name)
+ return NULL;
+
+ group_kobj = kset_find_obj(iommu_group_kset, name);
+ kfree(name);
+
+ if (!group_kobj)
+ return NULL;
+
+ grp = container_of(group_kobj, struct iommu_group, kobj);
+ BUG_ON(grp->id != id);
+
+ kobject_get(grp->devices_kobj);
+
+ return grp;
+}
+EXPORT_SYMBOL_GPL(iommu_group_get_by_id);
+
/**
* iommu_group_get_iommudata - retrieve iommu_data registered for a group
* @group: the group
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f3b99e1..00e5d7d 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -113,6 +113,7 @@ struct iommu_ops {
extern int bus_set_iommu(struct bus_type *bus, struct iommu_ops *ops);
extern bool iommu_present(struct bus_type *bus);
extern struct iommu_domain *iommu_domain_alloc(struct bus_type *bus);
+extern struct iommu_group *iommu_group_get_by_id(int id);
extern void iommu_domain_free(struct iommu_domain *domain);
extern int iommu_attach_device(struct iommu_domain *domain,
struct device *dev);
--
1.7.10.4

2013-03-22 16:07:56

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH] iommu: add a function to find an iommu group by id

On Fri, 2013-03-22 at 17:49 +1100, Alexey Kardashevskiy wrote:
> As IOMMU groups are exposed to the user space by their numbers,
> the user space can use them in various kernel APIs so the kernel
> might need an API to find a group by its ID.
>
> As an example, QEMU VFIO on PPC64 platform needs it to associate
> a logical bus number (LIOBN) with a specific IOMMU group in order
> to support in-kernel handling of DMA map/unmap requests.
>
> The patch adds the iommu_group_get_by_id(id) function which performs
> such search.
>
> Signed-off-by: Alexey Kardashevskiy <[email protected]>
> ---

v2: <describe what changed>

> drivers/iommu/iommu.c | 28 ++++++++++++++++++++++++++++
> include/linux/iommu.h | 1 +
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 1065a1a..531cbb5 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -204,6 +204,34 @@ again:
> }
> EXPORT_SYMBOL_GPL(iommu_group_alloc);
>
> +struct iommu_group *iommu_group_get_by_id(int id)
> +{
> + struct kobject *group_kobj;
> + struct iommu_group *grp;

This is named "group" everywhere else in this file.

> + const char *name;
> +
> + if (!iommu_group_kset)
> + return NULL;
> +
> + name = kasprintf(GFP_KERNEL, "%d", id);
> + if (!name)
> + return NULL;
> +
> + group_kobj = kset_find_obj(iommu_group_kset, name);

This does a kobject_get on group->kobj

> + kfree(name);
> +
> + if (!group_kobj)
> + return NULL;
> +
> + grp = container_of(group_kobj, struct iommu_group, kobj);
> + BUG_ON(grp->id != id);
> +

Here you want to exchange one reference for another, so first do the
get:

> + kobject_get(grp->devices_kobj);

Now you need to release the get from the find:

kobject_put(&group->kobj)

> +
> + return grp;
> +}
> +EXPORT_SYMBOL_GPL(iommu_group_get_by_id);
> +
> /**
> * iommu_group_get_iommudata - retrieve iommu_data registered for a group
> * @group: the group
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index f3b99e1..00e5d7d 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -113,6 +113,7 @@ struct iommu_ops {
> extern int bus_set_iommu(struct bus_type *bus, struct iommu_ops *ops);
> extern bool iommu_present(struct bus_type *bus);
> extern struct iommu_domain *iommu_domain_alloc(struct bus_type *bus);
> +extern struct iommu_group *iommu_group_get_by_id(int id);
> extern void iommu_domain_free(struct iommu_domain *domain);
> extern int iommu_attach_device(struct iommu_domain *domain,
> struct device *dev);


2013-03-24 23:24:06

by Alexey Kardashevskiy

[permalink] [raw]
Subject: [PATCH] iommu: add a function to find an iommu group by id

As IOMMU groups are exposed to the user space by their numbers,
the user space can use them in various kernel APIs so the kernel
might need an API to find a group by its ID.

As an example, QEMU VFIO on PPC64 platform needs it to associate
a logical bus number (LIOBN) with a specific IOMMU group in order
to support in-kernel handling of DMA map/unmap requests.

The patch adds the iommu_group_get_by_id(id) function which performs
such search.

v2: fixed reference counting.

Signed-off-by: Alexey Kardashevskiy <[email protected]>
---
drivers/iommu/iommu.c | 29 +++++++++++++++++++++++++++++
include/linux/iommu.h | 1 +
2 files changed, 30 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 1065a1a..0de83eb 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -204,6 +204,35 @@ again:
}
EXPORT_SYMBOL_GPL(iommu_group_alloc);

+struct iommu_group *iommu_group_get_by_id(int id)
+{
+ struct kobject *group_kobj;
+ struct iommu_group *group;
+ const char *name;
+
+ if (!iommu_group_kset)
+ return NULL;
+
+ name = kasprintf(GFP_KERNEL, "%d", id);
+ if (!name)
+ return NULL;
+
+ group_kobj = kset_find_obj(iommu_group_kset, name);
+ kfree(name);
+
+ if (!group_kobj)
+ return NULL;
+
+ group = container_of(group_kobj, struct iommu_group, kobj);
+ BUG_ON(group->id != id);
+
+ kobject_get(group->devices_kobj);
+ kobject_put(&group->kobj);
+
+ return group;
+}
+EXPORT_SYMBOL_GPL(iommu_group_get_by_id);
+
/**
* iommu_group_get_iommudata - retrieve iommu_data registered for a group
* @group: the group
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f3b99e1..00e5d7d 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -113,6 +113,7 @@ struct iommu_ops {
extern int bus_set_iommu(struct bus_type *bus, struct iommu_ops *ops);
extern bool iommu_present(struct bus_type *bus);
extern struct iommu_domain *iommu_domain_alloc(struct bus_type *bus);
+extern struct iommu_group *iommu_group_get_by_id(int id);
extern void iommu_domain_free(struct iommu_domain *domain);
extern int iommu_attach_device(struct iommu_domain *domain,
struct device *dev);
--
1.7.10.4

2013-03-25 14:28:49

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH] iommu: add a function to find an iommu group by id

On Mon, 2013-03-25 at 10:23 +1100, Alexey Kardashevskiy wrote:
> As IOMMU groups are exposed to the user space by their numbers,
> the user space can use them in various kernel APIs so the kernel
> might need an API to find a group by its ID.
>
> As an example, QEMU VFIO on PPC64 platform needs it to associate
> a logical bus number (LIOBN) with a specific IOMMU group in order
> to support in-kernel handling of DMA map/unmap requests.
>
> The patch adds the iommu_group_get_by_id(id) function which performs
> such search.

Subject: [PATCH v3]....

v2 was the last one, where's the changelog for v3?
> v2: fixed reference counting.
>
> Signed-off-by: Alexey Kardashevskiy <[email protected]>
> ---

Acked-by: Alex Williamson <[email protected]>

> drivers/iommu/iommu.c | 29 +++++++++++++++++++++++++++++
> include/linux/iommu.h | 1 +
> 2 files changed, 30 insertions(+)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 1065a1a..0de83eb 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -204,6 +204,35 @@ again:
> }
> EXPORT_SYMBOL_GPL(iommu_group_alloc);
>
> +struct iommu_group *iommu_group_get_by_id(int id)
> +{
> + struct kobject *group_kobj;
> + struct iommu_group *group;
> + const char *name;
> +
> + if (!iommu_group_kset)
> + return NULL;
> +
> + name = kasprintf(GFP_KERNEL, "%d", id);
> + if (!name)
> + return NULL;
> +
> + group_kobj = kset_find_obj(iommu_group_kset, name);
> + kfree(name);
> +
> + if (!group_kobj)
> + return NULL;
> +
> + group = container_of(group_kobj, struct iommu_group, kobj);
> + BUG_ON(group->id != id);
> +
> + kobject_get(group->devices_kobj);
> + kobject_put(&group->kobj);
> +
> + return group;
> +}
> +EXPORT_SYMBOL_GPL(iommu_group_get_by_id);
> +
> /**
> * iommu_group_get_iommudata - retrieve iommu_data registered for a group
> * @group: the group
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index f3b99e1..00e5d7d 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -113,6 +113,7 @@ struct iommu_ops {
> extern int bus_set_iommu(struct bus_type *bus, struct iommu_ops *ops);
> extern bool iommu_present(struct bus_type *bus);
> extern struct iommu_domain *iommu_domain_alloc(struct bus_type *bus);
> +extern struct iommu_group *iommu_group_get_by_id(int id);
> extern void iommu_domain_free(struct iommu_domain *domain);
> extern int iommu_attach_device(struct iommu_domain *domain,
> struct device *dev);


2013-03-25 15:07:37

by Donald Dutile

[permalink] [raw]
Subject: Re: [PATCH] iommu: add a function to find an iommu group by id

On 03/25/2013 10:28 AM, Alex Williamson wrote:
> On Mon, 2013-03-25 at 10:23 +1100, Alexey Kardashevskiy wrote:
>> As IOMMU groups are exposed to the user space by their numbers,
>> the user space can use them in various kernel APIs so the kernel
>> might need an API to find a group by its ID.
>>
>> As an example, QEMU VFIO on PPC64 platform needs it to associate
>> a logical bus number (LIOBN) with a specific IOMMU group in order
>> to support in-kernel handling of DMA map/unmap requests.
>>
>> The patch adds the iommu_group_get_by_id(id) function which performs
>> such search.
>
> Subject: [PATCH v3]....
>
> v2 was the last one, where's the changelog for v3?
>> v2: fixed reference counting.
>>
and changed function name...

>> Signed-off-by: Alexey Kardashevskiy<[email protected]>
>> ---
>
> Acked-by: Alex Williamson<[email protected]>
>
>> drivers/iommu/iommu.c | 29 +++++++++++++++++++++++++++++
>> include/linux/iommu.h | 1 +
>> 2 files changed, 30 insertions(+)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 1065a1a..0de83eb 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -204,6 +204,35 @@ again:
>> }
>> EXPORT_SYMBOL_GPL(iommu_group_alloc);
>>
>> +struct iommu_group *iommu_group_get_by_id(int id)
>> +{
>> + struct kobject *group_kobj;
>> + struct iommu_group *group;
>> + const char *name;
>> +
>> + if (!iommu_group_kset)
>> + return NULL;
>> +
>> + name = kasprintf(GFP_KERNEL, "%d", id);
>> + if (!name)
>> + return NULL;
>> +
>> + group_kobj = kset_find_obj(iommu_group_kset, name);
>> + kfree(name);
>> +
>> + if (!group_kobj)
>> + return NULL;
>> +
>> + group = container_of(group_kobj, struct iommu_group, kobj);
>> + BUG_ON(group->id != id);
>> +
>> + kobject_get(group->devices_kobj);
>> + kobject_put(&group->kobj);
>> +
>> + return group;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_group_get_by_id);
>> +
>> /**
>> * iommu_group_get_iommudata - retrieve iommu_data registered for a group
>> * @group: the group
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index f3b99e1..00e5d7d 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -113,6 +113,7 @@ struct iommu_ops {
>> extern int bus_set_iommu(struct bus_type *bus, struct iommu_ops *ops);
>> extern bool iommu_present(struct bus_type *bus);
>> extern struct iommu_domain *iommu_domain_alloc(struct bus_type *bus);
>> +extern struct iommu_group *iommu_group_get_by_id(int id);
>> extern void iommu_domain_free(struct iommu_domain *domain);
>> extern int iommu_attach_device(struct iommu_domain *domain,
>> struct device *dev);
>
>
>
> _______________________________________________
> iommu mailing list
> [email protected]
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

2013-04-24 18:01:53

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH] iommu: add a function to find an iommu group by id

On Mon, Mar 25, 2013 at 10:23:49AM +1100, Alexey Kardashevskiy wrote:
> As IOMMU groups are exposed to the user space by their numbers,
> the user space can use them in various kernel APIs so the kernel
> might need an API to find a group by its ID.
>
> As an example, QEMU VFIO on PPC64 platform needs it to associate
> a logical bus number (LIOBN) with a specific IOMMU group in order
> to support in-kernel handling of DMA map/unmap requests.
>
> The patch adds the iommu_group_get_by_id(id) function which performs
> such search.
>
> v2: fixed reference counting.
>
> Signed-off-by: Alexey Kardashevskiy <[email protected]>

Applied to core branch, thanks Alexey.