Re: [PATCH v3 00/18] Fix BUG_ON in vfio_iommu_group_notifier()

2021-12-16 Thread Lu Baolu

On 12/6/21 9:58 AM, Lu Baolu wrote:

Hi folks,

The iommu group is the minimal isolation boundary for DMA. Devices in
a group can access each other's MMIO registers via peer to peer DMA
and also need share the same I/O address space.

Once the I/O address space is assigned to user control it is no longer
available to the dma_map* API, which effectively makes the DMA API
non-working.

Second, userspace can use DMA initiated by a device that it controls
to access the MMIO spaces of other devices in the group. This allows
userspace to indirectly attack any kernel owned device and it's driver.

Therefore groups must either be entirely under kernel control or
userspace control, never a mixture. Unfortunately some systems have
problems with the granularity of groups and there are a couple of
important exceptions:

  - pci_stub allows the admin to block driver binding on a device and
make it permanently shared with userspace. Since PCI stub does not
do DMA it is safe, however the admin must understand that using
pci_stub allows userspace to attack whatever device it was bound
it.

  - PCI bridges are sometimes included in groups. Typically PCI bridges
do not use DMA, and generally do not have MMIO regions.

Generally any device that does not have any MMIO registers is a
possible candidate for an exception.

Currently vfio adopts a workaround to detect violations of the above
restrictions by monitoring the driver core BOUND event, and hardwiring
the above exceptions. Since there is no way for vfio to reject driver
binding at this point, BUG_ON() is triggered if a violation is
captured (kernel driver BOUND event on a group which already has some
devices assigned to userspace). Aside from the bad user experience
this opens a way for root userspace to crash the kernel, even in high
integrity configurations, by manipulating the module binding and
triggering the BUG_ON.

This series solves this problem by making the user/kernel ownership a
core concept at the IOMMU layer. The driver core enforces kernel
ownership while drivers are bound and violations now result in a error
codes during probe, not BUG_ON failures.

Patch partitions:
   [PATCH 1-9]: Detect DMA ownership conflicts during driver binding;
   [PATCH 10-13]: Add security context management for assigned devices;
   [PATCH 14-18]: Various cleanups.

This is part one of three initial series for IOMMUFD:
  * Move IOMMU Group security into the iommu layer
  - Generic IOMMUFD implementation
  - VFIO ability to consume IOMMUFD


Thank you very much for reviewing my series. The v4 of this series has
been posted here:

https://lore.kernel.org/linux-iommu/20211217063708.1740334-1-baolu...@linux.intel.com/

Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v4 13/13] drm/tegra: Use the iommu dma_owner mechanism

2021-12-16 Thread Lu Baolu
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 
Tested-by: Dmitry Osipenko  # Nexus7 T30
---
 drivers/gpu/drm/tegra/dc.c   |  1 +
 drivers/gpu/drm/tegra/drm.c  | 54 +---
 drivers/gpu/drm/tegra/gr2d.c |  1 +
 drivers/gpu/drm/tegra/gr3d.c |  1 +
 drivers/gpu/drm/tegra/vic.c  |  3 +-
 5 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index a29d64f87563..8fd7a083cc44 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -3108,6 +3108,7 @@ struct platform_driver tegra_dc_driver = {
.driver = {
.name = "tegra-dc",
.of_match_table = tegra_dc_of_match,
+   .suppress_auto_claim_dma_owner = true,
},
.probe = tegra_dc_probe,
.remove = tegra_dc_remove,
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 8d37d6b00562..8a5fd390f85f 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,48 +944,41 @@ 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.
 */
-   if (domain && domain != tegra->domain)
-   return 0;
+   client->group = NULL;
+   if (!client->dev->iommu_group || (domain && domain != tegra->domain))
+   return iommu_device_set_dma_owner(client->dev,
+ DMA_OWNER_DMA_API, NULL);
 
-   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);
-
-   iommu_group_put(client->group);
+   iommu_detach_device_shared(tegra->domain, client->dev);
+   iommu_device_release_dma_owner(client->dev,
+  DMA_OWNER_PRIVATE_DOMAIN);
client->group = NULL;
+   } else {
+   iommu_device_release_dma_owner(client->dev, DMA_OWNER_DMA_API);
}
 }
 
diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c

[PATCH v4 12/13] iommu: Remove iommu group changes notifier

2021-12-16 Thread Lu Baolu
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 1bc03118dfb3..860ac545ac77 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -417,13 +417,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);
@@ -496,10 +489,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);
@@ -913,18 +902,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 3ad66cb9bedc..053f6ea54fbd 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);
 
ret = ida_simple_get(_group_ida, 0, 0, GFP_KERNEL);
if (ret < 0) {
@@ -904,10 +901,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);
@@ -949,10 +942,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) {
@@ -1075,36 +1064,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 blocking_notifier_chain_register(>notifier, 

[PATCH v4 11/13] vfio: Remove iommu group notifier

2021-12-16 Thread Lu Baolu
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 6426b29e73a2..539f5da9eb34 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 v4 09/13] vfio: Remove use of vfio_group_viable()

2021-12-16 Thread Lu Baolu
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 b75ba7551079..241756b85705 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 v4 10/13] vfio: Delete the unbound_list

2021-12-16 Thread Lu Baolu
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 241756b85705..6426b29e73a2 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 v4 08/13] vfio: Set DMA USER ownership for VFIO devices

