>-----Original Message----- >From: Jason Gunthorpe [mailto:jgg@xxxxxxxx] >Sent: Friday, August 3, 2018 3:32 PM >To: linux-rdma@xxxxxxxxxxxxxxx; Leon Romanovsky <leonro@xxxxxxxxxxxx>; >Guy Levi(SW) <guyle@xxxxxxxxxxxx>; Yishai Hadas ><yishaih@xxxxxxxxxxxx>; Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx> >Cc: Jason Gunthorpe <jgg@xxxxxxxxxxxx> >Subject: [PATCH 01/10] IB/uverbs: Have the core code create the >uverbs_root_spec > >From: Jason Gunthorpe <jgg@xxxxxxxxxxxx> > >There is no reason for drivers to do this, the core code should take of >everything. The drivers will provide their information from rodata to >describe their modifications to the core's base uapi specification. > >The core uses this to build up the runtime uapi for each device. > >Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx> >--- > drivers/infiniband/core/uverbs_ioctl_merge.c | 2 - > drivers/infiniband/core/uverbs_main.c | 50 +++++++++++++------- > drivers/infiniband/core/uverbs_std_types.c | 1 - > drivers/infiniband/hw/mlx5/main.c | 45 +++++++----------- > drivers/infiniband/hw/mlx5/mlx5_ib.h | 1 + > include/rdma/ib_verbs.h | 2 +- > 6 files changed, 51 insertions(+), 50 deletions(-) > >diff --git a/drivers/infiniband/core/uverbs_ioctl_merge.c >b/drivers/infiniband/core/uverbs_ioctl_merge.c >index f81aa888ce5c4c..16b57592991581 100644 >--- a/drivers/infiniband/core/uverbs_ioctl_merge.c >+++ b/drivers/infiniband/core/uverbs_ioctl_merge.c >@@ -556,7 +556,6 @@ void uverbs_free_spec_tree(struct uverbs_root_spec >*root) > > kfree(root); > } >-EXPORT_SYMBOL(uverbs_free_spec_tree); > > struct uverbs_root_spec *uverbs_alloc_spec_tree(unsigned int num_trees, > const struct >uverbs_object_tree_def **trees) >@@ -661,4 +660,3 @@ struct uverbs_root_spec >*uverbs_alloc_spec_tree(unsigned int num_trees, > uverbs_free_spec_tree(root_spec); > return ERR_PTR(res); > } >-EXPORT_SYMBOL(uverbs_alloc_spec_tree); >diff --git a/drivers/infiniband/core/uverbs_main.c >b/drivers/infiniband/core/uverbs_main.c >index 6f62146e9738a0..20003594b5d6a7 100644 >--- a/drivers/infiniband/core/uverbs_main.c >+++ b/drivers/infiniband/core/uverbs_main.c >@@ -994,6 +994,36 @@ static DEVICE_ATTR(abi_version, S_IRUGO, >show_dev_abi_version, NULL); > static CLASS_ATTR_STRING(abi_version, S_IRUGO, > __stringify(IB_USER_VERBS_ABI_VERSION)); > >+static int ib_uverbs_create_uapi(struct ib_device *device, >+ struct ib_uverbs_device *uverbs_dev) >+{ >+ const struct uverbs_object_tree_def **specs; >+ struct uverbs_root_spec *specs_root; >+ unsigned int num_specs = 1; >+ unsigned int i; >+ >+ if (device->driver_specs) >+ for (i = 0; device->driver_specs[i]; i++) >+ num_specs++; >+ >+ specs = kmalloc_array(num_specs, sizeof(*specs), GFP_KERNEL); >+ if (!specs) >+ return -ENOMEM; >+ >+ specs[0] = uverbs_default_get_objects(); >+ if (device->driver_specs) >+ for (i = 0; device->driver_specs[i]; i++) >+ specs[i+1] = device->driver_specs[i]; >+ >+ specs_root = uverbs_alloc_spec_tree(num_specs, specs); >+ kfree(specs); >+ if (IS_ERR(specs_root)) >+ return PTR_ERR(specs_root); >+ >+ uverbs_dev->specs_root = specs_root; >+ return 0; >+} >+ > static void ib_uverbs_add_one(struct ib_device *device) > { > int devnum; >@@ -1036,6 +1066,9 @@ static void ib_uverbs_add_one(struct ib_device >*device) > rcu_assign_pointer(uverbs_dev->ib_dev, device); > uverbs_dev->num_comp_vectors = device->num_comp_vectors; > >+ if (ib_uverbs_create_uapi(device, uverbs_dev)) >+ goto err; >+ > cdev_init(&uverbs_dev->cdev, NULL); > uverbs_dev->cdev.owner = THIS_MODULE; > uverbs_dev->cdev.ops = device->mmap ? &uverbs_mmap_fops : >&uverbs_fops; >@@ -1055,23 +1088,6 @@ static void ib_uverbs_add_one(struct ib_device >*device) > if (device_create_file(uverbs_dev->dev, &dev_attr_abi_version)) > goto err_class; > >- if (!device->driver_specs_root) { >- const struct uverbs_object_tree_def *default_root[] = { >- uverbs_default_get_objects()}; >- >- uverbs_dev->specs_root = uverbs_alloc_spec_tree(1, >- default_root); >- if (IS_ERR(uverbs_dev->specs_root)) >- goto err_class; >- } else { >- uverbs_dev->specs_root = device->driver_specs_root; >- /* >- * Take responsibility to free the specs allocated by the >- * driver. >- */ >- device->driver_specs_root = NULL; >- } >- > ib_set_client_data(device, &uverbs_client, uverbs_dev); > > return; >diff --git a/drivers/infiniband/core/uverbs_std_types.c >b/drivers/infiniband/core/uverbs_std_types.c >index 3aa7c7deac749a..7f22b820a21ba0 100644 >--- a/drivers/infiniband/core/uverbs_std_types.c >+++ b/drivers/infiniband/core/uverbs_std_types.c >@@ -316,4 +316,3 @@ const struct uverbs_object_tree_def >*uverbs_default_get_objects(void) > { > return &uverbs_default_objects; > } >-EXPORT_SYMBOL_GPL(uverbs_default_get_objects); That is tremendously cleaner than the original path. This comment in uverbs_alloc_spec_tree() that seems to imply that device can create a parse tree that will disable this interface: /* * Devices which don't want to support ib_uverbs, should just allocate * an empty parsing tree. Every user-space command won't hit any valid * entry in the parsing tree and thus will fail. */ It seems that this patch would break this ability. Is this correct, or should the comment be updated/removed? If you would like it: Reviewed-by: Michael J. Ruhl <michael.j.ruhl@xxxxxxxxx> >diff --git a/drivers/infiniband/hw/mlx5/main.c >b/drivers/infiniband/hw/mlx5/main.c >index 13744b4631b452..f86d831ee27c5d 100644 >--- a/drivers/infiniband/hw/mlx5/main.c >+++ b/drivers/infiniband/hw/mlx5/main.c >@@ -5523,37 +5523,29 @@ ADD_UVERBS_ATTRIBUTES_SIMPLE( > > UVERBS_ATTR_FLAGS_IN(MLX5_IB_ATTR_CREATE_FLOW_ACTION_F >LAGS, > enum mlx5_ib_uapi_flow_action_flags)); > >-#define NUM_TREES 5 > static int populate_specs_root(struct mlx5_ib_dev *dev) > { >- const struct uverbs_object_tree_def *default_root[NUM_TREES + 1] >= { >- uverbs_default_get_objects()}; >- size_t num_trees = 1; >+ const struct uverbs_object_tree_def **trees = dev->driver_trees; >+ size_t num_trees = 0; > >- if (mlx5_accel_ipsec_device_caps(dev->mdev) & >MLX5_ACCEL_IPSEC_CAP_DEVICE && >- !WARN_ON(num_trees >= ARRAY_SIZE(default_root))) >- default_root[num_trees++] = &mlx5_ib_flow_action; >+ if (mlx5_accel_ipsec_device_caps(dev->mdev) & >+ MLX5_ACCEL_IPSEC_CAP_DEVICE) >+ trees[num_trees++] = &mlx5_ib_flow_action; > >- if (MLX5_CAP_DEV_MEM(dev->mdev, memic) && >- !WARN_ON(num_trees >= ARRAY_SIZE(default_root))) >- default_root[num_trees++] = &mlx5_ib_dm; >+ if (MLX5_CAP_DEV_MEM(dev->mdev, memic)) >+ trees[num_trees++] = &mlx5_ib_dm; > > if (MLX5_CAP_GEN_64(dev->mdev, general_obj_types) & >- MLX5_GENERAL_OBJ_TYPES_CAP_UCTX && >- !WARN_ON(num_trees >= ARRAY_SIZE(default_root))) >- default_root[num_trees++] = mlx5_ib_get_devx_tree(); >+ MLX5_GENERAL_OBJ_TYPES_CAP_UCTX) >+ trees[num_trees++] = mlx5_ib_get_devx_tree(); > >- num_trees += mlx5_ib_get_flow_trees(default_root + num_trees); >+ num_trees += mlx5_ib_get_flow_trees(trees + num_trees); > >- dev->ib_dev.driver_specs_root = >- uverbs_alloc_spec_tree(num_trees, default_root); >+ WARN_ON(num_trees >= ARRAY_SIZE(dev->driver_trees)); >+ trees[num_trees] = NULL; >+ dev->ib_dev.driver_specs = trees; > >- return PTR_ERR_OR_ZERO(dev->ib_dev.driver_specs_root); >-} >- >-static void depopulate_specs_root(struct mlx5_ib_dev *dev) >-{ >- uverbs_free_spec_tree(dev->ib_dev.driver_specs_root); >+ return 0; > } > > static int mlx5_ib_read_counters(struct ib_counters *counters, >@@ -6092,11 +6084,6 @@ int mlx5_ib_stage_ib_reg_init(struct mlx5_ib_dev >*dev) > return ib_register_device(&dev->ib_dev, NULL); > } > >-static void mlx5_ib_stage_depopulate_specs(struct mlx5_ib_dev *dev) >-{ >- depopulate_specs_root(dev); >-} >- > void mlx5_ib_stage_pre_ib_reg_umr_cleanup(struct mlx5_ib_dev *dev) > { > destroy_umrc_res(dev); >@@ -6231,7 +6218,7 @@ static const struct mlx5_ib_profile pf_profile = { > mlx5_ib_stage_pre_ib_reg_umr_cleanup), > STAGE_CREATE(MLX5_IB_STAGE_SPECS, > mlx5_ib_stage_populate_specs, >- mlx5_ib_stage_depopulate_specs), >+ NULL), > STAGE_CREATE(MLX5_IB_STAGE_IB_REG, > mlx5_ib_stage_ib_reg_init, > mlx5_ib_stage_ib_reg_cleanup), >@@ -6279,7 +6266,7 @@ static const struct mlx5_ib_profile nic_rep_profile = { > mlx5_ib_stage_pre_ib_reg_umr_cleanup), > STAGE_CREATE(MLX5_IB_STAGE_SPECS, > mlx5_ib_stage_populate_specs, >- mlx5_ib_stage_depopulate_specs), >+ NULL), > STAGE_CREATE(MLX5_IB_STAGE_IB_REG, > mlx5_ib_stage_ib_reg_init, > mlx5_ib_stage_ib_reg_cleanup), >diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h >b/drivers/infiniband/hw/mlx5/mlx5_ib.h >index b75754efc66309..320d4dfe8c2f41 100644 >--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h >+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h >@@ -860,6 +860,7 @@ to_mcounters(struct ib_counters *ibcntrs) > > struct mlx5_ib_dev { > struct ib_device ib_dev; >+ const struct uverbs_object_tree_def *driver_trees[6]; > struct mlx5_core_dev *mdev; > struct mlx5_roce roce[MLX5_MAX_PORTS]; > int num_ports; >diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h >index 4ffe3e11e8fb6c..3b07201b9a804e 100644 >--- a/include/rdma/ib_verbs.h >+++ b/include/rdma/ib_verbs.h >@@ -2580,7 +2580,7 @@ struct ib_device { > const struct cpumask *(*get_vector_affinity)(struct ib_device *ibdev, > int comp_vector); > >- struct uverbs_root_spec *driver_specs_root; >+ const struct uverbs_object_tree_def *const *driver_specs; > enum rdma_driver_id driver_id; > }; > >-- >2.18.0 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
