Refactor RDMA IOCTL declarations

Hi Doug,

This patch set refactors RDMA IOCTL declarations, unifies their location, moves all current IOCTL exporters to one place and renames them to fix the differences in coding style.

IOCTLs commands need to export the size of their data for their transfer which is usually declared as a struct defined in relevant submodule. These structures should be visible at the linkage stage.

There are number of possible solutions to overcome this limitations.

One of the options is to declare anonymous variable which will be visible during the linkage. As a downside, such option won't allow direct use of this header by other parts of code without their specific includes of other logically unrelated submodules.

Another possible option will be to move declarations of that structures to that common header file. Such move will cause to bloat this file with different submodules declarations.

Our decision was to choose direct include of other exported files with declarations of such structures to simplify future usage of this file. There is no change for legacy applications and libraries.

Our decision was to choose direct include of other exported files with declarations of such structures to simplify future usage of this file. There is no change for legacy applications and libraries.

This is the second version of patch series and the changes are in commit messages of patch number 3 and 6.

Available in the "topic/ioctl-header" topic branch of this git repo:
git://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git

Or for browsing:
https://git.kernel.org/cgit/linux/kernel/git/leon/linux-rdma.git/log/?h=topic/ioctl-header

Thanks

Leon Romanovsky (6):
RDMA/core: Commonize RDMA IOCTL declarations location RDMA/core: Move legacy MAD IOCTL declarations to common file RDMA/hfi1: Avoid redeclaration error
RDMA/core: Move HFI1 IOCTL declarations to common file RDMA/core: Rename RDMA magic number
RDMA/core: Unify style of IOCTL commands

 include/uapi/rdma/Kbuild            |   1 +
 include/uapi/rdma/hfi/Kbuild        |   1 +
 include/uapi/rdma/hfi/hfi1_ioctl.h  | 173 +++++++++++++++++++++++++++++++++++
 include/uapi/rdma/hfi/hfi1_user.h   | 175 +-----------------------------------
 include/uapi/rdma/ib_user_mad.h     |  14 +--
 include/uapi/rdma/rdma_user_ioctl.h | 127 ++++++++++++++++++++++++++
 6 files changed, 304 insertions(+), 187 deletions(-)
 create mode 100644 include/uapi/rdma/hfi/hfi1_ioctl.h
 create mode 100644 include/uapi/rdma/rdma_user_ioctl.h

--
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

This patch provides one common file (rdma_user_ioctl.h) for all RDMA UAPI IOCTLs.

---
 include/uapi/rdma/Kbuild            |  1 +
 include/uapi/rdma/hfi/hfi1_user.h   |  2 +-
 include/uapi/rdma/ib_user_mad.h     |  4 +---
 include/uapi/rdma/rdma_user_ioctl.h | 43 +++++++++++++++++++++++++++++++++++++
 4 files changed, 46 insertions(+), 4 deletions(-)
 create mode 100644 include/uapi/rdma/rdma_user_ioctl.h

index 4edb0f2..ac62908 100644
@@ -1,5 +1,6 @@
 # UAPI Header export list
 header-y += ib_user_cm.h
+header-y += rdma_user_ioctl.h
 header-y += ib_user_mad.h
 header-y += ib_user_sa.h
 header-y += ib_user_verbs.h

index d15e728..a404919 100644
@@ -57,6 +57,7 @@
 #define _LINUX__HFI1_USER_H
 
 #include <linux/types.h>
+#include <rdma/rdma_user_ioctl.h>
 
 /*
  * This version number is given to the driver by the user code during

@@ -132,7 +133,6 @@
  * User IOCTLs can not go above 128 if they do then see common.h and change the
  * base for the snoop ioctl
  */
-#define IB_IOCTL_MAGIC 0x1b /* See Documentation/ioctl/ioctl-number.txt */
 
 /*
  * Make the ioctls occupy the last 0xf0-0xff portion of the IB range

index 09f809f..9de7a2b 100644
@@ -35,7 +35,7 @@
 #define IB_USER_MAD_H
 
 #include <linux/types.h>
-#include <linux/ioctl.h>
+#include <rdma/rdma_user_ioctl.h>
 
 /*
  * Increment this value if any changes that break userspace ABI

@@ -230,8 +230,6 @@ struct ib_user_mad_reg_req2 {
 	__u8	reserved[3];
 };
 
-#define IB_IOCTL_MAGIC		0x1b
-
 #define IB_USER_MAD_REGISTER_AGENT	_IOWR(IB_IOCTL_MAGIC, 1, \
 					      struct ib_user_mad_reg_req)
 

2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

Move legacy MAD IOCTL declarations to rdma_user_ioctl.h file.

---
 include/uapi/rdma/ib_user_mad.h     | 10 ----------
 include/uapi/rdma/rdma_user_ioctl.h | 11 +++++++++++
 2 files changed, 11 insertions(+), 10 deletions(-)

index 9de7a2b..5c7abd8 100644
@@ -230,14 +230,4 @@ struct ib_user_mad_reg_req2 {
 	__u8	reserved[3];
 };
 
-#define IB_USER_MAD_REGISTER_AGENT	_IOWR(IB_IOCTL_MAGIC, 1, \
-					      struct ib_user_mad_reg_req)
-
-#define IB_USER_MAD_UNREGISTER_AGENT	_IOW(IB_IOCTL_MAGIC, 2, __u32)
-
-#define IB_USER_MAD_ENABLE_PKEY		_IO(IB_IOCTL_MAGIC, 3)
-
-#define IB_USER_MAD_REGISTER_AGENT2     _IOWR(IB_IOCTL_MAGIC, 4, \
-					      struct ib_user_mad_reg_req2)
-
 #endif /* IB_USER_MAD_H */

index ba1bcdd..820bf34 100644
@@ -35,9 +35,20 @@
 
 #include <linux/types.h>
 #include <linux/ioctl.h>
+#include <rdma/ib_user_mad.h>
 
 /* Documentation/ioctl/ioctl-number.txt */
 #define RDMA_IOCTL_MAGIC		0x1b
 #define IB_IOCTL_MAGIC			RDMA_IOCTL_MAGIC
 
+#define IB_USER_MAD_REGISTER_AGENT	_IOWR(IB_IOCTL_MAGIC, 1, \
+					      struct ib_user_mad_reg_req)
+
+#define IB_USER_MAD_UNREGISTER_AGENT	_IOW(IB_IOCTL_MAGIC, 2, __u32)
+
+#define IB_USER_MAD_ENABLE_PKEY		_IO(IB_IOCTL_MAGIC, 3)
+
+#define IB_USER_MAD_REGISTER_AGENT2     _IOWR(IB_IOCTL_MAGIC, 4, \
+					      struct ib_user_mad_reg_req2)
+
 #endif /* RDMA_USER_IOCTL_H */

2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

Move hfi1 ioctl definitions to a new header which can be included by both the hfi1 and qib drivers to avoid a duplicate enum definition as shown in this build error for qib:

CC [M] drivers/infiniband/hw/qib/qib_sysfs.o
In file included from ./include/uapi/rdma/rdma_user_ioctl.h:39:0,

		 from include/uapi/rdma/ib_user_mad.h:38,
		 from include/rdma/ib_mad.h:43,
		 from include/rdma/ib_pma.h:38,
		 from drivers/infiniband/hw/qib/qib_mad.h:37,
		 from drivers/infiniband/hw/qib/qib_init.c:49:

./include/uapi/rdma/hfi/hfi1_user.h:370:2: error: redeclaration of enumerator ‘ur_rcvhdrtail’
ur_rcvhdrtail = 0,

Move hfi1 structures to separate file to avoid this failure.

The actual move of the ioctl definitions comes in a follow on patch.

---
 include/uapi/rdma/hfi/Kbuild       |   1 +
 include/uapi/rdma/hfi/hfi1_ioctl.h | 173 +++++++++++++++++++++++++++++++++++++
 include/uapi/rdma/hfi/hfi1_user.h  | 120 +------------------------
 3 files changed, 175 insertions(+), 119 deletions(-)
 create mode 100644 include/uapi/rdma/hfi/hfi1_ioctl.h

index ef23c29..b65b0b3 100644
@@ -1,2 +1,3 @@
 # UAPI Header export list
 header-y += hfi1_user.h
