vdpa/mlx5: fix completion queue arming

Message ID 1582028721-637-1-git-send-email-matan@mellanox.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series vdpa/mlx5: fix completion queue arming |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/travis-robot warning Travis build: failed
ci/Intel-compilation fail apply issues

Commit Message

Matan Azrad Feb. 18, 2020, 12:25 p.m. UTC
  The mlx5 vDPA driver manages QP and CQ in order to forward the HW event
to the guest by the callfd file descriptor for each virtq.

The driver arms the CQ for the next CQE index that should be
completed by the HW in order to create completion event.

In the SW completion event handler, the driver arms the CQ again for the
next index,

The CQE index in the CQ doorbell and in the CQ doorbell record was
masked incorrectly with the CQ size mask while it should be masked only
with 0xFFFFFF mask.

Remove the CQ size mask, stay only with 0xFFFFFF mask.

Fixes: 8395927cdfaf ("vdpa/mlx5: prepare HW queues")

Signed-off-by: Matan Azrad <matan@mellanox.com>
---
 drivers/vdpa/mlx5/mlx5_vdpa_event.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)
  

Comments

Slava Ovsiienko Feb. 19, 2020, 9:15 a.m. UTC | #1
> -----Original Message-----
> From: Matan Azrad <matan@mellanox.com>
> Sent: Tuesday, February 18, 2020 14:25
> To: dev@dpdk.org
> Cc: Slava Ovsiienko <viacheslavo@mellanox.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>
> Subject: [PATCH] vdpa/mlx5: fix completion queue arming
> 
> The mlx5 vDPA driver manages QP and CQ in order to forward the HW event
> to the guest by the callfd file descriptor for each virtq.
> 
> The driver arms the CQ for the next CQE index that should be completed by
> the HW in order to create completion event.
> 
> In the SW completion event handler, the driver arms the CQ again for the
> next index,
> 
> The CQE index in the CQ doorbell and in the CQ doorbell record was masked
> incorrectly with the CQ size mask while it should be masked only with
> 0xFFFFFF mask.
> 
> Remove the CQ size mask, stay only with 0xFFFFFF mask.
> 
> Fixes: 8395927cdfaf ("vdpa/mlx5: prepare HW queues")
> 
> Signed-off-by: Matan Azrad <matan@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

> ---
>  drivers/vdpa/mlx5/mlx5_vdpa_event.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/vdpa/mlx5/mlx5_vdpa_event.c
> b/drivers/vdpa/mlx5/mlx5_vdpa_event.c
> index c50e58e..17fd9dd 100644
> --- a/drivers/vdpa/mlx5/mlx5_vdpa_event.c
> +++ b/drivers/vdpa/mlx5/mlx5_vdpa_event.c
> @@ -81,9 +81,8 @@
>  static inline void
>  mlx5_vdpa_cq_arm(struct mlx5_vdpa_priv *priv, struct mlx5_vdpa_cq *cq)  {
> -	const unsigned int cqe_mask = (1 << cq->log_desc_n) - 1;
>  	uint32_t arm_sn = cq->arm_sn << MLX5_CQ_SQN_OFFSET;
> -	uint32_t cq_ci = cq->cq_ci & MLX5_CI_MASK & cqe_mask;
> +	uint32_t cq_ci = cq->cq_ci & MLX5_CI_MASK;
>  	uint32_t doorbell_hi = arm_sn | MLX5_CQ_DBR_CMD_ALL | cq_ci;
>  	uint64_t doorbell = ((uint64_t)doorbell_hi << 32) | cq->cq->id;
>  	uint64_t db_be = rte_cpu_to_be_64(doorbell); @@ -182,15 +181,15
> @@  {
>  	struct mlx5_vdpa_event_qp *eqp =
>  				container_of(cq, struct mlx5_vdpa_event_qp,
> cq);
> -	const unsigned int cqe_size = 1 << cq->log_desc_n;
> -	const unsigned int cqe_mask = cqe_size - 1;
> +	const unsigned int cq_size = 1 << cq->log_desc_n;
> +	const unsigned int cq_mask = cq_size - 1;
>  	int ret;
> 
>  	do {
>  		volatile struct mlx5_cqe *cqe = cq->cqes + (cq->cq_ci &
> -							    cqe_mask);
> +							    cq_mask);
> 
> -		ret = check_cqe(cqe, cqe_size, cq->cq_ci);
> +		ret = check_cqe(cqe, cq_size, cq->cq_ci);
>  		switch (ret) {
>  		case MLX5_CQE_STATUS_ERR:
>  			cq->errors++;
> @@ -208,7 +207,7 @@
>  	cq->db_rec[0] = rte_cpu_to_be_32(cq->cq_ci);
>  	rte_io_wmb();
>  	/* Ring SW QP doorbell record. */
> -	eqp->db_rec[0] = rte_cpu_to_be_32(cq->cq_ci + cqe_size);
> +	eqp->db_rec[0] = rte_cpu_to_be_32(cq->cq_ci + cq_size);
>  }
> 
>  static void
> --
> 1.8.3.1
  
Maxime Coquelin Feb. 19, 2020, 10:02 a.m. UTC | #2
On 2/18/20 1:25 PM, Matan Azrad wrote:
> The mlx5 vDPA driver manages QP and CQ in order to forward the HW event
> to the guest by the callfd file descriptor for each virtq.
> 
> The driver arms the CQ for the next CQE index that should be
> completed by the HW in order to create completion event.
> 
> In the SW completion event handler, the driver arms the CQ again for the
> next index,
> 
> The CQE index in the CQ doorbell and in the CQ doorbell record was
> masked incorrectly with the CQ size mask while it should be masked only
> with 0xFFFFFF mask.
> 
> Remove the CQ size mask, stay only with 0xFFFFFF mask.
> 
> Fixes: 8395927cdfaf ("vdpa/mlx5: prepare HW queues")
> 
> Signed-off-by: Matan Azrad <matan@mellanox.com>
> ---
>  drivers/vdpa/mlx5/mlx5_vdpa_event.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)

Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>
  
Maxime Coquelin Feb. 19, 2020, 10:23 a.m. UTC | #3
On 2/18/20 1:25 PM, Matan Azrad wrote:
> The mlx5 vDPA driver manages QP and CQ in order to forward the HW event
> to the guest by the callfd file descriptor for each virtq.
> 
> The driver arms the CQ for the next CQE index that should be
> completed by the HW in order to create completion event.
> 
> In the SW completion event handler, the driver arms the CQ again for the
> next index,
> 
> The CQE index in the CQ doorbell and in the CQ doorbell record was
> masked incorrectly with the CQ size mask while it should be masked only
> with 0xFFFFFF mask.
> 
> Remove the CQ size mask, stay only with 0xFFFFFF mask.
> 
> Fixes: 8395927cdfaf ("vdpa/mlx5: prepare HW queues")
> 
> Signed-off-by: Matan Azrad <matan@mellanox.com>
> ---
>  drivers/vdpa/mlx5/mlx5_vdpa_event.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)

