Hi Alex,
On 2013/01/18 6:21, Alex Williamson wrote:
On Thu, 2013-01-17 at 12:20 +0900, Damian Hobson-Garcia wrote:
I mistakenly sent this with the tag [PATCH] when it should have been
[RFC]. Updated with this reply.
Hi Alex,
On 2013/01/17 6:47, Alex Williamson wrote:
On Tue, 2013-01-15 at 19:10 +0900, Damian Hobson-Garcia wrote:
We've recently been thinking about the best way to attach platform devices in
an IOMMU in a generic way without having to have every platform device
register itself at probe time. We've seen that at least on the tegra IOMMU,
using the platform_bus notifier seems like a viable option
(http://lists.linuxfoundation.org/pipermail/iommu/2012-December/004954.html).
We also noticed that
the IOMMU ops add_device() function is called (indirectly) from a similar bus
notifier and we're thinking that using the bus notifier in the common code
would be better than us rolling our own to do basically the same thing.
The problem, however, is that add_device() seems to be intended for use with
IOMMU groups. i.e. After calling add_device() on a device that is should be
in a group, dev->iommu_group should have some meaningful value.
I think though, that if the IOMMU groups requirement can be removed, it should
be more versatile.
I believe that removing the IOMMU groups dependency can be achieved with the
patch below. Currently the intel-iommu seems to be the only implementation
using these callbacks.
What stops you from creating a single iommu_group and adding all device
to it? Thanks,
>
> Alex
Thanks for your comment.
Nothing stops me, but putting everything in one group doesn't really add
any benefit over not using groups at all, does it? It seems like its
just adding more complexity(i.e code) to the driver without adding any
additional functionality.
Does your iommu provide any isolation or is it translate-only. As Varun
notes, groups enable vfio, so if there's any possibility that you care
about userspace drivers for your devices or assigning devices to qemu
guests, then groups might be useful. The overhead of groups is really
meant to be very minimal for the iommu driver. That said, I guess I
have no real objection to generalizing the iommu bus notifier for
add/remove. Thanks,
Our iommu is used for translation. We plan on using vfio at some point,
though how well that fits with the current IOMMU ARM implementation
remains to be seen. So considering your and Varun's comments, it looks
like we want to be implementing groups after all, though generalizing
the add/remove still seems to me like a good idea.
Thanks,
Damian
Alex
Signed-off-by: Damian Hobson-Garcia <[email protected]>
---
drivers/iommu/intel-iommu.c | 3 ++-
drivers/iommu/iommu.c | 2 +-
include/linux/iommu.h | 4 ++--
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index b9d0911..a763872 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4217,7 +4217,8 @@ root_bus:
static void intel_iommu_remove_device(struct device *dev)
{
- iommu_group_remove_device(dev);
+ if (dev->iommu_group)
+ iommu_group_remove_device(dev);
}
static struct iommu_ops intel_iommu_ops = {
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ddbdaca..a6046c2 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -510,7 +510,7 @@ static int iommu_bus_notifier(struct notifier_block *nb,
if (ops->add_device)
return ops->add_device(dev);
} else if (action == BUS_NOTIFY_DEL_DEVICE) {
- if (ops->remove_device && dev->iommu_group) {
+ if (ops->remove_device) {
ops->remove_device(dev);
return 0;
}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f3b99e1..2715592 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -74,8 +74,8 @@ enum iommu_attr {
* @unmap: unmap a physically contiguous memory region from an iommu domain
* @iova_to_phys: translate iova to physical address
* @domain_has_cap: domain capabilities query
- * @add_device: add device to iommu grouping
- * @remove_device: remove device from iommu grouping
+ * @add_device: add device to iommu grouping or domain
+ * @remove_device: remove device from iommu grouping or domain
* @domain_get_attr: Query domain attributes
* @domain_set_attr: Change domain attributes
* @pgsize_bitmap: bitmap of supported page sizes
--
Damian Hobson-Garcia
IGEL Co.,Ltd
http://www.igel.co.jp
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