+header-y += hfi1_ioctl.h

2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

Move HFI1 IOCTL declarations to rdma_user_ioctl.h file.

---
 include/uapi/rdma/hfi/hfi1_user.h   | 55 -------------------------------------
 include/uapi/rdma/rdma_user_ioctl.h | 54 ++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 55 deletions(-)

index 8aa3867..8807f06 100644
@@ -58,7 +58,6 @@
 
 #include <linux/types.h>
 #include <rdma/rdma_user_ioctl.h>
-#include <rdma/hfi/hfi1_ioctl.h>
 
 /*
  * This version number is given to the driver by the user code during

@@ -114,60 +113,6 @@
 #define HFI1_RCVHDR_ENTSIZE_16   (1UL << 1)
 #define HFI1_RCVDHR_ENTSIZE_32   (1UL << 2)
 
-/* User commands. */
-#define HFI1_CMD_ASSIGN_CTXT     1	/* allocate HFI and context */
-#define HFI1_CMD_CTXT_INFO       2	/* find out what resources we got */
-#define HFI1_CMD_USER_INFO       3	/* set up userspace */
-#define HFI1_CMD_TID_UPDATE      4	/* update expected TID entries */
-#define HFI1_CMD_TID_FREE        5	/* free expected TID entries */
-#define HFI1_CMD_CREDIT_UPD      6	/* force an update of PIO credit */
-
-#define HFI1_CMD_RECV_CTRL       8	/* control receipt of packets */
-#define HFI1_CMD_POLL_TYPE       9	/* set the kind of polling we want */
-#define HFI1_CMD_ACK_EVENT       10	/* ack & clear user status bits */
-#define HFI1_CMD_SET_PKEY        11     /* set context's pkey */
-#define HFI1_CMD_CTXT_RESET      12     /* reset context's HW send context */
-#define HFI1_CMD_TID_INVAL_READ  13     /* read TID cache invalidations */
-#define HFI1_CMD_GET_VERS	 14	/* get the version of the user cdev */
-
-/*
- * User IOCTLs can not go above 128 if they do then see common.h and change the
- * base for the snoop ioctl
- */
-
-/*
- * Make the ioctls occupy the last 0xf0-0xff portion of the IB range
- */
-#define __NUM(cmd) (HFI1_CMD_##cmd + 0xe0)
-
-struct hfi1_cmd;
-#define HFI1_IOCTL_ASSIGN_CTXT \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(ASSIGN_CTXT), struct hfi1_user_info)
-#define HFI1_IOCTL_CTXT_INFO \
-	_IOW(IB_IOCTL_MAGIC, __NUM(CTXT_INFO), struct hfi1_ctxt_info)
-#define HFI1_IOCTL_USER_INFO \
-	_IOW(IB_IOCTL_MAGIC, __NUM(USER_INFO), struct hfi1_base_info)
-#define HFI1_IOCTL_TID_UPDATE \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_UPDATE), struct hfi1_tid_info)
-#define HFI1_IOCTL_TID_FREE \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_FREE), struct hfi1_tid_info)
-#define HFI1_IOCTL_CREDIT_UPD \
-	_IO(IB_IOCTL_MAGIC, __NUM(CREDIT_UPD))
-#define HFI1_IOCTL_RECV_CTRL \
-	_IOW(IB_IOCTL_MAGIC, __NUM(RECV_CTRL), int)
-#define HFI1_IOCTL_POLL_TYPE \
-	_IOW(IB_IOCTL_MAGIC, __NUM(POLL_TYPE), int)
-#define HFI1_IOCTL_ACK_EVENT \
-	_IOW(IB_IOCTL_MAGIC, __NUM(ACK_EVENT), unsigned long)
-#define HFI1_IOCTL_SET_PKEY \
-	_IOW(IB_IOCTL_MAGIC, __NUM(SET_PKEY), __u16)
-#define HFI1_IOCTL_CTXT_RESET \
-	_IO(IB_IOCTL_MAGIC, __NUM(CTXT_RESET))
-#define HFI1_IOCTL_TID_INVAL_READ \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_INVAL_READ), struct hfi1_tid_info)
-#define HFI1_IOCTL_GET_VERS \
-	_IOR(IB_IOCTL_MAGIC, __NUM(GET_VERS), int)
-
 #define _HFI1_EVENT_FROZEN_BIT         0
 #define _HFI1_EVENT_LINKDOWN_BIT       1
 #define _HFI1_EVENT_LID_CHANGE_BIT     2

2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

Rename RDMA magic number to better describe IOCTLs.

---
 include/uapi/rdma/rdma_user_ioctl.h | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

index e9a69f0..7ecf8cd 100644
@@ -40,16 +40,17 @@
 
 /* Documentation/ioctl/ioctl-number.txt */
 #define RDMA_IOCTL_MAGIC		0x1b
+/* Legacy name, for user space application which already use it */
 #define IB_IOCTL_MAGIC			RDMA_IOCTL_MAGIC
 
-#define IB_USER_MAD_REGISTER_AGENT	_IOWR(IB_IOCTL_MAGIC, 1, \
+#define IB_USER_MAD_REGISTER_AGENT	_IOWR(RDMA_IOCTL_MAGIC, 1, \
 					      struct ib_user_mad_reg_req)
 
-#define IB_USER_MAD_UNREGISTER_AGENT	_IOW(IB_IOCTL_MAGIC, 2, __u32)
+#define IB_USER_MAD_UNREGISTER_AGENT	_IOW(RDMA_IOCTL_MAGIC, 2, __u32)
 
-#define IB_USER_MAD_ENABLE_PKEY		_IO(IB_IOCTL_MAGIC, 3)
+#define IB_USER_MAD_ENABLE_PKEY		_IO(RDMA_IOCTL_MAGIC, 3)
 
-#define IB_USER_MAD_REGISTER_AGENT2     _IOWR(IB_IOCTL_MAGIC, 4, \
+#define IB_USER_MAD_REGISTER_AGENT2     _IOWR(RDMA_IOCTL_MAGIC, 4, \
 					      struct ib_user_mad_reg_req2)
 
 /* User commands. */

@@ -79,30 +80,30 @@
 #define __NUM(cmd) (HFI1_CMD_##cmd + 0xe0)
 
 #define HFI1_IOCTL_ASSIGN_CTXT \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(ASSIGN_CTXT), struct hfi1_user_info)
+	_IOWR(RDMA_IOCTL_MAGIC, __NUM(ASSIGN_CTXT), struct hfi1_user_info)
 #define HFI1_IOCTL_CTXT_INFO \
-	_IOW(IB_IOCTL_MAGIC, __NUM(CTXT_INFO), struct hfi1_ctxt_info)
+	_IOW(RDMA_IOCTL_MAGIC, __NUM(CTXT_INFO), struct hfi1_ctxt_info)
 #define HFI1_IOCTL_USER_INFO \
-	_IOW(IB_IOCTL_MAGIC, __NUM(USER_INFO), struct hfi1_base_info)
+	_IOW(RDMA_IOCTL_MAGIC, __NUM(USER_INFO), struct hfi1_base_info)
 #define HFI1_IOCTL_TID_UPDATE \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_UPDATE), struct hfi1_tid_info)
+	_IOWR(RDMA_IOCTL_MAGIC, __NUM(TID_UPDATE), struct hfi1_tid_info)
 #define HFI1_IOCTL_TID_FREE \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_FREE), struct hfi1_tid_info)
+	_IOWR(RDMA_IOCTL_MAGIC, __NUM(TID_FREE), struct hfi1_tid_info)
 #define HFI1_IOCTL_CREDIT_UPD \
-	_IO(IB_IOCTL_MAGIC, __NUM(CREDIT_UPD))
+	_IO(RDMA_IOCTL_MAGIC, __NUM(CREDIT_UPD))
 #define HFI1_IOCTL_RECV_CTRL \
-	_IOW(IB_IOCTL_MAGIC, __NUM(RECV_CTRL), int)
+	_IOW(RDMA_IOCTL_MAGIC, __NUM(RECV_CTRL), int)
 #define HFI1_IOCTL_POLL_TYPE \