2021-12-16 Thread Lu Baolu
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   |  3 +++
 drivers/vfio/platform/vfio_amba.c |  1 +
 drivers/vfio/platform/vfio_platform.c |  1 +
 drivers/vfio/vfio.c   | 13 -
 5 files changed, 18 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..b749d092a185 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -587,6 +587,7 @@ static struct fsl_mc_driver vfio_fsl_mc_driver = {
.driver = {
.name   = "vfio-fsl-mc",
.owner  = THIS_MODULE,
+   .suppress_auto_claim_dma_owner = true,
},
 };
 
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index a5ce92beb655..ce3e814b5506 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -193,6 +193,9 @@ static struct pci_driver vfio_pci_driver = {
.remove = vfio_pci_remove,
.sriov_configure= vfio_pci_sriov_configure,
.err_handler= _pci_core_err_handlers,
+   .driver = {
+   .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..2146ee52901a 100644
--- a/drivers/vfio/platform/vfio_amba.c
+++ b/drivers/vfio/platform/vfio_amba.c
@@ -94,6 +94,7 @@ static struct amba_driver vfio_amba_driver = {
.drv = {
.name = "vfio-amba",
.owner = THIS_MODULE,
+   .suppress_auto_claim_dma_owner = true,
},
 };
 
diff --git a/drivers/vfio/platform/vfio_platform.c 
b/drivers/vfio/platform/vfio_platform.c
index 68a1c87066d7..5ef06e668192 100644
--- a/drivers/vfio/platform/vfio_platform.c
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -75,6 +75,7 @@ static struct platform_driver vfio_platform_driver = {
.remove = vfio_platform_remove,
.driver = {
.name   = "vfio-platform",
+   .suppress_auto_claim_dma_owner = true,
},
 };
 
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 735d1d344af9..b75ba7551079 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 v4 07/13] iommu: Add iommu_at[de]tach_device_shared() for multi-device groups

2021-12-16 Thread Lu Baolu
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 
Tested-by: Dmitry Osipenko 
---
 include/linux/iommu.h | 13 +++
 drivers/iommu/iommu.c | 79 +++
 2 files changed, 92 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5ad4cf13370d..1bc03118dfb3 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -703,6 +703,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 */
 
@@ -743,11 +745,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 8bec71b1cc18..3ad66cb9bedc 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;
unsigned int owner_cnt;
+   unsigned int attach_cnt;
void *owner_cookie;
 };
 
@@ -3512,3 +3513,81 @@ void iommu_device_release_dma_owner(struct device *dev, 
enum iommu_dma_owner own
iommu_group_put(group);
 }
 EXPORT_SYMBOL_GPL(iommu_device_release_dma_owner);
+
+/**
+ * iommu_attach_device_shared() - Attach shared domain to a device
+ * @domain: The shared domain.
+ * @dev: The device.
+ *
+ * Similar to iommu_attach_device(), but allowed for shared-group devices
+ * and guarantees that all devices in an iommu group could only be attached
+ * by a same iommu domain. The caller should explicitly set the dma ownership
+ * of DMA_OWNER_PRIVATE_DOMAIN or DMA_OWNER_PRIVATE_DOMAIN_USER type before
+ * calling it and use the paired helper iommu_detach_device_shared() for
+ * cleanup.
+ */
+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 (group->dma_owner != DMA_OWNER_PRIVATE_DOMAIN &&
+   group->dma_owner != DMA_OWNER_PRIVATE_DOMAIN_USER) {
+   ret = -EPERM;
+   goto unlock_out;
+   }
+
+   if (group->attach_cnt) {
+   if (group->domain != domain) {
+   ret = -EBUSY;
+   goto unlock_out;
+   }
+   } else {
+   ret = __iommu_attach_group(domain, group);
+   if (ret)
+   goto unlock_out;
+   }
+
+   group->attach_cnt++;
+unlock_out:
+   mutex_unlock(>mutex);
+   iommu_group_put(group);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_attach_device_shared);
+
+/**
+ * iommu_detach_device_shared() - Detach a domain from device
+ * @domain: The domain.
+ * @dev: The device.
+ *
+ * The detach helper paired with iommu_attach_device_shared().
+ */
+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->attach_cnt || group->domain != domain ||
+   

[PATCH v4 06/13] iommu: Expose group variants of dma ownership interfaces

2021-12-16 Thread Lu Baolu
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 | 47 ++-
 2 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 53a023ee1ac0..5ad4cf13370d 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -699,6 +699,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 */
 
@@ -1115,6 +1119,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 573e253bad51..8bec71b1cc18 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3365,9 +3365,19 @@ static ssize_t iommu_group_store_type(struct iommu_group 
*group,
return ret;
 }
 
