All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH rdma-next 0/6] Refactor RDMA IOCTL declarations
@ 2016-08-16 13:45 Leon Romanovsky
       [not found] ` <1471355123-6227-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  0 siblings, 1 reply; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-16 13:45 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

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.

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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH rdma-next 1/6] RDMA/core: Commonize RDMA IOCTL declarations location
       [not found] ` <1471355123-6227-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2016-08-16 13:45   ` Leon Romanovsky
  2016-08-16 13:45   ` [PATCH rdma-next 2/6] RDMA/core: Move legacy MAD IOCTL declarations to common file Leon Romanovsky
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-16 13:45 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky, Matan Barak,
	Haggai Eran

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

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

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 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

diff --git a/include/uapi/rdma/Kbuild b/include/uapi/rdma/Kbuild
index 4edb0f2..ac62908 100644
--- a/include/uapi/rdma/Kbuild
+++ b/include/uapi/rdma/Kbuild
@@ -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
diff --git a/include/uapi/rdma/hfi/hfi1_user.h b/include/uapi/rdma/hfi/hfi1_user.h
index d15e728..a404919 100644
--- a/include/uapi/rdma/hfi/hfi1_user.h
+++ b/include/uapi/rdma/hfi/hfi1_user.h
@@ -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
diff --git a/include/uapi/rdma/ib_user_mad.h b/include/uapi/rdma/ib_user_mad.h
index 09f809f..9de7a2b 100644
--- a/include/uapi/rdma/ib_user_mad.h
+++ b/include/uapi/rdma/ib_user_mad.h
@@ -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)
 
diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
new file mode 100644
index 0000000..ba1bcdd
--- /dev/null
+++ b/include/uapi/rdma/rdma_user_ioctl.h
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2016 Mellanox Technologies, LTD. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ *     Redistribution and use in source and binary forms, with or
+ *     without modification, are permitted provided that the following
+ *     conditions are met:
+ *
+ *      - Redistributions of source code must retain the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer.
+ *
+ *      - Redistributions in binary form must reproduce the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer in the documentation and/or other materials
+ *        provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef RDMA_USER_IOCTL_H
+#define RDMA_USER_IOCTL_H
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+/* Documentation/ioctl/ioctl-number.txt */
+#define RDMA_IOCTL_MAGIC		0x1b
+#define IB_IOCTL_MAGIC			RDMA_IOCTL_MAGIC
+
+#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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH rdma-next 2/6] RDMA/core: Move legacy MAD IOCTL declarations to common file
       [not found] ` <1471355123-6227-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  2016-08-16 13:45   ` [PATCH rdma-next 1/6] RDMA/core: Commonize RDMA IOCTL declarations location Leon Romanovsky
@ 2016-08-16 13:45   ` Leon Romanovsky
  2016-08-16 13:45   ` [PATCH rdma-next 3/6] RDMA/hfi1: Avoid redeclaration error Leon Romanovsky
                     ` (4 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-16 13:45 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky, Matan Barak,
	Haggai Eran

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Move legacy MAD IOCTL declarations to rdma_user_ioctl.h file.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 include/uapi/rdma/ib_user_mad.h     | 10 ----------
 include/uapi/rdma/rdma_user_ioctl.h | 11 +++++++++++
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/include/uapi/rdma/ib_user_mad.h b/include/uapi/rdma/ib_user_mad.h
index 9de7a2b..5c7abd8 100644
--- a/include/uapi/rdma/ib_user_mad.h
+++ b/include/uapi/rdma/ib_user_mad.h
@@ -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 */
diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
index ba1bcdd..820bf34 100644
--- a/include/uapi/rdma/rdma_user_ioctl.h
+++ b/include/uapi/rdma/rdma_user_ioctl.h
@@ -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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH rdma-next 3/6] RDMA/hfi1: Avoid redeclaration error
       [not found] ` <1471355123-6227-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  2016-08-16 13:45   ` [PATCH rdma-next 1/6] RDMA/core: Commonize RDMA IOCTL declarations location Leon Romanovsky
  2016-08-16 13:45   ` [PATCH rdma-next 2/6] RDMA/core: Move legacy MAD IOCTL declarations to common file Leon Romanovsky