-	_IOW(IB_IOCTL_MAGIC, __NUM(POLL_TYPE), int)
+	_IOW(RDMA_IOCTL_MAGIC, __NUM(POLL_TYPE), int)
 #define HFI1_IOCTL_ACK_EVENT \
-	_IOW(IB_IOCTL_MAGIC, __NUM(ACK_EVENT), unsigned long)
+	_IOW(RDMA_IOCTL_MAGIC, __NUM(ACK_EVENT), unsigned long)
 #define HFI1_IOCTL_SET_PKEY \
-	_IOW(IB_IOCTL_MAGIC, __NUM(SET_PKEY), __u16)
+	_IOW(RDMA_IOCTL_MAGIC, __NUM(SET_PKEY), __u16)
 #define HFI1_IOCTL_CTXT_RESET \
-	_IO(IB_IOCTL_MAGIC, __NUM(CTXT_RESET))
+	_IO(RDMA_IOCTL_MAGIC, __NUM(CTXT_RESET))
 #define HFI1_IOCTL_TID_INVAL_READ \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_INVAL_READ), struct hfi1_tid_info)
+	_IOWR(RDMA_IOCTL_MAGIC, __NUM(TID_INVAL_READ), struct hfi1_tid_info)
 #define HFI1_IOCTL_GET_VERS \
-	_IOR(IB_IOCTL_MAGIC, __NUM(GET_VERS), int)
+	_IOR(RDMA_IOCTL_MAGIC, __NUM(GET_VERS), int)
 
 #endif /* RDMA_USER_IOCTL_H */

2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

MAD and HFI1 have different naming convention, this patch simplifies and unifies their defines and names.

As part of cleanup, the HFI1 _NUM() macro was removed. It has a potential to break applications which use this macro directly.

---
 include/uapi/rdma/rdma_user_ioctl.h | 98 ++++++++++++++++++++++---------------
 1 file changed, 58 insertions(+), 40 deletions(-)

index 7ecf8cd..9abd6d1 100644
@@ -39,71 +39,89 @@
 #include <rdma/hfi/hfi1_ioctl.h>
 
 /* Documentation/ioctl/ioctl-number.txt */
-#define RDMA_IOCTL_MAGIC		0x1b
+#define RDMA_IOCTL_MAGIC	0x1b
 /* Legacy name, for user space application which already use it */
-#define IB_IOCTL_MAGIC			RDMA_IOCTL_MAGIC
+#define IB_IOCTL_MAGIC		RDMA_IOCTL_MAGIC
 
-#define IB_USER_MAD_REGISTER_AGENT	_IOWR(RDMA_IOCTL_MAGIC, 1, \
-					      struct ib_user_mad_reg_req)
+/* General blocks assignments */
+#define MAD_CMD_BASE		0x00
+#define HFI1_CMD_BASE		0xE0
 
-#define IB_USER_MAD_UNREGISTER_AGENT	_IOW(RDMA_IOCTL_MAGIC, 2, __u32)
+/* MAD specific section */
+#define MAD_CMD_REG_AGENT	(MAD_CMD_BASE + 0x01)
+#define MAD_CMD_UNREG_AGENT	(MAD_CMD_BASE + 0x02)
+#define MAD_CMD_ENABLE_PKEY	(MAD_CMD_BASE + 0x03)
+#define MAD_CMD_REG_AGENT2	(MAD_CMD_BASE + 0x04)
 
-#define IB_USER_MAD_ENABLE_PKEY		_IO(RDMA_IOCTL_MAGIC, 3)
-
-#define IB_USER_MAD_REGISTER_AGENT2     _IOWR(RDMA_IOCTL_MAGIC, 4, \
-					      struct ib_user_mad_reg_req2)
+#define IB_USER_MAD_REGISTER_AGENT \
+	_IOWR(RDMA_IOCTL_MAGIC, MAD_CMD_REG_AGENT, struct ib_user_mad_reg_req)
+#define IB_USER_MAD_UNREGISTER_AGENT \
+	_IOW(RDMA_IOCTL_MAGIC, MAD_CMD_UNREG_AGENT, __u32)
+#define IB_USER_MAD_ENABLE_PKEY \
+	_IO(RDMA_IOCTL_MAGIC, MAD_CMD_ENABLE_PKEY)
+#define IB_USER_MAD_REGISTER_AGENT2 \
+	_IOWR(RDMA_IOCTL_MAGIC, MAD_CMD_REG_AGENT2, struct ib_user_mad_reg_req2)
 
+/* HFI specific section */
 /* User commands. */
-#define HFI1_CMD_ASSIGN_CTXT     1	/* allocate HFI and context */
-#define HFI1_CMD_CTXT_INFO       2	/* find out what resources we got */
-#define HFI1_CMD_USER_INFO       3	/* set up userspace */
-#define HFI1_CMD_TID_UPDATE      4	/* update expected TID entries */
-#define HFI1_CMD_TID_FREE        5	/* free expected TID entries */
-#define HFI1_CMD_CREDIT_UPD      6	/* force an update of PIO credit */
+/* allocate HFI and context */
+#define HFI1_CMD_ASSIGN_CTXT     (HFI1_CMD_BASE + 0x01)
+/* find out what resources we got */
+#define HFI1_CMD_CTXT_INFO       (HFI1_CMD_BASE + 0x02)
+/* set up userspace */
+#define HFI1_CMD_USER_INFO       (HFI1_CMD_BASE + 0x03)
+/* update expected TID entries */
+#define HFI1_CMD_TID_UPDATE      (HFI1_CMD_BASE + 0x04)
+/* free expected TID entries */
+#define HFI1_CMD_TID_FREE        (HFI1_CMD_BASE + 0x05)
+/* force an update of PIO credit */
+#define HFI1_CMD_CREDIT_UPD      (HFI1_CMD_BASE + 0x06)
 
-#define HFI1_CMD_RECV_CTRL       8	/* control receipt of packets */
-#define HFI1_CMD_POLL_TYPE       9	/* set the kind of polling we want */
-#define HFI1_CMD_ACK_EVENT       10	/* ack & clear user status bits */
-#define HFI1_CMD_SET_PKEY        11     /* set context's pkey */
-#define HFI1_CMD_CTXT_RESET      12     /* reset context's HW send context */
-#define HFI1_CMD_TID_INVAL_READ  13     /* read TID cache invalidations */
-#define HFI1_CMD_GET_VERS	 14	/* get the version of the user cdev */
+/* control receipt of packets */
+#define HFI1_CMD_RECV_CTRL       (HFI1_CMD_BASE + 0x08)
+/* set the kind of polling we want */
+#define HFI1_CMD_POLL_TYPE       (HFI1_CMD_BASE + 0x09)
+/* ack & clear user status bits */
+#define HFI1_CMD_ACK_EVENT       (HFI1_CMD_BASE + 0x0A)
+/* set context's pkey */
+#define HFI1_CMD_SET_PKEY        (HFI1_CMD_BASE + 0x0B)
+/* reset context's HW send context */
+#define HFI1_CMD_CTXT_RESET      (HFI1_CMD_BASE + 0x0C)
+/* read TID cache invalidations */
+#define HFI1_CMD_TID_INVAL_READ  (HFI1_CMD_BASE + 0x0D)
+/* get the version of the user cdev */
+#define HFI1_CMD_GET_VERS	 (HFI1_CMD_BASE + 0x0E)
 
 /*
  * User IOCTLs can not go above 128 if they do then see common.h and change the
  * base for the snoop ioctl
  */
 
-/*
- * Make the ioctls occupy the last 0xf0-0xff portion of the IB range
- */
-#define __NUM(cmd) (HFI1_CMD_##cmd + 0xe0)
-
 #define HFI1_IOCTL_ASSIGN_CTXT \
-	_IOWR(RDMA_IOCTL_MAGIC, __NUM(ASSIGN_CTXT), struct hfi1_user_info)
+	_IOWR(RDMA_IOCTL_MAGIC, HFI1_CMD_ASSIGN_CTXT, struct hfi1_user_info)
 #define HFI1_IOCTL_CTXT_INFO \
-	_IOW(RDMA_IOCTL_MAGIC, __NUM(CTXT_INFO), struct hfi1_ctxt_info)
+	_IOW(RDMA_IOCTL_MAGIC, HFI1_CMD_CTXT_INFO, struct hfi1_ctxt_info)
 #define HFI1_IOCTL_USER_INFO \
