Re: [PATCH rdma-next 03/12] RDMA/mlx5: Move mkey ctrl segment logic to umr.c

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Apr 12, 2022 at 10:23:58AM +0300, Leon Romanovsky wrote:
> From: Aharon Landau <aharonl@nvidia.com>
> 
> Move set_reg_umr_segment() and its helpers to umr.c.
> 
> Signed-off-by: Aharon Landau <aharonl@nvidia.com>
> Reviewed-by: Michael Guralnik <michaelgur@nvidia.com>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
>  drivers/infiniband/hw/mlx5/mlx5_ib.h |   3 -
>  drivers/infiniband/hw/mlx5/qp.c      |   1 +
>  drivers/infiniband/hw/mlx5/umr.c     | 129 ++++++++++++++++++++++++
>  drivers/infiniband/hw/mlx5/umr.h     |  13 +++
>  drivers/infiniband/hw/mlx5/wr.c      | 142 +--------------------------
>  5 files changed, 147 insertions(+), 141 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> index 18ba11e4e2e6..d77a27503488 100644
> +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> @@ -311,9 +311,6 @@ struct mlx5_ib_flow_db {
>  #define MLX5_IB_QPT_DCT		IB_QPT_RESERVED4
>  #define MLX5_IB_WR_UMR		IB_WR_RESERVED1
>  
> -#define MLX5_IB_UMR_OCTOWORD	       16
> -#define MLX5_IB_UMR_XLT_ALIGNMENT      64
> -
>  #define MLX5_IB_UPD_XLT_ZAP	      BIT(0)
>  #define MLX5_IB_UPD_XLT_ENABLE	      BIT(1)
>  #define MLX5_IB_UPD_XLT_ATOMIC	      BIT(2)
> diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
> index 3f467557d34e..d2f243d3c4e2 100644
> +++ b/drivers/infiniband/hw/mlx5/qp.c
> @@ -40,6 +40,7 @@
>  #include "ib_rep.h"
>  #include "counters.h"
>  #include "cmd.h"
> +#include "umr.h"
>  #include "qp.h"
>  #include "wr.h"
>  
> diff --git a/drivers/infiniband/hw/mlx5/umr.c b/drivers/infiniband/hw/mlx5/umr.c
> index 46eaf919eb49..d3626a9dc8ab 100644
> +++ b/drivers/infiniband/hw/mlx5/umr.c
> @@ -4,6 +4,135 @@
>  #include "mlx5_ib.h"
>  #include "umr.h"
>  
> +static __be64 get_umr_enable_mr_mask(void)
> +{
> +	u64 result;
> +
> +	result = MLX5_MKEY_MASK_KEY |
> +		 MLX5_MKEY_MASK_FREE;
> +
> +	return cpu_to_be64(result);
> +}
> +
> +static __be64 get_umr_disable_mr_mask(void)
> +{
> +	u64 result;
> +
> +	result = MLX5_MKEY_MASK_FREE;
> +
> +	return cpu_to_be64(result);
> +}
> +
> +static __be64 get_umr_update_translation_mask(void)
> +{
> +	u64 result;
> +
> +	result = MLX5_MKEY_MASK_LEN |
> +		 MLX5_MKEY_MASK_PAGE_SIZE |
> +		 MLX5_MKEY_MASK_START_ADDR;
> +
> +	return cpu_to_be64(result);
> +}

This is pretty ugly, it is fine to copy it, but an add-on patch to
remove these function wrappers would be nice

#define UMR_MR_MASK_DISABLE cpu_to_be64(MLX5_MKEY_MASK_FREE)

At worst (and arguably it can just be open coded)

Jason