@ 2016-08-16 13:45   ` Leon Romanovsky
       [not found]     ` <1471355123-6227-4-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  2016-08-16 13:45   ` [PATCH rdma-next 4/6] RDMA/core: Move HFI1 IOCTL declarations to common file Leon Romanovsky
                     ` (3 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-16 13:45 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky, Matan Barak,
	Haggai Eran

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Plugging HFI1 IOCTL defines will cause to the following
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.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 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

diff --git a/include/uapi/rdma/hfi/Kbuild b/include/uapi/rdma/hfi/Kbuild
index ef23c29..b65b0b3 100644
--- a/include/uapi/rdma/hfi/Kbuild
+++ b/include/uapi/rdma/hfi/Kbuild
@@ -1,2 +1,3 @@
 # UAPI Header export list
 header-y += hfi1_user.h
+header-y += hfi1_ioctl.h
diff --git a/include/uapi/rdma/hfi/hfi1_ioctl.h b/include/uapi/rdma/hfi/hfi1_ioctl.h
new file mode 100644
index 0000000..4791cc8
--- /dev/null
+++ b/include/uapi/rdma/hfi/hfi1_ioctl.h
@@ -0,0 +1,173 @@
+/*
+ *
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright(c) 2015 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * BSD LICENSE
+ *
+ * Copyright(c) 2015 Intel Corporation.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *  - Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  - Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *  - Neither the name of Intel Corporation nor the names of its
+ *    contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#ifndef _LINUX__HFI1_IOCTL_H
+#define _LINUX__HFI1_IOCTL_H
+#include <linux/types.h>
+
+/*
+ * This structure is passed to the driver to tell it where
+ * user code buffers are, sizes, etc.   The offsets and sizes of the
+ * fields must remain unchanged, for binary compatibility.  It can
+ * be extended, if userversion is changed so user code can tell, if needed
+ */
+struct hfi1_user_info {
+	/*
+	 * version of user software, to detect compatibility issues.
+	 * Should be set to HFI1_USER_SWVERSION.
+	 */
+	__u32 userversion;
+	__u32 pad;
+	/*
+	 * If two or more processes wish to share a context, each process
+	 * must set the subcontext_cnt and subcontext_id to the same
+	 * values.  The only restriction on the subcontext_id is that
+	 * it be unique for a given node.
+	 */
+	__u16 subctxt_cnt;
+	__u16 subctxt_id;
+	/* 128bit UUID passed in by PSM. */
+	__u8 uuid[16];
+};
+
+struct hfi1_ctxt_info {
+	__u64 runtime_flags;    /* chip/drv runtime flags (HFI1_CAP_*) */
+	__u32 rcvegr_size;      /* size of each eager buffer */
+	__u16 num_active;       /* number of active units */
+	__u16 unit;             /* unit (chip) assigned to caller */
+	__u16 ctxt;             /* ctxt on unit assigned to caller */
+	__u16 subctxt;          /* subctxt on unit assigned to caller */
+	__u16 rcvtids;          /* number of Rcv TIDs for this context */
+	__u16 credits;          /* number of PIO credits for this context */
+	__u16 numa_node;        /* NUMA node of the assigned device */
+	__u16 rec_cpu;          /* cpu # for affinity (0xffff if none) */
+	__u16 send_ctxt;        /* send context in use by this user context */
+	__u16 egrtids;          /* number of RcvArray entries for Eager Rcvs */
+	__u16 rcvhdrq_cnt;      /* number of RcvHdrQ entries */
+	__u16 rcvhdrq_entsize;  /* size (in bytes) for each RcvHdrQ entry */
+	__u16 sdma_ring_size;   /* number of entries in SDMA request ring */
+};
+
+struct hfi1_tid_info {
+	/* virtual address of first page in transfer */
+	__u64 vaddr;
+	/* pointer to tid array. this array is big enough */
+	__u64 tidlist;
+	/* number of tids programmed by this request */
+	__u32 tidcnt;
+	/* length of transfer buffer programmed by this request */
+	__u32 length;
+};
+
+/*
+ * This structure is returned by the driver immediately after
+ * open to get implementation-specific info, and info specific to this
+ * instance.
+ *
+ * This struct must have explicit pad fields where type sizes
+ * may result in different alignments between 32 and 64 bit
+ * programs, since the 64 bit * bit kernel requires the user code
+ * to have matching offsets
+ */
+struct hfi1_base_info {
+	/* version of hardware, for feature checking. */
+	__u32 hw_version;
+	/* version of software, for feature checking. */
+	__u32 sw_version;
+	/* Job key */
+	__u16 jkey;
+	__u16 padding1;
+	/*
+	 * The special QP (queue pair) value that identifies PSM
+	 * protocol packet from standard IB packets.
+	 */
+	__u32 bthqp;
+	/* PIO credit return address, */
+	__u64 sc_credits_addr;
+	/*
+	 * Base address of write-only pio buffers for this process.
+	 * Each buffer has sendpio_credits*64 bytes.
+	 */
+	__u64 pio_bufbase_sop;
+	/*
+	 * Base address of write-only pio buffers for this process.
+	 * Each buffer has sendpio_credits*64 bytes.
+	 */
+	__u64 pio_bufbase;
+	/* address where receive buffer queue is mapped into */
+	__u64 rcvhdr_bufbase;
+	/* base address of Eager receive buffers. */
+	__u64 rcvegr_bufbase;
+	/* base address of SDMA completion ring */
+	__u64 sdma_comp_bufbase;
+	/*
+	 * User register base for init code, not to be used directly by
+	 * protocol or applications.  Always maps real chip register space.
+	 * the register addresses are:
+	 * ur_rcvhdrhead, ur_rcvhdrtail, ur_rcvegrhead, ur_rcvegrtail,
+	 * ur_rcvtidflow
+	 */
+	__u64 user_regbase;
+	/* notification events */
+	__u64 events_bufbase;
+	/* status page */
+	__u64 status_bufbase;
+	/* rcvhdrtail update */
+	__u64 rcvhdrtail_base;
+	/*
+	 * shared memory pages for subctxts if ctxt is shared; these cover
+	 * all the processes in the group sharing a single context.
+	 * all have enough space for the num_subcontexts value on this job.
+	 */
+	__u64 subctxt_uregbase;
+	__u64 subctxt_rcvegrbuf;
+	__u64 subctxt_rcvhdrbuf;
+};
+#endif /* _LINIUX__HFI1_IOCTL_H */
diff --git a/include/uapi/rdma/hfi/hfi1_user.h b/include/uapi/rdma/hfi/hfi1_user.h
index a404919..8aa3867 100644
--- a/include/uapi/rdma/hfi/hfi1_user.h
+++ b/include/uapi/rdma/hfi/hfi1_user.h
@@ -58,6 +58,7 @@
 
 #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
@@ -211,60 +212,6 @@ struct hfi1_cmd;
 #define HFI1_POLL_TYPE_ANYRCV     0x0
 #define HFI1_POLL_TYPE_URGENT     0x1
 
-/*
- * This structure is passed to the driver to tell it where
- * user code buffers are, sizes, etc.   The offsets and sizes of the
- * fields must remain unchanged, for binary compatibility.  It can
- * be extended, if userversion is changed so user code can tell, if needed
- */
-struct hfi1_user_info {
-	/*
-	 * version of user software, to detect compatibility issues.
-	 * Should be set to HFI1_USER_SWVERSION.
-	 */
-	__u32 userversion;
-	__u32 pad;
-	/*
-	 * If two or more processes wish to share a context, each process
-	 * must set the subcontext_cnt and subcontext_id to the same
-	 * values.  The only restriction on the subcontext_id is that
-	 * it be unique for a given node.
-	 */
-	__u16 subctxt_cnt;
-	__u16 subctxt_id;
-	/* 128bit UUID passed in by PSM. */
-	__u8 uuid[16];
-};
-
-struct hfi1_ctxt_info {
-	__u64 runtime_flags;    /* chip/drv runtime flags (HFI1_CAP_*) */
-	__u32 rcvegr_size;      /* size of each eager buffer */
-	__u16 num_active;       /* number of active units */
-	__u16 unit;             /* unit (chip) assigned to caller */
-	__u16 ctxt;             /* ctxt on unit assigned to caller */
-	__u16 subctxt;          /* subctxt on unit assigned to caller */
-	__u16 rcvtids;          /* number of Rcv TIDs for this context */
-	__u16 credits;          /* number of PIO credits for this context */
-	__u16 numa_node;        /* NUMA node of the assigned device */
-	__u16 rec_cpu;          /* cpu # for affinity (0xffff if none) */
-	__u16 send_ctxt;        /* send context in use by this user context */
-	__u16 egrtids;          /* number of RcvArray entries for Eager Rcvs */
-	__u16 rcvhdrq_cnt;      /* number of RcvHdrQ entries */
-	__u16 rcvhdrq_entsize;  /* size (in bytes) for each RcvHdrQ entry */
-	__u16 sdma_ring_size;   /* number of entries in SDMA request ring */
-};
-
-struct hfi1_tid_info {
-	/* virtual address of first page in transfer */
-	__u64 vaddr;
-	/* pointer to tid array. this array is big enough */
-	__u64 tidlist;
-	/* number of tids programmed by this request */
-	__u32 tidcnt;
-	/* length of transfer buffer programmed by this request */
-	__u32 length;
-};
-
 enum hfi1_sdma_comp_state {
 	FREE = 0,
 	QUEUED,
@@ -289,71 +236,6 @@ struct hfi1_status {
 	char freezemsg[0];
 };
 
-/*
- * This structure is returned by the driver immediately after
- * open to get implementation-specific info, and info specific to this
- * instance.
- *
- * This struct must have explicit pad fields where type sizes
- * may result in different alignments between 32 and 64 bit
- * programs, since the 64 bit * bit kernel requires the user code
- * to have matching offsets
- */
-struct hfi1_base_info {
-	/* version of hardware, for feature checking. */
-	__u32 hw_version;
-	/* version of software, for feature checking. */
-	__u32 sw_version;
-	/* Job key */
-	__u16 jkey;
-	__u16 padding1;
-	/*
-	 * The special QP (queue pair) value that identifies PSM
-	 * protocol packet from standard IB packets.
-	 */
-	__u32 bthqp;
-	/* PIO credit return address, */
-	__u64 sc_credits_addr;
-	/*
-	 * Base address of write-only pio buffers for this process.
-	 * Each buffer has sendpio_credits*64 bytes.
-	 */
-	__u64 pio_bufbase_sop;
-	/*
-	 * Base address of write-only pio buffers for this process.
-	 * Each buffer has sendpio_credits*64 bytes.
-	 */
-	__u64 pio_bufbase;
-	/* address where receive buffer queue is mapped into */
-	__u64 rcvhdr_bufbase;
-	/* base address of Eager receive buffers. */
-	__u64 rcvegr_bufbase;
-	/* base address of SDMA completion ring */
-	__u64 sdma_comp_bufbase;
-	/*
-	 * User register base for init code, not to be used directly by
-	 * protocol or applications.  Always maps real chip register space.
-	 * the register addresses are:
-	 * ur_rcvhdrhead, ur_rcvhdrtail, ur_rcvegrhead, ur_rcvegrtail,
-	 * ur_rcvtidflow
-	 */
-	__u64 user_regbase;
-	/* notification events */
-	__u64 events_bufbase;
-	/* status page */
-	__u64 status_bufbase;
-	/* rcvhdrtail update */
-	__u64 rcvhdrtail_base;
-	/*
-	 * shared memory pages for subctxts if ctxt is shared; these cover
-	 * all the processes in the group sharing a single context.
-	 * all have enough space for the num_subcontexts value on this job.
-	 */
-	__u64 subctxt_uregbase;
-	__u64 subctxt_rcvegrbuf;
-	__u64 subctxt_rcvhdrbuf;
-};
-
 enum sdma_req_opcode {
 	EXPECTED = 0,
 	EAGER
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH rdma-next 4/6] RDMA/core: Move HFI1 IOCTL declarations to common file
       [not found] ` <1471355123-6227-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
                     ` (2 preceding siblings ...)
  2016-08-16 13:45   ` [PATCH rdma-next 3/6] RDMA/hfi1: Avoid redeclaration error Leon Romanovsky
@ 2016-08-16 13:45   ` Leon Romanovsky
       [not found]     ` <1471355123-6227-5-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  2016-08-16 13:45   ` [PATCH rdma-next 5/6] RDMA/core: Rename RDMA magic number Leon Romanovsky
                     ` (2 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-16 13:45 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky, Matan Barak,
	Haggai Eran

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Move HFI1 IOCTL declarations to rdma_user_ioctl.h file.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 include/uapi/rdma/hfi/hfi1_user.h   | 55 -------------------------------------
 include/uapi/rdma/rdma_user_ioctl.h | 54 ++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 55 deletions(-)

diff --git a/include/uapi/rdma/hfi/hfi1_user.h b/include/uapi/rdma/hfi/hfi1_user.h
index 8aa3867..8807f06 100644
--- a/include/uapi/rdma/hfi/hfi1_user.h
+++ b/include/uapi/rdma/hfi/hfi1_user.h
@@ -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
diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
index 820bf34..e9a69f0 100644
--- a/include/uapi/rdma/rdma_user_ioctl.h
+++ b/include/uapi/rdma/rdma_user_ioctl.h
@@ -36,6 +36,7 @@
 #include <linux/types.h>
 #include <linux/ioctl.h>
 #include <rdma/ib_user_mad.h>
+#include <rdma/hfi/hfi1_ioctl.h>
 
 /* Documentation/ioctl/ioctl-number.txt */
 #define RDMA_IOCTL_MAGIC		0x1b
@@ -51,4 +52,57 @@
 #define IB_USER_MAD_REGISTER_AGENT2     _IOWR(IB_IOCTL_MAGIC, 4, \
 					      struct ib_user_mad_reg_req2)
 
+/* 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)
+
+#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)
+
 #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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH rdma-next 5/6] RDMA/core: Rename RDMA magic number
       [not found] ` <1471355123-6227-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
                     ` (3 preceding siblings ...)
  2016-08-16 13:45   ` [PATCH rdma-next 4/6] RDMA/core: Move HFI1 IOCTL declarations to common file Leon Romanovsky
@ 2016-08-16 13:45   ` Leon Romanovsky
  2016-08-16 13:45   ` [PATCH rdma-next 6/6] RDMA/core: Unify style of IOCTL commands Leon Romanovsky
  2016-08-18 11:08   ` [PATCH rdma-next 0/6] Refactor RDMA IOCTL declarations Yuval Shaia
  6 siblings, 0 replies; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-16 13:45 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky, Matan Barak,
	Haggai Eran

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Rename RDMA magic number to better describe IOCTLs.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 include/uapi/rdma/rdma_user_ioctl.h | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
index e9a69f0..7ecf8cd 100644
--- a/include/uapi/rdma/rdma_user_ioctl.h
+++ b/include/uapi/rdma/rdma_user_ioctl.h
@@ -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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH rdma-next 6/6] RDMA/core: Unify style of IOCTL commands
       [not found] ` <1471355123-6227-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
                     ` (4 preceding siblings ...)
  2016-08-16 13:45   ` [PATCH rdma-next 5/6] RDMA/core: Rename RDMA magic number Leon Romanovsky
@ 2016-08-16 13:45   ` Leon Romanovsky
       [not found]     ` <1471355123-6227-7-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  2016-08-18 11:08   ` [PATCH rdma-next 0/6] Refactor RDMA IOCTL declarations Yuval Shaia
  6 siblings, 1 reply; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-16 13:45 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky, Matan Barak,
	Haggai Eran

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

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 and MAD
indexes were renamed. It has a potential to break application
which use these defines directly.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 include/uapi/rdma/rdma_user_ioctl.h | 98 ++++++++++++++++++++++---------------
 1 file changed, 58 insertions(+), 40 deletions(-)

diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
index 7ecf8cd..9abd6d1 100644
--- a/include/uapi/rdma/rdma_user_ioctl.h
+++ b/include/uapi/rdma/rdma_user_ioctl.h
@@ -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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 6/6] RDMA/core: Unify style of IOCTL commands
       [not found]     ` <1471355123-6227-7-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2016-08-16 14:31       ` Dalessandro, Dennis
       [not found]         ` <1471357887.2661.28.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 26+ messages in thread
From: Dalessandro, Dennis @ 2016-08-16 14:31 UTC (permalink / raw)
  To: leon-DgEjT+Ai2ygdnm+yROfE0A, dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: matanb-VPRAkNaXOzVWk0Htik3J/w, Dalessandro, Dennis,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	haggaie-VPRAkNaXOzVWk0Htik3J/w, leonro-VPRAkNaXOzVWk0Htik3J/w

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 841 bytes --]

On Tue, 2016-08-16 at 16:45 +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> MAD and HFI1 have different naming convention, this patch
> simplifies and unifies their defines and names.

I don't know that I agree that it simplifies things. It changes a lot
of code for not much real value in my opinion. 

Was this something that was discussed in the verbs call? I have not
been able to attend that the last few weeks.

> As part of cleanup, the HFI1 _NUM() macro was removed and MAD
> indexes were renamed. It has a potential to break application
> which use these defines directly.

Why do you want to remove the _NUM() macro that Doug just put in?

-DennyN‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 6/6] RDMA/core: Unify style of IOCTL commands
       [not found]         ` <1471357887.2661.28.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2016-08-16 16:50           ` Leon Romanovsky
       [not found]             ` <20160816165041.GA5489-2ukJVAZIZ/Y@public.gmane.org>
  0 siblings, 1 reply; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-16 16:50 UTC (permalink / raw)
  To: Dalessandro, Dennis
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA, matanb-VPRAkNaXOzVWk0Htik3J/w,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	haggaie-VPRAkNaXOzVWk0Htik3J/w

[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]

On Tue, Aug 16, 2016 at 02:31:32PM +0000, Dalessandro, Dennis wrote:
> On Tue, 2016-08-16 at 16:45 +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > 
> > MAD and HFI1 have different naming convention, this patch
> > simplifies and unifies their defines and names.
> 
> I don't know that I agree that it simplifies things. It changes a lot
> of code for not much real value in my opinion. 
> 
> Was this something that was discussed in the verbs call? I have not
> been able to attend that the last few weeks.

It was discussed over mails, for example Jason's opinion [1],
Christopher Lameter's opinion [2], Christoph Hellwig's opinion [3].

> 
> > As part of cleanup, the HFI1 _NUM() macro was removed and MAD
> > indexes were renamed. It has a potential to break application
> > which use these defines directly.
> 
> Why do you want to remove the _NUM() macro that Doug just put in?

It is not used after refactoring and IMHO this macro doesn't
belong to UAPI, since it wouldn't in use by any users, but I'll be glad to
get an examples of its usage in real user space applications (libfabric???), if any.

> 
> -Denny

[1] http://marc.info/?l=linux-rdma&m=146835080018352&w=2
[2] http://marc.info/?l=linux-rdma&m=146897679911490&w=2
[3] http://marc.info/?l=linux-rdma&m=146908844509592&w=2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 6/6] RDMA/core: Unify style of IOCTL commands
       [not found]             ` <20160816165041.GA5489-2ukJVAZIZ/Y@public.gmane.org>
