[PATCH mlx5-next 0/4] mlx5 next misc updates

[PATCH mlx5-next 0/4] mlx5 next misc updates

[Saeed Mahameed]

Hi all,

This series includes updates to mlx5-next shared branch.

1) from Jason, improve mlx5_cmd_exec_cb async API to be safer
2) from Maxim Mikityanskiy, cleanups for mlx5_write64 doorbell API
3) from Michael Guralnik, Add pci AtomicOps request

Thanks,
Saeed.

---

Jason Gunthorpe (1):
  net/mlx5: Make mlx5_cmd_exec_cb() a safe API

Maxim Mikityanskiy (2):
  net/mlx5: Remove unused MLX5_*_DOORBELL_LOCK macros
  net/mlx5: Remove spinlock support from mlx5_write64

Michael Guralnik (1):
  net/mlx5: Add pci AtomicOps request

 drivers/infiniband/hw/mlx5/mlx5_ib.h          |  2 +
 drivers/infiniband/hw/mlx5/mr.c               | 39 +++----------
 drivers/infiniband/hw/mlx5/qp.c               |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 55 +++++++++++++++++--
 drivers/net/ethernet/mellanox/mlx5/core/en.h  |  2 +-
 .../ethernet/mellanox/mlx5/core/fpga/conn.c   |  2 +-
 .../net/ethernet/mellanox/mlx5/core/main.c    |  5 ++
 drivers/net/ethernet/mellanox/mlx5/core/mr.c  | 11 ++--
 include/linux/mlx5/cq.h                       |  2 +-
 include/linux/mlx5/doorbell.h                 | 36 ++----------
 include/linux/mlx5/driver.h                   | 32 +++++++++--
 11 files changed, 105 insertions(+), 83 deletions(-)

-- 
2.20.1

[PATCH mlx5-next 1/4] net/mlx5: Make mlx5_cmd_exec_cb() a safe API

[Saeed Mahameed]

From: Jason Gunthorpe <jgg@mellanox.com>

APIs that have deferred callbacks should have some kind of cleanup
function that callers can use to fence the callbacks. Otherwise things
like module unloading can lead to dangling function pointers, or worse.

The IB MR code is the only place that calls this function and had a
really poor attempt at creating this fence. Provide a good version in
the core code as future patches will add more places that need this
fence.

Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/infiniband/hw/mlx5/mlx5_ib.h          |  2 +
 drivers/infiniband/hw/mlx5/mr.c               | 39 +++----------
 drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 55 +++++++++++++++++--
 drivers/net/ethernet/mellanox/mlx5/core/mr.c  | 11 ++--
 include/linux/mlx5/driver.h                   | 32 +++++++++--
 5 files changed, 91 insertions(+), 48 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index efe383c0ac86..eedba0d2ec4b 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -587,6 +587,7 @@ struct mlx5_ib_mr {
 	struct mlx5_ib_mr      *parent;
 	atomic_t		num_leaf_free;
 	wait_queue_head_t       q_leaf_free;
+	struct mlx5_async_work  cb_work;
 };
 
 struct mlx5_ib_mw {
@@ -944,6 +945,7 @@ struct mlx5_ib_dev {
 	struct mlx5_memic	memic;
 	u16			devx_whitelist_uid;
 	struct mlx5_srq_table   srq_table;
+	struct mlx5_async_ctx   async_ctx;
 };
 
 static inline struct mlx5_ib_cq *to_mibcq(struct mlx5_core_cq *mcq)
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index fd6ea1f75085..bf2b6ea23851 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -123,9 +123,10 @@ static void update_odp_mr(struct mlx5_ib_mr *mr)
 }
 #endif
 
-static void reg_mr_callback(int status, void *context)
+static void reg_mr_callback(int status, struct mlx5_async_work *context)
 {
-	struct mlx5_ib_mr *mr = context;
+	struct mlx5_ib_mr *mr =
+		container_of(context, struct mlx5_ib_mr, cb_work);
 	struct mlx5_ib_dev *dev = mr->dev;
 	struct mlx5_mr_cache *cache = &dev->cache;
 	int c = order2idx(dev, mr->order);
@@ -216,9 +217,9 @@ static int add_keys(struct mlx5_ib_dev *dev, int c, int num)
 		ent->pending++;
 		spin_unlock_irq(&ent->lock);
 		err = mlx5_core_create_mkey_cb(dev->mdev, &mr->mmkey,
-					       in, inlen,
+					       &dev->async_ctx, in, inlen,
 					       mr->out, sizeof(mr->out),
-					       reg_mr_callback, mr);
+					       reg_mr_callback, &mr->cb_work);
 		if (err) {
 			spin_lock_irq(&ent->lock);
 			ent->pending--;
@@ -679,6 +680,7 @@ int mlx5_mr_cache_init(struct mlx5_ib_dev *dev)
 		return -ENOMEM;
 	}
 
+	mlx5_cmd_init_async_ctx(dev->mdev, &dev->async_ctx);
 	timer_setup(&dev->delay_timer, delay_time_func, 0);
 	for (i = 0; i < MAX_MR_CACHE_ENTRIES; i++) {
 		ent = &cache->ent[i];
@@ -725,33 +727,6 @@ int mlx5_mr_cache_init(struct mlx5_ib_dev *dev)
 	return 0;
 }
 
