IB/rdmavt: Add per verb driver callback checking

For each verb validate that all requirements for driver callbacks are met.
If a function is called without checking for a valid pointer, it is a
required function. Also document what each callback function does.

Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Doug Ledford <dledford@redhat.com>

drivers/infiniband/sw/rdmavt/vt.c

@@ -325,12 +325,375 @@ static int rvt_get_port_immutable(struct ib_device *ibdev, u8 port_num,
 	return 0;
 }
 
-/*
+enum {
- * Check driver override. If driver passes a value use it, otherwise we use our
+	MISC,
- * own value.
+	QUERY_DEVICE,
- */
+	MODIFY_DEVICE,
-#define CHECK_DRIVER_OVERRIDE(rdi, x) \
+	QUERY_PORT,
-	rdi->ibdev.x = rdi->ibdev.x ? : rvt_ ##x
+	MODIFY_PORT,
+	QUERY_PKEY,
+	QUERY_GID,
+	ALLOC_UCONTEXT,
+	DEALLOC_UCONTEXT,
+	GET_PORT_IMMUTABLE,
+	CREATE_QP,
+	MODIFY_QP,
+	DESTROY_QP,
+	QUERY_QP,
+	POST_SEND,
+	POST_RECV,
+	POST_SRQ_RECV,
+	CREATE_AH,
+	DESTROY_AH,
+	MODIFY_AH,
+	QUERY_AH,
+	CREATE_SRQ,
+	MODIFY_SRQ,
+	DESTROY_SRQ,
+	QUERY_SRQ,
+	ATTACH_MCAST,
+	DETACH_MCAST,
+	GET_DMA_MR,
+	REG_USER_MR,
+	DEREG_MR,
+	ALLOC_MR,
+	ALLOC_FMR,
+	MAP_PHYS_FMR,
+	UNMAP_FMR,
+	DEALLOC_FMR,
+	MMAP,
+	CREATE_CQ,
+	DESTROY_CQ,
+	POLL_CQ,
+	REQ_NOTFIY_CQ,
+	RESIZE_CQ,
+	ALLOC_PD,
+	DEALLOC_PD,
+	_VERB_IDX_MAX /* Must always be last! */
+};
+
+static inline int check_driver_override(struct rvt_dev_info *rdi,
+					size_t offset, void *func)
+{
+	if (!*(void **)((void *)&rdi->ibdev + offset)) {
+		*(void **)((void *)&rdi->ibdev + offset) = func;
+		return 0;
+	}
+
+	return 1;
+}
+
+static int check_support(struct rvt_dev_info *rdi, int verb)
+{
+	switch (verb) {
+	case MISC:
+		/*
+		 * These functions are not part of verbs specifically but are
+		 * required for rdmavt to function.
+		 */
+		if ((!rdi->driver_f.port_callback) ||
+		    (!rdi->driver_f.get_card_name) ||
+		    (!rdi->driver_f.get_pci_dev))
+			return -EINVAL;
+		break;
+
+	case QUERY_DEVICE:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    query_device),
+						    rvt_query_device);
+		break;
+
+	case MODIFY_DEVICE:
+		/*
+		 * rdmavt does not support modify device currently drivers must
+		 * provide.
+		 */
+		if (!check_driver_override(rdi, offsetof(struct ib_device,
+							 modify_device),
+					   rvt_modify_device))
+			return -EOPNOTSUPP;
+		break;
+
+	case QUERY_PORT:
+		if (!check_driver_override(rdi, offsetof(struct ib_device,
+							 query_port),
+					   rvt_query_port))
+			if (!rdi->driver_f.query_port_state)
+				return -EINVAL;
+		break;
+
+	case MODIFY_PORT:
+		if (!check_driver_override(rdi, offsetof(struct ib_device,
+							 modify_port),
+					   rvt_modify_port))
+			if (!rdi->driver_f.cap_mask_chg ||
+			    !rdi->driver_f.shut_down_port)
+				return -EINVAL;
+		break;
+
+	case QUERY_PKEY:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    query_pkey),
+				      rvt_query_pkey);
+		break;
+
+	case QUERY_GID:
+		if (!check_driver_override(rdi, offsetof(struct ib_device,
+							 query_gid),
+					   rvt_query_gid))
+			if (!rdi->driver_f.get_guid_be)
+				return -EINVAL;
+		break;
+
+	case ALLOC_UCONTEXT:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    alloc_ucontext),
+				      rvt_alloc_ucontext);
+		break;
+
+	case DEALLOC_UCONTEXT:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    dealloc_ucontext),
+				      rvt_dealloc_ucontext);
+		break;
+
+	case GET_PORT_IMMUTABLE:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    get_port_immutable),
+				      rvt_get_port_immutable);
+		break;
+
+	case CREATE_QP:
+		if (!check_driver_override(rdi, offsetof(struct ib_device,
+							 create_qp),
+					   rvt_create_qp))
+			if (!rdi->driver_f.qp_priv_alloc ||
+			    !rdi->driver_f.qp_priv_free ||
+			    !rdi->driver_f.notify_qp_reset ||
+			    !rdi->driver_f.flush_qp_waiters ||
+			    !rdi->driver_f.stop_send_queue ||
+			    !rdi->driver_f.quiesce_qp)
+				return -EINVAL;
+		break;
+
+	case MODIFY_QP:
+		if (!check_driver_override(rdi, offsetof(struct ib_device,
+							 modify_qp),
+					   rvt_modify_qp))
+			if (!rdi->driver_f.notify_qp_reset ||
+			    !rdi->driver_f.schedule_send ||
+			    !rdi->driver_f.get_pmtu_from_attr ||
+			    !rdi->driver_f.flush_qp_waiters ||
+			    !rdi->driver_f.stop_send_queue ||
+			    !rdi->driver_f.quiesce_qp ||
+			    !rdi->driver_f.notify_error_qp ||
+			    !rdi->driver_f.mtu_from_qp ||
+			    !rdi->driver_f.mtu_to_path_mtu ||
+			    !rdi->driver_f.shut_down_port ||
+			    !rdi->driver_f.cap_mask_chg)
+				return -EINVAL;
+		break;
+
+	case DESTROY_QP:
+		if (!check_driver_override(rdi, offsetof(struct ib_device,
+							 destroy_qp),
+					   rvt_destroy_qp))
+			if (!rdi->driver_f.qp_priv_free ||
+			    !rdi->driver_f.notify_qp_reset ||
+			    !rdi->driver_f.flush_qp_waiters ||
+			    !rdi->driver_f.stop_send_queue ||
+			    !rdi->driver_f.quiesce_qp)
+				return -EINVAL;
+		break;
+
+	case QUERY_QP:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    query_qp),
+						    rvt_query_qp);
+		break;
+
+	case POST_SEND:
+		if (!check_driver_override(rdi, offsetof(struct ib_device,
+							 post_send),
+					   rvt_post_send))
+			if (!rdi->driver_f.schedule_send ||
+			    !rdi->driver_f.do_send)
+				return -EINVAL;
+		break;
+
+	case POST_RECV:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    post_recv),
+				      rvt_post_recv);
+		break;
+	case POST_SRQ_RECV:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    post_srq_recv),
+				      rvt_post_srq_recv);
+		break;
+
+	case CREATE_AH:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    create_ah),
+				      rvt_create_ah);
+		break;
+
+	case DESTROY_AH:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    destroy_ah),
+				      rvt_destroy_ah);
+		break;
+
+	case MODIFY_AH:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    modify_ah),
+				      rvt_modify_ah);
+		break;
+
+	case QUERY_AH:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    query_ah),
+				      rvt_query_ah);
+		break;
+
+	case CREATE_SRQ:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    create_srq),
+				      rvt_create_srq);
+		break;
+
+	case MODIFY_SRQ:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    modify_srq),
+				      rvt_modify_srq);
+		break;
+
+	case DESTROY_SRQ:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    destroy_srq),
+				      rvt_destroy_srq);
+		break;
+
+	case QUERY_SRQ:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    query_srq),
+				      rvt_query_srq);
+		break;
+
+	case ATTACH_MCAST:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    attach_mcast),
+				      rvt_attach_mcast);
+		break;
+
+	case DETACH_MCAST:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    detach_mcast),
+				      rvt_detach_mcast);
+		break;
+
+	case GET_DMA_MR:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    get_dma_mr),
+				      rvt_get_dma_mr);
+		break;
+
+	case REG_USER_MR:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    reg_user_mr),
+				      rvt_reg_user_mr);
+		break;
+
+	case DEREG_MR:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    dereg_mr),
+				      rvt_dereg_mr);
+		break;
+
+	case ALLOC_FMR:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    alloc_fmr),
+				      rvt_alloc_fmr);
+		break;
+
+	case ALLOC_MR:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    alloc_mr),
+				      rvt_alloc_mr);
+		break;
+
+	case MAP_PHYS_FMR:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    map_phys_fmr),
+				      rvt_map_phys_fmr);
+		break;
+
+	case UNMAP_FMR:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    unmap_fmr),
+				      rvt_unmap_fmr);
+		break;
+
+	case DEALLOC_FMR:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    dealloc_fmr),
+				      rvt_dealloc_fmr);
+		break;
+
+	case MMAP:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    mmap),
+				      rvt_mmap);
+		break;
+
+	case CREATE_CQ:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    create_cq),
+				      rvt_create_cq);
+		break;
+
+	case DESTROY_CQ:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    destroy_cq),
+				      rvt_destroy_cq);
+		break;
+
+	case POLL_CQ:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    poll_cq),
+				      rvt_poll_cq);
+		break;
+
+	case REQ_NOTFIY_CQ:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    req_notify_cq),
+				      rvt_req_notify_cq);
+		break;
+
+	case RESIZE_CQ:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    resize_cq),
+				      rvt_resize_cq);
+		break;
+
+	case ALLOC_PD:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    alloc_pd),
+				      rvt_alloc_pd);
+		break;
+
+	case DEALLOC_PD:
+		check_driver_override(rdi, offsetof(struct ib_device,
+						    dealloc_pd),
+				      rvt_dealloc_pd);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
 
 /**
  * rvt_register_device - register a driver
@@ -343,35 +706,26 @@ static int rvt_get_port_immutable(struct ib_device *ibdev, u8 port_num,
  */
 int rvt_register_device(struct rvt_dev_info *rdi)
 {
-	/* Validate that drivers have provided the right information */
+	int ret = 0, i;
-	int ret = 0;
 
 	if (!rdi)
 		return -EINVAL;
 
-	if ((!rdi->driver_f.port_callback) ||
+	/*
-	    (!rdi->driver_f.get_card_name) ||
+	 * Check to ensure drivers have setup the required helpers for the verbs
-	    (!rdi->driver_f.get_pci_dev) ||
+	 * they want rdmavt to handle
-	    (!rdi->driver_f.check_ah)) {
+	 */
-		pr_err("Driver not supporting req func\n");
+	for (i = 0; i < _VERB_IDX_MAX; i++)
-		return -EINVAL;
+		if (check_support(rdi, i)) {
-	}
+			pr_err("Driver support req not met at %d\n", i);
+			return -EINVAL;
+		}
+
 
 	/* Once we get past here we can use rvt_pr macros and tracepoints */
 	trace_rvt_dbg(rdi, "Driver attempting registration");
 	rvt_mmap_init(rdi);
 
-	/* Dev Ops */
-	CHECK_DRIVER_OVERRIDE(rdi, query_device);
-	CHECK_DRIVER_OVERRIDE(rdi, modify_device);
-	CHECK_DRIVER_OVERRIDE(rdi, query_port);
-	CHECK_DRIVER_OVERRIDE(rdi, modify_port);
-	CHECK_DRIVER_OVERRIDE(rdi, query_pkey);
-	CHECK_DRIVER_OVERRIDE(rdi, query_gid);
-	CHECK_DRIVER_OVERRIDE(rdi, alloc_ucontext);
-	CHECK_DRIVER_OVERRIDE(rdi, dealloc_ucontext);
-	CHECK_DRIVER_OVERRIDE(rdi, get_port_immutable);
-
 	/* Queue Pairs */
 	ret = rvt_driver_qp_init(rdi);
 	if (ret) {
@@ -379,33 +733,15 @@ int rvt_register_device(struct rvt_dev_info *rdi)
 		return -EINVAL;
 	}
 
-	CHECK_DRIVER_OVERRIDE(rdi, create_qp);
-	CHECK_DRIVER_OVERRIDE(rdi, modify_qp);
-	CHECK_DRIVER_OVERRIDE(rdi, destroy_qp);
-	CHECK_DRIVER_OVERRIDE(rdi, query_qp);
-	CHECK_DRIVER_OVERRIDE(rdi, post_send);
-	CHECK_DRIVER_OVERRIDE(rdi, post_recv);
-	CHECK_DRIVER_OVERRIDE(rdi, post_srq_recv);
-
 	/* Address Handle */
-	CHECK_DRIVER_OVERRIDE(rdi, create_ah);
-	CHECK_DRIVER_OVERRIDE(rdi, destroy_ah);
-	CHECK_DRIVER_OVERRIDE(rdi, modify_ah);
-	CHECK_DRIVER_OVERRIDE(rdi, query_ah);
 	spin_lock_init(&rdi->n_ahs_lock);
 	rdi->n_ahs_allocated = 0;
 
 	/* Shared Receive Queue */
-	CHECK_DRIVER_OVERRIDE(rdi, create_srq);
-	CHECK_DRIVER_OVERRIDE(rdi, modify_srq);
-	CHECK_DRIVER_OVERRIDE(rdi, destroy_srq);
-	CHECK_DRIVER_OVERRIDE(rdi, query_srq);
 	rvt_driver_srq_init(rdi);
 
 	/* Multicast */
 	rvt_driver_mcast_init(rdi);
-	CHECK_DRIVER_OVERRIDE(rdi, attach_mcast);
-	CHECK_DRIVER_OVERRIDE(rdi, detach_mcast);
 
 	/* Mem Region */
 	ret = rvt_driver_mr_init(rdi);
@@ -414,35 +750,18 @@ int rvt_register_device(struct rvt_dev_info *rdi)
 		goto bail_no_mr;
 	}
 
