Skip to content

Commit 91a2e17

Browse files
jgunthorpejgunthorpe
committed
iommufd: Replace the hwpt->devices list with iommufd_group
The devices list was used as a simple way to avoid having per-group information. Now that this seems to be unavoidable, just commit to per-group information fully and remove the devices list from the HWPT. The iommufd_group stores the currently assigned HWPT for the entire group and we can manage the per-device attach/detach with a list in the iommufd_group. For destruction the flow is organized to make the following patches easier, the actual call to iommufd_object_destroy_user() is done at the top of the call chain without holding any locks. The HWPT to be destroyed is returned out from the locked region to make this possible. Later patches create locking that requires this. Link: https://lore.kernel.org/r/[email protected] Reviewed-by: Lu Baolu <[email protected]> Reviewed-by: Kevin Tian <[email protected]> Tested-by: Nicolin Chen <[email protected]> Signed-off-by: Jason Gunthorpe <[email protected]>
1 parent 3a3329a commit 91a2e17

File tree

3 files changed

+54
-81
lines changed

3 files changed

+54
-81
lines changed

‎drivers/iommu/iommufd/device.c

Lines changed: 46 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,12 @@ static void iommufd_group_release(struct kref *kref)
2020
struct iommufd_group *igroup =
2121
container_of(kref, struct iommufd_group, ref);
2222

23+
WARN_ON(igroup->hwpt || !list_empty(&igroup->device_list));
24+
2325
xa_cmpxchg(&igroup->ictx->groups, iommu_group_id(igroup->group), igroup,
2426
NULL, GFP_KERNEL);
2527
iommu_group_put(igroup->group);
28+
mutex_destroy(&igroup->lock);
2629
kfree(igroup);
2730
}
2831

@@ -83,6 +86,8 @@ static struct iommufd_group *iommufd_get_group(struct iommufd_ctx *ictx,
8386
}
8487

8588
kref_init(&new_igroup->ref);
89+
mutex_init(&new_igroup->lock);
90+
INIT_LIST_HEAD(&new_igroup->device_list);
8691
/* group reference moves into new_igroup */
8792
new_igroup->group = group;
8893

@@ -320,29 +325,18 @@ static int iommufd_device_setup_msi(struct iommufd_device *idev,
320325
return 0;
321326
}
322327

323-
static bool iommufd_hw_pagetable_has_group(struct iommufd_hw_pagetable *hwpt,
324-
struct iommufd_group *igroup)
325-
{
326-
struct iommufd_device *cur_dev;
327-
328-
lockdep_assert_held(&hwpt->devices_lock);
329-
330-
list_for_each_entry(cur_dev, &hwpt->devices, devices_item)
331-
if (cur_dev->igroup->group == igroup->group)
332-
return true;
333-
return false;
334-
}
335-
336328
int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
337329
struct iommufd_device *idev)
338330
{
339331
phys_addr_t sw_msi_start = PHYS_ADDR_MAX;
340332
int rc;
341333

342-
lockdep_assert_held(&hwpt->devices_lock);
334+
mutex_lock(&idev->igroup->lock);
343335

344-
if (WARN_ON(idev->hwpt))
345-
return -EINVAL;
336+
if (idev->igroup->hwpt != NULL && idev->igroup->hwpt != hwpt) {
337+
rc = -EINVAL;
338+
goto err_unlock;
339+
}
346340

347341
/*
348342
* Try to upgrade the domain we have, it is an iommu driver bug to
@@ -356,60 +350,62 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
356350
hwpt->domain->ops->enforce_cache_coherency(
357351
hwpt->domain);
358352
if (!hwpt->enforce_cache_coherency) {
359-
WARN_ON(list_empty(&hwpt->devices));
360-
return -EINVAL;
353+
WARN_ON(list_empty(&idev->igroup->device_list));
354+
rc = -EINVAL;
355+
goto err_unlock;
361356
}
362357
}
363358

364359
rc = iopt_table_enforce_group_resv_regions(&hwpt->ioas->iopt, idev->dev,
365360
idev->igroup->group,
366361
&sw_msi_start);
367362
if (rc)
368-
return rc;
363+
goto err_unlock;
369364

370365
rc = iommufd_device_setup_msi(idev, hwpt, sw_msi_start);
371366
if (rc)
372367
goto err_unresv;
373368

374369
/*
375-
* FIXME: Hack around missing a device-centric iommu api, only attach to
376-
* the group once for the first device that is in the group.
370+
* Only attach to the group once for the first device that is in the
371+
* group. All the other devices will follow this attachment. The user
372+
* should attach every device individually to the hwpt as the per-device
373+
* reserved regions are only updated during individual device
374+
* attachment.
377375
*/
378-
if (!iommufd_hw_pagetable_has_group(hwpt, idev->igroup)) {
376+
if (list_empty(&idev->igroup->device_list)) {
379377
rc = iommu_attach_group(hwpt->domain, idev->igroup->group);
380378
if (rc)
381379
goto err_unresv;
380+
idev->igroup->hwpt = hwpt;
382381
}
382+
refcount_inc(&hwpt->obj.users);
383+
list_add_tail(&idev->group_item, &idev->igroup->device_list);
384+
mutex_unlock(&idev->igroup->lock);
383385
return 0;
384386
err_unresv:
385387
iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->dev);
388+
err_unlock:
389+
mutex_unlock(&idev->igroup->lock);
386390
return rc;
387391
}
388392