@ 2016-08-16 17:09               ` Dalessandro, Dennis
       [not found]                 ` <1471367364.2661.45.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 26+ messages in thread
From: Dalessandro, Dennis @ 2016-08-16 17:09 UTC (permalink / raw)
  To: leon-DgEjT+Ai2ygdnm+yROfE0A
  Cc: matanb-VPRAkNaXOzVWk0Htik3J/w, Dalessandro, Dennis,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	dledford-H+wXaHxf7aLQT0dZR+AlfA, haggaie-VPRAkNaXOzVWk0Htik3J/w

On Tue, 2016-08-16 at 19:50 +0300, Leon Romanovsky wrote:
> On Tue, Aug 16, 2016 at 02:31:32PM +0000, Dalessandro, Dennis wrote:
> > On Tue, 2016-08-16 at 16:45 +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@mellanox.com>
> > > 
> > > MAD and HFI1 have different naming convention, this patch
> > > simplifies and unifies their defines and names.
> > 
> > I don't know that I agree that it simplifies things. It changes a
> > lot
> > of code for not much real value in my opinion. 
> > 
> > Was this something that was discussed in the verbs call? I have not
> > been able to attend that the last few weeks.
> 
> It was discussed over mails, for example Jason's opinion [1],
> Christopher Lameter's opinion [2], Christoph Hellwig's opinion [3].

I'm not opposed to trying to unify things, however this seems to be
more than plop down the hfi1 stuff from here and put it over there. It
is certainly not simplifying anything.

> > > As part of cleanup, the HFI1 _NUM() macro was removed and MAD
> > > indexes were renamed. It has a potential to break application
> > > which use these defines directly.
> > 
> > Why do you want to remove the _NUM() macro that Doug just put in?
> 
> It is not used after refactoring and IMHO this macro doesn't
> belong to UAPI, since it wouldn't in use by any users, but I'll be
> glad to
> get an examples of its usage in real user space applications
> (libfabric???), if any.
> 

That's a fair point. I can see getting rid of it now.

-Denny

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 3/6] RDMA/hfi1: Avoid redeclaration error
       [not found]     ` <1471355123-6227-4-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2016-08-16 17:15       ` Dalessandro, Dennis
       [not found]         ` <1471367729.2661.50.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  2016-08-17  6:01       ` ira.weiny
  1 sibling, 1 reply; 26+ messages in thread
From: Dalessandro, Dennis @ 2016-08-16 17:15 UTC (permalink / raw)
  To: leon-DgEjT+Ai2ygdnm+yROfE0A, dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: matanb-VPRAkNaXOzVWk0Htik3J/w, Dalessandro, Dennis,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	haggaie-VPRAkNaXOzVWk0Htik3J/w, leonro-VPRAkNaXOzVWk0Htik3J/w

On Tue, 2016-08-16 at 16:45 +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> Plugging HFI1 IOCTL defines will cause to the following
> 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.

I need to look at this some more. So does this mean this patch series
don't build if you just take the first 2? qib should not need anything
from hfi1_user.h so it should not be including it.

