Re: [PATCH v2] vdpa/mlx5: Fix firmware error on creation of 1k VQs

[Dragos Tatulea]

On Thu, 2023-08-31 at 18:50 +0300, Dragos Tatulea wrote:
> A firmware error is triggered when configuring a 9k MTU on the PF after
> switching to switchdev mode and then using a vdpa device with larger
> (1k) rings:
> mlx5_cmd_out_err: CREATE_GENERAL_OBJECT(0xa00) op_mod(0xd) failed, status bad
> resource(0x5), syndrome (0xf6db90), err(-22)
> 
> This is due to the fact that the hw VQ size parameters are computed
> based on the umem_1/2/3_buffer_param_a/b capabilities and all
> device capabilities are read only when the driver is moved to switchdev mode.
> 
> The problematic configuration flow looks like this:
> 1) Create VF
> 2) Unbind VF
> 3) Switch PF to switchdev mode.
> 4) Bind VF
> 5) Set PF MTU to 9k
> 6) create vDPA device
> 7) Start VM with vDPA device and 1K queue size
> 
> Note that setting the MTU before step 3) doesn't trigger this issue.
> 
> This patch reads the forementioned umem parameters at the latest point
> possible before the VQs of the device are created.
> 
> v2:
> - Allocate output with kmalloc to reduce stack frame size.
> - Removed stable from cc.
> 
Gentle ping. Any thoughts on this fix?

Thanks,
Dragos
> Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices")
> Signed-off-by: Dragos Tatulea <dtatulea@xxxxxxxxxx>
> 
> ---
>  drivers/vdpa/mlx5/net/mlx5_vnet.c | 63 ++++++++++++++++++++++++++-----
>  drivers/vdpa/mlx5/net/mlx5_vnet.h |  9 +++++
>  2 files changed, 63 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 40a03b08d7cf..ef5907b1d513 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -625,30 +625,70 @@ static void cq_destroy(struct mlx5_vdpa_net *ndev, u16
> idx)
>         mlx5_db_free(ndev->mvdev.mdev, &vcq->db);
>  }
>  
> +static int read_umem_params(struct mlx5_vdpa_net *ndev)
> +{
> +       u32 in[MLX5_ST_SZ_DW(query_hca_cap_in)] = {};
> +       u16 opmod = (MLX5_CAP_VDPA_EMULATION << 1) | (HCA_CAP_OPMOD_GET_CUR &
> 0x01);
> +       struct mlx5_core_dev *mdev = ndev->mvdev.mdev;
> +       int out_size;
> +       void *caps;
> +       void *out;
> +       int err;
> +
> +       out_size = MLX5_ST_SZ_BYTES(query_hca_cap_out);
> +       out = kzalloc(out_size, GFP_KERNEL);
> +       if (!out)
> +               return -ENOMEM;
> +
> +       MLX5_SET(query_hca_cap_in, in, opcode, MLX5_CMD_OP_QUERY_HCA_CAP);
> +       MLX5_SET(query_hca_cap_in, in, op_mod, opmod);
> +       err = mlx5_cmd_exec_inout(mdev, query_hca_cap, in, out);
> +       if (err) {
> +               mlx5_vdpa_warn(&ndev->mvdev,
> +                       "Failed reading vdpa umem capabilities with err %d\n",
> err);
> +               goto out;
> +       }
> +
> +       caps =  MLX5_ADDR_OF(query_hca_cap_out, out, capability);
> +
> +       ndev->umem_1_buffer_param_a = MLX5_GET(virtio_emulation_cap, caps,
> umem_1_buffer_param_a);
> +       ndev->umem_1_buffer_param_b = MLX5_GET(virtio_emulation_cap, caps,
> umem_1_buffer_param_b);
> +
> +       ndev->umem_2_buffer_param_a = MLX5_GET(virtio_emulation_cap, caps,
> umem_2_buffer_param_a);
> +       ndev->umem_2_buffer_param_b = MLX5_GET(virtio_emulation_cap, caps,
> umem_2_buffer_param_b);
> +
> +       ndev->umem_3_buffer_param_a = MLX5_GET(virtio_emulation_cap, caps,
> umem_3_buffer_param_a);
> +       ndev->umem_3_buffer_param_b = MLX5_GET(virtio_emulation_cap, caps,
> umem_3_buffer_param_b);
> +
> +out:
> +       kfree(out);
> +       return 0;
> +}
> +
>  static void set_umem_size(struct mlx5_vdpa_net *ndev, struct
> mlx5_vdpa_virtqueue *mvq, int num,
>                           struct mlx5_vdpa_umem **umemp)
>  {
> -       struct mlx5_core_dev *mdev = ndev->mvdev.mdev;
> -       int p_a;
> -       int p_b;
> +       u32 p_a;
> +       u32 p_b;
>  
>         switch (num) {
>         case 1:
> -               p_a = MLX5_CAP_DEV_VDPA_EMULATION(mdev,
> umem_1_buffer_param_a);
> -               p_b = MLX5_CAP_DEV_VDPA_EMULATION(mdev,
> umem_1_buffer_param_b);
> +               p_a = ndev->umem_1_buffer_param_a;
> +               p_b = ndev->umem_1_buffer_param_b;
>                 *umemp = &mvq->umem1;
>                 break;
>         case 2:
> -               p_a = MLX5_CAP_DEV_VDPA_EMULATION(mdev,
> umem_2_buffer_param_a);
> -               p_b = MLX5_CAP_DEV_VDPA_EMULATION(mdev,
> umem_2_buffer_param_b);
> +               p_a = ndev->umem_2_buffer_param_a;
> +               p_b = ndev->umem_2_buffer_param_b;
>                 *umemp = &mvq->umem2;
>                 break;
>         case 3:
> -               p_a = MLX5_CAP_DEV_VDPA_EMULATION(mdev,
> umem_3_buffer_param_a);
> -               p_b = MLX5_CAP_DEV_VDPA_EMULATION(mdev,
> umem_3_buffer_param_b);
> +               p_a = ndev->umem_3_buffer_param_a;
> +               p_b = ndev->umem_3_buffer_param_b;
>                 *umemp = &mvq->umem3;
>                 break;
>         }
> +
>         (*umemp)->size = p_a * mvq->num_ent + p_b;
>  }
>  
> @@ -2679,6 +2719,11 @@ static int setup_driver(struct mlx5_vdpa_dev *mvdev)
>                 goto out;
>         }
>         mlx5_vdpa_add_debugfs(ndev);
> +
> +       err = read_umem_params(ndev);
> +       if (err)
> +               goto err_setup;
> +
>         err = setup_virtqueues(mvdev);
>         if (err) {
>                 mlx5_vdpa_warn(mvdev, "setup_virtqueues\n");
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.h
> b/drivers/vdpa/mlx5/net/mlx5_vnet.h
> index 36c44d9fdd16..65ebbba20662 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.h
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.h
> @@ -65,6 +65,15 @@ struct mlx5_vdpa_net {
>         struct hlist_head macvlan_hash[MLX5V_MACVLAN_SIZE];
>         struct mlx5_vdpa_irq_pool irqp;
>         struct dentry *debugfs;
> +
> +       u32 umem_1_buffer_param_a;
> +       u32 umem_1_buffer_param_b;
> +
> +       u32 umem_2_buffer_param_a;
> +       u32 umem_2_buffer_param_b;
> +
> +       u32 umem_3_buffer_param_a;
> +       u32 umem_3_buffer_param_b;
>  };
>  
>  struct mlx5_vdpa_counter {