-	_IOW(RDMA_IOCTL_MAGIC, __NUM(USER_INFO), struct hfi1_base_info)
+	_IOW(RDMA_IOCTL_MAGIC, HFI1_CMD_USER_INFO, struct hfi1_base_info)
 #define HFI1_IOCTL_TID_UPDATE \
-	_IOWR(RDMA_IOCTL_MAGIC, __NUM(TID_UPDATE), struct hfi1_tid_info)
+	_IOWR(RDMA_IOCTL_MAGIC, HFI1_CMD_TID_UPDATE, struct hfi1_tid_info)
 #define HFI1_IOCTL_TID_FREE \
-	_IOWR(RDMA_IOCTL_MAGIC, __NUM(TID_FREE), struct hfi1_tid_info)
+	_IOWR(RDMA_IOCTL_MAGIC, HFI1_CMD_TID_FREE, struct hfi1_tid_info)
 #define HFI1_IOCTL_CREDIT_UPD \
-	_IO(RDMA_IOCTL_MAGIC, __NUM(CREDIT_UPD))
+	_IO(RDMA_IOCTL_MAGIC, HFI1_CMD_CREDIT_UPD)
 #define HFI1_IOCTL_RECV_CTRL \
-	_IOW(RDMA_IOCTL_MAGIC, __NUM(RECV_CTRL), int)
+	_IOW(RDMA_IOCTL_MAGIC, HFI1_CMD_RECV_CTRL, int)
 #define HFI1_IOCTL_POLL_TYPE \
-	_IOW(RDMA_IOCTL_MAGIC, __NUM(POLL_TYPE), int)
+	_IOW(RDMA_IOCTL_MAGIC, HFI1_CMD_POLL_TYPE, int)
 #define HFI1_IOCTL_ACK_EVENT \
-	_IOW(RDMA_IOCTL_MAGIC, __NUM(ACK_EVENT), unsigned long)
+	_IOW(RDMA_IOCTL_MAGIC, HFI1_CMD_ACK_EVENT, unsigned long)
 #define HFI1_IOCTL_SET_PKEY \
-	_IOW(RDMA_IOCTL_MAGIC, __NUM(SET_PKEY), __u16)
+	_IOW(RDMA_IOCTL_MAGIC, HFI1_CMD_SET_PKEY, __u16)
 #define HFI1_IOCTL_CTXT_RESET \
-	_IO(RDMA_IOCTL_MAGIC, __NUM(CTXT_RESET))
+	_IO(RDMA_IOCTL_MAGIC, HFI1_CMD_CTXT_RESET)
 #define HFI1_IOCTL_TID_INVAL_READ \
-	_IOWR(RDMA_IOCTL_MAGIC, __NUM(TID_INVAL_READ), struct hfi1_tid_info)
+	_IOWR(RDMA_IOCTL_MAGIC, HFI1_CMD_TID_INVAL_READ, struct hfi1_tid_info)
 #define HFI1_IOCTL_GET_VERS \
-	_IOR(RDMA_IOCTL_MAGIC, __NUM(GET_VERS), int)
+	_IOR(RDMA_IOCTL_MAGIC, HFI1_CMD_GET_VERS, int)
 
 #endif /* RDMA_USER_IOCTL_H */

2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

T24gV2VkLCAyMDE2LTA4LTI0IGF0IDAwOjMxICswMzAwLCBMZW9uIFJvbWFub3Zza3kgd3JvdGU6 DQo+IEZyb206IExlb24gUm9tYW5vdnNreSA8bGVvbnJvQG1lbGxhbm94LmNvbT4NCj7CoA0KPiAt I2RlZmluZSBIRkkxX0NNRF9BU1NJR05fQ1RYVMKgwqDCoMKgwqAxCS8qIGFsbG9jYXRlIEhGSSBh bmQNCj4gY29udGV4dCAqLw0KPiAtI2RlZmluZSBIRkkxX0NNRF9DVFhUX0lORk/CoMKgwqDCoMKg wqDCoDIJLyogZmluZCBvdXQgd2hhdCByZXNvdXJjZXMNCj4gd2UgZ290ICovDQo+IC0jZGVmaW5l IEhGSTFfQ01EX1VTRVJfSU5GT8KgwqDCoMKgwqDCoMKgMwkvKiBzZXQgdXAgdXNlcnNwYWNlICov DQo+IC0jZGVmaW5lIEhGSTFfQ01EX1RJRF9VUERBVEXCoMKgwqDCoMKgwqA0CS8qIHVwZGF0ZSBl eHBlY3RlZCBUSUQNCj4gZW50cmllcyAqLw0KPiAtI2RlZmluZSBIRkkxX0NNRF9USURfRlJFRcKg wqDCoMKgwqDCoMKgwqA1CS8qIGZyZWUgZXhwZWN0ZWQgVElEDQo+IGVudHJpZXMgKi8NCj4gLSNk ZWZpbmUgSEZJMV9DTURfQ1JFRElUX1VQRMKgwqDCoMKgwqDCoDYJLyogZm9yY2UgYW4gdXBkYXRl IG9mIFBJTw0KPiBjcmVkaXQgKi8NCj4gKy8qIGFsbG9jYXRlIEhGSSBhbmQgY29udGV4dCAqLw0K PiArI2RlZmluZSBIRkkxX0NNRF9BU1NJR05fQ1RYVMKgwqDCoMKgwqAoSEZJMV9DTURfQkFTRSAr IDB4MDEpDQo+ICsvKiBmaW5kIG91dCB3aGF0IHJlc291cmNlcyB3ZSBnb3QgKi8NCj4gKyNkZWZp bmUgSEZJMV9DTURfQ1RYVF9JTkZPwqDCoMKgwqDCoMKgwqAoSEZJMV9DTURfQkFTRSArIDB4MDIp DQo+ICsvKiBzZXQgdXAgdXNlcnNwYWNlICovDQo+ICsjZGVmaW5lIEhGSTFfQ01EX1VTRVJfSU5G T8KgwqDCoMKgwqDCoMKgKEhGSTFfQ01EX0JBU0UgKyAweDAzKQ0KPiArLyogdXBkYXRlIGV4cGVj dGVkIFRJRCBlbnRyaWVzICovDQo+ICsjZGVmaW5lIEhGSTFfQ01EX1RJRF9VUERBVEXCoMKgwqDC oMKgwqAoSEZJMV9DTURfQkFTRSArIDB4MDQpDQo+ICsvKiBmcmVlIGV4cGVjdGVkIFRJRCBlbnRy aWVzICovDQo+ICsjZGVmaW5lIEhGSTFfQ01EX1RJRF9GUkVFwqDCoMKgwqDCoMKgwqDCoChIRkkx X0NNRF9CQVNFICsgMHgwNSkNCj4gKy8qIGZvcmNlIGFuIHVwZGF0ZSBvZiBQSU8gY3JlZGl0ICov DQo+ICsjZGVmaW5lIEhGSTFfQ01EX0NSRURJVF9VUETCoMKgwqDCoMKgwqAoSEZJMV9DTURfQkFT RSArIDB4MDYpDQoNClRoaXMgaXMgYSBtaW5vciBpc3N1ZSwgYnV0IHRoZSBwcm9ibGVtIGhlcmUg aXMgd2hlbiB3ZSBidWlsZCBQU00NCmFnYWluc3QgdGhpcyBrZXJuZWwgaXQgd2lsbCBubyBsb25n ZXIgd29yayBmb3Igb2xkZXIga2VybmVscyBiZWNhdXNlDQp0aGUgdmFsdWUgb2YgSEZJMV9DTURf QVNTSUdOX0NUWFQgaGFzIGNoYW5nZWQgd2hlcmUgYXMgaXQgdXNlZCB0byBiZSAxLg0KUmlnaHQg bm93IFBTTSBpcyBiYWNrd2FyZHMgY29tcGF0aWJsZSwgdGhpcyBicmVha3MgdGhhdCBjb21wYXRp YmlsaXR5Lg0KDQpTbyB3aGlsZSBubyBvbmUgdXNlcyB0aGUgX19OVU0oKSBtYWNybyBkaXJlY3Rs eSBpdCBsZXRzIHVzIG5vdCBjaGFuZ2UNCnRoZSBQU00gY29tbWFuZCB2YWx1ZXMuIENhbiB3ZSBw dXQgdGhhdCBwYXJ0IGJhY2sgYW5kIGtlZXAgdGhlIGNvbW1hbmQNCnZhbHVlcyB1bmNoYW5nZWQ/ DQoNCi1EZW5ueQ0K
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Sure, I'll post new version.
Just to be sure, you are not using __NUM() and I can redefine that macro, am I right?

