[PATCH rdma-core 0/2] mlx5 DV fixes

[PATCH rdma-core 0/2] mlx5 DV fixes

[Yishai Hadas]

This series fixes 2 issues in the mlx5 DV flow:
Patch #1: Rename arm_db that was lastly added to match content.
Patch #2: Fix compile warning.

pull request was sent:
https://github.com/linux-rdma/rdma-core/pull/202

Leon Romanovsky (2):
  mlx5: Rename arm_db pointer to be cq_uar to better describe the
    content
  mlx5: Convert explicitly to signed char

 providers/mlx5/man/mlx5dv_init_obj.3 | 2 +-
 providers/mlx5/mlx5.c                | 4 ++--
 providers/mlx5/mlx5dv.h              | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-core 1/2] mlx5: Rename arm_db pointer to be cq_uar to better describe the content

[Yishai Hadas]

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The arm_db field which was renamed from uar in this rdma-core release is
actually cq_uar.

Change its name before the rdma-core library is released and update man
page accordingly.

Fixes: 9b713bd703e9 ("mlx5: Fix ABI break from revising the UAR pointer")
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 providers/mlx5/man/mlx5dv_init_obj.3 | 2 +-
 providers/mlx5/mlx5.c                | 4 ++--
 providers/mlx5/mlx5dv.h              | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/providers/mlx5/man/mlx5dv_init_obj.3 b/providers/mlx5/man/mlx5dv_init_obj.3
index 2468407..60d224a 100644
--- a/providers/mlx5/man/mlx5dv_init_obj.3
+++ b/providers/mlx5/man/mlx5dv_init_obj.3
@@ -52,7 +52,7 @@ void                    *buf;
 uint32_t                *dbrec;
 uint32_t                cqe_cnt;
 uint32_t                cqe_size;
-void                    *uar;
+void                    *cq_uar;
 uint32_t                cqn;
 uint64_t                comp_mask;
 .in -8
diff --git a/providers/mlx5/mlx5.c b/providers/mlx5/mlx5.c
index c949c0f..19e2aef 100644
--- a/providers/mlx5/mlx5.c
+++ b/providers/mlx5/mlx5.c
@@ -680,7 +680,7 @@ static int mlx5dv_get_cq(struct ibv_cq *cq_in,
 	cq_out->cqe_size  = mcq->cqe_sz;
 	cq_out->buf       = mcq->active_buf->buf;
 	cq_out->dbrec     = mcq->dbrec;
-	cq_out->arm_db	  = mctx->uar[0];
+	cq_out->cq_uar	  = mctx->uar[0];
 
 	mcq->flags	 |= MLX5_CQ_FLAGS_DV_OWNED;
 
@@ -747,7 +747,7 @@ COMPAT_SYMVER_FUNC(mlx5dv_init_obj, 1_0, "MLX5_1.0",
 		/* ABI version 1.0 returns the void ** in this memory
 		 * location
 		 */
-		obj->cq.out->arm_db = to_mctx(obj->cq.in->context)->uar;
+		obj->cq.out->cq_uar = to_mctx(obj->cq.in->context)->uar;
 	}
 	return ret;
 }
diff --git a/providers/mlx5/mlx5dv.h b/providers/mlx5/mlx5dv.h
index 1a2e257..5d11625 100644
--- a/providers/mlx5/mlx5dv.h
+++ b/providers/mlx5/mlx5dv.h
@@ -129,7 +129,7 @@ struct mlx5dv_cq {
 	__be32			*dbrec;
 	uint32_t		cqe_cnt;
 	uint32_t		cqe_size;
-	void			*arm_db;
+	void			*cq_uar;
 	uint32_t		cqn;
 	uint64_t		comp_mask;
 };
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-core 2/2] mlx5: Convert explicitly to signed char

[Yishai Hadas]

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The 0x80 value is a canonical way to mark as not-used in intrinsics
function calls.

However the gcc compiler is not aware of this convention and compilation
with -Werror=overflow option will cause to compilation failure, because
_mm_set_epi8() call expects chars as an input.

In order to avoid it, we will convert 0x80 to be -128.