-Denny


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 3/6] RDMA/hfi1: Avoid redeclaration error
       [not found]         ` <1471367729.2661.50.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2016-08-17  5:09           ` Leon Romanovsky
  0 siblings, 0 replies; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-17  5:09 UTC (permalink / raw)
  To: Dalessandro, Dennis
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA, matanb-VPRAkNaXOzVWk0Htik3J/w,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	haggaie-VPRAkNaXOzVWk0Htik3J/w

[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]

On Tue, Aug 16, 2016 at 05:15:39PM +0000, Dalessandro, Dennis wrote:
> On Tue, 2016-08-16 at 16:45 +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > 
> > Plugging HFI1 IOCTL defines will cause to the following
> > 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.
> 
> I need to look at this some more. So does this mean this patch series
> don't build if you just take the first 2?

No, they build perfectly. This patch is needed for next patch "RDMA/core:
Move HFI1 IOCTL declarations to common file".

> qib should not need anything
> from hfi1_user.h so it should not be including it.

It doesn't include directly, but qib includes ib_user_mad.h, which will
be part of common rdma_user_ioctl.h file. This common file will call to
hfi1_user.h too.

> 
> -Denny
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 6/6] RDMA/core: Unify style of IOCTL commands
       [not found]                 ` <1471367364.2661.45.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2016-08-17  5:19                   ` Leon Romanovsky
       [not found]                     ` <20160817051952.GC5489-2ukJVAZIZ/Y@public.gmane.org>
  0 siblings, 1 reply; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-17  5:19 UTC (permalink / raw)
  To: Dalessandro, Dennis
  Cc: matanb-VPRAkNaXOzVWk0Htik3J/w, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	dledford-H+wXaHxf7aLQT0dZR+AlfA, haggaie-VPRAkNaXOzVWk0Htik3J/w

[-- Attachment #1: Type: text/plain, Size: 2007 bytes --]

On Tue, Aug 16, 2016 at 05:09:27PM +0000, Dalessandro, Dennis wrote:
> On Tue, 2016-08-16 at 19:50 +0300, Leon Romanovsky wrote:
> > On Tue, Aug 16, 2016 at 02:31:32PM +0000, Dalessandro, Dennis wrote:
> > > On Tue, 2016-08-16 at 16:45 +0300, Leon Romanovsky wrote:
> > > > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > > 
> > > > MAD and HFI1 have different naming convention, this patch
> > > > simplifies and unifies their defines and names.
> > > 
> > > I don't know that I agree that it simplifies things. It changes a
> > > lot
> > > of code for not much real value in my opinion. 
> > > 
> > > Was this something that was discussed in the verbs call? I have not
> > > been able to attend that the last few weeks.
> > 
> > It was discussed over mails, for example Jason's opinion [1],
> > Christopher Lameter's opinion [2], Christoph Hellwig's opinion [3].
> 
> I'm not opposed to trying to unify things, however this seems to be
> more than plop down the hfi1 stuff from here and put it over there. It
> is certainly not simplifying anything.

It is "dark side" of UAPI - inability to change legacy declarations.
This is why I didn't remove anything except _NUM() macro.

The simplification comes from definition of one place for
declaration of IOCTLs numbers and exporting it to users. It gives
visibility for user space authors too.

> 
> > > > As part of cleanup, the HFI1 _NUM() macro was removed and MAD
> > > > indexes were renamed. It has a potential to break application
> > > > which use these defines directly.
> > > 
> > > Why do you want to remove the _NUM() macro that Doug just put in?
> > 
> > It is not used after refactoring and IMHO this macro doesn't
> > belong to UAPI, since it wouldn't in use by any users, but I'll be
> > glad to
> > get an examples of its usage in real user space applications
> > (libfabric???), if any.
> > 
> 
> That's a fair point. I can see getting rid of it now.
> 
> -Denny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 3/6] RDMA/hfi1: Avoid redeclaration error
       [not found]     ` <1471355123-6227-4-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  2016-08-16 17:15       ` Dalessandro, Dennis
@ 2016-08-17  6:01       ` ira.weiny
       [not found]         ` <20160817060139.GA27477-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
  1 sibling, 1 reply; 26+ messages in thread
From: ira.weiny @ 2016-08-17  6:01 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky, Matan Barak,
	Haggai Eran

On Tue, Aug 16, 2016 at 04:45:20PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Plugging HFI1 IOCTL defines will cause to the following
> 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,

I was pretty confused about what you were doing here mainly because you
reference a symbol you don't actually touch in the patch.

I see what you are doing now.  But a better explanation would be:

<quote>

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:

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,


The actual move of the ioctl definitions comes in a follow on patch.
</quote>

Ira

> 
> Move HFI1 structures to separate file to avoid this failure.
> 
> Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  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
> 
> diff --git a/include/uapi/rdma/hfi/Kbuild b/include/uapi/rdma/hfi/Kbuild
> index ef23c29..b65b0b3 100644
> --- a/include/uapi/rdma/hfi/Kbuild
> +++ b/include/uapi/rdma/hfi/Kbuild
> @@ -1,2 +1,3 @@
>  # UAPI Header export list
>  header-y += hfi1_user.h
> +header-y += hfi1_ioctl.h
> diff --git a/include/uapi/rdma/hfi/hfi1_ioctl.h b/include/uapi/rdma/hfi/hfi1_ioctl.h
> new file mode 100644
> index 0000000..4791cc8
> --- /dev/null
> +++ b/include/uapi/rdma/hfi/hfi1_ioctl.h
> @@ -0,0 +1,173 @@
> +/*
> + *
> + * This file is provided under a dual BSD/GPLv2 license.  When using or
> + * redistributing this file, you may do so under either license.
> + *
> + * GPL LICENSE SUMMARY
> + *
> + * Copyright(c) 2015 Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * BSD LICENSE
> + *
> + * Copyright(c) 2015 Intel Corporation.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + *  - Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + *  - Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in
> + *    the documentation and/or other materials provided with the
> + *    distribution.
> + *  - Neither the name of Intel Corporation nor the names of its
> + *    contributors may be used to endorse or promote products derived
> + *    from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + */
> +
> +#ifndef _LINUX__HFI1_IOCTL_H
> +#define _LINUX__HFI1_IOCTL_H
> +#include <linux/types.h>
> +
> +/*
> + * This structure is passed to the driver to tell it where
> + * user code buffers are, sizes, etc.   The offsets and sizes of the
> + * fields must remain unchanged, for binary compatibility.  It can
> + * be extended, if userversion is changed so user code can tell, if needed
> + */
> +struct hfi1_user_info {
> +	/*
> +	 * version of user software, to detect compatibility issues.
> +	 * Should be set to HFI1_USER_SWVERSION.
> +	 */
> +	__u32 userversion;
> +	__u32 pad;
> +	/*
> +	 * If two or more processes wish to share a context, each process
> +	 * must set the subcontext_cnt and subcontext_id to the same
> +	 * values.  The only restriction on the subcontext_id is that
> +	 * it be unique for a given node.
> +	 */
> +	__u16 subctxt_cnt;
> +	__u16 subctxt_id;
> +	/* 128bit UUID passed in by PSM. */
> +	__u8 uuid[16];
> +};
> +
> +struct hfi1_ctxt_info {
> +	__u64 runtime_flags;    /* chip/drv runtime flags (HFI1_CAP_*) */
> +	__u32 rcvegr_size;      /* size of each eager buffer */
> +	__u16 num_active;       /* number of active units */
> +	__u16 unit;             /* unit (chip) assigned to caller */
> +	__u16 ctxt;             /* ctxt on unit assigned to caller */
> +	__u16 subctxt;          /* subctxt on unit assigned to caller */
> +	__u16 rcvtids;          /* number of Rcv TIDs for this context */
> +	__u16 credits;          /* number of PIO credits for this context */
> +	__u16 numa_node;        /* NUMA node of the assigned device */
> +	__u16 rec_cpu;          /* cpu # for affinity (0xffff if none) */
> +	__u16 send_ctxt;        /* send context in use by this user context */
> +	__u16 egrtids;          /* number of RcvArray entries for Eager Rcvs */
> +	__u16 rcvhdrq_cnt;      /* number of RcvHdrQ entries */
> +	__u16 rcvhdrq_entsize;  /* size (in bytes) for each RcvHdrQ entry */
> +	__u16 sdma_ring_size;   /* number of entries in SDMA request ring */
> +};
> +
> +struct hfi1_tid_info {
> +	/* virtual address of first page in transfer */
> +	__u64 vaddr;
> +	/* pointer to tid array. this array is big enough */
> +	__u64 tidlist;
> +	/* number of tids programmed by this request */
> +	__u32 tidcnt;
> +	/* length of transfer buffer programmed by this request */
> +	__u32 length;
> +};
> +
> +/*
> + * This structure is returned by the driver immediately after
> + * open to get implementation-specific info, and info specific to this
> + * instance.
> + *
> + * This struct must have explicit pad fields where type sizes
> + * may result in different alignments between 32 and 64 bit
> + * programs, since the 64 bit * bit kernel requires the user code
> + * to have matching offsets
> + */
> +struct hfi1_base_info {
> +	/* version of hardware, for feature checking. */
> +	__u32 hw_version;
> +	/* version of software, for feature checking. */
> +	__u32 sw_version;
> +	/* Job key */
> +	__u16 jkey;
> +	__u16 padding1;
> +	/*
> +	 * The special QP (queue pair) value that identifies PSM
> +	 * protocol packet from standard IB packets.
> +	 */
> +	__u32 bthqp;
> +	/* PIO credit return address, */
> +	__u64 sc_credits_addr;
> +	/*
> +	 * Base address of write-only pio buffers for this process.
> +	 * Each buffer has sendpio_credits*64 bytes.
> +	 */
> +	__u64 pio_bufbase_sop;
> +	/*
> +	 * Base address of write-only pio buffers for this process.
> +	 * Each buffer has sendpio_credits*64 bytes.
> +	 */
> +	__u64 pio_bufbase;
> +	/* address where receive buffer queue is mapped into */
> +	__u64 rcvhdr_bufbase;
> +	/* base address of Eager receive buffers. */
> +	__u64 rcvegr_bufbase;
> +	/* base address of SDMA completion ring */
> +	__u64 sdma_comp_bufbase;
> +	/*
> +	 * User register base for init code, not to be used directly by
> +	 * protocol or applications.  Always maps real chip register space.
> +	 * the register addresses are:
> +	 * ur_rcvhdrhead, ur_rcvhdrtail, ur_rcvegrhead, ur_rcvegrtail,
> +	 * ur_rcvtidflow
> +	 */
> +	__u64 user_regbase;
> +	/* notification events */
> +	__u64 events_bufbase;
> +	/* status page */
> +	__u64 status_bufbase;
> +	/* rcvhdrtail update */
> +	__u64 rcvhdrtail_base;
> +	/*
> +	 * shared memory pages for subctxts if ctxt is shared; these cover
> +	 * all the processes in the group sharing a single context.
> +	 * all have enough space for the num_subcontexts value on this job.
> +	 */
> +	__u64 subctxt_uregbase;
> +	__u64 subctxt_rcvegrbuf;
> +	__u64 subctxt_rcvhdrbuf;
> +};
> +#endif /* _LINIUX__HFI1_IOCTL_H */
> diff --git a/include/uapi/rdma/hfi/hfi1_user.h b/include/uapi/rdma/hfi/hfi1_user.h
> index a404919..8aa3867 100644
> --- a/include/uapi/rdma/hfi/hfi1_user.h
> +++ b/include/uapi/rdma/hfi/hfi1_user.h
> @@ -58,6 +58,7 @@
>  
>  #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
> @@ -211,60 +212,6 @@ struct hfi1_cmd;
>  #define HFI1_POLL_TYPE_ANYRCV     0x0
>  #define HFI1_POLL_TYPE_URGENT     0x1
>  
> -/*
> - * This structure is passed to the driver to tell it where
> - * user code buffers are, sizes, etc.   The offsets and sizes of the
> - * fields must remain unchanged, for binary compatibility.  It can
> - * be extended, if userversion is changed so user code can tell, if needed
> - */
> -struct hfi1_user_info {
> -	/*
> -	 * version of user software, to detect compatibility issues.
> -	 * Should be set to HFI1_USER_SWVERSION.
> -	 */
> -	__u32 userversion;
> -	__u32 pad;
> -	/*
> -	 * If two or more processes wish to share a context, each process
> -	 * must set the subcontext_cnt and subcontext_id to the same
> -	 * values.  The only restriction on the subcontext_id is that
> -	 * it be unique for a given node.
> -	 */
> -	__u16 subctxt_cnt;
> -	__u16 subctxt_id;
> -	/* 128bit UUID passed in by PSM. */
> -	__u8 uuid[16];
> -};
> -
> -struct hfi1_ctxt_info {
> -	__u64 runtime_flags;    /* chip/drv runtime flags (HFI1_CAP_*) */
> -	__u32 rcvegr_size;      /* size of each eager buffer */
> -	__u16 num_active;       /* number of active units */
> -	__u16 unit;             /* unit (chip) assigned to caller */
> -	__u16 ctxt;             /* ctxt on unit assigned to caller */
> -	__u16 subctxt;          /* subctxt on unit assigned to caller */
> -	__u16 rcvtids;          /* number of Rcv TIDs for this context */
> -	__u16 credits;          /* number of PIO credits for this context */
> -	__u16 numa_node;        /* NUMA node of the assigned device */
> -	__u16 rec_cpu;          /* cpu # for affinity (0xffff if none) */
> -	__u16 send_ctxt;        /* send context in use by this user context */
> -	__u16 egrtids;          /* number of RcvArray entries for Eager Rcvs */
> -	__u16 rcvhdrq_cnt;      /* number of RcvHdrQ entries */
> -	__u16 rcvhdrq_entsize;  /* size (in bytes) for each RcvHdrQ entry */
> -	__u16 sdma_ring_size;   /* number of entries in SDMA request ring */
> -};
> -
> -struct hfi1_tid_info {
> -	/* virtual address of first page in transfer */
> -	__u64 vaddr;
> -	/* pointer to tid array. this array is big enough */
> -	__u64 tidlist;
> -	/* number of tids programmed by this request */
> -	__u32 tidcnt;
> -	/* length of transfer buffer programmed by this request */
> -	__u32 length;
> -};
> -
>  enum hfi1_sdma_comp_state {
>  	FREE = 0,
>  	QUEUED,
> @@ -289,71 +236,6 @@ struct hfi1_status {
>  	char freezemsg[0];
>  };
>  
> -/*
> - * This structure is returned by the driver immediately after
> - * open to get implementation-specific info, and info specific to this
> - * instance.
> - *
> - * This struct must have explicit pad fields where type sizes
> - * may result in different alignments between 32 and 64 bit
> - * programs, since the 64 bit * bit kernel requires the user code
> - * to have matching offsets
> - */
> -struct hfi1_base_info {
> -	/* version of hardware, for feature checking. */
> -	__u32 hw_version;
> -	/* version of software, for feature checking. */
> -	__u32 sw_version;
> -	/* Job key */
> -	__u16 jkey;
> -	__u16 padding1;
> -	/*
> -	 * The special QP (queue pair) value that identifies PSM
> -	 * protocol packet from standard IB packets.
> -	 */
> -	__u32 bthqp;
> -	/* PIO credit return address, */
> -	__u64 sc_credits_addr;
> -	/*
> -	 * Base address of write-only pio buffers for this process.
> -	 * Each buffer has sendpio_credits*64 bytes.
> -	 */
> -	__u64 pio_bufbase_sop;
> -	/*
> -	 * Base address of write-only pio buffers for this process.
> -	 * Each buffer has sendpio_credits*64 bytes.
> -	 */
> -	__u64 pio_bufbase;
> -	/* address where receive buffer queue is mapped into */
> -	__u64 rcvhdr_bufbase;
> -	/* base address of Eager receive buffers. */
> -	__u64 rcvegr_bufbase;
> -	/* base address of SDMA completion ring */
> -	__u64 sdma_comp_bufbase;
> -	/*
> -	 * User register base for init code, not to be used directly by
> -	 * protocol or applications.  Always maps real chip register space.
> -	 * the register addresses are:
> -	 * ur_rcvhdrhead, ur_rcvhdrtail, ur_rcvegrhead, ur_rcvegrtail,
> -	 * ur_rcvtidflow
> -	 */
> -	__u64 user_regbase;
> -	/* notification events */
> -	__u64 events_bufbase;
> -	/* status page */
> -	__u64 status_bufbase;
> -	/* rcvhdrtail update */
> -	__u64 rcvhdrtail_base;
> -	/*
> -	 * shared memory pages for subctxts if ctxt is shared; these cover
> -	 * all the processes in the group sharing a single context.
> -	 * all have enough space for the num_subcontexts value on this job.
> -	 */
> -	__u64 subctxt_uregbase;
> -	__u64 subctxt_rcvegrbuf;
> -	__u64 subctxt_rcvhdrbuf;
> -};
> -
>  enum sdma_req_opcode {
>  	EXPECTED = 0,
>  	EAGER
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 4/6] RDMA/core: Move HFI1 IOCTL declarations to common file
       [not found]     ` <1471355123-6227-5-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2016-08-17  6:16       ` ira.weiny
       [not found]         ` <20160817061630.GB27477-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
  0 siblings, 1 reply; 26+ messages in thread
