[PATCH for-next 00/14] Patch set for 3.14

[PATCH for-next 00/14] Patch set for 3.14

[Eli Cohen]

Hi Roland,

This is mlx5 update for 3.14, it includes fixes and small features
(implementation for the cq modify and resize verbs). The patches contain
changes to both mlx5_ib and mlx5_core but for the sake of simplicity I am
sending you all of them.

Thanks,
Eli

Dan Carpenter (1):
  mlx5_core: Remove dead code

Eli Cohen (12):
  IB/mlx5: Remove unused coded in mr.c
  IB/mlx5: Fix micro UAR allocator
  IB/mlx5: Clear out struct before create QP command
  net/mlx5_core: Use mlx5 core style warning
  IB/mlx5: Make sure doorbell record is visible before doorbell
  IB/mlx5: Implement modify CQ
  IB/mlx5: Add support for resize CQ
  net/mlx5_core: Improve debugfs readability
  mlx5_core: Fix PowerPC support
  IB/mlx5: Allow creation of QPs with zero length work queues
  IB/mlx5: Abort driver cleanup if teardown hca fails
  IB/mlx5: Remove old field for create mkey mailbox

Haggai Eran (1):
  net/mlx5_core: Fix out arg size in access_register command

 drivers/infiniband/hw/mlx5/cq.c                    | 306 +++++++++++++++++++--
 drivers/infiniband/hw/mlx5/main.c                  |  13 +-
 drivers/infiniband/hw/mlx5/mlx5_ib.h               |   4 +-
 drivers/infiniband/hw/mlx5/mr.c                    |   1 -
 drivers/infiniband/hw/mlx5/qp.c                    | 130 ++++++---
 drivers/infiniband/hw/mlx5/user.h                  |   3 +
 drivers/net/ethernet/mellanox/mlx5/core/cq.c       |  17 +-
 drivers/net/ethernet/mellanox/mlx5/core/debugfs.c  |  39 ++-
 drivers/net/ethernet/mellanox/mlx5/core/main.c     |  10 +-
 .../net/ethernet/mellanox/mlx5/core/pagealloc.c    |  13 +-
 drivers/net/ethernet/mellanox/mlx5/core/port.c     |   2 +-
 drivers/net/ethernet/mellanox/mlx5/core/qp.c       |   5 +-
 include/linux/mlx5/cq.h                            |  18 +-
 include/linux/mlx5/device.h                        |  31 ++-
 include/linux/mlx5/qp.h                            |  45 +++
 15 files changed, 542 insertions(+), 95 deletions(-)

-- 
1.8.5.2

--
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 01/14] IB/mlx5: Remove unused coded in mr.c

[Eli Cohen]

The variable start in struct mlx5_ib_mr is never used. Remove it.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/mlx5_ib.h | 1 -
 drivers/infiniband/hw/mlx5/mr.c      | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 4c134d9..5acef30 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -264,7 +264,6 @@ struct mlx5_ib_mr {
 	enum ib_wc_status	status;
 	struct mlx5_ib_dev     *dev;
 	struct mlx5_create_mkey_mbox_out out;
-	unsigned long		start;
 };
 
 struct mlx5_ib_fast_reg_page_list {
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index 039c3e4..7c95ca1 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -146,7 +146,6 @@ static int add_keys(struct mlx5_ib_dev *dev, int c, int num)
 		spin_lock_irq(&ent->lock);
 		ent->pending++;
 		spin_unlock_irq(&ent->lock);
-		mr->start = jiffies;
 		err = mlx5_core_create_mkey(&dev->mdev, &mr->mmr, in,
 					    sizeof(*in), reg_mr_callback,
 					    mr, &mr->out);
-- 
1.8.5.2

--
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 02/14] mlx5_core: Remove dead code

[Eli Cohen]

From: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Remove leftover of debug code.

Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
index 37b6ad1..c35c432 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
@@ -192,10 +192,8 @@ static int alloc_4k(struct mlx5_core_dev *dev, u64 *addr)
 	struct fw_page *fp;
 	unsigned n;
 
-	if (list_empty(&dev->priv.free_list)) {
+	if (list_empty(&dev->priv.free_list))
 		return -ENOMEM;
-		mlx5_core_warn(dev, "\n");
-	}
 
 	fp = list_entry(dev->priv.free_list.next, struct fw_page, list);
 	n = find_first_bit(&fp->bitmask, 8 * sizeof(fp->bitmask));
-- 
1.8.5.2

--
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 03/14] IB/mlx5: Fix micro UAR allocator

[Eli Cohen]

The micro UAR (uuar) allocator had a bug which resulted from the fact that in
each UAR we only have two micro UARs avaialable, those at index 0 and 1. This
patch defines iterators to aid in traversing the list of available micro UARs
when allocating a uuar.

In addition, change the logic in create_user_qp() so that if high class
allocation fails (high class means lower latency), we revert to medium class
and not to the low class.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/main.c | 13 ++++---
 drivers/infiniband/hw/mlx5/qp.c   | 77 +++++++++++++++++++++++++++++----------
 include/linux/mlx5/device.h       |  7 ++--
 3 files changed, 70 insertions(+), 27 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 3065341..9660d09 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -541,6 +541,7 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev,
 	struct mlx5_ib_ucontext *context;
 	struct mlx5_uuar_info *uuari;
 	struct mlx5_uar *uars;
+	int gross_uuars;
 	int num_uars;
 	int uuarn;
 	int err;
@@ -559,11 +560,13 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev,
 	if (req.total_num_uuars == 0)
 		return ERR_PTR(-EINVAL);
 
-	req.total_num_uuars = ALIGN(req.total_num_uuars, MLX5_BF_REGS_PER_PAGE);
+	req.total_num_uuars = ALIGN(req.total_num_uuars,
+				    MLX5_NON_FP_BF_REGS_PER_PAGE);
 	if (req.num_low_latency_uuars > req.total_num_uuars - 1)
 		return ERR_PTR(-EINVAL);
 
-	num_uars = req.total_num_uuars / MLX5_BF_REGS_PER_PAGE;
+	num_uars = req.total_num_uuars / MLX5_NON_FP_BF_REGS_PER_PAGE;
+	gross_uuars = num_uars * MLX5_BF_REGS_PER_PAGE;
 	resp.qp_tab_size      = 1 << dev->mdev.caps.log_max_qp;
 	resp.bf_reg_size      = dev->mdev.caps.bf_reg_size;
 	resp.cache_line_size  = L1_CACHE_BYTES;
@@ -585,7 +588,7 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev,
 		goto out_ctx;
 	}
 