Thanks.

T24gVGh1LCAyMDE2LTA5LTAxIGF0IDE5OjIwICswMzAwLCBMZW9uIFJvbWFub3Zza3kgd3JvdGU6 DQo+IE9uIFRodSwgU2VwIDAxLCAyMDE2IGF0IDAyOjA1OjQ0UE0gKzAwMDAsIERhbGVzc2FuZHJv LCBEZW5uaXMgd3JvdGU6DQo+ID4gT24gV2VkLCAyMDE2LTA4LTI0IGF0IDAwOjMxICswMzAwLCBM ZW9uIFJvbWFub3Zza3kgd3JvdGU6DQo+ID4gPiBGcm9tOiBMZW9uIFJvbWFub3Zza3kgPGxlb25y b0BtZWxsYW5veC5jb20+DQo+ID4gPiDCoA0KPiA+ID4gLSNkZWZpbmUgSEZJMV9DTURfQVNTSUdO X0NUWFTCoMKgwqDCoMKgMQkvKiBhbGxvY2F0ZSBIRkkgYW5kDQo+ID4gPiBjb250ZXh0ICovDQo+ ID4gPiAtI2RlZmluZSBIRkkxX0NNRF9DVFhUX0lORk/CoMKgwqDCoMKgwqDCoDIJLyogZmluZCBv dXQgd2hhdA0KPiA+ID4gcmVzb3VyY2VzDQo+ID4gPiB3ZSBnb3QgKi8NCj4gPiA+IC0jZGVmaW5l IEhGSTFfQ01EX1VTRVJfSU5GT8KgwqDCoMKgwqDCoMKgMwkvKiBzZXQgdXAgdXNlcnNwYWNlICov DQo+ID4gPiAtI2RlZmluZSBIRkkxX0NNRF9USURfVVBEQVRFwqDCoMKgwqDCoMKgNAkvKiB1cGRh dGUgZXhwZWN0ZWQgVElEDQo+ID4gPiBlbnRyaWVzICovDQo+ID4gPiAtI2RlZmluZSBIRkkxX0NN RF9USURfRlJFRcKgwqDCoMKgwqDCoMKgwqA1CS8qIGZyZWUgZXhwZWN0ZWQgVElEDQo+ID4gPiBl bnRyaWVzICovDQo+ID4gPiAtI2RlZmluZSBIRkkxX0NNRF9DUkVESVRfVVBEwqDCoMKgwqDCoMKg NgkvKiBmb3JjZSBhbiB1cGRhdGUgb2YNCj4gPiA+IFBJTw0KPiA+ID4gY3JlZGl0ICovDQo+ID4g PiArLyogYWxsb2NhdGUgSEZJIGFuZCBjb250ZXh0ICovDQo+ID4gPiArI2RlZmluZSBIRkkxX0NN RF9BU1NJR05fQ1RYVMKgwqDCoMKgwqAoSEZJMV9DTURfQkFTRSArIDB4MDEpDQo+ID4gPiArLyog ZmluZCBvdXQgd2hhdCByZXNvdXJjZXMgd2UgZ290ICovDQo+ID4gPiArI2RlZmluZSBIRkkxX0NN RF9DVFhUX0lORk/CoMKgwqDCoMKgwqDCoChIRkkxX0NNRF9CQVNFICsgMHgwMikNCj4gPiA+ICsv KiBzZXQgdXAgdXNlcnNwYWNlICovDQo+ID4gPiArI2RlZmluZSBIRkkxX0NNRF9VU0VSX0lORk/C oMKgwqDCoMKgwqDCoChIRkkxX0NNRF9CQVNFICsgMHgwMykNCj4gPiA+ICsvKiB1cGRhdGUgZXhw ZWN0ZWQgVElEIGVudHJpZXMgKi8NCj4gPiA+ICsjZGVmaW5lIEhGSTFfQ01EX1RJRF9VUERBVEXC oMKgwqDCoMKgwqAoSEZJMV9DTURfQkFTRSArIDB4MDQpDQo+ID4gPiArLyogZnJlZSBleHBlY3Rl ZCBUSUQgZW50cmllcyAqLw0KPiA+ID4gKyNkZWZpbmUgSEZJMV9DTURfVElEX0ZSRUXCoMKgwqDC oMKgwqDCoMKgKEhGSTFfQ01EX0JBU0UgKyAweDA1KQ0KPiA+ID4gKy8qIGZvcmNlIGFuIHVwZGF0 ZSBvZiBQSU8gY3JlZGl0ICovDQo+ID4gPiArI2RlZmluZSBIRkkxX0NNRF9DUkVESVRfVVBEwqDC oMKgwqDCoMKgKEhGSTFfQ01EX0JBU0UgKyAweDA2KQ0KPiA+IA0KPiA+IFRoaXMgaXMgYSBtaW5v ciBpc3N1ZSwgYnV0IHRoZSBwcm9ibGVtIGhlcmUgaXMgd2hlbiB3ZSBidWlsZCBQU00NCj4gPiBh Z2FpbnN0IHRoaXMga2VybmVsIGl0IHdpbGwgbm8gbG9uZ2VyIHdvcmsgZm9yIG9sZGVyIGtlcm5l bHMNCj4gPiBiZWNhdXNlDQo+ID4gdGhlIHZhbHVlIG9mIEhGSTFfQ01EX0FTU0lHTl9DVFhUIGhh cyBjaGFuZ2VkIHdoZXJlIGFzIGl0IHVzZWQgdG8NCj4gPiBiZSAxLg0KPiA+IFJpZ2h0IG5vdyBQ U00gaXMgYmFja3dhcmRzIGNvbXBhdGlibGUsIHRoaXMgYnJlYWtzIHRoYXQNCj4gPiBjb21wYXRp YmlsaXR5Lg0KPiA+IA0KPiA+IFNvIHdoaWxlIG5vIG9uZSB1c2VzIHRoZSBfX05VTSgpIG1hY3Jv IGRpcmVjdGx5IGl0IGxldHMgdXMgbm90DQo+ID4gY2hhbmdlDQo+ID4gdGhlIFBTTSBjb21tYW5k IHZhbHVlcy4gQ2FuIHdlIHB1dCB0aGF0IHBhcnQgYmFjayBhbmQga2VlcCB0aGUNCj4gPiBjb21t YW5kDQo+ID4gdmFsdWVzIHVuY2hhbmdlZD8NCj4gDQo+IFN1cmUsIEknbGwgcG9zdCBuZXcgdmVy c2lvbi4NCj4gSnVzdCB0byBiZSBzdXJlLCB5b3UgYXJlIG5vdCB1c2luZyBfX05VTSgpIGFuZCBJ IGNhbiByZWRlZmluZSB0aGF0DQo+IG1hY3JvLCBhbSBJIHJpZ2h0Pw0KPiANCg0KWWVhaCB5b3Ug Y2FuIGNhbGwgaXQgc29tZXRoaW5nIGVsc2UsIG9yIGRvIGl0IGluIGEgZGlmZmVyZW50IHdheS4g SQ0KdmVyaWZpZWQgdGhhdCB0aGUgcGF0Y2ggd29ya3Mgd2l0aCB0aGUgY3VycmVudCBQU00gcmV2 ZXJ0aW5nIHRoZSBDTUQNCm51bWJlcnMuIEknbGwgZ3JhYiB5b3VyIG5ldyBwYXRjaCBhbmQgcmUt dGVzdCBpdCB0aGVuLg0KDQpUaGFua3MNCg0KLURlbm55DQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

What? Someone renumberd an ioctl? When? Why? How?

Was this ioctl ever in a mainline non-staging kernel? If not, too bad, deal with it in your user space..

If yes, can we revert the renumbering? Why was that even done??

Please no, do not do crazy subtle things like this. If the ioctl has two valid numbers then you need two entries in the ioctl file.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

