From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f193.google.com (mail-wr0-f193.google.com [209.85.128.193]) by dpdk.org (Postfix) with ESMTP id A64041B5F0 for ; Thu, 2 Nov 2017 14:43:31 +0100 (CET) Received: by mail-wr0-f193.google.com with SMTP id l8so5048388wre.12 for ; Thu, 02 Nov 2017 06:43:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=TzOHniOCaCKI2qv+Ck/wSRktUahRM0YfpF+Wc5FkK8A=; b=mIVH0E2prOD2oc4CI4J9UAWtrjbaw6nY02uEq/blayl/nsYUlQskwVZ2kasX+zj3/X swI7S28H0FNKH+dhZV+fDtOYufnUfYG0QD8Hlatsx+IoThYDUA6es2EIaUnmG23JLkNf fkHrEMvEWwPbRffruCbCOdxnyh5UdHmsDaaaDh6RhVvMYnxyhR+bh/dCSmDz8cgo2Ct3 59RWWRtb3ERlRQGNDTcGPkbtzI5paxiMDGjDYTqbkixeJnlR+0nJUA5cpGe06Nj4/PTn gNXpRtKEyPTn6NiXc8dWVWb6Qf/mDqU1OVjcZf2xuH84GgeJE7ifseLNa8H2YZ6cEh15 BRkg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=TzOHniOCaCKI2qv+Ck/wSRktUahRM0YfpF+Wc5FkK8A=; b=U4a/M8GKjoeXjonVePlj/saoGBF/QbWasaYQpMg2RRKLZsqfcyCgA1ri+AgKNdiy/J 3kC60EvoejUzN/cgrVsolean/+PAE+izMSsNRwaFN5CaXUwntzCbO3Uo7BQNH2ydLxRh 2Y+9+2EsJDP3y6zz0/eIAno/GhrzQyTvLTcISveGa67pmnCfK88v8zqZ+IsGkISoUVWB OQ5YVsM6koufuQqv+FL65kZloL2GpXivtX9uNymC9TwLH6HW/wl8vAp0ObhWSnnbNQMU gnbgc4i2nRQObiiOXACEJ/mEpb0BhdnpRgZnHmR9PKhDqDe/fV2fIaZBy84p6w6IqI7w NhRQ== X-Gm-Message-State: AMCzsaWz8V7pngVBFNM1X1UVwe2rWyjr2tsuD96Mw8DL1m6iO7TAnFZS 3wGS60gu7lqNvBB9rj0LkAwu4A== X-Google-Smtp-Source: ABhQp+QX1GSFLCv3KcK7DR/5GT2qzXWfklf7Vzm9TciTrmtvix5iWkp7vj+444fablChMBIhFB+kpg== X-Received: by 10.223.147.39 with SMTP id 36mr2797081wro.175.1509630211265; Thu, 02 Nov 2017 06:43:31 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id k69sm4826395wmg.45.2017.11.02.06.43.30 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 02 Nov 2017 06:43:30 -0700 (PDT) Date: Thu, 2 Nov 2017 14:43:19 +0100 From: Adrien Mazarguil To: Matan Azrad Cc: dev@dpdk.org, Ophir Munk Message-ID: <20171102134319.GF24849@6wind.com> References: <1509358049-18854-1-git-send-email-matan@mellanox.com> <1509474093-31388-1-git-send-email-matan@mellanox.com> <1509474093-31388-8-git-send-email-matan@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1509474093-31388-8-git-send-email-matan@mellanox.com> Subject: Re: [dpdk-dev] [PATCH v4 7/8] net/mlx4: fix HW memory optimizations careless X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Nov 2017 13:43:31 -0000 On Tue, Oct 31, 2017 at 06:21:32PM +0000, Matan Azrad wrote: > Volatilize all Rx/Tx HW negotiation memories to be sure no compiler > optimization prevents either load or store commands. > > Fixes: c3c977bbecbd ("net/mlx4: add Tx bypassing Verbs") > Fixes: 9f57340a8087 ("net/mlx4: restore Rx offloads") > Fixes: 6681b845034c ("net/mlx4: add Rx bypassing Verbs") > Fixes: 62e96ffb93ad ("net/mlx4: fix no Rx interrupts") > > Signed-off-by: Matan Azrad Since this should fix all remaining concerns: Acked-by: Adrien Mazarguil A few minor comments below. > --- > drivers/net/mlx4/mlx4_prm.h | 18 ++++++------ > drivers/net/mlx4/mlx4_rxtx.c | 67 ++++++++++++++++++++++++-------------------- > 2 files changed, 45 insertions(+), 40 deletions(-) > > diff --git a/drivers/net/mlx4/mlx4_prm.h b/drivers/net/mlx4/mlx4_prm.h > index b0fd982..6d10b4e 100644 > --- a/drivers/net/mlx4/mlx4_prm.h > +++ b/drivers/net/mlx4/mlx4_prm.h > @@ -80,14 +80,14 @@ enum { > > /* Send queue information. */ > struct mlx4_sq { > - uint8_t *buf; /**< SQ buffer. */ > - uint8_t *eob; /**< End of SQ buffer */ > + volatile uint8_t *buf; /**< SQ buffer. */ > + volatile uint8_t *eob; /**< End of SQ buffer */ > uint32_t head; /**< SQ head counter in units of TXBBS. */ > uint32_t tail; /**< SQ tail counter in units of TXBBS. */ > uint32_t txbb_cnt; /**< Num of WQEBB in the Q (should be ^2). */ > uint32_t txbb_cnt_mask; /**< txbbs_cnt mask (txbb_cnt is ^2). */ > uint32_t headroom_txbbs; /**< Num of txbbs that should be kept free. */ > - uint32_t *db; /**< Pointer to the doorbell. */ > + volatile uint32_t *db; /**< Pointer to the doorbell. */ > uint32_t doorbell_qpn; /**< qp number to write to the doorbell. */ > }; > > @@ -101,10 +101,10 @@ struct mlx4_sq { > /* Completion queue information. */ > struct mlx4_cq { > void *cq_uar; /**< CQ user access region. */ I'm curious why UAR isn't volatile as well? > - void *cq_db_reg; /**< CQ doorbell register. */ > - uint32_t *set_ci_db; /**< Pointer to the completion queue doorbell. */ > - uint32_t *arm_db; /**< Pointer to doorbell for arming Rx events. */ > - uint8_t *buf; /**< Pointer to the completion queue buffer. */ > + volatile void *cq_db_reg; /**< CQ doorbell register. */ > + volatile uint32_t *set_ci_db; /**< Pointer to the CQ doorbell. */ > + volatile uint32_t *arm_db; /**< Arming Rx events doorbell. */ > + volatile uint8_t *buf; /**< Pointer to the completion queue buffer. */ > uint32_t cqe_cnt; /**< Number of entries in the queue. */ > uint32_t cqe_64:1; /**< CQ entry size is 64 bytes. */ > uint32_t cons_index; /**< Last queue entry that was handled. */ > @@ -128,10 +128,10 @@ struct mlx4_cq { > * @return > * Pointer to CQE entry. > */ > -static inline struct mlx4_cqe * > +static inline volatile struct mlx4_cqe * > mlx4_get_cqe(struct mlx4_cq *cq, uint32_t index) > { > - return (struct mlx4_cqe *)(cq->buf + > + return (volatile struct mlx4_cqe *)(cq->buf + > ((index & (cq->cqe_cnt - 1)) << > (5 + cq->cqe_64)) + > (cq->cqe_64 << 5)); > diff --git a/drivers/net/mlx4/mlx4_rxtx.c b/drivers/net/mlx4/mlx4_rxtx.c > index 176000f..bd6d888 100644 > --- a/drivers/net/mlx4/mlx4_rxtx.c > +++ b/drivers/net/mlx4/mlx4_rxtx.c > @@ -70,7 +70,7 @@ > * DWORD (32 byte) of a TXBB. > */ > struct pv { > - struct mlx4_wqe_data_seg *dseg; > + volatile struct mlx4_wqe_data_seg *dseg; > uint32_t val; > }; > > @@ -98,14 +98,15 @@ struct pv { > { > uint32_t stamp = rte_cpu_to_be_32(MLX4_SQ_STAMP_VAL | > (!!owner << MLX4_SQ_STAMP_SHIFT)); > - uint8_t *wqe = mlx4_get_send_wqe(sq, (index & sq->txbb_cnt_mask)); > - uint32_t *ptr = (uint32_t *)wqe; > + volatile uint8_t *wqe = mlx4_get_send_wqe(sq, > + (index & sq->txbb_cnt_mask)); > + volatile uint32_t *ptr = (volatile uint32_t *)wqe; > int i; > int txbbs_size; > int num_txbbs; > > /* Extract the size from the control segment of the WQE. */ > - num_txbbs = MLX4_SIZE_TO_TXBBS((((struct mlx4_wqe_ctrl_seg *) > + num_txbbs = MLX4_SIZE_TO_TXBBS((((volatile struct mlx4_wqe_ctrl_seg *) > wqe)->fence_size & 0x3f) << 4); > txbbs_size = num_txbbs * MLX4_TXBB_SIZE; > /* Optimize the common case when there is no wrap-around. */ > @@ -120,8 +121,8 @@ struct pv { > for (i = 0; i < txbbs_size; i += MLX4_SQ_STAMP_STRIDE) { > *ptr = stamp; > ptr += MLX4_SQ_STAMP_DWORDS; > - if ((uint8_t *)ptr >= sq->eob) { > - ptr = (uint32_t *)sq->buf; > + if ((volatile uint8_t *)ptr >= sq->eob) { > + ptr = (volatile uint32_t *)sq->buf; > stamp ^= RTE_BE32(0x80000000); > } > } > @@ -150,7 +151,7 @@ struct pv { > unsigned int elts_comp = txq->elts_comp; > unsigned int elts_tail = txq->elts_tail; > struct mlx4_cq *cq = &txq->mcq; > - struct mlx4_cqe *cqe; > + volatile struct mlx4_cqe *cqe; > uint32_t cons_index = cq->cons_index; > uint16_t new_index; > uint16_t nr_txbbs = 0; > @@ -161,7 +162,7 @@ struct pv { > * reported by them. > */ > do { > - cqe = (struct mlx4_cqe *)mlx4_get_cqe(cq, cons_index); > + cqe = (volatile struct mlx4_cqe *)mlx4_get_cqe(cq, cons_index); > if (unlikely(!!(cqe->owner_sr_opcode & MLX4_CQE_OWNER_MASK) ^ > !!(cons_index & cq->cqe_cnt))) > break; > @@ -172,8 +173,8 @@ struct pv { > #ifndef NDEBUG > if (unlikely((cqe->owner_sr_opcode & MLX4_CQE_OPCODE_MASK) == > MLX4_CQE_OPCODE_ERROR)) { > - struct mlx4_err_cqe *cqe_err = > - (struct mlx4_err_cqe *)cqe; > + volatile struct mlx4_err_cqe *cqe_err = > + (volatile struct mlx4_err_cqe *)cqe; > ERROR("%p CQE error - vendor syndrome: 0x%x" > " syndrome: 0x%x\n", > (void *)txq, cqe_err->vendor_err, > @@ -240,15 +241,15 @@ struct pv { > > static int > mlx4_tx_burst_segs(struct rte_mbuf *buf, struct txq *txq, > - struct mlx4_wqe_ctrl_seg **pctrl) > + volatile struct mlx4_wqe_ctrl_seg **pctrl) Looks like an indentation issue here. -- Adrien Mazarguil 6WIND