Skip to content

Commit 8cfc5b6

Browse files
jgunthorpeAlex Williamson
jgunthorpe
authored and
Alex Williamson
committed
vfio: Replace the iommu notifier with a device list
Instead of bouncing the function call to the driver op through a blocking notifier just have the iommu layer call it directly. Register each device that is being attached to the iommu with the lower driver which then threads them on a linked list and calls the appropriate driver op at the right time. Currently the only use is if dma_unmap() is defined. Also, fully lock all the debugging tests on the pinning path that a dma_unmap is registered. Reviewed-by: Christoph Hellwig <[email protected]> Reviewed-by: Kevin Tian <[email protected]> Signed-off-by: Jason Gunthorpe <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alex Williamson <[email protected]>
1 parent ce4b465 commit 8cfc5b6

File tree

4 files changed

+81
-77
lines changed

4 files changed

+81
-77
lines changed

‎drivers/vfio/vfio.c‎

Lines changed: 10 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops *ops)
231231
{
232232
struct vfio_iommu_driver *driver, *tmp;
233233

234-
if (WARN_ON(!ops->register_notifier != !ops->unregister_notifier))
234+
if (WARN_ON(!ops->register_device != !ops->unregister_device))
235235
return -EINVAL;
236236

237237
driver = kzalloc(sizeof(*driver), GFP_KERNEL);
@@ -1082,17 +1082,6 @@ static void vfio_device_unassign_container(struct vfio_device *device)
10821082
up_write(&device->group->group_rwsem);
10831083
}
10841084

1085-
static int vfio_iommu_notifier(struct notifier_block *nb, unsigned long action,
1086-
void *data)
1087-
{
1088-
struct vfio_device *vfio_device =
1089-
container_of(nb, struct vfio_device, iommu_nb);
1090-
struct vfio_iommu_type1_dma_unmap *unmap = data;
1091-
1092-
vfio_device->ops->dma_unmap(vfio_device, unmap->iova, unmap->size);
1093-
return NOTIFY_OK;
1094-
}
1095-
10961085
static struct file *vfio_device_open(struct vfio_device *device)
10971086
{
10981087
struct vfio_iommu_driver *iommu_driver;
@@ -1128,15 +1117,9 @@ static struct file *vfio_device_open(struct vfio_device *device)
11281117
}
11291118

11301119
iommu_driver = device->group->container->iommu_driver;
1131-
if (device->ops->dma_unmap && iommu_driver &&
1132-
iommu_driver->ops->register_notifier) {
1133-
unsigned long events = VFIO_IOMMU_NOTIFY_DMA_UNMAP;
1134-
1135-
device->iommu_nb.notifier_call = vfio_iommu_notifier;
1136-
iommu_driver->ops->register_notifier(
1137-
device->group->container->iommu_data, &events,
1138-
&device->iommu_nb);
1139-
}
1120+
if (iommu_driver && iommu_driver->ops->register_device)
1121+
iommu_driver->ops->register_device(
1122+
device->group->container->iommu_data, device);
11401123

11411124
up_read(&device->group->group_rwsem);
11421125
}
@@ -1176,11 +1159,9 @@ static struct file *vfio_device_open(struct vfio_device *device)
11761159
device->ops->close_device(device);
11771160

11781161
iommu_driver = device->group->container->iommu_driver;
1179-
if (device->ops->dma_unmap && iommu_driver &&
1180-
iommu_driver->ops->unregister_notifier)
1181-
iommu_driver->ops->unregister_notifier(
1182-
device->group->container->iommu_data,
1183-
&device->iommu_nb);
1162+
if (iommu_driver && iommu_driver->ops->unregister_device)
1163+
iommu_driver->ops->unregister_device(
1164+
device->group->container->iommu_data, device);
11841165
}
11851166
err_undo_count:
11861167
up_read(&device->group->group_rwsem);
@@ -1385,11 +1366,9 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep)
13851366
device->ops->close_device(device);
13861367

13871368
iommu_driver = device->group->container->iommu_driver;
1388-
if (device->ops->dma_unmap && iommu_driver &&
1389-
iommu_driver->ops->unregister_notifier)
1390-
iommu_driver->ops->unregister_notifier(
1391-
device->group->container->iommu_data,
1392-
&device->iommu_nb);
1369+
if (iommu_driver && iommu_driver->ops->unregister_device)
1370+
iommu_driver->ops->unregister_device(
1371+
device->group->container->iommu_data, device);
13931372
up_read(&device->group->group_rwsem);
13941373
device->open_count--;
13951374
if (device->open_count == 0)

‎drivers/vfio/vfio.h‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,6 @@ enum vfio_iommu_notify_type {
3333
VFIO_IOMMU_CONTAINER_CLOSE = 0,
3434
};
3535