-static void wait_for_async_commands(struct mlx5_ib_dev *dev)
-{
-	struct mlx5_mr_cache *cache = &dev->cache;
-	struct mlx5_cache_ent *ent;
-	int total = 0;
-	int i;
-	int j;
-
-	for (i = 0; i < MAX_MR_CACHE_ENTRIES; i++) {
-		ent = &cache->ent[i];
-		for (j = 0 ; j < 1000; j++) {
-			if (!ent->pending)
-				break;
-			msleep(50);
-		}
-	}
-	for (i = 0; i < MAX_MR_CACHE_ENTRIES; i++) {
-		ent = &cache->ent[i];
-		total += ent->pending;
-	}
-
-	if (total)
-		mlx5_ib_warn(dev, "aborted while there are %d pending mr requests\n", total);
-	else
-		mlx5_ib_warn(dev, "done with all pending requests\n");
-}
-
 int mlx5_mr_cache_cleanup(struct mlx5_ib_dev *dev)
 {
 	int i;
@@ -763,12 +738,12 @@ int mlx5_mr_cache_cleanup(struct mlx5_ib_dev *dev)
 	flush_workqueue(dev->cache.wq);
 
 	mlx5_mr_cache_debugfs_cleanup(dev);
+	mlx5_cmd_cleanup_async_ctx(&dev->async_ctx);
 
 	for (i = 0; i < MAX_MR_CACHE_ENTRIES; i++)
 		clean_keys(dev, i);
 
 	destroy_workqueue(dev->cache.wq);
-	wait_for_async_commands(dev);
 	del_timer_sync(&dev->delay_timer);
 
 	return 0;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
index 3e0fa8a8077b..a25a8c6f938e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
@@ -1711,12 +1711,57 @@ int mlx5_cmd_exec(struct mlx5_core_dev *dev, void *in, int in_size, void *out,
 }
 EXPORT_SYMBOL(mlx5_cmd_exec);
 
-int mlx5_cmd_exec_cb(struct mlx5_core_dev *dev, void *in, int in_size,
-		     void *out, int out_size, mlx5_cmd_cbk_t callback,
-		     void *context)
+void mlx5_cmd_init_async_ctx(struct mlx5_core_dev *dev,
+			     struct mlx5_async_ctx *ctx)
 {
-	return cmd_exec(dev, in, in_size, out, out_size, callback, context,
-			false);
+	ctx->dev = dev;
+	/* Starts at 1 to avoid doing wake_up if we are not cleaning up */
+	atomic_set(&ctx->num_inflight, 1);
+	init_waitqueue_head(&ctx->wait);
+}
+EXPORT_SYMBOL(mlx5_cmd_init_async_ctx);
+
+/**
+ * mlx5_cmd_cleanup_async_ctx - Clean up an async_ctx
+ * @ctx: The ctx to clean
+ *
+ * Upon return all callbacks given to mlx5_cmd_exec_cb() have been called. The
+ * caller must ensure that mlx5_cmd_exec_cb() is not called during or after
+ * the call mlx5_cleanup_async_ctx().
+ */
+void mlx5_cmd_cleanup_async_ctx(struct mlx5_async_ctx *ctx)
+{
+	atomic_dec(&ctx->num_inflight);
+	wait_event(ctx->wait, atomic_read(&ctx->num_inflight) == 0);
+}
+EXPORT_SYMBOL(mlx5_cmd_cleanup_async_ctx);
+
+static void mlx5_cmd_exec_cb_handler(int status, void *_work)
+{
+	struct mlx5_async_work *work = _work;
+	struct mlx5_async_ctx *ctx = work->ctx;
+
+	work->user_callback(status, work);
+	if (atomic_dec_and_test(&ctx->num_inflight))
+		wake_up(&ctx->wait);
+}
+
+int mlx5_cmd_exec_cb(struct mlx5_async_ctx *ctx, void *in, int in_size,
+		     void *out, int out_size, mlx5_async_cbk_t callback,
+		     struct mlx5_async_work *work)
+{
+	int ret;
+
+	work->ctx = ctx;
+	work->user_callback = callback;
+	if (WARN_ON(!atomic_inc_not_zero(&ctx->num_inflight)))
+		return -EIO;
+	ret = cmd_exec(ctx->dev, in, in_size, out, out_size,
+		       mlx5_cmd_exec_cb_handler, work, false);
+	if (ret && atomic_dec_and_test(&ctx->num_inflight))
+		wake_up(&ctx->wait);
+
+	return ret;
 }
 EXPORT_SYMBOL(mlx5_cmd_exec_cb);
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/mr.c b/drivers/net/ethernet/mellanox/mlx5/core/mr.c
index 0670165afd5f..ea744d8466ea 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/mr.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/mr.c
@@ -51,9 +51,10 @@ void mlx5_cleanup_mkey_table(struct mlx5_core_dev *dev)
 
 int mlx5_core_create_mkey_cb(struct mlx5_core_dev *dev,
 			     struct mlx5_core_mkey *mkey,
-			     u32 *in, int inlen,
-			     u32 *out, int outlen,
-			     mlx5_cmd_cbk_t callback, void *context)
+			     struct mlx5_async_ctx *async_ctx, u32 *in,
+			     int inlen, u32 *out, int outlen,
+			     mlx5_async_cbk_t callback,
+			     struct mlx5_async_work *context)
 {
 	struct mlx5_mkey_table *table = &dev->priv.mkey_table;
 	u32 lout[MLX5_ST_SZ_DW(create_mkey_out)] = {0};
@@ -71,7 +72,7 @@ int mlx5_core_create_mkey_cb(struct mlx5_core_dev *dev,
 	MLX5_SET(mkc, mkc, mkey_7_0, key);
 
 	if (callback)
-		return mlx5_cmd_exec_cb(dev, in, inlen, out, outlen,
+		return mlx5_cmd_exec_cb(async_ctx, in, inlen, out, outlen,
 					callback, context);
 
 	err = mlx5_cmd_exec(dev, in, inlen, lout, sizeof(lout));
@@ -105,7 +106,7 @@ int mlx5_core_create_mkey(struct mlx5_core_dev *dev,
 			  struct mlx5_core_mkey *mkey,
 			  u32 *in, int inlen)
 {
-	return mlx5_core_create_mkey_cb(dev, mkey, in, inlen,
+	return mlx5_core_create_mkey_cb(dev, mkey, NULL, in, inlen,
 					NULL, 0, NULL, NULL);
 }
 EXPORT_SYMBOL(mlx5_core_create_mkey);
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index 4e444863054a..039c9398614c 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -850,11 +850,30 @@ void mlx5_cmd_cleanup(struct mlx5_core_dev *dev);
 void mlx5_cmd_use_events(struct mlx5_core_dev *dev);
 void mlx5_cmd_use_polling(struct mlx5_core_dev *dev);
 
+struct mlx5_async_ctx {
+	struct mlx5_core_dev *dev;
+	atomic_t num_inflight;
+	struct wait_queue_head wait;
+};
+
+struct mlx5_async_work;
+
+typedef void (*mlx5_async_cbk_t)(int status, struct mlx5_async_work *context);
+
+struct mlx5_async_work {
+	struct mlx5_async_ctx *ctx;
+	mlx5_async_cbk_t user_callback;
+};
+
+void mlx5_cmd_init_async_ctx(struct mlx5_core_dev *dev,
+			     struct mlx5_async_ctx *ctx);
+void mlx5_cmd_cleanup_async_ctx(struct mlx5_async_ctx *ctx);
+int mlx5_cmd_exec_cb(struct mlx5_async_ctx *ctx, void *in, int in_size,
+		     void *out, int out_size, mlx5_async_cbk_t callback,
+		     struct mlx5_async_work *work);
+
 int mlx5_cmd_exec(struct mlx5_core_dev *dev, void *in, int in_size, void *out,
 		  int out_size);
-int mlx5_cmd_exec_cb(struct mlx5_core_dev *dev, void *in, int in_size,
-		     void *out, int out_size, mlx5_cmd_cbk_t callback,
-		     void *context);
 int mlx5_cmd_exec_polling(struct mlx5_core_dev *dev, void *in, int in_size,
 			  void *out, int out_size);
 void mlx5_cmd_mbox_status(void *out, u8 *status, u32 *syndrome);
@@ -885,9 +904,10 @@ void mlx5_init_mkey_table(struct mlx5_core_dev *dev);
 void mlx5_cleanup_mkey_table(struct mlx5_core_dev *dev);
 int mlx5_core_create_mkey_cb(struct mlx5_core_dev *dev,
 			     struct mlx5_core_mkey *mkey,
-			     u32 *in, int inlen,
-			     u32 *out, int outlen,
-			     mlx5_cmd_cbk_t callback, void *context);
+			     struct mlx5_async_ctx *async_ctx, u32 *in,
+			     int inlen, u32 *out, int outlen,
+			     mlx5_async_cbk_t callback,
+			     struct mlx5_async_work *context);
 int mlx5_core_create_mkey(struct mlx5_core_dev *dev,
 			  struct mlx5_core_mkey *mkey,
 			  u32 *in, int inlen);
-- 
2.20.1

[PATCH mlx5-next 2/4] net/mlx5: Add pci AtomicOps request

[Saeed Mahameed]

From: Michael Guralnik <michaelgur@mellanox.com>

Calling pci_enable_atomic_ops_to_root enables AtomicOp requests to pci
root port.

AtomicOp requests will be enabled only if the completer and all
intermediate pci bridges support PCI atomic operations.
This, together with appropriate settings in the NVCONFIG should enable
PCI atomic operations on the device.

PCI atomic operations were first introduced in PCI Express Base Specification
2.1. The Supported operations are Swap (Unconditional Swap), CAS (Compare and
Swap) and FetchAdd (Fetch and Add).

Unlike other atomic operation modes PCI atomic operations gives the user
the option to do atomic operations on local memory, without involving verbs
api, without it compromising the operation's atomicity.

Signed-off-by: Michael Guralnik <michaelgur@mellanox.com>
Reviewed-by: Majd Dibbiny <majd@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index be81b319b0dc..085e1133b8d5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -693,6 +693,11 @@ static int mlx5_pci_init(struct mlx5_core_dev *dev, struct mlx5_priv *priv)
 		goto err_clr_master;
 	}
 
+	if (pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP32) &&
+	    pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP64) &&
+	    pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP128))
+		mlx5_core_dbg(dev, "Enabling pci atomics failed\n");
+
 	dev->iseg_base = pci_resource_start(dev->pdev, 0);
 	dev->iseg = ioremap(dev->iseg_base, sizeof(*dev->iseg));
 	if (!dev->iseg) {
-- 
2.20.1

[PATCH mlx5-next 3/4] net/mlx5: Remove unused MLX5_*_DOORBELL_LOCK macros

[Saeed Mahameed]

From: Maxim Mikityanskiy <maximmi@mellanox.com>

MLX5_*_DOORBELL_LOCK macros provided a way to avoid locking for
mlx5_write64 on 64-bit platforms where it's not necessary. Currently all
calls to mlx5_write64 don't use a spinlock, so the macros became unused.

Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 include/linux/mlx5/doorbell.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/include/linux/mlx5/doorbell.h b/include/linux/mlx5/doorbell.h
index 0787de28f2fc..9ef3f9d00154 100644
--- a/include/linux/mlx5/doorbell.h
+++ b/include/linux/mlx5/doorbell.h
@@ -42,10 +42,6 @@
  * PCI so we won't worry about it.
  */
 
-#define MLX5_DECLARE_DOORBELL_LOCK(name)
-#define MLX5_INIT_DOORBELL_LOCK(ptr)    do { } while (0)
-#define MLX5_GET_DOORBELL_LOCK(ptr)      (NULL)
-
 static inline void mlx5_write64(__be32 val[2], void __iomem *dest,
 				spinlock_t *doorbell_lock)
 {
@@ -59,10 +55,6 @@ static inline void mlx5_write64(__be32 val[2], void __iomem *dest,
  * MMIO writes.
  */
 
-#define MLX5_DECLARE_DOORBELL_LOCK(name) spinlock_t name;
-#define MLX5_INIT_DOORBELL_LOCK(ptr)     spin_lock_init(ptr)
-#define MLX5_GET_DOORBELL_LOCK(ptr)      (ptr)
-
 static inline void mlx5_write64(__be32 val[2], void __iomem *dest,
 				spinlock_t *doorbell_lock)
 {
-- 
2.20.1

[PATCH mlx5-next 4/4] net/mlx5: Remove spinlock support from mlx5_write64

[Saeed Mahameed]

From: Maxim Mikityanskiy <maximmi@mellanox.com>

As there is no user of mlx5_write64 that passes a spinlock to
mlx5_write64, remove this functionality and simplify the function.

Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/infiniband/hw/mlx5/qp.c               |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en.h  |  2 +-
 .../ethernet/mellanox/mlx5/core/fpga/conn.c   |  2 +-
 include/linux/mlx5/cq.h                       |  2 +-
 include/linux/mlx5/doorbell.h                 | 28 ++++---------------
 5 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index dd2ae640bc84..816c34ee91cf 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -5015,7 +5015,7 @@ static int _mlx5_ib_post_send(struct ib_qp *ibqp, const struct ib_send_wr *wr,
 		wmb();
 
 		/* currently we support only regular doorbells */
-		mlx5_write64((__be32 *)ctrl, bf->bfreg->map + bf->offset, NULL);
+		mlx5_write64((__be32 *)ctrl, bf->bfreg->map + bf->offset);
 		/* Make sure doorbells don't leak out of SQ spinlock
 		 * and reach the HCA out of order.
 		 */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 8fa8fdd30b85..2623d3fb6b96 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -916,7 +916,7 @@ void mlx5e_notify_hw(struct mlx5_wq_cyc *wq, u16 pc,
 	 */
 	wmb();
 
-	mlx5_write64((__be32 *)ctrl, uar_map, NULL);
+	mlx5_write64((__be32 *)ctrl, uar_map);
 }
 
 static inline void mlx5e_cq_arm(struct mlx5e_cq *cq)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c
index 873541ef4c1b..ca2296a2f9ee 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c
@@ -135,7 +135,7 @@ static void mlx5_fpga_conn_notify_hw(struct mlx5_fpga_conn *conn, void *wqe)
 	*conn->qp.wq.sq.db = cpu_to_be32(conn->qp.sq.pc);
 	/* Make sure that doorbell record is visible before ringing */
 	wmb();
-	mlx5_write64(wqe, conn->fdev->conn_res.uar->map + MLX5_BF_OFFSET, NULL);
+	mlx5_write64(wqe, conn->fdev->conn_res.uar->map + MLX5_BF_OFFSET);
 }
 
 static void mlx5_fpga_conn_post_send(struct mlx5_fpga_conn *conn,
diff --git a/include/linux/mlx5/cq.h b/include/linux/mlx5/cq.h
index 612c8c2f2466..769326ea1d9b 100644
--- a/include/linux/mlx5/cq.h
+++ b/include/linux/mlx5/cq.h
@@ -170,7 +170,7 @@ static inline void mlx5_cq_arm(struct mlx5_core_cq *cq, u32 cmd,
 	doorbell[0] = cpu_to_be32(sn << 28 | cmd | ci);
 	doorbell[1] = cpu_to_be32(cq->cqn);
 
-	mlx5_write64(doorbell, uar_page + MLX5_CQ_DOORBELL, NULL);
+	mlx5_write64(doorbell, uar_page + MLX5_CQ_DOORBELL);
 }
 
 static inline void mlx5_cq_hold(struct mlx5_core_cq *cq)
diff --git a/include/linux/mlx5/doorbell.h b/include/linux/mlx5/doorbell.h
index 9ef3f9d00154..c12523cc8102 100644
--- a/include/linux/mlx5/doorbell.h
+++ b/include/linux/mlx5/doorbell.h
@@ -36,38 +36,20 @@
 #define MLX5_BF_OFFSET	      0x800
 #define MLX5_CQ_DOORBELL      0x20
 
-#if BITS_PER_LONG == 64
 /* Assume that we can just write a 64-bit doorbell atomically.  s390
  * actually doesn't have writeq() but S/390 systems don't even have
- * PCI so we won't worry about it.
+ * PCI so we won't worry about it. Note that the write is not atomic
+ * on 32-bit systems.
  */
 
-static inline void mlx5_write64(__be32 val[2], void __iomem *dest,
-				spinlock_t *doorbell_lock)
+static inline void mlx5_write64(__be32 val[2], void __iomem *dest)
 {
+#if BITS_PER_LONG == 64
 	__raw_writeq(*(u64 *)val, dest);
-}
-
 #else
-
-/* Just fall back to a spinlock to protect the doorbell if
- * BITS_PER_LONG is 32 -- there's no portable way to do atomic 64-bit
- * MMIO writes.
- */
-
-static inline void mlx5_write64(__be32 val[2], void __iomem *dest,
-				spinlock_t *doorbell_lock)
-{
-	unsigned long flags;
-
-	if (doorbell_lock)
-		spin_lock_irqsave(doorbell_lock, flags);
 	__raw_writel((__force u32) val[0], dest);
 	__raw_writel((__force u32) val[1], dest + 4);
-	if (doorbell_lock)
-		spin_unlock_irqrestore(doorbell_lock, flags);
-}
-
 #endif
+}
 
 #endif /* MLX5_DOORBELL_H */
-- 
2.20.1

Re: [PATCH mlx5-next 4/4] net/mlx5: Remove spinlock support from mlx5_write64

[Leon Romanovsky]

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

On Fri, Jan 18, 2019 at 04:33:13PM -0800, Saeed Mahameed wrote:
> From: Maxim Mikityanskiy <maximmi@mellanox.com>
>
> As there is no user of mlx5_write64 that passes a spinlock to
> mlx5_write64, remove this functionality and simplify the function.
>
> Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
> Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
>  drivers/infiniband/hw/mlx5/qp.c               |  2 +-
>  drivers/net/ethernet/mellanox/mlx5/core/en.h  |  2 +-
>  .../ethernet/mellanox/mlx5/core/fpga/conn.c   |  2 +-
>  include/linux/mlx5/cq.h                       |  2 +-
>  include/linux/mlx5/doorbell.h                 | 28 ++++---------------
>  5 files changed, 9 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
> index dd2ae640bc84..816c34ee91cf 100644
> --- a/drivers/infiniband/hw/mlx5/qp.c
> +++ b/drivers/infiniband/hw/mlx5/qp.c
> @@ -5015,7 +5015,7 @@ static int _mlx5_ib_post_send(struct ib_qp *ibqp, const struct ib_send_wr *wr,
>  		wmb();
>
>  		/* currently we support only regular doorbells */
> -		mlx5_write64((__be32 *)ctrl, bf->bfreg->map + bf->offset, NULL);
> +		mlx5_write64((__be32 *)ctrl, bf->bfreg->map + bf->offset);
>  		/* Make sure doorbells don't leak out of SQ spinlock
>  		 * and reach the HCA out of order.
>  		 */
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index 8fa8fdd30b85..2623d3fb6b96 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -916,7 +916,7 @@ void mlx5e_notify_hw(struct mlx5_wq_cyc *wq, u16 pc,
>  	 */
>  	wmb();
>
> -	mlx5_write64((__be32 *)ctrl, uar_map, NULL);
> +	mlx5_write64((__be32 *)ctrl, uar_map);
>  }
>
>  static inline void mlx5e_cq_arm(struct mlx5e_cq *cq)
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c
> index 873541ef4c1b..ca2296a2f9ee 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c
> @@ -135,7 +135,7 @@ static void mlx5_fpga_conn_notify_hw(struct mlx5_fpga_conn *conn, void *wqe)
>  	*conn->qp.wq.sq.db = cpu_to_be32(conn->qp.sq.pc);
>  	/* Make sure that doorbell record is visible before ringing */
>  	wmb();
> -	mlx5_write64(wqe, conn->fdev->conn_res.uar->map + MLX5_BF_OFFSET, NULL);
> +	mlx5_write64(wqe, conn->fdev->conn_res.uar->map + MLX5_BF_OFFSET);
>  }
>
>  static void mlx5_fpga_conn_post_send(struct mlx5_fpga_conn *conn,
> diff --git a/include/linux/mlx5/cq.h b/include/linux/mlx5/cq.h
> index 612c8c2f2466..769326ea1d9b 100644
> --- a/include/linux/mlx5/cq.h
> +++ b/include/linux/mlx5/cq.h
> @@ -170,7 +170,7 @@ static inline void mlx5_cq_arm(struct mlx5_core_cq *cq, u32 cmd,
>  	doorbell[0] = cpu_to_be32(sn << 28 | cmd | ci);
>  	doorbell[1] = cpu_to_be32(cq->cqn);
>
> -	mlx5_write64(doorbell, uar_page + MLX5_CQ_DOORBELL, NULL);
> +	mlx5_write64(doorbell, uar_page + MLX5_CQ_DOORBELL);
>  }
>
>  static inline void mlx5_cq_hold(struct mlx5_core_cq *cq)
> diff --git a/include/linux/mlx5/doorbell.h b/include/linux/mlx5/doorbell.h
> index 9ef3f9d00154..c12523cc8102 100644
> --- a/include/linux/mlx5/doorbell.h
> +++ b/include/linux/mlx5/doorbell.h
> @@ -36,38 +36,20 @@
>  #define MLX5_BF_OFFSET	      0x800
>  #define MLX5_CQ_DOORBELL      0x20
>
> -#if BITS_PER_LONG == 64
>  /* Assume that we can just write a 64-bit doorbell atomically.  s390
>   * actually doesn't have writeq() but S/390 systems don't even have
> - * PCI so we won't worry about it.
> + * PCI so we won't worry about it. Note that the write is not atomic
> + * on 32-bit systems.
>   */
>
> -static inline void mlx5_write64(__be32 val[2], void __iomem *dest,
> -				spinlock_t *doorbell_lock)
> +static inline void mlx5_write64(__be32 val[2], void __iomem *dest)
>  {
> +#if BITS_PER_LONG == 64
>  	__raw_writeq(*(u64 *)val, dest);
> -}
> -
>  #else
> -
> -/* Just fall back to a spinlock to protect the doorbell if
> - * BITS_PER_LONG is 32 -- there's no portable way to do atomic 64-bit
> - * MMIO writes.
> - */
> -
> -static inline void mlx5_write64(__be32 val[2], void __iomem *dest,
> -				spinlock_t *doorbell_lock)
> -{
> -	unsigned long flags;
> -
> -	if (doorbell_lock)
> -		spin_lock_irqsave(doorbell_lock, flags);

Saeed, Maxim

It is incomplete change. In your patch 3, you removed declaration
of spinlock from 32bits compiles and made this write completely
unsafe.

You need to do one of two things:
1. Require CONFIG_64BIT and delete this 32bit code.
2. Declare global mlx5 DB spinlock and use on 32bit systems, something
like this:
#if BITS_PER_LONG == 64
 __raw_writeq(*(u64 *)val, dest);
#else
  spin_lock_irqsave(doorbell_lock, flags);
  __raw_writel((__force u32) val[0], dest);
  __raw_writel((__force u32) val[1], dest + 4);
   spin_unlock_irqrestore(doorbell_lock, flags);
#endif

Thanks

>  	__raw_writel((__force u32) val[0], dest);
>  	__raw_writel((__force u32) val[1], dest + 4);
> -	if (doorbell_lock)
> -		spin_unlock_irqrestore(doorbell_lock, flags);
> -}
> -
>  #endif
> +}
>
>  #endif /* MLX5_DOORBELL_H */
> --
> 2.20.1
>

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

Re: [PATCH mlx5-next 4/4] net/mlx5: Remove spinlock support from mlx5_write64

[Jason Gunthorpe]

On Sat, Jan 19, 2019 at 12:43:14AM -0700, Leon Romanovsky wrote:
> You need to do one of two things:
> 1. Require CONFIG_64BIT and delete this 32bit code.
> 2. Declare global mlx5 DB spinlock and use on 32bit systems, something
> like this:
> #if BITS_PER_LONG == 64
>  __raw_writeq(*(u64 *)val, dest);
> #else
>   spin_lock_irqsave(doorbell_lock, flags);
>   __raw_writel((__force u32) val[0], dest);
>   __raw_writel((__force u32) val[1], dest + 4);
>    spin_unlock_irqrestore(doorbell_lock, flags);
> #endif

And why is this code using the __raw_ versions? Seems wrong too...

Jason

Re: [PATCH mlx5-next 4/4] net/mlx5: Remove spinlock support from mlx5_write64

[Saeed Mahameed]

On Fri, Jan 18, 2019 at 11:43 PM Leon Romanovsky <leonro@mellanox.com> wrote:
>
> On Fri, Jan 18, 2019 at 04:33:13PM -0800, Saeed Mahameed wrote:
> > From: Maxim Mikityanskiy <maximmi@mellanox.com>
> >
> > As there is no user of mlx5_write64 that passes a spinlock to
> > mlx5_write64, remove this functionality and simplify the function.
> >
> > Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
> > Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > ---
> >  drivers/infiniband/hw/mlx5/qp.c               |  2 +-
> >  drivers/net/ethernet/mellanox/mlx5/core/en.h  |  2 +-
> >  .../ethernet/mellanox/mlx5/core/fpga/conn.c   |  2 +-
> >  include/linux/mlx5/cq.h                       |  2 +-
> >  include/linux/mlx5/doorbell.h                 | 28 ++++---------------
> >  5 files changed, 9 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
> > index dd2ae640bc84..816c34ee91cf 100644
> > --- a/drivers/infiniband/hw/mlx5/qp.c
> > +++ b/drivers/infiniband/hw/mlx5/qp.c
> > @@ -5015,7 +5015,7 @@ static int _mlx5_ib_post_send(struct ib_qp *ibqp, const struct ib_send_wr *wr,
> >               wmb();
> >
> >               /* currently we support only regular doorbells */
> > -             mlx5_write64((__be32 *)ctrl, bf->bfreg->map + bf->offset, NULL);
> > +             mlx5_write64((__be32 *)ctrl, bf->bfreg->map + bf->offset);
> >               /* Make sure doorbells don't leak out of SQ spinlock
> >                * and reach the HCA out of order.
> >                */
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> > index 8fa8fdd30b85..2623d3fb6b96 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> > @@ -916,7 +916,7 @@ void mlx5e_notify_hw(struct mlx5_wq_cyc *wq, u16 pc,
> >        */
> >       wmb();
> >
> > -     mlx5_write64((__be32 *)ctrl, uar_map, NULL);
> > +     mlx5_write64((__be32 *)ctrl, uar_map);
> >  }
> >
> >  static inline void mlx5e_cq_arm(struct mlx5e_cq *cq)
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c
> > index 873541ef4c1b..ca2296a2f9ee 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c
> > @@ -135,7 +135,7 @@ static void mlx5_fpga_conn_notify_hw(struct mlx5_fpga_conn *conn, void *wqe)
> >       *conn->qp.wq.sq.db = cpu_to_be32(conn->qp.sq.pc);
> >       /* Make sure that doorbell record is visible before ringing */
> >       wmb();
> > -     mlx5_write64(wqe, conn->fdev->conn_res.uar->map + MLX5_BF_OFFSET, NULL);
> > +     mlx5_write64(wqe, conn->fdev->conn_res.uar->map + MLX5_BF_OFFSET);
> >  }
> >
> >  static void mlx5_fpga_conn_post_send(struct mlx5_fpga_conn *conn,
> > diff --git a/include/linux/mlx5/cq.h b/include/linux/mlx5/cq.h
> > index 612c8c2f2466..769326ea1d9b 100644
> > --- a/include/linux/mlx5/cq.h
> > +++ b/include/linux/mlx5/cq.h
> > @@ -170,7 +170,7 @@ static inline void mlx5_cq_arm(struct mlx5_core_cq *cq, u32 cmd,
> >       doorbell[0] = cpu_to_be32(sn << 28 | cmd | ci);
> >       doorbell[1] = cpu_to_be32(cq->cqn);
> >
> > -     mlx5_write64(doorbell, uar_page + MLX5_CQ_DOORBELL, NULL);
> > +     mlx5_write64(doorbell, uar_page + MLX5_CQ_DOORBELL);
> >  }
> >
> >  static inline void mlx5_cq_hold(struct mlx5_core_cq *cq)
> > diff --git a/include/linux/mlx5/doorbell.h b/include/linux/mlx5/doorbell.h
> > index 9ef3f9d00154..c12523cc8102 100644
> > --- a/include/linux/mlx5/doorbell.h
> > +++ b/include/linux/mlx5/doorbell.h
> > @@ -36,38 +36,20 @@
> >  #define MLX5_BF_OFFSET             0x800
> >  #define MLX5_CQ_DOORBELL      0x20
> >
> > -#if BITS_PER_LONG == 64
> >  /* Assume that we can just write a 64-bit doorbell atomically.  s390
> >   * actually doesn't have writeq() but S/390 systems don't even have
> > - * PCI so we won't worry about it.
> > + * PCI so we won't worry about it. Note that the write is not atomic
> > + * on 32-bit systems.
> >   */
> >
> > -static inline void mlx5_write64(__be32 val[2], void __iomem *dest,
> > -                             spinlock_t *doorbell_lock)
> > +static inline void mlx5_write64(__be32 val[2], void __iomem *dest)
> >  {
> > +#if BITS_PER_LONG == 64
> >       __raw_writeq(*(u64 *)val, dest);
> > -}
> > -
> >  #else
> > -
> > -/* Just fall back to a spinlock to protect the doorbell if
> > - * BITS_PER_LONG is 32 -- there's no portable way to do atomic 64-bit
> > - * MMIO writes.
> > - */
> > -
> > -static inline void mlx5_write64(__be32 val[2], void __iomem *dest,
> > -                             spinlock_t *doorbell_lock)
> > -{
> > -     unsigned long flags;
> > -
> > -     if (doorbell_lock)
> > -             spin_lock_irqsave(doorbell_lock, flags);
>
> Saeed, Maxim
>
> It is incomplete change. In your patch 3, you removed declaration
> of spinlock from 32bits compiles and made this write completely
> unsafe.
>
> You need to do one of two things:
> 1. Require CONFIG_64BIT and delete this 32bit code.

Not required, if something did compile for 32bit, why stop it now ?
it compiles today and it doesn't function, there is a big warning in
the code and no one is using a spinlock in 32bit arch,
so this is just an API cleanup for unused code path.

> 2. Declare global mlx5 DB spinlock and use on 32bit systems, something
> like this:
> #if BITS_PER_LONG == 64
>  __raw_writeq(*(u64 *)val, dest);
> #else
>   spin_lock_irqsave(doorbell_lock, flags);
>   __raw_writel((__force u32) val[0], dest);
>   __raw_writel((__force u32) val[1], dest + 4);
>    spin_unlock_irqrestore(doorbell_lock, flags);
> #endif
>

I would rather keep it broken than adding some global spinlock.

Every time we touch this code someone gets sensitive about it, this is
Just a cleanup patch !!
Bottom line, 32bit doesn't work, we are cleaning up API, don't make a
big deal out of it,
Please don't mix two different things, 32bit support is not related to
this patch at all.
if you want to fix 32bit, you can do it on top of this patch.

> Thanks
>
> >       __raw_writel((__force u32) val[0], dest);
> >       __raw_writel((__force u32) val[1], dest + 4);
> > -     if (doorbell_lock)
> > -             spin_unlock_irqrestore(doorbell_lock, flags);
> > -}
> > -
> >  #endif
> > +}
> >
> >  #endif /* MLX5_DOORBELL_H */
> > --
> > 2.20.1
> >

Re: [PATCH mlx5-next 4/4] net/mlx5: Remove spinlock support from mlx5_write64

[Saeed Mahameed]

On Mon, Jan 21, 2019 at 8:46 AM Jason Gunthorpe <jgg@mellanox.com> wrote:
>
> On Sat, Jan 19, 2019 at 12:43:14AM -0700, Leon Romanovsky wrote:
> > You need to do one of two things:
> > 1. Require CONFIG_64BIT and delete this 32bit code.
> > 2. Declare global mlx5 DB spinlock and use on 32bit systems, something
> > like this:
> > #if BITS_PER_LONG == 64
> >  __raw_writeq(*(u64 *)val, dest);
> > #else
> >   spin_lock_irqsave(doorbell_lock, flags);
> >   __raw_writel((__force u32) val[0], dest);
> >   __raw_writel((__force u32) val[1], dest + 4);
> >    spin_unlock_irqrestore(doorbell_lock, flags);
> > #endif
>
> And why is this code using the __raw_ versions? Seems wrong too...
>

for 64 and 32 as well?
what is wrong with the raw version ?

> Jason

Re: [PATCH mlx5-next 4/4] net/mlx5: Remove spinlock support from mlx5_write64

[Jason Gunthorpe]

On Mon, Jan 21, 2019 at 10:12:58AM -0800, Saeed Mahameed wrote:
> On Mon, Jan 21, 2019 at 8:46 AM Jason Gunthorpe <jgg@mellanox.com> wrote:
> >
> > On Sat, Jan 19, 2019 at 12:43:14AM -0700, Leon Romanovsky wrote:
> > > You need to do one of two things:
> > > 1. Require CONFIG_64BIT and delete this 32bit code.
> > > 2. Declare global mlx5 DB spinlock and use on 32bit systems, something
> > > like this:
> > > #if BITS_PER_LONG == 64
> > >  __raw_writeq(*(u64 *)val, dest);
> > > #else
> > >   spin_lock_irqsave(doorbell_lock, flags);
> > >   __raw_writel((__force u32) val[0], dest);
> > >   __raw_writel((__force u32) val[1], dest + 4);
> > >    spin_unlock_irqrestore(doorbell_lock, flags);
> > > #endif
> >
> > And why is this code using the __raw_ versions? Seems wrong too...
> >
> 
> for 64 and 32 as well?

yes

> what is wrong with the raw version ?

It should only be used by arch code (or in drivers linked to a
specific arch). The actual properties of the 'raw' version are arch
specific and make it hard to know if the driver will work on different
archs. ie some arches may not byte swap their raw accessors, or may
omit barriers.

Most likely this just wants to be writeq for 64 bit and
writel_relaxed() & writel() for 32 bit - unless there was some reason
to have used __raw versions in the first place (in which case a
comment is missing).

Jason

Re: [PATCH mlx5-next 0/4] mlx5 next misc updates

[Leon Romanovsky]

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

On Fri, Jan 18, 2019 at 04:33:09PM -0800, Saeed Mahameed wrote:
> Hi all,
>
> This series includes updates to mlx5-next shared branch.
>
> 1) from Jason, improve mlx5_cmd_exec_cb async API to be safer
> 2) from Maxim Mikityanskiy, cleanups for mlx5_write64 doorbell API
> 3) from Michael Guralnik, Add pci AtomicOps request
>
> Thanks,
> Saeed.
>
> ---
>
> Jason Gunthorpe (1):
>   net/mlx5: Make mlx5_cmd_exec_cb() a safe API
>
> Michael Guralnik (1):
>   net/mlx5: Add pci AtomicOps request

Those two were applied to mlx5-next branch.

ce4eee5340a9 (mlx5-next) net/mlx5: Add pci AtomicOps request
e355477ed9e4 net/mlx5: Make mlx5_cmd_exec_cb() a safe API

> Maxim Mikityanskiy (2):
>   net/mlx5: Remove unused MLX5_*_DOORBELL_LOCK macros
>   net/mlx5: Remove spinlock support from mlx5_write64

Those two needs extra work,

Thanks

>
> --
> 2.20.1
>

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

Re: [PATCH mlx5-next 0/4] mlx5 next misc updates

[Saeed Mahameed]

On Thu, Jan 24, 2019 at 4:30 AM Leon Romanovsky <leonro@mellanox.com> wrote:
>
> On Fri, Jan 18, 2019 at 04:33:09PM -0800, Saeed Mahameed wrote:
> > Hi all,
> >
> > This series includes updates to mlx5-next shared branch.
> >
> > 1) from Jason, improve mlx5_cmd_exec_cb async API to be safer
> > 2) from Maxim Mikityanskiy, cleanups for mlx5_write64 doorbell API
> > 3) from Michael Guralnik, Add pci AtomicOps request
> >
> > Thanks,
> > Saeed.
> >
> > ---
> >
> > Jason Gunthorpe (1):
> >   net/mlx5: Make mlx5_cmd_exec_cb() a safe API
> >
> > Michael Guralnik (1):
> >   net/mlx5: Add pci AtomicOps request
>
> Those two were applied to mlx5-next branch.
>
> ce4eee5340a9 (mlx5-next) net/mlx5: Add pci AtomicOps request
> e355477ed9e4 net/mlx5: Make mlx5_cmd_exec_cb() a safe API
>
> > Maxim Mikityanskiy (2):
> >   net/mlx5: Remove unused MLX5_*_DOORBELL_LOCK macros
> >   net/mlx5: Remove spinlock support from mlx5_write64
>
> Those two needs extra work,

