mlxsw: Preparations for improving performance

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 72 +++++++++++------------
 1 file changed, 36 insertions(+), 36 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +36 ~~-36~~

index f42a1b1c9368..93569b14b357 100644
@@ -765,42 +765,6 @@ static u8 mlxsw_pci_cq_elem_size(const struct mlxsw_pci_queue *q)
 					       MLXSW_PCI_CQE01_SIZE;
 }
 
-static int mlxsw_pci_eq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
-			     struct mlxsw_pci_queue *q)
-{
-	int i;
-	int err;
-
-	q->consumer_counter = 0;
-
-	for (i = 0; i < q->count; i++) {
-		char *elem = mlxsw_pci_queue_elem_get(q, i);
-
-		mlxsw_pci_eqe_owner_set(elem, 1);
-	}
-
-	mlxsw_cmd_mbox_sw2hw_eq_int_msix_set(mbox, 1); /* MSI-X used */
-	mlxsw_cmd_mbox_sw2hw_eq_st_set(mbox, 1); /* armed */
-	mlxsw_cmd_mbox_sw2hw_eq_log_eq_size_set(mbox, ilog2(q->count));
-	for (i = 0; i < MLXSW_PCI_AQ_PAGES; i++) {
-		dma_addr_t mapaddr = __mlxsw_pci_queue_page_get(q, i);
-
-		mlxsw_cmd_mbox_sw2hw_eq_pa_set(mbox, i, mapaddr);
-	}
-	err = mlxsw_cmd_sw2hw_eq(mlxsw_pci->core, mbox, q->num);
-	if (err)
-		return err;
-	mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
-	mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
-	return 0;
-}
-
-static void mlxsw_pci_eq_fini(struct mlxsw_pci *mlxsw_pci,
-			      struct mlxsw_pci_queue *q)
-{
-	mlxsw_cmd_hw2sw_eq(mlxsw_pci->core, q->num);
-}
-
 static void mlxsw_pci_eq_cmd_event(struct mlxsw_pci *mlxsw_pci, char *eqe)
 {
 	mlxsw_pci->cmd.comp.status = mlxsw_pci_eqe_cmd_status_get(eqe);

@@ -877,6 +841,42 @@ static void mlxsw_pci_eq_tasklet(struct tasklet_struct *t)
 	}
 }
 
+static int mlxsw_pci_eq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
+			     struct mlxsw_pci_queue *q)
+{
+	int i;
+	int err;
+
+	q->consumer_counter = 0;
+
+	for (i = 0; i < q->count; i++) {
+		char *elem = mlxsw_pci_queue_elem_get(q, i);
+
+		mlxsw_pci_eqe_owner_set(elem, 1);
+	}
+
+	mlxsw_cmd_mbox_sw2hw_eq_int_msix_set(mbox, 1); /* MSI-X used */
+	mlxsw_cmd_mbox_sw2hw_eq_st_set(mbox, 1); /* armed */
+	mlxsw_cmd_mbox_sw2hw_eq_log_eq_size_set(mbox, ilog2(q->count));
+	for (i = 0; i < MLXSW_PCI_AQ_PAGES; i++) {
+		dma_addr_t mapaddr = __mlxsw_pci_queue_page_get(q, i);
+
+		mlxsw_cmd_mbox_sw2hw_eq_pa_set(mbox, i, mapaddr);
+	}
+	err = mlxsw_cmd_sw2hw_eq(mlxsw_pci->core, mbox, q->num);
+	if (err)
+		return err;
+	mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
+	mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
+	return 0;
+}
+
+static void mlxsw_pci_eq_fini(struct mlxsw_pci *mlxsw_pci,
+			      struct mlxsw_pci_queue *q)
+{
+	mlxsw_cmd_hw2sw_eq(mlxsw_pci->core, q->num);
+}
+
 struct mlxsw_pci_queue_ops {
 	const char *name;
 	enum mlxsw_pci_queue_type type;

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 1/15

2024-04-03 12:37 UTC #2 lore raw

Re: [PATCH net-next 01/15] mlxsw: pci: Move mlxsw_pci_eq_{init, fini}()

Quoted 8 lines · Petr Machata

From: Amit Cohen <[email protected]>
Move mlxsw_pci_eq_{init, fini}() after mlxsw_pci_eq_tasklet() as a next patch will setup the tasklet as part of initialization.
Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

Petr Machata

author maintainer

2024-04-02 13:57 UTC #3 lore raw

PATCH 2/15 mlxsw: pci: Move mlxsw_pci_cq_{init, fini}()

From: Amit Cohen <[email protected]>

Move mlxsw_pci_cq_{init, fini}() after mlxsw_pci_cq_tasklet() as a next patch will setup the tasklet as part of initialization.

Signed-off-by Amit Cohen <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 86 +++++++++++------------
 1 file changed, 43 insertions(+), 43 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +43 ~~-43~~

index 93569b14b357..97c5c7a91aea 100644
@@ -470,49 +470,6 @@ static void mlxsw_pci_cq_pre_init(struct mlxsw_pci *mlxsw_pci,
 		q->u.cq.v = MLXSW_PCI_CQE_V1;
 }
 
-static int mlxsw_pci_cq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
-			     struct mlxsw_pci_queue *q)
-{
-	int i;
-	int err;
-
-	q->consumer_counter = 0;
-
-	for (i = 0; i < q->count; i++) {
-		char *elem = mlxsw_pci_queue_elem_get(q, i);
-
-		mlxsw_pci_cqe_owner_set(q->u.cq.v, elem, 1);
-	}
-
-	if (q->u.cq.v == MLXSW_PCI_CQE_V1)
-		mlxsw_cmd_mbox_sw2hw_cq_cqe_ver_set(mbox,
-				MLXSW_CMD_MBOX_SW2HW_CQ_CQE_VER_1);
-	else if (q->u.cq.v == MLXSW_PCI_CQE_V2)
-		mlxsw_cmd_mbox_sw2hw_cq_cqe_ver_set(mbox,
-				MLXSW_CMD_MBOX_SW2HW_CQ_CQE_VER_2);
-
-	mlxsw_cmd_mbox_sw2hw_cq_c_eqn_set(mbox, MLXSW_PCI_EQ_COMP_NUM);
-	mlxsw_cmd_mbox_sw2hw_cq_st_set(mbox, 0);
-	mlxsw_cmd_mbox_sw2hw_cq_log_cq_size_set(mbox, ilog2(q->count));
-	for (i = 0; i < MLXSW_PCI_AQ_PAGES; i++) {
-		dma_addr_t mapaddr = __mlxsw_pci_queue_page_get(q, i);
-
-		mlxsw_cmd_mbox_sw2hw_cq_pa_set(mbox, i, mapaddr);
-	}
-	err = mlxsw_cmd_sw2hw_cq(mlxsw_pci->core, mbox, q->num);
-	if (err)
-		return err;
-	mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
-	mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
-	return 0;
-}
-
-static void mlxsw_pci_cq_fini(struct mlxsw_pci *mlxsw_pci,
-			      struct mlxsw_pci_queue *q)
-{
-	mlxsw_cmd_hw2sw_cq(mlxsw_pci->core, q->num);
-}
-
 static unsigned int mlxsw_pci_read32_off(struct mlxsw_pci *mlxsw_pci,
 					 ptrdiff_t off)
 {

@@ -753,6 +710,49 @@ static void mlxsw_pci_cq_tasklet(struct tasklet_struct *t)
 		mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
 }
 
