| Message ID | 1391005649-17932-1-git-send-email-eli@mellanox.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi, Le mercredi 29 janvier 2014 à 16:27 +0200, Eli Cohen a écrit : > Commit c1be523 "Fix micro UAR allocator" broke binary compatibility between ^^^^^^^ 12 digits identifier are the norm for kernel. Please update your git configuration: git config --global core.abbrev 12 See http://lwn.net/Articles/571980/ http://blog.cuviper.com/2013/11/10/how-short-can-git-abbreviate/ > libmlx5 and mlx5_ib since it defines a different value to the number of micro > UARs per page, leading to wrong calculation in libmlx5. This patch defines > struct mlx5_ib_alloc_ucontext_req_v2 as an extension to struct > mlx5_ib_alloc_ucontext_req. The extended size is determined in > mlx5_ib_alloc_ucontext() and in case of old library we use uuarn 0 which works > fine. For new libraries we use the more sophisticated allocation algorithm. > > Fixes: c1be523 ('Fix micro UAR allocator') ^^^^^^^ Likewise > Signed-off-by: Eli Cohen <eli@mellanox.com> > --- > > Hi Roland, > > This fixes a regression introduced during the 3.14 merge window, and must > be applied for 3.14, thanks > > Eli > > drivers/infiniband/hw/mlx5/main.c | 19 +++++++++++++++++-- > drivers/infiniband/hw/mlx5/qp.c | 11 +++++++++-- > drivers/infiniband/hw/mlx5/user.h | 7 +++++++ > include/linux/mlx5/driver.h | 1 + > 4 files changed, 34 insertions(+), 4 deletions(-) > > diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c > index 9660d09..e24d912 100644 > --- a/drivers/infiniband/hw/mlx5/main.c > +++ b/drivers/infiniband/hw/mlx5/main.c > @@ -536,24 +536,38 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev, > struct ib_udata *udata) > { > struct mlx5_ib_dev *dev = to_mdev(ibdev); > - struct mlx5_ib_alloc_ucontext_req req; > + struct mlx5_ib_alloc_ucontext_req_v2 req; > struct mlx5_ib_alloc_ucontext_resp resp; > struct mlx5_ib_ucontext *context; > struct mlx5_uuar_info *uuari; > struct mlx5_uar *uars; > int gross_uuars; > int num_uars; > + int ver; > int uuarn; > int err; > int i; > + int reqlen; > > if (!dev->ib_active) > return ERR_PTR(-EAGAIN); > > - err = ib_copy_from_udata(&req, udata, sizeof(req)); > + memset(&req, 0, sizeof(req)); > + reqlen = udata->inlen - sizeof(struct ib_uverbs_get_context); I'm not sure how this could work without subtracting sizeof(struct ib_uverbs_cmd_hdr). As I explained in "Re: [PATCHv4 for-3.13 00/10] create_flow/destroy_flow fixes for v3.13" [1] ib_uverbs_write() does not decrement input length: it gives hdr.in_words * 4 to the uverbs function, here ib_uverbs_get_context(). Then, the function built struct ib_udata without taking care of the extra bytes count in in_len: struct ib_uverbs_get_context cmd; ... INIT_UDATA(&udata, buf + sizeof cmd, (unsigned long) cmd.response + sizeof resp, in_len - sizeof cmd, out_len - sizeof resp); Driver mthca does some handling which look like to what's proposed in your patch, but takes care of subtracting the header size from the input length, see mthca_reg_user_mr()[2]. [1] <http://marc.info/?i=1387493822.11925.217.camel@localhost.localdomain> [2] <http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/infiniband/hw/mthca/mthca_provider.c?id=0e47c969c65e213421450c31043353ebe3c67e0c#n988> > + if (reqlen == sizeof(struct mlx5_ib_alloc_ucontext_req)) > + ver = 0; > + else if (reqlen == sizeof(struct mlx5_ib_alloc_ucontext_req_v2)) > + ver = 2; > + else > + return ERR_PTR(-EINVAL); > + Doing so introduce a subtle regression: there was no check on the length before, so it was legal to pass a input buffer far larger than needed, aka. trailing garbage. With such new test in place, it's no more allowed, and this is a regression. It's not a big issue, but a little departure from current behavor. BTW, this is the correct way to handle the request, every other uverbs functions should behave like this, eg. being strict on its accepted input. > + err = ib_copy_from_udata(&req, udata, reqlen); > if (err) > return ERR_PTR(err); > > + if (req.flags || req.reserved) > + return ERR_PTR(-EINVAL); > + Just like this :) > if (req.total_num_uuars > MLX5_MAX_UUARS) > return ERR_PTR(-ENOMEM); > > @@ -626,6 +640,7 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev, > if (err) > goto out_uars; > > + uuari->ver = ver; > uuari->num_low_latency_uuars = req.num_low_latency_uuars; > uuari->uars = uars; > uuari->num_uars = num_uars; > diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c > index 492dc33..300475c 100644 > --- a/drivers/infiniband/hw/mlx5/qp.c > +++ b/drivers/infiniband/hw/mlx5/qp.c > @@ -430,11 +430,17 @@ static int alloc_uuar(struct mlx5_uuar_info *uuari, > break; > > case MLX5_IB_LATENCY_CLASS_MEDIUM: > - uuarn = alloc_med_class_uuar(uuari); > + if (uuari->ver < 2) > + uuarn = -ENOMEM; In the commit message, you specified that uuarn is set to 0 when v1 is used. But here it's set to -ENOMEM. > + else > + uuarn = alloc_med_class_uuar(uuari); > break; > > case MLX5_IB_LATENCY_CLASS_HIGH: > - uuarn = alloc_high_class_uuar(uuari); > + if (uuari->ver < 2) > + uuarn = -ENOMEM; Likewise. > + else > + uuarn = alloc_high_class_uuar(uuari); > break; > > case MLX5_IB_LATENCY_CLASS_FAST_PATH: > @@ -559,6 +565,7 @@ static int create_user_qp(struct mlx5_ib_dev *dev, struct ib_pd *pd, > } > } > > + Remove this white space. > uar_index = uuarn_to_uar_index(&context->uuari, uuarn); > mlx5_ib_dbg(dev, "uuarn 0x%x, uar_index 0x%x\n", uuarn, uar_index); > > diff --git a/drivers/infiniband/hw/mlx5/user.h b/drivers/infiniband/hw/mlx5/user.h > index 32a2a5d..0f4f8e4 100644 > --- a/drivers/infiniband/hw/mlx5/user.h > +++ b/drivers/infiniband/hw/mlx5/user.h > @@ -62,6 +62,13 @@ struct mlx5_ib_alloc_ucontext_req { > __u32 num_low_latency_uuars; > }; > > +struct mlx5_ib_alloc_ucontext_req_v2 { > + __u32 total_num_uuars; > + __u32 num_low_latency_uuars; > + __u32 flags; > + __u32 reserved; > +}; > + > struct mlx5_ib_alloc_ucontext_resp { > __u32 qp_tab_size; > __u32 bf_reg_size; > diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h > index 554548c..32cb18c 100644 > --- a/include/linux/mlx5/driver.h > +++ b/include/linux/mlx5/driver.h > @@ -227,6 +227,7 @@ struct mlx5_uuar_info { > * protect uuar allocation data structs > */ > struct mutex lock; > + u32 ver; > }; > > struct mlx5_bf { Regards.
On Wed, Jan 29, 2014 at 09:48:43PM +0100, Yann Droneaud wrote: Yann, thanks for reviewing, your comments are helpful :-) > > 12 digits identifier are the norm for kernel. Please update your git > configuration: > > git config --global core.abbrev 12 > > See http://lwn.net/Articles/571980/ > http://blog.cuviper.com/2013/11/10/how-short-can-git-abbreviate/ > > > libmlx5 and mlx5_ib since it defines a different value to the number of micro > > UARs per page, leading to wrong calculation in libmlx5. This patch defines > > struct mlx5_ib_alloc_ucontext_req_v2 as an extension to struct > > mlx5_ib_alloc_ucontext_req. The extended size is determined in > > mlx5_ib_alloc_ucontext() and in case of old library we use uuarn 0 which works > > fine. For new libraries we use the more sophisticated allocation algorithm. > > > > Fixes: c1be523 ('Fix micro UAR allocator') > ^^^^^^^ > Likewise Will fix. > > I'm not sure how this could work without subtracting sizeof(struct > ib_uverbs_cmd_hdr). struct ib_uverbs_get_context happens to have the same size as struct ib_uverbs_cmd_hdr so it passed all my sanity tests. Correct, will fix that too. > > As I explained in "Re: [PATCHv4 for-3.13 00/10] create_flow/destroy_flow > fixes for v3.13" [1] ib_uverbs_write() does not decrement input length: > it gives hdr.in_words * 4 to the uverbs function, here > ib_uverbs_get_context(). Then, the function built struct ib_udata > without taking care of the extra bytes count in in_len: > > struct ib_uverbs_get_context cmd; > ... > INIT_UDATA(&udata, buf + sizeof cmd, > (unsigned long) cmd.response + sizeof resp, > in_len - sizeof cmd, out_len - sizeof resp); So this just seems broken and the fix is to do the subtraction here so the hardware driver gets the correct size without needing to subtract the extra bytes, like this: INIT_UDATA(&udata, buf + sizeof cmd, (unsigned long) cmd.response + sizeof resp, in_len - sizeof cmd - sizeof (struct ib_uverbs_cmd_hdr), out_len - sizeof resp); And the hardware driver gets the correct size for its struct. > > Driver mthca does some handling which look like to what's proposed in > your patch, but takes care of subtracting the header size from the input > length, see mthca_reg_user_mr()[2]. > > [1] > <http://marc.info/?i=1387493822.11925.217.camel@localhost.localdomain> > > [2] > <http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/infiniband/hw/mthca/mthca_provider.c?id=0e47c969c65e213421450c31043353ebe3c67e0c#n988> > > > + if (reqlen == sizeof(struct mlx5_ib_alloc_ucontext_req)) > > + ver = 0; > > + else if (reqlen == sizeof(struct mlx5_ib_alloc_ucontext_req_v2)) > > + ver = 2; > > + else > > + return ERR_PTR(-EINVAL); > > + > > Doing so introduce a subtle regression: there was no check on the length > before, so it was legal to pass a input buffer far larger than needed, > aka. trailing garbage. > > With such new test in place, it's no more allowed, and this is a > regression. It's not a big issue, but a little departure from current > behavor. Well it's a regression if there was out there another library for mlx5 out there which misbehaves and there is any as far as I know. So I think it's safe to add this strict check. > > BTW, this is the correct way to handle the request, every other uverbs > functions should behave like this, eg. being strict on its accepted > input. > > > + err = ib_copy_from_udata(&req, udata, reqlen); > > if (err) > > return ERR_PTR(err); > > > > + if (req.flags || req.reserved) > > + return ERR_PTR(-EINVAL); > > + > > Just like this :) > > > if (req.total_num_uuars > MLX5_MAX_UUARS) > > return ERR_PTR(-ENOMEM); > > > > @@ -626,6 +640,7 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev, > > if (err) > > goto out_uars; > > > > + uuari->ver = ver; > > uuari->num_low_latency_uuars = req.num_low_latency_uuars; > > uuari->uars = uars; > > uuari->num_uars = num_uars; > > diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c > > index 492dc33..300475c 100644 > > --- a/drivers/infiniband/hw/mlx5/qp.c > > +++ b/drivers/infiniband/hw/mlx5/qp.c > > @@ -430,11 +430,17 @@ static int alloc_uuar(struct mlx5_uuar_info *uuari, > > break; > > > > case MLX5_IB_LATENCY_CLASS_MEDIUM: > > - uuarn = alloc_med_class_uuar(uuari); > > + if (uuari->ver < 2) > > + uuarn = -ENOMEM; > > In the commit message, you specified that uuarn is set to 0 when v1 is > used. But here it's set to -ENOMEM. > If you look at qp.c you can see that the code falls back from high to medium to low class. Low class always succeeds and gives 0. > > + else > > + uuarn = alloc_med_class_uuar(uuari); > > break; > > > > case MLX5_IB_LATENCY_CLASS_HIGH: > > - uuarn = alloc_high_class_uuar(uuari); > > + if (uuari->ver < 2) > > + uuarn = -ENOMEM; > > Likewise. > > > + else > > + uuarn = alloc_high_class_uuar(uuari); > > break; > > > > case MLX5_IB_LATENCY_CLASS_FAST_PATH: > > @@ -559,6 +565,7 @@ static int create_user_qp(struct mlx5_ib_dev *dev, struct ib_pd *pd, > > } > > } > > > > + > > Remove this white space. > > > uar_index = uuarn_to_uar_index(&context->uuari, uuarn); > > mlx5_ib_dbg(dev, "uuarn 0x%x, uar_index 0x%x\n", uuarn, uar_index); > > > > diff --git a/drivers/infiniband/hw/mlx5/user.h b/drivers/infiniband/hw/mlx5/user.h > > index 32a2a5d..0f4f8e4 100644 > > --- a/drivers/infiniband/hw/mlx5/user.h > > +++ b/drivers/infiniband/hw/mlx5/user.h > > @@ -62,6 +62,13 @@ struct mlx5_ib_alloc_ucontext_req { > > __u32 num_low_latency_uuars; > > }; > > > > +struct mlx5_ib_alloc_ucontext_req_v2 { > > + __u32 total_num_uuars; > > + __u32 num_low_latency_uuars; > > + __u32 flags; > > + __u32 reserved; > > +}; > > + > > > struct mlx5_ib_alloc_ucontext_resp { > > __u32 qp_tab_size; > > __u32 bf_reg_size; > > diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h > > index 554548c..32cb18c 100644 > > --- a/include/linux/mlx5/driver.h > > +++ b/include/linux/mlx5/driver.h > > @@ -227,6 +227,7 @@ struct mlx5_uuar_info { > > * protect uuar allocation data structs > > */ > > struct mutex lock; > > + u32 ver; > > }; > > > > struct mlx5_bf { > > Regards. > > -- > Yann Droneaud > OPTEYA > -- 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
Hi, Le jeudi 30 janvier 2014 à 01:33 +0200, Eli Cohen a écrit : > On Wed, Jan 29, 2014 at 09:48:43PM +0100, Yann Droneaud wrote: > Yann, > thanks for reviewing, your comments are helpful :-) > > > > > 12 digits identifier are the norm for kernel. Please update your git > > configuration: > > > > git config --global core.abbrev 12 > > > > See http://lwn.net/Articles/571980/ > > http://blog.cuviper.com/2013/11/10/how-short-can-git-abbreviate/ > > > > > libmlx5 and mlx5_ib since it defines a different value to the number of micro > > > UARs per page, leading to wrong calculation in libmlx5. This patch defines > > > struct mlx5_ib_alloc_ucontext_req_v2 as an extension to struct > > > mlx5_ib_alloc_ucontext_req. The extended size is determined in > > > mlx5_ib_alloc_ucontext() and in case of old library we use uuarn 0 which works > > > fine. For new libraries we use the more sophisticated allocation algorithm. > > > > > > Fixes: c1be523 ('Fix micro UAR allocator') > > ^^^^^^^ > > Likewise > > Will fix. > > > > I'm not sure how this could work without subtracting sizeof(struct > > ib_uverbs_cmd_hdr). > > struct ib_uverbs_get_context happens to have the same size as struct > ib_uverbs_cmd_hdr so it passed all my sanity tests. Correct, will fix > that too. > > > > As I explained in "Re: [PATCHv4 for-3.13 00/10] create_flow/destroy_flow > > fixes for v3.13" [1] ib_uverbs_write() does not decrement input length: > > it gives hdr.in_words * 4 to the uverbs function, here > > ib_uverbs_get_context(). Then, the function built struct ib_udata > > without taking care of the extra bytes count in in_len: > > > > struct ib_uverbs_get_context cmd; > > ... > > INIT_UDATA(&udata, buf + sizeof cmd, > > (unsigned long) cmd.response + sizeof resp, > > in_len - sizeof cmd, out_len - sizeof resp); > > So this just seems broken and the fix is to do the subtraction here so > the hardware driver gets the correct size without needing to subtract > the extra bytes, like this: > INIT_UDATA(&udata, buf + sizeof cmd, > (unsigned long) cmd.response + sizeof resp, > in_len - sizeof cmd - sizeof (struct ib_uverbs_cmd_hdr), > out_len - sizeof resp); > > And the hardware driver gets the correct size for its struct. > It's not the place to address the issue: the proper fix is to subtract it in ib_uverbs_write(). I have a patch for this but I'm not going to submit it for 3.14: it's part of a patchset that need testing and polishing. I will submit this large patchset for 3.15 (after v3.14-rc1). So you have to subtract the header size in your driver, just like mthca does. And don't worry I will take care of removing it in my patch that remove the header size from input length. > > > > Driver mthca does some handling which look like to what's proposed in > > your patch, but takes care of subtracting the header size from the input > > length, see mthca_reg_user_mr()[2]. > > > > [1] > > <http://marc.info/?i=1387493822.11925.217.camel@localhost.localdomain> > > > > [2] > > <http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/infiniband/hw/mthca/mthca_provider.c?id=0e47c969c65e213421450c31043353ebe3c67e0c#n988> > > > > > + if (reqlen == sizeof(struct mlx5_ib_alloc_ucontext_req)) > > > + ver = 0; > > > + else if (reqlen == sizeof(struct mlx5_ib_alloc_ucontext_req_v2)) > > > + ver = 2; > > > + else > > > + return ERR_PTR(-EINVAL); > > > + > > > > Doing so introduce a subtle regression: there was no check on the length > > before, so it was legal to pass a input buffer far larger than needed, > > aka. trailing garbage. > > > > With such new test in place, it's no more allowed, and this is a > > regression. It's not a big issue, but a little departure from current > > behavor. > Well it's a regression if there was out there another library for mlx5 > out there which misbehaves and there is any as far as I know. So I > think it's safe to add this strict check. Yes I think so. But as I'm used to write it here, there's no clear line drawn: where is the ABI that we should enforce ? Is it the kernel uverbs data structure for response and request or libuverbs/lib<hw> ? In the first case, your change add a regression, in the second case, it's not. It's just a theoretical question. In practice, as you wrote, we can say there's no other library that implement the userspace part of mlx5 rdma driver. > > > > BTW, this is the correct way to handle the request, every other uverbs > > functions should behave like this, eg. being strict on its accepted > > input. > > > > > + err = ib_copy_from_udata(&req, udata, reqlen); > > > if (err) > > > return ERR_PTR(err); > > > > > > + if (req.flags || req.reserved) > > > + return ERR_PTR(-EINVAL); > > > + > > > > Just like this :) > > > > > if (req.total_num_uuars > MLX5_MAX_UUARS) > > > return ERR_PTR(-ENOMEM); > > > > > > @@ -626,6 +640,7 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev, > > > if (err) > > > goto out_uars; > > > > > > + uuari->ver = ver; > > > uuari->num_low_latency_uuars = req.num_low_latency_uuars; > > > uuari->uars = uars; > > > uuari->num_uars = num_uars; > > > diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c > > > index 492dc33..300475c 100644 > > > --- a/drivers/infiniband/hw/mlx5/qp.c > > > +++ b/drivers/infiniband/hw/mlx5/qp.c > > > @@ -430,11 +430,17 @@ static int alloc_uuar(struct mlx5_uuar_info *uuari, > > > break; > > > > > > case MLX5_IB_LATENCY_CLASS_MEDIUM: > > > - uuarn = alloc_med_class_uuar(uuari); > > > + if (uuari->ver < 2) > > > + uuarn = -ENOMEM; > > > > In the commit message, you specified that uuarn is set to 0 when v1 is > > used. But here it's set to -ENOMEM. > > > If you look at qp.c you can see that the code falls back from high to > medium to low class. Low class always succeeds and gives 0. > It was not clear from the commit message. > > > + else > > > + uuarn = alloc_med_class_uuar(uuari); > > > break; > > > > > > case MLX5_IB_LATENCY_CLASS_HIGH: > > > - uuarn = alloc_high_class_uuar(uuari); > > > + if (uuari->ver < 2) > > > + uuarn = -ENOMEM; > > > > Likewise. > > > > > + else > > > + uuarn = alloc_high_class_uuar(uuari); > > > break; > > > Regards
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c index 9660d09..e24d912 100644 --- a/drivers/infiniband/hw/mlx5/main.c +++ b/drivers/infiniband/hw/mlx5/main.c @@ -536,24 +536,38 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev, struct ib_udata *udata) { struct mlx5_ib_dev *dev = to_mdev(ibdev); - struct mlx5_ib_alloc_ucontext_req req; + struct mlx5_ib_alloc_ucontext_req_v2 req; struct mlx5_ib_alloc_ucontext_resp resp; struct mlx5_ib_ucontext *context; struct mlx5_uuar_info *uuari; struct mlx5_uar *uars; int gross_uuars; int num_uars; + int ver; int uuarn; int err; int i; + int reqlen; if (!dev->ib_active) return ERR_PTR(-EAGAIN); - err = ib_copy_from_udata(&req, udata, sizeof(req)); + memset(&req, 0, sizeof(req)); + reqlen = udata->inlen - sizeof(struct ib_uverbs_get_context); + if (reqlen == sizeof(struct mlx5_ib_alloc_ucontext_req)) + ver = 0; + else if (reqlen == sizeof(struct mlx5_ib_alloc_ucontext_req_v2)) + ver = 2; + else + return ERR_PTR(-EINVAL); + + err = ib_copy_from_udata(&req, udata, reqlen); if (err) return ERR_PTR(err); + if (req.flags || req.reserved) + return ERR_PTR(-EINVAL); + if (req.total_num_uuars > MLX5_MAX_UUARS) return ERR_PTR(-ENOMEM); @@ -626,6 +640,7 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev, if (err) goto out_uars; + uuari->ver = ver; uuari->num_low_latency_uuars = req.num_low_latency_uuars; uuari->uars = uars; uuari->num_uars = num_uars; diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c index 492dc33..300475c 100644 --- a/drivers/infiniband/hw/mlx5/qp.c +++ b/drivers/infiniband/hw/mlx5/qp.c @@ -430,11 +430,17 @@ static int alloc_uuar(struct mlx5_uuar_info *uuari, break; case MLX5_IB_LATENCY_CLASS_MEDIUM: - uuarn = alloc_med_class_uuar(uuari); + if (uuari->ver < 2) + uuarn = -ENOMEM; + else + uuarn = alloc_med_class_uuar(uuari); break; case MLX5_IB_LATENCY_CLASS_HIGH: - uuarn = alloc_high_class_uuar(uuari); + if (uuari->ver < 2) + uuarn = -ENOMEM; + else + uuarn = alloc_high_class_uuar(uuari); break; case MLX5_IB_LATENCY_CLASS_FAST_PATH: @@ -559,6 +565,7 @@ static int create_user_qp(struct mlx5_ib_dev *dev, struct ib_pd *pd, } } + uar_index = uuarn_to_uar_index(&context->uuari, uuarn); mlx5_ib_dbg(dev, "uuarn 0x%x, uar_index 0x%x\n", uuarn, uar_index); diff --git a/drivers/infiniband/hw/mlx5/user.h b/drivers/infiniband/hw/mlx5/user.h index 32a2a5d..0f4f8e4 100644 --- a/drivers/infiniband/hw/mlx5/user.h +++ b/drivers/infiniband/hw/mlx5/user.h @@ -62,6 +62,13 @@ struct mlx5_ib_alloc_ucontext_req { __u32 num_low_latency_uuars; }; +struct mlx5_ib_alloc_ucontext_req_v2 { + __u32 total_num_uuars; + __u32 num_low_latency_uuars; + __u32 flags; + __u32 reserved; +}; + struct mlx5_ib_alloc_ucontext_resp { __u32 qp_tab_size; __u32 bf_reg_size; diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h index 554548c..32cb18c 100644 --- a/include/linux/mlx5/driver.h +++ b/include/linux/mlx5/driver.h @@ -227,6 +227,7 @@ struct mlx5_uuar_info { * protect uuar allocation data structs */ struct mutex lock; + u32 ver; }; struct mlx5_bf {
Commit c1be523 "Fix micro UAR allocator" broke binary compatibility between libmlx5 and mlx5_ib since it defines a different value to the number of micro UARs per page, leading to wrong calculation in libmlx5. This patch defines struct mlx5_ib_alloc_ucontext_req_v2 as an extension to struct mlx5_ib_alloc_ucontext_req. The extended size is determined in mlx5_ib_alloc_ucontext() and in case of old library we use uuarn 0 which works fine. For new libraries we use the more sophisticated allocation algorithm. Fixes: c1be523 ('Fix micro UAR allocator') Signed-off-by: Eli Cohen <eli@mellanox.com> --- Hi Roland, This fixes a regression introduced during the 3.14 merge window, and must be applied for 3.14, thanks Eli drivers/infiniband/hw/mlx5/main.c | 19 +++++++++++++++++-- drivers/infiniband/hw/mlx5/qp.c | 11 +++++++++-- drivers/infiniband/hw/mlx5/user.h | 7 +++++++ include/linux/mlx5/driver.h | 1 + 4 files changed, 34 insertions(+), 4 deletions(-)