On Thu, Jan 10, 2019 at 11:36:27AM +0200, Gal Pressman 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 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 | 38 +++++++++++++++++++++--------------
> drivers/infiniband/core/uverbs_main.c | 1 +
> include/rdma/ib_verbs.h | 5 +++++
> 3 files changed, 29 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 8872453e26c0..6b3f06d6c3dc 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) {
> + 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)
This check should be in add_client_context() to avoid duplication.
> + 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;
>
> 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/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
>
Attachment:
signature.asc
Description: PGP signature