+static int mlxsw_pci_cq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
+			     struct mlxsw_pci_queue *q)
+{
+	int i;
+	int err;
+
+	q->consumer_counter = 0;
+
+	for (i = 0; i < q->count; i++) {
+		char *elem = mlxsw_pci_queue_elem_get(q, i);
+
+		mlxsw_pci_cqe_owner_set(q->u.cq.v, elem, 1);
+	}
+
+	if (q->u.cq.v == MLXSW_PCI_CQE_V1)
+		mlxsw_cmd_mbox_sw2hw_cq_cqe_ver_set(mbox,
+				MLXSW_CMD_MBOX_SW2HW_CQ_CQE_VER_1);
+	else if (q->u.cq.v == MLXSW_PCI_CQE_V2)
+		mlxsw_cmd_mbox_sw2hw_cq_cqe_ver_set(mbox,
+				MLXSW_CMD_MBOX_SW2HW_CQ_CQE_VER_2);
+
+	mlxsw_cmd_mbox_sw2hw_cq_c_eqn_set(mbox, MLXSW_PCI_EQ_COMP_NUM);
+	mlxsw_cmd_mbox_sw2hw_cq_st_set(mbox, 0);
+	mlxsw_cmd_mbox_sw2hw_cq_log_cq_size_set(mbox, ilog2(q->count));
+	for (i = 0; i < MLXSW_PCI_AQ_PAGES; i++) {
+		dma_addr_t mapaddr = __mlxsw_pci_queue_page_get(q, i);
+
+		mlxsw_cmd_mbox_sw2hw_cq_pa_set(mbox, i, mapaddr);
+	}
+	err = mlxsw_cmd_sw2hw_cq(mlxsw_pci->core, mbox, q->num);
+	if (err)
+		return err;
+	mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
+	mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
+	return 0;
+}
+
+static void mlxsw_pci_cq_fini(struct mlxsw_pci *mlxsw_pci,
+			      struct mlxsw_pci_queue *q)
+{
+	mlxsw_cmd_hw2sw_cq(mlxsw_pci->core, q->num);
+}
+
 static u16 mlxsw_pci_cq_elem_count(const struct mlxsw_pci_queue *q)
 {
 	return q->u.cq.v == MLXSW_PCI_CQE_V2 ? MLXSW_PCI_CQE2_COUNT :

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 2/15

2024-04-03 12:37 UTC #4 lore raw

Re: [PATCH net-next 02/15] mlxsw: pci: Move mlxsw_pci_cq_{init, fini}()

Quoted 8 lines · Petr Machata

From: Amit Cohen <[email protected]>
Move mlxsw_pci_cq_{init, fini}() after mlxsw_pci_cq_tasklet() as a next patch will setup the tasklet as part of initialization.
Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

Petr Machata

author maintainer

2024-04-02 13:57 UTC #5 lore raw

PATCH 3/15 mlxsw: pci: Do not setup tasklet from operation

From: Amit Cohen <[email protected]>

Currently, the structure 'mlxsw_pci_queue_ops' holds a pointer to the callback function of tasklet. This is used only for EQ and CQ. mlxsw driver will use NAPI in a following patch set, so CQ will not use tasklet anymore. As preparation, remove this pointer from the shared operation structure and setup the tasklet as part of queue initialization. For now, setup tasklet for EQ and CQ. Later, CQ code will be changed.

Signed-off-by Amit Cohen <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +2 -6

index 97c5c7a91aea..2031487a9dae 100644
@@ -742,6 +742,7 @@ static int mlxsw_pci_cq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 	err = mlxsw_cmd_sw2hw_cq(mlxsw_pci->core, mbox, q->num);
 	if (err)
 		return err;
+	tasklet_setup(&q->tasklet, mlxsw_pci_cq_tasklet);
 	mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
 	mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
 	return 0;

@@ -866,6 +867,7 @@ static int mlxsw_pci_eq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 	err = mlxsw_cmd_sw2hw_eq(mlxsw_pci->core, mbox, q->num);
 	if (err)
 		return err;
+	tasklet_setup(&q->tasklet, mlxsw_pci_eq_tasklet);
 	mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
 	mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
 	return 0;

@@ -886,7 +888,6 @@ struct mlxsw_pci_queue_ops {
 		    struct mlxsw_pci_queue *q);
 	void (*fini)(struct mlxsw_pci *mlxsw_pci,
 		     struct mlxsw_pci_queue *q);
-	void (*tasklet)(struct tasklet_struct *t);
 	u16 (*elem_count_f)(const struct mlxsw_pci_queue *q);
 	u8 (*elem_size_f)(const struct mlxsw_pci_queue *q);
 	u16 elem_count;

@@ -914,7 +915,6 @@ static const struct mlxsw_pci_queue_ops mlxsw_pci_cq_ops = {
 	.pre_init	= mlxsw_pci_cq_pre_init,
 	.init		= mlxsw_pci_cq_init,
 	.fini		= mlxsw_pci_cq_fini,
-	.tasklet	= mlxsw_pci_cq_tasklet,
 	.elem_count_f	= mlxsw_pci_cq_elem_count,
 	.elem_size_f	= mlxsw_pci_cq_elem_size
 };

@@ -923,7 +923,6 @@ static const struct mlxsw_pci_queue_ops mlxsw_pci_eq_ops = {
 	.type		= MLXSW_PCI_QUEUE_TYPE_EQ,
 	.init		= mlxsw_pci_eq_init,
 	.fini		= mlxsw_pci_eq_fini,
-	.tasklet	= mlxsw_pci_eq_tasklet,
 	.elem_count	= MLXSW_PCI_EQE_COUNT,
 	.elem_size	= MLXSW_PCI_EQE_SIZE
 };

@@ -948,9 +947,6 @@ static int mlxsw_pci_queue_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 	q->type = q_ops->type;
 	q->pci = mlxsw_pci;
 
-	if (q_ops->tasklet)
-		tasklet_setup(&q->tasklet, q_ops->tasklet);
-
 	mem_item->size = MLXSW_PCI_AQ_SIZE;
 	mem_item->buf = dma_alloc_coherent(&mlxsw_pci->pdev->dev,
 					   mem_item->size, &mem_item->mapaddr,

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 3/15

2024-04-03 12:37 UTC #6 lore raw

Re: [PATCH net-next 03/15] mlxsw: pci: Do not setup tasklet from operation

Quoted 9 lines · Petr Machata

From: Amit Cohen <[email protected]>
Currently, the structure 'mlxsw_pci_queue_ops' holds a pointer to the callback function of tasklet. This is used only for EQ and CQ. mlxsw driver will use NAPI in a following patch set, so CQ will not use tasklet anymore. As preparation, remove this pointer from the shared operation structure and setup the tasklet as part of queue initialization. For now, setup tasklet for EQ and CQ. Later, CQ code will be changed.

Quoted 3 lines

Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

Petr Machata

author maintainer

2024-04-02 13:57 UTC #7 lore raw

PATCH 4/15 mlxsw: pci: Arm CQ doorbell regardless of number of completions

From: Amit Cohen <[email protected]>

Currently, as part of mlxsw_pci_cq_tasklet(), we check if any item was handled, and only in such case we arm doorbell. This is unlikely case, as we schedule tasklet only for CQs that we get an event for them, which means that they contain completions to handle. Remove this check, which is supposed to be true always, and even if it is false, it is not a mistake to ring the doorbell. We can warn on such case, but it is not really worth to add a check which will be run for each CQ handling when we do not expect to reach it and it does not point to logic error that should be handled.

Signed-off-by Amit Cohen <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +2 -2

index 2031487a9dae..3a5f902b625d 100644
@@ -706,8 +706,8 @@ static void mlxsw_pci_cq_tasklet(struct tasklet_struct *t)
 		if (++items == credits)
 			break;
 	}
-	if (items)
-		mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
+
+	mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
 }
 
 static int mlxsw_pci_cq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 4/15

2024-04-03 12:37 UTC #8 lore raw

Re: [PATCH net-next 04/15] mlxsw: pci: Arm CQ doorbell regardless of number of completions

Quoted 11 lines · Petr Machata

From: Amit Cohen <[email protected]>
Currently, as part of mlxsw_pci_cq_tasklet(), we check if any item was handled, and only in such case we arm doorbell. This is unlikely case, as we schedule tasklet only for CQs that we get an event for them, which means that they contain completions to handle. Remove this check, which is supposed to be true always, and even if it is false, it is not a mistake to ring the doorbell. We can warn on such case, but it is not really worth to add a check which will be run for each CQ handling when we do not expect to reach it and it does not point to logic error that should be handled.

Quoted 3 lines

Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

Petr Machata

author maintainer

2024-04-02 13:57 UTC #9 lore raw

PATCH 5/15 mlxsw: pci: Remove unused counters

From: Amit Cohen <[email protected]>

The structure 'mlxsw_pci_queue' stores several counters which were consumed via debugfs. Since commit 9a32562becd9 ("mlxsw: Remove debugfs interface"), these counters are not used. Remove them. This makes the 'union u' and 'struct eq' redundant. Maintain 'struct cq' as it will be extended later.

Replace increasing 'q->u.eq.ev_other_count' with WARN_ON_ONCE(), as it is used in an unreasonable case of receiving event in EQ which is not EQ0 or EQ1. When the queues are initialized, we check number of event queues and fail with the print "Unsupported number of queues" in case that the driver tries to initialize more than two queues.

Signed-off-by Amit Cohen <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 49 +++++++++--------------
 1 file changed, 18 insertions(+), 31 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +18 ~~-31~~