-	uuari->bitmap = kcalloc(BITS_TO_LONGS(req.total_num_uuars),
+	uuari->bitmap = kcalloc(BITS_TO_LONGS(gross_uuars),
 				sizeof(*uuari->bitmap),
 				GFP_KERNEL);
 	if (!uuari->bitmap) {
@@ -595,13 +598,13 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev,
 	/*
 	 * clear all fast path uuars
 	 */
-	for (i = 0; i < req.total_num_uuars; i++) {
+	for (i = 0; i < gross_uuars; i++) {
 		uuarn = i & 3;
 		if (uuarn == 2 || uuarn == 3)
 			set_bit(i, uuari->bitmap);
 	}
 
-	uuari->count = kcalloc(req.total_num_uuars, sizeof(*uuari->count), GFP_KERNEL);
+	uuari->count = kcalloc(gross_uuars, sizeof(*uuari->count), GFP_KERNEL);
 	if (!uuari->count) {
 		err = -ENOMEM;
 		goto out_bitmap;
diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index 7c6b4ba..a056c24 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -340,14 +340,57 @@ static int qp_has_rq(struct ib_qp_init_attr *attr)
 	return 1;
 }
 
+static int first_med_uuar(void)
+{
+	return 1;
+}
+
+static int next_uuar(int n)
+{
+	n++;
+
+	while (((n % 4) & 2))
+		n++;
+
+	return n;
+}
+
+static int num_med_uuar(struct mlx5_uuar_info *uuari)
+{
+	int n;
+
+	n = uuari->num_uars * MLX5_NON_FP_BF_REGS_PER_PAGE -
+		uuari->num_low_latency_uuars - 1;
+
+	return n >= 0 ? n : 0;
+}
+
+static int max_uuari(struct mlx5_uuar_info *uuari)
+{
+	return uuari->num_uars * 4;
+}
+
+static int first_hi_uuar(struct mlx5_uuar_info *uuari)
+{
+	int med;
+	int i;
+	int t;
+
+	med = num_med_uuar(uuari);
+	for (t = 0, i = first_med_uuar();; i = next_uuar(i)) {
+		t++;
+		if (t == med)
+			return next_uuar(i);
+	}
+
+	return 0;
+}
+
 static int alloc_high_class_uuar(struct mlx5_uuar_info *uuari)
 {
-	int nuuars = uuari->num_uars * MLX5_BF_REGS_PER_PAGE;
-	int start_uuar;
 	int i;
 
-	start_uuar = nuuars - uuari->num_low_latency_uuars;
-	for (i = start_uuar; i < nuuars; i++) {
+	for (i = first_hi_uuar(uuari); i < max_uuari(uuari); i = next_uuar(i)) {
 		if (!test_bit(i, uuari->bitmap)) {
 			set_bit(i, uuari->bitmap);
 			uuari->count[i]++;
@@ -360,19 +403,10 @@ static int alloc_high_class_uuar(struct mlx5_uuar_info *uuari)
 
 static int alloc_med_class_uuar(struct mlx5_uuar_info *uuari)
 {
-	int nuuars = uuari->num_uars * MLX5_BF_REGS_PER_PAGE;
-	int minidx = 1;
-	int uuarn;
-	int end;
+	int minidx = first_med_uuar();
 	int i;
 
-	end = nuuars - uuari->num_low_latency_uuars;
-
-	for (i = 1; i < end; i++) {
-		uuarn = i & 3;
-		if (uuarn == 2 || uuarn == 3)
-			continue;
-
+	for (i = first_med_uuar(); i < first_hi_uuar(uuari); i = next_uuar(i)) {
 		if (uuari->count[i] < uuari->count[minidx])
 			minidx = i;
 	}
@@ -510,11 +544,16 @@ static int create_user_qp(struct mlx5_ib_dev *dev, struct ib_pd *pd,
 	uuarn = alloc_uuar(&context->uuari, MLX5_IB_LATENCY_CLASS_HIGH);
 	if (uuarn < 0) {
 		mlx5_ib_dbg(dev, "failed to allocate low latency UUAR\n");
-		mlx5_ib_dbg(dev, "reverting to high latency\n");
-		uuarn = alloc_uuar(&context->uuari, MLX5_IB_LATENCY_CLASS_LOW);
+		mlx5_ib_dbg(dev, "reverting to medium latency\n");
+		uuarn = alloc_uuar(&context->uuari, MLX5_IB_LATENCY_CLASS_MEDIUM);
 		if (uuarn < 0) {
-			mlx5_ib_dbg(dev, "uuar allocation failed\n");
-			return uuarn;
+			mlx5_ib_dbg(dev, "failed to allocate medium latency UUAR\n");
+			mlx5_ib_dbg(dev, "reverting to high latency\n");
+			uuarn = alloc_uuar(&context->uuari, MLX5_IB_LATENCY_CLASS_LOW);
+			if (uuarn < 0) {
+				mlx5_ib_warn(dev, "uuar allocation failed\n");
+				return uuarn;
+			}
 		}
 	}
 
diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
index da78875..2c1c62e 100644
--- a/include/linux/mlx5/device.h
+++ b/include/linux/mlx5/device.h
@@ -104,9 +104,10 @@ enum {
 };
 
 enum {
-	MLX5_BF_REGS_PER_PAGE	= 4,
-	MLX5_MAX_UAR_PAGES	= 1 << 8,
-	MLX5_MAX_UUARS		= MLX5_MAX_UAR_PAGES * MLX5_BF_REGS_PER_PAGE,
+	MLX5_BF_REGS_PER_PAGE		= 4,
+	MLX5_MAX_UAR_PAGES		= 1 << 8,
+	MLX5_NON_FP_BF_REGS_PER_PAGE	= 2,
+	MLX5_MAX_UUARS	= MLX5_MAX_UAR_PAGES * MLX5_NON_FP_BF_REGS_PER_PAGE,
 };
 
 enum {
-- 
1.8.5.2

--
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 04/14] net/mlx5_core: Fix out arg size in access_register command

[Eli Cohen]

From: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The output size should be the sum of the core access reg output struct plus the
size of the specific register data provided by the caller.

Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/port.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/port.c b/drivers/net/ethernet/mellanox/mlx5/core/port.c
index f6afe7b..8c9ac87 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/port.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/port.c
@@ -57,7 +57,7 @@ int mlx5_core_access_reg(struct mlx5_core_dev *dev, void *data_in,
 	in->arg = cpu_to_be32(arg);
 	in->register_id = cpu_to_be16(reg_num);
 	err = mlx5_cmd_exec(dev, in, sizeof(*in) + size_in, out,
-			    sizeof(out) + size_out);
+			    sizeof(*out) + size_out);
 	if (err)
 		goto ex2;
 
-- 
1.8.5.2

--
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 05/14] IB/mlx5: Clear out struct before create QP command

[Eli Cohen]

Output structs are expected by firmware to be cleared when a command is called.
Clear the "out" struct instead of "dout" which is used only later.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/qp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/qp.c b/drivers/net/ethernet/mellanox/mlx5/core/qp.c
index 54faf8b..e794184 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/qp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/qp.c
@@ -74,7 +74,7 @@ int mlx5_core_create_qp(struct mlx5_core_dev *dev,
 	struct mlx5_destroy_qp_mbox_out dout;
 	int err;
 
-	memset(&dout, 0, sizeof(dout));
+	memset(&out, 0, sizeof(out));
 	in->hdr.opcode = cpu_to_be16(MLX5_CMD_OP_CREATE_QP);
 
 	err = mlx5_cmd_exec(dev, in, inlen, &out, sizeof(out));
-- 
1.8.5.2

--
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 06/14] net/mlx5_core: Use mlx5 core style warning

[Eli Cohen]

Use mlx5_core_warn(), which is the standard warning emitter function, instead
of pr_warn.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/qp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/qp.c b/drivers/net/ethernet/mellanox/mlx5/core/qp.c
index e794184..5105762 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/qp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/qp.c
@@ -84,7 +84,8 @@ int mlx5_core_create_qp(struct mlx5_core_dev *dev,
 	}
 
 	if (out.hdr.status) {
-		pr_warn("current num of QPs 0x%x\n", atomic_read(&dev->num_qps));
+		mlx5_core_warn(dev, "current num of QPs 0x%x\n",
+			       atomic_read(&dev->num_qps));
 		return mlx5_cmd_status_to_err(&out.hdr);
 	}
 
-- 
1.8.5.2

--
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 07/14] IB/mlx5: Make sure doorbell record is visible before doorbell

[Eli Cohen]

Put a wmb() to make sure the doorbell record is visible to the HCA before we
hit doorbell.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/qp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index a056c24..87b7fb1 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -2251,6 +2251,10 @@ out:
 
 		qp->db.db[MLX5_SND_DBR] = cpu_to_be32(qp->sq.cur_post);
 
+		/* Make sure doorbell record is visible to the HCA before
+		 * we hit doorbell */
+		wmb();
+
 		if (bf->need_lock)
 			spin_lock(&bf->lock);
 
-- 
1.8.5.2

--
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 07/14] IB/mlx5: Make sure doorbell record is visible before doorbell

[Yann Droneaud]

Le mardi 14 janvier 2014 à 17:45 +0200, Eli Cohen a écrit :
> Put a wmb() to make sure the doorbell record is visible to the HCA before we
> hit doorbell.
> 
> Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/infiniband/hw/mlx5/qp.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
> index a056c24..87b7fb1 100644
> --- a/drivers/infiniband/hw/mlx5/qp.c
> +++ b/drivers/infiniband/hw/mlx5/qp.c
> @@ -2251,6 +2251,10 @@ out:
>  
>  		qp->db.db[MLX5_SND_DBR] = cpu_to_be32(qp->sq.cur_post);
>  
> +		/* Make sure doorbell record is visible to the HCA before
> +		 * we hit doorbell */
> +		wmb();
> +

In linux-next, as of next-20140114, I'm seeing the write memory barrier
before "qp->db.db[MLX5_SND_DBR] = cpu_to_be32(qp->sq.cur_post);"
It was introduced by commit e126ba97dba9e:

out:
        if (likely(nreq)) {
                qp->sq.head += nreq;

                /* Make sure that descriptors are written before
                 * updating doorbell record and ringing the doorbell
                 */
                wmb();

                qp->db.db[MLX5_SND_DBR] = cpu_to_be32(qp->sq.cur_post);

                if (bf->need_lock)
                        spin_lock(&bf->lock);

So this add a second wmb().
Is it really necessary ?

>  		if (bf->need_lock)
>  			spin_lock(&bf->lock);
>  

Regards.

-- 
Yann Droneaud
OPTEYA


--
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 07/14] IB/mlx5: Make sure doorbell record is visible before doorbell

[Eli Cohen]

> 
> In linux-next, as of next-20140114, I'm seeing the write memory barrier
> before "qp->db.db[MLX5_SND_DBR] = cpu_to_be32(qp->sq.cur_post);"
> It was introduced by commit e126ba97dba9e:
> 
> out:
>         if (likely(nreq)) {
>                 qp->sq.head += nreq;
> 
>                 /* Make sure that descriptors are written before
>                  * updating doorbell record and ringing the doorbell
>                  */
>                 wmb();
> 
>                 qp->db.db[MLX5_SND_DBR] = cpu_to_be32(qp->sq.cur_post);
> 
>                 if (bf->need_lock)
>                         spin_lock(&bf->lock);
> 
> So this add a second wmb().
> Is it really necessary ?
> 

Yes both are required. The first one makes sure that the descriptors
are visible before update of the doorbell record, and the newly added
one is required to ensure the doorbell record is visible before the
hardware is notified.
--
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 07/14] IB/mlx5: Make sure doorbell record is visible before doorbell

[Yann Droneaud]

Le mercredi 15 janvier 2014 à 08:47 +0200, Eli Cohen a écrit :
> > 
> > In linux-next, as of next-20140114, I'm seeing the write memory barrier
> > before "qp->db.db[MLX5_SND_DBR] = cpu_to_be32(qp->sq.cur_post);"
> > It was introduced by commit e126ba97dba9e:
> > 
> > out:
> >         if (likely(nreq)) {
> >                 qp->sq.head += nreq;
> > 
> >                 /* Make sure that descriptors are written before
> >                  * updating doorbell record and ringing the doorbell
> >                  */
> >                 wmb();
> > 
> >                 qp->db.db[MLX5_SND_DBR] = cpu_to_be32(qp->sq.cur_post);
> > 
> >                 if (bf->need_lock)
> >                         spin_lock(&bf->lock);
> > 
> > So this add a second wmb().
> > Is it really necessary ?
> > 
> 
> Yes both are required. The first one makes sure that the descriptors
> are visible before update of the doorbell record, and the newly added
> one is required to ensure the doorbell record is visible before the
> hardware is notified.

OK.

They are both needed to ensure ordered write (correct serialization), so
that the descriptors are valids after reading the record.

Regards.

-- 
Yann Droneaud
OPTEYA


--
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 08/14] IB/mlx5: Implement modify CQ

[Eli Cohen]

Modify CQ is used by ULPs like IPoIB to change moderation parameters. This
patch adds such support in mlx5.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/cq.c              | 26 +++++++++++++++++++++++++-
 drivers/net/ethernet/mellanox/mlx5/core/cq.c | 17 +++++++++++++++--
 include/linux/mlx5/cq.h                      |  8 ++++----
 include/linux/mlx5/device.h                  | 15 +++++++++++++++
 4 files changed, 59 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
index b726274..b4c122e 100644
--- a/drivers/infiniband/hw/mlx5/cq.c
+++ b/drivers/infiniband/hw/mlx5/cq.c
@@ -818,7 +818,31 @@ void mlx5_ib_cq_clean(struct mlx5_ib_cq *cq, u32 qpn, struct mlx5_ib_srq *srq)
 
 int mlx5_ib_modify_cq(struct ib_cq *cq, u16 cq_count, u16 cq_period)
 {
-	return -ENOSYS;
+	struct mlx5_modify_cq_mbox_in *in;
+	struct mlx5_ib_dev *dev = to_mdev(cq->device);
+	struct mlx5_ib_cq *mcq = to_mcq(cq);
+	int err;
+	u32 fsel;
+
+	if (!(dev->mdev.caps.flags & MLX5_DEV_CAP_FLAG_CQ_MODER))
+		return -ENOSYS;
+
+	in = kzalloc(sizeof(*in), GFP_KERNEL);
+	if (!in)
+		return -ENOMEM;
+
+	in->cqn = cpu_to_be32(mcq->mcq.cqn);
+	fsel = (MLX5_CQ_MODIFY_PERIOD | MLX5_CQ_MODIFY_COUNT);
+	in->ctx.cq_period = cpu_to_be16(cq_period);
+	in->ctx.cq_max_count = cpu_to_be16(cq_count);
+	in->field_select = cpu_to_be32(fsel);
+	err = mlx5_core_modify_cq(&dev->mdev, &mcq->mcq, in);
+	kfree(in);
+
+	if (err)
+		mlx5_ib_warn(dev, "modify cq 0x%x failed\n", mcq->mcq.cqn);
+
+	return err;
 }
 
 int mlx5_ib_resize_cq(struct ib_cq *ibcq, int entries, struct ib_udata *udata)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cq.c b/drivers/net/ethernet/mellanox/mlx5/core/cq.c
index c2d660b..e6fedcf 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/cq.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/cq.c
@@ -201,10 +201,23 @@ EXPORT_SYMBOL(mlx5_core_query_cq);
 
 
 int mlx5_core_modify_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq,
-			int type, struct mlx5_cq_modify_params *params)
+			struct mlx5_modify_cq_mbox_in *in)
 {
-	return -ENOSYS;
+	struct mlx5_modify_cq_mbox_out out;
+	int err;
+
+	memset(&out, 0, sizeof(out));
+	in->hdr.opcode = cpu_to_be16(MLX5_CMD_OP_MODIFY_CQ);
+	err = mlx5_cmd_exec(dev, in, sizeof(*in), &out, sizeof(out));
+	if (err)
+		return err;
+
+	if (out.hdr.status)
+		return mlx5_cmd_status_to_err(&out.hdr);
+
+	return 0;
 }
+EXPORT_SYMBOL(mlx5_core_modify_cq);
 
 int mlx5_init_cq_table(struct mlx5_core_dev *dev)
 {
diff --git a/include/linux/mlx5/cq.h b/include/linux/mlx5/cq.h
index 3db67f7..c3cf5a4 100644
--- a/include/linux/mlx5/cq.h
+++ b/include/linux/mlx5/cq.h
@@ -85,9 +85,9 @@ enum {
 };
 
 enum {
-	MLX5_CQ_MODIFY_RESEIZE = 0,
-	MLX5_CQ_MODIFY_MODER = 1,
-	MLX5_CQ_MODIFY_MAPPING = 2,
+	MLX5_CQ_MODIFY_PERIOD	= 1 << 0,
+	MLX5_CQ_MODIFY_COUNT	= 1 << 1,
+	MLX5_CQ_MODIFY_OVERRUN	= 1 << 2,
 };
 
 struct mlx5_cq_modify_params {
@@ -158,7 +158,7 @@ int mlx5_core_destroy_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq);
 int mlx5_core_query_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq,
 		       struct mlx5_query_cq_mbox_out *out);
 int mlx5_core_modify_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq,
-			int type, struct mlx5_cq_modify_params *params);
+			struct mlx5_modify_cq_mbox_in *in);
 int mlx5_debug_cq_add(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq);
 void mlx5_debug_cq_remove(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq);
 
diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
index 2c1c62e..dbb03ca 100644
--- a/include/linux/mlx5/device.h
+++ b/include/linux/mlx5/device.h
@@ -177,6 +177,7 @@ enum {
 	MLX5_DEV_CAP_FLAG_APM		= 1LL << 17,
 	MLX5_DEV_CAP_FLAG_ATOMIC	= 1LL << 18,
 	MLX5_DEV_CAP_FLAG_ON_DMND_PG	= 1LL << 24,
+	MLX5_DEV_CAP_FLAG_CQ_MODER	= 1LL << 29,
 	MLX5_DEV_CAP_FLAG_RESIZE_SRQ	= 1LL << 32,
 	MLX5_DEV_CAP_FLAG_REMOTE_FENCE	= 1LL << 38,
 	MLX5_DEV_CAP_FLAG_TLP_HINTS	= 1LL << 39,
@@ -698,6 +699,19 @@ struct mlx5_query_cq_mbox_out {
 	__be64			pas[0];
 };
 
+struct mlx5_modify_cq_mbox_in {
+	struct mlx5_inbox_hdr	hdr;
+	__be32			cqn;
+	__be32			field_select;
+	struct mlx5_cq_context	ctx;
+	u8			rsvd[192];
+	__be64			pas[0];
+};
+
+struct mlx5_modify_cq_mbox_out {
+	struct mlx5_outbox_hdr	hdr;
+};
+
 struct mlx5_enable_hca_mbox_in {
 	struct mlx5_inbox_hdr	hdr;
 	u8			rsvd[8];
@@ -872,6 +886,7 @@ struct mlx5_modify_mkey_mbox_in {
 
 struct mlx5_modify_mkey_mbox_out {
 	struct mlx5_outbox_hdr	hdr;
+	u8			rsvd[8];
 };
 
 struct mlx5_dump_mkey_mbox_in {
-- 
1.8.5.2

--
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 09/14] IB/mlx5: Add support for resize CQ

[Eli Cohen]

Implement resize CQ which is a mandatory verb in mlx5.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Conflicts:
	include/linux/mlx5/device.h
---
 drivers/infiniband/hw/mlx5/cq.c              | 282 +++++++++++++++++++++++++--
 drivers/infiniband/hw/mlx5/mlx5_ib.h         |   3 +-
 drivers/infiniband/hw/mlx5/user.h            |   3 +
 drivers/net/ethernet/mellanox/mlx5/core/cq.c |   4 +-
 include/linux/mlx5/cq.h                      |  12 +-
 include/linux/mlx5/device.h                  |   2 +
 6 files changed, 284 insertions(+), 22 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
index b4c122e..50b03a8 100644
--- a/drivers/infiniband/hw/mlx5/cq.c
+++ b/drivers/infiniband/hw/mlx5/cq.c
@@ -73,14 +73,24 @@ static void *get_cqe(struct mlx5_ib_cq *cq, int n)
 	return get_cqe_from_buf(&cq->buf, n, cq->mcq.cqe_sz);
 }
 
+static u8 sw_ownership_bit(int n, int nent)
+{
+	return (n & nent) ? 1 : 0;
+}
+
 static void *get_sw_cqe(struct mlx5_ib_cq *cq, int n)
 {
 	void *cqe = get_cqe(cq, n & cq->ibcq.cqe);
 	struct mlx5_cqe64 *cqe64;
 
 	cqe64 = (cq->mcq.cqe_sz == 64) ? cqe : cqe + 64;
-	return ((cqe64->op_own & MLX5_CQE_OWNER_MASK) ^
-		!!(n & (cq->ibcq.cqe + 1))) ? NULL : cqe;
+
+	if (likely((cqe64->op_own) >> 4 != MLX5_CQE_INVALID) &&
+	    !((cqe64->op_own & MLX5_CQE_OWNER_MASK) ^ !!(n & (cq->ibcq.cqe + 1)))) {
+		return cqe;
+	} else {
+		return NULL;
+	}
 }
 
 static void *next_cqe_sw(struct mlx5_ib_cq *cq)
@@ -351,6 +361,11 @@ static void handle_atomics(struct mlx5_ib_qp *qp, struct mlx5_cqe64 *cqe64,
 	qp->sq.last_poll = tail;
 }
 
+static void free_cq_buf(struct mlx5_ib_dev *dev, struct mlx5_ib_cq_buf *buf)
+{
+	mlx5_buf_free(&dev->mdev, &buf->buf);
+}
+
 static int mlx5_poll_one(struct mlx5_ib_cq *cq,
 			 struct mlx5_ib_qp **cur_qp,
 			 struct ib_wc *wc)
@@ -366,6 +381,7 @@ static int mlx5_poll_one(struct mlx5_ib_cq *cq,
 	void *cqe;
 	int idx;
 
+repoll:
 	cqe = next_cqe_sw(cq);
 	if (!cqe)
 		return -EAGAIN;
@@ -379,7 +395,18 @@ static int mlx5_poll_one(struct mlx5_ib_cq *cq,
 	 */
 	rmb();
 
-	/* TBD: resize CQ */
+	opcode = cqe64->op_own >> 4;
+	if (unlikely(opcode == MLX5_CQE_RESIZE_CQ)) {
+		if (likely(cq->resize_buf)) {
+			free_cq_buf(dev, &cq->buf);
+			cq->buf = *cq->resize_buf;
+			kfree(cq->resize_buf);
+			cq->resize_buf = NULL;
+			goto repoll;
+		} else {
+			mlx5_ib_warn(dev, "unexpected resize cqe\n");
+		}
+	}
 
 	qpn = ntohl(cqe64->sop_drop_qpn) & 0xffffff;
 	if (!*cur_qp || (qpn != (*cur_qp)->ibqp.qp_num)) {
@@ -398,7 +425,6 @@ static int mlx5_poll_one(struct mlx5_ib_cq *cq,
 	}
 
 	wc->qp  = &(*cur_qp)->ibqp;
-	opcode = cqe64->op_own >> 4;
 	switch (opcode) {
 	case MLX5_CQE_REQ:
 		wq = &(*cur_qp)->sq;
@@ -503,15 +529,11 @@ static int alloc_cq_buf(struct mlx5_ib_dev *dev, struct mlx5_ib_cq_buf *buf,
 		return err;
 
 	buf->cqe_size = cqe_size;
+	buf->nent = nent;
 
 	return 0;
 }
 
-static void free_cq_buf(struct mlx5_ib_dev *dev, struct mlx5_ib_cq_buf *buf)
-{
-	mlx5_buf_free(&dev->mdev, &buf->buf);
-}
-
 static int create_cq_user(struct mlx5_ib_dev *dev, struct ib_udata *udata,
 			  struct ib_ucontext *context, struct mlx5_ib_cq *cq,
 			  int entries, struct mlx5_create_cq_mbox_in **cqb,
@@ -576,16 +598,16 @@ static void destroy_cq_user(struct mlx5_ib_cq *cq, struct ib_ucontext *context)
 	ib_umem_release(cq->buf.umem);
 }
 
-static void init_cq_buf(struct mlx5_ib_cq *cq, int nent)
+static void init_cq_buf(struct mlx5_ib_cq *cq, struct mlx5_ib_cq_buf *buf)
 {
 	int i;
 	void *cqe;
 	struct mlx5_cqe64 *cqe64;
 
-	for (i = 0; i < nent; i++) {
-		cqe = get_cqe(cq, i);
-		cqe64 = (cq->buf.cqe_size == 64) ? cqe : cqe + 64;
-		cqe64->op_own = 0xf1;
+	for (i = 0; i < buf->nent; i++) {
+		cqe = get_cqe_from_buf(buf, i, buf->cqe_size);
+		cqe64 = buf->cqe_size == 64 ? cqe : cqe + 64;
+		cqe64->op_own = MLX5_CQE_INVALID << 4;
 	}
 }
 
@@ -610,7 +632,7 @@ static int create_cq_kernel(struct mlx5_ib_dev *dev, struct mlx5_ib_cq *cq,
 	if (err)
 		goto err_db;
 
-	init_cq_buf(cq, entries);
+	init_cq_buf(cq, &cq->buf);
 
 	*inlen = sizeof(**cqb) + sizeof(*(*cqb)->pas) * cq->buf.buf.npages;
 	*cqb = mlx5_vzalloc(*inlen);
@@ -836,7 +858,7 @@ int mlx5_ib_modify_cq(struct ib_cq *cq, u16 cq_count, u16 cq_period)
 	in->ctx.cq_period = cpu_to_be16(cq_period);
 	in->ctx.cq_max_count = cpu_to_be16(cq_count);
 	in->field_select = cpu_to_be32(fsel);
-	err = mlx5_core_modify_cq(&dev->mdev, &mcq->mcq, in);
+	err = mlx5_core_modify_cq(&dev->mdev, &mcq->mcq, in, sizeof(*in));
 	kfree(in);
 
 	if (err)
@@ -845,9 +867,235 @@ int mlx5_ib_modify_cq(struct ib_cq *cq, u16 cq_count, u16 cq_period)
 	return err;
 }
 
+static int resize_user(struct mlx5_ib_dev *dev, struct mlx5_ib_cq *cq,
+		       int entries, struct ib_udata *udata, int *npas,
+		       int *page_shift, int *cqe_size)
+{
+	struct mlx5_ib_resize_cq ucmd;
+	struct ib_umem *umem;
+	int err;
+	int npages;
+	struct ib_ucontext *context = cq->buf.umem->context;
+
+	if (ib_copy_from_udata(&ucmd, udata, sizeof(ucmd)))
+		return -EFAULT;
+
+	umem = ib_umem_get(context, ucmd.buf_addr, entries * ucmd.cqe_size,
+			   IB_ACCESS_LOCAL_WRITE, 1);
+	if (IS_ERR(umem)) {
+		err = PTR_ERR(umem);
+		return err;
+	}
+
+	mlx5_ib_cont_pages(umem, ucmd.buf_addr, &npages, page_shift,
+			   npas, NULL);
+
+	cq->resize_umem = umem;
+	*cqe_size = ucmd.cqe_size;
+
+	return 0;
+}
+
+static void un_resize_user(struct mlx5_ib_cq *cq)
+{
+	ib_umem_release(cq->resize_umem);
+}
+
+static int resize_kernel(struct mlx5_ib_dev *dev, struct mlx5_ib_cq *cq,
+			 int entries, int cqe_size)
+{
+	int err;
+
+	cq->resize_buf = kzalloc(sizeof(*cq->resize_buf), GFP_KERNEL);
+	if (!cq->resize_buf)
+		return -ENOMEM;
+
+	err = alloc_cq_buf(dev, cq->resize_buf, entries, cqe_size);
+	if (err)
+		goto ex;
+
+	init_cq_buf(cq, cq->resize_buf);
+
+	return 0;
+
+ex:
+	kfree(cq->resize_buf);
+	return err;
+}
+
+static void un_resize_kernel(struct mlx5_ib_dev *dev, struct mlx5_ib_cq *cq)
+{
+	free_cq_buf(dev, cq->resize_buf);
+	cq->resize_buf = NULL;
+}
+
+static int copy_resize_cqes(struct mlx5_ib_cq *cq)
+{
+	struct mlx5_ib_dev *dev = to_mdev(cq->ibcq.device);
+	struct mlx5_cqe64 *scqe64;
+	struct mlx5_cqe64 *dcqe64;
+	void *start_cqe;
+	void *scqe;
+	void *dcqe;
+	int ssize;
+	int dsize;
+	int i;
+	u8 sw_own;
+
+	ssize = cq->buf.cqe_size;
+	dsize = cq->resize_buf->cqe_size;
+	if (ssize != dsize) {
+		mlx5_ib_warn(dev, "resize from different cqe size is not supported\n");
+		return -EINVAL;
+	}
+
+	i = cq->mcq.cons_index;
+	scqe = get_sw_cqe(cq, i);
+	scqe64 = ssize == 64 ? scqe : scqe + 64;
+	start_cqe = scqe;
+	if (!scqe) {
+		mlx5_ib_warn(dev, "expected cqe in sw ownership\n");
+		return -EINVAL;
+	}
+
+	while ((scqe64->op_own >> 4) != MLX5_CQE_RESIZE_CQ) {
+		dcqe = get_cqe_from_buf(cq->resize_buf,
+					(i + 1) & (cq->resize_buf->nent),
+					dsize);
+		dcqe64 = dsize == 64 ? dcqe : dcqe + 64;
+		sw_own = sw_ownership_bit(i + 1, cq->resize_buf->nent);
+		memcpy(dcqe, scqe, dsize);
+		dcqe64->op_own = (dcqe64->op_own & ~MLX5_CQE_OWNER_MASK) | sw_own;
+
+		++i;
+		scqe = get_sw_cqe(cq, i);
+		scqe64 = ssize == 64 ? scqe : scqe + 64;
+		if (!scqe) {
+			mlx5_ib_warn(dev, "expected cqe in sw ownership\n");
+			return -EINVAL;
+		}
+
+		if (scqe == start_cqe) {
+			pr_warn("resize CQ failed to get resize CQE, CQN 0x%x\n",
+				cq->mcq.cqn);
+			return -ENOMEM;
+		}
+	}
+	++cq->mcq.cons_index;
+	return 0;
+}
+
 int mlx5_ib_resize_cq(struct ib_cq *ibcq, int entries, struct ib_udata *udata)
 {
-	return -ENOSYS;
+	struct mlx5_ib_dev *dev = to_mdev(ibcq->device);
+	struct mlx5_ib_cq *cq = to_mcq(ibcq);
+	struct mlx5_modify_cq_mbox_in *in;
+	int err;
+	int npas;
+	int page_shift;
+	int inlen;
+	int uninitialized_var(cqe_size);
+	unsigned long flags;
+
+	if (!(dev->mdev.caps.flags & MLX5_DEV_CAP_FLAG_RESIZE_CQ)) {
+		pr_info("Firmware does not support resize CQ\n");
+		return -ENOSYS;
+	}
+
+	if (entries < 1)
+		return -EINVAL;
+
+	entries = roundup_pow_of_two(entries + 1);
+	if (entries > dev->mdev.caps.max_cqes + 1)
+		return -EINVAL;
+
+	if (entries == ibcq->cqe + 1)
+		return 0;
+
+	mutex_lock(&cq->resize_mutex);
+	if (udata) {
+		err = resize_user(dev, cq, entries, udata, &npas, &page_shift,
+				  &cqe_size);
+	} else {
+		cqe_size = 64;
+		err = resize_kernel(dev, cq, entries, cqe_size);
+		if (!err) {
+			npas = cq->resize_buf->buf.npages;
+			page_shift = cq->resize_buf->buf.page_shift;
+		}
+	}
+
+	if (err)
+		goto ex;
+
+	inlen = sizeof(*in) + npas * sizeof(in->pas[0]);
+	in = mlx5_vzalloc(inlen);
+	if (!in) {
+		err = -ENOMEM;
+		goto ex_resize;
+	}
+
+	if (udata)
+		mlx5_ib_populate_pas(dev, cq->resize_umem, page_shift,
+				     in->pas, 0);
+	else
+		mlx5_fill_page_array(&cq->resize_buf->buf, in->pas);
+
+	in->field_select = cpu_to_be32(MLX5_MODIFY_CQ_MASK_LOG_SIZE  |
+				       MLX5_MODIFY_CQ_MASK_PG_OFFSET |
+				       MLX5_MODIFY_CQ_MASK_PG_SIZE);
+	in->ctx.log_pg_sz = page_shift - MLX5_ADAPTER_PAGE_SHIFT;
+	in->ctx.cqe_sz_flags = cqe_sz_to_mlx_sz(cqe_size) << 5;
+	in->ctx.page_offset = 0;
+	in->ctx.log_sz_usr_page = cpu_to_be32(ilog2(entries) << 24);
+	in->hdr.opmod = cpu_to_be16(MLX5_CQ_OPMOD_RESIZE);
+	in->cqn = cpu_to_be32(cq->mcq.cqn);
+
+	err = mlx5_core_modify_cq(&dev->mdev, &cq->mcq, in, inlen);
+	if (err)
+		goto ex_alloc;
+
+	if (udata) {
+		cq->ibcq.cqe = entries - 1;
+		ib_umem_release(cq->buf.umem);
+		cq->buf.umem = cq->resize_umem;
+		cq->resize_umem = NULL;
+	} else {
+		struct mlx5_ib_cq_buf tbuf;
+		int resized = 0;
+
+		spin_lock_irqsave(&cq->lock, flags);
+		if (cq->resize_buf) {
+			err = copy_resize_cqes(cq);
+			if (!err) {
+				tbuf = cq->buf;
+				cq->buf = *cq->resize_buf;
+				kfree(cq->resize_buf);
+				cq->resize_buf = NULL;
+				resized = 1;
+			}
+		}
+		cq->ibcq.cqe = entries - 1;
+		spin_unlock_irqrestore(&cq->lock, flags);
+		if (resized)
+			free_cq_buf(dev, &tbuf);
+	}
+	mutex_unlock(&cq->resize_mutex);
+
+	mlx5_vfree(in);
+	return 0;
+
+ex_alloc:
+	mlx5_vfree(in);
+
+ex_resize:
+	if (udata)
+		un_resize_user(cq);
+	else
+		un_resize_kernel(dev, cq);
+ex:
+	mutex_unlock(&cq->resize_mutex);
+	return err;
 }
 
 int mlx5_ib_get_cqe_size(struct mlx5_ib_dev *dev, struct ib_cq *ibcq)
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 5acef30..389e319 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -195,6 +195,7 @@ struct mlx5_ib_cq_buf {
 	struct mlx5_buf		buf;
 	struct ib_umem		*umem;
 	int			cqe_size;
+	int			nent;
 };
 
 enum mlx5_ib_qp_flags {
@@ -220,7 +221,7 @@ struct mlx5_ib_cq {
 	/* protect resize cq
 	 */
 	struct mutex		resize_mutex;
-	struct mlx5_ib_cq_resize *resize_buf;
+	struct mlx5_ib_cq_buf  *resize_buf;
 	struct ib_umem	       *resize_umem;
 	int			cqe_size;
 };
diff --git a/drivers/infiniband/hw/mlx5/user.h b/drivers/infiniband/hw/mlx5/user.h
index a886de3..32a2a5d 100644
--- a/drivers/infiniband/hw/mlx5/user.h
+++ b/drivers/infiniband/hw/mlx5/user.h
@@ -93,6 +93,9 @@ struct mlx5_ib_create_cq_resp {
 
 struct mlx5_ib_resize_cq {
 	__u64	buf_addr;
+	__u16	cqe_size;
+	__u16	reserved0;
+	__u32	reserved1;
 };
 
 struct mlx5_ib_create_srq {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cq.c b/drivers/net/ethernet/mellanox/mlx5/core/cq.c
index e6fedcf..43c5f48 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/cq.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/cq.c
@@ -201,14 +201,14 @@ EXPORT_SYMBOL(mlx5_core_query_cq);
 
 
 int mlx5_core_modify_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq,
-			struct mlx5_modify_cq_mbox_in *in)
+			struct mlx5_modify_cq_mbox_in *in, int in_sz)
 {
 	struct mlx5_modify_cq_mbox_out out;
 	int err;
 
 	memset(&out, 0, sizeof(out));
 	in->hdr.opcode = cpu_to_be16(MLX5_CMD_OP_MODIFY_CQ);
-	err = mlx5_cmd_exec(dev, in, sizeof(*in), &out, sizeof(out));
+	err = mlx5_cmd_exec(dev, in, in_sz, &out, sizeof(out));
 	if (err)
 		return err;
 
diff --git a/include/linux/mlx5/cq.h b/include/linux/mlx5/cq.h
index c3cf5a4..2202c7f 100644
--- a/include/linux/mlx5/cq.h
+++ b/include/linux/mlx5/cq.h
@@ -79,9 +79,10 @@ enum {
 	MLX5_CQE_RESP_SEND	= 2,
 	MLX5_CQE_RESP_SEND_IMM	= 3,
 	MLX5_CQE_RESP_SEND_INV	= 4,
-	MLX5_CQE_RESIZE_CQ	= 0xff, /* TBD */
+	MLX5_CQE_RESIZE_CQ	= 5,
 	MLX5_CQE_REQ_ERR	= 13,
 	MLX5_CQE_RESP_ERR	= 14,
+	MLX5_CQE_INVALID	= 15,
 };
 
 enum {
@@ -90,6 +91,13 @@ enum {
 	MLX5_CQ_MODIFY_OVERRUN	= 1 << 2,
 };
 
+enum {
+	MLX5_CQ_OPMOD_RESIZE		= 1,
+	MLX5_MODIFY_CQ_MASK_LOG_SIZE	= 1 << 0,
+	MLX5_MODIFY_CQ_MASK_PG_OFFSET	= 1 << 1,
+	MLX5_MODIFY_CQ_MASK_PG_SIZE	= 1 << 2,
+};
+
 struct mlx5_cq_modify_params {
 	int	type;
 	union {
@@ -158,7 +166,7 @@ int mlx5_core_destroy_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq);
 int mlx5_core_query_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq,
 		       struct mlx5_query_cq_mbox_out *out);
 int mlx5_core_modify_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq,
-			struct mlx5_modify_cq_mbox_in *in);
+			struct mlx5_modify_cq_mbox_in *in, int in_sz);
 int mlx5_debug_cq_add(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq);
 void mlx5_debug_cq_remove(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq);
 
diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
index dbb03ca..87e2371 100644
--- a/include/linux/mlx5/device.h
+++ b/include/linux/mlx5/device.h
@@ -178,6 +178,7 @@ enum {
 	MLX5_DEV_CAP_FLAG_ATOMIC	= 1LL << 18,
 	MLX5_DEV_CAP_FLAG_ON_DMND_PG	= 1LL << 24,
 	MLX5_DEV_CAP_FLAG_CQ_MODER	= 1LL << 29,
+	MLX5_DEV_CAP_FLAG_RESIZE_CQ	= 1LL << 30,
 	MLX5_DEV_CAP_FLAG_RESIZE_SRQ	= 1LL << 32,
 	MLX5_DEV_CAP_FLAG_REMOTE_FENCE	= 1LL << 38,
 	MLX5_DEV_CAP_FLAG_TLP_HINTS	= 1LL << 39,
@@ -710,6 +711,7 @@ struct mlx5_modify_cq_mbox_in {
 
 struct mlx5_modify_cq_mbox_out {
 	struct mlx5_outbox_hdr	hdr;
+	u8			rsvd[8];
 };
 
 struct mlx5_enable_hca_mbox_in {
-- 
1.8.5.2

--
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 09/14] IB/mlx5: Add support for resize CQ

[Yann Droneaud]

Hi,

Le mardi 14 janvier 2014 à 17:45 +0200, Eli Cohen a écrit :
> Implement resize CQ which is a mandatory verb in mlx5.
> 
> Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Conflicts:
> 	include/linux/mlx5/device.h
> ---
>  drivers/infiniband/hw/mlx5/cq.c              | 282 +++++++++++++++++++++++++--
>  drivers/infiniband/hw/mlx5/mlx5_ib.h         |   3 +-
>  drivers/infiniband/hw/mlx5/user.h            |   3 +
>  drivers/net/ethernet/mellanox/mlx5/core/cq.c |   4 +-
>  include/linux/mlx5/cq.h                      |  12 +-
>  include/linux/mlx5/device.h                  |   2 +
>  6 files changed, 284 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
> index b4c122e..50b03a8 100644
> --- a/drivers/infiniband/hw/mlx5/cq.c
> +++ b/drivers/infiniband/hw/mlx5/cq.c
> @@ -73,14 +73,24 @@ static void *get_cqe(struct mlx5_ib_cq *cq, int n)
>  	return get_cqe_from_buf(&cq->buf, n, cq->mcq.cqe_sz);
>  }
>  
> +static u8 sw_ownership_bit(int n, int nent)
> +{
> +	return (n & nent) ? 1 : 0;
> +}
> +
>  static void *get_sw_cqe(struct mlx5_ib_cq *cq, int n)
>  {
>  	void *cqe = get_cqe(cq, n & cq->ibcq.cqe);
>  	struct mlx5_cqe64 *cqe64;
>  
>  	cqe64 = (cq->mcq.cqe_sz == 64) ? cqe : cqe + 64;
> -	return ((cqe64->op_own & MLX5_CQE_OWNER_MASK) ^
> -		!!(n & (cq->ibcq.cqe + 1))) ? NULL : cqe;
> +
> +	if (likely((cqe64->op_own) >> 4 != MLX5_CQE_INVALID) &&
> +	    !((cqe64->op_own & MLX5_CQE_OWNER_MASK) ^ !!(n & (cq->ibcq.cqe + 1)))) {
> +		return cqe;
> +	} else {
> +		return NULL;
> +	}
>  }
>  
>  static void *next_cqe_sw(struct mlx5_ib_cq *cq)
> @@ -351,6 +361,11 @@ static void handle_atomics(struct mlx5_ib_qp *qp, struct mlx5_cqe64 *cqe64,
>  	qp->sq.last_poll = tail;
>  }
>  
> +static void free_cq_buf(struct mlx5_ib_dev *dev, struct mlx5_ib_cq_buf *buf)
> +{
> +	mlx5_buf_free(&dev->mdev, &buf->buf);
> +}
> +
>  static int mlx5_poll_one(struct mlx5_ib_cq *cq,
>  			 struct mlx5_ib_qp **cur_qp,
>  			 struct ib_wc *wc)
> @@ -366,6 +381,7 @@ static int mlx5_poll_one(struct mlx5_ib_cq *cq,
>  	void *cqe;
>  	int idx;
>  
> +repoll:
>  	cqe = next_cqe_sw(cq);
>  	if (!cqe)
>  		return -EAGAIN;
> @@ -379,7 +395,18 @@ static int mlx5_poll_one(struct mlx5_ib_cq *cq,
>  	 */
>  	rmb();
>  
> -	/* TBD: resize CQ */
> +	opcode = cqe64->op_own >> 4;
> +	if (unlikely(opcode == MLX5_CQE_RESIZE_CQ)) {
> +		if (likely(cq->resize_buf)) {
> +			free_cq_buf(dev, &cq->buf);
> +			cq->buf = *cq->resize_buf;
> +			kfree(cq->resize_buf);
> +			cq->resize_buf = NULL;
> +			goto repoll;
> +		} else {
> +			mlx5_ib_warn(dev, "unexpected resize cqe\n");
> +		}
> +	}
>  
>  	qpn = ntohl(cqe64->sop_drop_qpn) & 0xffffff;
>  	if (!*cur_qp || (qpn != (*cur_qp)->ibqp.qp_num)) {
> @@ -398,7 +425,6 @@ static int mlx5_poll_one(struct mlx5_ib_cq *cq,
>  	}
>  
>  	wc->qp  = &(*cur_qp)->ibqp;
> -	opcode = cqe64->op_own >> 4;
>  	switch (opcode) {
>  	case MLX5_CQE_REQ:
>  		wq = &(*cur_qp)->sq;
> @@ -503,15 +529,11 @@ static int alloc_cq_buf(struct mlx5_ib_dev *dev, struct mlx5_ib_cq_buf *buf,
>  		return err;
>  
>  	buf->cqe_size = cqe_size;
> +	buf->nent = nent;
>  
>  	return 0;
>  }
>  
> -static void free_cq_buf(struct mlx5_ib_dev *dev, struct mlx5_ib_cq_buf *buf)
> -{
> -	mlx5_buf_free(&dev->mdev, &buf->buf);
> -}
> -
>  static int create_cq_user(struct mlx5_ib_dev *dev, struct ib_udata *udata,
>  			  struct ib_ucontext *context, struct mlx5_ib_cq *cq,
>  			  int entries, struct mlx5_create_cq_mbox_in **cqb,
> @@ -576,16 +598,16 @@ static void destroy_cq_user(struct mlx5_ib_cq *cq, struct ib_ucontext *context)
>  	ib_umem_release(cq->buf.umem);
>  }
>  
> -static void init_cq_buf(struct mlx5_ib_cq *cq, int nent)
> +static void init_cq_buf(struct mlx5_ib_cq *cq, struct mlx5_ib_cq_buf *buf)
>  {
>  	int i;
>  	void *cqe;
>  	struct mlx5_cqe64 *cqe64;
>  
> -	for (i = 0; i < nent; i++) {
> -		cqe = get_cqe(cq, i);
> -		cqe64 = (cq->buf.cqe_size == 64) ? cqe : cqe + 64;
> -		cqe64->op_own = 0xf1;
> +	for (i = 0; i < buf->nent; i++) {
> +		cqe = get_cqe_from_buf(buf, i, buf->cqe_size);
> +		cqe64 = buf->cqe_size == 64 ? cqe : cqe + 64;
> +		cqe64->op_own = MLX5_CQE_INVALID << 4;
>  	}
>  }
>  
> @@ -610,7 +632,7 @@ static int create_cq_kernel(struct mlx5_ib_dev *dev, struct mlx5_ib_cq *cq,
>  	if (err)
>  		goto err_db;
>  
> -	init_cq_buf(cq, entries);
> +	init_cq_buf(cq, &cq->buf);
>  
>  	*inlen = sizeof(**cqb) + sizeof(*(*cqb)->pas) * cq->buf.buf.npages;
>  	*cqb = mlx5_vzalloc(*inlen);
> @@ -836,7 +858,7 @@ int mlx5_ib_modify_cq(struct ib_cq *cq, u16 cq_count, u16 cq_period)
>  	in->ctx.cq_period = cpu_to_be16(cq_period);
>  	in->ctx.cq_max_count = cpu_to_be16(cq_count);
>  	in->field_select = cpu_to_be32(fsel);
> -	err = mlx5_core_modify_cq(&dev->mdev, &mcq->mcq, in);
> +	err = mlx5_core_modify_cq(&dev->mdev, &mcq->mcq, in, sizeof(*in));
>  	kfree(in);
>  
>  	if (err)
> @@ -845,9 +867,235 @@ int mlx5_ib_modify_cq(struct ib_cq *cq, u16 cq_count, u16 cq_period)
>  	return err;
>  }
>  
> +static int resize_user(struct mlx5_ib_dev *dev, struct mlx5_ib_cq *cq,
> +		       int entries, struct ib_udata *udata, int *npas,
> +		       int *page_shift, int *cqe_size)
> +{
> +	struct mlx5_ib_resize_cq ucmd;
> +	struct ib_umem *umem;
> +	int err;
> +	int npages;
> +	struct ib_ucontext *context = cq->buf.umem->context;
> +
> +	if (ib_copy_from_udata(&ucmd, udata, sizeof(ucmd)))
> +		return -EFAULT;
> +

You might also write

         err = ib_copy_from_udata(&ucmd, udata, sizeof(ucmd));
         if (err)
                 return err;

Then you should check reserved fields being set to the default value:
As noted by Daniel Vetter in its article "Botching up ioctls"[1]
  "Check *all* unused fields and flags and all the padding for whether 
   it's 0, and reject the ioctl if that's not the case. Otherwise your 
   nice plan for future extensions is going right down the gutters 
   since someone *will* submit an ioctl struct with random stack 
   garbage in the yet unused parts. Which then bakes in the ABI that 
   those fields can never be used for anything else but garbage."
It's  important to ensure that reserved fields are set to known value,
so that it will be possible to use them latter to extend the ABI.

[1] http://blog.ffwll.ch/2013/11/botching-up-ioctls.html

         if (ucmd.reserved0 || ucmd.reserved1)
                 return -EINVAL;

> +	umem = ib_umem_get(context, ucmd.buf_addr, entries * ucmd.cqe_size,
> +			   IB_ACCESS_LOCAL_WRITE, 1);
> +	if (IS_ERR(umem)) {
> +		err = PTR_ERR(umem);
> +		return err;
> +	}
> +
> +	mlx5_ib_cont_pages(umem, ucmd.buf_addr, &npages, page_shift,
> +			   npas, NULL);
> +
> +	cq->resize_umem = umem;
> +	*cqe_size = ucmd.cqe_size;
> +
> +	return 0;
> +}
> +


>  int mlx5_ib_resize_cq(struct ib_cq *ibcq, int entries, struct ib_udata *udata)
>  {
> -	return -ENOSYS;
> +	struct mlx5_ib_dev *dev = to_mdev(ibcq->device);
> +	struct mlx5_ib_cq *cq = to_mcq(ibcq);
> +	struct mlx5_modify_cq_mbox_in *in;
> +	int err;
> +	int npas;
> +	int page_shift;
> +	int inlen;
> +	int uninitialized_var(cqe_size);
> +	unsigned long flags;
> +
> +	if (!(dev->mdev.caps.flags & MLX5_DEV_CAP_FLAG_RESIZE_CQ)) {
> +		pr_info("Firmware does not support resize CQ\n");
> +		return -ENOSYS;
> +	}
> +
> +	if (entries < 1)
> +		return -EINVAL;
> +
> +	entries = roundup_pow_of_two(entries + 1);
> +	if (entries > dev->mdev.caps.max_cqes + 1)
> +		return -EINVAL;
> +
> +	if (entries == ibcq->cqe + 1)
> +		return 0;
> +
> +	mutex_lock(&cq->resize_mutex);
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> +	if (udata) {
> +		err = resize_user(dev, cq, entries, udata, &npas, &page_shift,
> +				  &cqe_size);
> +	} else {
> +		cqe_size = 64;
> +		err = resize_kernel(dev, cq, entries, cqe_size);
> +		if (!err) {
> +			npas = cq->resize_buf->buf.npages;
> +			page_shift = cq->resize_buf->buf.page_shift;
> +		}
> +	}
> +
> +	if (err)
> +		goto ex;
> +
> +	inlen = sizeof(*in) + npas * sizeof(in->pas[0]);
> +	in = mlx5_vzalloc(inlen);
> +	if (!in) {
> +		err = -ENOMEM;
> +		goto ex_resize;
> +	}
> +
> +	if (udata)
> +		mlx5_ib_populate_pas(dev, cq->resize_umem, page_shift,
> +				     in->pas, 0);
> +	else
> +		mlx5_fill_page_array(&cq->resize_buf->buf, in->pas);
> +
> +	in->field_select = cpu_to_be32(MLX5_MODIFY_CQ_MASK_LOG_SIZE  |
> +				       MLX5_MODIFY_CQ_MASK_PG_OFFSET |
> +				       MLX5_MODIFY_CQ_MASK_PG_SIZE);
> +	in->ctx.log_pg_sz = page_shift - MLX5_ADAPTER_PAGE_SHIFT;
> +	in->ctx.cqe_sz_flags = cqe_sz_to_mlx_sz(cqe_size) << 5;
> +	in->ctx.page_offset = 0;
> +	in->ctx.log_sz_usr_page = cpu_to_be32(ilog2(entries) << 24);
> +	in->hdr.opmod = cpu_to_be16(MLX5_CQ_OPMOD_RESIZE);
> +	in->cqn = cpu_to_be32(cq->mcq.cqn);
> +
> +	err = mlx5_core_modify_cq(&dev->mdev, &cq->mcq, in, inlen);
> +	if (err)
> +		goto ex_alloc;
> +
> +	if (udata) {
> +		cq->ibcq.cqe = entries - 1;
> +		ib_umem_release(cq->buf.umem);
> +		cq->buf.umem = cq->resize_umem;
> +		cq->resize_umem = NULL;
> +	} else {
> +		struct mlx5_ib_cq_buf tbuf;
> +		int resized = 0;
> +
> +		spin_lock_irqsave(&cq->lock, flags);
> +		if (cq->resize_buf) {
> +			err = copy_resize_cqes(cq);
> +			if (!err) {
> +				tbuf = cq->buf;
> +				cq->buf = *cq->resize_buf;
> +				kfree(cq->resize_buf);
> +				cq->resize_buf = NULL;
> +				resized = 1;
> +			}
> +		}
> +		cq->ibcq.cqe = entries - 1;
> +		spin_unlock_irqrestore(&cq->lock, flags);
> +		if (resized)
> +			free_cq_buf(dev, &tbuf);
> +	}
> +	mutex_unlock(&cq->resize_mutex);
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Is everything in this section really critical.
For example, allocating and setting 'in' structure or releasing the
ressources could probably move outside the mutex protected section ?

> +
> +	mlx5_vfree(in);
> +	return 0;
> +
> +ex_alloc:
> +	mlx5_vfree(in);
> +
> +ex_resize:
> +	if (udata)
> +		un_resize_user(cq);
> +	else
> +		un_resize_kernel(dev, cq);
> +ex:
> +	mutex_unlock(&cq->resize_mutex);
> +	return err;
>  }
>  

>  
> 
>  int mlx5_core_modify_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq,
> -			struct mlx5_modify_cq_mbox_in *in)
> +			struct mlx5_modify_cq_mbox_in *in, int in_sz)
                                                            ^^^^^^^^^^

Should probably be 'unsigned' ? size_t ?

>  {
>  	struct mlx5_modify_cq_mbox_out out;
>  	int err;
>  
>  	memset(&out, 0, sizeof(out));
>  	in->hdr.opcode = cpu_to_be16(MLX5_CMD_OP_MODIFY_CQ);
> -	err = mlx5_cmd_exec(dev, in, sizeof(*in), &out, sizeof(out));
> +	err = mlx5_cmd_exec(dev, in, in_sz, &out, sizeof(out));
>  	if (err)
>  		return err;
>  
> @@ -158,7 +166,7 @@ int mlx5_core_destroy_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq);
>  int mlx5_core_query_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq,
>  		       struct mlx5_query_cq_mbox_out *out);
>  int mlx5_core_modify_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq,
> -			struct mlx5_modify_cq_mbox_in *in);
> +			struct mlx5_modify_cq_mbox_in *in, int in_sz);
                                                            ^^^^^^^^^^
same here.

diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
> index dbb03ca..87e2371 100644
> --- a/include/linux/mlx5/device.h
> +++ b/include/linux/mlx5/device.h
> @@ -710,6 +711,7 @@ struct mlx5_modify_cq_mbox_in {
>  
>  struct mlx5_modify_cq_mbox_out {
>  	struct mlx5_outbox_hdr	hdr;
> +	u8			rsvd[8];
>  };
>  
>  struct mlx5_enable_hca_mbox_in {
> 

It not clear why 8 bytes are needed here ?

Regards.

-- 
Yann Droneaud
OPTEYA


--
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 09/14] IB/mlx5: Add support for resize CQ

[Eli Cohen]

On Tue, Jan 14, 2014 at 05:36:50PM +0100, Yann Droneaud wrote:
> > +	if (ib_copy_from_udata(&ucmd, udata, sizeof(ucmd)))
> > +		return -EFAULT;
> > +
> 
> You might also write
> 
>          err = ib_copy_from_udata(&ucmd, udata, sizeof(ucmd));
>          if (err)
>                  return err;
> 
> Then you should check reserved fields being set to the default value:
> As noted by Daniel Vetter in its article "Botching up ioctls"[1]
>   "Check *all* unused fields and flags and all the padding for whether 
>    it's 0, and reject the ioctl if that's not the case. Otherwise your 
>    nice plan for future extensions is going right down the gutters 
>    since someone *will* submit an ioctl struct with random stack 
>    garbage in the yet unused parts. Which then bakes in the ABI that 
>    those fields can never be used for anything else but garbage."
> It's  important to ensure that reserved fields are set to known value,
> so that it will be possible to use them latter to extend the ABI.
> 
> [1] http://blog.ffwll.ch/2013/11/botching-up-ioctls.html
> 
>          if (ucmd.reserved0 || ucmd.reserved1)
>                  return -EINVAL;
> 
It is not likely that someone will pass non-zero values here since
libmlx5 clears and most apps will use it. But I agree with your
comment - thanks for pointing this out. Probably there are other
places that need to be checked.


> > +	}
> > +	mutex_unlock(&cq->resize_mutex);
>          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Is everything in this section really critical.
> For example, allocating and setting 'in' structure or releasing the
> ressources could probably move outside the mutex protected section ?
> 

Well, you could move things around to shorten the overall time the
lock is held but that might require structural changes in the code
that will not necessairily fit nice. Resizing a CQ is not a frequent
operation and this lock is used to avoid concurrent attempts of
resizing of the same CQ so I would not invest more effort here.

> >  
> > 
> >  int mlx5_core_modify_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq,
> > -			struct mlx5_modify_cq_mbox_in *in)
> > +			struct mlx5_modify_cq_mbox_in *in, int in_sz)
>                                                             ^^^^^^^^^^
> 
> Should probably be 'unsigned' ? size_t ?
> 
> same here.
> 

The resized value is defined int at the ib core layer so I chose to
follow the same type to avoid need for casting. Maybe a future patch
could change the type all over.

> diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
> > index dbb03ca..87e2371 100644
> > --- a/include/linux/mlx5/device.h
> > +++ b/include/linux/mlx5/device.h
> > @@ -710,6 +711,7 @@ struct mlx5_modify_cq_mbox_in {
> >  
> >  struct mlx5_modify_cq_mbox_out {
> >  	struct mlx5_outbox_hdr	hdr;
> > +	u8			rsvd[8];
> >  };
> >  
> >  struct mlx5_enable_hca_mbox_in {
> > 
> 
> It not clear why 8 bytes are needed here ?
> 
This is a requirement of the driver/firmware interface.
--
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 09/14] IB/mlx5: Add support for resize CQ

[Yann Droneaud]

Hi Eli, 

Le mercredi 15 janvier 2014 à 09:33 +0200, Eli Cohen a écrit :
> On Tue, Jan 14, 2014 at 05:36:50PM +0100, Yann Droneaud wrote:
> > > +	if (ib_copy_from_udata(&ucmd, udata, sizeof(ucmd)))
> > > +		return -EFAULT;
> > > +
> > 
> > You might also write
> > 
> >          err = ib_copy_from_udata(&ucmd, udata, sizeof(ucmd));
> >          if (err)
> >                  return err;

I'd like to have your opinion on this change.

I'm going to patch every call to ib_copy_from_udata() to follow my
proposed scheme (eg. returned error code from ib_copy_from_udata()
instead of hard coded error value).

> > 
> > Then you should check reserved fields being set to the default value:
> > As noted by Daniel Vetter in its article "Botching up ioctls"[1]
> >   "Check *all* unused fields and flags and all the padding for whether 
> >    it's 0, and reject the ioctl if that's not the case. Otherwise your 
> >    nice plan for future extensions is going right down the gutters 
> >    since someone *will* submit an ioctl struct with random stack 
> >    garbage in the yet unused parts. Which then bakes in the ABI that 
> >    those fields can never be used for anything else but garbage."
> > It's  important to ensure that reserved fields are set to known value,
> > so that it will be possible to use them latter to extend the ABI.
> > 
> > [1] http://blog.ffwll.ch/2013/11/botching-up-ioctls.html
> > 
> >          if (ucmd.reserved0 || ucmd.reserved1)
> >                  return -EINVAL;
> > 
> It is not likely that someone will pass non-zero values here since
> libmlx5 clears and most apps will use it.

Is libmlx5/libibverbs the ABI ?

Unfortunately, anybody is allowed to access the kernel uverbs API
directly, so we must take care of the kernel ABI, just in case.

> But I agree with your
> comment - thanks for pointing this out. Probably there are other
> places that need to be checked.
> 

For code not yet part of a released kernel version, we can fix that.
But for all other, it would require proper checking/thinking before
rejecting reserved field not set to 0 since it might theoterically break
existing userspace program: it will be a departure from previous ABI.

> 
> > > +	}
> > > +	mutex_unlock(&cq->resize_mutex);
> >          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > 
> > Is everything in this section really critical.
> > For example, allocating and setting 'in' structure or releasing the
> > ressources could probably move outside the mutex protected section ?
> > 
> 
> Well, you could move things around to shorten the overall time the
> lock is held but that might require structural changes in the code
> that will not necessairily fit nice. Resizing a CQ is not a frequent
> operation and this lock is used to avoid concurrent attempts of
> resizing of the same CQ so I would not invest more effort here.
> 

I agree.

I would found more readable to have the two locks held next each other.
YMMV.


> > >  
> > > 
> > >  int mlx5_core_modify_cq(struct mlx5_core_dev *dev, struct mlx5_core_cq *cq,
> > > -			struct mlx5_modify_cq_mbox_in *in)
> > > +			struct mlx5_modify_cq_mbox_in *in, int in_sz)
> >                                                             ^^^^^^^^^^
> > 
> > Should probably be 'unsigned' ? size_t ?
> > 
> > same here.
> > 
> 
> The resized value is defined int at the ib core layer so I chose to
> follow the same type to avoid need for casting. Maybe a future patch
> could change the type all over.
> 

But it's the size of struct mlx5_modify_cq_mbox_in, not the number of
'cqe' to resize the cq to.

> > diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
> > > index dbb03ca..87e2371 100644
> > > --- a/include/linux/mlx5/device.h
> > > +++ b/include/linux/mlx5/device.h
> > > @@ -710,6 +711,7 @@ struct mlx5_modify_cq_mbox_in {
> > >  
> > >  struct mlx5_modify_cq_mbox_out {
> > >  	struct mlx5_outbox_hdr	hdr;
> > > +	u8			rsvd[8];
> > >  };
> > >  
> > >  struct mlx5_enable_hca_mbox_in {
> > > 
> > 
> > It not clear why 8 bytes are needed here ?
> > 
> This is a requirement of the driver/firmware interface.

It's a bit of magic :(

Regards.

-- 
Yann Droneaud
OPTEYA


--
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 09/14] IB/mlx5: Add support for resize CQ

[Eli Cohen]

On Wed, Jan 15, 2014 at 11:02:55AM +0100, Yann Droneaud wrote:
> 
> I'd like to have your opinion on this change.
> 
> I'm going to patch every call to ib_copy_from_udata() to follow my
> proposed scheme (eg. returned error code from ib_copy_from_udata()
> instead of hard coded error value).
>
Yes, makes sense to me.

> 
> Is libmlx5/libibverbs the ABI ?
> 
> Unfortunately, anybody is allowed to access the kernel uverbs API
> directly, so we must take care of the kernel ABI, just in case.
>

I am not sure I understand the question but libibverbs to libmlx5
relation is somehow like the relationship between ib_core and mlx5_ib.
e.g. libibverbs sets the API and libmlx5 implements it.
> 
> For code not yet part of a released kernel version, we can fix that.
> But for all other, it would require proper checking/thinking before
> rejecting reserved field not set to 0 since it might theoterically break
> existing userspace program: it will be a departure from previous ABI.
> 

Yes, this needs to be done carefully for each driver. We need to check
if either libibverbs or any other provider library clears the struct
before sending. If this is the case then it should be safe to fix the
kernel part.

> 
> I would found more readable to have the two locks held next each other.
> YMMV.
> 
I am not following you here. Please be more specific.
> 
> 
> But it's the size of struct mlx5_modify_cq_mbox_in, not the number of
> 'cqe' to resize the cq to.
> 

Oh I see. Well, I don't think it will ever matter that I using int.
I use it in other places too.
--
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 09/14] IB/mlx5: Add support for resize CQ

[Yann Droneaud]

Hi,

Le mercredi 15 janvier 2014 à 13:24 +0200, Eli Cohen a écrit :
> On Wed, Jan 15, 2014 at 11:02:55AM +0100, Yann Droneaud wrote:
> > 
> > Is libmlx5/libibverbs the ABI ?
> > 
> > Unfortunately, anybody is allowed to access the kernel uverbs API
> > directly, so we must take care of the kernel ABI, just in case.
> >
> 
> I am not sure I understand the question but libibverbs to libmlx5
> relation is somehow like the relationship between ib_core and mlx5_ib.
> e.g. libibverbs sets the API and libmlx5 implements it.

It's a theoretical question:
where to draw the line of the kernel ABI when it comes to uverbs ?
Is it write() + all the data structures for request and response ?
Or is it libibverbs + provider specific library ?

In second case, we can change the behavor of the uverbs provided that it
doesn't interfere with libibverbs: modifying corner case behavors which
were never hit by libibverbs is then safe: it won't break userspace
application.

But in the first case, we cannot for sure attest that modifying behavor
will not break hypothetical userspace application issuing uverbs command
without going through libibverbs: such specific/specialized application
might call uverbs in a way different that libibverbs which could break.

I'm not aware of such unknown application, but I believe it could exists
in the "proprietary" world.

> > 
> > For code not yet part of a released kernel version, we can fix that.
> > But for all other, it would require proper checking/thinking before
> > rejecting reserved field not set to 0 since it might theoterically break
> > existing userspace program: it will be a departure from previous ABI.
> > 
> 
> Yes, this needs to be done carefully for each driver. We need to check
> if either libibverbs or any other provider library clears the struct
> before sending. If this is the case then it should be safe to fix the
> kernel part.
> 
> > 
> > I would found more readable to have the two locks held next each other.
> > YMMV.
> > 
> I am not following you here. Please be more specific.

I thought that reworking the code to move mutex_lock(&cq->resize_mutex)
and spin_lock_irqsave(&cq->lock, flags); next to each other
could show that the first one would not be needed: perhaps is it
possible doing:
 - prepare everything,
 - take the lock,
 - swap,
 - release the lock,
 - clean up.
(oh, that remind me RCU).

I agree that resizing the CQ is not a frequent operation, so trying to
rewrite the code as I suggest won't give a lot of advantages.

Regards.

-- 
Yann Droneaud
OPTEYA


--
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 10/14] net/mlx5_core: Improve debugfs readability

[Eli Cohen]

Use strings to display transport service or state of QPs. Use numerical value
for MTU of a QP.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/debugfs.c | 39 +++++++++++++++++---
 include/linux/mlx5/qp.h                           | 45 +++++++++++++++++++++++
 2 files changed, 78 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c b/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
index 80f6d12..10e1f1a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
@@ -275,7 +275,7 @@ void mlx5_cq_debugfs_cleanup(struct mlx5_core_dev *dev)
 }
 
 static u64 qp_read_field(struct mlx5_core_dev *dev, struct mlx5_core_qp *qp,
-			 int index)
+			 int index, int *is_str)
 {
 	struct mlx5_query_qp_mbox_out *out;
 	struct mlx5_qp_context *ctx;
@@ -293,19 +293,40 @@ static u64 qp_read_field(struct mlx5_core_dev *dev, struct mlx5_core_qp *qp,
 		goto out;
 	}
 
+	*is_str = 0;
 	ctx = &out->ctx;
 	switch (index) {
 	case QP_PID:
 		param = qp->pid;
 		break;
 	case QP_STATE:
-		param = be32_to_cpu(ctx->flags) >> 28;
+		param = (u64)mlx5_qp_state_str(be32_to_cpu(ctx->flags) >> 28);
+		*is_str = 1;
 		break;
 	case QP_XPORT:
-		param = (be32_to_cpu(ctx->flags) >> 16) & 0xff;
+		param = (u64)mlx5_qp_type_str((be32_to_cpu(ctx->flags) >> 16) & 0xff);
+		*is_str = 1;
 		break;
 	case QP_MTU:
-		param = ctx->mtu_msgmax >> 5;
+		switch (ctx->mtu_msgmax >> 5) {
+		case IB_MTU_256:
+			param = 256;
+			break;
+		case IB_MTU_512:
+			param = 512;
+			break;
+		case IB_MTU_1024:
+			param = 1024;
+			break;
+		case IB_MTU_2048:
+			param = 2048;
+			break;
+		case IB_MTU_4096:
+			param = 4096;
+			break;
+		default:
+			param = 0;
+		}
 		break;
 	case QP_N_RECV:
 		param = 1 << ((ctx->rq_size_stride >> 3) & 0xf);
@@ -414,6 +435,7 @@ static ssize_t dbg_read(struct file *filp, char __user *buf, size_t count,
 	struct mlx5_field_desc *desc;
 	struct mlx5_rsc_debug *d;
 	char tbuf[18];
+	int is_str = 0;
 	u64 field;
 	int ret;
 
@@ -424,7 +446,7 @@ static ssize_t dbg_read(struct file *filp, char __user *buf, size_t count,
 	d = (void *)(desc - desc->i) - sizeof(*d);
 	switch (d->type) {
 	case MLX5_DBG_RSC_QP:
-		field = qp_read_field(d->dev, d->object, desc->i);
+		field = qp_read_field(d->dev, d->object, desc->i, &is_str);
 		break;
 
 	case MLX5_DBG_RSC_EQ:
@@ -440,7 +462,12 @@ static ssize_t dbg_read(struct file *filp, char __user *buf, size_t count,
 		return -EINVAL;
 	}
 
-	ret = snprintf(tbuf, sizeof(tbuf), "0x%llx\n", field);
+
+	if (is_str)
+		ret = snprintf(tbuf, sizeof(tbuf), "%s\n", (const char *)field);
+	else
+		ret = snprintf(tbuf, sizeof(tbuf), "0x%llx\n", field);
+
 	if (ret > 0) {
 		if (copy_to_user(buf, tbuf, ret))
 			return -EFAULT;
diff --git a/include/linux/mlx5/qp.h b/include/linux/mlx5/qp.h
index d9e3eac..d51eff7 100644
--- a/include/linux/mlx5/qp.h
+++ b/include/linux/mlx5/qp.h
@@ -464,4 +464,49 @@ void mlx5_cleanup_qp_table(struct mlx5_core_dev *dev);
 int mlx5_debug_qp_add(struct mlx5_core_dev *dev, struct mlx5_core_qp *qp);
 void mlx5_debug_qp_remove(struct mlx5_core_dev *dev, struct mlx5_core_qp *qp);
 
+static inline const char *mlx5_qp_type_str(int type)
+{
+	switch (type) {
+	case MLX5_QP_ST_RC: return "RC";
+	case MLX5_QP_ST_UC: return "C";
+	case MLX5_QP_ST_UD: return "UD";
+	case MLX5_QP_ST_XRC: return "XRC";
+	case MLX5_QP_ST_MLX: return "MLX";
+	case MLX5_QP_ST_QP0: return "QP0";
+	case MLX5_QP_ST_QP1: return "QP1";
+	case MLX5_QP_ST_RAW_ETHERTYPE: return "RAW_ETHERTYPE";
+	case MLX5_QP_ST_RAW_IPV6: return "RAW_IPV6";
+	case MLX5_QP_ST_SNIFFER: return "SNIFFER";
+	case MLX5_QP_ST_SYNC_UMR: return "SYNC_UMR";
+	case MLX5_QP_ST_PTP_1588: return "PTP_1588";
+	case MLX5_QP_ST_REG_UMR: return "REG_UMR";
+	default: return "Invalid transport type";
+	}
+}
+
+static inline const char *mlx5_qp_state_str(int state)
+{
+	switch (state) {
+	case MLX5_QP_STATE_RST:
+	return "RST";
+	case MLX5_QP_STATE_INIT:
+	return "INIT";
+	case MLX5_QP_STATE_RTR:
+	return "RTR";
+	case MLX5_QP_STATE_RTS:
+	return "RTS";
+	case MLX5_QP_STATE_SQER:
+	return "SQER";
+	case MLX5_QP_STATE_SQD:
+	return "SQD";
+	case MLX5_QP_STATE_ERR:
+	return "ERR";
+	case MLX5_QP_STATE_SQ_DRAINING:
+	return "SQ_DRAINING";
+	case MLX5_QP_STATE_SUSPENDED:
+	return "SUSPENDED";
+	default: return "Invalid QP state";
+	}
+}
+
 #endif /* MLX5_QP_H */
-- 
1.8.5.2

--
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 11/14] mlx5_core: Fix PowerPC support

[Eli Cohen]

1. Fix derivation of sub-page index from the dma address in free_4k.
2. Fix the DMA address passed to dma_unmap_page by masking it properly.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c | 9 +++++----
 include/linux/mlx5/device.h                         | 3 ++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
index c35c432..d59790a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
@@ -99,7 +99,7 @@ enum {
 
 enum {
 	MLX5_MAX_RECLAIM_TIME_MILI	= 5000,
-	MLX5_NUM_4K_IN_PAGE		= PAGE_SIZE / 4096,
+	MLX5_NUM_4K_IN_PAGE		= PAGE_SIZE / MLX5_ADAPTER_PAGE_SIZE,
 };
 
 static int insert_page(struct mlx5_core_dev *dev, u64 addr, struct page *page, u16 func_id)
@@ -206,7 +206,7 @@ static int alloc_4k(struct mlx5_core_dev *dev, u64 *addr)
 	if (!fp->free_count)
 		list_del(&fp->list);
 
-	*addr = fp->addr + n * 4096;
+	*addr = fp->addr + n * MLX5_ADAPTER_PAGE_SIZE;
 
 	return 0;
 }
@@ -222,14 +222,15 @@ static void free_4k(struct mlx5_core_dev *dev, u64 addr)
 		return;
 	}
 
-	n = (addr & ~PAGE_MASK) % 4096;
+	n = (addr & ~PAGE_MASK) >> MLX5_ADAPTER_PAGE_SHIFT;
 	fwp->free_count++;
 	set_bit(n, &fwp->bitmask);
 	if (fwp->free_count == MLX5_NUM_4K_IN_PAGE) {
 		rb_erase(&fwp->rb_node, &dev->priv.page_root);
 		if (fwp->free_count != 1)
 			list_del(&fwp->list);
-		dma_unmap_page(&dev->pdev->dev, addr, PAGE_SIZE, DMA_BIDIRECTIONAL);
+		dma_unmap_page(&dev->pdev->dev, addr & PAGE_MASK, PAGE_SIZE,
+			       DMA_BIDIRECTIONAL);
 		__free_page(fwp->page);
 		kfree(fwp);
 	} else if (fwp->free_count == 1) {
diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
index 87e2371..1d05909 100644
--- a/include/linux/mlx5/device.h
+++ b/include/linux/mlx5/device.h
@@ -234,7 +234,8 @@ enum {
 };
 
 enum {
-	MLX5_ADAPTER_PAGE_SHIFT		= 12
+	MLX5_ADAPTER_PAGE_SHIFT		= 12,
+	MLX5_ADAPTER_PAGE_SIZE		= 1 << MLX5_ADAPTER_PAGE_SHIFT,
 };
 
 enum {
-- 
1.8.5.2

--
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 12/14] IB/mlx5: Allow creation of QPs with zero length work queues

[Eli Cohen]

The current code attmepts to call ib_umem_get() even if the length is zero
which causes a failure. Since the spec allows zero length work queues, change
the code so we don't call ib_umem_get() in those cases.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/qp.c | 49 ++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index 87b7fb1..70dd770 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -523,12 +523,12 @@ static int create_user_qp(struct mlx5_ib_dev *dev, struct ib_pd *pd,
 {
 	struct mlx5_ib_ucontext *context;
 	struct mlx5_ib_create_qp ucmd;
-	int page_shift;
+	int page_shift = 0;
 	int uar_index;
 	int npages;
-	u32 offset;
+	u32 offset = 0;
 	int uuarn;
-	int ncont;
+	int ncont = 0;
 	int err;
 
 	err = ib_copy_from_udata(&ucmd, udata, sizeof(ucmd));
@@ -564,23 +564,29 @@ static int create_user_qp(struct mlx5_ib_dev *dev, struct ib_pd *pd,
 	if (err)
 		goto err_uuar;
 
-	qp->umem = ib_umem_get(pd->uobject->context, ucmd.buf_addr,
-			       qp->buf_size, 0, 0);
-	if (IS_ERR(qp->umem)) {
-		mlx5_ib_dbg(dev, "umem_get failed\n");
-		err = PTR_ERR(qp->umem);
-		goto err_uuar;
+	if (ucmd.buf_addr && qp->buf_size) {
+		qp->umem = ib_umem_get(pd->uobject->context, ucmd.buf_addr,
+				       qp->buf_size, 0, 0);
+		if (IS_ERR(qp->umem)) {
+			mlx5_ib_dbg(dev, "umem_get failed\n");
+			err = PTR_ERR(qp->umem);
+			goto err_uuar;
+		}
+	} else {
+		qp->umem = NULL;
 	}
 
-	mlx5_ib_cont_pages(qp->umem, ucmd.buf_addr, &npages, &page_shift,
-			   &ncont, NULL);
-	err = mlx5_ib_get_buf_offset(ucmd.buf_addr, page_shift, &offset);
-	if (err) {
-		mlx5_ib_warn(dev, "bad offset\n");
-		goto err_umem;
+	if (qp->umem) {
+		mlx5_ib_cont_pages(qp->umem, ucmd.buf_addr, &npages, &page_shift,
+				   &ncont, NULL);
+		err = mlx5_ib_get_buf_offset(ucmd.buf_addr, page_shift, &offset);
+		if (err) {
+			mlx5_ib_warn(dev, "bad offset\n");
+			goto err_umem;
+		}
+		mlx5_ib_dbg(dev, "addr 0x%llx, size %d, npages %d, page_shift %d, ncont %d, offset %d\n",
+			    ucmd.buf_addr, qp->buf_size, npages, page_shift, ncont, offset);
 	}
-	mlx5_ib_dbg(dev, "addr 0x%llx, size %d, npages %d, page_shift %d, ncont %d, offset %d\n",
-		    ucmd.buf_addr, qp->buf_size, npages, page_shift, ncont, offset);
 
 	*inlen = sizeof(**in) + sizeof(*(*in)->pas) * ncont;
 	*in = mlx5_vzalloc(*inlen);
@@ -588,7 +594,8 @@ static int create_user_qp(struct mlx5_ib_dev *dev, struct ib_pd *pd,
 		err = -ENOMEM;
 		goto err_umem;
 	}
-	mlx5_ib_populate_pas(dev, qp->umem, page_shift, (*in)->pas, 0);
+	if (qp->umem)
+		mlx5_ib_populate_pas(dev, qp->umem, page_shift, (*in)->pas, 0);
 	(*in)->ctx.log_pg_sz_remote_qpn =
 		cpu_to_be32((page_shift - MLX5_ADAPTER_PAGE_SHIFT) << 24);
 	(*in)->ctx.params2 = cpu_to_be32(offset << 6);
@@ -619,7 +626,8 @@ err_free:
 	mlx5_vfree(*in);
 
 err_umem:
-	ib_umem_release(qp->umem);
+	if (qp->umem)
+		ib_umem_release(qp->umem);
 
 err_uuar:
 	free_uuar(&context->uuari, uuarn);
@@ -632,7 +640,8 @@ static void destroy_qp_user(struct ib_pd *pd, struct mlx5_ib_qp *qp)
 
 	context = to_mucontext(pd->uobject->context);
 	mlx5_ib_db_unmap_user(context, &qp->db);
-	ib_umem_release(qp->umem);
+	if (qp->umem)
+		ib_umem_release(qp->umem);
 	free_uuar(&context->uuari, qp->uuarn);
 }
 
-- 
1.8.5.2

--
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 13/14] IB/mlx5: Abort driver cleanup if teardown hca fails

[Eli Cohen]

Do not continue with cleanup flow. If this ever happens we can check which
resources remained open.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/main.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 40a9f5e..a064f06 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -460,7 +460,10 @@ disable_msix:
 
 err_stop_poll:
 	mlx5_stop_health_poll(dev);
-	mlx5_cmd_teardown_hca(dev);
+	if (mlx5_cmd_teardown_hca(dev)) {
+		dev_err(&dev->pdev->dev, "tear_down_hca failed, skip cleanup\n");
+		return err;
+	}
 
 err_pagealloc_stop:
 	mlx5_pagealloc_stop(dev);
@@ -503,7 +506,10 @@ void mlx5_dev_cleanup(struct mlx5_core_dev *dev)
 	mlx5_eq_cleanup(dev);
 	mlx5_disable_msix(dev);
 	mlx5_stop_health_poll(dev);
-	mlx5_cmd_teardown_hca(dev);
+	if (mlx5_cmd_teardown_hca(dev)) {
+		dev_err(&dev->pdev->dev, "tear_down_hca failed, skip cleanup\n");
+		return;
+	}
 	mlx5_pagealloc_stop(dev);
 	mlx5_reclaim_startup_pages(dev);
 	mlx5_core_disable_hca(dev);
-- 
1.8.5.2

--
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 14/14] IB/mlx5: Remove old field for create mkey mailbox

[Eli Cohen]

Align to firmware specification.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 include/linux/mlx5/device.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
index 1d05909..817a6fa 100644
--- a/include/linux/mlx5/device.h
+++ b/include/linux/mlx5/device.h
@@ -849,8 +849,8 @@ struct mlx5_create_mkey_mbox_in {
 	struct mlx5_mkey_seg	seg;
 	u8			rsvd1[16];
 	__be32			xlat_oct_act_size;
-	__be32			bsf_coto_act_size;
-	u8			rsvd2[168];
+	__be32			rsvd2;
+	u8			rsvd3[168];
 	__be64			pas[0];
 };
 
-- 
1.8.5.2

--
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 14/14] IB/mlx5: Remove old field for create mkey mailbox

[Yann Droneaud]

Le mardi 14 janvier 2014 à 17:45 +0200, Eli Cohen a écrit :
> Align to firmware specification.
> 

Is it subject to change between various firmware revision ?

If yes, it could of user interest to specify the base/current
version of the applicable firmware.

> Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  include/linux/mlx5/device.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
> index 1d05909..817a6fa 100644
> --- a/include/linux/mlx5/device.h
> +++ b/include/linux/mlx5/device.h
> @@ -849,8 +849,8 @@ struct mlx5_create_mkey_mbox_in {
>  	struct mlx5_mkey_seg	seg;
>  	u8			rsvd1[16];
>  	__be32			xlat_oct_act_size;
> -	__be32			bsf_coto_act_size;
> -	u8			rsvd2[168];
> +	__be32			rsvd2;

For a reserved field, I think we don't care of it being be or le.

> +	u8			rsvd3[168];
>  	__be64			pas[0];
>  };
>  

Regards.

-- 
Yann Droneaud
OPTEYA


--
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 14/14] IB/mlx5: Remove old field for create mkey mailbox

[Eli Cohen]

On Tue, Jan 14, 2014 at 05:37:17PM +0100, Yann Droneaud wrote:
> 
> Is it subject to change between various firmware revision ?
> 
> If yes, it could of user interest to specify the base/current
> version of the applicable firmware.

The sizes never changed and this field was never used so I just
redefined it as reserved.
> 
> > @@ -849,8 +849,8 @@ struct mlx5_create_mkey_mbox_in {
> >  	struct mlx5_mkey_seg	seg;
> >  	u8			rsvd1[16];
> >  	__be32			xlat_oct_act_size;
> > -	__be32			bsf_coto_act_size;
> > -	u8			rsvd2[168];
> > +	__be32			rsvd2;
> 
> For a reserved field, I think we don't care of it being be or le.
> 
Right, so be is just as good as le. The "be" is a sort of reminder
that driver/firmware interface is always big endian.

> > +	u8			rsvd3[168];
> >  	__be64			pas[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