<snip> > > > > > Hi, Honnappa > Hi Slava, thanks for the feedback. > > > > > We discussed the barrier here: > > http://patches.dpdk.org/project/dpdk/patch/20210606164948.35997-1- > > [email protected]/ > Yes, I have changed the patch according to the discussion. i.e. barrier is > needed, but different (inner sharable domain) barrier is required. > > > > > (BTW, it is good practice to keep the reference to previous patch > > versions below Commit Message of the next ones). > > > > This barrier is not about compiler ordering, it is about external HW > > agent memory action completions. > > So, I'm not sure the rte_atomic_thread_fence() is safe for x86 - patch > > impacts > > x86 as well. > The earlier barrier 'rte_io_rmb()', resolves to a compiler barrier on x86 [1]. > The rte_atomic_thread_fence(__ATOMIC_ACQUIRE) on x86 also acts as a > compiler barrier. So, there is no change for x86. > > > [1] > https://github.com/DPDK/dpdk/blob/main/lib/eal/x86/include/rte_atomic.h# > L80 Hi Slava, any more comments on this?
> > > > > With best regards, > > Slava > > > > > -----Original Message----- > > > From: Honnappa Nagarahalli <[email protected]> > > > Sent: Tuesday, August 30, 2022 23:01 > > > To: [email protected]; [email protected]; > > [email protected]; > > > Matan Azrad <[email protected]>; Shahaf Shuler > <[email protected]>; > > > Slava Ovsiienko <[email protected]> > > > Cc: [email protected]; Matan Azrad <[email protected]>; [email protected] > > > Subject: [PATCH v2] net/mlx5: use just sufficient barrier for Arm > > > platforms > > > > > > cqe->op_own indicates if the CQE is owned by the NIC. The rest of > > > the fields in CQE should be read only after op_own is read. On Arm > > > platforms using "dmb ishld" is sufficient to enforce this. > > > > > > Fixes: 88c0733535d6 ("net/mlx5: extend Rx completion with error > > > handling") > > > Cc: [email protected] > > > Cc: [email protected] > > > > > > Signed-off-by: Honnappa Nagarahalli <[email protected]> > > > Reviewed-by: Ruifeng Wang <[email protected]> > > > --- > > > drivers/common/mlx5/mlx5_common.h | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/common/mlx5/mlx5_common.h > > > b/drivers/common/mlx5/mlx5_common.h > > > index 5028a05b49..ac2e85b15f 100644 > > > --- a/drivers/common/mlx5/mlx5_common.h > > > +++ b/drivers/common/mlx5/mlx5_common.h > > > @@ -195,7 +195,11 @@ check_cqe(volatile struct mlx5_cqe *cqe, const > > > uint16_t cqes_n, > > > > > > if (unlikely((op_owner != (!!(idx))) || (op_code == > > > MLX5_CQE_INVALID))) > > > return MLX5_CQE_STATUS_HW_OWN; > > > - rte_io_rmb(); > > > + /* Prevent speculative reading of other fields in CQE until > > > + * CQE is valid. > > > + */ > > > + rte_atomic_thread_fence(__ATOMIC_ACQUIRE); > > > + > > > if (unlikely(op_code == MLX5_CQE_RESP_ERR || > > > op_code == MLX5_CQE_REQ_ERR)) > > > return MLX5_CQE_STATUS_ERR; > > > -- > > > 2.17.1