Applied to dpdk-next-virtio/master

Thanks,
Maxime
  

Patch

diff --git a/drivers/vdpa/mlx5/mlx5_vdpa_event.c b/drivers/vdpa/mlx5/mlx5_vdpa_event.c
index c50e58e..17fd9dd 100644
--- a/drivers/vdpa/mlx5/mlx5_vdpa_event.c
+++ b/drivers/vdpa/mlx5/mlx5_vdpa_event.c
@@ -81,9 +81,8 @@ 
 static inline void
 mlx5_vdpa_cq_arm(struct mlx5_vdpa_priv *priv, struct mlx5_vdpa_cq *cq)
 {
-	const unsigned int cqe_mask = (1 << cq->log_desc_n) - 1;
 	uint32_t arm_sn = cq->arm_sn << MLX5_CQ_SQN_OFFSET;
-	uint32_t cq_ci = cq->cq_ci & MLX5_CI_MASK & cqe_mask;
+	uint32_t cq_ci = cq->cq_ci & MLX5_CI_MASK;
 	uint32_t doorbell_hi = arm_sn | MLX5_CQ_DBR_CMD_ALL | cq_ci;
 	uint64_t doorbell = ((uint64_t)doorbell_hi << 32) | cq->cq->id;
 	uint64_t db_be = rte_cpu_to_be_64(doorbell);
@@ -182,15 +181,15 @@ 
 {
 	struct mlx5_vdpa_event_qp *eqp =
 				container_of(cq, struct mlx5_vdpa_event_qp, cq);
-	const unsigned int cqe_size = 1 << cq->log_desc_n;
-	const unsigned int cqe_mask = cqe_size - 1;
+	const unsigned int cq_size = 1 << cq->log_desc_n;
+	const unsigned int cq_mask = cq_size - 1;
 	int ret;
 
 	do {
 		volatile struct mlx5_cqe *cqe = cq->cqes + (cq->cq_ci &
-							    cqe_mask);
+							    cq_mask);
 
-		ret = check_cqe(cqe, cqe_size, cq->cq_ci);
+		ret = check_cqe(cqe, cq_size, cq->cq_ci);
 		switch (ret) {
 		case MLX5_CQE_STATUS_ERR:
 			cq->errors++;
@@ -208,7 +207,7 @@ 
 	cq->db_rec[0] = rte_cpu_to_be_32(cq->cq_ci);
 	rte_io_wmb();
 	/* Ring SW QP doorbell record. */
-	eqp->db_rec[0] = rte_cpu_to_be_32(cq->cq_ci + cqe_size);
+	eqp->db_rec[0] = rte_cpu_to_be_32(cq->cq_ci + cq_size);
 }
 
 static void