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

Reply via email to