-static int iommu_group_set_dma_owner(struct iommu_group *group,
-enum iommu_dma_owner owner,
-void *owner_cookie)
+/**
+ * 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 = 0;
 
@@ -3398,9 +3408,16 @@ static int iommu_group_set_dma_owner(struct iommu_group 
*group,
 
return ret;
 }
+EXPORT_SYMBOL_GPL(iommu_group_set_dma_owner);
 
-static void iommu_group_release_dma_owner(struct iommu_group *group,
- enum iommu_dma_owner 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);
if (WARN_ON(!group->owner_cnt || group->dma_owner != owner))
@@ -3423,6 +3440,26 @@ static void iommu_group_release_dma_owner(struct 
iommu_group *group,
 unlock_out:
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)
+{
+   unsigned int user;
+
+   mutex_lock(>mutex);
+   user = group->owner_cnt;
+   mutex_unlock(>mutex);
+
+   return !user;
+}
+EXPORT_SYMBOL_GPL(iommu_group_dma_owner_unclaimed);
 
 /**
  * iommu_device_set_dma_owner() - Set DMA ownership of a device
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v4 05/13] iommu: Add security context management for assigned devices

2021-12-16 Thread Lu Baolu
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 | 35 ---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5439bf45afb2..573e253bad51 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);
@@ -2323,7 +2328,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,
@@ -2360,7 +2365,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;
@@ -3373,6 +3383,16 @@ static int iommu_group_set_dma_owner(struct iommu_group 
*group,
group->owner_cookie = owner_cookie;
group->owner_cnt++;
 
+   /*
+* 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 &&
+   group->domain &&
+   !WARN_ON(group->domain != group->default_domain))
+   __iommu_detach_group(group->domain, group);
+
 unlock_out:
mutex_unlock(>mutex);
 
@@ -3391,6 +3411,15 @@ static void iommu_group_release_dma_owner(struct 
iommu_group *group,
 
group->dma_owner = DMA_OWNER_DMA_API;
 
+   /*
+* 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);
+   }
+
 unlock_out:
mutex_unlock(>mutex);
 }
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v4 04/13] PCI: portdrv: Suppress kernel DMA ownership auto-claiming

2021-12-16 Thread Lu Baolu
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 | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index 35eca6277a96..c48a8734f9c4 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -202,7 +202,10 @@ static struct pci_driver pcie_portdriver = {
 
.err_handler= _portdrv_err_handler,
 
-   .driver.pm  = PCIE_PORTDRV_PM_OPS,
+   .driver = {
+   .pm = PCIE_PORTDRV_PM_OPS,
+   .suppress_auto_claim_dma_owner = true,
+   },
 };
 
 static int __init dmi_pcie_pme_disable_msi(const struct dmi_system_id *d)
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v4 03/13] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming

2021-12-16 Thread Lu Baolu
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 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/pci-stub.c b/drivers/pci/pci-stub.c
index e408099fea52..6324c68602b4 100644
--- a/drivers/pci/pci-stub.c
+++ b/drivers/pci/pci-stub.c
@@ -36,6 +36,9 @@ static struct pci_driver stub_driver = {
.name   = "pci-stub",
.id_table   = NULL, /* only dynamic id's */
.probe  = pci_stub_probe,
+   .driver = {
+   .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 v4 02/13] driver core: Set DMA ownership during driver bind/unbind

2021-12-16 Thread Lu Baolu
This extends really_probe() to allow checking for dma ownership conflict
during the driver binding process. By default, the DMA_OWNER_DMA_API is
claimed for the bound driver before calling its .probe() callback. If this
operation fails (e.g. the iommu group of the target device already has the
DMA_OWNER_USER set), the binding process is aborted to avoid breaking the
security contract for devices in the iommu group.

Without this change, 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 can force the kernel to
BUG() even with lockdown=integrity.

