> On Jan 8, 2019, at 2:42 PM, Gal Pressman <galpress@xxxxxxxxxx> wrote:
>
> Drivers that do not provide kernel verbs support should not be used by
> ib kernel clients and fail.
> In case a device does not implement all mandatory verbs for kverbs usage
> mark it as a non kverbs provider and prevent its usage for all clients
> except for uverbs.
>
> The device is marked as a non kverbs provider using the
> 'kverbs_provider' flag which should only be set by the core code.
> The clients can choose whether kverbs are requested for it
its
> usage using
> the 'no_kverbs_req' flag which is currently set for uverbs only.
>
> This patch allows drivers to remove mandatory verbs stubs and simply set
> the callback to NULL. The IB device will be registered as a non-kverbs
> provider.
>
> Signed-off-by: Gal Pressman <galpress@xxxxxxxxxx>
> ---
> drivers/infiniband/core/device.c | 41 ++++++++++++++++++++++-------------
> drivers/infiniband/core/uverbs_main.c | 1 +
> drivers/infiniband/core/verbs.c | 28 +++++++++++++++++++++---
> drivers/infiniband/hw/mlx5/main.c | 3 +++
> include/rdma/ib_verbs.h | 5 +++++
> 5 files changed, 60 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 8872453e26c0..156f1b2ebf16 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -121,13 +121,12 @@ static int ib_device_check_mandatory(struct ib_device *device)
> };
> int i;
>
> + device->kverbs_provider = true;
> for (i = 0; i < ARRAY_SIZE(mandatory_table); ++i) {
> if (!*(void **) ((void *) &device->ops +
> mandatory_table[i].offset)) {
> - dev_warn(&device->dev,
> - "Device is missing mandatory function %s\n",
> - mandatory_table[i].name);
> - return -EINVAL;
> + device->kverbs_provider = false;
> + break;
> }
> }
>
> @@ -374,10 +373,12 @@ static int read_port_immutable(struct ib_device *device)
> return -ENOMEM;
>
> for (port = start_port; port <= end_port; ++port) {
> - ret = device->ops.get_port_immutable(
> - device, port, &device->port_immutable[port]);
> - if (ret)
> - return ret;
> + if (device->ops.get_port_immutable) {
> + ret = device->ops.get_port_immutable(
> + device, port, &device->port_immutable[port]);
> + if (ret)
> + return ret;
> + }
>
> if (verify_immutable(device, port))
> return -EINVAL;
> @@ -537,11 +538,13 @@ static int setup_device(struct ib_device *device)
> }
>
> memset(&device->attrs, 0, sizeof(device->attrs));
> - ret = device->ops.query_device(device, &device->attrs, &uhw);
> - if (ret) {
> - dev_warn(&device->dev,
> - "Couldn't query the device attributes\n");
> - goto port_cleanup;
> + if (device->ops.query_device) {
Why do we need these kind of checks now?
In case device doesn’t implement all mandatory kverbs, then clients won’t add it.. uverbs has uverbs_cmd_mask in write path, and checks function pointer in the ioctl path..
Maybe I’m missing something..
> + ret = device->ops.query_device(device, &device->attrs, &uhw);
> + if (ret) {
> + dev_warn(&device->dev,
> + "Couldn't query the device attributes\n");
> + goto port_cleanup;
> + }
> }
>
> ret = setup_port_pkey_list(device);
> @@ -624,7 +627,8 @@ int ib_register_device(struct ib_device *device, const char *name,
>
> list_for_each_entry(client, &client_list, list)
> if (!add_client_context(device, client) && client->add)
> - client->add(device);
> + if (device->kverbs_provider || client->no_kverbs_req)
> + client->add(device);
>
> down_write(&lists_rwsem);
> list_add_tail(&device->core_list, &device_list);
> @@ -721,7 +725,8 @@ int ib_register_client(struct ib_client *client)
>
> list_for_each_entry(device, &device_list, core_list)
> if (!add_client_context(device, client) && client->add)
> - client->add(device);
> + if (device->kverbs_provider || client->no_kverbs_req)
> + client->add(device);
>
> down_write(&lists_rwsem);
> list_add_tail(&client->list, &client_list);
> @@ -920,6 +925,9 @@ int ib_query_port(struct ib_device *device,
> union ib_gid gid;
> int err;
>
> + if (!device->ops.query_port)
> + return -EOPNOTSUPP;
> +
> if (!rdma_is_port_valid(device, port_num))
> return -EINVAL;
>
> @@ -1043,6 +1051,9 @@ int ib_enum_all_devs(nldev_callback nldev_cb, struct sk_buff *skb,
> int ib_query_pkey(struct ib_device *device,
> u8 port_num, u16 index, u16 *pkey)
> {
> + if (!device->ops.query_pkey)
> + return -EOPNOTSUPP;
> +
> if (!rdma_is_port_valid(device, port_num))
> return -EINVAL;
>
> diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
> index fb0007aa0c27..0eafee9a2ffc 100644
> --- a/drivers/infiniband/core/uverbs_main.c
> +++ b/drivers/infiniband/core/uverbs_main.c
> @@ -1127,6 +1127,7 @@ static const struct file_operations uverbs_mmap_fops = {
>
> static struct ib_client uverbs_client = {
> .name = "uverbs",
> + .no_kverbs_req = true,
> .add = ib_uverbs_add_one,
> .remove = ib_uverbs_remove_one
> };
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index c51e2505a9ad..54be4521c235 100644
> --- a/drivers/infiniband/core/verbs.c
> +++ b/drivers/infiniband/core/verbs.c
> @@ -257,6 +257,10 @@ struct ib_pd *__ib_alloc_pd(struct ib_device *device, unsigned int flags,
> struct ib_pd *pd;
> int mr_access_flags = 0;
>
> + if (!device->ops.alloc_pd ||
> + (mr_access_flags && !pd->device->ops.get_dma_mr))
> + return ERR_PTR(-EOPNOTSUPP);
> +
> pd = device->ops.alloc_pd(device, NULL, NULL);
> if (IS_ERR(pd))
> return pd;
> @@ -320,6 +324,10 @@ void ib_dealloc_pd(struct ib_pd *pd)
> {
> int ret;
>
> + if (!pd->device->ops.dealloc_pd ||
> + (pd->__internal_mr && !pd->device->ops.dereg_mr))
> + return;
> +
> if (pd->__internal_mr) {
> ret = pd->device->ops.dereg_mr(pd->__internal_mr);
> WARN_ON(ret);
> @@ -1124,10 +1132,12 @@ static struct ib_qp *ib_create_xrc_qp(struct ib_qp *qp,
>
> qp = __ib_open_qp(real_qp, qp_init_attr->event_handler,
> qp_init_attr->qp_context);
> - if (!IS_ERR(qp))
> + if (!IS_ERR(qp)) {
> __ib_insert_xrcd_qp(qp_init_attr->xrcd, real_qp);
> - else
> - real_qp->device->ops.destroy_qp(real_qp);
> + } else {
> + if (real_qp->device->ops.destroy_qp)
> + real_qp->device->ops.destroy_qp(real_qp);
> + }
> return qp;
> }
>
> @@ -1604,6 +1614,9 @@ static int _ib_modify_qp(struct ib_qp *qp, struct ib_qp_attr *attr,
> const struct ib_gid_attr *old_sgid_attr_alt_av;
> int ret;
>
> + if (!qp->device->ops.modify_qp)
> + return -EOPNOTSUPP;
> +
> if (attr_mask & IB_QP_AV) {
> ret = rdma_fill_sgid_attr(qp->device, &attr->ah_attr,
> &old_sgid_attr_av);
> @@ -1841,6 +1854,9 @@ int ib_destroy_qp(struct ib_qp *qp)
> struct ib_qp_security *sec;
> int ret;
>
> + if (!qp->device->ops.destroy_qp)
> + return -EOPNOTSUPP;
> +
> WARN_ON_ONCE(qp->mrs_used > 0);
>
> if (atomic_read(&qp->usecnt))
> @@ -1928,6 +1944,9 @@ EXPORT_SYMBOL(rdma_set_cq_moderation);
>
> int ib_destroy_cq(struct ib_cq *cq)
> {
> + if (!cq->device->ops.destroy_cq)
> + return -EOPNOTSUPP;
> +
> if (atomic_read(&cq->usecnt))
> return -EBUSY;
>
> @@ -1951,6 +1970,9 @@ int ib_dereg_mr(struct ib_mr *mr)
> struct ib_dm *dm = mr->dm;
> int ret;
>
> + if (!mr->device->ops.dereg_mr)
> + return -EOPNOTSUPP;
> +
> rdma_restrack_del(&mr->res);
> ret = mr->device->ops.dereg_mr(mr);
> if (!ret) {
> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> index 94fe253d4956..072fd8be3355 100644
> --- a/drivers/infiniband/hw/mlx5/main.c
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -149,6 +149,9 @@ static int get_port_state(struct ib_device *ibdev,
> struct ib_port_attr attr;
> int ret;
>
> + if (!ibdev->ops.query_port)
> + return -EOPNOTSUPP;
> +
> memset(&attr, 0, sizeof(attr));
> ret = ibdev->ops.query_port(ibdev, port_num, &attr);
> if (!ret)
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index c073a4720d28..fb6074d2e394 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -2566,6 +2566,8 @@ struct ib_device {
> __be64 node_guid;
> u32 local_dma_lkey;
> u16 is_switch:1;
> + /* Indicates kernel verbs support, should not be used in drivers */
> + u8 kverbs_provider:1;
> u8 node_type;
> u8 phys_port_cnt;
> struct ib_device_attr attrs;
> @@ -2620,6 +2622,9 @@ struct ib_client {
> const struct sockaddr *addr,
> void *client_data);
> struct list_head list;
> +
> + /* kverbs are not required by the client */
> + u8 no_kverbs_req:1;
> };
>
> struct ib_device *ib_alloc_device(size_t size);
> --
> 2.7.4
>
