Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
MIME-Version: 1.0
References: <20230912130132.561193-1-dtatulea@nvidia.com> <20230912130132.561193-6-dtatulea@nvidia.com>
In-Reply-To: <20230912130132.561193-6-dtatulea@nvidia.com>
From:   Jason Wang <jasowang@redhat.com>
Date:   Tue, 26 Sep 2023 11:28:45 +0800
Message-ID: <CACGkMEuB93-gmyCrfeubT5p-i-4M96UmkDJbuDMCAu5LXSyitA@mail.gmail.com>
Subject: Re: [PATCH 05/16] vdpa/mlx5: Decouple cvq iotlb handling from hw
 mapping code
To:     Dragos Tatulea <dtatulea@nvidia.com>
Cc:     =?UTF-8?Q?Eugenio_P=C3=A9rez?= <eperezma@redhat.com>,
        Si-Wei Liu <si-wei.liu@oracle.com>,
        "Michael S . Tsirkin" <mst@redhat.com>,
        Saeed Mahameed <saeedm@nvidia.com>,
        virtualization@lists.linux-foundation.org,
        linux-kernel@vger.kernel.org, Parav Pandit <parav@nvidia.com>,
        Xuan Zhuo <xuanzhuo@linux.alibaba.com>, kvm@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Sep 12, 2023 at 9:02=E2=80=AFPM Dragos Tatulea <dtatulea@nvidia.com=
> wrote:
>
> The handling of the cvq iotlb is currently coupled with the creation
> and destruction of the hardware mkeys (mr).
>
> This patch moves cvq iotlb handling into its own function and shifts it
> to a scope that is not related to mr handling. As cvq handling is just a
> prune_iotlb + dup_iotlb cycle, put it all in the same "update" function.
> Finally, the destruction path is handled by directly pruning the iotlb.
>
> After this move is done the ASID mr code can be collapsed into a single
> function.
>
> Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
> ---
>  drivers/vdpa/mlx5/core/mlx5_vdpa.h |  3 ++
>  drivers/vdpa/mlx5/core/mr.c        | 57 +++++++++++-------------------
>  drivers/vdpa/mlx5/net/mlx5_vnet.c  |  7 ++--
>  3 files changed, 28 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/=
mlx5_vdpa.h
> index 3748f027cfe9..554899a80241 100644
> --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> @@ -120,6 +120,9 @@ int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, =
struct vhost_iotlb *iotlb,
>                         unsigned int asid);
>  void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev);
>  void mlx5_vdpa_destroy_mr_asid(struct mlx5_vdpa_dev *mvdev, unsigned int=
 asid);
> +int mlx5_vdpa_update_cvq_iotlb(struct mlx5_vdpa_dev *mvdev,
> +                               struct vhost_iotlb *iotlb,
> +                               unsigned int asid);
>  int mlx5_vdpa_create_dma_mr(struct mlx5_vdpa_dev *mvdev);
>
>  #define mlx5_vdpa_warn(__dev, format, ...)                              =
                           \
> diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> index 7bd0883b8b25..fcb6ae32e9ed 100644
> --- a/drivers/vdpa/mlx5/core/mr.c
> +++ b/drivers/vdpa/mlx5/core/mr.c
> @@ -489,14 +489,6 @@ static void destroy_user_mr(struct mlx5_vdpa_dev *mv=
dev, struct mlx5_vdpa_mr *mr
>         }
>  }
>
> -static void _mlx5_vdpa_destroy_cvq_mr(struct mlx5_vdpa_dev *mvdev, unsig=
ned int asid)
> -{
> -       if (mvdev->group2asid[MLX5_VDPA_CVQ_GROUP] !=3D asid)
> -               return;
> -
> -       prune_iotlb(mvdev);
> -}
> -
>  static void _mlx5_vdpa_destroy_dvq_mr(struct mlx5_vdpa_dev *mvdev, unsig=
ned int asid)
>  {
>         struct mlx5_vdpa_mr *mr =3D &mvdev->mr;
> @@ -522,25 +514,14 @@ void mlx5_vdpa_destroy_mr_asid(struct mlx5_vdpa_dev=
 *mvdev, unsigned int asid)
>         mutex_lock(&mr->mkey_mtx);
>
>         _mlx5_vdpa_destroy_dvq_mr(mvdev, asid);
> -       _mlx5_vdpa_destroy_cvq_mr(mvdev, asid);
>
>         mutex_unlock(&mr->mkey_mtx);
>  }
>
>  void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev)
>  {
> -       mlx5_vdpa_destroy_mr_asid(mvdev, mvdev->group2asid[MLX5_VDPA_CVQ_=
GROUP]);
>         mlx5_vdpa_destroy_mr_asid(mvdev, mvdev->group2asid[MLX5_VDPA_DATA=
VQ_GROUP]);
> -}
> -
> -static int _mlx5_vdpa_create_cvq_mr(struct mlx5_vdpa_dev *mvdev,
> -                                   struct vhost_iotlb *iotlb,
> -                                   unsigned int asid)
> -{
> -       if (mvdev->group2asid[MLX5_VDPA_CVQ_GROUP] !=3D asid)
> -               return 0;
> -
> -       return dup_iotlb(mvdev, iotlb);
> +       prune_iotlb(mvdev);
>  }
>
>  static int _mlx5_vdpa_create_dvq_mr(struct mlx5_vdpa_dev *mvdev,
> @@ -572,22 +553,7 @@ static int _mlx5_vdpa_create_dvq_mr(struct mlx5_vdpa=
_dev *mvdev,
>  static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev,
>                                 struct vhost_iotlb *iotlb, unsigned int a=
sid)
>  {
> -       int err;
> -
> -       err =3D _mlx5_vdpa_create_dvq_mr(mvdev, iotlb, asid);
> -       if (err)
> -               return err;
> -
> -       err =3D _mlx5_vdpa_create_cvq_mr(mvdev, iotlb, asid);
> -       if (err)
> -               goto out_err;
> -
> -       return 0;
> -
> -out_err:
> -       _mlx5_vdpa_destroy_dvq_mr(mvdev, asid);
> -
> -       return err;
> +       return _mlx5_vdpa_create_dvq_mr(mvdev, iotlb, asid);
>  }
>
>  int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb =
*iotlb,
> @@ -620,7 +586,24 @@ int mlx5_vdpa_handle_set_map(struct mlx5_vdpa_dev *m=
vdev, struct vhost_iotlb *io
>         return err;
>  }
>
> +int mlx5_vdpa_update_cvq_iotlb(struct mlx5_vdpa_dev *mvdev,
> +                               struct vhost_iotlb *iotlb,
> +                               unsigned int asid)
> +{
> +       if (mvdev->group2asid[MLX5_VDPA_CVQ_GROUP] !=3D asid)
> +               return 0;
> +
> +       prune_iotlb(mvdev);
> +       return dup_iotlb(mvdev, iotlb);
> +}
> +
>  int mlx5_vdpa_create_dma_mr(struct mlx5_vdpa_dev *mvdev)
>  {
> -       return mlx5_vdpa_create_mr(mvdev, NULL, 0);
> +       int err;
> +
> +       err =3D mlx5_vdpa_create_mr(mvdev, NULL, 0);
> +       if (err)
> +               return err;
> +
> +       return mlx5_vdpa_update_cvq_iotlb(mvdev, NULL, 0);
>  }

Nit: Still a little bit coupling but anyhow:

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/ml=
x5_vnet.c
> index d34c19b4e139..061d8f7a661a 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -2884,10 +2884,13 @@ static int set_map_data(struct mlx5_vdpa_dev *mvd=
ev, struct vhost_iotlb *iotlb,
>                 return err;
>         }
>
> -       if (change_map)
> +       if (change_map) {
>                 err =3D mlx5_vdpa_change_map(mvdev, iotlb, asid);
> +               if (err)
> +                       return err;
> +       }
>
> -       return err;
> +       return mlx5_vdpa_update_cvq_iotlb(mvdev, iotlb, asid);
>  }
>
>  static int mlx5_vdpa_set_map(struct vdpa_device *vdev, unsigned int asid=
,
> --
> 2.41.0
>