On Thu, Jan 05, 2023 at 04:58:22PM -0800, Saeed Mahameed wrote:
> On 04 Jan 10:11, Leon Romanovsky wrote:
> > From: Or Har-Toov <ohartoov@xxxxxxxxxx>
> >
> > Using query_sepcial_contexts in order to get the correct value of
> > terminate_scatter_list_mkey, as FW will change it in some configurations.
> > This is done one time when the device is loading and the value is being
> > saved in the device context.
> >
> > Signed-off-by: Or Har-Toov <ohartoov@xxxxxxxxxx>
> > Reviewed-by: Michael Guralnik <michaelgur@xxxxxxxxxx>
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx>
> > ---
> > .../net/ethernet/mellanox/mlx5/core/en_main.c | 2 +-
> > .../net/ethernet/mellanox/mlx5/core/main.c | 27 +++++++++++++++++++
> > include/linux/mlx5/driver.h | 1 +
> > 3 files changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > index c76f15505a76..33d7a7095988 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > @@ -826,7 +826,7 @@ static int mlx5e_alloc_rq(struct mlx5e_params *params,
> > if (rq->wqe.info.num_frags < (1 << rq->wqe.info.log_num_frags)) {
> > wqe->data[f].byte_count = 0;
> > wqe->data[f].lkey =
> > - MLX5_TERMINATE_SCATTER_LIST_LKEY;
> > + mdev->terminate_scatter_list_mkey;
> > wqe->data[f].addr = 0;
> > }
> > }
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> > index 7f5db13e3550..d39d758744a0 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> > @@ -1221,6 +1221,28 @@ static int mlx5_function_teardown(struct mlx5_core_dev *dev, bool boot)
> > return 0;
> > }
> >
> > +static int mlx5_get_terminate_scatter_list_mkey(struct mlx5_core_dev *dev)
> > +{
> > + u32 out[MLX5_ST_SZ_DW(query_special_contexts_out)] = {};
> > + u32 in[MLX5_ST_SZ_DW(query_special_contexts_in)] = {};
> > + int err;
> > +
> > + MLX5_SET(query_special_contexts_in, in, opcode,
> > + MLX5_CMD_OP_QUERY_SPECIAL_CONTEXTS);
> > + err = mlx5_cmd_exec_inout(dev, query_special_contexts, in, out);
> > + if (err)
> > + return err;
> > +
> > + if (MLX5_CAP_GEN(dev, terminate_scatter_list_mkey)) {
>
> Why did you execute the command unconditionally if the output is only read
> conditionally?
>
> early exit before executing the command, older FW might fail and driver will
> be unusable ..
According to the documentation, this functionality was forever, but you
are presenting valid concern.
>
> > + dev->terminate_scatter_list_mkey =
> > + cpu_to_be32(MLX5_GET(query_special_contexts_out, out,
> > + terminate_scatter_list_mkey));
> > + return 0;
> > + }
> > + dev->terminate_scatter_list_mkey = MLX5_TERMINATE_SCATTER_LIST_LKEY;
>
> Another concern, what happens with old driver that is using the hardcoded
> value with newer fw ? will it fail ? will it be accepted ?
It will be accepted and everything will work without glitches.
FW keeps backward compatibility and use MLX5_TERMINATE_SCATTER_LIST_LKEY
internally as a default.
>
> The commit message doesn't explain what's going on here very well,
> Let's discuss internally before you submit V2.
NP.
>
> > + return 0;
> > +}
> > +
> > static int mlx5_load(struct mlx5_core_dev *dev)
> > {
> > int err;
> > @@ -1235,6 +1257,11 @@ static int mlx5_load(struct mlx5_core_dev *dev)
> > mlx5_events_start(dev);
> > mlx5_pagealloc_start(dev);
> >
> > + err = mlx5_get_terminate_scatter_list_mkey(dev);
> > + if (err) {
> > + mlx5_core_err(dev, "Failed to query special contexts\n");
> print the err;
> > + goto err_irq_table;
> > + }
>
> you are querying FW too soon, no issue here but better if you move it after
> mlx5_eq_table_create() to use the command EQ rather than the primitive cmd
> interface.
I'll take a look.
Thanks
>
> > err = mlx5_irq_table_create(dev);
> > if (err) {
> > mlx5_core_err(dev, "Failed to alloc IRQs\n");
> > diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
> > index d476255c9a3f..5f2c4038d638 100644
> > --- a/include/linux/mlx5/driver.h
> > +++ b/include/linux/mlx5/driver.h
> > @@ -801,6 +801,7 @@ struct mlx5_core_dev {
> > struct mlx5_rsc_dump *rsc_dump;
> > u32 vsc_addr;
> > struct mlx5_hv_vhca *hv_vhca;
> > + __be32 terminate_scatter_list_mkey;
> > };
> >
> > struct mlx5_db {
> > --
> > 2.38.1
> >