-	CHECK_DRIVER_OVERRIDE(rdi, get_dma_mr);
-	CHECK_DRIVER_OVERRIDE(rdi, reg_user_mr);
-	CHECK_DRIVER_OVERRIDE(rdi, dereg_mr);
-	CHECK_DRIVER_OVERRIDE(rdi, alloc_mr);
-	CHECK_DRIVER_OVERRIDE(rdi, alloc_fmr);
-	CHECK_DRIVER_OVERRIDE(rdi, map_phys_fmr);
-	CHECK_DRIVER_OVERRIDE(rdi, unmap_fmr);
-	CHECK_DRIVER_OVERRIDE(rdi, dealloc_fmr);
-	CHECK_DRIVER_OVERRIDE(rdi, mmap);
-
 	/* Completion queues */
 	ret = rvt_driver_cq_init(rdi);
 	if (ret) {
 		pr_err("Error in driver CQ init.\n");
 		goto bail_mr;
 	}
-	CHECK_DRIVER_OVERRIDE(rdi, create_cq);
-	CHECK_DRIVER_OVERRIDE(rdi, destroy_cq);
-	CHECK_DRIVER_OVERRIDE(rdi, poll_cq);
-	CHECK_DRIVER_OVERRIDE(rdi, req_notify_cq);
-	CHECK_DRIVER_OVERRIDE(rdi, resize_cq);
 
 	/* DMA Operations */
 	rdi->ibdev.dma_ops =
 		rdi->ibdev.dma_ops ? : &rvt_default_dma_mapping_ops;
 
 	/* Protection Domain */