I didn't renumbered ioctls, but renumbered one of the internals number which is not used in kernel, but for any reasons used in their user-space.

➜ linux-rdma git:(master) grep -r HFI1_CMD_ASSIGN_CTXT drivers/infiniband/hw/hfi1/* include/*
include/uapi/rdma/hfi/hfi1_user.h:#define HFI1_CMD_ASSIGN_CTXT 1 /* allocate HFI and context */

So what should I do? respin or not?

T24gVGh1LCAyMDE2LTA5LTAxIGF0IDE5OjU1ICswMzAwLCBMZW9uIFJvbWFub3Zza3kgd3JvdGU6 DQo+wqANCj4gSSBkaWRuJ3QgcmVudW1iZXJlZCBpb2N0bHMsIGJ1dCByZW51bWJlcmVkIG9uZSBv ZiB0aGUgaW50ZXJuYWxzDQo+IG51bWJlcg0KPiB3aGljaCBpcyBub3QgdXNlZCBpbiBrZXJuZWws IGJ1dCBmb3IgYW55IHJlYXNvbnMgdXNlZCBpbiB0aGVpcg0KPiB1c2VyLXNwYWNlLg0KPiANCj4g 4p6cwqDCoGxpbnV4LXJkbWEgZ2l0OihtYXN0ZXIpIGdyZXAgLXIgSEZJMV9DTURfQVNTSUdOX0NU WFQNCj4gZHJpdmVycy9pbmZpbmliYW5kL2h3L2hmaTEvKiBpbmNsdWRlLyoNCj4gaW5jbHVkZS91 YXBpL3JkbWEvaGZpL2hmaTFfdXNlci5oOiNkZWZpbmUgSEZJMV9DTURfQVNTSUdOX0NUWFTCoMKg wqDCoMKgMQ0KPiAvKiBhbGxvY2F0ZSBIRkkgYW5kIGNvbnRleHQgKi8NCj4gDQo+IFNvIHdoYXQg c2hvdWxkIEkgZG8/IHJlc3BpbiBvciBub3Q/DQo+IA0KDQpJJ2QgcmF0aGVyIG5vdCBoYXZlIHRv IGdvIHJldG9vbCBQU00gZm9yIHNvbWV0aGluZyB0aGF0IHNob3VsZG4ndA0KcmVhbGx5IG1hdHRl ciB0byB0aGUga2VybmVsLiBTbyB3ZSdkIGFwcHJlY2lhdGUga2VlcGluZyB0aGUNCkhGSTFfQ01E X0FTU0lHTl9DVFhUIGFuZCBmcmllbmRzIHZhbHVlcyB0aGUgc2FtZS4NCg0KLURlbm55 --
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

If it is not used in the kernel why is it in this header?

Ah, I see, this was part of the staging clean up, this is part of the old non-ioctl UAPI that got trashed ??

Drop the cruft. These header file definitions should have been deleted when the API was dropped during the staging review.

We do not promise anything for drivers during their time in staging, and we do not support ABIs that only existed during staging.

Dennis should use an internal definition in PSM if he wishes to continue to support the staging kernel ABI.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

T24gVGh1LCAyMDE2LTA5LTAxIGF0IDExOjA3IC0wNjAwLCBKYXNvbiBHdW50aG9ycGUgd3JvdGU6 DQo+IE9uIFRodSwgU2VwIDAxLCAyMDE2IGF0IDA3OjU1OjUyUE0gKzAzMDAsIExlb24gUm9tYW5v dnNreSB3cm90ZToNCj4gPiBPbiBUaHUsIFNlcCAwMSwgMjAxNiBhdCAxMDo0NjoyNEFNIC0wNjAw LCBKYXNvbiBHdW50aG9ycGUgd3JvdGU6DQo+ID4gPiBPbiBUaHUsIFNlcCAwMSwgMjAxNiBhdCAw MjowNTo0NFBNICswMDAwLCBEYWxlc3NhbmRybywgRGVubmlzDQo+ID4gPiB3cm90ZToNCj4gPiA+ IA0KPiA+ID4gPiA+ICsvKiBhbGxvY2F0ZSBIRkkgYW5kIGNvbnRleHQgKi8NCj4gPiA+ID4gPiAr I2RlZmluZSBIRkkxX0NNRF9BU1NJR05fQ1RYVD8/Pz8/Pz8/Pz8oSEZJMV9DTURfQkFTRSArIDB4 MDEpDQo+ID4gPiA+IA0KPiA+ID4gPiBUaGlzIGlzIGEgbWlub3IgaXNzdWUsIGJ1dCB0aGUgcHJv YmxlbSBoZXJlIGlzIHdoZW4gd2UgYnVpbGQNCj4gPiA+ID4gUFNNDQo+ID4gPiA+IGFnYWluc3Qg dGhpcyBrZXJuZWwgaXQgd2lsbCBubyBsb25nZXIgd29yayBmb3Igb2xkZXIga2VybmVscw0KPiA+ ID4gPiBiZWNhdXNlDQo+ID4gPiA+IHRoZSB2YWx1ZSBvZiBIRkkxX0NNRF9BU1NJR05fQ1RYVCBo YXMgY2hhbmdlZCB3aGVyZSBhcyBpdCB1c2VkDQo+ID4gPiA+IHRvIGJlIDEuDQo+ID4gPiA+IFJp Z2h0IG5vdyBQU00gaXMgYmFja3dhcmRzIGNvbXBhdGlibGUsIHRoaXMgYnJlYWtzIHRoYXQNCj4g PiA+ID4gY29tcGF0aWJpbGl0eS4NCj4gPiA+IA0KPiA+ID4gV2hhdD8gU29tZW9uZSByZW51bWJl cmQgYW4gaW9jdGw/IFdoZW4/IFdoeT8gSG93Pw0KPiA+ID4gDQo+ID4gPiBXYXMgdGhpcyBpb2N0 bCBldmVyIGluIGEgbWFpbmxpbmUgbm9uLXN0YWdpbmcga2VybmVsPyBJZiBub3QsIHRvbw0KPiA+ ID4gYmFkLA0KPiA+ID4gZGVhbCB3aXRoIGl0IGluIHlvdXIgdXNlciBzcGFjZS4uDQo+ID4gPiAN Cj4gPiA+IElmIHllcywgY2FuIHdlIHJldmVydCB0aGUgcmVudW1iZXJpbmc/IFdoeSB3YXMgdGhh dCBldmVuIGRvbmU/Pw0KPiA+ID4gDQo+ID4gPiA+IFNvIHdoaWxlIG5vIG9uZSB1c2VzIHRoZSBf X05VTSgpIG1hY3JvIGRpcmVjdGx5IGl0IGxldHMgdXMgbm90DQo+ID4gPiA+IGNoYW5nZQ0KPiA+ ID4gPiB0aGUgUFNNIGNvbW1hbmQgdmFsdWVzLiBDYW4gd2UgcHV0IHRoYXQgcGFydCBiYWNrIGFu ZCBrZWVwIHRoZQ0KPiA+ID4gPiBjb21tYW5kDQo+ID4gPiA+IHZhbHVlcyB1bmNoYW5nZWQ/DQo+ ID4gPiANCj4gPiA+IFBsZWFzZSBubywgZG8gbm90IGRvIGNyYXp5IHN1YnRsZSB0aGluZ3MgbGlr ZSB0aGlzLiBJZiB0aGUgaW9jdGwNCj4gPiA+IGhhcw0KPiA+ID4gdHdvIHZhbGlkIG51bWJlcnMg dGhlbiB5b3UgbmVlZCB0d28gZW50cmllcyBpbiB0aGUgaW9jdGwgZmlsZS4NCj4gPiANCj4gPiBJ IGRpZG4ndCByZW51bWJlcmVkIGlvY3RscywgYnV0IHJlbnVtYmVyZWQgb25lIG9mIHRoZSBpbnRl cm5hbHMNCj4gPiBudW1iZXINCj4gPiB3aGljaCBpcyBub3QgdXNlZCBpbiBrZXJuZWwsIGJ1dCBm b3IgYW55IHJlYXNvbnMgdXNlZCBpbiB0aGVpcg0KPiA+IHVzZXItc3BhY2UuDQo+IA0KPiBJZiBp dCBpcyBub3QgdXNlZCBpbiB0aGUga2VybmVsIHdoeSBpcyBpdCBpbiB0aGlzIGhlYWRlcj8NCj4g DQo+IEFoLCBJIHNlZSwgdGhpcyB3YXMgcGFydCBvZiB0aGUgc3RhZ2luZyBjbGVhbiB1cCwgdGhp cyBpcyBwYXJ0IG9mIHRoZQ0KPiBvbGQgbm9uLWlvY3RsIFVBUEkgdGhhdCBnb3QgdHJhc2hlZCA/ Pw0KPiANCj4gPiA/Pz/CoMKgbGludXgtcmRtYSBnaXQ6KG1hc3RlcikgZ3JlcCAtciBIRkkxX0NN RF9BU1NJR05fQ1RYVA0KPiA+IGRyaXZlcnMvaW5maW5pYmFuZC9ody9oZmkxLyogaW5jbHVkZS8q DQo+ID4gaW5jbHVkZS91YXBpL3JkbWEvaGZpL2hmaTFfdXNlci5oOiNkZWZpbmUNCj4gPiBIRkkx X0NNRF9BU1NJR05fQ1RYVMKgwqDCoMKgwqAxIC8qIGFsbG9jYXRlIEhGSSBhbmQgY29udGV4dCAq Lw0KPiA+IA0KPiA+IFNvIHdoYXQgc2hvdWxkIEkgZG8/IHJlc3BpbiBvciBub3Q/DQo+IA0KPiBE cm9wIHRoZSBjcnVmdC4gVGhlc2UgaGVhZGVyIGZpbGUgZGVmaW5pdGlvbnMgc2hvdWxkIGhhdmUg YmVlbg0KPiBkZWxldGVkDQo+IHdoZW4gdGhlIEFQSSB3YXMgZHJvcHBlZCBkdXJpbmcgdGhlIHN0 YWdpbmcgcmV2aWV3Lg0KPiANCj4gV2UgZG8gbm90IHByb21pc2UgYW55dGhpbmcgZm9yIGRyaXZl cnMgZHVyaW5nIHRoZWlyIHRpbWUgaW4gc3RhZ2luZywNCj4gYW5kIHdlIGRvIG5vdCBzdXBwb3J0 IEFCSXMgdGhhdCBvbmx5IGV4aXN0ZWQgZHVyaW5nIHN0YWdpbmcuDQo+IA0KPiBEZW5uaXMgc2hv dWxkIHVzZSBhbiBpbnRlcm5hbCBkZWZpbml0aW9uIGluIFBTTSBpZiBoZSB3aXNoZXMgdG8NCj4g Y29udGludWUgdG8gc3VwcG9ydCB0aGUgc3RhZ2luZyBrZXJuZWwgQUJJLg0KDQpJdCdzIG5vdCBq dXN0IHRoZSBiYWNrd2FyZCBjb21wYXRpYmlsaXR5LiBQU00gdXNlcyB0aGVzZSBjb21tYW5kDQpk ZWZpbml0aW9ucy5TbyB0aGlzIGJyZWFrcyBjdXJyZW50IHN1cHBvcnQgd2l0aCBjdXJyZW50IGRy aXZlci4NCg0KLURlbm55DQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

