| Message ID | 1547112988-7089-2-git-send-email-galpress@amazon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | IB device in-kernel API support indication | expand |
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@amazon.com> > --- > 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 >
On 10-Jan-19 12:59, Leon Romanovsky wrote: >> @@ -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. Thanks, will do.
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) + 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);
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@amazon.com> --- 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(-)