-	CHECK_DRIVER_OVERRIDE(rdi, alloc_pd);
-	CHECK_DRIVER_OVERRIDE(rdi, dealloc_pd);
 	spin_lock_init(&rdi->n_pds_lock);
 	rdi->n_pds_allocated = 0;
 

include/rdma/rdma_vt.h

@@ -208,62 +208,157 @@ struct rvt_dev_info;
 struct rvt_swqe;
 struct rvt_driver_provided {
 	/*
-	 * The work to create port files in /sys/class Infiniband is different
+	 * Which functions are required depends on which verbs rdmavt is
-	 * depending on the driver. This should not be extracted away and
+	 * providing and which verbs the driver is overriding. See
-	 * instead drivers are responsible for setting the correct callback for
+	 * check_support() for details.
-	 * this.
 	 */
 
-	/* -------------------*/
+	/* Passed to ib core registration. Callback to create syfs files */
-	/* Required functions */
-	/* -------------------*/
 	int (*port_callback)(struct ib_device *, u8, struct kobject *);
+
+	/*
+	 * Returns a string to represent the device for which is being
+	 * registered. This is primarily used for error and debug messages on
+	 * the console.
+	 */
 	const char * (*get_card_name)(struct rvt_dev_info *rdi);
+
+	/*
+	 * Returns a pointer to the undelying hardware's PCI device. This is
+	 * used to display information as to what hardware is being referenced
+	 * in an output message
+	 */
 	struct pci_dev * (*get_pci_dev)(struct rvt_dev_info *rdi);
-	unsigned (*free_all_qps)(struct rvt_dev_info *rdi);
+
+	/*
+	 * Allocate a private queue pair data structure for driver specific
+	 * information which is opaque to rdmavt.
+	 */
 	void * (*qp_priv_alloc)(struct rvt_dev_info *rdi, struct rvt_qp *qp,
 				gfp_t gfp);
+
+	/*
+	 * Free the driver's private qp structure.
+	 */
 	void (*qp_priv_free)(struct rvt_dev_info *rdi, struct rvt_qp *qp);
+
+	/*
+	 * Inform the driver the particular qp in quesiton has been reset so
+	 * that it can clean up anything it needs to.
+	 */
 	void (*notify_qp_reset)(struct rvt_qp *qp);
+
+	/*
+	 * Give the driver a notice that there is send work to do. It is up to
+	 * the driver to generally push the packets out, this just queues the
+	 * work with the driver. There are two variants here. The no_lock
+	 * version requires the s_lock not to be held. The other assumes the
+	 * s_lock is held.
+	 */
 	void (*schedule_send)(struct rvt_qp *qp);
 	void (*schedule_send_no_lock)(struct rvt_qp *qp);
+
+	/*
+	 * Sometimes rdmavt needs to kick the driver's send progress. That is
+	 * done by this call back.
+	 */
 	void (*do_send)(struct rvt_qp *qp);
+
+	/*
+	 * Get a path mtu from the driver based on qp attributes.
+	 */
 	int (*get_pmtu_from_attr)(struct rvt_dev_info *rdi, struct rvt_qp *qp,
 				  struct ib_qp_attr *attr);
+
+	/*
+	 * Notify driver that it needs to flush any outstanding IO requests that
+	 * are waiting on a qp.
+	 */
 	void (*flush_qp_waiters)(struct rvt_qp *qp);
+
+	/*
+	 * Notify driver to stop its queue of sending packets. Nothing else
+	 * should be posted to the queue pair after this has been called.
+	 */
 	void (*stop_send_queue)(struct rvt_qp *qp);
+
+	/*
+	 * Have the drivr drain any in progress operations
+	 */
 	void (*quiesce_qp)(struct rvt_qp *qp);
+
+	/*
+	 * Inform the driver a qp has went to error state.
+	 */
 	void (*notify_error_qp)(struct rvt_qp *qp);
+
+	/*
+	 * Get an MTU for a qp.
+	 */
 	u32 (*mtu_from_qp)(struct rvt_dev_info *rdi, struct rvt_qp *qp,
 			   u32 pmtu);
+	/*
+	 * Convert an mtu to a path mtu
+	 */
 	int (*mtu_to_path_mtu)(u32 mtu);
+
+	/*
+	 * Get the guid of a port in big endian byte order
+	 */
 	int (*get_guid_be)(struct rvt_dev_info *rdi, struct rvt_ibport *rvp,
 			   int guid_index, __be64 *guid);
+
+	/*
+	 * Query driver for the state of the port.
+	 */
 	int (*query_port_state)(struct rvt_dev_info *rdi, u8 port_num,
 				struct ib_port_attr *props);
+
+	/*
+	 * Tell driver to shutdown a port
+	 */
 	int (*shut_down_port)(struct rvt_dev_info *rdi, u8 port_num);
+
+	/* Tell driver to send a trap for changed  port capabilities */
 	void (*cap_mask_chg)(struct rvt_dev_info *rdi, u8 port_num);
 
-	/*--------------------*/
+	/*
-	/* Optional functions */
+	 * The following functions can be safely ignored completely. Any use of
-	/*--------------------*/
+	 * these is checked for NULL before blindly calling. Rdmavt should also
+	 * be functional if drivers omit these.
+	 */
+
+	/* Called to inform the driver that all qps should now be freed. */
+	unsigned (*free_all_qps)(struct rvt_dev_info *rdi);
+
+	/* Driver specific AH validation */
 	int (*check_ah)(struct ib_device *, struct ib_ah_attr *);
+
+	/* Inform the driver a new AH has been created */
 	void (*notify_new_ah)(struct ib_device *, struct ib_ah_attr *,
 			      struct rvt_ah *);
+
+	/* Let the driver pick the next queue pair number*/
 	int (*alloc_qpn)(struct rvt_dev_info *rdi, struct rvt_qpn_table *qpt,
 			 enum ib_qp_type type, u8 port_num, gfp_t gfp);
-	/**
+
-	 * Return 0 if modification is valid, -errno otherwise
+	/* Determine if its safe or allowed to modify the qp */
-	 */
 	int (*check_modify_qp)(struct rvt_qp *qp, struct ib_qp_attr *attr,
 			       int attr_mask, struct ib_udata *udata);
+
+	/* Driver specific QP modification/notification-of */
 	void (*modify_qp)(struct rvt_qp *qp, struct ib_qp_attr *attr,
 			  int attr_mask, struct ib_udata *udata);
 
+	/* Driver specific work request checking */
 	int (*check_send_wqe)(struct rvt_qp *qp, struct rvt_swqe *wqe);
 
+	/* Notify driver a mad agent has been created */
 	void (*notify_create_mad_agent)(struct rvt_dev_info *rdi, int port_idx);
+
+	/* Notify driver a mad agent has been removed */
 	void (*notify_free_mad_agent)(struct rvt_dev_info *rdi, int port_idx);
+
 };
 
 struct rvt_dev_info {