How exactly? The HFI1_CMD_ASSIGN_CTXT constant is never used in the kernel?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

T24gVGh1LCAyMDE2LTA5LTAxIGF0IDExOjExIC0wNjAwLCBKYXNvbiBHdW50aG9ycGUgd3JvdGU6 DQo+IE9uIFRodSwgU2VwIDAxLCAyMDE2IGF0IDA1OjA5OjMxUE0gKzAwMDAsIERhbGVzc2FuZHJv LCBEZW5uaXMgd3JvdGU6DQo+IA0KPiA+ID4gRGVubmlzIHNob3VsZCB1c2UgYW4gaW50ZXJuYWwg ZGVmaW5pdGlvbiBpbiBQU00gaWYgaGUgd2lzaGVzIHRvDQo+ID4gPiBjb250aW51ZSB0byBzdXBw b3J0IHRoZSBzdGFnaW5nIGtlcm5lbCBBQkkuDQo+ID4gDQo+ID4gSXQncyBub3QganVzdCB0aGUg YmFja3dhcmQgY29tcGF0aWJpbGl0eS4gUFNNIHVzZXMgdGhlc2UgY29tbWFuZA0KPiA+IGRlZmlu aXRpb25zLlNvIHRoaXMgYnJlYWtzIGN1cnJlbnQgc3VwcG9ydCB3aXRoIGN1cnJlbnQgZHJpdmVy Lg0KPiANCj4gSG93IGV4YWN0bHk/IFRoZSBIRkkxX0NNRF9BU1NJR05fQ1RYVCBjb25zdGFudCBp cyBuZXZlciB1c2VkIGluIHRoZQ0KPiBrZXJuZWw/DQoNClRoaXMgaXMgdXNlZCBpbiBQU00gbGli cmFyeS4gQWdyZWUgaXQncyBub3QgdXNlZCBpbiB0aGUga2VybmVsLCBzbyBJDQpjYW4gc2VlIHRo ZSBhcmd1bWVudCB0byBnZXQgcmlkIG9mIGl0LCBvciBub3QgY2FyZS4gSG93ZXZlcg0KcmVtb3Zp bmcvY2hhbmdpbmcgaXQgYnJlYWtzIFBTTSBhbmQgcmVxdWlyZXMgY2hhbmdlcy4gVGhleSBhcmUg bm90IGhhcmQNCmNoYW5nZXMgdG8gbWFrZSwgYnV0IGFzIGl0IHN0YW5kcyBub3cgUFNNIHdpbGwg d29yayB3aXRoIDQuNyBidXQgbm90DQp3aXRoIDQuOC4NCg0KUmV2ZXJ0aW5nIHRoaXMgcGFydCBv ZiB0aGUgcGF0Y2ggZG9lc24ndCBjaGFuZ2Ugb3IgZGV0cmFjdCBmcm9tIHdoYXQNCnRoZSBzZXJp ZXMgZG9lcyBpbiB0aGUgZmlyc3QgcGxhY2UuIFBhdGNoIHN0aWxsIGFjY29tcGxpc2hlcyBpdHMg am9iLg0KDQotRGVubnk=
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Lets get rid of all the #defines. Leon you should just inline the ioctl numbers into the ioctl definition like normal and get rid of this extra layer of macros.

This way Dennis will get a build failure when PSM is build with these headers instead of subtle runtime breakage. He can import the required definitions to support the staging compat ABI into PSM, where they belong, and make a new release to build with new kernel headers. Lots of time to do that before these headers hit the distros.

Upstream is not the place to carry that stuff, and keeping strange subtleness with __NUM is just going to risk future breakage.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Ohh, it looks like you returned to us filled with positive energy :)

I tried to follow DRM subsystem [1] while converted these names with minimal impact on the current implementation.

I would love to go in this direction, to get rid of unused defines (especially in UAPIs).

Let's wait till Sunday, maybe we will see more discussions on the topic.

[1] http://lxr.free-electrons.com/source/include/uapi/drm/i915_drm.h

I like the idea of borrowing from the DRM subsystem but that is not quite what I see here.

That subsystem defines a constant command base and a common Command base (defined as an "end" for vendor specific):

/**
* Device specific ioctls should only be in their respective headers
* The device specific ioctl range is from 0x40 to 0x9f.
* Generic IOCTLS restart at 0xA0.
*
* \sa drmCommandNone(), drmCommandRead(), drmCommandWrite(), and
* drmCommandReadWrite().
**/
#define DRM_COMMAND_BASE 0x40
#define DRM_COMMAND_END 0xA0

And then all the vendor specific commands are defined like:

#define DRM_AMDGPU_GEM_CREATE 0x00
...

#define DRM_I810_INIT 0x00
...

#define DRM_I915_INIT 0x00
...

These defines are _not_ used anywhere in the kernel but rather define a device specific command _offset_ within the drm vendor ioctl range.

When we agreed that HFI1 would use an 0x80 offset I thought that was the idea.[*] That IB would have an upper range which was device specific and HFI1 would be the first users of that range.

Maybe it would be better to use:

