| Message ID | 20170823160816.GA11188@obsidianresearch.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Aug 23, 2017 at 10:08:16AM -0600, Jason Gunthorpe wrote: > On Wed, Aug 23, 2017 at 03:13:38PM +0300, Yishai Hadas wrote: > > On 8/22/2017 7:30 PM, Jason Gunthorpe wrote: > > >On Tue, Aug 22, 2017 at 11:36:42AM +0300, Yishai Hadas wrote: > > >>On 8/21/2017 6:22 PM, Jason Gunthorpe wrote: > > >>>Is there existing code out there that uses cq_out->uar and works > > >>>properly today? Yes or No? > > >> > > >>No, only this fix enables that to work properly. > > > > >Particularly since a patch to do this with proper compatibility > > >exists, and there is no reason at all to take a shortcut. > > > > > > > We are not looking for a shortcut but for a solution that will match > > majority users if not all. > > Here is the patch that takes care of everything properly. It is PR > 188. > > Please be more careful with the ABI requirements in the future. > > From 32ef444e45006112ff759106dcdee2b99999501a Mon Sep 17 00:00:00 2001 > From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > Date: Wed, 23 Aug 2017 10:05:24 -0600 > Subject: [PATCH] mlx5: Fix ABI break from revising the UAR pointer > > Provide two implementations of mlx5dv_init_obj, one that has the > historical behaviour that has existed until now of returning the > void **uar and a new version that returns the 'void *' version > renamed to arb_db. arb_db -> arm_db > > Apps that use this feature must refer to it as arb_db, they will not > compile on pre-rdma-core 15 releases, and they will not dynamically > link to old versions either. This provides a sane level of safety for > the end users of this library. > > Fixes: c6e3439aaa93 ("mlx5: Return pointer to CQ doorbell") Strange, in github it has 7 digits (the same was with rsocket fixes) Fixes: c6e3439 ("mlx5: Return pointer to CQ doorbell") > Signed-off-by: Leon Romanovsky <leonro@mellanox.com> > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> The overall looks good, but I need to run checks with before applying it. Thanks for doing it.
On Wed, Aug 23, 2017 at 07:45:55PM +0300, Leon Romanovsky wrote: > > Provide two implementations of mlx5dv_init_obj, one that has the > > historical behaviour that has existed until now of returning the > > void **uar and a new version that returns the 'void *' version > > renamed to arb_db. > > arb_db -> arm_db Oops, I fixed that, thanks. > Strange, in github it has 7 digits (the same was with rsocket fixes) > Fixes: c6e3439 ("mlx5: Return pointer to CQ doorbell") Yeah, side effect of github rendering it into a web link, I think. > The overall looks good, but I need to run checks with before > applying it. Sure, I didn't test it obviously, but the readelf is what I expect: 101: 000000000000e4d0 55 FUNC GLOBAL DEFAULT 13 mlx5dv_init_obj@MLX5_1.0 168: 000000000000e4d0 55 FUNC LOCAL DEFAULT 13 __mlx5dv_init_obj_1_0 102: 000000000000e300 451 FUNC GLOBAL DEFAULT 13 mlx5dv_init_obj@@MLX5_1.2 169: 000000000000e300 451 FUNC LOCAL DEFAULT 13 __mlx5dv_init_obj_1_2 Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/debian/ibverbs-providers.symbols b/debian/ibverbs-providers.symbols index 5aa3b877b55b59..b03c33a2ba9fce 100644 --- a/debian/ibverbs-providers.symbols +++ b/debian/ibverbs-providers.symbols @@ -4,6 +4,8 @@ libmlx4.so.1 ibverbs-providers #MINVER# libmlx5.so.1 ibverbs-providers #MINVER# MLX5_1.0@MLX5_1.0 13 MLX5_1.1@MLX5_1.1 14 + MLX5_1.2@MLX5_1.2 15 mlx5dv_init_obj@MLX5_1.0 13 + mlx5dv_init_obj@MLX5_1.2 15 mlx5dv_query_device@MLX5_1.0 13 mlx5dv_create_cq@MLX5_1.1 14 diff --git a/providers/mlx5/CMakeLists.txt b/providers/mlx5/CMakeLists.txt index 0fb9907bf258d1..ab6a42d8915a2a 100644 --- a/providers/mlx5/CMakeLists.txt +++ b/providers/mlx5/CMakeLists.txt @@ -11,7 +11,7 @@ if (MLX5_MW_DEBUG) endif() rdma_shared_provider(mlx5 libmlx5.map - 1 1.1.${PACKAGE_VERSION} + 1 1.2.${PACKAGE_VERSION} buf.c cq.c dbrec.c diff --git a/providers/mlx5/libmlx5.map b/providers/mlx5/libmlx5.map index 6d9fb25f00a3db..e7fe9f41697009 100644 --- a/providers/mlx5/libmlx5.map +++ b/providers/mlx5/libmlx5.map @@ -11,3 +11,8 @@ MLX5_1.1 { global: mlx5dv_create_cq; } MLX5_1.0; + +MLX5_1.2 { + global: + mlx5dv_init_obj; +} MLX5_1.1; diff --git a/providers/mlx5/mlx5.c b/providers/mlx5/mlx5.c index dd825561ec740f..7ec5951b6ff1f8 100644 --- a/providers/mlx5/mlx5.c +++ b/providers/mlx5/mlx5.c @@ -678,7 +678,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->uar = mctx->uar[0]; + cq_out->arm_db = mctx->uar[0]; mcq->flags |= MLX5_CQ_FLAGS_DV_OWNED; @@ -716,7 +716,8 @@ static int mlx5dv_get_srq(struct ibv_srq *srq_in, return 0; } -int mlx5dv_init_obj(struct mlx5dv_obj *obj, uint64_t obj_type) +int __mlx5dv_init_obj_1_2(struct mlx5dv_obj *obj, uint64_t obj_type); +int __mlx5dv_init_obj_1_2(struct mlx5dv_obj *obj, uint64_t obj_type) { int ret = 0; @@ -731,6 +732,23 @@ int mlx5dv_init_obj(struct mlx5dv_obj *obj, uint64_t obj_type) return ret; } +asm(".symver __mlx5dv_init_obj_1_2, mlx5dv_init_obj@@MLX5_1.2"); + +int __mlx5dv_init_obj_1_0(struct mlx5dv_obj *obj, uint64_t obj_type); +int __mlx5dv_init_obj_1_0(struct mlx5dv_obj *obj, uint64_t obj_type) +{ + int ret = 0; + + ret = __mlx5dv_init_obj_1_2(obj, obj_type); + if (!ret && (obj_type & MLX5DV_OBJ_CQ)) { + /* ABI version 1.0 returns the void ** in this memory + * location + */ + obj->cq.out->arm_db = to_mctx(obj->cq.in->context)->uar; + } + return ret; +} +asm(".symver __mlx5dv_init_obj_1_0, mlx5dv_init_obj@MLX5_1.0"); static void adjust_uar_info(struct mlx5_device *mdev, struct mlx5_context *context, diff --git a/providers/mlx5/mlx5dv.h b/providers/mlx5/mlx5dv.h index cff3a10457300f..1a2e257c4bcc6f 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 *uar; + void *arm_db; uint32_t cqn; uint64_t comp_mask; };