[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACGkMEuTXO+PB+z6PpOqvov-yRhkZfXSLvs6N-_9ikixsrr-kA@mail.gmail.com>
Date: Tue, 15 Nov 2022 11:33:09 +0800
From: Jason Wang <jasowang@...hat.com>
To: Eli Cohen <elic@...dia.com>
Cc: mst@...hat.com, linux-kernel@...r.kernel.org,
virtualization@...ts.linux-foundation.org, si-wei.liu@...cle.com,
eperezma@...hat.com, lulu@...hat.com
Subject: Re: [PATH v2 5/8] vdpa/mlx5: Avoid overwriting CVQ iotlb
On Mon, Nov 14, 2022 at 9:18 PM Eli Cohen <elic@...dia.com> wrote:
>
> When qemu uses different address spaces for data and control virtqueues,
> the current code would overwrite the control virtqueue iotlb through the
> dup_iotlb call. Fix this by referring to the address space identifier
> and the group to asid mapping to determine which mapping needs to be
> updated. We also move the address space logic from mlx5 net to core
> directory.
>
> Reported-by: Eugenio PĂ©rez <eperezma@...hat.com>
> Signed-off-by: Eli Cohen <elic@...dia.com>
Acked-by: Jason Wang <jasowang@...hat.com>
Thanks
> ---
> drivers/vdpa/mlx5/core/mlx5_vdpa.h | 5 +--
> drivers/vdpa/mlx5/core/mr.c | 44 ++++++++++++++++-----------
> drivers/vdpa/mlx5/net/mlx5_vnet.c | 49 ++++++------------------------
> 3 files changed, 39 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> index 6af9fdbb86b7..058fbe28107e 100644
> --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> @@ -116,8 +116,9 @@ int mlx5_vdpa_create_mkey(struct mlx5_vdpa_dev *mvdev, u32 *mkey, u32 *in,
> int inlen);
> int mlx5_vdpa_destroy_mkey(struct mlx5_vdpa_dev *mvdev, u32 mkey);
> int mlx5_vdpa_handle_set_map(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb,
> - bool *change_map);
> -int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb);
> + bool *change_map, unsigned int asid);
> +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);
>
> #define mlx5_vdpa_warn(__dev, format, ...) \
> diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> index a639b9208d41..a4d7ee2339fa 100644
> --- a/drivers/vdpa/mlx5/core/mr.c
> +++ b/drivers/vdpa/mlx5/core/mr.c
> @@ -511,7 +511,8 @@ void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev)
> mutex_unlock(&mr->mkey_mtx);
> }
>
> -static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> +static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev,
> + struct vhost_iotlb *iotlb, unsigned int asid)
> {
> struct mlx5_vdpa_mr *mr = &mvdev->mr;
> int err;
> @@ -519,42 +520,49 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
> if (mr->initialized)
> return 0;
>
> - if (iotlb)
> - err = create_user_mr(mvdev, iotlb);
> - else
> - err = create_dma_mr(mvdev, mr);
> + if (mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] == asid) {
> + if (iotlb)
> + err = create_user_mr(mvdev, iotlb);
> + else
> + err = create_dma_mr(mvdev, mr);
>
> - if (err)
> - return err;
> + if (err)
> + return err;
> + }
>
> - err = dup_iotlb(mvdev, iotlb);
> - if (err)
> - goto out_err;
> + if (mvdev->group2asid[MLX5_VDPA_CVQ_GROUP] == asid) {
> + err = dup_iotlb(mvdev, iotlb);
> + if (err)
> + goto out_err;
> + }
>
> mr->initialized = true;
> return 0;
>
> out_err:
> - if (iotlb)
> - destroy_user_mr(mvdev, mr);
> - else
> - destroy_dma_mr(mvdev, mr);
> + if (mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] == asid) {
> + if (iotlb)
> + destroy_user_mr(mvdev, mr);
> + else
> + destroy_dma_mr(mvdev, mr);
> + }
>
> return err;
> }
>
> -int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> +int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb,
> + unsigned int asid)
> {
> int err;
>
> mutex_lock(&mvdev->mr.mkey_mtx);
> - err = _mlx5_vdpa_create_mr(mvdev, iotlb);
> + err = _mlx5_vdpa_create_mr(mvdev, iotlb, asid);
> mutex_unlock(&mvdev->mr.mkey_mtx);
> return err;
> }
>
> int mlx5_vdpa_handle_set_map(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb,
> - bool *change_map)
> + bool *change_map, unsigned int asid)
> {
> struct mlx5_vdpa_mr *mr = &mvdev->mr;
> int err = 0;
> @@ -566,7 +574,7 @@ int mlx5_vdpa_handle_set_map(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *io
> *change_map = true;
> }
> if (!*change_map)
> - err = _mlx5_vdpa_create_mr(mvdev, iotlb);
> + err = _mlx5_vdpa_create_mr(mvdev, iotlb, asid);
> mutex_unlock(&mr->mkey_mtx);
>
> return err;
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 98dd8ce8af26..3a6dbbc6440d 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -2394,7 +2394,8 @@ static void restore_channels_info(struct mlx5_vdpa_net *ndev)
> }
> }
>
> -static int mlx5_vdpa_change_map(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> +static int mlx5_vdpa_change_map(struct mlx5_vdpa_dev *mvdev,
> + struct vhost_iotlb *iotlb, unsigned int asid)
> {
> struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
> int err;
> @@ -2406,7 +2407,7 @@ static int mlx5_vdpa_change_map(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
>
> teardown_driver(ndev);
> mlx5_vdpa_destroy_mr(mvdev);
> - err = mlx5_vdpa_create_mr(mvdev, iotlb);
> + err = mlx5_vdpa_create_mr(mvdev, iotlb, asid);
> if (err)
> goto err_mr;
>
> @@ -2587,7 +2588,7 @@ static int mlx5_vdpa_reset(struct vdpa_device *vdev)
> ++mvdev->generation;
>
> if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0)) {
> - if (mlx5_vdpa_create_mr(mvdev, NULL))
> + if (mlx5_vdpa_create_mr(mvdev, NULL, 0))
> mlx5_vdpa_warn(mvdev, "create MR failed\n");
> }
> up_write(&ndev->reslock);
> @@ -2623,41 +2624,20 @@ static u32 mlx5_vdpa_get_generation(struct vdpa_device *vdev)
> return mvdev->generation;
> }
>
> -static int set_map_control(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> -{
> - u64 start = 0ULL, last = 0ULL - 1;
> - struct vhost_iotlb_map *map;
> - int err = 0;
> -
> - spin_lock(&mvdev->cvq.iommu_lock);
> - vhost_iotlb_reset(mvdev->cvq.iotlb);
> -
> - for (map = vhost_iotlb_itree_first(iotlb, start, last); map;
> - map = vhost_iotlb_itree_next(map, start, last)) {
> - err = vhost_iotlb_add_range(mvdev->cvq.iotlb, map->start,
> - map->last, map->addr, map->perm);
> - if (err)
> - goto out;
> - }
> -
> -out:
> - spin_unlock(&mvdev->cvq.iommu_lock);
> - return err;
> -}
> -
> -static int set_map_data(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> +static int set_map_data(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb,
> + unsigned int asid)
> {
> bool change_map;
> int err;
>
> - err = mlx5_vdpa_handle_set_map(mvdev, iotlb, &change_map);
> + err = mlx5_vdpa_handle_set_map(mvdev, iotlb, &change_map, asid);
> if (err) {
> mlx5_vdpa_warn(mvdev, "set map failed(%d)\n", err);
> return err;
> }
>
> if (change_map)
> - err = mlx5_vdpa_change_map(mvdev, iotlb);
> + err = mlx5_vdpa_change_map(mvdev, iotlb, asid);
>
> return err;
> }
> @@ -2670,16 +2650,7 @@ static int mlx5_vdpa_set_map(struct vdpa_device *vdev, unsigned int asid,
> int err = -EINVAL;
>
> down_write(&ndev->reslock);
> - if (mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] == asid) {
> - err = set_map_data(mvdev, iotlb);
> - if (err)
> - goto out;
> - }
> -
> - if (mvdev->group2asid[MLX5_VDPA_CVQ_GROUP] == asid)
> - err = set_map_control(mvdev, iotlb);
> -
> -out:
> + err = set_map_data(mvdev, iotlb, asid);
> up_write(&ndev->reslock);
> return err;
> }
> @@ -3182,7 +3153,7 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
> goto err_mpfs;
>
> if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0)) {
> - err = mlx5_vdpa_create_mr(mvdev, NULL);
> + err = mlx5_vdpa_create_mr(mvdev, NULL, 0);
> if (err)
> goto err_res;
> }
> --
> 2.38.1
>
Powered by blists - more mailing lists