#define RDMA_VENDOR_BASE 0x80

Rather than HFI1_CMD_BASE?

What I think is wrong is that your patch changes the HFI1_CMD_ASSIGN_CTXT define from being an offset from the base (0x01) to a hard coded RDMA define 0x81.

This deviates from the idea that these defines are offsets from the common base. I can see the argument Jason presents. But I think we should leave the current defines for the following reasons:

1) that was not the agreed upon interface when hfi1 came out of staging.
2) This prevents other devices from utilizing this vendor range.
3) This allows us to use this range in the future for other devices
4) There is little reason to break userspace unnecessarily.

All that said:

I know that we are not heading down the path of vendor devices having the same IOCTLs but I don't think it is an idea we should abandon.

The patch set from Matan has the following in ib_uverbs_ioctl:

	if (cmd == RDMA_DIRECT_IOCTL) {
                /* TODO? */
                err = -ENOSYS;
                goto out;
        } else {

Given that we already have a vendor ioctl range concept I think we should just keep this range for potential use instead of the above code. And keep the defines the way they are: as an HFI1 specific offset into the vendor ioctl range.

Finally, I have to disagree with Jason that any define in a uapi file is required to be used in the kernel. Looking through the drm code that is clearly not the case.

Ira

[*] I can't find the email reference at this time... :-(

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

That is not completely true, the defines are used when setting up the kernel ioctl #. I have no idea why they did this, to me it is polluting a uapi header with unneeded defines which is a big no-no.

Userspace should certainly *NEVER* use these constants, removing them is the best way to achieve that.

I'd rather not have non-unique ioctls if we can avoid it......

But even if we do, removing these indirection constants does nothing to change that - other drivers can still alias 0x80.

That is doable, particularly if the struct is a different size, then we still have unique ioctl numbers. With some care other vendors can probably manage...

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Well... That is the same for the HFI1 defines as the __NUM macro expanded to those defines...

#define __NUM(cmd) (HFI1_CMD_##cmd + 0xe0)

But really aren't we just nit picking now?

I assume the DRM user space uses them to help distinguish between

#define DRM_AMDGPU_GEM_CREATE 0x00

And

#define DRM_I810_INIT 0x00

I'm not at all familiar with the drm user space code but I could see some sort of "provider" architecture where the providers pass these constants and some "core" layer adds the vendor offset automatically???

I'm still mulling over the plan for this.

Yes but having walked through the drm headers it is nice in how explicit they are.

Yes, I am sure we can manage. There are always different ways to do things. In the end if Doug accepts this series we will manage in PSM. However, I really don't think what was done was "wrong".

Ira

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

"followed" != ("borrowed" || "copy-pasted")

To make long story short.

1. You took 0xE0 as a base for HFI while claimed to take 0xF0. There is nothing about 0x80 as a base. See commit 8d970cf991a6c38a5566572979487b906d643740

+/*
+ * Make the ioctls occupy the last 0xf0-0xff portion of the IB range
+ */
+#define __NUM(cmd) (HFI1_CMD_##cmd + 0xe0)

2. In new ABI, you will define your special objects, mark them as a vendor specific and the driver will be called immediately. IMHO, there is no need in special ioctl.

3. The Jason's point do not pollute code with defines/code which not in use has very solid background. It is right time to stop misuse of UAPI which you did. I'm pretty sure that you was supposed to update your PSM library when you moved from staging and converted from write to ioctl. I didn't hear complains about it.

Thanks.

Understood.

Well I'm still trying to get time to comment on that series. One thing which we are a bit worried about are the number of objects which may be tracked by for a device and the performance to get to a special driver object (like a PSM context). Right now PSM opens a FD and associates a single context with that FD so the access of that context is O(1). The addition of special objects and looking them up adds overhead which _MAY_ be ok but until we get a chance to evaluate better I can't commit that what you say here is true.

We did update PSM to move from staging to the ioctls. The fact that we are using these defines is because they were exported (just like the drm exports similar defines.) Perhaps drm does not use their defines I don't know.

Perhaps these are "polluting" the header but generally I just can't agree that it is "wrong" to have defines like this. I think it is just a different style.

As I said to Jason, we can adapt and I will not lose any sleep over this.

Ira

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Good to know.

T24gVGh1LCAyMDE2LTA5LTAxIGF0IDEwOjQ2IC0wNjAwLCBKYXNvbiBHdW50aG9ycGUgd3JvdGU6 DQo+IE9uIFRodSwgU2VwIDAxLCAyMDE2IGF0IDAyOjA1OjQ0UE0gKzAwMDAsIERhbGVzc2FuZHJv LCBEZW5uaXMgd3JvdGU6DQo+IA0KPiA+ID4gKy8qIGFsbG9jYXRlIEhGSSBhbmQgY29udGV4dCAq Lw0KPiA+ID4gKyNkZWZpbmUgSEZJMV9DTURfQVNTSUdOX0NUWFTCoMKgwqDCoMKgKEhGSTFfQ01E X0JBU0UgKyAweDAxKQ0KPiA+IA0KPiA+IFRoaXMgaXMgYSBtaW5vciBpc3N1ZSwgYnV0IHRoZSBw cm9ibGVtIGhlcmUgaXMgd2hlbiB3ZSBidWlsZCBQU00NCj4gPiBhZ2FpbnN0IHRoaXMga2VybmVs IGl0IHdpbGwgbm8gbG9uZ2VyIHdvcmsgZm9yIG9sZGVyIGtlcm5lbHMNCj4gPiBiZWNhdXNlDQo+ ID4gdGhlIHZhbHVlIG9mIEhGSTFfQ01EX0FTU0lHTl9DVFhUIGhhcyBjaGFuZ2VkIHdoZXJlIGFz IGl0IHVzZWQgdG8NCj4gPiBiZSAxLg0KPiA+IFJpZ2h0IG5vdyBQU00gaXMgYmFja3dhcmRzIGNv bXBhdGlibGUsIHRoaXMgYnJlYWtzIHRoYXQNCj4gPiBjb21wYXRpYmlsaXR5Lg0KPiANCj4gV2hh dD8gU29tZW9uZSByZW51bWJlcmQgYW4gaW9jdGw/IFdoZW4/IFdoeT8gSG93Pw0KDQpObyBpb2N0 bCBudW1iZXIgd2FzIHN0aWxsIHRoZSBzYW1lLiBJdCdzIHRoZSB0aGluZyB0aGF0IGlzIHVzZWQg dG8NCmRlZmluZSB0aGUgaW9jdGwgbnVtYmVyLg0KDQpUaGUgaW9jdGwgd2FzIG9yaWdpbmFsbHkg ZGVmaW5lZCBhczoNCl9JT1dSKFJETUFfSU9DVExfTUFHSUMsIF9fTlVNKEFTU0lHTl9DVFhUKSwg c3RydWN0IGhmaTFfdXNlcl9pbmZvKQ0KDQpXaGVyZSBfX05VTShBU1NJR05fQ1RYVCkgPSBIRkkx X0NNRF9BU1NJR05fQ1RYVCArIDB4RTAgPSAxICsgMHhFMA0KDQpJdCB3YXMgY2hhbmdlZCB0byBi ZToNCg0KX0lPV1IoUkRNQV9JT0NUTF9NQUdJQywgSEZJMV9DTURfQVNTSUdOX0NUWFQsIHN0cnVj dCBoZmkxX3VzZXJfaW5mbykNCg0KV2hlcmUgSEZJMV9DTURfQVNTSUdOX0NUWFQgPSAweEUxLg0K DQpTbyB0aGUgaW9jdGwgbnVtYmVyIGRvZW5zJ3QgY2hhbmdlLiBUaGUgdmFsdWUgb2YgSEZJMV9D TURfQVNTSUdOX0NUWFQNCmRvZXMsIGl0IHdhcyAxIG5vdyBpdCBpcyAweEUxLiBIRkkxX0NNRF9B U1NJR05fQ1RYVCBpcyB1c2VkIGJ5DQp1c2Vyc3BhY2UuDQoNCi1EZW5ueQ0KDQo= --
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

The end result looks reasonable enough to me.

For the series

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Thank you

Hi Ira and Dennis,
Can I get your tested-by/reviewed-by too?

I want to be sure that it is done deal.

Thanks.