What extra work ?

>
> Thanks
>
> >
> > --
> > 2.20.1
> >

Re: [PATCH mlx5-next 4/4] net/mlx5: Remove spinlock support from mlx5_write64

[Saeed Mahameed]

On Mon, 2019-01-21 at 18:22 +0000, Jason Gunthorpe wrote:
> On Mon, Jan 21, 2019 at 10:12:58AM -0800, Saeed Mahameed wrote:
> > On Mon, Jan 21, 2019 at 8:46 AM Jason Gunthorpe <jgg@mellanox.com>
> > wrote:
> > > On Sat, Jan 19, 2019 at 12:43:14AM -0700, Leon Romanovsky wrote:
> > > > You need to do one of two things:
> > > > 1. Require CONFIG_64BIT and delete this 32bit code.
> > > > 2. Declare global mlx5 DB spinlock and use on 32bit systems,
> > > > something
> > > > like this:
> > > > #if BITS_PER_LONG == 64
> > > >  __raw_writeq(*(u64 *)val, dest);
> > > > #else
> > > >   spin_lock_irqsave(doorbell_lock, flags);
> > > >   __raw_writel((__force u32) val[0], dest);
> > > >   __raw_writel((__force u32) val[1], dest + 4);
> > > >    spin_unlock_irqrestore(doorbell_lock, flags);
> > > > #endif
> > > 
> > > And why is this code using the __raw_ versions? Seems wrong
> > > too...
> > > 
> > 
> > for 64 and 32 as well?
> 
> yes
> 
> > what is wrong with the raw version ?
> 
> It should only be used by arch code (or in drivers linked to a
> specific arch). The actual properties of the 'raw' version are arch
> specific and make it hard to know if the driver will work on
> different
> archs. ie some arches may not byte swap their raw accessors, or may
> omit barriers.
> 
> Most likely this just wants to be writeq for 64 bit and
> writel_relaxed() & writel() for 32 bit - unless there was some reason
> to have used __raw versions in the first place (in which case a
> comment is missing).