36-
/* events for register_notifier() */
37-
#define VFIO_IOMMU_NOTIFY_DMA_UNMAP BIT(0)
38-
3936
/**
4037
* struct vfio_iommu_driver_ops - VFIO IOMMU driver callbacks
4138
*/
@@ -58,11 +55,10 @@ struct vfio_iommu_driver_ops {
5855
unsigned long *phys_pfn);
5956
int (*unpin_pages)(void *iommu_data,
6057
unsigned long *user_pfn, int npage);
61-
int (*register_notifier)(void *iommu_data,
62-
unsigned long *events,
63-
struct notifier_block *nb);
64-
int (*unregister_notifier)(void *iommu_data,
65-
struct notifier_block *nb);
58+
void (*register_device)(void *iommu_data,
59+
struct vfio_device *vdev);
60+
void (*unregister_device)(void *iommu_data,
61+
struct vfio_device *vdev);
6662
int (*dma_rw)(void *iommu_data, dma_addr_t user_iova,
6763
void *data, size_t count, bool write);
6864
struct iommu_domain *(*group_iommu_domain)(void *iommu_data,

‎drivers/vfio/vfio_iommu_type1.c‎

Lines changed: 66 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ struct vfio_iommu {
6767
struct list_head iova_list;
6868
struct mutex lock;
6969
struct rb_root dma_list;
70-
struct blocking_notifier_head notifier;
70+
struct list_head device_list;
71+
struct mutex device_list_lock;
7172
unsigned int dma_avail;
7273
unsigned int vaddr_invalid_count;
7374
uint64_t pgsize_bitmap;
@@ -865,8 +866,8 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
865866
}
866867
}
867868

868-
/* Fail if notifier list is empty */
869-
if (!iommu->notifier.head) {
869+
/* Fail if no dma_umap notifier is registered */
870+
if (list_empty(&iommu->device_list)) {
870871
ret = -EINVAL;
871872
goto pin_done;
872873
}
@@ -1287,6 +1288,35 @@ static int verify_bitmap_size(uint64_t npages, uint64_t bitmap_size)
12871288
return 0;
12881289
}
12891290

1291+
/*
1292+
* Notify VFIO drivers using vfio_register_emulated_iommu_dev() to invalidate
1293+
* and unmap iovas within the range we're about to unmap. Drivers MUST unpin
1294+
* pages in response to an invalidation.
1295+
*/
1296+
static void vfio_notify_dma_unmap(struct vfio_iommu *iommu,
1297+
struct vfio_dma *dma)
1298+
{
1299+
struct vfio_device *device;
1300+
1301+
if (list_empty(&iommu->device_list))
1302+
return;
1303+
1304+
/*
1305+
* The device is expected to call vfio_unpin_pages() for any IOVA it has
1306+
* pinned within the range. Since vfio_unpin_pages() will eventually
1307+
* call back down to this code and try to obtain the iommu->lock we must
1308+
* drop it.
1309+
*/
1310+
mutex_lock(&iommu->device_list_lock);
1311+
mutex_unlock(&iommu->lock);
1312+
1313+
list_for_each_entry(device, &iommu->device_list, iommu_entry)
1314+
device->ops->dma_unmap(device, dma->iova, dma->size);
1315+
1316+
mutex_unlock(&iommu->device_list_lock);
1317+
mutex_lock(&iommu->lock);
1318+
}
1319+
12901320
static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
12911321
struct vfio_iommu_type1_dma_unmap *unmap,
12921322
struct vfio_bitmap *bitmap)
@@ -1400,29 +1430,14 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
14001430
}
14011431

14021432
if (!RB_EMPTY_ROOT(&dma->pfn_list)) {
1403-
struct vfio_iommu_type1_dma_unmap nb_unmap;
1404-
14051433
if (dma_last == dma) {
14061434
BUG_ON(++retries > 10);
14071435
} else {
14081436
dma_last = dma;
14091437
retries = 0;
14101438
}
14111439

1412-
nb_unmap.iova = dma->iova;
1413-
nb_unmap.size = dma->size;
1414-
1415-
/*
1416-
* Notify anyone (mdev vendor drivers) to invalidate and
1417-
* unmap iovas within the range we're about to unmap.
1418-
* Vendor drivers MUST unpin pages in response to an
1419-
* invalidation.
1420-
*/
1421-
mutex_unlock(&iommu->lock);
1422-
blocking_notifier_call_chain(&iommu->notifier,
1423-
VFIO_IOMMU_NOTIFY_DMA_UNMAP,
1424-
&nb_unmap);
1425-
mutex_lock(&iommu->lock);
1440+
vfio_notify_dma_unmap(iommu, dma);
14261441
goto again;
14271442
}
14281443

@@ -2475,7 +2490,7 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
24752490