389-
void iommufd_hw_pagetable_detach(struct iommufd_hw_pagetable *hwpt,
390-
struct iommufd_device *idev)
393+
struct iommufd_hw_pagetable *
394+
iommufd_hw_pagetable_detach(struct iommufd_device *idev)
391395
{
392-
if (!iommufd_hw_pagetable_has_group(hwpt, idev->igroup))
396+
struct iommufd_hw_pagetable *hwpt = idev->igroup->hwpt;
397+
398+
mutex_lock(&idev->igroup->lock);
399+
list_del(&idev->group_item);
400+
if (list_empty(&idev->igroup->device_list)) {
393401
iommu_detach_group(hwpt->domain, idev->igroup->group);
402+
idev->igroup->hwpt = NULL;
403+
}
394404
iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->dev);
395-
}
396-
397-
static int iommufd_device_do_attach(struct iommufd_device *idev,
398-
struct iommufd_hw_pagetable *hwpt)
399-
{
400-
int rc;
401-
402-
mutex_lock(&hwpt->devices_lock);
403-
rc = iommufd_hw_pagetable_attach(hwpt, idev);
404-
if (rc)
405-
goto out_unlock;
405+
mutex_unlock(&idev->igroup->lock);
406406

407-
idev->hwpt = hwpt;
408-
refcount_inc(&hwpt->obj.users);
409-
list_add(&idev->devices_item, &hwpt->devices);
410-
out_unlock:
411-
mutex_unlock(&hwpt->devices_lock);
412-
return rc;
407+
/* Caller must destroy hwpt */
408+
return hwpt;
413409
}
414410

415411
/*
@@ -418,7 +414,7 @@ static int iommufd_device_do_attach(struct iommufd_device *idev,
418414
* Automatic domain selection will never pick a manually created domain.
419415
*/
420416
static int iommufd_device_auto_get_domain(struct iommufd_device *idev,
421-
struct iommufd_ioas *ioas)
417+
struct iommufd_ioas *ioas, u32 *pt_id)
422418
{
423419
struct iommufd_hw_pagetable *hwpt;
424420
int rc;
@@ -435,7 +431,7 @@ static int iommufd_device_auto_get_domain(struct iommufd_device *idev,
435431

436432
if (!iommufd_lock_obj(&hwpt->obj))
437433
continue;
438-
rc = iommufd_device_do_attach(idev, hwpt);
434+
rc = iommufd_hw_pagetable_attach(hwpt, idev);
439435
iommufd_put_object(&hwpt->obj);
440436

441437
/*
@@ -445,6 +441,7 @@ static int iommufd_device_auto_get_domain(struct iommufd_device *idev,
445441
*/
446442
if (rc == -EINVAL)
447443
continue;
444+
*pt_id = hwpt->obj.id;
448445
goto out_unlock;
449446
}
450447

@@ -454,6 +451,7 @@ static int iommufd_device_auto_get_domain(struct iommufd_device *idev,
454451
goto out_unlock;
455452
}
456453
hwpt->auto_domain = true;
454+
*pt_id = hwpt->obj.id;
457455

458456
mutex_unlock(&ioas->mutex);
459457
iommufd_object_finalize(idev->ictx, &hwpt->obj);
@@ -489,7 +487,7 @@ int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id)
489487
struct iommufd_hw_pagetable *hwpt =
490488
container_of(pt_obj, struct iommufd_hw_pagetable, obj);
491489

492-
rc = iommufd_device_do_attach(idev, hwpt);
490+
rc = iommufd_hw_pagetable_attach(hwpt, idev);
493491
if (rc)
494492
goto out_put_pt_obj;
495493
break;
@@ -498,7 +496,7 @@ int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id)
498496
struct iommufd_ioas *ioas =
499497
container_of(pt_obj, struct iommufd_ioas, obj);
500498

501-
rc = iommufd_device_auto_get_domain(idev, ioas);
499+
rc = iommufd_device_auto_get_domain(idev, ioas, pt_id);
502500
if (rc)
503501
goto out_put_pt_obj;
504502
break;
@@ -509,7 +507,6 @@ int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id)
509507
}
510508

511509
refcount_inc(&idev->obj.users);
512-
*pt_id = idev->hwpt->obj.id;
513510
rc = 0;
514511