Ok, after some internal discussion it seems that
{read,write}{b,w,l,q}_relaxed() can be used instead of the __raw API
currently used in the driver, Adding as a future task.

This has nothing to do the the current cleanup patch.

Thanks,
Saeed.

> 
> Jason

Re: [PATCH mlx5-next 0/4] mlx5 next misc updates

[Leon Romanovsky]

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

On Fri, Jan 25, 2019 at 10:08:00AM -0800, Saeed Mahameed wrote:
> On Thu, Jan 24, 2019 at 4:30 AM Leon Romanovsky <leonro@mellanox.com> wrote:
> >
> > On Fri, Jan 18, 2019 at 04:33:09PM -0800, Saeed Mahameed wrote:
> > > Hi all,
> > >
> > > This series includes updates to mlx5-next shared branch.
> > >
> > > 1) from Jason, improve mlx5_cmd_exec_cb async API to be safer
> > > 2) from Maxim Mikityanskiy, cleanups for mlx5_write64 doorbell API
> > > 3) from Michael Guralnik, Add pci AtomicOps request
> > >
> > > Thanks,
> > > Saeed.
> > >
> > > ---
> > >
> > > Jason Gunthorpe (1):
> > >   net/mlx5: Make mlx5_cmd_exec_cb() a safe API
> > >
> > > Michael Guralnik (1):
> > >   net/mlx5: Add pci AtomicOps request
> >
> > Those two were applied to mlx5-next branch.
> >
> > ce4eee5340a9 (mlx5-next) net/mlx5: Add pci AtomicOps request
> > e355477ed9e4 net/mlx5: Make mlx5_cmd_exec_cb() a safe API
> >
> > > Maxim Mikityanskiy (2):
> > >   net/mlx5: Remove unused MLX5_*_DOORBELL_LOCK macros
> > >   net/mlx5: Remove spinlock support from mlx5_write64
> >
> > Those two needs extra work,
>
> What extra work ?

