On Tue, Sep 26, 2023 at 01:14:47AM -0700, Tian, Kevin wrote:
From: Liu, Yi L yi.l.liu@intel.com Sent: Thursday, September 21, 2023 3:51 PM
+static void iommufd_user_managed_hwpt_abort(struct iommufd_object *obj) +{
struct iommufd_hw_pagetable *hwpt =container_of(obj, struct iommufd_hw_pagetable, obj);/* The parent->mutex must be held until finalize is called. */lockdep_assert_held(&hwpt->parent->mutex);iommufd_hw_pagetable_destroy(obj);+}
Can you elaborate what exactly is protected by parent->mutex?
My gut-feeling that the child just grabs a refcnt on the parent object. It doesn't change any other data of the parent.
Ah, you are right. It's added here just for symmetry so we wouldn't end up with something like: if (!hwpt->user_managed) mutex_lock(&hwpt->mutex); alloc_fn(); if (!hwpt->user_managed) mutex_unlock(&hwpt->mutex);
Perhaps I should move the pair of mutex calls to the kernel-managed hwpt allocator.
+/**
- iommufd_user_managed_hwpt_alloc() - Get a user-managed
hw_pagetable
- @ictx: iommufd context
- @pt_obj: Parent object to an HWPT to associate the domain with
- @idev: Device to get an iommu_domain for
- @flags: Flags from userspace
- @hwpt_type: Requested type of hw_pagetable
- @user_data: user_data pointer
- @dummy: never used
- Allocate a new iommu_domain (must be IOMMU_DOMAIN_NESTED) and
return it as
- a user-managed hw_pagetable.
Add some text to highlight the requirement being a parent, e.g. not an auto domain, must be capable of being a parent, etc.
OK.
case IOMMUFD_OBJ_HW_PAGETABLE:parent = container_of(pt_obj, struct iommufd_hw_pagetable,obj);
/* No user-managed HWPT on top of an user-managed one*/
if (parent->user_managed) {rc = -EINVAL;goto out_put_pt;}move to alloc_fn().
Though being a bit covert, this is actually to avoid a data buffer allocation in the common pathway before calling alloc_fn(), which is added in the following patch. And the reason why it's in the common function is because we previously support a kernel-managed hwpt allocation with data too.
But now, I think we can just move this sanity and data allocation together into the user-managed hwpt allocator.
Thanks Nicolin