netdev - Re: [PATCH mlx5-next 12/12] net/mlx5: Set ODP SRQ support in firmware

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 4 Feb 2019 23:47:23 +0000
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     Jason Gunthorpe <jgg@...lanox.com>,
        "leon@...nel.org" <leon@...nel.org>,
        "dledford@...hat.com" <dledford@...hat.com>
CC:     Majd Dibbiny <majd@...lanox.com>, Moni Shoua <monis@...lanox.com>,
        Leon Romanovsky <leonro@...lanox.com>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH mlx5-next 12/12] net/mlx5: Set ODP SRQ support in firmware

On Tue, 2019-01-22 at 08:48 +0200, Leon Romanovsky wrote:
> From: Moni Shoua <monis@...lanox.com>
> 
> To avoid compatibility issue with older kernels the firmware doesn't
> allow SRQ to work with ODP unless kernel asks for it.
> 
> Signed-off-by: Moni Shoua <monis@...lanox.com>
> Reviewed-by: Majd Dibbiny <majd@...lanox.com>
> Signed-off-by: Leon Romanovsky <leonro@...lanox.com>
> ---
>  .../net/ethernet/mellanox/mlx5/core/main.c    | 53
> +++++++++++++++++++
>  include/linux/mlx5/device.h                   |  3 ++
>  include/linux/mlx5/mlx5_ifc.h                 |  1 +
>  3 files changed, 57 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> index be81b319b0dc..b3a76df0cf6c 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> @@ -459,6 +459,53 @@ static int handle_hca_cap_atomic(struct
> mlx5_core_dev *dev)
>  	return err;
>  }
>  
> +static int handle_hca_cap_odp(struct mlx5_core_dev *dev)
> +{
> +	void *set_ctx;
> +	void *set_hca_cap;
> +	int set_sz = MLX5_ST_SZ_BYTES(set_hca_cap_in);
> +	int err;
> +

reversed xmas tree.

> +	if (!MLX5_CAP_GEN(dev, pg))
> +		return 0;
> +
> +	err = mlx5_core_get_caps(dev, MLX5_CAP_ODP);
> +	if (err)
> +		return err;
> +
> +	/**
> +	 * If all bits are cleared we shouldn't try to set it
> +	 * or we might fail while trying to access a reserved bit.
> +	 */

"set them" not "set it" ? 

to me this is a redundant comment, the code is self explanatory.

> +	if (!(MLX5_CAP_ODP_MAX(dev, ud_odp_caps.srq_receive) ||
> +	      MLX5_CAP_ODP_MAX(dev, rc_odp_caps.srq_receive) ||
> +	      MLX5_CAP_ODP_MAX(dev, xrc_odp_caps.srq_receive)))
> +		return 0;
> +
> +	set_ctx = kzalloc(set_sz, GFP_KERNEL);
> +	if (!set_ctx)
> +		return -ENOMEM;
> +
> +	set_hca_cap = MLX5_ADDR_OF(set_hca_cap_in, set_ctx,
> capability);
> +	memcpy(set_hca_cap, dev->caps.hca_cur[MLX5_CAP_ODP],
> +	       MLX5_ST_SZ_BYTES(odp_cap));
> +
> +	/* set ODP SRQ support for RC/UD and XRC transports */
> +	MLX5_SET(odp_cap, set_hca_cap, ud_odp_caps.srq_receive,
> +		 (MLX5_CAP_ODP_MAX(dev, ud_odp_caps.srq_receive)));
> +
> +	MLX5_SET(odp_cap, set_hca_cap, rc_odp_caps.srq_receive,
> +		 (MLX5_CAP_ODP_MAX(dev, rc_odp_caps.srq_receive)));
> +
> +	MLX5_SET(odp_cap, set_hca_cap, xrc_odp_caps.srq_receive,
> +		 (MLX5_CAP_ODP_MAX(dev, xrc_odp_caps.srq_receive)));
> +

Redundant parentheses around the 3rd parameter. 

> +	err = set_caps(dev, set_ctx, set_sz,
> MLX5_SET_HCA_CAP_OP_MOD_ODP);
> +
> +	kfree(set_ctx);
> +	return err;
> +}
> +
>  static int handle_hca_cap(struct mlx5_core_dev *dev)
>  {
>  	void *set_ctx = NULL;
> @@ -926,6 +973,12 @@ static int mlx5_load_one(struct mlx5_core_dev
> *dev, struct mlx5_priv *priv,
>  		goto reclaim_boot_pages;
>  	}
>  
> +	err = handle_hca_cap_odp(dev);
> +	if (err) {
> +		dev_err(&pdev->dev, "handle_hca_cap_odp failed\n");
> +		goto reclaim_boot_pages;
> +	}
> +
>  	err = mlx5_satisfy_startup_pages(dev, 0);
>  	if (err) {
>  		dev_err(&pdev->dev, "failed to allocate init pages\n");
> diff --git a/include/linux/mlx5/device.h
> b/include/linux/mlx5/device.h
> index 8c4a820bd4c1..0845a227a7b2 100644
> --- a/include/linux/mlx5/device.h
> +++ b/include/linux/mlx5/device.h
> @@ -1201,6 +1201,9 @@ enum mlx5_qcam_feature_groups {
>  #define MLX5_CAP_ODP(mdev, cap)\
>  	MLX5_GET(odp_cap, mdev->caps.hca_cur[MLX5_CAP_ODP], cap)
>  
> +#define MLX5_CAP_ODP_MAX(mdev, cap)\
> +	MLX5_GET(odp_cap, mdev->caps.hca_max[MLX5_CAP_ODP], cap)
> +
>  #define MLX5_CAP_VECTOR_CALC(mdev, cap) \
>  	MLX5_GET(vector_calc_cap, \
>  		 mdev->caps.hca_cur[MLX5_CAP_VECTOR_CALC], cap)
> diff --git a/include/linux/mlx5/mlx5_ifc.h
> b/include/linux/mlx5/mlx5_ifc.h
> index 5407db8ba8e1..c5c679390fbd 100644
> --- a/include/linux/mlx5/mlx5_ifc.h
> +++ b/include/linux/mlx5/mlx5_ifc.h
> @@ -72,6 +72,7 @@ enum {
>  
>  enum {
>  	MLX5_SET_HCA_CAP_OP_MOD_GENERAL_DEVICE        = 0x0,
> +	MLX5_SET_HCA_CAP_OP_MOD_ODP                   = 0x2,
>  	MLX5_SET_HCA_CAP_OP_MOD_ATOMIC                = 0x3,
>  };
>