You got two comments for area you are touching:
1. Replace _rww writes to something else.
2. Protect with spinlock 32-bits writes instead of ignoring it.

Both of those changes will touch the same 2-4 lines and there
is very little benefit in creating more than one-two patches
just for that.

Thanks

>
> >
> > Thanks
> >
> > >
> > > --
> > > 2.20.1
> > >

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

Re: [PATCH mlx5-next 0/4] mlx5 next misc updates

[Saeed Mahameed]

On Sun, 2019-01-27 at 07:51 +0000, Leon Romanovsky wrote:
> On Fri, Jan 25, 2019 at 10:08:00AM -0800, Saeed Mahameed wrote:
> > On Thu, Jan 24, 2019 at 4:30 AM Leon Romanovsky <
> > leonro@mellanox.com> wrote:
> > > On Fri, Jan 18, 2019 at 04:33:09PM -0800, Saeed Mahameed wrote:
> > > > Hi all,
> > > > 
> > > > This series includes updates to mlx5-next shared branch.
> > > > 
> > > > 1) from Jason, improve mlx5_cmd_exec_cb async API to be safer
> > > > 2) from Maxim Mikityanskiy, cleanups for mlx5_write64 doorbell
> > > > API
> > > > 3) from Michael Guralnik, Add pci AtomicOps request
> > > > 
> > > > Thanks,
> > > > Saeed.
> > > > 
> > > > ---
> > > > 
> > > > Jason Gunthorpe (1):
> > > >   net/mlx5: Make mlx5_cmd_exec_cb() a safe API
> > > > 
> > > > Michael Guralnik (1):
> > > >   net/mlx5: Add pci AtomicOps request
> > > 
> > > Those two were applied to mlx5-next branch.
> > > 
> > > ce4eee5340a9 (mlx5-next) net/mlx5: Add pci AtomicOps request
> > > e355477ed9e4 net/mlx5: Make mlx5_cmd_exec_cb() a safe API
> > > 
> > > > Maxim Mikityanskiy (2):
> > > >   net/mlx5: Remove unused MLX5_*_DOORBELL_LOCK macros
> > > >   net/mlx5: Remove spinlock support from mlx5_write64
> > > 
> > > Those two needs extra work,
> > 
> > What extra work ?
> 
> You got two comments for area you are touching:
> 1. Replace _rww writes to something else.