24762491
if (list_empty(&iommu->emulated_iommu_groups) &&
24772492
list_empty(&iommu->domain_list)) {
2478-
WARN_ON(iommu->notifier.head);
2493+
WARN_ON(!list_empty(&iommu->device_list));
24792494
vfio_iommu_unmap_unpin_all(iommu);
24802495
}
24812496
goto detach_group_done;
@@ -2507,7 +2522,8 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
25072522
if (list_empty(&domain->group_list)) {
25082523
if (list_is_singular(&iommu->domain_list)) {
25092524
if (list_empty(&iommu->emulated_iommu_groups)) {
2510-
WARN_ON(iommu->notifier.head);
2525+
WARN_ON(!list_empty(
2526+
&iommu->device_list));
25112527
vfio_iommu_unmap_unpin_all(iommu);
25122528
} else {
25132529
vfio_iommu_unmap_unpin_reaccount(iommu);
@@ -2568,7 +2584,8 @@ static void *vfio_iommu_type1_open(unsigned long arg)
25682584
iommu->dma_avail = dma_entry_limit;
25692585
iommu->container_open = true;
25702586
mutex_init(&iommu->lock);
2571-
BLOCKING_INIT_NOTIFIER_HEAD(&iommu->notifier);
2587+
mutex_init(&iommu->device_list_lock);
2588+
INIT_LIST_HEAD(&iommu->device_list);
25722589
init_waitqueue_head(&iommu->vaddr_wait);
25732590
iommu->pgsize_bitmap = PAGE_MASK;
25742591
INIT_LIST_HEAD(&iommu->emulated_iommu_groups);
@@ -3005,28 +3022,40 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
30053022
}
30063023
}
30073024

3008-
static int vfio_iommu_type1_register_notifier(void *iommu_data,
3009-
unsigned long *events,
3010-
struct notifier_block *nb)
3025+
static void vfio_iommu_type1_register_device(void *iommu_data,
3026+
struct vfio_device *vdev)
30113027
{
30123028
struct vfio_iommu *iommu = iommu_data;
30133029

3014-
/* clear known events */
3015-
*events &= ~VFIO_IOMMU_NOTIFY_DMA_UNMAP;
3016-
3017-
/* refuse to register if still events remaining */
3018-
if (*events)
3019-
return -EINVAL;
3030+
if (!vdev->ops->dma_unmap)
3031+
return;
30203032

3021-
return blocking_notifier_chain_register(&iommu->notifier, nb);
3033+
/*
3034+
* list_empty(&iommu->device_list) is tested under the iommu->lock while
3035+
* iteration for dma_unmap must be done under the device_list_lock.
3036+
* Holding both locks here allows avoiding the device_list_lock in
3037+
* several fast paths. See vfio_notify_dma_unmap()
3038+
*/
3039+
mutex_lock(&iommu->lock);
3040+
mutex_lock(&iommu->device_list_lock);
3041+
list_add(&vdev->iommu_entry, &iommu->device_list);
3042+
mutex_unlock(&iommu->device_list_lock);
3043+
mutex_unlock(&iommu->lock);
30223044
}
30233045

3024-
static int vfio_iommu_type1_unregister_notifier(void *iommu_data,
3025-
struct notifier_block *nb)
3046+
static void vfio_iommu_type1_unregister_device(void *iommu_data,
3047+
struct vfio_device *vdev)
30263048
{
30273049
struct vfio_iommu *iommu = iommu_data;
30283050

3029-
return blocking_notifier_chain_unregister(&iommu->notifier, nb);
3051+
if (!vdev->ops->dma_unmap)
3052+
return;
3053+
3054+
mutex_lock(&iommu->lock);
3055+
mutex_lock(&iommu->device_list_lock);
3056+
list_del(&vdev->iommu_entry);
3057+
mutex_unlock(&iommu->device_list_lock);
3058+
mutex_unlock(&iommu->lock);
30303059
}
30313060

30323061
static int vfio_iommu_type1_dma_rw_chunk(struct vfio_iommu *iommu,
@@ -3160,8 +3189,8 @@ static const struct vfio_iommu_driver_ops vfio_iommu_driver_ops_type1 = {
31603189
.detach_group = vfio_iommu_type1_detach_group,
31613190
.pin_pages = vfio_iommu_type1_pin_pages,
31623191
.unpin_pages = vfio_iommu_type1_unpin_pages,
3163-
.register_notifier = vfio_iommu_type1_register_notifier,
3164-
.unregister_notifier = vfio_iommu_type1_unregister_notifier,
3192+
.register_device = vfio_iommu_type1_register_device,
3193+
.unregister_device = vfio_iommu_type1_unregister_device,
31653194
.dma_rw = vfio_iommu_type1_dma_rw,
31663195
.group_iommu_domain = vfio_iommu_type1_group_iommu_domain,
31673196
.notify = vfio_iommu_type1_notify,

‎include/linux/vfio.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ struct vfio_device {
4949
unsigned int open_count;
5050
struct completion comp;
5151
struct list_head group_next;
52-
struct notifier_block iommu_nb;
52+
struct list_head iommu_entry;
5353
};
5454

5555
/**

0 commit comments

Comments
 (0)