RE: [RFC 2/3] vdpa/mlx5: conditionally delete cvq iotlb in destroy_mr

[Eli Cohen]

> From: Eugenio Pérez <eperezma@xxxxxxxxxx>
> Sent: Thursday, 12 January 2023 16:22
> To: mst@xxxxxxxxxx; Eli Cohen <elic@xxxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx; Parav Pandit <parav@xxxxxxxxxx>;
> lulu@xxxxxxxxxx; jasowang@xxxxxxxxxx; virtualization@lists.linux-
> foundation.org; sgarzare@xxxxxxxxxx; si-wei.liu@xxxxxxxxxx
> Subject: [RFC 2/3] vdpa/mlx5: conditionally delete cvq iotlb in destroy_mr
> 
> mlx5_vdpa_destroy_mr can be called by setting a map to data ASID after
> populating control virtqueue ASID iotlb.  Control vq iotlb must not be
> cleared, since it will not be populated again.
> 
> Adding a conditional in the function so the caller specifies if it is
> resetting, cleaning, or just changing data memory.
> 
> Fixes: 8fcd20c30704 ("vdpa/mlx5: Support different address spaces for
> control and data")
> Signed-off-by: Eugenio Pérez <eperezma@xxxxxxxxxx>
> ---
>  drivers/vdpa/mlx5/core/mlx5_vdpa.h |  2 +-
>  drivers/vdpa/mlx5/core/mr.c        |  5 +++--
>  drivers/vdpa/mlx5/net/mlx5_vnet.c  | 12 ++++++------
>  3 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> index 058fbe28107e..000b144019ec 100644
> --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> @@ -119,7 +119,7 @@ int mlx5_vdpa_handle_set_map(struct
> mlx5_vdpa_dev *mvdev, struct vhost_iotlb *io
>  			     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);
> +void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev, bool
> delete_cvq_iotlb);
> 
>  #define mlx5_vdpa_warn(__dev, format, ...)
> \
>  	dev_warn((__dev)->mdev->device, "%s:%d:(pid %d) warning: "
> format, __func__, __LINE__,     \
> diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> index ae34dcac9a3f..878ee94efa78 100644
> --- a/drivers/vdpa/mlx5/core/mr.c
> +++ b/drivers/vdpa/mlx5/core/mr.c
> @@ -491,7 +491,7 @@ static void destroy_user_mr(struct mlx5_vdpa_dev
> *mvdev, struct mlx5_vdpa_mr *mr
>  	}
>  }
> 
> -void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev)
> +void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev, bool
> delete_cvq_iotlb)
>  {
>  	struct mlx5_vdpa_mr *mr = &mvdev->mr;
> 
> @@ -499,7 +499,8 @@ void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev
> *mvdev)
>  	if (!mr->initialized)
>  		goto out;
> 
> -	prune_iotlb(mvdev);
> +	if (delete_cvq_iotlb)
> +		prune_iotlb(mvdev);
>  	if (mr->user_mr)
>  		destroy_user_mr(mvdev, mr);
>  	else
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 6632651b1e54..1f1f341f602b 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -2433,7 +2433,7 @@ static int mlx5_vdpa_change_map(struct
> mlx5_vdpa_dev *mvdev,
>  		goto err_mr;
> 
>  	teardown_driver(ndev);
> -	mlx5_vdpa_destroy_mr(mvdev);
> +	mlx5_vdpa_destroy_mr(mvdev, mvdev-
> >group2asid[MLX5_VDPA_CVQ_GROUP] == asid);

Looks to me we need to handle this in a more generic manner. The asid should be used conditionally for either CVQ or data VQ updates. You are protecting CVQ but same thing should hold also for data VQs iotlb. Meaning, if qemu wants to update only CVQ than data VQ translation must not be affected.

>  	err = mlx5_vdpa_create_mr(mvdev, iotlb, asid);
>  	if (err)
>  		goto err_mr;
> @@ -2449,7 +2449,7 @@ static int mlx5_vdpa_change_map(struct
> mlx5_vdpa_dev *mvdev,
>  	return 0;
> 
>  err_setup:
> -	mlx5_vdpa_destroy_mr(mvdev);
> +	mlx5_vdpa_destroy_mr(mvdev, mvdev-
> >group2asid[MLX5_VDPA_CVQ_GROUP] == asid);
>  err_mr:
>  	return err;
>  }
> @@ -2578,7 +2578,7 @@ static void mlx5_vdpa_set_status(struct
> vdpa_device *vdev, u8 status)
>  	return;
> 
>  err_setup:
> -	mlx5_vdpa_destroy_mr(&ndev->mvdev);
> +	mlx5_vdpa_destroy_mr(&ndev->mvdev, true);
>  	ndev->mvdev.status |= VIRTIO_CONFIG_S_FAILED;
>  err_clear:
>  	up_write(&ndev->reslock);
> @@ -2604,7 +2604,7 @@ static int mlx5_vdpa_reset(struct vdpa_device
> *vdev)
>  	down_write(&ndev->reslock);
>  	teardown_driver(ndev);
>  	clear_vqs_ready(ndev);
> -	mlx5_vdpa_destroy_mr(&ndev->mvdev);
> +	mlx5_vdpa_destroy_mr(&ndev->mvdev, true);
>  	ndev->mvdev.status = 0;
>  	ndev->cur_num_vqs = 0;
>  	ndev->mvdev.cvq.received_desc = 0;
> @@ -2691,7 +2691,7 @@ static void mlx5_vdpa_free(struct vdpa_device
> *vdev)
>  	ndev = to_mlx5_vdpa_ndev(mvdev);
> 
>  	free_resources(ndev);
> -	mlx5_vdpa_destroy_mr(mvdev);
> +	mlx5_vdpa_destroy_mr(mvdev, true);
>  	if (!is_zero_ether_addr(ndev->config.mac)) {
>  		pfmdev = pci_get_drvdata(pci_physfn(mvdev->mdev->pdev));
>  		mlx5_mpfs_del_mac(pfmdev, ndev->config.mac);
> @@ -3214,7 +3214,7 @@ static int mlx5_vdpa_dev_add(struct
> vdpa_mgmt_dev *v_mdev, const char *name,
>  err_res2:
>  	free_resources(ndev);
>  err_mr:
> -	mlx5_vdpa_destroy_mr(mvdev);
> +	mlx5_vdpa_destroy_mr(mvdev, true);
>  err_res:
>  	mlx5_vdpa_free_resources(&ndev->mvdev);
>  err_mpfs:
> --
> 2.31.1