Driver may set a new flag (suppress_auto_claim_dma_owner) to disable auto
claim in the binding process. Examples include kernel drivers (pci_stub,
PCI bridge drivers, etc.) which don't trigger DMA at all thus can be safely
exempted in DMA ownership check and userspace framework drivers (vfio/vdpa
etc.) which need to manually claim DMA_OWNER_USER when assigning a device
to userspace.

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/driver.h |  2 ++
 drivers/base/dd.c | 37 ++-
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
index a498ebcf4993..f5bf7030c416 100644
--- a/include/linux/device/driver.h
+++ b/include/linux/device/driver.h
@@ -54,6 +54,7 @@ enum probe_type {
  * @owner: The module owner.
  * @mod_name:  Used for built-in modules.
  * @suppress_bind_attrs: Disables bind/unbind via sysfs.
+ * @suppress_auto_claim_dma_owner: Disable kernel dma auto-claim.
  * @probe_type:Type of the probe (synchronous or asynchronous) to use.
  * @of_match_table: The open firmware table.
  * @acpi_match_table: The ACPI match table.
@@ -100,6 +101,7 @@ struct device_driver {
const char  *mod_name;  /* used for built-in modules */
 
bool suppress_bind_attrs;   /* disables bind/unbind via sysfs */
+   bool suppress_auto_claim_dma_owner;
enum probe_type probe_type;
 
const struct of_device_id   *of_match_table;
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 68ea1f949daa..b04eec5dcefa 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "base.h"
 #include "power/power.h"
@@ -538,6 +539,32 @@ static int call_driver_probe(struct device *dev, struct 
device_driver *drv)
return ret;
 }
 
+static int device_dma_configure(struct device *dev, struct device_driver *drv)
+{
+   int ret;
+
+   if (!dev->bus->dma_configure)
+   return 0;
+
+   ret = dev->bus->dma_configure(dev);
+   if (ret)
+   return ret;
+
+   if (!drv->suppress_auto_claim_dma_owner)
+   ret = iommu_device_set_dma_owner(dev, DMA_OWNER_DMA_API, NULL);
+
+   return ret;
+}
+
+static void device_dma_cleanup(struct device *dev, struct device_driver *drv)
+{
+   if (!dev->bus->dma_configure)
+   return;
+
+   if (!drv->suppress_auto_claim_dma_owner)
+   iommu_device_release_dma_owner(dev, DMA_OWNER_DMA_API);
+}
+
 static int really_probe(struct device *dev, struct device_driver *drv)
 {
bool test_remove = IS_ENABLED(CONFIG_DEBUG_TEST_DRIVER_REMOVE) &&
@@ -574,11 +601,8 @@ static int really_probe(struct device *dev, struct 
device_driver *drv)
if (ret)
goto pinctrl_bind_failed;
 
-   if (dev->bus->dma_configure) {
-   ret = dev->bus->dma_configure(dev);
-   if (ret)
-   goto probe_failed;
-   }
+   if (device_dma_configure(dev, drv))
+   goto pinctrl_bind_failed;
 
ret = driver_sysfs_add(dev);
if (ret) {
@@ -660,6 +684,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);
+
+   device_dma_cleanup(dev, drv);
 pinctrl_bind_failed:
device_links_no_driver(dev);
devres_release_all(dev);
@@ -1204,6 +1230,7 @@ static void __device_release_driver(struct device *dev, 
struct device *parent)
else if (drv->remove)
drv->remove(dev);
 
+   device_dma_cleanup(dev, drv);
device_links_driver_cleanup(dev);
 
devres_release_all(dev);
-- 
2.25.1

___
iommu mailing list

[PATCH v4 01/13] iommu: Add device dma ownership set/release interfaces

2021-12-16 Thread Lu Baolu
>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 | 34 
 drivers/iommu/iommu.c | 95 +++
 2 files changed, 129 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index d2f3435e7d17..53a023ee1ac0 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -162,6 +162,21 @@ enum iommu_dev_features {
IOMMU_DEV_FEAT_IOPF,
 };
 
+/**
+ * enum iommu_dma_owner - IOMMU 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_DMA_API,
+   DMA_OWNER_PRIVATE_DOMAIN,
+   DMA_OWNER_PRIVATE_DOMAIN_USER,
+};
+
 #define IOMMU_PASID_INVALID(-1U)
 
 #ifdef CONFIG_IOMMU_API
@@ -681,6 +696,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 +1100,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..5439bf45afb2 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;
+   unsigned int owner_cnt;
+   void *owner_cookie;
 };
 
 struct group_device {
@@ -3351,3 +3354,95 @@ static ssize_t iommu_group_store_type(struct iommu_group 
*group,
 
return ret;
 }
+
+static int iommu_group_set_dma_owner(struct iommu_group *group,
+enum iommu_dma_owner owner,
+void *owner_cookie)
+{
+   int ret = 0;
+
+   mutex_lock(>mutex);
+   if (group->owner_cnt &&
+   

[PATCH v4 00/13] Fix BUG_ON in vfio_iommu_group_notifier()

2021-12-16 Thread Lu Baolu
Hi folks,

The iommu group is the minimal isolation boundary for DMA. Devices in
a group can access each other's MMIO registers via peer to peer DMA
and also need share the same I/O address space.

Once the I/O address space is assigned to user control it is no longer
available to the dma_map* API, which effectively makes the DMA API
non-working.

Second, userspace can use DMA initiated by a device that it controls
to access the MMIO spaces of other devices in the group. This allows
userspace to indirectly attack any kernel owned device and it's driver.

Therefore groups must either be entirely under kernel control or
userspace control, never a mixture. Unfortunately some systems have
problems with the granularity of groups and there are a couple of
important exceptions:

 - pci_stub allows the admin to block driver binding on a device and
   make it permanently shared with userspace. Since PCI stub does not
   do DMA it is safe, however the admin must understand that using
   pci_stub allows userspace to attack whatever device it was bound
   it.

 - PCI bridges are sometimes included in groups. Typically PCI bridges
   do not use DMA, and generally do not have MMIO regions.

Generally any device that does not have any MMIO registers is a
possible candidate for an exception.

Currently vfio adopts a workaround to detect violations of the above
restrictions by monitoring the driver core BOUND event, and hardwiring
the above exceptions. Since there is no way for vfio to reject driver
binding at this point, BUG_ON() is triggered if a violation is
captured (kernel driver BOUND event on a group which already has some
devices assigned to userspace). Aside from the bad user experience
this opens a way for root userspace to crash the kernel, even in high
integrity configurations, by manipulating the module binding and
triggering the BUG_ON.

This series solves this problem by making the user/kernel ownership a
core concept at the IOMMU layer. The driver core enforces kernel
ownership while drivers are bound and violations now result in a error
codes during probe, not BUG_ON failures.

Patch partitions:
  [PATCH 1-4]: Detect DMA ownership conflicts during driver binding;
  [PATCH 5-8]: Add security context management for assigned devices;
  [PATCH 9-13]: Various cleanups.

This is also part one of three initial series for IOMMUFD:
 * Move IOMMU Group security into the iommu layer
 - Generic IOMMUFD implementation
 - VFIO ability to consume IOMMUFD

Change log:
v1: initial post
  - 
https://lore.kernel.org/linux-iommu/2025020552.2378167-1-baolu...@linux.intel.com/

v2:
  - 
https://lore.kernel.org/linux-iommu/20211128025051.355578-1-baolu...@linux.intel.com/

  - 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.

v3:
  - 
https://lore.kernel.org/linux-iommu/20211206015903.88687-1-baolu...@linux.intel.com/

  - Rename bus_type::dma_unconfigure to bus_type::dma_cleanup. [Greg]

https://lore.kernel.org/linux-iommu/c3230ace-c878-39db-1663-2b752ff53...@linux.intel.com/T/#m6711e041e47cb0cbe3964fad0a3466f5ae4b3b9b

  - Avoid _platform_dma_configure for platform_bus_type::dma_configure.
[Greg]

https://lore.kernel.org/linux-iommu/c3230ace-c878-39db-1663-2b752ff53...@linux.intel.com/T/#m43fc46286611aa56a5c0eeaad99d539e5519f3f6

  - Patch "0012-iommu-Add-iommu_at-de-tach_device_shared-for-mult.patch"
and "0018-drm-tegra-Use-the-iommu-dma_owner-mechanism.patch" have
been tested by Dmitry Osipenko .

v4:
  - Remove unnecessary tegra->domain chech in the tegra patch. (Jason)
  - Remove DMA_OWNER_NONE. (Joerg)
  - Change refcount to unsigned int. (Christoph)
  - Move mutex lock into group set_dma_owner functions. (Christoph)
  - Add kernel doc for iommu_attach/detach_domain_shared(). (Christoph)
  - Move dma auto-claim into driver core. (Jason/Christoph)

This is based on next branch of linux-iommu tree:
https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git
and also available on github:
https://github.com/LuBaolu/intel-iommu/commits/iommu-dma-ownership-v4

Merry Christmas to you all!

Best regards,
baolu

Jason Gunthorpe (2):
  vfio: Delete the unbound_list
  drm/tegra: Use the iommu dma_owner mechanism

Lu Baolu (11):
  

Re: [patch V3 00/35] genirq/msi, PCI/MSI: Spring cleaning - Part 2

2021-12-16 Thread Thomas Gleixner
Nishanth,

On Wed, Dec 15 2021 at 19:45, Nishanth Menon wrote:
> On 17:35-20211215, Thomas Gleixner wrote:
> Thanks once again for your help. Hope we can roll in the fixes for
> part3.

Sure, it's only the one-liner for ti sci. Got it folded already.

Thanks for your help and testing!

   tglx
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 00/33] Separate struct slab from struct page

2021-12-16 Thread Hyeonggon Yoo
On Tue, Dec 14, 2021 at 01:57:22PM +0100, Vlastimil Babka wrote:
> On 12/1/21 19:14, Vlastimil Babka wrote:
> > Folks from non-slab subsystems are Cc'd only to patches affecting them, and
> > this cover letter.
> > 
> > Series also available in git, based on 5.16-rc3:
> > https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/linux.git/log/?h=slab-struct_slab-v2r2
> 
> Pushed a new branch slab-struct-slab-v3r3 with accumulated fixes and small 
> tweaks
> and a new patch from Hyeonggon Yoo on top. To avoid too much spam, here's a 
> range diff:

Reviewing the whole patch series takes longer than I thought.
I'll try to review and test rest of patches when I have time.

I added Tested-by if kernel builds okay and kselftests
does not break the kernel on my machine.
(with CONFIG_SLAB/SLUB/SLOB depending on the patch),
Let me know me if you know better way to test a patch.

# mm/slub: Define struct slab fields for CONFIG_SLUB_CPU_PARTIAL only when 
enabled

Reviewed-by: Hyeonggon Yoo <42.hye...@gmail.com>
Tested-by: Hyeonggon Yoo <42.hye...@gmail.com>

Comment:
Works on both SLUB_CPU_PARTIAL and !SLUB_CPU_PARTIAL.
btw, do we need slabs_cpu_partial attribute when we don't use
cpu partials? (!SLUB_CPU_PARTIAL)

# mm/slub: Simplify struct slab slabs field definition
Comment:

This is how struct page looks on the top of v3r3 branch:
struct page {
[...]
struct {/* slab, slob and slub */
union {
struct list_head slab_list;
struct {/* Partial pages */
struct page *next;
#ifdef CONFIG_64BIT
int pages;  /* Nr of pages left */
#else
short int pages;
#endif
};
};
[...]

It's not consistent with struct slab.
I think this is because "mm: Remove slab from struct page" was dropped.
Would you update some of patches?

# mm/sl*b: Differentiate struct slab fields by sl*b implementations
Reviewed-by: Hyeonggon Yoo <42.hye...@gmail.com>
Tested-by: Hyeonggon Yoo <42.hye...@gmail.com>
Works SL[AUO]B on my machine and makes code much better.

# mm/slob: Convert SLOB to use struct slab and struct folio
Reviewed-by: Hyeonggon Yoo <42.hye...@gmail.com>
Tested-by: Hyeonggon Yoo <42.hye...@gmail.com>
It still works fine on SLOB.

# mm/slab: Convert kmem_getpages() and kmem_freepages() to struct slab
Reviewed-by: Hyeonggon Yoo <42.hye...@gmail.com>
Tested-by: Hyeonggon Yoo <42.hye...@gmail.com>

# mm/slub: Convert __free_slab() to use struct slab
Reviewed-by: Hyeonggon Yoo <42.hye...@gmail.com>
Tested-by: Hyeonggon Yoo <42.hye...@gmail.com>

Thanks,
Hyeonggon.

> 
>  1:  10b656f9eb1e =  1:  10b656f9eb1e mm: add virt_to_folio() and 
> folio_address()
>  2:  5e6ad846acf1 =  2:  5e6ad846acf1 mm/slab: Dissolve slab_map_pages() in 
> its caller
>  3:  48d4e9407aa0 =  3:  48d4e9407aa0 mm/slub: Make object_err() static
>  4:  fe1e19081321 =  4:  fe1e19081321 mm: Split slab into its own type
>  5:  af7fd46fbb9b =  5:  af7fd46fbb9b mm: Add account_slab() and 
> unaccount_slab()
>  6:  7ed088d601d9 =  6:  7ed088d601d9 mm: Convert virt_to_cache() to use 
> struct slab
>  7:  1d41188b9401 =  7:  1d41188b9401 mm: Convert __ksize() to struct slab
>  8:  5d9d1231461f !  8:  8fd22e0b086e mm: Use struct slab in kmem_obj_info()
> @@ Commit message
>  slab type instead of the page type, we make it obvious that this can
>  only be called for slabs.
>  
> +[ vba...@suse.cz: also convert the related kmem_valid_obj() to 
> folios ]
> +
>  Signed-off-by: Matthew Wilcox (Oracle) 
>  Signed-off-by: Vlastimil Babka 
>  
> @@ mm/slab.h: struct kmem_obj_info {
>   #endif /* MM_SLAB_H */
>  
>   ## mm/slab_common.c ##
> +@@ mm/slab_common.c: bool slab_is_available(void)
> +  */
> + bool kmem_valid_obj(void *object)
> + {
> +-struct page *page;
> ++struct folio *folio;
> + 
> + /* Some arches consider ZERO_SIZE_PTR to be a valid address. */
> + if (object < (void *)PAGE_SIZE || !virt_addr_valid(object))
> + return false;
> +-page = virt_to_head_page(object);
> +-return PageSlab(page);
> ++folio = virt_to_folio(object);
> ++return folio_test_slab(folio);
> + }
> + EXPORT_SYMBOL_GPL(kmem_valid_obj);
> + 
>  @@ mm/slab_common.c: void kmem_dump_obj(void *object)
>   {
>   char *cp = IS_ENABLED(CONFIG_MMU) ? "" : "/vmalloc";
> @@ mm/slub.c: int __kmem_cache_shutdown(struct kmem_cache *s)
>   objp = base + s->size * objnr;
>   kpp->kp_objp = objp;
>  -if (WARN_ON_ONCE(objp < base || objp >= base + page->objects * 
> s->size || (objp - base) % s->size) ||
> -+if (WARN_ON_ONCE(objp < base || objp 

[PATCH] drm/tegra: Add missing DMA API includes

2021-12-16 Thread Robin Murphy
Include dma-mapping.h directly in the remaining places that touch the
DMA API, to avoid imminent breakage from refactoring other headers.

Signed-off-by: Robin Murphy 
---

This makes arm64 allmodconfig build cleanly for me with the IOVA series,
so hopefully is the last of it...

 drivers/gpu/drm/tegra/dc.c| 1 +
 drivers/gpu/drm/tegra/hub.c   | 1 +
 drivers/gpu/drm/tegra/plane.c | 1 +
 3 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index a29d64f87563..c33420e1fb07 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
index b910155f80c4..7d52a403334b 100644
--- a/drivers/gpu/drm/tegra/hub.c
+++ b/drivers/gpu/drm/tegra/hub.c
@@ -5,6 +5,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index 16a1cdc28657..75db069dd26f 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2017 NVIDIA CORPORATION.  All rights reserved.
  */
 
+#include 
 #include 
 #include 
 
-- 
2.28.0.dirty

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] iommu/virtio: Fix typo in a comment

2021-12-16 Thread Xiang wangx
The double `as' in a comment is repeated, thus it should be removed.

Signed-off-by: Xiang wangx 
---
 drivers/iommu/virtio-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index c9e8367d2962..162bd07e32fe 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -743,7 +743,7 @@ static int viommu_attach_dev(struct iommu_domain *domain, 
struct device *dev)
 
/*
 * In the virtio-iommu device, when attaching the endpoint to a new
-* domain, it is detached from the old one and, if as as a result the
+* domain, it is detached from the old one and, if as a result the
 * old domain isn't attached to any endpoint, all mappings are removed
 * from the old domain and it is freed.
 *
-- 
2.20.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu: Use correctly sized arguments for bit field

2021-12-16 Thread Yury Norov
On Wed, Dec 15, 2021 at 3:24 PM Kees Cook  wrote:
>
> The find.h APIs are designed to be used only on unsigned long arguments.
> This can technically result in a over-read, but it is harmless in this
> case. Regardless, fix it to avoid the warning seen under -Warray-bounds,
> which we'd like to enable globally:
>
> In file included from ./include/linux/bitmap.h:9,
>  from drivers/iommu/intel/iommu.c:17:
> drivers/iommu/intel/iommu.c: In function 'domain_context_mapping_one':
> ./include/linux/find.h:119:37: warning: array subscript 'long unsigned 
> int[0]' is partly outside array bounds of 'int[1]' [-Warray-bounds]
>   119 | unsigned long val = *addr & GENMASK(size - 1, 0);
>   | ^
> drivers/iommu/intel/iommu.c:2115:18: note: while referencing 'max_pde'
>  2115 | int pds, max_pde;
>   |  ^~~
>
> Signed-off-by: Kees Cook 

For all patches in this (not a) series
Acked-by: Yury Norov 

But can you explain, what for did you split this change? The
Documentation/process says: "Solve only one problem per patch.",
but here one problem is solved per 4 patches with identical
description.

I think it would be more logical to move-in this change as a single
commitment rather than random scattered patches.

Thanks,
Yury

> ---
>  drivers/iommu/intel/iommu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index b6a8f3282411..99f9e8229384 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -2112,10 +2112,10 @@ static void domain_exit(struct dmar_domain *domain)
>   */
>  static inline unsigned long context_get_sm_pds(struct pasid_table *table)
>  {
> -   int pds, max_pde;
> +   unsigned long pds, max_pde;
>
> max_pde = table->max_pasid >> PASID_PDE_SHIFT;
> -   pds = find_first_bit((unsigned long *)_pde, MAX_NR_PASID_BITS);
> +   pds = find_first_bit(_pde, MAX_NR_PASID_BITS);
> if (pds < 7)
> return 0;
>
> --
> 2.30.2
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH V7 1/5] swiotlb: Add swiotlb bounce buffer remap function for HV IVM

2021-12-16 Thread Wei Liu
On Wed, Dec 15, 2021 at 01:00:38PM +0800, Tianyu Lan wrote:
> 
> 
> On 12/15/2021 6:40 AM, Dave Hansen wrote:
> > On 12/14/21 2:23 PM, Tom Lendacky wrote:
> > > > I don't really understand how this can be more general any *not* get
> > > > utilized by the existing SEV support.
> > > 
> > > The Virtual Top-of-Memory (VTOM) support is an SEV-SNP feature that is
> > > meant to be used with a (relatively) un-enlightened guest. The idea is
> > > that the C-bit in the guest page tables must be 0 for all accesses. It
> > > is only the physical address relative to VTOM that determines if the
> > > access is encrypted or not. So setting sme_me_mask will actually cause
> > > issues when running with this feature. Since all DMA for an SEV-SNP
> > > guest must still be to shared (unencrypted) memory, some enlightenment
> > > is needed. In this case, memory mapped above VTOM will provide that via
> > > the SWIOTLB update. For SEV-SNP guests running with VTOM, they are
> > > likely to also be running with the Reflect #VC feature, allowing a
> > > "paravisor" to handle any #VCs generated by the guest.
> > > 
> > > See sections 15.36.8 "Virtual Top-of-Memory" and 15.36.9 "Reflect #VC"
> > > in volume 2 of the AMD APM [1].
> > 
> > Thanks, Tom, that's pretty much what I was looking for.
> > 
> > The C-bit normally comes from the page tables.  But, the hardware also
> > provides an alternative way to effectively get C-bit behavior without
> > actually setting the bit in the page tables: Virtual Top-of-Memory
> > (VTOM).  Right?
> > 
> > It sounds like Hyper-V has chosen to use VTOM instead of requiring the
> > guest to do the C-bit in its page tables.
> > 
> > But, the thing that confuses me is when you said: "it (VTOM) is meant to
> > be used with a (relatively) un-enlightened guest".  We don't have an
> > unenlightened guest here.  We have Linux, which is quite enlightened.
> > 
> > > Is VTOM being used because there's something that completely rules out
> > > using the C-bit in the page tables?  What's that "something"?
> 
> 
> For "un-enlightened" guest, there is an another system running insider
> the VM to emulate some functions(tsc, timer, interrupt and so on) and
> this helps not to modify OS(Linux/Windows) a lot. In Hyper-V Isolation
> VM, we called the new system as HCL/paravisor. HCL runs in the VMPL0 and
> Linux runs in VMPL2. This is similar with nested virtualization. HCL
> plays similar role as L1 hypervisor to emulate some general functions
> (e.g, rdmsr/wrmsr accessing and interrupt injection) which needs to be
> enlightened in the enlightened guest. Linux kernel needs to handle
> #vc/#ve exception directly in the enlightened guest. HCL handles such
> exception in un-enlightened guest and emulate interrupt injection which
> helps not to modify OS core part code. Using vTOM also is same purpose.
> Hyper-V uses vTOM avoid changing page table related code in OS(include
> Windows and Linux)and just needs to map memory into decrypted address
> space above vTOM in the driver code.
> 
> Linux has generic swiotlb bounce buffer implementation and so introduce
> swiotlb_unencrypted_base here to set shared memory boundary or vTOM.
> Hyper-V Isolation VM is un-enlightened guest. Hyper-V doesn't expose sev/sme
> capability to guest and so SEV code actually doesn't work.
> So we also can't interact current existing SEV code and these code is
> for enlightened guest support without HCL/paravisor. If other platforms
> or SEV want to use similar vTOM feature, swiotlb_unencrypted_base can
> be reused. So swiotlb_unencrypted_base is a general solution for all
> platforms besides SEV and Hyper-V.
> 

Thanks for the detailed explanation.

Dave, are you happy with this?

The code looks pretty solid to my untrained eyes. And the series has
collected necessary acks from stakeholders. If I don't hear objection by
EOD Friday I will apply this series to hyperv-next.

Wei.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 00/33] Separate struct slab from struct page

2021-12-16 Thread Vlastimil Babka
On 12/16/21 00:38, Roman Gushchin wrote:
> On Tue, Dec 14, 2021 at 05:03:12PM -0800, Roman Gushchin wrote:
>> On Tue, Dec 14, 2021 at 01:57:22PM +0100, Vlastimil Babka wrote:
>> > On 12/1/21 19:14, Vlastimil Babka wrote:
>> > > Folks from non-slab subsystems are Cc'd only to patches affecting them, 
>> > > and
>> > > this cover letter.
>> > > 
>> > > Series also available in git, based on 5.16-rc3:
>> > > https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/linux.git/log/?h=slab-struct_slab-v2r2
>> > 
>> > Pushed a new branch slab-struct-slab-v3r3 with accumulated fixes and small 
>> > tweaks
>> > and a new patch from Hyeonggon Yoo on top. To avoid too much spam, here's 
>> > a range diff:
>> 
>> Hi Vlastimil!
>> 
>> I've started to review this patchset (btw, a really nice work, I like
>> the resulting code way more). Because I'm looking at v3 and I don't have

Thanks a lot, Roman!

...

> 
> * mm/slab: Convert most struct page to struct slab by spatch
> 
> Another patch with the same title? Rebase error?
> 
> * mm/slab: Finish struct page to struct slab conversion
> 
> And this one too?

No, these are for mm/slab.c, the previous were for mm/slub.c :)

> 
> Thanks!
> 
> Roman

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 03/11] drm/tegra: vic: Fix DMA API misuse

2021-12-16 Thread Thierry Reding
On Fri, Dec 10, 2021 at 05:54:44PM +, Robin Murphy wrote:
> Upon failure, dma_alloc_coherent() returns NULL. If that does happen,
> passing some uninitialised stack contents to dma_mapping_error() - which
> belongs to a different API in the first place - has precious little
> chance of detecting it.
> 
> Also include the correct header, because the fragile transitive
> inclusion currently providing it is going to break soon.
> 
> Fixes: 20e7dce255e9 ("drm/tegra: Remove memory allocation from Falcon 
> library")
> CC: Thierry Reding 
> CC: Mikko Perttunen 
> CC: dri-de...@lists.freedesktop.org
> Signed-off-by: Robin Murphy 
> 
> ---
> 
> It also doesn't appear to handle failure of the tegra_drm_alloc() path
> either, but that's a loose thread I have no desire to pull on... ;)
> 
> v2: Resend as part of the series, originally posted separately here:
> 
> https://lore.kernel.org/dri-devel/2703882439344010e33bf21ecd63cf9e5e6dc00d.1637781007.git.robin.mur...@arm.com/
> 
>  drivers/gpu/drm/tegra/vic.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)

Applied, thanks. I've also fixed up the missing failure handling for
tegra_drm_alloc(), which was actually quite trivial to do.

Thierry


signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 02/11] gpu: host1x: Add missing DMA API include

2021-12-16 Thread Thierry Reding
On Fri, Dec 10, 2021 at 05:54:43PM +, Robin Murphy wrote:
> Host1x seems to be relying on picking up dma-mapping.h transitively from
> iova.h, which has no reason to include it in the first place. Fix the
> former issue before we totally break things by fixing the latter one.
> 
> CC: Thierry Reding 
> CC: Mikko Perttunen 
> CC: dri-de...@lists.freedesktop.org
> Signed-off-by: Robin Murphy 
> ---
> 
> v2: No change
> 
>  drivers/gpu/host1x/bus.c | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/1] iommu/vt-d: Remove unused macros

2021-12-16 Thread Christoph Hellwig
On Thu, Dec 16, 2021 at 09:17:03AM +0800, Lu Baolu wrote:
> These macros has no reference in the tree anymore. Cleanup them.
> 
> Signed-off-by: Lu Baolu 

Looks good,

Reviewed-by: Christoph Hellwig 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 05/11] iommu/iova: Squash flush_cb abstraction

2021-12-16 Thread Christoph Hellwig
> @@ -147,7 +142,7 @@ struct iova *reserve_iova(struct iova_domain *iovad, 
> unsigned long pfn_lo,
>   unsigned long pfn_hi);
>  void init_iova_domain(struct iova_domain *iovad, unsigned long granule,
>   unsigned long start_pfn);
> -int init_iova_flush_queue(struct iova_domain *iovad, iova_flush_cb flush_cb);
> +int init_iova_flush_queue(struct iova_domain *iovad, struct iommu_domain 
> *fq_domain);

Overly long line here.

Otherwise looks good:

Reviewed-by: Christoph Hellwig 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 04/11] iommu/iova: Squash entry_dtor abstraction

2021-12-16 Thread Christoph Hellwig
Looks good,

Reviewed-by: Christoph Hellwig 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 03/11] drm/tegra: vic: Fix DMA API misuse

2021-12-16 Thread Christoph Hellwig
Looks good,

Reviewed-by: Christoph Hellwig 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu