Re: [PATCH mlx5-next] IB/mlx5: Prevent allocating UMEM and UCTX as some general object

On Thu, Dec 20, 2018 at 05:02:12PM +0000, Jason Gunthorpe wrote:

On Thu, Dec 20, 2018 at 08:21:33AM +0200, Leon Romanovsky wrote:

quoted

On Wed, Dec 19, 2018 at 07:14:12PM +0000, Jason Gunthorpe wrote:

quoted

On Wed, Dec 19, 2018 at 04:28:15PM +0200, Leon Romanovsky wrote:

quoted

From: Yishai Hadas <redacted>

The driver needs to prevent a user space application to create a
UMEM and UCTX via the general object command.

The UMEM must go through the kernel UMEM_REG method to prevent the user
from setting physical addresses by himself.  The UCTX is some internal
kernel object and shouldn't be exposed.

As of not being any more part of the general object the caps bits were
moved to be some log_xxx indication in the general HCA caps, 0 means not
supported.

The firmware code was adapted as well to match the above.

This commit message is a bit wonky.. how about

IB/mlx5: Use the correct commands for UMEM and UCTX allocation

During testing the command format was changed to close a security
hole. Revise the driver to use the command format that will actually
be supported in GA firmware.

Both the UMEM and UCTX are intended only for use by the kernel and
cannot be executed using a general command.

Since the UMEM and CTX are not part of the general object the caps
bits were moved to be some log_xxx location in the general HCA caps.

I'm fine with this description too.

quoted

quoted

Signed-off-by: Yishai Hadas <redacted>
Reviewed-by: Achiad Shochat <redacted>
Signed-off-by: Leon Romanovsky <redacted>

Also add a fixes line please, any kernel with the devx needs this
patch to work with GA firmware.

Fixes: a8b92ca1b0e5 ("IB/mlx5: Introduce DEVX")

quoted

quoted

 drivers/infiniband/hw/mlx5/devx.c | 34 ++++++++---------
 drivers/infiniband/hw/mlx5/main.c |  3 +-
 include/linux/mlx5/mlx5_ifc.h     | 62 +++++++++++++++++++++----------
 3 files changed, 58 insertions(+), 41 deletions(-)

Otherwise the patch looks fine, please apply to the shared branch..

Jason,

I have procedural question. This patch is based on rdma-next and there
is diversion in both devx.c and main.c in those areas, enough do not
apply cleanly. Will it be easier if you take this patch to rdma-next,
instead me applying different patch to mlx5-next and you hitting merge
conflict later on while trying to merge it into rdma-next?

Sure, but it has the unsplit _ifc update...

If it was possible, I wouldn't bother you. This change should be as one
piece otherwise we will have failed to build patch.

I think we are at the end of the cycle so this probably won't make a
conflict - lets just go in RDMA then.

From what I see in queues, you are right, merge conflict is unlikely to happen.

Thanks

Jason