Fixes: 00c91653ef9a ("mlx5: Add WQE segments implementation")
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 providers/mlx5/mlx5dv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/providers/mlx5/mlx5dv.h b/providers/mlx5/mlx5dv.h
index 5d11625..e9efcf5 100644
--- a/providers/mlx5/mlx5dv.h
+++ b/providers/mlx5/mlx5dv.h
@@ -499,7 +499,7 @@ void mlx5dv_x86_set_ctrl_seg(struct mlx5_wqe_ctrl_seg *seg, uint16_t pi,
 				     (signature << 24) | (opcode << 16) | (opmod << 8) | fm_ce_se);
 	__m128i mask = _mm_set_epi8(15, 14, 13, 12,	/* immediate */
 				     0,			/* signal/fence_mode */
-				     0x80, 0x80,	/* reserved */
+				     -128, -128,	/* reserved */
 				     3,			/* signature */
 				     6,			/* data size */
 				     8, 9, 10,		/* QP num */
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-core 2/2] mlx5: Convert explicitly to signed char

[Jason Gunthorpe]

On Wed, Aug 30, 2017 at 05:19:24PM +0300, Yishai Hadas wrote:
> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> The 0x80 value is a canonical way to mark as not-used in intrinsics
> function calls.

This commit message is not good, try:

_mm_shuffle_epi8 requires 0x80 to set the output byte to zero, but
_mm_set_epi8() accepts char. If gcc is compiling in a configuration
with a signed char then it can produce a -Werror=overflow warning.

And this fix is wrong, since it just moves the warning to
configurations that have an unsigned char. (eg -funsigned-char)

It is really broken that the _mm_set_epi uses a char as input :|

I think you need to do this instead:

#include <limits.h>
#if CHAR_MIN < 0
#define SHUFFLE_IGNORE  -128
#else
#define SHUFFLE_IGNORE  0x80
#endif

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-core 2/2] mlx5: Convert explicitly to signed char

[Leon Romanovsky]

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

On Wed, Aug 30, 2017 at 10:49:52AM -0600, Jason Gunthorpe wrote:
> On Wed, Aug 30, 2017 at 05:19:24PM +0300, Yishai Hadas wrote:
> > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > The 0x80 value is a canonical way to mark as not-used in intrinsics
> > function calls.
>
> This commit message is not good, try:
>
> _mm_shuffle_epi8 requires 0x80 to set the output byte to zero, but
> _mm_set_epi8() accepts char. If gcc is compiling in a configuration
> with a signed char then it can produce a -Werror=overflow warning.
>
> And this fix is wrong, since it just moves the warning to
> configurations that have an unsigned char. (eg -funsigned-char)

I have gut feeling that with such option "-funsigned-char" half of the
world will break.

>
> It is really broken that the _mm_set_epi uses a char as input :|
>
> I think you need to do this instead:
>
> #include <limits.h>
> #if CHAR_MIN < 0
> #define SHUFFLE_IGNORE  -128
> #else
> #define SHUFFLE_IGNORE  0x80
> #endif
>
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH rdma-core 2/2] mlx5: Convert explicitly to signed char

[Jason Gunthorpe]

On Wed, Aug 30, 2017 at 08:44:02PM +0300, Leon Romanovsky wrote:
> On Wed, Aug 30, 2017 at 10:49:52AM -0600, Jason Gunthorpe wrote:
> > On Wed, Aug 30, 2017 at 05:19:24PM +0300, Yishai Hadas wrote:
> > > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > >
> > > The 0x80 value is a canonical way to mark as not-used in intrinsics
> > > function calls.
> >
> > This commit message is not good, try:
> >
> > _mm_shuffle_epi8 requires 0x80 to set the output byte to zero, but
> > _mm_set_epi8() accepts char. If gcc is compiling in a configuration
> > with a signed char then it can produce a -Werror=overflow warning.
> >
> > And this fix is wrong, since it just moves the warning to
> > configurations that have an unsigned char. (eg -funsigned-char)
> 
> I have gut feeling that with such option "-funsigned-char" half of the
> world will break.

Nope, some arches default to it on. aarch64 for instance.

This is why we have uint8_t, and using 'char' for anything other than
a string character is very wrong.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html