Re: [PATCH] iommu: making IOMMU sysfs nodes API public
From: Alexey Kardashevskiy
Date: Mon Feb 18 2013 - 01:15:54 EST
On 13/02/13 04:15, Alex Williamson wrote:
On Wed, 2013-02-13 at 01:42 +1100, Alexey Kardashevskiy wrote:
On 12/02/13 16:07, Alex Williamson wrote:
On Tue, 2013-02-12 at 15:06 +1100, Alexey Kardashevskiy wrote:
Having this patch in a tree, adding new nodes in sysfs
for IOMMU groups is going to be easier.
The first candidate for this change is a "dma-window-size"
property which tells a size of a DMA window of the specific
IOMMU group which can be used later for locked pages accounting.
I'm still churning on this one; I'm nervous this would basically creat
a /proc free-for-all under /sys/kernel/iommu_group/$GROUP/ where any
iommu driver can add random attributes. That can get ugly for
userspace.
Is not it exactly what sysfs is for (unlike /proc)? :)
Um, I hope it's a little more thought out than /proc.
On the other hand, for the application of userspace knowing how much
memory to lock for vfio use of a group, it's an appealing location to
get that information. Something like libvirt would already be poking
around here to figure out which devices to bind. Page limits need to be
setup prior to use through vfio, so sysfs is more convenient than
through vfio ioctls.
True. DMA window properties do not change since boot so sysfs is the right
place to expose them.
But then is dma-window-size just a vfio requirement leaking over into
iommu groups? Can we allow iommu driver based attributes without giving
up control of the namespace? Thanks,
Who are you asking these questions? :)
Anyone, including you. Rather than dropping misc files in sysfs to
describe things about the group, I think the better solution in your
case might be a link from the group to an existing sysfs directory
describing the PE. I believe your PE is rooted in a PCI bridge, so that
presumably already has a representation in sysfs. Can the aperture size
be determined from something in sysfs for that bridge already? I'm just
not ready to create a grab bag of sysfs entries for a group yet.
Thanks,
At the moment there is no information neither in sysfs nor
/proc/device-tree about the dma-window. And adding a sysfs entry per PE
(powerpc partitionable end-point which is often a PHB but not always) just
for VFIO is quite heavy.
We could add a ppc64 subfolder under /sys/kernel/iommu/xxx/ and put the
"dma-window" property there. And replace it with a symlink when and if we
add something for PE later. Would work?
Alex
Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxxxx>
---
drivers/iommu/iommu.c | 19 ++-----------------
include/linux/iommu.h | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b0afd3d..58cc298 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -52,22 +52,6 @@ struct iommu_device {
char *name;
};
-struct iommu_group_attribute {
- struct attribute attr;
- ssize_t (*show)(struct iommu_group *group, char *buf);
- ssize_t (*store)(struct iommu_group *group,
- const char *buf, size_t count);
-};
-
-#define IOMMU_GROUP_ATTR(_name, _mode, _show, _store) \
-struct iommu_group_attribute iommu_group_attr_##_name = \
- __ATTR(_name, _mode, _show, _store)
-
-#define to_iommu_group_attr(_attr) \
- container_of(_attr, struct iommu_group_attribute, attr)
-#define to_iommu_group(_kobj) \
- container_of(_kobj, struct iommu_group, kobj)
-
static ssize_t iommu_group_attr_show(struct kobject *kobj,
struct attribute *__attr, char *buf)
{
@@ -98,11 +82,12 @@ static const struct sysfs_ops iommu_group_sysfs_ops = {
.store = iommu_group_attr_store,
};
-static int iommu_group_create_file(struct iommu_group *group,
+int iommu_group_create_file(struct iommu_group *group,
struct iommu_group_attribute *attr)
{
return sysfs_create_file(&group->kobj, &attr->attr);
}
+EXPORT_SYMBOL_GPL(iommu_group_create_file);
static void iommu_group_remove_file(struct iommu_group *group,
struct iommu_group_attribute *attr)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f3b99e1..6d24ba7 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -21,6 +21,7 @@
#include <linux/errno.h>
#include <linux/types.h>
+#include <linux/stat.h>
#define IOMMU_READ (1)
#define IOMMU_WRITE (2)
@@ -197,6 +198,25 @@ static inline int report_iommu_fault(struct iommu_domain *domain,
return ret;
}
+struct iommu_group_attribute {
+ struct attribute attr;
+ ssize_t (*show)(struct iommu_group *group, char *buf);
+ ssize_t (*store)(struct iommu_group *group,
+ const char *buf, size_t count);
+};
+
+#define IOMMU_GROUP_ATTR(_name, _mode, _show, _store) \
+struct iommu_group_attribute iommu_group_attr_##_name = \
+ __ATTR(_name, _mode, _show, _store)
+
+#define to_iommu_group_attr(_attr) \
+ container_of(_attr, struct iommu_group_attribute, attr)
+#define to_iommu_group(_kobj) \
+ container_of(_kobj, struct iommu_group, kobj)
+
+extern int iommu_group_create_file(struct iommu_group *group,
+ struct iommu_group_attribute *attr);
+
#else /* CONFIG_IOMMU_API */
struct iommu_ops {};
--
Alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/