Messages in this thread | | | From | Eugenio Perez Martin <> | Date | Mon, 16 Jan 2023 19:03:47 +0100 | Subject | Re: [RFC 2/3] vdpa/mlx5: conditionally delete cvq iotlb in destroy_mr |
| |
On Mon, Jan 16, 2023 at 8:03 AM Eli Cohen <elic@nvidia.com> wrote: > > > From: Eugenio Pérez <eperezma@redhat.com> > > Sent: Thursday, 12 January 2023 16:22 > > To: mst@redhat.com; Eli Cohen <elic@nvidia.com> > > Cc: linux-kernel@vger.kernel.org; Parav Pandit <parav@nvidia.com>; > > lulu@redhat.com; jasowang@redhat.com; virtualization@lists.linux- > > foundation.org; sgarzare@redhat.com; si-wei.liu@oracle.com > > 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@redhat.com> > > --- > > 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.
I agree. Maybe the best option is to replace the boolean indicating the ASID we want to destroy mr? Then, at cleanup, we can iterate by all vq groups / ASID.
> Meaning, if qemu wants to update only CVQ than data VQ translation must not be affected.
_mlx5_vdpa_create_mr. is the one that checks it If I recall correctly. It is not obvious in this change though.
Thanks!
> > > 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 >
| |