515512
out_put_pt_obj:
@@ -527,14 +524,9 @@ EXPORT_SYMBOL_NS_GPL(iommufd_device_attach, IOMMUFD);
527524
*/
528525
void iommufd_device_detach(struct iommufd_device *idev)
529526
{
530-
struct iommufd_hw_pagetable *hwpt = idev->hwpt;
531-
532-
mutex_lock(&hwpt->devices_lock);
533-
list_del(&idev->devices_item);
534-
idev->hwpt = NULL;
535-
iommufd_hw_pagetable_detach(hwpt, idev);
536-
mutex_unlock(&hwpt->devices_lock);
527+
struct iommufd_hw_pagetable *hwpt;
537528

529+
hwpt = iommufd_hw_pagetable_detach(idev);
538530
if (hwpt->auto_domain)
539531
iommufd_object_destroy_user(idev->ictx, &hwpt->obj);
540532
else

‎drivers/iommu/iommufd/hw_pagetable.c

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ void iommufd_hw_pagetable_destroy(struct iommufd_object *obj)
1111
struct iommufd_hw_pagetable *hwpt =
1212
container_of(obj, struct iommufd_hw_pagetable, obj);
1313

14-
WARN_ON(!list_empty(&hwpt->devices));
15-
1614
if (!list_empty(&hwpt->hwpt_item)) {
1715
mutex_lock(&hwpt->ioas->mutex);
1816
list_del(&hwpt->hwpt_item);
@@ -25,7 +23,6 @@ void iommufd_hw_pagetable_destroy(struct iommufd_object *obj)
2523
iommu_domain_free(hwpt->domain);
2624

2725
refcount_dec(&hwpt->ioas->obj.users);
28-
mutex_destroy(&hwpt->devices_lock);
2926
}
3027

3128
/**
@@ -52,9 +49,7 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
5249
if (IS_ERR(hwpt))
5350
return hwpt;
5451

55-
INIT_LIST_HEAD(&hwpt->devices);
5652
INIT_LIST_HEAD(&hwpt->hwpt_item);
57-
mutex_init(&hwpt->devices_lock);
5853
/* Pairs with iommufd_hw_pagetable_destroy() */
5954
refcount_inc(&ioas->obj.users);
6055
hwpt->ioas = ioas;
@@ -65,8 +60,6 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
6560
goto out_abort;
6661
}
6762

68-
mutex_lock(&hwpt->devices_lock);
69-
7063
/*
7164
* immediate_attach exists only to accommodate iommu drivers that cannot
7265
* directly allocate a domain. These drivers do not finish creating the
@@ -76,29 +69,18 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
7669
if (immediate_attach) {
7770
rc = iommufd_hw_pagetable_attach(hwpt, idev);
7871
if (rc)
79-
goto out_unlock;
72+
goto out_abort;
8073
}
8174

8275
rc = iopt_table_add_domain(&hwpt->ioas->iopt, hwpt->domain);
8376
if (rc)
8477
goto out_detach;
8578
list_add_tail(&hwpt->hwpt_item, &hwpt->ioas->hwpt_list);
86-
87-
if (immediate_attach) {
88-
/* See iommufd_device_do_attach() */
89-
refcount_inc(&hwpt->obj.users);
90-
idev->hwpt = hwpt;
91-
list_add(&idev->devices_item, &hwpt->devices);
92-
}
93-
94-
mutex_unlock(&hwpt->devices_lock);
9579
return hwpt;
9680

9781
out_detach:
9882
if (immediate_attach)
99-
iommufd_hw_pagetable_detach(hwpt, idev);
100-
out_unlock:
101-
mutex_unlock(&hwpt->devices_lock);
83+
iommufd_hw_pagetable_detach(idev);
10284
out_abort:
10385
iommufd_object_abort_and_destroy(ictx, &hwpt->obj);
10486
return ERR_PTR(rc);

‎drivers/iommu/iommufd/iommufd_private.h

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -250,23 +250,24 @@ struct iommufd_hw_pagetable {
250250
bool msi_cookie : 1;
251251
/* Head at iommufd_ioas::hwpt_list */
252252
struct list_head hwpt_item;
253-
struct mutex devices_lock;
254-
struct list_head devices;
255253
};
256254

257255
struct iommufd_hw_pagetable *
258256
iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
259257
struct iommufd_device *idev, bool immediate_attach);
260258
int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
261259
struct iommufd_device *idev);
262-
void iommufd_hw_pagetable_detach(struct iommufd_hw_pagetable *hwpt,
263-
struct iommufd_device *idev);
260+
struct iommufd_hw_pagetable *
261+
iommufd_hw_pagetable_detach(struct iommufd_device *idev);
264262
void iommufd_hw_pagetable_destroy(struct iommufd_object *obj);
265263

266264
struct iommufd_group {
267265
struct kref ref;
266+
struct mutex lock;
268267
struct iommufd_ctx *ictx;
269268
struct iommu_group *group;
269+
struct iommufd_hw_pagetable *hwpt;
270+
struct list_head device_list;
270271
};
271272

272273
/*
@@ -278,9 +279,7 @@ struct iommufd_device {
278279
struct iommufd_object obj;
279280
struct iommufd_ctx *ictx;
280281
struct iommufd_group *igroup;
281-
struct iommufd_hw_pagetable *hwpt;
282-
/* Head at iommufd_hw_pagetable::devices */
283-
struct list_head devices_item;
282+
struct list_head group_item;
284283
/* always the physical device */
285284
struct device *dev;
286285
bool enforce_cache_coherency;

0 commit comments

Comments
 (0)