index 3a5f902b625d..f05137b85483 100644
@@ -80,18 +80,9 @@ struct mlxsw_pci_queue {
 	enum mlxsw_pci_queue_type type;
 	struct tasklet_struct tasklet; /* queue processing tasklet */
 	struct mlxsw_pci *pci;
-	union {
-		struct {
-			u32 comp_sdq_count;
-			u32 comp_rdq_count;
-			enum mlxsw_pci_cqe_v v;
-		} cq;
-		struct {
-			u32 ev_cmd_count;
-			u32 ev_comp_count;
-			u32 ev_other_count;
-		} eq;
-	} u;
+	struct {
+		enum mlxsw_pci_cqe_v v;
+	} cq;
 };
 
 struct mlxsw_pci_queue_type_group {

@@ -462,12 +453,12 @@ static void mlxsw_pci_rdq_fini(struct mlxsw_pci *mlxsw_pci,
 static void mlxsw_pci_cq_pre_init(struct mlxsw_pci *mlxsw_pci,
 				  struct mlxsw_pci_queue *q)
 {
-	q->u.cq.v = mlxsw_pci->max_cqe_ver;
+	q->cq.v = mlxsw_pci->max_cqe_ver;
 
-	if (q->u.cq.v == MLXSW_PCI_CQE_V2 &&
+	if (q->cq.v == MLXSW_PCI_CQE_V2 &&
 	    q->num < mlxsw_pci->num_sdq_cqs &&
 	    !mlxsw_core_sdq_supports_cqe_v2(mlxsw_pci->core))
-		q->u.cq.v = MLXSW_PCI_CQE_V1;
+		q->cq.v = MLXSW_PCI_CQE_V1;
 }
 
 static unsigned int mlxsw_pci_read32_off(struct mlxsw_pci *mlxsw_pci,

@@ -663,7 +654,7 @@ static char *mlxsw_pci_cq_sw_cqe_get(struct mlxsw_pci_queue *q)
 
 	elem_info = mlxsw_pci_queue_elem_info_consumer_get(q);
 	elem = elem_info->elem;
-	owner_bit = mlxsw_pci_cqe_owner_get(q->u.cq.v, elem);
+	owner_bit = mlxsw_pci_cqe_owner_get(q->cq.v, elem);
 	if (mlxsw_pci_elem_hw_owned(q, owner_bit))
 		return NULL;
 	q->consumer_counter++;

@@ -681,8 +672,8 @@ static void mlxsw_pci_cq_tasklet(struct tasklet_struct *t)
 
 	while ((cqe = mlxsw_pci_cq_sw_cqe_get(q))) {
 		u16 wqe_counter = mlxsw_pci_cqe_wqe_counter_get(cqe);
-		u8 sendq = mlxsw_pci_cqe_sr_get(q->u.cq.v, cqe);
-		u8 dqn = mlxsw_pci_cqe_dqn_get(q->u.cq.v, cqe);
+		u8 sendq = mlxsw_pci_cqe_sr_get(q->cq.v, cqe);
+		u8 dqn = mlxsw_pci_cqe_dqn_get(q->cq.v, cqe);
 		char ncqe[MLXSW_PCI_CQE_SIZE_MAX];
 
 		memcpy(ncqe, cqe, q->elem_size);

@@ -693,15 +684,13 @@ static void mlxsw_pci_cq_tasklet(struct tasklet_struct *t)
 
 			sdq = mlxsw_pci_sdq_get(mlxsw_pci, dqn);
 			mlxsw_pci_cqe_sdq_handle(mlxsw_pci, sdq,
-						 wqe_counter, q->u.cq.v, ncqe);
-			q->u.cq.comp_sdq_count++;
+						 wqe_counter, q->cq.v, ncqe);
 		} else {
 			struct mlxsw_pci_queue *rdq;
 
 			rdq = mlxsw_pci_rdq_get(mlxsw_pci, dqn);
 			mlxsw_pci_cqe_rdq_handle(mlxsw_pci, rdq,
-						 wqe_counter, q->u.cq.v, ncqe);
-			q->u.cq.comp_rdq_count++;
+						 wqe_counter, q->cq.v, ncqe);
 		}
 		if (++items == credits)
 			break;

@@ -721,13 +710,13 @@ static int mlxsw_pci_cq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 	for (i = 0; i < q->count; i++) {
 		char *elem = mlxsw_pci_queue_elem_get(q, i);
 
-		mlxsw_pci_cqe_owner_set(q->u.cq.v, elem, 1);
+		mlxsw_pci_cqe_owner_set(q->cq.v, elem, 1);
 	}
 
-	if (q->u.cq.v == MLXSW_PCI_CQE_V1)
+	if (q->cq.v == MLXSW_PCI_CQE_V1)
 		mlxsw_cmd_mbox_sw2hw_cq_cqe_ver_set(mbox,
 				MLXSW_CMD_MBOX_SW2HW_CQ_CQE_VER_1);
-	else if (q->u.cq.v == MLXSW_PCI_CQE_V2)
+	else if (q->cq.v == MLXSW_PCI_CQE_V2)
 		mlxsw_cmd_mbox_sw2hw_cq_cqe_ver_set(mbox,
 				MLXSW_CMD_MBOX_SW2HW_CQ_CQE_VER_2);

@@ -756,13 +745,13 @@ static void mlxsw_pci_cq_fini(struct mlxsw_pci *mlxsw_pci,
 
 static u16 mlxsw_pci_cq_elem_count(const struct mlxsw_pci_queue *q)
 {
-	return q->u.cq.v == MLXSW_PCI_CQE_V2 ? MLXSW_PCI_CQE2_COUNT :
-					       MLXSW_PCI_CQE01_COUNT;
+	return q->cq.v == MLXSW_PCI_CQE_V2 ? MLXSW_PCI_CQE2_COUNT :
+					     MLXSW_PCI_CQE01_COUNT;
 }
 
 static u8 mlxsw_pci_cq_elem_size(const struct mlxsw_pci_queue *q)
 {
-	return q->u.cq.v == MLXSW_PCI_CQE_V2 ? MLXSW_PCI_CQE2_SIZE :
+	return q->cq.v == MLXSW_PCI_CQE_V2 ? MLXSW_PCI_CQE2_SIZE :
 					       MLXSW_PCI_CQE01_SIZE;
 }

@@ -815,16 +804,14 @@ static void mlxsw_pci_eq_tasklet(struct tasklet_struct *t)
 		switch (q->num) {
 		case MLXSW_PCI_EQ_ASYNC_NUM:
 			mlxsw_pci_eq_cmd_event(mlxsw_pci, eqe);
-			q->u.eq.ev_cmd_count++;
 			break;
 		case MLXSW_PCI_EQ_COMP_NUM:
 			cqn = mlxsw_pci_eqe_cqn_get(eqe);
 			set_bit(cqn, active_cqns);
 			cq_handle = true;
-			q->u.eq.ev_comp_count++;
 			break;
 		default:
-			q->u.eq.ev_other_count++;
+			WARN_ON_ONCE(1);
 		}
 		if (++items == credits)
 			break;

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 5/15

2024-04-03 12:38 UTC #10 lore raw

Re: [PATCH net-next 05/15] mlxsw: pci: Remove unused counters

Quoted 13 lines · Petr Machata

From: Amit Cohen <[email protected]>
The structure 'mlxsw_pci_queue' stores several counters which were consumed via debugfs. Since commit 9a32562becd9 ("mlxsw: Remove debugfs interface"), these counters are not used. Remove them. This makes the 'union u' and 'struct eq' redundant. Maintain 'struct cq' as it will be extended later.
Replace increasing 'q->u.eq.ev_other_count' with WARN_ON_ONCE(), as it is used in an unreasonable case of receiving event in EQ which is not EQ0 or EQ1. When the queues are initialized, we check number of event queues and fail with the print "Unsupported number of queues" in case that the driver tries to initialize more than two queues.

Quoted 3 lines

Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

Petr Machata

author maintainer

2024-04-02 13:57 UTC #11 lore raw

PATCH 6/15 mlxsw: pci: Make style changes in mlxsw_pci_eq_tasklet()

From: Amit Cohen <[email protected]>

This function will be used later only for EQ1. As preparation, reorder variables to reverse xmas tree and return earlier when it is possible, to simplify the code.

Signed-off-by Amit Cohen <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +13 ~~-10~~

index f05137b85483..c9bd9a98cf1e 100644
@@ -783,15 +783,14 @@ static char *mlxsw_pci_eq_sw_eqe_get(struct mlxsw_pci_queue *q)
 
 static void mlxsw_pci_eq_tasklet(struct tasklet_struct *t)
 {
-	struct mlxsw_pci_queue *q = from_tasklet(q, t, tasklet);
-	struct mlxsw_pci *mlxsw_pci = q->pci;
-	u8 cq_count = mlxsw_pci_cq_count(mlxsw_pci);
 	unsigned long active_cqns[BITS_TO_LONGS(MLXSW_PCI_CQS_MAX)];
-	char *eqe;
-	u8 cqn;
+	struct mlxsw_pci_queue *q = from_tasklet(q, t, tasklet);
+	struct mlxsw_pci *mlxsw_pci = q->pci;
+	int credits = q->count >> 1;
 	bool cq_handle = false;
+	u8 cqn, cq_count;
 	int items = 0;
-	int credits = q->count >> 1;
+	char *eqe;
 
 	memset(&active_cqns, 0, sizeof(active_cqns));

@@ -816,13 +815,17 @@ static void mlxsw_pci_eq_tasklet(struct tasklet_struct *t)
 		if (++items == credits)
 			break;
 	}
-	if (items) {
-		mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
-		mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
-	}
+
+	if (!items)
+		return;
+
+	mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
+	mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
 
 	if (!cq_handle)
 		return;
+
+	cq_count = mlxsw_pci_cq_count(mlxsw_pci);
 	for_each_set_bit(cqn, active_cqns, cq_count) {
 		q = mlxsw_pci_cq_get(mlxsw_pci, cqn);
 		mlxsw_pci_queue_tasklet_schedule(q);

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 6/15

2024-04-03 12:38 UTC #12 lore raw

Re: [PATCH net-next 06/15] mlxsw: pci: Make style changes in mlxsw_pci_eq_tasklet()

Quoted 9 lines · Petr Machata

From: Amit Cohen <[email protected]>
This function will be used later only for EQ1. As preparation, reorder variables to reverse xmas tree and return earlier when it is possible, to simplify the code.
Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

Petr Machata

author maintainer

2024-04-02 13:57 UTC #13 lore raw

PATCH 7/15 mlxsw: pci: Poll command interface for each cmd_exec()

From: Amit Cohen <[email protected]>

Command interface is used for configuring and querying FW when EMADs are not available. During the time that the driver sets up the asynchronous queues, it polls the command interface for getting completions. Then, there is a short period when asynchronous queues work, but EMADs are not available (marked in the code as nopoll = true). During this time, we send commands via command interface, but we do not poll it, as we can get an interrupt for the completion. Completions of command interface are received from HW in EQ0 (event queue 0).

The usage of EQ0 instead of polling is done only 4 times during initialization and one time during tear down, but it makes an overhead during lifetime of the driver. For each interrupt, we have to check if we get events in EQ0 or EQ1 and handle them. This is really ineffective, especially because of the fact that EQ0 is used only as part of driver init/fini.

Instead, we can poll command interface for each call of cmd_exec(). It means that when we send a command via command interface (as EMADs are not available), we will poll it, regardless of availability of the asynchronous queues. This will allow us to configure later only EQ1 and simplify the flow.

Remove 'nopoll' indication and change mlxsw_pci_cmd_exec() to poll till answer/timeout regardless of queues' state. For now, completions are handled also by EQ0, but it will be removed in next patch. Additional cleanups will be added in next patches.

Signed-off-by Amit Cohen <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 48 ++++++++---------------
 1 file changed, 17 insertions(+), 31 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +17 ~~-31~~

index c9bd9a98cf1e..b7a83b9ab495 100644
@@ -111,7 +111,6 @@ struct mlxsw_pci {
 		struct mlxsw_pci_mem_item out_mbox;
 		struct mlxsw_pci_mem_item in_mbox;
 		struct mutex lock; /* Lock access to command registers */
-		bool nopoll;
 		wait_queue_head_t wait;
 		bool wait_done;
 		struct {

@@ -1105,8 +1104,6 @@ static int mlxsw_pci_aqs_init(struct mlxsw_pci *mlxsw_pci, char *mbox)
 		goto err_rdqs_init;
 	}
 
-	/* We have to poll in command interface until queues are initialized */
-	mlxsw_pci->cmd.nopoll = true;
 	return 0;
 
 err_rdqs_init:

@@ -1120,7 +1117,6 @@ static int mlxsw_pci_aqs_init(struct mlxsw_pci *mlxsw_pci, char *mbox)
 
 static void mlxsw_pci_aqs_fini(struct mlxsw_pci *mlxsw_pci)
 {
-	mlxsw_pci->cmd.nopoll = false;
 	mlxsw_pci_queue_group_fini(mlxsw_pci, &mlxsw_pci_rdq_ops);
 	mlxsw_pci_queue_group_fini(mlxsw_pci, &mlxsw_pci_sdq_ops);
 	mlxsw_pci_queue_group_fini(mlxsw_pci, &mlxsw_pci_cq_ops);

@@ -1846,9 +1842,9 @@ static int mlxsw_pci_cmd_exec(void *bus_priv, u16 opcode, u8 opcode_mod,
 {
 	struct mlxsw_pci *mlxsw_pci = bus_priv;
 	dma_addr_t in_mapaddr = 0, out_mapaddr = 0;
-	bool evreq = mlxsw_pci->cmd.nopoll;
 	unsigned long timeout = msecs_to_jiffies(MLXSW_PCI_CIR_TIMEOUT_MSECS);
 	bool *p_wait_done = &mlxsw_pci->cmd.wait_done;
+	unsigned long end;
 	int err;
 
 	*p_status = MLXSW_CMD_STATUS_OK;

@@ -1877,28 +1873,20 @@ static int mlxsw_pci_cmd_exec(void *bus_priv, u16 opcode, u8 opcode_mod,
 	wmb(); /* all needs to be written before we write control register */
 	mlxsw_pci_write32(mlxsw_pci, CIR_CTRL,
 			  MLXSW_PCI_CIR_CTRL_GO_BIT |
-			  (evreq ? MLXSW_PCI_CIR_CTRL_EVREQ_BIT : 0) |
 			  (opcode_mod << MLXSW_PCI_CIR_CTRL_OPCODE_MOD_SHIFT) |
 			  opcode);
 
-	if (!evreq) {
-		unsigned long end;
+	end = jiffies + timeout;
+	do {
+		u32 ctrl = mlxsw_pci_read32(mlxsw_pci, CIR_CTRL);
 
-		end = jiffies + timeout;
-		do {
-			u32 ctrl = mlxsw_pci_read32(mlxsw_pci, CIR_CTRL);
-
-			if (!(ctrl & MLXSW_PCI_CIR_CTRL_GO_BIT)) {
-				*p_wait_done = true;
-				*p_status = ctrl >> MLXSW_PCI_CIR_CTRL_STATUS_SHIFT;
-				break;
-			}
-			cond_resched();
-		} while (time_before(jiffies, end));
-	} else {
-		wait_event_timeout(mlxsw_pci->cmd.wait, *p_wait_done, timeout);
-		*p_status = mlxsw_pci->cmd.comp.status;
-	}
+		if (!(ctrl & MLXSW_PCI_CIR_CTRL_GO_BIT)) {
+			*p_wait_done = true;
+			*p_status = ctrl >> MLXSW_PCI_CIR_CTRL_STATUS_SHIFT;
+			break;
+		}
+		cond_resched();
+	} while (time_before(jiffies, end));
 
 	err = 0;
 	if (*p_wait_done) {

@@ -1915,14 +1903,12 @@ static int mlxsw_pci_cmd_exec(void *bus_priv, u16 opcode, u8 opcode_mod,
 		 */
 		__be32 tmp;
 
-		if (!evreq) {
-			tmp = cpu_to_be32(mlxsw_pci_read32(mlxsw_pci,
-							   CIR_OUT_PARAM_HI));
-			memcpy(out_mbox, &tmp, sizeof(tmp));
-			tmp = cpu_to_be32(mlxsw_pci_read32(mlxsw_pci,
-							   CIR_OUT_PARAM_LO));
-			memcpy(out_mbox + sizeof(tmp), &tmp, sizeof(tmp));
-		}
+		tmp = cpu_to_be32(mlxsw_pci_read32(mlxsw_pci,
+						   CIR_OUT_PARAM_HI));
+		memcpy(out_mbox, &tmp, sizeof(tmp));
+		tmp = cpu_to_be32(mlxsw_pci_read32(mlxsw_pci,
+						   CIR_OUT_PARAM_LO));
+		memcpy(out_mbox + sizeof(tmp), &tmp, sizeof(tmp));
 	} else if (!err && out_mbox) {
 		memcpy(out_mbox, mlxsw_pci->cmd.out_mbox.buf, out_mbox_size);
 	}

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 7/15

2024-04-03 12:38 UTC #14 lore raw

Re: [PATCH net-next 07/15] mlxsw: pci: Poll command interface for each cmd_exec()

Quoted 24 lines · Petr Machata

From: Amit Cohen <[email protected]>
Command interface is used for configuring and querying FW when EMADs are not available. During the time that the driver sets up the asynchronous queues, it polls the command interface for getting completions. Then, there is a short period when asynchronous queues work, but EMADs are not available (marked in the code as nopoll = true). During this time, we send commands via command interface, but we do not poll it, as we can get an interrupt for the completion. Completions of command interface are received from HW in EQ0 (event queue 0).
The usage of EQ0 instead of polling is done only 4 times during initialization and one time during tear down, but it makes an overhead during lifetime of the driver. For each interrupt, we have to check if we get events in EQ0 or EQ1 and handle them. This is really ineffective, especially because of the fact that EQ0 is used only as part of driver init/fini.
Instead, we can poll command interface for each call of cmd_exec(). It means that when we send a command via command interface (as EMADs are not available), we will poll it, regardless of availability of the asynchronous queues. This will allow us to configure later only EQ1 and simplify the flow.

Quoted 8 lines

Remove 'nopoll' indication and change mlxsw_pci_cmd_exec() to poll till answer/timeout regardless of queues' state. For now, completions are handled also by EQ0, but it will be removed in next patch. Additional cleanups will be added in next patches.
Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

Petr Machata

author maintainer

2024-04-02 13:57 UTC #15 lore raw

PATCH 8/15 mlxsw: pci: Rename MLXSW_PCI_EQS_COUNT

From: Amit Cohen <[email protected]>

Currently we use MLXSW_PCI_EQS_COUNT event queues. A next patch will change the driver to initialize only EQ1, as EQ0 is not required anymore when we poll command interface.

Rename the macro to MLXSW_PCI_EQS_MAX as later we will not initialize the maximum supported EQs, this value represents the maximum and a new macro will be added to represent the actual used queues.

Signed-off-by Amit Cohen <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c    | 4 ++--
 drivers/net/ethernet/mellanox/mlxsw/pci_hw.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +2 -2

index b7a83b9ab495..cb960917e462 100644
@@ -1059,7 +1059,7 @@ static int mlxsw_pci_aqs_init(struct mlxsw_pci *mlxsw_pci, char *mbox)
 
 	if (num_sdqs + num_rdqs > num_cqs ||
 	    num_sdqs < MLXSW_PCI_SDQS_MIN ||
-	    num_cqs > MLXSW_PCI_CQS_MAX || num_eqs != MLXSW_PCI_EQS_COUNT) {
+	    num_cqs > MLXSW_PCI_CQS_MAX || num_eqs != MLXSW_PCI_EQS_MAX) {
 		dev_err(&pdev->dev, "Unsupported number of queues\n");
 		return -EINVAL;
 	}

@@ -1416,7 +1416,7 @@ static irqreturn_t mlxsw_pci_eq_irq_handler(int irq, void *dev_id)
 	struct mlxsw_pci_queue *q;
 	int i;
 
-	for (i = 0; i < MLXSW_PCI_EQS_COUNT; i++) {
+	for (i = 0; i < MLXSW_PCI_EQS_MAX; i++) {
 		q = mlxsw_pci_eq_get(mlxsw_pci, i);
 		mlxsw_pci_queue_tasklet_schedule(q);
 	}

drivers/net/ethernet/mellanox/mlxsw/pci_hw.h +1 -1

index 7cdf0ce24f28..32a4f436d24d 100644
@@ -42,7 +42,7 @@
 	((offset) + (type_offset) + (num) * 4)
 
 #define MLXSW_PCI_CQS_MAX	96
-#define MLXSW_PCI_EQS_COUNT	2
+#define MLXSW_PCI_EQS_MAX	2
 #define MLXSW_PCI_EQ_ASYNC_NUM	0
 #define MLXSW_PCI_EQ_COMP_NUM	1

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 8/15

2024-04-03 12:39 UTC #16 lore raw

Re: [PATCH net-next 08/15] mlxsw: pci: Rename MLXSW_PCI_EQS_COUNT

Quoted 6 lines · Petr Machata

From: Amit Cohen <[email protected]>
Currently we use MLXSW_PCI_EQS_COUNT event queues. A next patch will change the driver to initialize only EQ1, as EQ0 is not required anymore when we poll command interface.

Quoted 7 lines

Rename the macro to MLXSW_PCI_EQS_MAX as later we will not initialize the maximum supported EQs, this value represents the maximum and a new macro will be added to represent the actual used queues.
Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

Petr Machata

author maintainer

2024-04-02 13:57 UTC #17 lore raw

PATCH 9/15 mlxsw: pci: Use only one event queue

From: Amit Cohen <[email protected]>

The device supports two event queues. EQ0 is used for command interface completion events. EQ1 is used for completion events of RDQ or SDQ.

Currently, for each EQE (event queue element), we check the queue number and handle accordingly. More than that, for each interrupt we schedule tasklets for both EQs. This is really ineffective, especially because of the fact that EQ0 is used only as part of driver init/fini, when EMADs are not available. There is no point to schedule the tasklet for it and check each EQE.

A previous patch changed the code to poll command interface for each use of it. It means that now there is no real reason to use EQ0, as we poll the command interface.

Initialize only one event queue and use it as EQ1 (this is determined by queue number). Then, for each interrupt we can schedule the tasklet only for one queue and we do not have to check the queue number. This simplifies the code and should improve performance. Note that polling command interface is ok as we use it only as part of driver init/fini.

Signed-off-by Amit Cohen <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c    | 54 ++++++--------------
 drivers/net/ethernet/mellanox/mlxsw/pci_hw.h |  2 +-
 2 files changed, 16 insertions(+), 40 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +15 ~~-39~~

index cb960917e462..3460a4ef7d9a 100644
@@ -223,10 +223,10 @@ static struct mlxsw_pci_queue *mlxsw_pci_cq_get(struct mlxsw_pci *mlxsw_pci,
 	return __mlxsw_pci_queue_get(mlxsw_pci, MLXSW_PCI_QUEUE_TYPE_CQ, q_num);
 }
 
-static struct mlxsw_pci_queue *mlxsw_pci_eq_get(struct mlxsw_pci *mlxsw_pci,
-						u8 q_num)
+static struct mlxsw_pci_queue *mlxsw_pci_eq_get(struct mlxsw_pci *mlxsw_pci)
 {
-	return __mlxsw_pci_queue_get(mlxsw_pci, MLXSW_PCI_QUEUE_TYPE_EQ, q_num);
+	/* There is only one EQ at index 0. */
+	return __mlxsw_pci_queue_get(mlxsw_pci, MLXSW_PCI_QUEUE_TYPE_EQ, 0);
 }
 
 static void __mlxsw_pci_queue_doorbell_set(struct mlxsw_pci *mlxsw_pci,

@@ -754,16 +754,6 @@ static u8 mlxsw_pci_cq_elem_size(const struct mlxsw_pci_queue *q)
 					       MLXSW_PCI_CQE01_SIZE;
 }
 
-static void mlxsw_pci_eq_cmd_event(struct mlxsw_pci *mlxsw_pci, char *eqe)
-{
-	mlxsw_pci->cmd.comp.status = mlxsw_pci_eqe_cmd_status_get(eqe);
-	mlxsw_pci->cmd.comp.out_param =
-		((u64) mlxsw_pci_eqe_cmd_out_param_h_get(eqe)) << 32 |
-		mlxsw_pci_eqe_cmd_out_param_l_get(eqe);
-	mlxsw_pci->cmd.wait_done = true;
-	wake_up(&mlxsw_pci->cmd.wait);
-}
-
 static char *mlxsw_pci_eq_sw_eqe_get(struct mlxsw_pci_queue *q)
 {
 	struct mlxsw_pci_queue_elem_info *elem_info;

@@ -786,7 +776,6 @@ static void mlxsw_pci_eq_tasklet(struct tasklet_struct *t)
 	struct mlxsw_pci_queue *q = from_tasklet(q, t, tasklet);
 	struct mlxsw_pci *mlxsw_pci = q->pci;
 	int credits = q->count >> 1;
-	bool cq_handle = false;
 	u8 cqn, cq_count;
 	int items = 0;
 	char *eqe;

@@ -794,23 +783,9 @@ static void mlxsw_pci_eq_tasklet(struct tasklet_struct *t)
 	memset(&active_cqns, 0, sizeof(active_cqns));
 
 	while ((eqe = mlxsw_pci_eq_sw_eqe_get(q))) {
+		cqn = mlxsw_pci_eqe_cqn_get(eqe);
+		set_bit(cqn, active_cqns);
 
-		/* Command interface completion events are always received on
-		 * queue MLXSW_PCI_EQ_ASYNC_NUM (EQ0) and completion events
-		 * are mapped to queue MLXSW_PCI_EQ_COMP_NUM (EQ1).
-		 */
-		switch (q->num) {
-		case MLXSW_PCI_EQ_ASYNC_NUM:
-			mlxsw_pci_eq_cmd_event(mlxsw_pci, eqe);
-			break;
-		case MLXSW_PCI_EQ_COMP_NUM:
-			cqn = mlxsw_pci_eqe_cqn_get(eqe);
-			set_bit(cqn, active_cqns);
-			cq_handle = true;
-			break;
-		default:
-			WARN_ON_ONCE(1);
-		}
 		if (++items == credits)
 			break;
 	}

@@ -821,9 +796,6 @@ static void mlxsw_pci_eq_tasklet(struct tasklet_struct *t)
 	mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
 	mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
 
-	if (!cq_handle)
-		return;
-
 	cq_count = mlxsw_pci_cq_count(mlxsw_pci);
 	for_each_set_bit(cqn, active_cqns, cq_count) {
 		q = mlxsw_pci_cq_get(mlxsw_pci, cqn);

@@ -837,6 +809,13 @@ static int mlxsw_pci_eq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 	int i;
 	int err;
 
+	/* We expect to initialize only one EQ, which gets num=0 as it is
+	 * located at index zero. We use the EQ as EQ1, so set the number for
+	 * future use.
+	 */
+	WARN_ON_ONCE(q->num);
+	q->num = MLXSW_PCI_EQ_COMP_NUM;
+
 	q->consumer_counter = 0;
 
 	for (i = 0; i < q->count; i++) {

@@ -1077,7 +1056,7 @@ static int mlxsw_pci_aqs_init(struct mlxsw_pci *mlxsw_pci, char *mbox)
 	mlxsw_pci->num_sdq_cqs = num_sdqs;
 
 	err = mlxsw_pci_queue_group_init(mlxsw_pci, mbox, &mlxsw_pci_eq_ops,
-					 num_eqs);
+					 MLXSW_PCI_EQS_COUNT);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to initialize event queues\n");
 		return err;

@@ -1414,12 +1393,9 @@ static irqreturn_t mlxsw_pci_eq_irq_handler(int irq, void *dev_id)
 {
 	struct mlxsw_pci *mlxsw_pci = dev_id;
 	struct mlxsw_pci_queue *q;
-	int i;
 
-	for (i = 0; i < MLXSW_PCI_EQS_MAX; i++) {
-		q = mlxsw_pci_eq_get(mlxsw_pci, i);
-		mlxsw_pci_queue_tasklet_schedule(q);
-	}
+	q = mlxsw_pci_eq_get(mlxsw_pci);
+	mlxsw_pci_queue_tasklet_schedule(q);
 	return IRQ_HANDLED;
 }

drivers/net/ethernet/mellanox/mlxsw/pci_hw.h +1 -1

index 32a4f436d24d..6bed495dcf0f 100644
@@ -43,7 +43,7 @@
 
 #define MLXSW_PCI_CQS_MAX	96
 #define MLXSW_PCI_EQS_MAX	2
-#define MLXSW_PCI_EQ_ASYNC_NUM	0
+#define MLXSW_PCI_EQS_COUNT	1
 #define MLXSW_PCI_EQ_COMP_NUM	1
 
 #define MLXSW_PCI_SDQS_MIN	2 /* EMAD and control traffic */

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 9/15

2024-04-03 12:39 UTC #18 lore raw

Re: [PATCH net-next 09/15] mlxsw: pci: Use only one event queue

Quoted 22 lines · Petr Machata

From: Amit Cohen <[email protected]>
The device supports two event queues. EQ0 is used for command interface completion events. EQ1 is used for completion events of RDQ or SDQ.
Currently, for each EQE (event queue element), we check the queue number and handle accordingly. More than that, for each interrupt we schedule tasklets for both EQs. This is really ineffective, especially because of the fact that EQ0 is used only as part of driver init/fini, when EMADs are not available. There is no point to schedule the tasklet for it and check each EQE.
A previous patch changed the code to poll command interface for each use of it. It means that now there is no real reason to use EQ0, as we poll the command interface.
Initialize only one event queue and use it as EQ1 (this is determined by queue number). Then, for each interrupt we can schedule the tasklet only for one queue and we do not have to check the queue number. This simplifies the code and should improve performance. Note that polling command interface is ok as we use it only as part of driver init/fini.

Quoted 3 lines

Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

Petr Machata

author maintainer

2024-04-02 13:57 UTC #19 lore raw

PATCH 10/15 mlxsw: pci: Remove unused wait queue

From: Amit Cohen <[email protected]>

The previous patch changed the code to do not handle command interface from event queue. With this change the wait queue is not used anymore. Remove it and 'wait_done' variable.

Signed-off-by Amit Cohen <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +4 -8

index 3460a4ef7d9a..7f059306af5a 100644
@@ -8,7 +8,6 @@
 #include <linux/device.h>
 #include <linux/pci.h>
 #include <linux/interrupt.h>
-#include <linux/wait.h>
 #include <linux/types.h>
 #include <linux/skbuff.h>
 #include <linux/if_vlan.h>

@@ -111,8 +110,6 @@ struct mlxsw_pci {
 		struct mlxsw_pci_mem_item out_mbox;
 		struct mlxsw_pci_mem_item in_mbox;
 		struct mutex lock; /* Lock access to command registers */
-		wait_queue_head_t wait;
-		bool wait_done;
 		struct {
 			u8 status;
 			u64 out_param;

@@ -1819,8 +1816,8 @@ static int mlxsw_pci_cmd_exec(void *bus_priv, u16 opcode, u8 opcode_mod,
 	struct mlxsw_pci *mlxsw_pci = bus_priv;
 	dma_addr_t in_mapaddr = 0, out_mapaddr = 0;
 	unsigned long timeout = msecs_to_jiffies(MLXSW_PCI_CIR_TIMEOUT_MSECS);
-	bool *p_wait_done = &mlxsw_pci->cmd.wait_done;
 	unsigned long end;
+	bool wait_done;
 	int err;
 
 	*p_status = MLXSW_CMD_STATUS_OK;

@@ -1844,7 +1841,7 @@ static int mlxsw_pci_cmd_exec(void *bus_priv, u16 opcode, u8 opcode_mod,
 	mlxsw_pci_write32(mlxsw_pci, CIR_IN_MODIFIER, in_mod);
 	mlxsw_pci_write32(mlxsw_pci, CIR_TOKEN, 0);
 
-	*p_wait_done = false;
+	wait_done = false;
 
 	wmb(); /* all needs to be written before we write control register */
 	mlxsw_pci_write32(mlxsw_pci, CIR_CTRL,

@@ -1857,7 +1854,7 @@ static int mlxsw_pci_cmd_exec(void *bus_priv, u16 opcode, u8 opcode_mod,
 		u32 ctrl = mlxsw_pci_read32(mlxsw_pci, CIR_CTRL);
 
 		if (!(ctrl & MLXSW_PCI_CIR_CTRL_GO_BIT)) {
-			*p_wait_done = true;
+			wait_done = true;
 			*p_status = ctrl >> MLXSW_PCI_CIR_CTRL_STATUS_SHIFT;
 			break;
 		}

@@ -1865,7 +1862,7 @@ static int mlxsw_pci_cmd_exec(void *bus_priv, u16 opcode, u8 opcode_mod,
 	} while (time_before(jiffies, end));
 
 	err = 0;
-	if (*p_wait_done) {
+	if (wait_done) {
 		if (*p_status)
 			err = -EIO;
 	} else {

@@ -1963,7 +1960,6 @@ static int mlxsw_pci_cmd_init(struct mlxsw_pci *mlxsw_pci)
 	int err;
 
 	mutex_init(&mlxsw_pci->cmd.lock);
-	init_waitqueue_head(&mlxsw_pci->cmd.wait);
 
 	err = mlxsw_pci_mbox_alloc(mlxsw_pci, &mlxsw_pci->cmd.in_mbox);
 	if (err)

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 10/15

2024-04-03 12:39 UTC #20 lore raw

Re: [PATCH net-next 10/15] mlxsw: pci: Remove unused wait queue

Quoted 9 lines · Petr Machata

From: Amit Cohen <[email protected]>
The previous patch changed the code to do not handle command interface from event queue. With this change the wait queue is not used anymore. Remove it and 'wait_done' variable.
Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

Petr Machata

author maintainer

2024-04-02 13:58 UTC #21 lore raw

PATCH 11/15 mlxsw: pci: Make style change in mlxsw_pci_cq_tasklet()

From: Amit Cohen <[email protected]>

This function will be broken into several functions later. As preparation, reorder variables to reverse xmas tree.

Signed-off-by Amit Cohen <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +2 -2

index 7f059306af5a..7c4b6e66f1fb 100644
@@ -662,9 +662,9 @@ static void mlxsw_pci_cq_tasklet(struct tasklet_struct *t)
 {
 	struct mlxsw_pci_queue *q = from_tasklet(q, t, tasklet);
 	struct mlxsw_pci *mlxsw_pci = q->pci;
+	int credits = q->count >> 1;
+	int items = 0;
 	char *cqe;
-	int items = 0;
-	int credits = q->count >> 1;
 
 	while ((cqe = mlxsw_pci_cq_sw_cqe_get(q))) {
 		u16 wqe_counter = mlxsw_pci_cqe_wqe_counter_get(cqe);

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 11/15

2024-04-03 12:39 UTC #22 lore raw

Re: [PATCH net-next 11/15] mlxsw: pci: Make style change in mlxsw_pci_cq_tasklet()

Quoted 8 lines · Petr Machata

From: Amit Cohen <[email protected]>
This function will be broken into several functions later. As preparation, reorder variables to reverse xmas tree.
Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

Petr Machata

author maintainer

2024-04-02 13:58 UTC #23 lore raw

PATCH 12/15 mlxsw: pci: Break mlxsw_pci_cq_tasklet() into tasklets per queue type

From: Amit Cohen <[email protected]>

Completion queues are used for completions of RDQ or SDQ. Each completion queue is used for one DQ. The first CQs are used for SDQs and the rest are used for RDQs.

Currently, for each CQE (completion queue element), we check 'sr' value (send/receive) to know if it is completion of RDQ or SDQ. Actually, we do not really have to check it, as according to the queue number we know if it handles completions of Rx or Tx.

Break the tasklet into two - one for Rx (RDQ) and one for Tx (SDQ). Then, setup the appropriate tasklet for each queue as part of queue initialization. Use 'sr' value for unlikely case that we get completion with type that we do not expect. Call WARN_ON_ONCE() only after checking the value, to avoid calling this method for each completion.

A next patch set will use NAPI to handle events, then we will have a separate poll method for Rx and Tx. This change is a preparation for NAPI usage.

Signed-off-by Amit Cohen <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 86 +++++++++++++++++++----
 1 file changed, 74 insertions(+), 12 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +74 ~~-12~~

index 7c4b6e66f1fb..1839ab840b35 100644
@@ -35,6 +35,11 @@ enum mlxsw_pci_queue_type {
 
 #define MLXSW_PCI_QUEUE_TYPE_COUNT	4
 
+enum mlxsw_pci_cq_type {
+	MLXSW_PCI_CQ_SDQ,
+	MLXSW_PCI_CQ_RDQ,
+};
+
 static const u16 mlxsw_pci_doorbell_type_offset[] = {
 	MLXSW_PCI_DOORBELL_SDQ_OFFSET,	/* for type MLXSW_PCI_QUEUE_TYPE_SDQ */
 	MLXSW_PCI_DOORBELL_RDQ_OFFSET,	/* for type MLXSW_PCI_QUEUE_TYPE_RDQ */

@@ -658,7 +663,7 @@ static char *mlxsw_pci_cq_sw_cqe_get(struct mlxsw_pci_queue *q)
 	return elem;
 }
 
-static void mlxsw_pci_cq_tasklet(struct tasklet_struct *t)
+static void mlxsw_pci_cq_rx_tasklet(struct tasklet_struct *t)
 {
 	struct mlxsw_pci_queue *q = from_tasklet(q, t, tasklet);
 	struct mlxsw_pci *mlxsw_pci = q->pci;

@@ -671,23 +676,54 @@ static void mlxsw_pci_cq_tasklet(struct tasklet_struct *t)
 		u8 sendq = mlxsw_pci_cqe_sr_get(q->cq.v, cqe);
 		u8 dqn = mlxsw_pci_cqe_dqn_get(q->cq.v, cqe);
 		char ncqe[MLXSW_PCI_CQE_SIZE_MAX];
+		struct mlxsw_pci_queue *rdq;
+
+		if (unlikely(sendq)) {
+			WARN_ON_ONCE(1);
+			continue;
+		}
 
 		memcpy(ncqe, cqe, q->elem_size);
 		mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
 
-		if (sendq) {
-			struct mlxsw_pci_queue *sdq;
+		rdq = mlxsw_pci_rdq_get(mlxsw_pci, dqn);
+		mlxsw_pci_cqe_rdq_handle(mlxsw_pci, rdq,
+					 wqe_counter, q->cq.v, ncqe);
+
+		if (++items == credits)
+			break;
+	}
+
+	mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
+}
 
-			sdq = mlxsw_pci_sdq_get(mlxsw_pci, dqn);
-			mlxsw_pci_cqe_sdq_handle(mlxsw_pci, sdq,
-						 wqe_counter, q->cq.v, ncqe);
-		} else {
-			struct mlxsw_pci_queue *rdq;
+static void mlxsw_pci_cq_tx_tasklet(struct tasklet_struct *t)
+{
+	struct mlxsw_pci_queue *q = from_tasklet(q, t, tasklet);
+	struct mlxsw_pci *mlxsw_pci = q->pci;
+	int credits = q->count >> 1;
+	int items = 0;
+	char *cqe;
 
-			rdq = mlxsw_pci_rdq_get(mlxsw_pci, dqn);
-			mlxsw_pci_cqe_rdq_handle(mlxsw_pci, rdq,
-						 wqe_counter, q->cq.v, ncqe);
+	while ((cqe = mlxsw_pci_cq_sw_cqe_get(q))) {
+		u16 wqe_counter = mlxsw_pci_cqe_wqe_counter_get(cqe);
+		u8 sendq = mlxsw_pci_cqe_sr_get(q->cq.v, cqe);
+		u8 dqn = mlxsw_pci_cqe_dqn_get(q->cq.v, cqe);
+		char ncqe[MLXSW_PCI_CQE_SIZE_MAX];
+		struct mlxsw_pci_queue *sdq;
+
+		if (unlikely(!sendq)) {
+			WARN_ON_ONCE(1);
+			continue;
 		}
+
+		memcpy(ncqe, cqe, q->elem_size);
+		mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
+
+		sdq = mlxsw_pci_sdq_get(mlxsw_pci, dqn);
+		mlxsw_pci_cqe_sdq_handle(mlxsw_pci, sdq,
+					 wqe_counter, q->cq.v, ncqe);
+
 		if (++items == credits)
 			break;
 	}

@@ -695,6 +731,32 @@ static void mlxsw_pci_cq_tasklet(struct tasklet_struct *t)
 	mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
 }
 
+static enum mlxsw_pci_cq_type
+mlxsw_pci_cq_type(const struct mlxsw_pci *mlxsw_pci,
+		  const struct mlxsw_pci_queue *q)
+{
+	/* Each CQ is mapped to one DQ. The first 'num_sdq_cqs' queues are used
+	 * for SDQs and the rest are used for RDQs.
+	 */
+	if (q->num < mlxsw_pci->num_sdq_cqs)
+		return MLXSW_PCI_CQ_SDQ;
+
+	return MLXSW_PCI_CQ_RDQ;
+}
+
+static void mlxsw_pci_cq_tasklet_setup(struct mlxsw_pci_queue *q,
+				       enum mlxsw_pci_cq_type cq_type)
+{
+	switch (cq_type) {
+	case MLXSW_PCI_CQ_SDQ:
+		tasklet_setup(&q->tasklet, mlxsw_pci_cq_tx_tasklet);
+		break;
+	case MLXSW_PCI_CQ_RDQ:
+		tasklet_setup(&q->tasklet, mlxsw_pci_cq_rx_tasklet);
+		break;
+	}
+}
+
 static int mlxsw_pci_cq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 			     struct mlxsw_pci_queue *q)
 {

@@ -727,7 +789,7 @@ static int mlxsw_pci_cq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 	err = mlxsw_cmd_sw2hw_cq(mlxsw_pci->core, mbox, q->num);
 	if (err)
 		return err;
-	tasklet_setup(&q->tasklet, mlxsw_pci_cq_tasklet);
+	mlxsw_pci_cq_tasklet_setup(q, mlxsw_pci_cq_type(mlxsw_pci, q));
 	mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
 	mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
 	return 0;

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 12/15

2024-04-03 12:40 UTC #24 lore raw

Re: [PATCH net-next 12/15] mlxsw: pci: Break mlxsw_pci_cq_tasklet() into tasklets per queue type

Quoted 17 lines · Petr Machata

From: Amit Cohen <[email protected]>
Completion queues are used for completions of RDQ or SDQ. Each completion queue is used for one DQ. The first CQs are used for SDQs and the rest are used for RDQs.
Currently, for each CQE (completion queue element), we check 'sr' value (send/receive) to know if it is completion of RDQ or SDQ. Actually, we do not really have to check it, as according to the queue number we know if it handles completions of Rx or Tx.
Break the tasklet into two - one for Rx (RDQ) and one for Tx (SDQ). Then, setup the appropriate tasklet for each queue as part of queue initialization. Use 'sr' value for unlikely case that we get completion with type that we do not expect. Call WARN_ON_ONCE() only after checking the value, to avoid calling this method for each completion.

Quoted 7 lines

A next patch set will use NAPI to handle events, then we will have a separate poll method for Rx and Tx. This change is a preparation for NAPI usage.
Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

Petr Machata

author maintainer

2024-04-02 13:58 UTC #25 lore raw

PATCH 13/15 mlxsw: pci: Remove mlxsw_pci_sdq_count()

From: Amit Cohen <[email protected]>

The number of SDQs is stored as part of 'mlxsw_pci' structure. In some cases, the driver uses this value and in some cases it calls mlxsw_pci_sdq_count() to get the value. Align the code to use the stored value. This simplifies the code and makes it clearer that the value is always the same. Rename 'mlxsw_pci->num_sdq_cqs' to 'mlxsw_pci->num_sdqs' as now it is used not only in CQ context.

Signed-off-by Amit Cohen <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +7 ~~-12~~

index 1839ab840b35..a7ede97a3bcc 100644
@@ -123,7 +123,7 @@ struct mlxsw_pci {
 	struct mlxsw_bus_info bus_info;
 	const struct pci_device_id *id;
 	enum mlxsw_pci_cqe_v max_cqe_ver; /* Maximal supported CQE version */
-	u8 num_sdq_cqs; /* Number of CQs used for SDQs */
+	u8 num_sdqs; /* Number of SDQs */
 	bool skip_reset;
 };

@@ -188,11 +188,6 @@ static u8 __mlxsw_pci_queue_count(struct mlxsw_pci *mlxsw_pci,
 	return queue_group->count;
 }
 
-static u8 mlxsw_pci_sdq_count(struct mlxsw_pci *mlxsw_pci)
-{
-	return __mlxsw_pci_queue_count(mlxsw_pci, MLXSW_PCI_QUEUE_TYPE_SDQ);
-}
-
 static u8 mlxsw_pci_cq_count(struct mlxsw_pci *mlxsw_pci)
 {
 	return __mlxsw_pci_queue_count(mlxsw_pci, MLXSW_PCI_QUEUE_TYPE_CQ);

@@ -391,7 +386,7 @@ static int mlxsw_pci_rdq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 			      struct mlxsw_pci_queue *q)
 {
 	struct mlxsw_pci_queue_elem_info *elem_info;
-	u8 sdq_count = mlxsw_pci_sdq_count(mlxsw_pci);
+	u8 sdq_count = mlxsw_pci->num_sdqs;
 	int i;
 	int err;

@@ -457,7 +452,7 @@ static void mlxsw_pci_cq_pre_init(struct mlxsw_pci *mlxsw_pci,
 	q->cq.v = mlxsw_pci->max_cqe_ver;
 
 	if (q->cq.v == MLXSW_PCI_CQE_V2 &&
-	    q->num < mlxsw_pci->num_sdq_cqs &&
+	    q->num < mlxsw_pci->num_sdqs &&
 	    !mlxsw_core_sdq_supports_cqe_v2(mlxsw_pci->core))
 		q->cq.v = MLXSW_PCI_CQE_V1;
 }

@@ -735,10 +730,10 @@ static enum mlxsw_pci_cq_type
 mlxsw_pci_cq_type(const struct mlxsw_pci *mlxsw_pci,
 		  const struct mlxsw_pci_queue *q)
 {
-	/* Each CQ is mapped to one DQ. The first 'num_sdq_cqs' queues are used
+	/* Each CQ is mapped to one DQ. The first 'num_sdqs' queues are used
 	 * for SDQs and the rest are used for RDQs.
 	 */
-	if (q->num < mlxsw_pci->num_sdq_cqs)
+	if (q->num < mlxsw_pci->num_sdqs)
 		return MLXSW_PCI_CQ_SDQ;
 
 	return MLXSW_PCI_CQ_RDQ;

@@ -1112,7 +1107,7 @@ static int mlxsw_pci_aqs_init(struct mlxsw_pci *mlxsw_pci, char *mbox)
 		return -EINVAL;
 	}
 
-	mlxsw_pci->num_sdq_cqs = num_sdqs;
+	mlxsw_pci->num_sdqs = num_sdqs;
 
 	err = mlxsw_pci_queue_group_init(mlxsw_pci, mbox, &mlxsw_pci_eq_ops,
 					 MLXSW_PCI_EQS_COUNT);

@@ -1778,7 +1773,7 @@ static struct mlxsw_pci_queue *
 mlxsw_pci_sdq_pick(struct mlxsw_pci *mlxsw_pci,
 		   const struct mlxsw_tx_info *tx_info)
 {
-	u8 ctl_sdq_count = mlxsw_pci_sdq_count(mlxsw_pci) - 1;
+	u8 ctl_sdq_count = mlxsw_pci->num_sdqs - 1;
 	u8 sdqn;
 
 	if (tx_info->is_emad) {

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 13/15

2024-04-03 12:40 UTC #26 lore raw

Re: [PATCH net-next 13/15] mlxsw: pci: Remove mlxsw_pci_sdq_count()

Quoted 9 lines · Petr Machata

From: Amit Cohen <[email protected]>
The number of SDQs is stored as part of 'mlxsw_pci' structure. In some cases, the driver uses this value and in some cases it calls mlxsw_pci_sdq_count() to get the value. Align the code to use the stored value. This simplifies the code and makes it clearer that the value is always the same. Rename 'mlxsw_pci->num_sdq_cqs' to 'mlxsw_pci->num_sdqs' as now it is used not only in CQ context.

Quoted 3 lines

Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

Petr Machata

author maintainer

2024-04-02 13:58 UTC #27 lore raw

PATCH 14/15 mlxsw: pci: Remove mlxsw_pci_cq_count()

From: Amit Cohen <[email protected]>

Currently, for each interrupt we call mlxsw_pci_cq_count() to determine the number of CQs. This call makes additional two function's calls. This can be removed by storing this value as part of structure 'mlxsw_pci', as we already do for number of SDQs. Remove the function and __mlxsw_pci_queue_count() which is now not used and store the value instead.

Signed-off-by Amit Cohen <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +3 ~~-15~~

index a7ede97a3bcc..2148110542cb 100644
@@ -123,6 +123,7 @@ struct mlxsw_pci {
 	struct mlxsw_bus_info bus_info;
 	const struct pci_device_id *id;
 	enum mlxsw_pci_cqe_v max_cqe_ver; /* Maximal supported CQE version */
+	u8 num_cqs; /* Number of CQs */
 	u8 num_sdqs; /* Number of SDQs */
 	bool skip_reset;
 };

@@ -179,20 +180,6 @@ mlxsw_pci_queue_type_group_get(struct mlxsw_pci *mlxsw_pci,
 	return &mlxsw_pci->queues[q_type];
 }
 
-static u8 __mlxsw_pci_queue_count(struct mlxsw_pci *mlxsw_pci,
-				  enum mlxsw_pci_queue_type q_type)
-{
-	struct mlxsw_pci_queue_type_group *queue_group;
-
-	queue_group = mlxsw_pci_queue_type_group_get(mlxsw_pci, q_type);
-	return queue_group->count;
-}
-
-static u8 mlxsw_pci_cq_count(struct mlxsw_pci *mlxsw_pci)
-{
-	return __mlxsw_pci_queue_count(mlxsw_pci, MLXSW_PCI_QUEUE_TYPE_CQ);
-}
-
 static struct mlxsw_pci_queue *
 __mlxsw_pci_queue_get(struct mlxsw_pci *mlxsw_pci,
 		      enum mlxsw_pci_queue_type q_type, u8 q_num)

@@ -850,7 +837,7 @@ static void mlxsw_pci_eq_tasklet(struct tasklet_struct *t)
 	mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
 	mlxsw_pci_queue_doorbell_arm_consumer_ring(mlxsw_pci, q);
 
-	cq_count = mlxsw_pci_cq_count(mlxsw_pci);
+	cq_count = mlxsw_pci->num_cqs;
 	for_each_set_bit(cqn, active_cqns, cq_count) {
 		q = mlxsw_pci_cq_get(mlxsw_pci, cqn);
 		mlxsw_pci_queue_tasklet_schedule(q);

@@ -1107,6 +1094,7 @@ static int mlxsw_pci_aqs_init(struct mlxsw_pci *mlxsw_pci, char *mbox)
 		return -EINVAL;
 	}
 
+	mlxsw_pci->num_cqs = num_cqs;
 	mlxsw_pci->num_sdqs = num_sdqs;
 
 	err = mlxsw_pci_queue_group_init(mlxsw_pci, mbox, &mlxsw_pci_eq_ops,

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 14/15

2024-04-03 12:40 UTC #28 lore raw

Re: [PATCH net-next 14/15] mlxsw: pci: Remove mlxsw_pci_cq_count()

Quoted 9 lines · Petr Machata

From: Amit Cohen <[email protected]>
Currently, for each interrupt we call mlxsw_pci_cq_count() to determine the number of CQs. This call makes additional two function's calls. This can be removed by storing this value as part of structure 'mlxsw_pci', as we already do for number of SDQs. Remove the function and __mlxsw_pci_queue_count() which is now not used and store the value instead.

Quoted 3 lines

Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

Petr Machata

author maintainer

2024-04-02 13:58 UTC #29 lore raw

PATCH 15/15 mlxsw: pci: Store DQ pointer as part of CQ structure

From: Amit Cohen <[email protected]>

Currently, for each completion, we check the number of descriptor queue and take it via mlxsw_pci_{sdq,rdq}_get(). This is inefficient, the DQ should be the same for all the completions in CQ, as each CQ handles only one DQ - SDQ or RDQ. This mapping is handled as part of DQ initialization via mlxsw_cmd_mbox_sw2hw_dq_cq_set().

Instead, as part of DQ initialization, set DQ pointer in the appropriate CQ structure. When we handle completions, warn in case that the DQ number that we expect is different from the number we get in the CQE. Call WARN_ON_ONCE() only after checking the value, to avoid calling this method for each completion.

Signed-off-by Amit Cohen <[email protected]>

---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 41 ++++++++++++++++-------
 1 file changed, 28 insertions(+), 13 deletions(-)

drivers/net/ethernet/mellanox/mlxsw/pci.c +28 ~~-13~~

index 2148110542cb..13fd067c39ed 100644
@@ -86,6 +86,7 @@ struct mlxsw_pci_queue {
 	struct mlxsw_pci *pci;
 	struct {
 		enum mlxsw_pci_cqe_v v;
+		struct mlxsw_pci_queue *dq;
 	} cq;
 };

@@ -194,13 +195,6 @@ static struct mlxsw_pci_queue *mlxsw_pci_sdq_get(struct mlxsw_pci *mlxsw_pci,
 				     MLXSW_PCI_QUEUE_TYPE_SDQ, q_num);
 }
 
-static struct mlxsw_pci_queue *mlxsw_pci_rdq_get(struct mlxsw_pci *mlxsw_pci,
-						 u8 q_num)
-{
-	return __mlxsw_pci_queue_get(mlxsw_pci,
-				     MLXSW_PCI_QUEUE_TYPE_RDQ, q_num);
-}
-
 static struct mlxsw_pci_queue *mlxsw_pci_cq_get(struct mlxsw_pci *mlxsw_pci,
 						u8 q_num)
 {

@@ -265,7 +259,9 @@ static dma_addr_t __mlxsw_pci_queue_page_get(struct mlxsw_pci_queue *q,
 static int mlxsw_pci_sdq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 			      struct mlxsw_pci_queue *q)
 {
+	struct mlxsw_pci_queue *cq;
 	int tclass;
+	u8 cq_num;
 	int lp;
 	int i;
 	int err;

@@ -278,7 +274,8 @@ static int mlxsw_pci_sdq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 						  MLXSW_CMD_MBOX_SW2HW_DQ_SDQ_LP_WQE;
 
 	/* Set CQ of same number of this SDQ. */
-	mlxsw_cmd_mbox_sw2hw_dq_cq_set(mbox, q->num);
+	cq_num = q->num;
+	mlxsw_cmd_mbox_sw2hw_dq_cq_set(mbox, cq_num);
 	mlxsw_cmd_mbox_sw2hw_dq_sdq_lp_set(mbox, lp);
 	mlxsw_cmd_mbox_sw2hw_dq_sdq_tclass_set(mbox, tclass);
 	mlxsw_cmd_mbox_sw2hw_dq_log2_dq_sz_set(mbox, 3); /* 8 pages */

@@ -291,6 +288,9 @@ static int mlxsw_pci_sdq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 	err = mlxsw_cmd_sw2hw_sdq(mlxsw_pci->core, mbox, q->num);
 	if (err)
 		return err;
+
+	cq = mlxsw_pci_cq_get(mlxsw_pci, cq_num);
+	cq->cq.dq = q;
 	mlxsw_pci_queue_doorbell_producer_ring(mlxsw_pci, q);
 	return 0;
 }

@@ -374,6 +374,8 @@ static int mlxsw_pci_rdq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 {
 	struct mlxsw_pci_queue_elem_info *elem_info;
 	u8 sdq_count = mlxsw_pci->num_sdqs;
+	struct mlxsw_pci_queue *cq;
+	u8 cq_num;
 	int i;
 	int err;

@@ -383,7 +385,8 @@ static int mlxsw_pci_rdq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 	/* Set CQ of same number of this RDQ with base
 	 * above SDQ count as the lower ones are assigned to SDQs.
 	 */
-	mlxsw_cmd_mbox_sw2hw_dq_cq_set(mbox, sdq_count + q->num);
+	cq_num = sdq_count + q->num;
+	mlxsw_cmd_mbox_sw2hw_dq_cq_set(mbox, cq_num);
 	mlxsw_cmd_mbox_sw2hw_dq_log2_dq_sz_set(mbox, 3); /* 8 pages */
 	for (i = 0; i < MLXSW_PCI_AQ_PAGES; i++) {
 		dma_addr_t mapaddr = __mlxsw_pci_queue_page_get(q, i);

@@ -395,6 +398,9 @@ static int mlxsw_pci_rdq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 	if (err)
 		return err;
 
+	cq = mlxsw_pci_cq_get(mlxsw_pci, cq_num);
+	cq->cq.dq = q;
+
 	mlxsw_pci_queue_doorbell_producer_ring(mlxsw_pci, q);
 
 	for (i = 0; i < q->count; i++) {

@@ -415,6 +421,7 @@ static int mlxsw_pci_rdq_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 		elem_info = mlxsw_pci_queue_elem_info_get(q, i);
 		mlxsw_pci_rdq_skb_free(mlxsw_pci, elem_info);
 	}
+	cq->cq.dq = NULL;
 	mlxsw_cmd_hw2sw_rdq(mlxsw_pci->core, q->num);
 
 	return err;

@@ -648,6 +655,7 @@ static char *mlxsw_pci_cq_sw_cqe_get(struct mlxsw_pci_queue *q)
 static void mlxsw_pci_cq_rx_tasklet(struct tasklet_struct *t)
 {
 	struct mlxsw_pci_queue *q = from_tasklet(q, t, tasklet);
+	struct mlxsw_pci_queue *rdq = q->cq.dq;
 	struct mlxsw_pci *mlxsw_pci = q->pci;
 	int credits = q->count >> 1;
 	int items = 0;

@@ -658,17 +666,20 @@ static void mlxsw_pci_cq_rx_tasklet(struct tasklet_struct *t)
 		u8 sendq = mlxsw_pci_cqe_sr_get(q->cq.v, cqe);
 		u8 dqn = mlxsw_pci_cqe_dqn_get(q->cq.v, cqe);
 		char ncqe[MLXSW_PCI_CQE_SIZE_MAX];
-		struct mlxsw_pci_queue *rdq;
 
 		if (unlikely(sendq)) {
 			WARN_ON_ONCE(1);
 			continue;
 		}
 
+		if (unlikely(dqn != rdq->num)) {
+			WARN_ON_ONCE(1);
+			continue;
+		}
+
 		memcpy(ncqe, cqe, q->elem_size);
 		mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
 
-		rdq = mlxsw_pci_rdq_get(mlxsw_pci, dqn);
 		mlxsw_pci_cqe_rdq_handle(mlxsw_pci, rdq,
 					 wqe_counter, q->cq.v, ncqe);

@@ -682,6 +693,7 @@ static void mlxsw_pci_cq_rx_tasklet(struct tasklet_struct *t)
 static void mlxsw_pci_cq_tx_tasklet(struct tasklet_struct *t)
 {
 	struct mlxsw_pci_queue *q = from_tasklet(q, t, tasklet);
+	struct mlxsw_pci_queue *sdq = q->cq.dq;
 	struct mlxsw_pci *mlxsw_pci = q->pci;
 	int credits = q->count >> 1;
 	int items = 0;

@@ -692,17 +704,20 @@ static void mlxsw_pci_cq_tx_tasklet(struct tasklet_struct *t)
 		u8 sendq = mlxsw_pci_cqe_sr_get(q->cq.v, cqe);
 		u8 dqn = mlxsw_pci_cqe_dqn_get(q->cq.v, cqe);
 		char ncqe[MLXSW_PCI_CQE_SIZE_MAX];
-		struct mlxsw_pci_queue *sdq;
 
 		if (unlikely(!sendq)) {
 			WARN_ON_ONCE(1);
 			continue;
 		}
 
+		if (unlikely(dqn != sdq->num)) {
+			WARN_ON_ONCE(1);
+			continue;
+		}
+
 		memcpy(ncqe, cqe, q->elem_size);
 		mlxsw_pci_queue_doorbell_consumer_ring(mlxsw_pci, q);
 
-		sdq = mlxsw_pci_sdq_get(mlxsw_pci, dqn);
 		mlxsw_pci_cqe_sdq_handle(mlxsw_pci, sdq,
 					 wqe_counter, q->cq.v, ncqe);

Signature · 1 line

2.43.0

Simon Horman

reviewer Reviewed-by on v1 15/15

2024-04-03 12:41 UTC #30 lore raw

Re: [PATCH net-next 15/15] mlxsw: pci: Store DQ pointer as part of CQ structure

Quoted 14 lines · Petr Machata

From: Amit Cohen <[email protected]>
Currently, for each completion, we check the number of descriptor queue and take it via mlxsw_pci_{sdq,rdq}_get(). This is inefficient, the DQ should be the same for all the completions in CQ, as each CQ handles only one DQ - SDQ or RDQ. This mapping is handled as part of DQ initialization via mlxsw_cmd_mbox_sw2hw_dq_cq_set().
Instead, as part of DQ initialization, set DQ pointer in the appropriate CQ structure. When we handle completions, warn in case that the DQ number that we expect is different from the number we get in the CQE. Call WARN_ON_ONCE() only after checking the value, to avoid calling this method for each completion.

Quoted 3 lines

Signed-off-by Amit Cohen <[email protected]>
Reviewed-by Ido Schimmel <[email protected]>
Signed-off-by Petr Machata <[email protected]>

[email protected]

CI Applied

2024-04-04 03:50 UTC #31 lore raw