Not related to this cleanup patchset.

> 2. Protect with spinlock 32-bits writes instead of ignoring it.

Same as above, I already explained this.

> 
> Both of those changes will touch the same 2-4 lines and there
> is very little benefit in creating more than one-two patches
> just for that.
> 

Future work, as it needs verification and careful testing.

Leon I would like to move on with those 2 small cleanup patches, no
functionality change here, please confirm you are ok with them.

Thanks,
Saeed.

> Thanks
> 
> > > Thanks
> > > 
> > > > --
> > > > 2.20.1
> > > > 

Re: [PATCH mlx5-next 0/4] mlx5 next misc updates

[Leon Romanovsky]

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

On Mon, Jan 28, 2019 at 07:11:01PM +0000, Saeed Mahameed wrote:
> On Sun, 2019-01-27 at 07:51 +0000, Leon Romanovsky wrote:
> > On Fri, Jan 25, 2019 at 10:08:00AM -0800, Saeed Mahameed wrote:
> > > On Thu, Jan 24, 2019 at 4:30 AM Leon Romanovsky <
> > > leonro@mellanox.com> wrote:
> > > > On Fri, Jan 18, 2019 at 04:33:09PM -0800, Saeed Mahameed wrote:
> > > > > Hi all,
> > > > >
> > > > > This series includes updates to mlx5-next shared branch.
> > > > >
> > > > > 1) from Jason, improve mlx5_cmd_exec_cb async API to be safer
> > > > > 2) from Maxim Mikityanskiy, cleanups for mlx5_write64 doorbell
> > > > > API
> > > > > 3) from Michael Guralnik, Add pci AtomicOps request
> > > > >
> > > > > Thanks,
> > > > > Saeed.
> > > > >
> > > > > ---
> > > > >
> > > > > Jason Gunthorpe (1):
> > > > >   net/mlx5: Make mlx5_cmd_exec_cb() a safe API
> > > > >
> > > > > Michael Guralnik (1):
> > > > >   net/mlx5: Add pci AtomicOps request
> > > >
> > > > Those two were applied to mlx5-next branch.
> > > >
> > > > ce4eee5340a9 (mlx5-next) net/mlx5: Add pci AtomicOps request
> > > > e355477ed9e4 net/mlx5: Make mlx5_cmd_exec_cb() a safe API
> > > >
> > > > > Maxim Mikityanskiy (2):
> > > > >   net/mlx5: Remove unused MLX5_*_DOORBELL_LOCK macros
> > > > >   net/mlx5: Remove spinlock support from mlx5_write64
> > > >
> > > > Those two needs extra work,
> > >
> > > What extra work ?
> >
> > You got two comments for area you are touching:
> > 1. Replace _rww writes to something else.
>
> Not related to this cleanup patchset.
>
> > 2. Protect with spinlock 32-bits writes instead of ignoring it.
>
> Same as above, I already explained this.
>
> >
> > Both of those changes will touch the same 2-4 lines and there
> > is very little benefit in creating more than one-two patches
> > just for that.
> >
>
> Future work, as it needs verification and careful testing.
>
> Leon I would like to move on with those 2 small cleanup patches, no
> functionality change here, please confirm you are ok with them.

At least write large and scary comment that this mode was always broken.

>
> Thanks,
> Saeed.
>
> > Thanks
> >
> > > > Thanks
> > > >
> > > > > --
> > > > > 2.20.1
> > > > >

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