[PATCH v2 17/17] drm/tegra: Use the iommu dma_owner mechanism
From: Jason Gunthorpe
Tegra joins many platform devices onto the same iommu_domain and builds
sort-of a DMA API on top of it.
Given that iommu_attach/detatch_device_shared() has supported this usage
model. Each device that wants to use the special domain will use
suppress_auto_claim_dma_owner and call iommu_attach_device_shared() which
will use dma owner framework to lock out other usages of the group and
refcount the domain attachment.
When the last device calls detatch the domain will be disconnected.
Signed-off-by: Jason Gunthorpe
Signed-off-by: Lu Baolu
---
drivers/gpu/drm/tegra/dc.c | 1 +
drivers/gpu/drm/tegra/drm.c | 55 ++--
drivers/gpu/drm/tegra/gr2d.c | 1 +
drivers/gpu/drm/tegra/gr3d.c | 1 +
drivers/gpu/drm/tegra/vic.c | 1 +
5 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index a29d64f87563..3a2458f56fbc 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -3111,4 +3111,5 @@ struct platform_driver tegra_dc_driver = {
},
.probe = tegra_dc_probe,
.remove = tegra_dc_remove,
+ .suppress_auto_claim_dma_owner = true,
};
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 8d37d6b00562..e6e2da799674 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -928,12 +928,15 @@ int tegra_drm_unregister_client(struct tegra_drm *tegra,
return 0;
}
+/*
+ * Clients which use this function must set suppress_auto_claim_dma_owner in
+ * their platform_driver's device_driver struct.
+ */
int host1x_client_iommu_attach(struct host1x_client *client)
{
struct iommu_domain *domain = iommu_get_domain_for_dev(client->dev);
struct drm_device *drm = dev_get_drvdata(client->host);
struct tegra_drm *tegra = drm->dev_private;
- struct iommu_group *group = NULL;
int err;
/*
@@ -941,47 +944,45 @@ int host1x_client_iommu_attach(struct host1x_client
*client)
* not the shared IOMMU domain, don't try to attach it to a different
* domain. This allows using the IOMMU-backed DMA API.
*/
+ client->group = NULL;
if (domain && domain != tegra->domain)
+ return iommu_device_set_dma_owner(client->dev,
+ DMA_OWNER_DMA_API, NULL);
+
+ if (!tegra->domain)
return 0;
- if (tegra->domain) {
- group = iommu_group_get(client->dev);
- if (!group)
- return -ENODEV;
-
- if (domain != tegra->domain) {
- err = iommu_attach_group(tegra->domain, group);
- if (err < 0) {
- iommu_group_put(group);
- return err;
- }
- }
+ err = iommu_device_set_dma_owner(client->dev,
+DMA_OWNER_PRIVATE_DOMAIN, NULL);
+ if (err)
+ return err;
- tegra->use_explicit_iommu = true;
+ err = iommu_attach_device_shared(tegra->domain, client->dev);
+ if (err) {
+ iommu_device_release_dma_owner(client->dev,
+ DMA_OWNER_PRIVATE_DOMAIN);
+ return err;
}
- client->group = group;
-
+ tegra->use_explicit_iommu = true;
+ client->group = client->dev->iommu_group;
return 0;
}
void host1x_client_iommu_detach(struct host1x_client *client)
{
+ struct iommu_domain *domain = iommu_get_domain_for_dev(client->dev);
struct drm_device *drm = dev_get_drvdata(client->host);
struct tegra_drm *tegra = drm->dev_private;
- struct iommu_domain *domain;
- if (client->group) {
- /*
-* Devices that are part of the same group may no longer be
-* attached to a domain at this point because their group may
-* have been detached by an earlier client.
-*/
- domain = iommu_get_domain_for_dev(client->dev);
- if (domain)
- iommu_detach_group(tegra->domain, client->group);
+ if (domain && domain != tegra->domain)
+ return iommu_device_release_dma_owner(client->dev,
+ DMA_OWNER_DMA_API);
- iommu_group_put(client->group);
+ if (client->group) {
+ iommu_detach_device_shared(tegra->domain, client->dev);
+ iommu_device_release_dma_owner(client->dev,
+ DMA_OWNER_PRIVATE_DOMAIN);
client->group = NULL;
}
}
diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c
index de288cba3905..8d92141c3c86 100644
--- a/drivers/gpu/drm/tegra/gr2d.c
+++
[PATCH v2 16/17] iommu: Remove iommu group changes notifier
The iommu group changes notifer is not referenced in the tree. Remove it
to avoid dead code.
Suggested-by: Christoph Hellwig
Signed-off-by: Lu Baolu
---
include/linux/iommu.h | 23 -
drivers/iommu/iommu.c | 75 ---
2 files changed, 98 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 8c81ba11ae8c..e7eeac801bcd 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -419,13 +419,6 @@ static inline void iommu_iotlb_gather_init(struct
iommu_iotlb_gather *gather)
};
}
-#define IOMMU_GROUP_NOTIFY_ADD_DEVICE 1 /* Device added */
-#define IOMMU_GROUP_NOTIFY_DEL_DEVICE 2 /* Pre Device removed */
-#define IOMMU_GROUP_NOTIFY_BIND_DRIVER 3 /* Pre Driver bind */
-#define IOMMU_GROUP_NOTIFY_BOUND_DRIVER4 /* Post Driver bind */
-#define IOMMU_GROUP_NOTIFY_UNBIND_DRIVER 5 /* Pre Driver unbind */
-#define IOMMU_GROUP_NOTIFY_UNBOUND_DRIVER 6 /* Post Driver unbind */
-
extern int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops);
extern int bus_iommu_probe(struct bus_type *bus);
extern bool iommu_present(struct bus_type *bus);
@@ -498,10 +491,6 @@ extern int iommu_group_for_each_dev(struct iommu_group
*group, void *data,
extern struct iommu_group *iommu_group_get(struct device *dev);
extern struct iommu_group *iommu_group_ref_get(struct iommu_group *group);
extern void iommu_group_put(struct iommu_group *group);
-extern int iommu_group_register_notifier(struct iommu_group *group,
-struct notifier_block *nb);
-extern int iommu_group_unregister_notifier(struct iommu_group *group,
- struct notifier_block *nb);
extern int iommu_register_device_fault_handler(struct device *dev,
iommu_dev_fault_handler_t handler,
void *data);
@@ -915,18 +904,6 @@ static inline void iommu_group_put(struct iommu_group
*group)
{
}
-static inline int iommu_group_register_notifier(struct iommu_group *group,
- struct notifier_block *nb)
-{
- return -ENODEV;
-}
-
-static inline int iommu_group_unregister_notifier(struct iommu_group *group,
- struct notifier_block *nb)
-{
- return 0;
-}
-
static inline
int iommu_register_device_fault_handler(struct device *dev,
iommu_dev_fault_handler_t handler,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f9cb96acbac8..9cb1cebfd884 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -18,7 +18,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -40,7 +39,6 @@ struct iommu_group {
struct kobject *devices_kobj;
struct list_head devices;
struct mutex mutex;
- struct blocking_notifier_head notifier;
void *iommu_data;
void (*iommu_data_release)(void *iommu_data);
char *name;
@@ -629,7 +627,6 @@ struct iommu_group *iommu_group_alloc(void)
mutex_init(>mutex);
INIT_LIST_HEAD(>devices);
INIT_LIST_HEAD(>entry);
- BLOCKING_INIT_NOTIFIER_HEAD(>notifier);
group->dma_owner = DMA_OWNER_NONE;
ret = ida_simple_get(_group_ida, 0, 0, GFP_KERNEL);
@@ -905,10 +902,6 @@ int iommu_group_add_device(struct iommu_group *group,
struct device *dev)
if (ret)
goto err_put_group;
- /* Notify any listeners about change to group. */
- blocking_notifier_call_chain(>notifier,
-IOMMU_GROUP_NOTIFY_ADD_DEVICE, dev);
-
trace_add_device_to_group(group->id, dev);
dev_info(dev, "Adding to iommu group %d\n", group->id);
@@ -950,10 +943,6 @@ void iommu_group_remove_device(struct device *dev)
dev_info(dev, "Removing from iommu group %d\n", group->id);
- /* Pre-notify listeners that a device is being removed. */
- blocking_notifier_call_chain(>notifier,
-IOMMU_GROUP_NOTIFY_DEL_DEVICE, dev);
-
mutex_lock(>mutex);
list_for_each_entry(tmp_device, >devices, list) {
if (tmp_device->dev == dev) {
@@ -1076,36 +1065,6 @@ void iommu_group_put(struct iommu_group *group)
}
EXPORT_SYMBOL_GPL(iommu_group_put);
-/**
- * iommu_group_register_notifier - Register a notifier for group changes
- * @group: the group to watch
- * @nb: notifier block to signal
- *
- * This function allows iommu group users to track changes in a group.
- * See include/linux/iommu.h for actions sent via this notifier. Caller
- * should hold a reference to the group throughout notifier registration.
- */
-int iommu_group_register_notifier(struct iommu_group *group,
- struct notifier_block *nb)
-{
- return
[PATCH v2 15/17] vfio: Remove iommu group notifier
The iommu core and driver core have been enhanced to avoid unsafe driver
binding to a live group after iommu_group_set_dma_owner(PRIVATE_USER)
has been called. There's no need to register iommu group notifier. This
removes the iommu group notifer which contains BUG_ON() and WARN().
Signed-off-by: Lu Baolu
---
drivers/vfio/vfio.c | 147
1 file changed, 147 deletions(-)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 5c81346367b1..33d984ff3cc5 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -71,7 +71,6 @@ struct vfio_group {
struct vfio_container *container;
struct list_headdevice_list;
struct mutexdevice_lock;
- struct notifier_block nb;
struct list_headvfio_next;
struct list_headcontainer_next;
atomic_topened;
@@ -274,8 +273,6 @@ void vfio_unregister_iommu_driver(const struct
vfio_iommu_driver_ops *ops)
}
EXPORT_SYMBOL_GPL(vfio_unregister_iommu_driver);
-static int vfio_iommu_group_notifier(struct notifier_block *nb,
-unsigned long action, void *data);
static void vfio_group_get(struct vfio_group *group);
/**
@@ -395,13 +392,6 @@ static struct vfio_group *vfio_create_group(struct
iommu_group *iommu_group,
goto err_put;
}
- group->nb.notifier_call = vfio_iommu_group_notifier;
- err = iommu_group_register_notifier(iommu_group, >nb);
- if (err) {
- ret = ERR_PTR(err);
- goto err_put;
- }
-
mutex_lock(_lock);
/* Did we race creating this group? */
@@ -422,7 +412,6 @@ static struct vfio_group *vfio_create_group(struct
iommu_group *iommu_group,
err_unlock:
mutex_unlock(_lock);
- iommu_group_unregister_notifier(group->iommu_group, >nb);
err_put:
put_device(>dev);
return ret;
@@ -447,7 +436,6 @@ static void vfio_group_put(struct vfio_group *group)
cdev_device_del(>cdev, >dev);
mutex_unlock(_lock);
- iommu_group_unregister_notifier(group->iommu_group, >nb);
put_device(>dev);
}
@@ -503,141 +491,6 @@ static struct vfio_device *vfio_group_get_device(struct
vfio_group *group,
return NULL;
}
-/*
- * Some drivers, like pci-stub, are only used to prevent other drivers from
- * claiming a device and are therefore perfectly legitimate for a user owned
- * group. The pci-stub driver has no dependencies on DMA or the IOVA mapping
- * of the device, but it does prevent the user from having direct access to
- * the device, which is useful in some circumstances.
- *
- * We also assume that we can include PCI interconnect devices, ie. bridges.
- * IOMMU grouping on PCI necessitates that if we lack isolation on a bridge
- * then all of the downstream devices will be part of the same IOMMU group as
- * the bridge. Thus, if placing the bridge into the user owned IOVA space
- * breaks anything, it only does so for user owned devices downstream. Note
- * that error notification via MSI can be affected for platforms that handle
- * MSI within the same IOVA space as DMA.
- */
-static const char * const vfio_driver_allowed[] = { "pci-stub" };
-
-static bool vfio_dev_driver_allowed(struct device *dev,
- struct device_driver *drv)
-{
- if (dev_is_pci(dev)) {
- struct pci_dev *pdev = to_pci_dev(dev);
-
- if (pdev->hdr_type != PCI_HEADER_TYPE_NORMAL)
- return true;
- }
-
- return match_string(vfio_driver_allowed,
- ARRAY_SIZE(vfio_driver_allowed),
- drv->name) >= 0;
-}
-
-/*
- * A vfio group is viable for use by userspace if all devices are in
- * one of the following states:
- * - driver-less
- * - bound to a vfio driver
- * - bound to an otherwise allowed driver
- * - a PCI interconnect device
- *
- * We use two methods to determine whether a device is bound to a vfio
- * driver. The first is to test whether the device exists in the vfio
- * group. The second is to test if the device exists on the group
- * unbound_list, indicating it's in the middle of transitioning from
- * a vfio driver to driver-less.
- */
-static int vfio_dev_viable(struct device *dev, void *data)
-{
- struct vfio_group *group = data;
- struct vfio_device *device;
- struct device_driver *drv = READ_ONCE(dev->driver);
-
- if (!drv || vfio_dev_driver_allowed(dev, drv))
- return 0;
-
- device = vfio_group_get_device(group, dev);
- if (device) {
- vfio_device_put(device);
- return 0;
- }
-
- return -EINVAL;
-}
-
-/**
- * Async device support
- */
-static int vfio_group_nb_add_dev(struct vfio_group *group, struct device *dev)
-{
- struct vfio_device *device;
-
[PATCH v2 14/17] vfio: Delete the unbound_list
From: Jason Gunthorpe
commit 60720a0fc646 ("vfio: Add device tracking during unbind") added the
unbound list to plug a problem with KVM where KVM_DEV_VFIO_GROUP_DEL
relied on vfio_group_get_external_user() succeeding to return the
vfio_group from a group file descriptor. The unbound list allowed
vfio_group_get_external_user() to continue to succeed in edge cases.
However commit 5d6dee80a1e9 ("vfio: New external user group/file match")
deleted the call to vfio_group_get_external_user() during
KVM_DEV_VFIO_GROUP_DEL. Instead vfio_external_group_match_file() is used
to directly match the file descriptor to the group pointer.
This in turn avoids the call down to vfio_dev_viable() during
KVM_DEV_VFIO_GROUP_DEL and also avoids the trouble the first commit was
trying to fix.
There are no other users of vfio_dev_viable() that care about the time
after vfio_unregister_group_dev() returns, so simply delete the
unbound_list entirely.
Reviewed-by: Chaitanya Kulkarni
Signed-off-by: Jason Gunthorpe
Signed-off-by: Lu Baolu
---
drivers/vfio/vfio.c | 74 ++---
1 file changed, 2 insertions(+), 72 deletions(-)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 63e24f746b82..5c81346367b1 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -62,11 +62,6 @@ struct vfio_container {
boolnoiommu;
};
-struct vfio_unbound_dev {
- struct device *dev;
- struct list_headunbound_next;
-};
-
struct vfio_group {
struct device dev;
struct cdev cdev;
@@ -79,8 +74,6 @@ struct vfio_group {
struct notifier_block nb;
struct list_headvfio_next;
struct list_headcontainer_next;
- struct list_headunbound_list;
- struct mutexunbound_lock;
atomic_topened;
wait_queue_head_t container_q;
enum vfio_group_typetype;
@@ -340,16 +333,8 @@ vfio_group_get_from_iommu(struct iommu_group *iommu_group)
static void vfio_group_release(struct device *dev)
{
struct vfio_group *group = container_of(dev, struct vfio_group, dev);
- struct vfio_unbound_dev *unbound, *tmp;
-
- list_for_each_entry_safe(unbound, tmp,
->unbound_list, unbound_next) {
- list_del(>unbound_next);
- kfree(unbound);
- }
mutex_destroy(>device_lock);
- mutex_destroy(>unbound_lock);
iommu_group_put(group->iommu_group);
ida_free(_ida, MINOR(group->dev.devt));
kfree(group);
@@ -381,8 +366,6 @@ static struct vfio_group *vfio_group_alloc(struct
iommu_group *iommu_group,
refcount_set(>users, 1);
INIT_LIST_HEAD(>device_list);
mutex_init(>device_lock);
- INIT_LIST_HEAD(>unbound_list);
- mutex_init(>unbound_lock);
init_waitqueue_head(>container_q);
group->iommu_group = iommu_group;
/* put in vfio_group_release() */
@@ -571,19 +554,8 @@ static int vfio_dev_viable(struct device *dev, void *data)
struct vfio_group *group = data;
struct vfio_device *device;
struct device_driver *drv = READ_ONCE(dev->driver);
- struct vfio_unbound_dev *unbound;
- int ret = -EINVAL;
- mutex_lock(>unbound_lock);
- list_for_each_entry(unbound, >unbound_list, unbound_next) {
- if (dev == unbound->dev) {
- ret = 0;
- break;
- }
- }
- mutex_unlock(>unbound_lock);
-
- if (!ret || !drv || vfio_dev_driver_allowed(dev, drv))
+ if (!drv || vfio_dev_driver_allowed(dev, drv))
return 0;
device = vfio_group_get_device(group, dev);
@@ -592,7 +564,7 @@ static int vfio_dev_viable(struct device *dev, void *data)
return 0;
}
- return ret;
+ return -EINVAL;
}
/**
@@ -634,7 +606,6 @@ static int vfio_iommu_group_notifier(struct notifier_block
*nb,
{
struct vfio_group *group = container_of(nb, struct vfio_group, nb);
struct device *dev = data;
- struct vfio_unbound_dev *unbound;
switch (action) {
case IOMMU_GROUP_NOTIFY_ADD_DEVICE:
@@ -663,28 +634,6 @@ static int vfio_iommu_group_notifier(struct notifier_block
*nb,
__func__, iommu_group_id(group->iommu_group),
dev->driver->name);
break;
- case IOMMU_GROUP_NOTIFY_UNBOUND_DRIVER:
- dev_dbg(dev, "%s: group %d unbound from driver\n", __func__,
- iommu_group_id(group->iommu_group));
- /*
-* XXX An unbound device in a live group is ok, but we'd
-* really like to avoid the above BUG_ON by preventing other
-* drivers
[PATCH v2 13/17] vfio: Remove use of vfio_group_viable()
As DMA USER ownership is claimed for the iommu group when a vfio group is
added to a vfio container, the vfio group viability is guaranteed as long
as group->container_users > 0. Remove those unnecessary group viability
checks which are only hit when group->container_users is not zero.
The only remaining reference is in GROUP_GET_STATUS, which could be called
at any time when group fd is valid. Here we just replace the
vfio_group_viable() by directly calling iommu core to get viability status.
Signed-off-by: Lu Baolu
---
drivers/vfio/vfio.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 00f89acfec39..63e24f746b82 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1316,12 +1316,6 @@ static int vfio_group_set_container(struct vfio_group
*group, int container_fd)
return ret;
}
-static bool vfio_group_viable(struct vfio_group *group)
-{
- return (iommu_group_for_each_dev(group->iommu_group,
-group, vfio_dev_viable) == 0);
-}
-
static int vfio_group_add_container_user(struct vfio_group *group)
{
if (!atomic_inc_not_zero(>container_users))
@@ -1331,7 +1325,7 @@ static int vfio_group_add_container_user(struct
vfio_group *group)
atomic_dec(>container_users);
return -EPERM;
}
- if (!group->container->iommu_driver || !vfio_group_viable(group)) {
+ if (!group->container->iommu_driver) {
atomic_dec(>container_users);
return -EINVAL;
}
@@ -1349,7 +1343,7 @@ static int vfio_group_get_device_fd(struct vfio_group
*group, char *buf)
int ret = 0;
if (0 == atomic_read(>container_users) ||
- !group->container->iommu_driver || !vfio_group_viable(group))
+ !group->container->iommu_driver)
return -EINVAL;
if (group->type == VFIO_NO_IOMMU && !capable(CAP_SYS_RAWIO))
@@ -1441,11 +1435,11 @@ static long vfio_group_fops_unl_ioctl(struct file
*filep,
status.flags = 0;
- if (vfio_group_viable(group))
- status.flags |= VFIO_GROUP_FLAGS_VIABLE;
-
if (group->container)
- status.flags |= VFIO_GROUP_FLAGS_CONTAINER_SET;
+ status.flags |= VFIO_GROUP_FLAGS_CONTAINER_SET |
+ VFIO_GROUP_FLAGS_VIABLE;
+ else if (iommu_group_dma_owner_unclaimed(group->iommu_group))
+ status.flags |= VFIO_GROUP_FLAGS_VIABLE;
if (copy_to_user((void __user *)arg, , minsz))
return -EFAULT;
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 12/17] vfio: Set DMA USER ownership for VFIO devices
Set DMA_OWNER_PRIVATE_DOMAIN_USER when an iommu group is set to a
container, and release DMA_OWNER_USER once the iommu group is unset
from a container.
Signed-off-by: Lu Baolu
---
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 1 +
drivers/vfio/pci/vfio_pci.c | 1 +
drivers/vfio/platform/vfio_amba.c | 1 +
drivers/vfio/platform/vfio_platform.c | 1 +
drivers/vfio/vfio.c | 13 -
5 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 6e2e62c6f47a..5f36ffbb07d1 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -588,6 +588,7 @@ static struct fsl_mc_driver vfio_fsl_mc_driver = {
.name = "vfio-fsl-mc",
.owner = THIS_MODULE,
},
+ .suppress_auto_claim_dma_owner = true,
};
static int __init vfio_fsl_mc_driver_init(void)
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index a5ce92beb655..31810b074aa4 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -193,6 +193,7 @@ static struct pci_driver vfio_pci_driver = {
.remove = vfio_pci_remove,
.sriov_configure= vfio_pci_sriov_configure,
.err_handler= _pci_core_err_handlers,
+ .suppress_auto_claim_dma_owner = true,
};
static void __init vfio_pci_fill_ids(void)
diff --git a/drivers/vfio/platform/vfio_amba.c
b/drivers/vfio/platform/vfio_amba.c
index badfffea14fb..e598d6af90ad 100644
--- a/drivers/vfio/platform/vfio_amba.c
+++ b/drivers/vfio/platform/vfio_amba.c
@@ -95,6 +95,7 @@ static struct amba_driver vfio_amba_driver = {
.name = "vfio-amba",
.owner = THIS_MODULE,
},
+ .suppress_auto_claim_dma_owner = true,
};
module_amba_driver(vfio_amba_driver);
diff --git a/drivers/vfio/platform/vfio_platform.c
b/drivers/vfio/platform/vfio_platform.c
index 68a1c87066d7..8bda68775b97 100644
--- a/drivers/vfio/platform/vfio_platform.c
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -76,6 +76,7 @@ static struct platform_driver vfio_platform_driver = {
.driver = {
.name = "vfio-platform",
},
+ .suppress_auto_claim_dma_owner = true,
};
module_platform_driver(vfio_platform_driver);
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 82fb75464f92..00f89acfec39 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1198,6 +1198,9 @@ static void __vfio_group_unset_container(struct
vfio_group *group)
driver->ops->detach_group(container->iommu_data,
group->iommu_group);
+ iommu_group_release_dma_owner(group->iommu_group,
+ DMA_OWNER_PRIVATE_DOMAIN_USER);
+
group->container = NULL;
wake_up(>container_q);
list_del(>container_next);
@@ -1282,13 +1285,21 @@ static int vfio_group_set_container(struct vfio_group
*group, int container_fd)
goto unlock_out;
}
+ ret = iommu_group_set_dma_owner(group->iommu_group,
+ DMA_OWNER_PRIVATE_DOMAIN_USER, f.file);
+ if (ret)
+ goto unlock_out;
+
driver = container->iommu_driver;
if (driver) {
ret = driver->ops->attach_group(container->iommu_data,
group->iommu_group,
group->type);
- if (ret)
+ if (ret) {
+ iommu_group_release_dma_owner(group->iommu_group,
+
DMA_OWNER_PRIVATE_DOMAIN_USER);
goto unlock_out;
+ }
}
group->container = container;
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 11/17] iommu: Add iommu_at[de]tach_device_shared() for multi-device groups
The iommu_attach/detach_device() interfaces were exposed for the device
drivers to attach/detach their own domains. The commit <426a273834eae>
("iommu: Limit iommu_attach/detach_device to device with their own group")
restricted them to singleton groups to avoid different device in a group
attaching different domain.
As we've introduced device DMA ownership into the iommu core. We can now
introduce interfaces for muliple-device groups, and "all devices are in the
same address space" is still guaranteed.
The iommu_attach/detach_device_shared() could be used when multiple drivers
sharing the group claim the DMA_OWNER_PRIVATE_DOMAIN ownership. The first
call of iommu_attach_device_shared() attaches the domain to the group.
Other drivers could join it later. The domain will be detached from the
group after all drivers unjoin it.
Signed-off-by: Jason Gunthorpe
Signed-off-by: Lu Baolu
---
include/linux/iommu.h | 13 +
drivers/iommu/iommu.c | 61 +++
2 files changed, 74 insertions(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index afcc07bc8d41..8c81ba11ae8c 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -705,6 +705,8 @@ int iommu_group_set_dma_owner(struct iommu_group *group,
enum iommu_dma_owner ow
void *owner_cookie);
void iommu_group_release_dma_owner(struct iommu_group *group, enum
iommu_dma_owner owner);
bool iommu_group_dma_owner_unclaimed(struct iommu_group *group);
+int iommu_attach_device_shared(struct iommu_domain *domain, struct device
*dev);
+void iommu_detach_device_shared(struct iommu_domain *domain, struct device
*dev);
#else /* CONFIG_IOMMU_API */
@@ -745,11 +747,22 @@ static inline int iommu_attach_device(struct iommu_domain
*domain,
return -ENODEV;
}
+static inline int iommu_attach_device_shared(struct iommu_domain *domain,
+struct device *dev)
+{
+ return -ENODEV;
+}
+
static inline void iommu_detach_device(struct iommu_domain *domain,
struct device *dev)
{
}
+static inline void iommu_detach_device_shared(struct iommu_domain *domain,
+ struct device *dev)
+{
+}
+
static inline struct iommu_domain *iommu_get_domain_for_dev(struct device *dev)
{
return NULL;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 423197db99a9..f9cb96acbac8 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -50,6 +50,7 @@ struct iommu_group {
struct list_head entry;
enum iommu_dma_owner dma_owner;
refcount_t owner_cnt;
+ refcount_t attach_cnt;
void *owner_cookie;
};
@@ -2026,6 +2027,41 @@ int iommu_attach_device(struct iommu_domain *domain,
struct device *dev)
}
EXPORT_SYMBOL_GPL(iommu_attach_device);
+int iommu_attach_device_shared(struct iommu_domain *domain, struct device *dev)
+{
+ struct iommu_group *group;
+ int ret = 0;
+
+ group = iommu_group_get(dev);
+ if (!group)
+ return -ENODEV;
+
+ mutex_lock(>mutex);
+ if (refcount_inc_not_zero(>attach_cnt)) {
+ if (group->domain != domain ||
+ (group->dma_owner != DMA_OWNER_PRIVATE_DOMAIN &&
+group->dma_owner != DMA_OWNER_PRIVATE_DOMAIN_USER)) {
+ refcount_dec(>attach_cnt);
+ ret = -EBUSY;
+ }
+
+ goto unlock_out;
+ }
+
+ ret = __iommu_attach_group(domain, group);
+ if (ret)
+ goto unlock_out;
+
+ refcount_set(>owner_cnt, 1);
+
+unlock_out:
+ mutex_unlock(>mutex);
+ iommu_group_put(group);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_attach_device_shared);
+
int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain)
{
const struct iommu_ops *ops = domain->ops;
@@ -2281,6 +2317,31 @@ void iommu_detach_device(struct iommu_domain *domain,
struct device *dev)
}
EXPORT_SYMBOL_GPL(iommu_detach_device);
+void iommu_detach_device_shared(struct iommu_domain *domain, struct device
*dev)
+{
+ struct iommu_group *group;
+
+ group = iommu_group_get(dev);
+ if (!group)
+ return;
+
+ mutex_lock(>mutex);
+ if (WARN_ON(group->domain != domain ||
+ (group->dma_owner != DMA_OWNER_PRIVATE_DOMAIN &&
+group->dma_owner != DMA_OWNER_PRIVATE_DOMAIN_USER)))
+ goto unlock_out;
+
+ if (refcount_dec_and_test(>owner_cnt)) {
+ __iommu_detach_group(domain, group);
+ group->dma_owner = DMA_OWNER_NONE;
+ }
+
+unlock_out:
+ mutex_unlock(>mutex);
+ iommu_group_put(group);
+}
+EXPORT_SYMBOL_GPL(iommu_detach_device_shared);
+
struct iommu_domain *iommu_get_domain_for_dev(struct device *dev)
{
struct iommu_domain *domain;
--
2.25.1
[PATCH v2 10/17] iommu: Expose group variants of dma ownership interfaces
The vfio needs to set DMA_OWNER_PRIVATE_DOMAIN_USER for the entire group
when attaching it to a vfio container. Expose group variants of setting/
releasing dma ownership for this purpose.
This also exposes the helper iommu_group_dma_owner_unclaimed() for vfio
to report to userspace if the group is viable to user assignment for
compatibility with VFIO_GROUP_FLAGS_VIABLE.
Signed-off-by: Lu Baolu
---
include/linux/iommu.h | 21
drivers/iommu/iommu.c | 58 +++
2 files changed, 79 insertions(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 24676b498f38..afcc07bc8d41 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -701,6 +701,10 @@ u32 iommu_sva_get_pasid(struct iommu_sva *handle);
int iommu_device_set_dma_owner(struct device *dev, enum iommu_dma_owner owner,
void *owner_cookie);
void iommu_device_release_dma_owner(struct device *dev, enum iommu_dma_owner
owner);
+int iommu_group_set_dma_owner(struct iommu_group *group, enum iommu_dma_owner
owner,
+ void *owner_cookie);
+void iommu_group_release_dma_owner(struct iommu_group *group, enum
iommu_dma_owner owner);
+bool iommu_group_dma_owner_unclaimed(struct iommu_group *group);
#else /* CONFIG_IOMMU_API */
@@ -1117,6 +1121,23 @@ static inline void iommu_device_release_dma_owner(struct
device *dev,
enum iommu_dma_owner owner)
{
}
+
+static inline int iommu_group_set_dma_owner(struct iommu_group *group,
+ enum iommu_dma_owner owner,
+ void *owner_cookie)
+{
+ return -EINVAL;
+}
+
+static inline void iommu_group_release_dma_owner(struct iommu_group *group,
+enum iommu_dma_owner owner)
+{
+}
+
+static inline bool iommu_group_dma_owner_unclaimed(struct iommu_group *group)
+{
+ return false;
+}
#endif /* CONFIG_IOMMU_API */
/**
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0cba04a8ea3b..423197db99a9 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3425,6 +3425,64 @@ static void __iommu_group_release_dma_owner(struct
iommu_group *group,
}
}
+/**
+ * iommu_group_set_dma_owner() - Set DMA ownership of a group
+ * @group: The group.
+ * @owner: DMA owner type.
+ * @owner_cookie: Caller specified pointer. Could be used for exclusive
+ *declaration. Could be NULL.
+ *
+ * This is to support backward compatibility for legacy vfio which manages
+ * dma ownership in group level. New invocations on this interface should be
+ * prohibited. Instead, please turn to iommu_device_set_dma_owner().
+ */
+int iommu_group_set_dma_owner(struct iommu_group *group, enum iommu_dma_owner
owner,
+ void *owner_cookie)
+{
+ int ret;
+
+ mutex_lock(>mutex);
+ ret = __iommu_group_set_dma_owner(group, owner, owner_cookie);
+ mutex_unlock(>mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_group_set_dma_owner);
+
+/**
+ * iommu_group_release_dma_owner() - Release DMA ownership of a group
+ * @group: The group.
+ * @owner: DMA owner type.
+ *
+ * Release the DMA ownership claimed by iommu_group_set_dma_owner().
+ */
+void iommu_group_release_dma_owner(struct iommu_group *group, enum
iommu_dma_owner owner)
+{
+ mutex_lock(>mutex);
+ __iommu_group_release_dma_owner(group, owner);
+ mutex_unlock(>mutex);
+}
+EXPORT_SYMBOL_GPL(iommu_group_release_dma_owner);
+
+/**
+ * iommu_group_dma_owner_unclaimed() - Is group dma ownership claimed
+ * @group: The group.
+ *
+ * This provides status check on a given group. It is racey and only for
+ * non-binding status reporting.
+ */
+bool iommu_group_dma_owner_unclaimed(struct iommu_group *group)
+{
+ enum iommu_dma_owner owner;
+
+ mutex_lock(>mutex);
+ owner = group->dma_owner;
+ mutex_unlock(>mutex);
+
+ return owner == DMA_OWNER_NONE;
+}
+EXPORT_SYMBOL_GPL(iommu_group_dma_owner_unclaimed);
+
/**
* iommu_device_set_dma_owner() - Set DMA ownership of a device
* @dev: The device.
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 09/17] iommu: Add security context management for assigned devices
When an iommu group has DMA_OWNER_PRIVATE_DOMAIN_USER set for the first
time, it is a contract that the group could be assigned to userspace from
now on. The group must be detached from the default iommu domain and all
devices in this group are blocked from doing DMA until it is attached to a
user controlled iommu_domain. Correspondingly, the default domain should
be reattached after the last DMA_OWNER_USER is released.
Signed-off-by: Lu Baolu
---
drivers/iommu/iommu.c | 42 +++---
1 file changed, 39 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 1de520a07518..0cba04a8ea3b 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -292,7 +292,12 @@ int iommu_probe_device(struct device *dev)
mutex_lock(>mutex);
iommu_alloc_default_domain(group, dev);
- if (group->default_domain) {
+ /*
+* If any device in the group has been initialized for user dma,
+* avoid attaching the default domain.
+*/
+ if (group->default_domain &&
+ group->dma_owner != DMA_OWNER_PRIVATE_DOMAIN_USER) {
ret = __iommu_attach_device(group->default_domain, dev);
if (ret) {
mutex_unlock(>mutex);
@@ -2324,7 +2329,7 @@ static int __iommu_attach_group(struct iommu_domain
*domain,
{
int ret;
- if (group->default_domain && group->domain != group->default_domain)
+ if (group->domain && group->domain != group->default_domain)
return -EBUSY;
ret = __iommu_group_for_each_dev(group, domain,
@@ -2361,7 +2366,12 @@ static void __iommu_detach_group(struct iommu_domain
*domain,
{
int ret;
- if (!group->default_domain) {
+ /*
+* If any device in the group has been initialized for user dma,
+* avoid re-attaching the default domain.
+*/
+ if (!group->default_domain ||
+ group->dma_owner == DMA_OWNER_PRIVATE_DOMAIN_USER) {
__iommu_group_for_each_dev(group, domain,
iommu_group_do_detach_device);
group->domain = NULL;
@@ -3371,6 +3381,23 @@ static int __iommu_group_set_dma_owner(struct
iommu_group *group,
}
group->dma_owner = owner;
+
+ /*
+* We must ensure that any device DMAs issued after this call
+* are discarded. DMAs can only reach real memory once someone
+* has attached a real domain.
+*/
+ if (owner == DMA_OWNER_PRIVATE_DOMAIN_USER) {
+ if (group->domain) {
+ if (group->domain != group->default_domain) {
+ group->dma_owner = DMA_OWNER_NONE;
+ return -EBUSY;
+ }
+
+ __iommu_detach_group(group->domain, group);
+ }
+ }
+
group->owner_cookie = owner_cookie;
refcount_set(>owner_cnt, 1);
@@ -3387,6 +3414,15 @@ static void __iommu_group_release_dma_owner(struct
iommu_group *group,
return;
group->dma_owner = DMA_OWNER_NONE;
+
+ /*
+* The UNMANAGED domain should be detached before all USER
+* owners have been released.
+*/
+ if (owner == DMA_OWNER_PRIVATE_DOMAIN_USER) {
+ if (!WARN_ON(group->domain) && group->default_domain)
+ __iommu_attach_group(group->default_domain, group);
+ }
}
/**
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 08/17] PCI: portdrv: Suppress kernel DMA ownership auto-claiming
IOMMU grouping on PCI necessitates that if we lack isolation on a bridge
then all of the downstream devices will be part of the same IOMMU group
as the bridge. The existing vfio framework allows the portdrv driver to
be bound to the bridge while its downstream devices are assigned to user
space. The pci_dma_configure() marks the iommu_group as containing only
devices with kernel drivers that manage DMA. Avoid this default behavior
for the portdrv driver in order for compatibility with the current vfio
policy.
The commit 5f096b14d421b ("vfio: Whitelist PCI bridges") extended above
policy to all kernel drivers of bridge class. This is not always safe.
For example, The shpchp_core driver relies on the PCI MMIO access for the
controller functionality. With its downstream devices assigned to the
userspace, the MMIO might be changed through user initiated P2P accesses
without any notification. This might break the kernel driver integrity
and lead to some unpredictable consequences.
For any bridge driver, in order to avoiding default kernel DMA ownership
claiming, we should consider:
1) Does the bridge driver use DMA? Calling pci_set_master() or
a dma_map_* API is a sure indicate the driver is doing DMA
2) If the bridge driver uses MMIO, is it tolerant to hostile
userspace also touching the same MMIO registers via P2P DMA
attacks?
Conservatively if the driver maps an MMIO region at all, we can say that
it fails the test.
Suggested-by: Jason Gunthorpe
Suggested-by: Kevin Tian
Signed-off-by: Lu Baolu
---
drivers/pci/pcie/portdrv_pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index 35eca6277a96..c66a83f2c987 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -202,6 +202,8 @@ static struct pci_driver pcie_portdriver = {
.err_handler= _portdrv_err_handler,
+ .suppress_auto_claim_dma_owner = true,
+
.driver.pm = PCIE_PORTDRV_PM_OPS,
};
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 07/17] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming
The pci_dma_configure() marks the iommu_group as containing only devices
with kernel drivers that manage DMA. Avoid this default behavior for the
pci_stub because it does not program any DMA itself. This allows the
pci_stub still able to be used by the admin to block driver binding after
applying the DMA ownership to vfio.
Signed-off-by: Lu Baolu
---
drivers/pci/pci-stub.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/pci-stub.c b/drivers/pci/pci-stub.c
index e408099fea52..14b9b9f2ad2b 100644
--- a/drivers/pci/pci-stub.c
+++ b/drivers/pci/pci-stub.c
@@ -36,6 +36,7 @@ static struct pci_driver stub_driver = {
.name = "pci-stub",
.id_table = NULL, /* only dynamic id's */
.probe = pci_stub_probe,
+ .suppress_auto_claim_dma_owner = true,
};
static int __init pci_stub_init(void)
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 06/17] bus: fsl-mc: Add driver dma ownership management
Multiple fsl-mc devices may be placed in the same IOMMU group because
they cannot be isolated from each other. These devices must either be
entirely under kernel control or userspace control, never a mixture. This
checks and sets DMA ownership during driver binding, and release the
ownership during driver unbinding.
Driver may set a new flag (suppress_auto_claim_dma_owner) to disable auto
claiming DMA_OWNER_DMA_API ownership in the binding process. For instance,
the userspace framework drivers (vfio etc.) which need to manually claim
DMA_OWNER_PRIVATE_DOMAIN_USER when assigning a device to userspace.
Signed-off-by: Lu Baolu
---
include/linux/fsl/mc.h | 5 +
drivers/bus/fsl-mc/fsl-mc-bus.c | 26 --
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/include/linux/fsl/mc.h b/include/linux/fsl/mc.h
index e026f6c48b49..3a26421ee3dc 100644
--- a/include/linux/fsl/mc.h
+++ b/include/linux/fsl/mc.h
@@ -32,6 +32,10 @@ struct fsl_mc_io;
* @shutdown: Function called at shutdown time to quiesce the device
* @suspend: Function called when a device is stopped
* @resume: Function called when a device is resumed
+ * @suppress_auto_claim_dma_owner: Disable auto claiming of kernel DMA owner.
+ * Drivers which don't require DMA or want to manually claim the
+ * owner type (e.g. userspace driver frameworks) could set this
+ * flag.
*
* Generic DPAA device driver object for device drivers that are registered
* with a DPRC bus. This structure is to be embedded in each device-specific
@@ -45,6 +49,7 @@ struct fsl_mc_driver {
void (*shutdown)(struct fsl_mc_device *dev);
int (*suspend)(struct fsl_mc_device *dev, pm_message_t state);
int (*resume)(struct fsl_mc_device *dev);
+ bool suppress_auto_claim_dma_owner;
};
#define to_fsl_mc_driver(_drv) \
diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index 8fd4a356a86e..23dd5f0070e7 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -140,15 +140,36 @@ static int fsl_mc_dma_configure(struct device *dev)
{
struct device *dma_dev = dev;
struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+ struct fsl_mc_driver *mc_drv = to_fsl_mc_driver(dev->driver);
u32 input_id = mc_dev->icid;
+ int ret;
+
+ if (!mc_drv->suppress_auto_claim_dma_owner) {
+ ret = iommu_device_set_dma_owner(dev, DMA_OWNER_DMA_API, NULL);
+ if (ret)
+ return ret;
+ }
while (dev_is_fsl_mc(dma_dev))
dma_dev = dma_dev->parent;
if (dev_of_node(dma_dev))
- return of_dma_configure_id(dev, dma_dev->of_node, 0, _id);
+ ret = of_dma_configure_id(dev, dma_dev->of_node, 0, _id);
+ else
+ ret = acpi_dma_configure_id(dev, DEV_DMA_COHERENT, _id);
+
+ if (ret && !mc_drv->suppress_auto_claim_dma_owner)
+ iommu_device_release_dma_owner(dev, DMA_OWNER_DMA_API);
+
+ return ret;
+}
+
+static void fsl_mc_dma_unconfigure(struct device *dev)
+{
+ struct fsl_mc_driver *mc_drv = to_fsl_mc_driver(dev->driver);
- return acpi_dma_configure_id(dev, DEV_DMA_COHERENT, _id);
+ if (!mc_drv->suppress_auto_claim_dma_owner)
+ iommu_device_release_dma_owner(dev, DMA_OWNER_DMA_API);
}
static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
@@ -312,6 +333,7 @@ struct bus_type fsl_mc_bus_type = {
.match = fsl_mc_bus_match,
.uevent = fsl_mc_bus_uevent,
.dma_configure = fsl_mc_dma_configure,
+ .dma_unconfigure = fsl_mc_dma_unconfigure,
.dev_groups = fsl_mc_dev_groups,
.bus_groups = fsl_mc_bus_groups,
};
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 05/17] amba: Add driver dma ownership management
Multiple amba devices may be placed in the same IOMMU group because
they cannot be isolated from each other. These devices must either be
entirely under kernel control or userspace control, never a mixture. This
checks and sets DMA ownership during driver binding, and release the
ownership during driver unbinding.
Driver may set a new flag (suppress_auto_claim_dma_owner) to disable auto
claiming DMA_OWNER_DMA_API ownership in the binding process. For instance,
the userspace framework drivers (vfio etc.) which need to manually claim
DMA_OWNER_PRIVATE_DOMAIN_USER when assigning a device to userspace.
Signed-off-by: Lu Baolu
---
include/linux/amba/bus.h | 1 +
drivers/amba/bus.c | 30 +-
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index edfcf7a14dcd..745c5a60ddd8 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -79,6 +79,7 @@ struct amba_driver {
void(*remove)(struct amba_device *);
void(*shutdown)(struct amba_device *);
const struct amba_id*id_table;
+ bool suppress_auto_claim_dma_owner;
};
/*
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 720aa6cdd402..cd0cd9d141ec 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
@@ -305,6 +306,32 @@ static const struct dev_pm_ops amba_pm = {
)
};
+static int amba_dma_configure(struct device *dev)
+{
+ struct amba_driver *drv = to_amba_driver(dev->driver);
+ int ret;
+
+ if (!drv->suppress_auto_claim_dma_owner) {
+ ret = iommu_device_set_dma_owner(dev, DMA_OWNER_DMA_API, NULL);
+ if (ret)
+ return ret;
+ }
+
+ ret = platform_dma_configure(dev);
+ if (ret && !drv->suppress_auto_claim_dma_owner)
+ iommu_device_release_dma_owner(dev, DMA_OWNER_DMA_API);
+
+ return ret;
+}
+
+static void amba_dma_unconfigure(struct device *dev)
+{
+ struct amba_driver *drv = to_amba_driver(dev->driver);
+
+ if (!drv->suppress_auto_claim_dma_owner)
+ iommu_device_release_dma_owner(dev, DMA_OWNER_DMA_API);
+}
+
/*
* Primecells are part of the Advanced Microcontroller Bus Architecture,
* so we call the bus "amba".
@@ -319,7 +346,8 @@ struct bus_type amba_bustype = {
.probe = amba_probe,
.remove = amba_remove,
.shutdown = amba_shutdown,
- .dma_configure = platform_dma_configure,
+ .dma_configure = amba_dma_configure,
+ .dma_unconfigure = amba_dma_unconfigure,
.pm = _pm,
};
EXPORT_SYMBOL_GPL(amba_bustype);
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 04/17] driver core: platform: Add driver dma ownership management
Multiple platform devices may be placed in the same IOMMU group because
they cannot be isolated from each other. These devices must either be
entirely under kernel control or userspace control, never a mixture. This
checks and sets DMA ownership during driver binding, and release the
ownership during driver unbinding.
Driver may set a new flag (suppress_auto_claim_dma_owner) to disable auto
claiming DMA_OWNER_DMA_API ownership in the binding process. For instance,
the userspace framework drivers (vfio etc.) which need to manually claim
DMA_OWNER_PRIVATE_DOMAIN_USER when assigning a device to userspace.
Signed-off-by: Lu Baolu
---
include/linux/platform_device.h | 1 +
drivers/base/platform.c | 30 +-
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 7c96f169d274..779bcf2a851c 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -210,6 +210,7 @@ struct platform_driver {
struct device_driver driver;
const struct platform_device_id *id_table;
bool prevent_deferred_probe;
+ bool suppress_auto_claim_dma_owner;
};
#define to_platform_driver(drv)(container_of((drv), struct
platform_driver, \
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 598acf93a360..df4b385c8a52 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include "base.h"
#include "power/power.h"
@@ -1465,6 +1466,32 @@ int platform_dma_configure(struct device *dev)
return ret;
}
+static int _platform_dma_configure(struct device *dev)
+{
+ struct platform_driver *drv = to_platform_driver(dev->driver);
+ int ret;
+
+ if (!drv->suppress_auto_claim_dma_owner) {
+ ret = iommu_device_set_dma_owner(dev, DMA_OWNER_DMA_API, NULL);
+ if (ret)
+ return ret;
+ }
+
+ ret = platform_dma_configure(dev);
+ if (ret && !drv->suppress_auto_claim_dma_owner)
+ iommu_device_release_dma_owner(dev, DMA_OWNER_DMA_API);
+
+ return ret;
+}
+
+static void _platform_dma_unconfigure(struct device *dev)
+{
+ struct platform_driver *drv = to_platform_driver(dev->driver);
+
+ if (!drv->suppress_auto_claim_dma_owner)
+ iommu_device_release_dma_owner(dev, DMA_OWNER_DMA_API);
+}
+
static const struct dev_pm_ops platform_dev_pm_ops = {
SET_RUNTIME_PM_OPS(pm_generic_runtime_suspend,
pm_generic_runtime_resume, NULL)
USE_PLATFORM_PM_SLEEP_OPS
@@ -1478,7 +1505,8 @@ struct bus_type platform_bus_type = {
.probe = platform_probe,
.remove = platform_remove,
.shutdown = platform_shutdown,
- .dma_configure = platform_dma_configure,
+ .dma_configure = _platform_dma_configure,
+ .dma_unconfigure = _platform_dma_unconfigure,
.pm = _dev_pm_ops,
};
EXPORT_SYMBOL_GPL(platform_bus_type);
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 03/17] PCI: Add driver dma ownership management
Multiple PCI devices may be placed in the same IOMMU group because they
cannot be isolated from each other. These devices must either be entirely
under kernel control or userspace control, never a mixture. This checks
and sets DMA ownership during driver binding, and vice versa, release the
ownership during driver unbinding.
Driver may set a new flag (suppress_auto_claim_dma_owner) to disable auto
claiming DMA_OWNER_DMA_API ownership in the binding process. For instance,
the userspace framework drivers (vfio etc.) which need to manually claim
DMA_OWNER_PRIVATE_DOMAIN_USER when assigning a device to userspace.
Signed-off-by: Lu Baolu
---
include/linux/pci.h | 5 +
drivers/pci/pci-driver.c | 21 +
2 files changed, 26 insertions(+)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 18a75c8e615c..1b29af0ab43b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -882,6 +882,10 @@ struct module;
* created once it is bound to the driver.
* @driver:Driver model structure.
* @dynids:List of dynamically added device IDs.
+ * @suppress_auto_claim_dma_owner: Disable auto claiming of kernel DMA owner.
+ * Drivers which don't require DMA or want to manually claim the
+ * owner type (e.g. userspace driver frameworks) could set this
+ * flag.
*/
struct pci_driver {
struct list_headnode;
@@ -900,6 +904,7 @@ struct pci_driver {
const struct attribute_group **dev_groups;
struct device_driverdriver;
struct pci_dynids dynids;
+ bool suppress_auto_claim_dma_owner;
};
static inline struct pci_driver *to_pci_driver(struct device_driver *drv)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 588588cfda48..cad299d79f1a 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include "pci.h"
#include "pcie/portdrv.h"
@@ -1590,9 +1591,16 @@ static int pci_bus_num_vf(struct device *dev)
*/
static int pci_dma_configure(struct device *dev)
{
+ struct pci_driver *driver = to_pci_driver(dev->driver);
struct device *bridge;
int ret = 0;
+ if (!driver->suppress_auto_claim_dma_owner) {
+ ret = iommu_device_set_dma_owner(dev, DMA_OWNER_DMA_API, NULL);
+ if (ret)
+ return ret;
+ }
+
bridge = pci_get_host_bridge_device(to_pci_dev(dev));
if (IS_ENABLED(CONFIG_OF) && bridge->parent &&
@@ -1605,9 +1613,21 @@ static int pci_dma_configure(struct device *dev)
}
pci_put_host_bridge_device(bridge);
+
+ if (ret && !driver->suppress_auto_claim_dma_owner)
+ iommu_device_release_dma_owner(dev, DMA_OWNER_DMA_API);
+
return ret;
}
+static void pci_dma_unconfigure(struct device *dev)
+{
+ struct pci_driver *driver = to_pci_driver(dev->driver);
+
+ if (!driver->suppress_auto_claim_dma_owner)
+ iommu_device_release_dma_owner(dev, DMA_OWNER_DMA_API);
+}
+
struct bus_type pci_bus_type = {
.name = "pci",
.match = pci_bus_match,
@@ -1621,6 +1641,7 @@ struct bus_type pci_bus_type = {
.pm = PCI_PM_OPS_PTR,
.num_vf = pci_bus_num_vf,
.dma_configure = pci_dma_configure,
+ .dma_unconfigure = pci_dma_unconfigure,
};
EXPORT_SYMBOL(pci_bus_type);
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 02/17] driver core: Add dma_unconfigure callback in bus_type
The bus_type structure defines dma_configure() callback for bus drivers
to configure DMA on the devices. This adds the paired dma_unconfigure()
callback and calls it during driver unbinding so that bus drivers can do
some cleanup work.
One use case for this paired DMA callbacks is for the bus driver to check
for DMA ownership conflicts during driver binding, where multiple devices
belonging to a same IOMMU group (the minimum granularity of isolation and
protection) may be assigned to kernel drivers or user space respectively.
Without this change, for example, the vfio driver has to listen to a bus
BOUND_DRIVER event and then BUG_ON() in case of dma ownership conflict.
This leads to bad user experience since careless driver binding operation
may crash the system if the admin overlooks the group restriction. Aside
from bad design, this leads to a security problem as a root user, even with
lockdown=integrity, can force the kernel to BUG.
With this change, the bus driver could check and set the DMA ownership in
driver binding process and fail on ownership conflicts. The DMA ownership
should be released during driver unbinding.
Suggested-by: Jason Gunthorpe
Link: https://lore.kernel.org/linux-iommu/20210922123931.gi327...@nvidia.com/
Link: https://lore.kernel.org/linux-iommu/20210928115751.gk964...@nvidia.com/
Signed-off-by: Lu Baolu
---
include/linux/device/bus.h | 3 +++
drivers/base/dd.c | 7 ++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
index a039ab809753..ef54a71e5f8f 100644
--- a/include/linux/device/bus.h
+++ b/include/linux/device/bus.h
@@ -59,6 +59,8 @@ struct fwnode_handle;
* bus supports.
* @dma_configure: Called to setup DMA configuration on a device on
* this bus.
+ * @dma_unconfigure: Called to cleanup DMA configuration on a device on
+ * this bus.
* @pm:Power management operations of this bus, callback the
specific
* device driver's pm-ops.
* @iommu_ops: IOMMU specific operations for this bus, used to attach IOMMU
@@ -103,6 +105,7 @@ struct bus_type {
int (*num_vf)(struct device *dev);
int (*dma_configure)(struct device *dev);
+ void (*dma_unconfigure)(struct device *dev);
const struct dev_pm_ops *pm;
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 68ea1f949daa..a37aafff5fde 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -577,7 +577,7 @@ static int really_probe(struct device *dev, struct
device_driver *drv)
if (dev->bus->dma_configure) {
ret = dev->bus->dma_configure(dev);
if (ret)
- goto probe_failed;
+ goto pinctrl_bind_failed;
}
ret = driver_sysfs_add(dev);
@@ -660,6 +660,8 @@ static int really_probe(struct device *dev, struct
device_driver *drv)
if (dev->bus)
blocking_notifier_call_chain(>bus->p->bus_notifier,
BUS_NOTIFY_DRIVER_NOT_BOUND, dev);
+ if (dev->bus->dma_unconfigure)
+ dev->bus->dma_unconfigure(dev);
pinctrl_bind_failed:
device_links_no_driver(dev);
devres_release_all(dev);
@@ -1204,6 +1206,9 @@ static void __device_release_driver(struct device *dev,
struct device *parent)
else if (drv->remove)
drv->remove(dev);
+ if (dev->bus->dma_unconfigure)
+ dev->bus->dma_unconfigure(dev);
+
device_links_driver_cleanup(dev);
devres_release_all(dev);
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 01/17] iommu: Add device dma ownership set/release interfaces
>From the perspective of who is initiating the device to do DMA, device
DMA could be divided into the following types:
DMA_OWNER_DMA_API: Device DMAs are initiated by a kernel driver
through the kernel DMA API.
DMA_OWNER_PRIVATE_DOMAIN: Device DMAs are initiated by a kernel
driver with its own PRIVATE domain.
DMA_OWNER_PRIVATE_DOMAIN_USER: Device DMAs are initiated by
userspace.
Different DMA ownerships are exclusive for all devices in the same iommu
group as an iommu group is the smallest granularity of device isolation
and protection that the IOMMU subsystem can guarantee. This extends the
iommu core to enforce this exclusion.
Basically two new interfaces are provided:
int iommu_device_set_dma_owner(struct device *dev,
enum iommu_dma_owner type, void *owner_cookie);
void iommu_device_release_dma_owner(struct device *dev,
enum iommu_dma_owner type);
Although above interfaces are per-device, DMA owner is tracked per group
under the hood. An iommu group cannot have different dma ownership set
at the same time. Violation of this assumption fails
iommu_device_set_dma_owner().
Kernel driver which does DMA have DMA_OWNER_DMA_API automatically set/
released in the driver binding/unbinding process (see next patch).
Kernel driver which doesn't do DMA could avoid setting the owner type.
Device bound to such driver is considered same as a driver-less device
which is compatible to all owner types.
Userspace driver framework (e.g. vfio) should set
DMA_OWNER_PRIVATE_DOMAIN_USER for a device before the userspace is allowed
to access it, plus a owner cookie pointer to mark the user identity so a
single group cannot be operated by multiple users simultaneously. Vice
versa, the owner type should be released after the user access permission
is withdrawn.
Signed-off-by: Jason Gunthorpe
Signed-off-by: Kevin Tian
Signed-off-by: Lu Baolu
---
include/linux/iommu.h | 36 +
drivers/iommu/iommu.c | 93 +++
2 files changed, 129 insertions(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index d2f3435e7d17..24676b498f38 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -162,6 +162,23 @@ enum iommu_dev_features {
IOMMU_DEV_FEAT_IOPF,
};
+/**
+ * enum iommu_dma_owner - IOMMU DMA ownership
+ * @DMA_OWNER_NONE: No DMA ownership.
+ * @DMA_OWNER_DMA_API: Device DMAs are initiated by a kernel driver through
+ * the kernel DMA API.
+ * @DMA_OWNER_PRIVATE_DOMAIN: Device DMAs are initiated by a kernel driver
+ * which provides an UNMANAGED domain.
+ * @DMA_OWNER_PRIVATE_DOMAIN_USER: Device DMAs are initiated by userspace,
+ * kernel ensures that DMAs never go to kernel memory.
+ */
+enum iommu_dma_owner {
+ DMA_OWNER_NONE,
+ DMA_OWNER_DMA_API,
+ DMA_OWNER_PRIVATE_DOMAIN,
+ DMA_OWNER_PRIVATE_DOMAIN_USER,
+};
+
#define IOMMU_PASID_INVALID(-1U)
#ifdef CONFIG_IOMMU_API
@@ -681,6 +698,10 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev,
void iommu_sva_unbind_device(struct iommu_sva *handle);
u32 iommu_sva_get_pasid(struct iommu_sva *handle);
+int iommu_device_set_dma_owner(struct device *dev, enum iommu_dma_owner owner,
+ void *owner_cookie);
+void iommu_device_release_dma_owner(struct device *dev, enum iommu_dma_owner
owner);
+
#else /* CONFIG_IOMMU_API */
struct iommu_ops {};
@@ -1081,6 +1102,21 @@ static inline struct iommu_fwspec
*dev_iommu_fwspec_get(struct device *dev)
{
return NULL;
}
+
+static inline int iommu_device_set_dma_owner(struct device *dev,
+enum iommu_dma_owner owner,
+void *owner_cookie)
+{
+ if (owner != DMA_OWNER_DMA_API)
+ return -EINVAL;
+
+ return 0;
+}
+
+static inline void iommu_device_release_dma_owner(struct device *dev,
+ enum iommu_dma_owner owner)
+{
+}
#endif /* CONFIG_IOMMU_API */
/**
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 8b86406b7162..1de520a07518 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -48,6 +48,9 @@ struct iommu_group {
struct iommu_domain *default_domain;
struct iommu_domain *domain;
struct list_head entry;
+ enum iommu_dma_owner dma_owner;
+ refcount_t owner_cnt;
+ void *owner_cookie;
};
struct group_device {
@@ -621,6 +624,7 @@ struct iommu_group *iommu_group_alloc(void)
INIT_LIST_HEAD(>devices);
INIT_LIST_HEAD(>entry);
BLOCKING_INIT_NOTIFIER_HEAD(>notifier);
+ group->dma_owner = DMA_OWNER_NONE;
ret = ida_simple_get(_group_ida, 0, 0, GFP_KERNEL);
if (ret < 0) {
@@ -3351,3 +3355,92 @@ static ssize_t
[PATCH v2 00/17] Fix BUG_ON in vfio_iommu_group_notifier()
The original post and intent of this series is here. https://lore.kernel.org/linux-iommu/2025020552.2378167-1-baolu...@linux.intel.com/ Change log: v1: initial post - https://lore.kernel.org/linux-iommu/2025020552.2378167-1-baolu...@linux.intel.com/ v2: - Move kernel dma ownership auto-claiming from driver core to bus callback. [Greg/Christoph/Robin/Jason] https://lore.kernel.org/linux-iommu/2025020552.2378167-1-baolu...@linux.intel.com/T/#m153706912b770682cb12e3c28f57e171aa1f9d0c - Code and interface refactoring for iommu_set/release_dma_owner() interfaces. [Jason] https://lore.kernel.org/linux-iommu/2025020552.2378167-1-baolu...@linux.intel.com/T/#mea70ed8e4e3665aedf32a5a0a7db095bf680325e - [NEW]Add new iommu_attach/detach_device_shared() interfaces for multiple devices group. [Robin/Jason] https://lore.kernel.org/linux-iommu/2025020552.2378167-1-baolu...@linux.intel.com/T/#mea70ed8e4e3665aedf32a5a0a7db095bf680325e - [NEW]Use iommu_attach/detach_device_shared() in drm/tegra drivers. - Refactoring and description refinement. This is based on v5.16-rc2 and available on github: https://github.com/LuBaolu/intel-iommu/commits/iommu-dma-ownership-v2 Best regards, baolu Jason Gunthorpe (2): vfio: Delete the unbound_list drm/tegra: Use the iommu dma_owner mechanism Lu Baolu (15): iommu: Add device dma ownership set/release interfaces driver core: Add dma_unconfigure callback in bus_type PCI: Add driver dma ownership management driver core: platform: Add driver dma ownership management amba: Add driver dma ownership management bus: fsl-mc: Add driver dma ownership management PCI: pci_stub: Suppress kernel DMA ownership auto-claiming PCI: portdrv: Suppress kernel DMA ownership auto-claiming iommu: Add security context management for assigned devices iommu: Expose group variants of dma ownership interfaces iommu: Add iommu_at[de]tach_device_shared() for multi-device groups vfio: Set DMA USER ownership for VFIO devices vfio: Remove use of vfio_group_viable() vfio: Remove iommu group notifier iommu: Remove iommu group changes notifier include/linux/amba/bus.h | 1 + include/linux/device/bus.h| 3 + include/linux/fsl/mc.h| 5 + include/linux/iommu.h | 93 ++-- include/linux/pci.h | 5 + include/linux/platform_device.h | 1 + drivers/amba/bus.c| 30 ++- drivers/base/dd.c | 7 +- drivers/base/platform.c | 30 ++- drivers/bus/fsl-mc/fsl-mc-bus.c | 26 +- drivers/gpu/drm/tegra/dc.c| 1 + drivers/gpu/drm/tegra/drm.c | 55 ++--- drivers/gpu/drm/tegra/gr2d.c | 1 + drivers/gpu/drm/tegra/gr3d.c | 1 + drivers/gpu/drm/tegra/vic.c | 1 + drivers/iommu/iommu.c | 329 -- drivers/pci/pci-driver.c | 21 ++ drivers/pci/pci-stub.c| 1 + drivers/pci/pcie/portdrv_pci.c| 2 + drivers/vfio/fsl-mc/vfio_fsl_mc.c | 1 + drivers/vfio/pci/vfio_pci.c | 1 + drivers/vfio/platform/vfio_amba.c | 1 + drivers/vfio/platform/vfio_platform.c | 1 + drivers/vfio/vfio.c | 248 ++- 24 files changed, 502 insertions(+), 363 deletions(-) -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] mm: kmemleak: Ignore kmemleak scanning on CMA regions
On Sat, Nov 27, 2021 at 04:07:18PM -0800, Andrew Morton wrote:
>On Fri, 26 Nov 2021 10:47:11 +0800 Calvin Zhang
>wrote:
>
>> Just like this:
>> commit 620951e27457 ("mm/cma: make kmemleak ignore CMA regions").
>>
>> Add kmemleak_ignore_phys() for CMA created from of reserved node.
>
>Could we please have a full, standalone changelog for this patch?
Yes, I will add.
>
>The 620951e27457 changelog says "Without this, the kernel crashes...".
>Does your patch also fix a crash? If so under what circumstances and
>should we backport this fix into -stable kernels?
No crash occurred. 620951e27457 avoids crashes caused by accessing
highmem and it was fixed later. Now kmemleak_alloc_phys() and
kmemleak_ignore_phys() skip highmem. This patch is based on the
point that CMA regions don't contain pointers to other kmemleak
objects, and ignores CMA regions from reserved memory as what
620951e27457 did.
--
Thanks
Calvin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch 00/37] genirq/msi, PCI/MSI: Spring cleaning - Part 2
On Sat, Nov 27, 2021 at 02:21:17AM +0100, Thomas Gleixner wrote: > This is the second part of [PCI]MSI refactoring which aims to provide the > ability of expanding MSI-X vectors after enabling MSI-X. > > The first part of this work can be found here: > > https://lore.kernel.org/r/20211126222700.862407...@linutronix.de > > This second part has the following important changes: > >1) Cleanup of the MSI related data in struct device > > struct device contains at the moment various MSI related parts. Some > of them (the irq domain pointer) cannot be moved out, but the rest > can be allocated on first use. This is in preparation of adding more > per device MSI data later on. > >2) Consolidation of sysfs handling > > As a first step this moves the sysfs pointer from struct msi_desc > into the new per device MSI data structure where it belongs. > > Later changes will cleanup this code further, but that's not possible > at this point. > >3) Store per device properties in the per device MSI data to avoid > looking up MSI descriptors and analysing their data. Cleanup all > related use cases. > >4) Provide a function to retrieve the Linux interrupt number for a given > MSI index similar to pci_irq_vector() and cleanup all open coded > variants. The msi_get_virq() sure does make a big difference.. Though it does highlight there is some asymmetry with how platform and PCI works here where PCI fills some 'struct msix_entry *'. Many drivers would be quite happy to just call msi_get_virq() and avoid the extra memory, so I think the msi_get_virq() version is good. Reviewed-by: Jason Gunthorpe Thanks, Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] mm: kmemleak: Ignore kmemleak scanning on CMA regions
On Fri, 26 Nov 2021 10:47:11 +0800 Calvin Zhang
wrote:
> Just like this:
> commit 620951e27457 ("mm/cma: make kmemleak ignore CMA regions").
>
> Add kmemleak_ignore_phys() for CMA created from of reserved node.
Could we please have a full, standalone changelog for this patch?
The 620951e27457 changelog says "Without this, the kernel crashes...".
Does your patch also fix a crash? If so under what circumstances and
should we backport this fix into -stable kernels?
Etcetera.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch 02/37] device: Add device::msi_data pointer and struct msi_device_data
On Sat, Nov 27, 2021 at 02:20:09AM +0100, Thomas Gleixner wrote:
> +/**
> + * msi_setup_device_data - Setup MSI device data
> + * @dev: Device for which MSI device data should be set up
> + *
> + * Return: 0 on success, appropriate error code otherwise
> + *
> + * This can be called more than once for @dev. If the MSI device data is
> + * already allocated the call succeeds. The allocated memory is
> + * automatically released when the device is destroyed.
I would say 'by devres when the driver is removed' rather than device
is destroyed - to me the latter implies it would happen at device_del
or perhaps during release..
> +int msi_setup_device_data(struct device *dev)
> +{
> + struct msi_device_data *md;
> +
> + if (dev->msi.data)
> + return 0;
> +
> + md = devres_alloc(msi_device_data_release, sizeof(*md), GFP_KERNEL);
> + if (!md)
> + return -ENOMEM;
> +
> + raw_spin_lock_init(>lock);
I also couldn't guess why this needed to be raw?
Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 4/5] iommu/virtio: Pass end address to viommu_add_mapping()
On Sat, Nov 27, 2021 at 06:09:40PM +0100, Eric Auger wrote:
>
>
> On 11/23/21 4:53 PM, Jean-Philippe Brucker wrote:
> > To support identity mappings, the virtio-iommu driver must be able to
> > represent full 64-bit ranges internally. Pass (start, end) instead of
> > (start, size) to viommu_add/del_mapping().
> >
> > Clean comments. The one about the returned size was never true: when
> > sweeping the whole address space the returned size will most certainly
> > be smaller than 2^64.
> >
> > Reviewed-by: Kevin Tian
> > Signed-off-by: Jean-Philippe Brucker
> Reviewed-by: Eric Auger
>
> Eric
>
> > ---
> > drivers/iommu/virtio-iommu.c | 31 +++
> > 1 file changed, 15 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> > index d63ec4d11b00..eceb9281c8c1 100644
> > --- a/drivers/iommu/virtio-iommu.c
> > +++ b/drivers/iommu/virtio-iommu.c
> > @@ -311,8 +311,8 @@ static int viommu_send_req_sync(struct viommu_dev
> > *viommu, void *buf,
> > *
> > * On success, return the new mapping. Otherwise return NULL.
> > */
> > -static int viommu_add_mapping(struct viommu_domain *vdomain, unsigned long
> > iova,
> > - phys_addr_t paddr, size_t size, u32 flags)
> > +static int viommu_add_mapping(struct viommu_domain *vdomain, u64 iova, u64
> > end,
> > + phys_addr_t paddr, u32 flags)
> > {
> > unsigned long irqflags;
> > struct viommu_mapping *mapping;
I am worried that API changes like that will cause subtle
bugs since types of arguments change but not their
number. If we forgot to update some callers it will all be messed up.
How about passing struct Range instead?
> > @@ -323,7 +323,7 @@ static int viommu_add_mapping(struct viommu_domain
> > *vdomain, unsigned long iova,
> >
> > mapping->paddr = paddr;
> > mapping->iova.start = iova;
> > - mapping->iova.last = iova + size - 1;
> > + mapping->iova.last = end;
> > mapping->flags = flags;
> >
> > spin_lock_irqsave(>mappings_lock, irqflags);
> > @@ -338,26 +338,24 @@ static int viommu_add_mapping(struct viommu_domain
> > *vdomain, unsigned long iova,
> > *
> > * @vdomain: the domain
> > * @iova: start of the range
> > - * @size: size of the range. A size of 0 corresponds to the entire address
> > - * space.
> > + * @end: end of the range
> > *
> > - * On success, returns the number of unmapped bytes (>= size)
> > + * On success, returns the number of unmapped bytes
> > */
> > static size_t viommu_del_mappings(struct viommu_domain *vdomain,
> > - unsigned long iova, size_t size)
> > + u64 iova, u64 end)
> > {
> > size_t unmapped = 0;
> > unsigned long flags;
> > - unsigned long last = iova + size - 1;
> > struct viommu_mapping *mapping = NULL;
> > struct interval_tree_node *node, *next;
> >
> > spin_lock_irqsave(>mappings_lock, flags);
> > - next = interval_tree_iter_first(>mappings, iova, last);
> > + next = interval_tree_iter_first(>mappings, iova, end);
> > while (next) {
> > node = next;
> > mapping = container_of(node, struct viommu_mapping, iova);
> > - next = interval_tree_iter_next(node, iova, last);
> > + next = interval_tree_iter_next(node, iova, end);
> >
> > /* Trying to split a mapping? */
> > if (mapping->iova.start < iova)
> > @@ -656,8 +654,8 @@ static void viommu_domain_free(struct iommu_domain
> > *domain)
> > {
> > struct viommu_domain *vdomain = to_viommu_domain(domain);
> >
> > - /* Free all remaining mappings (size 2^64) */
> > - viommu_del_mappings(vdomain, 0, 0);
> > + /* Free all remaining mappings */
> > + viommu_del_mappings(vdomain, 0, ULLONG_MAX);
> >
> > if (vdomain->viommu)
> > ida_free(>viommu->domain_ids, vdomain->id);
> > @@ -742,6 +740,7 @@ static int viommu_map(struct iommu_domain *domain,
> > unsigned long iova,
> > {
> > int ret;
> > u32 flags;
> > + u64 end = iova + size - 1;
> > struct virtio_iommu_req_map map;
> > struct viommu_domain *vdomain = to_viommu_domain(domain);
> >
> > @@ -752,7 +751,7 @@ static int viommu_map(struct iommu_domain *domain,
> > unsigned long iova,
> > if (flags & ~vdomain->map_flags)
> > return -EINVAL;
> >
> > - ret = viommu_add_mapping(vdomain, iova, paddr, size, flags);
> > + ret = viommu_add_mapping(vdomain, iova, end, paddr, flags);
> > if (ret)
> > return ret;
> >
> > @@ -761,7 +760,7 @@ static int viommu_map(struct iommu_domain *domain,
> > unsigned long iova,
> > .domain = cpu_to_le32(vdomain->id),
> > .virt_start = cpu_to_le64(iova),
> > .phys_start = cpu_to_le64(paddr),
> > - .virt_end = cpu_to_le64(iova + size - 1),
> > + .virt_end =
Re: [PATCH v2 5/5] iommu/virtio: Support identity-mapped domains
Hi Jean,
On 11/23/21 4:53 PM, Jean-Philippe Brucker wrote:
> Support identity domains for devices that do not offer the
> VIRTIO_IOMMU_F_BYPASS_CONFIG feature, by creating 1:1 mappings between
> the virtual and physical address space. Identity domains created this
> way still perform noticeably better than DMA domains, because they don't
> have the overhead of setting up and tearing down mappings at runtime.
> The performance difference between this and bypass is minimal in
> comparison.
>
> It does not matter that the physical addresses in the identity mappings
> do not all correspond to memory. By enabling passthrough we are trusting
> the device driver and the device itself to only perform DMA to suitable
> locations. In some cases it may even be desirable to perform DMA to MMIO
> regions.
>
> Reviewed-by: Kevin Tian
> Signed-off-by: Jean-Philippe Brucker
> ---
> drivers/iommu/virtio-iommu.c | 63 +---
> 1 file changed, 58 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index eceb9281c8c1..6a8a52b4297b 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -375,6 +375,55 @@ static size_t viommu_del_mappings(struct viommu_domain
> *vdomain,
> return unmapped;
> }
>
> +/*
> + * Fill the domain with identity mappings, skipping the device's reserved
> + * regions.
> + */
> +static int viommu_domain_map_identity(struct viommu_endpoint *vdev,
> + struct viommu_domain *vdomain)
> +{
> + int ret;
> + struct iommu_resv_region *resv;
> + u64 iova = vdomain->domain.geometry.aperture_start;
> + u64 limit = vdomain->domain.geometry.aperture_end;
> + u32 flags = VIRTIO_IOMMU_MAP_F_READ | VIRTIO_IOMMU_MAP_F_WRITE;
> + unsigned long granule = 1UL << __ffs(vdomain->domain.pgsize_bitmap);
> +
> + iova = ALIGN(iova, granule);
> + limit = ALIGN_DOWN(limit + 1, granule) - 1;
> +
> + list_for_each_entry(resv, >resv_regions, list) {
> + u64 resv_start = ALIGN_DOWN(resv->start, granule);
> + u64 resv_end = ALIGN(resv->start + resv->length, granule) - 1;
> +
> + if (resv_end < iova || resv_start > limit)
> + /* No overlap */
> + continue;
> +
> + if (resv_start > iova) {
> + ret = viommu_add_mapping(vdomain, iova, resv_start - 1,
> + (phys_addr_t)iova, flags);
> + if (ret)
> + goto err_unmap;
> + }
> +
> + if (resv_end >= limit)
> + return 0;
> +
> + iova = resv_end + 1;
> + }
> +
> + ret = viommu_add_mapping(vdomain, iova, limit, (phys_addr_t)iova,
> + flags);
> + if (ret)
> + goto err_unmap;
> + return 0;
> +
> +err_unmap:
> + viommu_del_mappings(vdomain, 0, iova);
> + return ret;
> +}
> +
> /*
> * viommu_replay_mappings - re-send MAP requests
> *
> @@ -637,14 +686,18 @@ static int viommu_domain_finalise(struct
> viommu_endpoint *vdev,
> vdomain->viommu = viommu;
>
> if (domain->type == IOMMU_DOMAIN_IDENTITY) {
> - if (!virtio_has_feature(viommu->vdev,
> - VIRTIO_IOMMU_F_BYPASS_CONFIG)) {
> + if (virtio_has_feature(viommu->vdev,
> +VIRTIO_IOMMU_F_BYPASS_CONFIG)) {
> + vdomain->bypass = true;
> + return 0;
> + }
> +
> + ret = viommu_domain_map_identity(vdev, vdomain);
> + if (ret) {
> ida_free(>domain_ids, vdomain->id);
> - vdomain->viommu = 0;
> + vdomain->viommu = NULL;
nit: that change could have been done in patch 2
> return -EOPNOTSUPP;
> }
> -
> - vdomain->bypass = true;
> }
>
> return 0;
Besides
Reviewed-by: Eric Auger
Eric
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/5] iommu/virtio: Sort reserved regions
Hi Jean,
On 11/23/21 4:53 PM, Jean-Philippe Brucker wrote:
> To ease identity mapping support, keep the list of reserved regions
> sorted.
>
> Reviewed-by: Kevin Tian
> Signed-off-by: Jean-Philippe Brucker
Reviewed-by: Eric Auger
Eric
> ---
> drivers/iommu/virtio-iommu.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index ee8a7afd667b..d63ec4d11b00 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -423,7 +423,7 @@ static int viommu_add_resv_mem(struct viommu_endpoint
> *vdev,
> size_t size;
> u64 start64, end64;
> phys_addr_t start, end;
> - struct iommu_resv_region *region = NULL;
> + struct iommu_resv_region *region = NULL, *next;
> unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
>
> start = start64 = le64_to_cpu(mem->start);
> @@ -454,7 +454,12 @@ static int viommu_add_resv_mem(struct viommu_endpoint
> *vdev,
> if (!region)
> return -ENOMEM;
>
> - list_add(>list, >resv_regions);
> + /* Keep the list sorted */
> + list_for_each_entry(next, >resv_regions, list) {
> + if (next->start > region->start)
> + break;
> + }
> + list_add_tail(>list, >list);
> return 0;
> }
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 4/5] iommu/virtio: Pass end address to viommu_add_mapping()
On 11/23/21 4:53 PM, Jean-Philippe Brucker wrote:
> To support identity mappings, the virtio-iommu driver must be able to
> represent full 64-bit ranges internally. Pass (start, end) instead of
> (start, size) to viommu_add/del_mapping().
>
> Clean comments. The one about the returned size was never true: when
> sweeping the whole address space the returned size will most certainly
> be smaller than 2^64.
>
> Reviewed-by: Kevin Tian
> Signed-off-by: Jean-Philippe Brucker
Reviewed-by: Eric Auger
Eric
> ---
> drivers/iommu/virtio-iommu.c | 31 +++
> 1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index d63ec4d11b00..eceb9281c8c1 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -311,8 +311,8 @@ static int viommu_send_req_sync(struct viommu_dev
> *viommu, void *buf,
> *
> * On success, return the new mapping. Otherwise return NULL.
> */
> -static int viommu_add_mapping(struct viommu_domain *vdomain, unsigned long
> iova,
> - phys_addr_t paddr, size_t size, u32 flags)
> +static int viommu_add_mapping(struct viommu_domain *vdomain, u64 iova, u64
> end,
> + phys_addr_t paddr, u32 flags)
> {
> unsigned long irqflags;
> struct viommu_mapping *mapping;
> @@ -323,7 +323,7 @@ static int viommu_add_mapping(struct viommu_domain
> *vdomain, unsigned long iova,
>
> mapping->paddr = paddr;
> mapping->iova.start = iova;
> - mapping->iova.last = iova + size - 1;
> + mapping->iova.last = end;
> mapping->flags = flags;
>
> spin_lock_irqsave(>mappings_lock, irqflags);
> @@ -338,26 +338,24 @@ static int viommu_add_mapping(struct viommu_domain
> *vdomain, unsigned long iova,
> *
> * @vdomain: the domain
> * @iova: start of the range
> - * @size: size of the range. A size of 0 corresponds to the entire address
> - * space.
> + * @end: end of the range
> *
> - * On success, returns the number of unmapped bytes (>= size)
> + * On success, returns the number of unmapped bytes
> */
> static size_t viommu_del_mappings(struct viommu_domain *vdomain,
> - unsigned long iova, size_t size)
> + u64 iova, u64 end)
> {
> size_t unmapped = 0;
> unsigned long flags;
> - unsigned long last = iova + size - 1;
> struct viommu_mapping *mapping = NULL;
> struct interval_tree_node *node, *next;
>
> spin_lock_irqsave(>mappings_lock, flags);
> - next = interval_tree_iter_first(>mappings, iova, last);
> + next = interval_tree_iter_first(>mappings, iova, end);
> while (next) {
> node = next;
> mapping = container_of(node, struct viommu_mapping, iova);
> - next = interval_tree_iter_next(node, iova, last);
> + next = interval_tree_iter_next(node, iova, end);
>
> /* Trying to split a mapping? */
> if (mapping->iova.start < iova)
> @@ -656,8 +654,8 @@ static void viommu_domain_free(struct iommu_domain
> *domain)
> {
> struct viommu_domain *vdomain = to_viommu_domain(domain);
>
> - /* Free all remaining mappings (size 2^64) */
> - viommu_del_mappings(vdomain, 0, 0);
> + /* Free all remaining mappings */
> + viommu_del_mappings(vdomain, 0, ULLONG_MAX);
>
> if (vdomain->viommu)
> ida_free(>viommu->domain_ids, vdomain->id);
> @@ -742,6 +740,7 @@ static int viommu_map(struct iommu_domain *domain,
> unsigned long iova,
> {
> int ret;
> u32 flags;
> + u64 end = iova + size - 1;
> struct virtio_iommu_req_map map;
> struct viommu_domain *vdomain = to_viommu_domain(domain);
>
> @@ -752,7 +751,7 @@ static int viommu_map(struct iommu_domain *domain,
> unsigned long iova,
> if (flags & ~vdomain->map_flags)
> return -EINVAL;
>
> - ret = viommu_add_mapping(vdomain, iova, paddr, size, flags);
> + ret = viommu_add_mapping(vdomain, iova, end, paddr, flags);
> if (ret)
> return ret;
>
> @@ -761,7 +760,7 @@ static int viommu_map(struct iommu_domain *domain,
> unsigned long iova,
> .domain = cpu_to_le32(vdomain->id),
> .virt_start = cpu_to_le64(iova),
> .phys_start = cpu_to_le64(paddr),
> - .virt_end = cpu_to_le64(iova + size - 1),
> + .virt_end = cpu_to_le64(end),
> .flags = cpu_to_le32(flags),
> };
>
> @@ -770,7 +769,7 @@ static int viommu_map(struct iommu_domain *domain,
> unsigned long iova,
>
> ret = viommu_send_req_sync(vdomain->viommu, , sizeof(map));
> if (ret)
> - viommu_del_mappings(vdomain, iova, size);
> + viommu_del_mappings(vdomain, iova, end);
>
> return ret;
> }
> @@
Re: [PATCH v2 2/5] iommu/virtio: Support bypass domains
Hi Jean,
On 11/23/21 4:52 PM, Jean-Philippe Brucker wrote:
> The VIRTIO_IOMMU_F_BYPASS_CONFIG feature adds a new flag to the ATTACH
> request, that creates a bypass domain. Use it to enable identity
> domains.
>
> When VIRTIO_IOMMU_F_BYPASS_CONFIG is not supported by the device, we
> currently fail attaching to an identity domain. Future patches will
> instead create identity mappings in this case.
>
> Reviewed-by: Kevin Tian
> Signed-off-by: Jean-Philippe Brucker
> ---
> drivers/iommu/virtio-iommu.c | 20 +++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index 80930ce04a16..ee8a7afd667b 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -71,6 +71,7 @@ struct viommu_domain {
> struct rb_root_cached mappings;
>
> unsigned long nr_endpoints;
> + boolbypass;
> };
>
> struct viommu_endpoint {
> @@ -587,7 +588,9 @@ static struct iommu_domain *viommu_domain_alloc(unsigned
> type)
> {
> struct viommu_domain *vdomain;
>
> - if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
> + if (type != IOMMU_DOMAIN_UNMANAGED &&
> + type != IOMMU_DOMAIN_DMA &&
> + type != IOMMU_DOMAIN_IDENTITY)
> return NULL;
>
> vdomain = kzalloc(sizeof(*vdomain), GFP_KERNEL);
> @@ -630,6 +633,17 @@ static int viommu_domain_finalise(struct viommu_endpoint
> *vdev,
> vdomain->map_flags = viommu->map_flags;
> vdomain->viommu = viommu;
>
> + if (domain->type == IOMMU_DOMAIN_IDENTITY) {
> + if (!virtio_has_feature(viommu->vdev,
nit: couldn't the check be done before the ida_alloc_range(),
simplifying the failure cleanup?
Thanks
Eric
> + VIRTIO_IOMMU_F_BYPASS_CONFIG)) {
> + ida_free(>domain_ids, vdomain->id);
> + vdomain->viommu = 0;
> + return -EOPNOTSUPP;
> + }
> +
> + vdomain->bypass = true;
> + }
> +
> return 0;
> }
>
> @@ -691,6 +705,9 @@ static int viommu_attach_dev(struct iommu_domain *domain,
> struct device *dev)
> .domain = cpu_to_le32(vdomain->id),
> };
>
> + if (vdomain->bypass)
> + req.flags |= cpu_to_le32(VIRTIO_IOMMU_ATTACH_F_BYPASS);
> +
> for (i = 0; i < fwspec->num_ids; i++) {
> req.endpoint = cpu_to_le32(fwspec->ids[i]);
>
> @@ -1132,6 +1149,7 @@ static unsigned int features[] = {
> VIRTIO_IOMMU_F_DOMAIN_RANGE,
> VIRTIO_IOMMU_F_PROBE,
> VIRTIO_IOMMU_F_MMIO,
> + VIRTIO_IOMMU_F_BYPASS_CONFIG,
> };
>
> static struct virtio_device_id id_table[] = {
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch 04/37] PCI/MSI: Use lock from msi_device_data
On Sat, Nov 27, 2021 at 02:20:13AM +0100, Thomas Gleixner wrote: > Remove the register lock from struct device and use the one in > struct msi_device_data. > > Signed-off-by: Thomas Gleixner > --- > drivers/base/core.c|1 - > drivers/pci/msi/msi.c |2 +- > include/linux/device.h |2 -- > 3 files changed, 1 insertion(+), 4 deletions(-) Reviewed-by: Greg Kroah-Hartman ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch 01/37] device: Move MSI related data into a struct
On Sat, Nov 27, 2021 at 02:20:08AM +0100, Thomas Gleixner wrote: > The only unconditional part of MSI data in struct device is the irqdomain > pointer. Everything else can be allocated on demand. Create a data > structure and move the irqdomain pointer into it. The other MSI specific > parts are going to be removed from struct device in later steps. > > Signed-off-by: Thomas Gleixner > Cc: Greg Kroah-Hartman > Cc: Will Deacon > Cc: Santosh Shilimkar > Cc: iommu@lists.linux-foundation.org > Cc: dmaeng...@vger.kernel.org > --- > drivers/base/platform-msi.c | 12 ++-- > drivers/dma/ti/k3-udma.c|4 ++-- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |2 +- > drivers/irqchip/irq-mvebu-icu.c |6 +++--- > drivers/soc/ti/k3-ringacc.c |4 ++-- > drivers/soc/ti/ti_sci_inta_msi.c|2 +- > include/linux/device.h | 19 +-- > 7 files changed, 28 insertions(+), 21 deletions(-) Reviewed-by: Greg Kroah-Hartman ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch 00/37] genirq/msi, PCI/MSI: Spring cleaning - Part 2
On Sat, Nov 27, 2021 at 02:20:06AM +0100, Thomas Gleixner wrote: > This is the second part of [PCI]MSI refactoring which aims to provide the > ability of expanding MSI-X vectors after enabling MSI-X. > > The first part of this work can be found here: > > https://lore.kernel.org/r/20211126222700.862407...@linutronix.de > > This second part has the following important changes: > >1) Cleanup of the MSI related data in struct device > > struct device contains at the moment various MSI related parts. Some > of them (the irq domain pointer) cannot be moved out, but the rest > can be allocated on first use. This is in preparation of adding more > per device MSI data later on. > >2) Consolidation of sysfs handling > > As a first step this moves the sysfs pointer from struct msi_desc > into the new per device MSI data structure where it belongs. > > Later changes will cleanup this code further, but that's not possible > at this point. > >3) Store per device properties in the per device MSI data to avoid > looking up MSI descriptors and analysing their data. Cleanup all > related use cases. > >4) Provide a function to retrieve the Linux interrupt number for a given > MSI index similar to pci_irq_vector() and cleanup all open coded > variants. > > This second series is based on: > > git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git > msi-v1-part-1 > > and also available from git: > > git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git > msi-v1-part-2 > Instead of responding to each individual patch, I've read them all, thanks for the cleanups, look good to me: Reviewed-by: Greg Kroah-Hartman ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch 02/37] device: Add device::msi_data pointer and struct msi_device_data
On Sat, Nov 27, 2021 at 02:20:09AM +0100, Thomas Gleixner wrote: > Create struct msi_device_data and add a pointer of that type to struct > dev_msi_info, which is part of struct device. Provide an allocator function > which can be invoked from the MSI interrupt allocation code pathes. > > Signed-off-by: Thomas Gleixner > --- > include/linux/device.h |5 + > include/linux/msi.h| 12 +++- > kernel/irq/msi.c | 33 + > 3 files changed, 49 insertions(+), 1 deletion(-) Reviewed-by: Greg Kroah-Hartman ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 12/33] iommu/mediatek: Always tlb_flush_all when each PM resume
On 10.11.21 09:50, Yong Wu wrote:
On Wed, 2021-11-10 at 07:29 +0200, Dafna Hirschfeld wrote:
On 10.11.21 04:20, Yong Wu wrote:
On Tue, 2021-11-09 at 14:21 +0200, Dafna Hirschfeld wrote:
Hi
This patch is needed in order to update the tlb when a device is
powered on.
Could you send this patch alone without the whole series so it
get
accepted easier?
Which SoC are you testing on? In previous SoC, the IOMMU HW don't
have
power-domain, and we have a "has_pm"[1] in the tlb function for
that
case. The "has_pm" should be always 0 for the previous SoC like
mt8173,
it should always tlb synchronize.
thus, Could you help share more about your issue? In which case it
lack
the necessary tlb operation. At least, We need confirm if it needs
a
"Fixes" tags if sending this patch alone.
Hi,
I work with the mtk-vcodec driver on mt8173. As you wrote, the iommu
doesn't
have a power-domain and so when allocating buffers before the device
is powered
on, there is the warning
"Partial TLB flush timed out, falling back to full flush"
flooding the log buf.
oh. Thanks very much for your information. Get it now.
This issue should be introduced by the:
b34ea31fe013 ("iommu/mediatek: Always enable the clk on resume")
Hi, reverting this commit didn't solve those warnings,
I think this is because in the function mtk_iommu_attach_device
the first call to pm_runtime_resume_and_get does not turn the clks on
since m4u_dom is not yet initialize. And then mtk_iommu_attach_device
calls pm_runtime_put right after mtk_iommu_hw_init is called
(where the clks are turned on)
thanks,
Dafna
tlb failed due to the bclk is not enabled. Could you help try that
after reverting this?
Sebastian Reichel suggested to remove the 'if(has_pm)' check to avoid
this warning,
and avoid flushing the tlb if the device is off:
[1] http://ix.io/3Eyr
This fixes the warning, but then the tlb is not flushed in sync,
Therefore the tlb should be flushed when the device is resumed.
So the two patches (the one suggested in the link [1] and this patch)
should be sent together as a 2-patch series.
then this is reasonable. You could help this into a new patchset if you
are free(add Fixes tag).
Thanks.
Thanks,
Dafna
Thanks.
[1]
https://elixir.bootlin.com/linux/v5.15/source/drivers/iommu/mtk_iommu.c#L236
I can resend the patch on your behalf if you want.
Thanks,
Dafna
On 23.09.21 14:58, Yong Wu wrote:
Prepare for 2 HWs that sharing pgtable in different power-
domains.
When there are 2 M4U HWs, it may has problem in the flush_range
in
which
we get the pm_status via the m4u dev, BUT that function don't
reflect the
real power-domain status of the HW since there may be other HW
also
use
that power-domain.
The function dma_alloc_attrs help allocate the iommu buffer
which
need the corresponding power domain since tlb flush is needed
when
preparing iova. BUT this function only is for allocating
buffer,
we have no good reason to request the user always call
pm_runtime_get
before calling dma_alloc_xxx. Therefore, we add a tlb_flush_all
in the pm_runtime_resume to make sure the tlb always is clean.
Another solution is always call pm_runtime_get in the
tlb_flush_range.
This will trigger pm runtime resume/backup so often when the
iommu
power is not active at some time(means user don't call
pm_runtime_get
before calling dma_alloc_xxx), This may cause the performance
drop.
thus we don't use this.
In other case, the iommu's power should always be active via
device
link with smi.
The previous SoC don't have PM except mt8192. the mt8192 IOMMU
is
display's
power-domain which nearly always is enabled. thus no need fix
tags
here.
Prepare for mt8195.
Signed-off-by: Yong Wu
---
drivers/iommu/mtk_iommu.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/iommu/mtk_iommu.c
b/drivers/iommu/mtk_iommu.c
index 44cf5547d084..e9e94944ed91 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -984,6 +984,17 @@ static int __maybe_unused
mtk_iommu_runtime_resume(struct device *dev)
return ret;
}
+ /*
+* Users may allocate dma buffer before they call
pm_runtime_get, then
+* it will lack the necessary tlb flush.
+*
+* We have no good reason to request the users always
call
dma_alloc_xx
+* after pm_runtime_get_sync.
+*
+* Thus, Make sure the tlb always is clean after each
PM
resume.
+*/
+ mtk_iommu_tlb_do_flush_all(data);
+
/*
* Uppon first resume, only enable the clk and return,
since
the values of the
* registers are not yet set.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