From: ira.weiny @ 2016-08-17  6:16 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky, Matan Barak,
	Haggai Eran

On Tue, Aug 16, 2016 at 04:45:21PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Move HFI1 IOCTL declarations to rdma_user_ioctl.h file.

I have not tried with the patch but I'm 99% sure this will break the PSM2
library build which includes hfi1_user.h.

This is one of those things I have pondered in the past.  Most of the rdma
libraries don't actually use these definitions directly.  PSM2 does.

I'm not sure what other libraries do.

In the final patch of this series you admit that the name changes in that patch
will break userspace which uses the defines directly.  Can we, should we, do
that?

To be completely pedantic I think we need to maintain the old names at least
for some time.  Perhaps they are best put in an rdma_user_compat_ioctl.h which
is included in the final rdma_user_ioctl.h?

For this patch I think it would be sufficient to include rdma_user_ioctl.h in
hfi1_user.h but I would have to double check that.

Ira

> 
> Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  include/uapi/rdma/hfi/hfi1_user.h   | 55 -------------------------------------
>  include/uapi/rdma/rdma_user_ioctl.h | 54 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+), 55 deletions(-)
> 
> diff --git a/include/uapi/rdma/hfi/hfi1_user.h b/include/uapi/rdma/hfi/hfi1_user.h
> index 8aa3867..8807f06 100644
> --- a/include/uapi/rdma/hfi/hfi1_user.h
> +++ b/include/uapi/rdma/hfi/hfi1_user.h
> @@ -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
> diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
> index 820bf34..e9a69f0 100644
> --- a/include/uapi/rdma/rdma_user_ioctl.h
> +++ b/include/uapi/rdma/rdma_user_ioctl.h
> @@ -36,6 +36,7 @@
>  #include <linux/types.h>
>  #include <linux/ioctl.h>
>  #include <rdma/ib_user_mad.h>
> +#include <rdma/hfi/hfi1_ioctl.h>
>  
>  /* Documentation/ioctl/ioctl-number.txt */
>  #define RDMA_IOCTL_MAGIC		0x1b
> @@ -51,4 +52,57 @@
>  #define IB_USER_MAD_REGISTER_AGENT2     _IOWR(IB_IOCTL_MAGIC, 4, \
>  					      struct ib_user_mad_reg_req2)
>  
> +/* 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)
> +
> +#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)
> +
>  #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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 3/6] RDMA/hfi1: Avoid redeclaration error
       [not found]         ` <20160817060139.GA27477-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
@ 2016-08-17  6:45           ` Leon Romanovsky
  0 siblings, 0 replies; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-17  6:45 UTC (permalink / raw)
  To: ira.weiny
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Matan Barak, Haggai Eran

[-- Attachment #1: Type: text/plain, Size: 1851 bytes --]

On Wed, Aug 17, 2016 at 02:01:40AM -0400, ira.weiny wrote:
> On Tue, Aug 16, 2016 at 04:45:20PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > 
> > Plugging HFI1 IOCTL defines will cause to the following
> > 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,
> 
> I was pretty confused about what you were doing here mainly because you
> reference a symbol you don't actually touch in the patch.
> 
> I see what you are doing now.  But a better explanation would be:
> 
> <quote>
> 
> 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:
> 
> 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,
> 
> 
> The actual move of the ioctl definitions comes in a follow on patch.
> </quote>
> 

It is definitely better.
I'll update it in next version.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 4/6] RDMA/core: Move HFI1 IOCTL declarations to common file
       [not found]         ` <20160817061630.GB27477-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
@ 2016-08-17  6:55           ` Leon Romanovsky
       [not found]             ` <20160817065529.GG5489-2ukJVAZIZ/Y@public.gmane.org>
  2016-08-19 18:32           ` Jason Gunthorpe
  1 sibling, 1 reply; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-17  6:55 UTC (permalink / raw)
  To: ira.weiny
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Matan Barak, Haggai Eran

[-- Attachment #1: Type: text/plain, Size: 9135 bytes --]

On Wed, Aug 17, 2016 at 02:16:31AM -0400, ira.weiny wrote:
> On Tue, Aug 16, 2016 at 04:45:21PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > 
> > Move HFI1 IOCTL declarations to rdma_user_ioctl.h file.
> 
> I have not tried with the patch but I'm 99% sure this will break the PSM2
> library build which includes hfi1_user.h.
> 
> This is one of those things I have pondered in the past.  Most of the rdma
> libraries don't actually use these definitions directly.  PSM2 does.

I'm not so convinced about it.
"#include <rdma/rdma_user_ioctl.h>" was added to hfi1_user.h to share
all definitions. PSM2 library will see it.

> 
> I'm not sure what other libraries do.
> 
> In the final patch of this series you admit that the name changes in that patch
> will break userspace which uses the defines directly.  Can we, should we, do
> that?

I'm talking about __NUM() macros.

Do you really use __NUM(ASSIGN_CTXT) in user application? Why did you do
it? You supposed to use HFI1_IOCTL_ASSIGN_CTXT instead.

> 
> To be completely pedantic I think we need to maintain the old names at least
> for some time.  Perhaps they are best put in an rdma_user_compat_ioctl.h which
> is included in the final rdma_user_ioctl.h?

Except __NUM() macro, there is no change in names.

> 
> For this patch I think it would be sufficient to include rdma_user_ioctl.h in
> hfi1_user.h but I would have to double check that.

It is already done in patches before.

> 
> Ira
> 
> > 
> > Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > ---
> >  include/uapi/rdma/hfi/hfi1_user.h   | 55 -------------------------------------
> >  include/uapi/rdma/rdma_user_ioctl.h | 54 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 54 insertions(+), 55 deletions(-)
> > 
> > diff --git a/include/uapi/rdma/hfi/hfi1_user.h b/include/uapi/rdma/hfi/hfi1_user.h
> > index 8aa3867..8807f06 100644
> > --- a/include/uapi/rdma/hfi/hfi1_user.h
> > +++ b/include/uapi/rdma/hfi/hfi1_user.h
> > @@ -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
> > diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
> > index 820bf34..e9a69f0 100644
> > --- a/include/uapi/rdma/rdma_user_ioctl.h
> > +++ b/include/uapi/rdma/rdma_user_ioctl.h
> > @@ -36,6 +36,7 @@
> >  #include <linux/types.h>
> >  #include <linux/ioctl.h>
> >  #include <rdma/ib_user_mad.h>
> > +#include <rdma/hfi/hfi1_ioctl.h>
> >  
> >  /* Documentation/ioctl/ioctl-number.txt */
> >  #define RDMA_IOCTL_MAGIC		0x1b
> > @@ -51,4 +52,57 @@
> >  #define IB_USER_MAD_REGISTER_AGENT2     _IOWR(IB_IOCTL_MAGIC, 4, \
> >  					      struct ib_user_mad_reg_req2)
> >  
> > +/* 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)
> > +
> > +#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)
> > +
> >  #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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 4/6] RDMA/core: Move HFI1 IOCTL declarations to common file
       [not found]             ` <20160817065529.GG5489-2ukJVAZIZ/Y@public.gmane.org>
@ 2016-08-17 13:35               ` Dalessandro, Dennis
       [not found]                 ` <1471440949.19634.7.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  2016-08-17 18:10               ` ira.weiny
  1 sibling, 1 reply; 26+ messages in thread
From: Dalessandro, Dennis @ 2016-08-17 13:35 UTC (permalink / raw)
  To: leon-DgEjT+Ai2ygdnm+yROfE0A, Weiny, Ira
  Cc: matanb-VPRAkNaXOzVWk0Htik3J/w, Dalessandro, Dennis,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	dledford-H+wXaHxf7aLQT0dZR+AlfA, haggaie-VPRAkNaXOzVWk0Htik3J/w

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1642 bytes --]

On Wed, 2016-08-17 at 09:55 +0300, Leon Romanovsky wrote:
> On Wed, Aug 17, 2016 at 02:16:31AM -0400, ira.weiny wrote:
> > On Tue, Aug 16, 2016 at 04:45:21PM +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@mellanox.com>
> > > 
> > > Move HFI1 IOCTL declarations to rdma_user_ioctl.h file.
> > 
> > I have not tried with the patch but I'm 99% sure this will break
> > the PSM2
> > library build which includes hfi1_user.h.
> > 
> > This is one of those things I have pondered in the past.  Most of
> > the rdma
> > libraries don't actually use these definitions directly.  PSM2
> > does.
> 
> I'm not so convinced about it.
> "#include <rdma/rdma_user_ioctl.h>" was added to hfi1_user.h to share
> all definitions. PSM2 library will see it.

On face value that should be true. I would want to run it by the PSM
folks and test it out though to be sure.



> > I'm not sure what other libraries do.
> > 
> > In the final patch of this series you admit that the name changes
> > in that patch
> > will break userspace which uses the defines directly.  Can we,
> > should we, do
> > that?
> 
> I'm talking about __NUM() macros.
> 
> Do you really use __NUM(ASSIGN_CTXT) in user application? Why did you
> do
> it? You supposed to use HFI1_IOCTL_ASSIGN_CTXT instead.

Yeah I think we can get rid of the __NUM() macro. It should not
strictly be needed by user space. User should be using the thing that
__NUM() was used in, the IOCTL number.

-DennyN‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 6/6] RDMA/core: Unify style of IOCTL commands
       [not found]                     ` <20160817051952.GC5489-2ukJVAZIZ/Y@public.gmane.org>
@ 2016-08-17 13:45                       ` Dalessandro, Dennis
       [not found]                         ` <1471441516.19634.15.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 26+ messages in thread
From: Dalessandro, Dennis @ 2016-08-17 13:45 UTC (permalink / raw)
  To: leon-DgEjT+Ai2ygdnm+yROfE0A
  Cc: matanb-VPRAkNaXOzVWk0Htik3J/w, Dalessandro, Dennis,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	dledford-H+wXaHxf7aLQT0dZR+AlfA, haggaie-VPRAkNaXOzVWk0Htik3J/w

On Wed, 2016-08-17 at 08:19 +0300, Leon Romanovsky wrote:


> > I'm not opposed to trying to unify things, however this seems to be
> > more than plop down the hfi1 stuff from here and put it over there.
> > It
> > is certainly not simplifying anything.
> 
> It is "dark side" of UAPI - inability to change legacy declarations.
> This is why I didn't remove anything except _NUM() macro.
> 
> The simplification comes from definition of one place for
> declaration of IOCTLs numbers and exporting it to users. It gives
> visibility for user space authors too.

How often does a user need the IOCTL numbers for MAD and PSM and
whatever else we come up with at the same time? Why would they care
about the other IOCTL numbers? 

I don't really see a problem with keeping them separate, but I'm not
really opposed to collapsing into one place either.

-Denny

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 4/6] RDMA/core: Move HFI1 IOCTL declarations to common file
       [not found]                 ` <1471440949.19634.7.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2016-08-17 14:16                   ` Leon Romanovsky
  0 siblings, 0 replies; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-17 14:16 UTC (permalink / raw)
  To: Dalessandro, Dennis
  Cc: Weiny, Ira, matanb-VPRAkNaXOzVWk0Htik3J/w,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	dledford-H+wXaHxf7aLQT0dZR+AlfA, haggaie-VPRAkNaXOzVWk0Htik3J/w

[-- Attachment #1: Type: text/plain, Size: 1053 bytes --]

On Wed, Aug 17, 2016 at 01:35:53PM +0000, Dalessandro, Dennis wrote:
> On Wed, 2016-08-17 at 09:55 +0300, Leon Romanovsky wrote:
> > On Wed, Aug 17, 2016 at 02:16:31AM -0400, ira.weiny wrote:
> > > On Tue, Aug 16, 2016 at 04:45:21PM +0300, Leon Romanovsky wrote:
> > > > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > > 
> > > > Move HFI1 IOCTL declarations to rdma_user_ioctl.h file.
> > > 
> > > I have not tried with the patch but I'm 99% sure this will break
> > > the PSM2
> > > library build which includes hfi1_user.h.
> > > 
> > > This is one of those things I have pondered in the past.  Most of
> > > the rdma
> > > libraries don't actually use these definitions directly.  PSM2
> > > does.
> > 
> > I'm not so convinced about it.
> > "#include <rdma/rdma_user_ioctl.h>" was added to hfi1_user.h to share
> > all definitions. PSM2 library will see it.
> 
> On face value that should be true. I would want to run it by the PSM
> folks and test it out though to be sure.

I agree,
Thanks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 6/6] RDMA/core: Unify style of IOCTL commands
       [not found]                         ` <1471441516.19634.15.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2016-08-17 14:20                           ` Leon Romanovsky
  0 siblings, 0 replies; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-17 14:20 UTC (permalink / raw)
  To: Dalessandro, Dennis
  Cc: matanb-VPRAkNaXOzVWk0Htik3J/w, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	dledford-H+wXaHxf7aLQT0dZR+AlfA, haggaie-VPRAkNaXOzVWk0Htik3J/w

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

On Wed, Aug 17, 2016 at 01:45:21PM +0000, Dalessandro, Dennis wrote:
> On Wed, 2016-08-17 at 08:19 +0300, Leon Romanovsky wrote:
> 
> 
> > > I'm not opposed to trying to unify things, however this seems to be
> > > more than plop down the hfi1 stuff from here and put it over there.
> > > It
> > > is certainly not simplifying anything.
> > 
> > It is "dark side" of UAPI - inability to change legacy declarations.
> > This is why I didn't remove anything except _NUM() macro.
> > 
> > The simplification comes from definition of one place for
> > declaration of IOCTLs numbers and exporting it to users. It gives
> > visibility for user space authors too.
> 
> How often does a user need the IOCTL numbers for MAD and PSM and
> whatever else we come up with at the same time? Why would they care
> about the other IOCTL numbers? 

We will need new IOCTLs numbers for ABI and better if they be different
from already defined.

> 
> I don't really see a problem with keeping them separate, but I'm not
> really opposed to collapsing into one place either.

Thanks

> 
> -Denny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 4/6] RDMA/core: Move HFI1 IOCTL declarations to common file
       [not found]             ` <20160817065529.GG5489-2ukJVAZIZ/Y@public.gmane.org>
  2016-08-17 13:35               ` Dalessandro, Dennis
@ 2016-08-17 18:10               ` ira.weiny
       [not found]                 ` <20160817181050.GC27477-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
  1 sibling, 1 reply; 26+ messages in thread
From: ira.weiny @ 2016-08-17 18:10 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Matan Barak, Haggai Eran

On Wed, Aug 17, 2016 at 09:55:29AM +0300, Leon Romanovsky wrote:
> On Wed, Aug 17, 2016 at 02:16:31AM -0400, ira.weiny wrote:
> > On Tue, Aug 16, 2016 at 04:45:21PM +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > 
> > > Move HFI1 IOCTL declarations to rdma_user_ioctl.h file.
> > 
> > I have not tried with the patch but I'm 99% sure this will break the PSM2
> > library build which includes hfi1_user.h.
> > 
> > This is one of those things I have pondered in the past.  Most of the rdma
> > libraries don't actually use these definitions directly.  PSM2 does.
> 
> I'm not so convinced about it.
> "#include <rdma/rdma_user_ioctl.h>" was added to hfi1_user.h to share
> all definitions. PSM2 library will see it.

Ok I see it now.  Sorry it was late.

> 
> > 
> > I'm not sure what other libraries do.
> > 
> > In the final patch of this series you admit that the name changes in that patch
> > will break userspace which uses the defines directly.  Can we, should we, do
> > that?
> 
> I'm talking about __NUM() macros.
> 
> Do you really use __NUM(ASSIGN_CTXT) in user application? Why did you do
> it? You supposed to use HFI1_IOCTL_ASSIGN_CTXT instead.

Your commit message says "... and MAD indexes were renamed.  It has the
potential to break application which use these defines directly."

I took that at face value.  I've taken a closer look ... I see now that neither
the numbers nor the define names changed so ok, my mistake.

Ira

> 
> > 
> > To be completely pedantic I think we need to maintain the old names at least
> > for some time.  Perhaps they are best put in an rdma_user_compat_ioctl.h which
> > is included in the final rdma_user_ioctl.h?
> 
> Except __NUM() macro, there is no change in names.
> 
> > 
> > For this patch I think it would be sufficient to include rdma_user_ioctl.h in
> > hfi1_user.h but I would have to double check that.
> 
> It is already done in patches before.
> 
> > 
> > Ira
> > 
> > > 
> > > Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > ---
> > >  include/uapi/rdma/hfi/hfi1_user.h   | 55 -------------------------------------
> > >  include/uapi/rdma/rdma_user_ioctl.h | 54 ++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 54 insertions(+), 55 deletions(-)
> > > 
> > > diff --git a/include/uapi/rdma/hfi/hfi1_user.h b/include/uapi/rdma/hfi/hfi1_user.h
> > > index 8aa3867..8807f06 100644
> > > --- a/include/uapi/rdma/hfi/hfi1_user.h
> > > +++ b/include/uapi/rdma/hfi/hfi1_user.h
> > > @@ -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
> > > diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
> > > index 820bf34..e9a69f0 100644
> > > --- a/include/uapi/rdma/rdma_user_ioctl.h
> > > +++ b/include/uapi/rdma/rdma_user_ioctl.h
> > > @@ -36,6 +36,7 @@
> > >  #include <linux/types.h>
> > >  #include <linux/ioctl.h>
> > >  #include <rdma/ib_user_mad.h>
> > > +#include <rdma/hfi/hfi1_ioctl.h>
> > >  
> > >  /* Documentation/ioctl/ioctl-number.txt */
> > >  #define RDMA_IOCTL_MAGIC		0x1b
> > > @@ -51,4 +52,57 @@
> > >  #define IB_USER_MAD_REGISTER_AGENT2     _IOWR(IB_IOCTL_MAGIC, 4, \
> > >  					      struct ib_user_mad_reg_req2)
> > >  
> > > +/* 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)
> > > +
> > > +#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)
> > > +
> > >  #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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 4/6] RDMA/core: Move HFI1 IOCTL declarations to common file
       [not found]                 ` <20160817181050.GC27477-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
@ 2016-08-18  6:23                   ` Leon Romanovsky
  0 siblings, 0 replies; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-18  6:23 UTC (permalink / raw)
  To: ira.weiny
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Matan Barak, Haggai Eran

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

On Wed, Aug 17, 2016 at 02:10:51PM -0400, ira.weiny wrote:
> On Wed, Aug 17, 2016 at 09:55:29AM +0300, Leon Romanovsky wrote:
> > On Wed, Aug 17, 2016 at 02:16:31AM -0400, ira.weiny wrote:
> > > On Tue, Aug 16, 2016 at 04:45:21PM +0300, Leon Romanovsky wrote:
> > > > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > > 
> > > > Move HFI1 IOCTL declarations to rdma_user_ioctl.h file.
> > > 
> > > I have not tried with the patch but I'm 99% sure this will break the PSM2
> > > library build which includes hfi1_user.h.
> > > 
> > > This is one of those things I have pondered in the past.  Most of the rdma
> > > libraries don't actually use these definitions directly.  PSM2 does.
> > 
> > I'm not so convinced about it.
> > "#include <rdma/rdma_user_ioctl.h>" was added to hfi1_user.h to share
> > all definitions. PSM2 library will see it.
> 
> Ok I see it now.  Sorry it was late.
> 
> > 
> > > 
> > > I'm not sure what other libraries do.
> > > 
> > > In the final patch of this series you admit that the name changes in that patch
> > > will break userspace which uses the defines directly.  Can we, should we, do
> > > that?
> > 
> > I'm talking about __NUM() macros.
> > 
> > Do you really use __NUM(ASSIGN_CTXT) in user application? Why did you do
> > it? You supposed to use HFI1_IOCTL_ASSIGN_CTXT instead.
> 
> Your commit message says "... and MAD indexes were renamed.  It has the
> potential to break application which use these defines directly."
> 
> I took that at face value.  I've taken a closer look ... I see now that neither
> the numbers nor the define names changed so ok, my mistake.

Sorry for inconvenience,
I'll update commit message, so it will better reflect reality.

Thanks


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 0/6] Refactor RDMA IOCTL declarations
       [not found] ` <1471355123-6227-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
                     ` (5 preceding siblings ...)
  2016-08-16 13:45   ` [PATCH rdma-next 6/6] RDMA/core: Unify style of IOCTL commands Leon Romanovsky
@ 2016-08-18 11:08   ` Yuval Shaia
       [not found]     ` <20160818110819.GB8590-Hxa29pjIrETlQW142y8m19+IiqhCXseY@public.gmane.org>
  6 siblings, 1 reply; 26+ messages in thread
From: Yuval Shaia @ 2016-08-18 11:08 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA, linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Tue, Aug 16, 2016 at 04:45:17PM +0300, Leon Romanovsky wrote:
> 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.

Just for my understanding - this is not an issue only with RDMA-IOCTLs,
right?
(Wondering how other modules approaches this).

> 
> 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.
> 
> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 0/6] Refactor RDMA IOCTL declarations
       [not found]     ` <20160818110819.GB8590-Hxa29pjIrETlQW142y8m19+IiqhCXseY@public.gmane.org>
@ 2016-08-18 11:28       ` Leon Romanovsky
  0 siblings, 0 replies; 26+ messages in thread
From: Leon Romanovsky @ 2016-08-18 11:28 UTC (permalink / raw)
  To: Yuval Shaia
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA, linux-rdma-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 930 bytes --]

On Thu, Aug 18, 2016 at 02:08:20PM +0300, Yuval Shaia wrote:
> On Tue, Aug 16, 2016 at 04:45:17PM +0300, Leon Romanovsky wrote:
> > 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.
> 
> Just for my understanding - this is not an issue only with RDMA-IOCTLs,
> right?
> (Wondering how other modules approaches this).

I'm not familiar with all subsystems, but IMHO, we will find all
possible solutions. While working on the patches, I took a look on
other subsystems and they exposed structures in the same file
as IOCTLs from the day one.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH rdma-next 4/6] RDMA/core: Move HFI1 IOCTL declarations to common file
       [not found]         ` <20160817061630.GB27477-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
  2016-08-17  6:55           ` Leon Romanovsky
@ 2016-08-19 18:32           ` Jason Gunthorpe
  1 sibling, 0 replies; 26+ messages in thread
From: Jason Gunthorpe @ 2016-08-19 18:32 UTC (permalink / raw)
  To: ira.weiny
  Cc: Leon Romanovsky, dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky, Matan Barak,
	Haggai Eran

On Wed, Aug 17, 2016 at 02:16:31AM -0400, ira.weiny wrote:
> On Tue, Aug 16, 2016 at 04:45:21PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > 
> > Move HFI1 IOCTL declarations to rdma_user_ioctl.h file.
> 
> I have not tried with the patch but I'm 99% sure this will break the PSM2
> library build which includes hfi1_user.h.
> 
> This is one of those things I have pondered in the past.  Most of the rdma
> libraries don't actually use these definitions directly.  PSM2 does.
> 
> I'm not sure what other libraries do.

They should all be fixed to use the new uapi headers instead of
cloning them privately. If we go ahead with the rdma-plumbing I might
even do it..

> In the final patch of this series you admit that the name changes in that patch
> will break userspace which uses the defines directly.  Can we, should we, do
> that?

This happens from time to time, userspace should gain a build-time
test to deal with it, if it is a big enough deal, IMHO.

We don't guarentee 100% source compatibility..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2016-08-19 18:32 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-16 13:45 [PATCH rdma-next 0/6] Refactor RDMA IOCTL declarations Leon Romanovsky
     [not found] ` <1471355123-6227-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2016-08-16 13:45   ` [PATCH rdma-next 1/6] RDMA/core: Commonize RDMA IOCTL declarations location Leon Romanovsky
2016-08-16 13:45   ` [PATCH rdma-next 2/6] RDMA/core: Move legacy MAD IOCTL declarations to common file Leon Romanovsky
2016-08-16 13:45   ` [PATCH rdma-next 3/6] RDMA/hfi1: Avoid redeclaration error Leon Romanovsky
     [not found]     ` <1471355123-6227-4-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2016-08-16 17:15       ` Dalessandro, Dennis
     [not found]         ` <1471367729.2661.50.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2016-08-17  5:09           ` Leon Romanovsky
2016-08-17  6:01       ` ira.weiny
     [not found]         ` <20160817060139.GA27477-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
2016-08-17  6:45           ` Leon Romanovsky
2016-08-16 13:45   ` [PATCH rdma-next 4/6] RDMA/core: Move HFI1 IOCTL declarations to common file Leon Romanovsky
     [not found]     ` <1471355123-6227-5-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2016-08-17  6:16       ` ira.weiny
     [not found]         ` <20160817061630.GB27477-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
2016-08-17  6:55           ` Leon Romanovsky
     [not found]             ` <20160817065529.GG5489-2ukJVAZIZ/Y@public.gmane.org>
2016-08-17 13:35               ` Dalessandro, Dennis
     [not found]                 ` <1471440949.19634.7.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2016-08-17 14:16                   ` Leon Romanovsky
2016-08-17 18:10               ` ira.weiny
     [not found]                 ` <20160817181050.GC27477-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
2016-08-18  6:23                   ` Leon Romanovsky
2016-08-19 18:32           ` Jason Gunthorpe
2016-08-16 13:45   ` [PATCH rdma-next 5/6] RDMA/core: Rename RDMA magic number Leon Romanovsky
2016-08-16 13:45   ` [PATCH rdma-next 6/6] RDMA/core: Unify style of IOCTL commands Leon Romanovsky
     [not found]     ` <1471355123-6227-7-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2016-08-16 14:31       ` Dalessandro, Dennis
     [not found]         ` <1471357887.2661.28.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2016-08-16 16:50           ` Leon Romanovsky
     [not found]             ` <20160816165041.GA5489-2ukJVAZIZ/Y@public.gmane.org>
2016-08-16 17:09               ` Dalessandro, Dennis
     [not found]                 ` <1471367364.2661.45.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2016-08-17  5:19                   ` Leon Romanovsky
     [not found]                     ` <20160817051952.GC5489-2ukJVAZIZ/Y@public.gmane.org>
2016-08-17 13:45                       ` Dalessandro, Dennis
     [not found]                         ` <1471441516.19634.15.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2016-08-17 14:20                           ` Leon Romanovsky
2016-08-18 11:08   ` [PATCH rdma-next 0/6] Refactor RDMA IOCTL declarations Yuval Shaia
     [not found]     ` <20160818110819.GB8590-Hxa29pjIrETlQW142y8m19+IiqhCXseY@public.gmane.org>
2016-08-18 11:28       ` Leon Romanovsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.