[PATCH for-next 0/4] User-space time-stamping support for mlx5_ib

[PATCH for-next 0/4] User-space time-stamping support for mlx5_ib

[Matan Barak]

Hi Eli,

This patch-set adds user-space support for time-stamping in mlx5_ib.
It implements the necessary API:
(a) ib_create_cq_ex - Add support for CQ creation flags
(b) ib_query_device - return timestamp_mask and hca_core_clock.

We also add support for mmaping the HCA's free running clock.
In order to do so, we use the response of the vendor's extended
part in ib_query_device. This allows us to pass the page offset
of the free running clock register to the user-space driver.
In order to implement it in a future extensible manner, we  use the
same mechanism of verbs extensions to the mlx5 vendor part as well.

Regards,
Matan



Matan Barak (4):
  IB/mlx5: Add create_cq extended command
  IB/core: Add ib_is_udata_cleared
  IB/mlx5: Add support querying timestamp related fields in query_device
  IB/mlx5: Mmap the HCA's core clock register to user-space

 drivers/infiniband/hw/mlx5/cq.c      |  7 ++++
 drivers/infiniband/hw/mlx5/main.c    | 69 ++++++++++++++++++++++++++++++++++--
 drivers/infiniband/hw/mlx5/mlx5_ib.h | 22 +++++++++++-
 include/linux/mlx5/device.h          | 10 +++---
 include/linux/mlx5/mlx5_ifc.h        |  9 +++--
 include/rdma/ib_verbs.h              | 20 +++++++++++
 6 files changed, 125 insertions(+), 12 deletions(-)

-- 
2.1.0

--
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

[PATCH for-next 1/4] IB/mlx5: Add create_cq extended command

[Matan Barak]

In order to create a CQ that supports timestamp, mlx5 needs to
support the extended create CQ command with the timestamp flag.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/cq.c   | 7 +++++++
 drivers/infiniband/hw/mlx5/main.c | 3 ++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
index 2d0dbbf..674f857 100644
--- a/drivers/infiniband/hw/mlx5/cq.c
+++ b/drivers/infiniband/hw/mlx5/cq.c
@@ -743,6 +743,10 @@ static void destroy_cq_kernel(struct mlx5_ib_dev *dev, struct mlx5_ib_cq *cq)
 	mlx5_db_free(dev->mdev, &cq->db);
 }
 
+enum {
+	CQ_CREATE_FLAGS_SUPPORTED = IB_CQ_FLAGS_TIMESTAMP_COMPLETION
+};
+
 struct ib_cq *mlx5_ib_create_cq(struct ib_device *ibdev,
 				const struct ib_cq_init_attr *attr,
 				struct ib_ucontext *context,
@@ -766,6 +770,9 @@ struct ib_cq *mlx5_ib_create_cq(struct ib_device *ibdev,
 	if (entries < 0)
 		return ERR_PTR(-EINVAL);
 
+	if (attr->flags & ~CQ_CREATE_FLAGS_SUPPORTED)
+		return ERR_PTR(-EINVAL);
+
 	entries = roundup_pow_of_two(entries + 1);
 	if (entries > (1 << MLX5_CAP_GEN(dev->mdev, log_max_cq_sz)))
 		return ERR_PTR(-EINVAL);
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index f1ccd40..05f00da 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -1385,7 +1385,8 @@ static void *mlx5_ib_add(struct mlx5_core_dev *mdev)
 		(1ull << IB_USER_VERBS_CMD_CREATE_XSRQ)		|
 		(1ull << IB_USER_VERBS_CMD_OPEN_QP);
 	dev->ib_dev.uverbs_ex_cmd_mask =
-		(1ull << IB_USER_VERBS_EX_CMD_QUERY_DEVICE);
+		(1ull << IB_USER_VERBS_EX_CMD_QUERY_DEVICE)	|
+		(1ull << IB_USER_VERBS_EX_CMD_CREATE_CQ);
 
 	dev->ib_dev.query_device	= mlx5_ib_query_device;
 	dev->ib_dev.query_port		= mlx5_ib_query_port;
-- 
2.1.0

--
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

[PATCH for-next 2/4] IB/core: Add ib_is_udata_cleared

[Matan Barak]

Extending core and vendor verb commands require us to check that the
unknown part of the user's given command is all zeros.
Adding ib_is_udata_cleared in order to do so.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 include/rdma/ib_verbs.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index e4cc389..43f3cf2 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -1855,6 +1855,26 @@ static inline int ib_copy_to_udata(struct ib_udata *udata, void *src, size_t len
 	return copy_to_user(udata->outbuf, src, len) ? -EFAULT : 0;
 }
 
+static inline bool ib_is_udata_cleared(struct ib_udata *udata,
+				       char cleared_char,
+				       size_t offset,
+				       size_t len)
+{
+	short i;
+
+	for (i = 0; i < len; i++) {
+		char c;
+
+		if (copy_from_user(&c, udata->inbuf + offset + i, sizeof(c)))
+			return false;
+
+		if (c != cleared_char)
+			return false;
+	}
+
+	return true;
+}
+
 /**
  * ib_modify_qp_is_ok - Check that the supplied attribute mask
  * contains all required attributes and no attributes not allowed for
-- 
2.1.0

--
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

[PATCH for-next 3/4] IB/mlx5: Add support querying timestamp related fields in query_device

[Matan Barak]

Add support for querying hca_core_lock, timestmap_mask and
hca_core_clock_offset in query device verb. This is necessary
in order to support completion timestamp and querying the
HCA's core clock.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/main.c    | 42 ++++++++++++++++++++++++++++++++++--
 drivers/infiniband/hw/mlx5/mlx5_ib.h | 18 ++++++++++++++++
 include/linux/mlx5/device.h          | 10 ++++-----
 include/linux/mlx5/mlx5_ifc.h        |  9 +++++---
 4 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 05f00da..b8be3ad 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -213,10 +213,33 @@ static int mlx5_ib_query_device(struct ib_device *ibdev,
 	int max_rq_sg;
 	int max_sq_sg;
 	u64 min_page_size = 1ull << MLX5_CAP_GEN(mdev, log_pg_sz);
+	struct mlx5_uverbs_ex_query_device cmd = {};
+	struct mlx5_uverbs_ex_query_device_resp resp = {};
 
-	if (uhw->inlen || uhw->outlen)
-		return -EINVAL;
+	if (uhw->inlen) {
+		if (uhw->inlen < offsetof(struct mlx5_uverbs_ex_query_device,
+					  comp_mask) +
+				 sizeof(cmd.comp_mask))
+			return -EINVAL;
+
+		err = ib_copy_from_udata(&cmd, uhw, min(sizeof(cmd),
+							uhw->inlen));
+		if (err)
+			return err;
+
+		if (cmd.comp_mask)
+			return -EINVAL;
+
+		if (cmd.reserved)
+			return -EINVAL;
 
+		if (!ib_is_udata_cleared(uhw, '\0', sizeof(cmd),
+					 sizeof(cmd) - uhw->inlen))
+			return -EINVAL;
+	}
+
+	resp.response_length = offsetof(typeof(resp), response_length) +
+		sizeof(resp.response_length);
 	memset(props, 0, sizeof(*props));
 	err = mlx5_query_system_image_guid(ibdev,
 					   &props->sys_image_guid);
@@ -293,6 +316,8 @@ static int mlx5_ib_query_device(struct ib_device *ibdev,
 	props->max_total_mcast_qp_attach = props->max_mcast_qp_attach *
 					   props->max_mcast_grp;
 	props->max_map_per_fmr = INT_MAX; /* no limit in ConnectIB */
+	props->hca_core_clock = MLX5_CAP_GEN(mdev, device_frequency_khz);
+	props->timestamp_mask = 0xFFFFFFFFFFFFFFULL;
 
 #ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
 	if (MLX5_CAP_GEN(mdev, pg))
@@ -300,6 +325,19 @@ static int mlx5_ib_query_device(struct ib_device *ibdev,
 	props->odp_caps = dev->odp_caps;
 #endif
 
+	if (field_avail(typeof(resp), hca_core_clock_offset, uhw->outlen)) {
+		resp.response_length += sizeof(resp.hca_core_clock_offset);
+		resp.comp_mask |= QUERY_DEVICE_RESP_MASK_TIMESTAMP;
+		resp.hca_core_clock_offset =
+			offsetof(struct mlx5_init_seg, internal_timer_h) % PAGE_SIZE;
+	}
+
+	if (uhw->outlen) {
+		err = ib_copy_to_udata(uhw, &resp, resp.response_length);
+		if (err)
+			return err;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 22123b7..c33cf8f 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -55,6 +55,9 @@ pr_err("%s:%s:%d:(pid %d): " format, (dev)->ib_dev.name, __func__,	\
 pr_warn("%s:%s:%d:(pid %d): " format, (dev)->ib_dev.name, __func__,	\
 	__LINE__, current->pid, ##arg)
 
+#define field_avail(type, fld, sz) (offsetof(type, fld) +		\
+				    sizeof(((type *)0)->fld) <= (sz))
+
 enum {
 	MLX5_IB_MMAP_CMD_SHIFT	= 8,
 	MLX5_IB_MMAP_CMD_MASK	= 0xff,
@@ -441,6 +444,21 @@ struct mlx5_ib_dev {
 #endif
 };
 
+struct mlx5_uverbs_ex_query_device {
+	__u32 comp_mask;
+	__u32 reserved;
+};
+
+enum query_device_resp_mask {
+	QUERY_DEVICE_RESP_MASK_TIMESTAMP = 1UL << 0,
+};
+
+struct mlx5_uverbs_ex_query_device_resp {
+	__u32 comp_mask;
+	__u32 response_length;
+	__u64 hca_core_clock_offset;
+};
+
 static inline struct mlx5_ib_cq *to_mibcq(struct mlx5_core_cq *mcq)
 {
 	return container_of(mcq, struct mlx5_ib_cq, mcq);
diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
index 250b1ff..b7d7471 100644
--- a/include/linux/mlx5/device.h
+++ b/include/linux/mlx5/device.h
@@ -441,12 +441,12 @@ struct mlx5_init_seg {
 	__be32			cmd_dbell;
 	__be32			rsvd1[121];
 	struct health_buffer	health;
-	__be32			rsvd2[884];
+	__be32			rsvd2[880];
+	__be32			internal_timer_h;
+	__be32			internal_timer_l;
+	__be32			rsvd3[2];
 	__be32			health_counter;
-	__be32			rsvd3[1019];
-	__be64			ieee1588_clk;
-	__be32			ieee1588_clk_type;
-	__be32			clr_intx;
+	__be32			rsvd4[1019];
 };
 
 struct mlx5_eqe_comp {
diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index dd20974..4172db6 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -792,15 +792,18 @@ struct mlx5_ifc_cmd_hca_cap_bits {
 	u8         reserved_63[0x8];
 	u8         log_uar_page_sz[0x10];
 
-	u8         reserved_64[0x100];
+	u8	   reserved_64[0x20];
+	u8	   device_frequency_mhz[0x20];
+	u8	   device_frequency_khz[0x20];
+	u8         reserved_65[0xa0];
 
-	u8         reserved_65[0x1f];
+	u8         reserved_66[0x1f];
 	u8         cqe_zip[0x1];
 
 	u8         cqe_zip_timeout[0x10];
 	u8         cqe_zip_max_num[0x10];
 
-	u8         reserved_66[0x220];
+	u8         reserved_67[0x220];
 };
 
 enum {
-- 
2.1.0

--
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

[PATCH for-next 4/4] IB/mlx5: Mmap the HCA's core clock register to user-space

[Matan Barak]

In order to read the HCA's current cycles register, we need
to map it to user-space. Add support to map this register
via mmap command.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/main.c    | 24 ++++++++++++++++++++++++
 drivers/infiniband/hw/mlx5/mlx5_ib.h |  4 +++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index b8be3ad..23cedb4 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -825,6 +825,30 @@ static int mlx5_ib_mmap(struct ib_ucontext *ibcontext, struct vm_area_struct *vm
 	case MLX5_IB_MMAP_GET_CONTIGUOUS_PAGES:
 		return -ENOSYS;
 
+	case MLX5_IB_MMAP_CORE_CLOCK:
+	{
+		phys_addr_t pfn;
+
+		if (vma->vm_end - vma->vm_start != PAGE_SIZE)
+			return -EINVAL;
+
+		if (vma->vm_flags & (VM_WRITE | VM_EXEC))
+			return -EINVAL;
+
+		vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+		pfn = (dev->mdev->iseg_base +
+		       offsetof(struct mlx5_init_seg, internal_timer_h)) >>
+			PAGE_SHIFT;
+		if (io_remap_pfn_range(vma, vma->vm_start, pfn,
+				       PAGE_SIZE, vma->vm_page_prot))
+			return -EAGAIN;
+
+		mlx5_ib_dbg(dev, "mapped internal timer at 0x%lx, PA 0x%llx\n",
+			    vma->vm_start,
+			    (unsigned long long)pfn << PAGE_SHIFT);
+		break;
+	}
+
 	default:
 		return -EINVAL;
 	}
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index c33cf8f..87cd616 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -65,7 +65,9 @@ enum {
 
 enum mlx5_ib_mmap_cmd {
 	MLX5_IB_MMAP_REGULAR_PAGE		= 0,
-	MLX5_IB_MMAP_GET_CONTIGUOUS_PAGES	= 1, /* always last */
+	MLX5_IB_MMAP_GET_CONTIGUOUS_PAGES	= 1,
+	/* 5 is chosen in order to be compatible with old versions of libmlx5 */
+	MLX5_IB_MMAP_CORE_CLOCK			= 5,
 };
 
 enum {
-- 
2.1.0

--
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

Re: [PATCH for-next 1/4] IB/mlx5: Add create_cq extended command

[Eli Cohen]

On Mon, Nov 09, 2015 at 06:30:54PM +0200, Matan Barak wrote:
> In order to create a CQ that supports timestamp, mlx5 needs to
> support the extended create CQ command with the timestamp flag.
> 
i> Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Acked-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
--
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

Re: [PATCH for-next 1/4] IB/mlx5: Add create_cq extended command

[Eli Cohen]

On Mon, Nov 09, 2015 at 06:30:54PM +0200, Matan Barak wrote:
> In order to create a CQ that supports timestamp, mlx5 needs to
> support the extended create CQ command with the timestamp flag.
> 
> Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Acked-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
--
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

Re: [PATCH for-next 2/4] IB/core: Add ib_is_udata_cleared

[Eli Cohen]

On Mon, Nov 09, 2015 at 06:30:55PM +0200, Matan Barak wrote:
> Extending core and vendor verb commands require us to check that the
> unknown part of the user's given command is all zeros.
> Adding ib_is_udata_cleared in order to do so.
> 
> Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Acked-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
--
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

Re: [PATCH for-next 3/4] IB/mlx5: Add support querying timestamp related fields in query_device

[Eli Cohen]

On Mon, Nov 09, 2015 at 06:30:56PM +0200, Matan Barak wrote:
> Add support for querying hca_core_lock, timestmap_mask and
> hca_core_clock_offset in query device verb. This is necessary
> in order to support completion timestamp and querying the
> HCA's core clock.
> 
> Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Acked-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
--
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

Re: [PATCH for-next 4/4] IB/mlx5: Mmap the HCA's core clock register to user-space

[Eli Cohen]

On Mon, Nov 09, 2015 at 06:30:57PM +0200, Matan Barak wrote:
> In order to read the HCA's current cycles register, we need
> to map it to user-space. Add support to map this register
> via mmap command.
> 
> Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Acked-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
--
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

Re: [PATCH for-next 2/4] IB/core: Add ib_is_udata_cleared

[Leon Romanovsky]

On Mon, Nov 09, 2015 at 06:30:55PM +0200, Matan Barak wrote:
>  
> +static inline bool ib_is_udata_cleared(struct ib_udata *udata,
> +				       char cleared_char,
> +				       size_t offset,
> +				       size_t len)
> +{
> +	short i;
> +
> +	for (i = 0; i < len; i++) {
You are comparing "len" which is declared as size_t which is "unsigned" int and "i" which is "short".
--
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

Re: [PATCH for-next 3/4] IB/mlx5: Add support querying timestamp related fields in query_device

[Leon Romanovsky]

On Mon, Nov 09, 2015 at 06:30:56PM +0200, Matan Barak wrote:
> +
> +	if (uhw->outlen) {
> +		err = ib_copy_to_udata(uhw, &resp, resp.response_length);
> +		if (err)
> +			return err;
> +	}
> +
>  	return 0;
What do you think about to rewrite this part of code to be something
like that?
+	int ret = 0;
.....
+	if (uhw->outlen)
+		ret = ib_copy_to_udata(uhw, &resp, resp.response_length);
+	return ret;
--
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

Re: [PATCH for-next 1/4] IB/mlx5: Add create_cq extended command

[Jason Gunthorpe]

On Mon, Nov 09, 2015 at 06:30:54PM +0200, Matan Barak wrote:
> In order to create a CQ that supports timestamp, mlx5 needs to
> support the extended create CQ command with the timestamp flag.
> 
> Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>  drivers/infiniband/hw/mlx5/cq.c   | 7 +++++++
>  drivers/infiniband/hw/mlx5/main.c | 3 ++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
> index 2d0dbbf..674f857 100644
> +++ b/drivers/infiniband/hw/mlx5/cq.c
> @@ -743,6 +743,10 @@ static void destroy_cq_kernel(struct mlx5_ib_dev *dev, struct mlx5_ib_cq *cq)
>  	mlx5_db_free(dev->mdev, &cq->db);
>  }
>  
> +enum {
> +	CQ_CREATE_FLAGS_SUPPORTED = IB_CQ_FLAGS_TIMESTAMP_COMPLETION
> +};
> +
>  struct ib_cq *mlx5_ib_create_cq(struct ib_device *ibdev,
>  				const struct ib_cq_init_attr *attr,
>  				struct ib_ucontext *context,
> @@ -766,6 +770,9 @@ struct ib_cq *mlx5_ib_create_cq(struct ib_device *ibdev,
>  	if (entries < 0)
>  		return ERR_PTR(-EINVAL);
>  
> +	if (attr->flags & ~CQ_CREATE_FLAGS_SUPPORTED)
> +		return ERR_PTR(-EINVAL);

And this is what I was just mentioning to Eli, EINVAL is not the right
return, and this same comment applies to the places where the above
was copy and pasted into drivers during the ex patching.

Try for EOPNOTSUPP maybe?

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

Re: [PATCH for-next 1/4] IB/mlx5: Add create_cq extended command

[Jason Gunthorpe]

On Mon, Nov 09, 2015 at 06:30:54PM +0200, Matan Barak wrote:
> @@ -1385,7 +1385,8 @@ static void *mlx5_ib_add(struct mlx5_core_dev *mdev)
>  		(1ull << IB_USER_VERBS_CMD_CREATE_XSRQ)		|
>  		(1ull << IB_USER_VERBS_CMD_OPEN_QP);
>  	dev->ib_dev.uverbs_ex_cmd_mask =
> -		(1ull << IB_USER_VERBS_EX_CMD_QUERY_DEVICE);
> +		(1ull << IB_USER_VERBS_EX_CMD_QUERY_DEVICE)	|
> +		(1ull << IB_USER_VERBS_EX_CMD_CREATE_CQ);

Eli posted a series that gets rid of this stuff, can you please
coordinate?

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

Re: [PATCH for-next 1/4] IB/mlx5: Add create_cq extended command

[Matan Barak]

On Tue, Nov 10, 2015 at 1:22 AM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> On Mon, Nov 09, 2015 at 06:30:54PM +0200, Matan Barak wrote:
>> In order to create a CQ that supports timestamp, mlx5 needs to
>> support the extended create CQ command with the timestamp flag.
>>
>> Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>  drivers/infiniband/hw/mlx5/cq.c   | 7 +++++++
>>  drivers/infiniband/hw/mlx5/main.c | 3 ++-
>>  2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
>> index 2d0dbbf..674f857 100644
>> +++ b/drivers/infiniband/hw/mlx5/cq.c
>> @@ -743,6 +743,10 @@ static void destroy_cq_kernel(struct mlx5_ib_dev *dev, struct mlx5_ib_cq *cq)
>>       mlx5_db_free(dev->mdev, &cq->db);
>>  }
>>
>> +enum {
>> +     CQ_CREATE_FLAGS_SUPPORTED = IB_CQ_FLAGS_TIMESTAMP_COMPLETION
>> +};
>> +
>>  struct ib_cq *mlx5_ib_create_cq(struct ib_device *ibdev,
>>                               const struct ib_cq_init_attr *attr,
>>                               struct ib_ucontext *context,
>> @@ -766,6 +770,9 @@ struct ib_cq *mlx5_ib_create_cq(struct ib_device *ibdev,
>>       if (entries < 0)
>>               return ERR_PTR(-EINVAL);
>>
>> +     if (attr->flags & ~CQ_CREATE_FLAGS_SUPPORTED)
>> +             return ERR_PTR(-EINVAL);
>
> And this is what I was just mentioning to Eli, EINVAL is not the right
> return, and this same comment applies to the places where the above
> was copy and pasted into drivers during the ex patching.
>
> Try for EOPNOTSUPP maybe?
>

Agree. Thanks, I'll fix.

> Jason

Matan

> --
> 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

Re: [PATCH for-next 1/4] IB/mlx5: Add create_cq extended command

[Matan Barak]

On Tue, Nov 10, 2015 at 1:24 AM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> On Mon, Nov 09, 2015 at 06:30:54PM +0200, Matan Barak wrote:
>> @@ -1385,7 +1385,8 @@ static void *mlx5_ib_add(struct mlx5_core_dev *mdev)
>>               (1ull << IB_USER_VERBS_CMD_CREATE_XSRQ)         |
>>               (1ull << IB_USER_VERBS_CMD_OPEN_QP);
>>       dev->ib_dev.uverbs_ex_cmd_mask =
>> -             (1ull << IB_USER_VERBS_EX_CMD_QUERY_DEVICE);
>> +             (1ull << IB_USER_VERBS_EX_CMD_QUERY_DEVICE)     |
>> +             (1ull << IB_USER_VERBS_EX_CMD_CREATE_CQ);
>
> Eli posted a series that gets rid of this stuff, can you please
> coordinate?
>

It'll create a dependency between this series and Eli's series, but
I'll change that.

> Jason

Matan

> --
> 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

Re: [PATCH for-next 2/4] IB/core: Add ib_is_udata_cleared

[Matan Barak]

On Mon, Nov 9, 2015 at 9:18 PM, Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org> wrote:
> On Mon, Nov 09, 2015 at 06:30:55PM +0200, Matan Barak wrote:
>>
>> +static inline bool ib_is_udata_cleared(struct ib_udata *udata,
>> +                                    char cleared_char,
>> +                                    size_t offset,
>> +                                    size_t len)
>> +{
>> +     short i;
>> +
>> +     for (i = 0; i < len; i++) {
> You are comparing "len" which is declared as size_t which is "unsigned" int and "i" which is "short".

Thanks, I'll change i to be unsigned.

> --
> 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

Re: [PATCH for-next 3/4] IB/mlx5: Add support querying timestamp related fields in query_device

[Matan Barak]

On Mon, Nov 9, 2015 at 9:26 PM, Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org> wrote:
> On Mon, Nov 09, 2015 at 06:30:56PM +0200, Matan Barak wrote:
>> +
>> +     if (uhw->outlen) {
>> +             err = ib_copy_to_udata(uhw, &resp, resp.response_length);
>> +             if (err)
>> +                     return err;
>> +     }
>> +
>>       return 0;
> What do you think about to rewrite this part of code to be something
> like that?
> +       int ret = 0;
> .....
> +       if (uhw->outlen)
> +               ret = ib_copy_to_udata(uhw, &resp, resp.response_length);
> +       return ret;

I'll change that. Thanks.

> --
> 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

Re: [PATCH for-next 1/4] IB/mlx5: Add create_cq extended command

[Christoph Hellwig]

> +enum {
> +	CQ_CREATE_FLAGS_SUPPORTED = IB_CQ_FLAGS_TIMESTAMP_COMPLETION
> +};


How does userspace know the value of IB_CQ_FLAGS_TIMESTAMP_COMPLETION?

It's not defined in any UAPI header.
--
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

Re: [PATCH for-next 1/4] IB/mlx5: Add create_cq extended command

[Matan Barak]

On Thu, Nov 19, 2015 at 4:54 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>> +enum {
>> +     CQ_CREATE_FLAGS_SUPPORTED = IB_CQ_FLAGS_TIMESTAMP_COMPLETION
>> +};
>
>
> How does userspace know the value of IB_CQ_FLAGS_TIMESTAMP_COMPLETION?
>
> It's not defined in any UAPI header.

Currently, a lot of flags are defined both in rdma/ib_verbs.h and are
redefined as part of libibverbs.
For example, IB_ODP_SUPPORT is defined in rdma/ib_verbs.h and
redefined in libibverbs. We took the same approach here.

We could put all of these flags in uverbs, but I think we should be
consistent. So either only uapi exclusive structures should be placed
in
uapi or all uapi related data should be placed there.

Regards,
Matan

> --
> 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

Re: [PATCH for-next 1/4] IB/mlx5: Add create_cq extended command

[Christoph Hellwig]

Hi Matan,

IB_CQ_FLAGS_TIMESTAMP_COMPLETION is _only_ used in the uverbs interface.

But other than that userspace ABIs really must go to the UAPI headers,
otherwise they will be broken sooner or later.  RDMA is an unfortunate
oddball under the kernel subsystems that's been ignoring the rule for
now.  Please ensure at least new interfaces follow this principle, and
old code will need to be fixed up as well.
--
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

Re: [PATCH for-next 1/4] IB/mlx5: Add create_cq extended command

[Matan Barak]

On Sun, Nov 22, 2015 at 12:15 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> Hi Matan,
>
> IB_CQ_FLAGS_TIMESTAMP_COMPLETION is _only_ used in the uverbs interface.
>

Currently this is used only for uverbs interface. The reason for that
is that we've only pushed completion timestamping for user-space
support.
However, there's no reason not to add support for completion
timestamping for ULPs. Therefore, the same flag could be relevant for
ULPs as well.

> But other than that userspace ABIs really must go to the UAPI headers,
> otherwise they will be broken sooner or later.  RDMA is an unfortunate
> oddball under the kernel subsystems that's been ignoring the rule for

So do you imply ib_verbs.h should include ib_user_verbs.h?

> now.  Please ensure at least new interfaces follow this principle, and
> old code will need to be fixed up as well.

Ok, I'll follow this principle for new features.

Matan
--
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