Messages in this thread | | | Date | Wed, 25 Oct 2023 18:19:46 +0800 | Subject | Re: [PATCH v6 07/10] iommufd: Add a nested HW pagetable object | From | Yi Liu <> |
| |
On 2023/10/25 01:30, Jason Gunthorpe wrote: > On Tue, Oct 24, 2023 at 02:18:10PM -0300, Jason Gunthorpe wrote: >> On Tue, Oct 24, 2023 at 08:06:06AM -0700, Yi Liu wrote: >>> @@ -195,6 +279,10 @@ int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd) >>> if (pt_obj->type == IOMMUFD_OBJ_IOAS) { >>> struct iommufd_hwpt_paging *hwpt_paging; >>> >>> + if (cmd->data_type != IOMMU_HWPT_DATA_NONE) { >>> + rc = -EINVAL; >>> + goto out_put_pt; >>> + } >>> ioas = container_of(pt_obj, struct iommufd_ioas, obj); >>> mutex_lock(&ioas->mutex); >>> hwpt_paging = iommufd_hwpt_paging_alloc(ucmd->ictx, ioas, idev, >> >> ?? What is this? >> >> Ah something went wrong earlier in "iommu: Pass in parent domain with >> user_data to domain_alloc_user op" > > Bah, I got confused because that had half the uapi, so in this pathc > >> Once we added the user_data we should flow it through to the op >> always. > > Like this:
ack.
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c > index 92859b907bb93c..a3382811af8a81 100644 > --- a/drivers/iommu/iommufd/device.c > +++ b/drivers/iommu/iommufd/device.c > @@ -586,8 +586,8 @@ iommufd_device_auto_get_domain(struct iommufd_device *idev, > goto out_unlock; > } > > - hwpt_paging = iommufd_hwpt_paging_alloc(idev->ictx, ioas, idev, > - 0, immediate_attach); > + hwpt_paging = iommufd_hwpt_paging_alloc(idev->ictx, ioas, idev, 0, > + immediate_attach, NULL); > if (IS_ERR(hwpt_paging)) { > destroy_hwpt = ERR_CAST(hwpt_paging); > goto out_unlock; > diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c > index cfd85df693d7b2..324a6d73f032ee 100644 > --- a/drivers/iommu/iommufd/hw_pagetable.c > +++ b/drivers/iommu/iommufd/hw_pagetable.c > @@ -96,7 +96,8 @@ iommufd_hwpt_paging_enforce_cc(struct iommufd_hwpt_paging *hwpt_paging) > struct iommufd_hwpt_paging * > iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas, > struct iommufd_device *idev, u32 flags, > - bool immediate_attach) > + bool immediate_attach, > + const struct iommu_user_data *user_data) > { > const u32 valid_flags = IOMMU_HWPT_ALLOC_NEST_PARENT | > IOMMU_HWPT_ALLOC_DIRTY_TRACKING; > @@ -107,7 +108,7 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas, > > lockdep_assert_held(&ioas->mutex); > > - if (flags && !ops->domain_alloc_user) > + if ((flags || user_data) && !ops->domain_alloc_user) > return ERR_PTR(-EOPNOTSUPP); > if (flags & ~valid_flags) > return ERR_PTR(-EOPNOTSUPP); > @@ -127,7 +128,7 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas, > > if (ops->domain_alloc_user) { > hwpt->domain = ops->domain_alloc_user(idev->dev, flags, > - NULL, NULL); > + NULL, user_data); > if (IS_ERR(hwpt->domain)) { > rc = PTR_ERR(hwpt->domain); > hwpt->domain = NULL; > @@ -210,8 +211,7 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx, > struct iommufd_hw_pagetable *hwpt; > int rc; > > - if (flags || user_data->type == IOMMU_HWPT_DATA_NONE || > - !ops->domain_alloc_user) > + if (flags || !user_data->len || !ops->domain_alloc_user) > return ERR_PTR(-EOPNOTSUPP); > if (parent->auto_domain || !parent->nest_parent) > return ERR_PTR(-EINVAL); > @@ -249,6 +249,11 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx, > int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd) > { > struct iommu_hwpt_alloc *cmd = ucmd->cmd; > + const struct iommu_user_data user_data = { > + .type = cmd->data_type, > + .uptr = u64_to_user_ptr(cmd->data_uptr), > + .len = cmd->data_len, > + }; > struct iommufd_hw_pagetable *hwpt; > struct iommufd_ioas *ioas = NULL; > struct iommufd_object *pt_obj; > @@ -273,25 +278,17 @@ int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd) > if (pt_obj->type == IOMMUFD_OBJ_IOAS) { > struct iommufd_hwpt_paging *hwpt_paging; > > - if (cmd->data_type != IOMMU_HWPT_DATA_NONE) { > - rc = -EINVAL; > - goto out_put_pt; > - }
it can be covered by iommu driver when checking user data pointer. hence remove above if.
> ioas = container_of(pt_obj, struct iommufd_ioas, obj); > mutex_lock(&ioas->mutex); > - hwpt_paging = iommufd_hwpt_paging_alloc(ucmd->ictx, ioas, idev, > - cmd->flags, false); > + hwpt_paging = iommufd_hwpt_paging_alloc( > + ucmd->ictx, ioas, idev, cmd->flags, false, > + user_data.len ? &user_data : NULL); > if (IS_ERR(hwpt_paging)) { > rc = PTR_ERR(hwpt_paging); > goto out_unlock; > } > hwpt = &hwpt_paging->common; > } else if (pt_obj->type == IOMMUFD_OBJ_HWPT_PAGING) { > - const struct iommu_user_data user_data = { > - .type = cmd->data_type, > - .uptr = u64_to_user_ptr(cmd->data_uptr), > - .len = cmd->data_len, > - }; > struct iommufd_hwpt_nested *hwpt_nested; > struct iommufd_hwpt_paging *parent; > > diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h > index 6fdad56893af4d..24e5a36fc875e0 100644 > --- a/drivers/iommu/iommufd/iommufd_private.h > +++ b/drivers/iommu/iommufd/iommufd_private.h > @@ -290,7 +290,8 @@ int iommufd_hwpt_get_dirty_bitmap(struct iommufd_ucmd *ucmd); > struct iommufd_hwpt_paging * > iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas, > struct iommufd_device *idev, u32 flags, > - bool immediate_attach); > + bool immediate_attach, > + const struct iommu_user_data *user_data); > int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt, > struct iommufd_device *idev); > struct iommufd_hw_pagetable *
-- Regards, Yi Liu
| |