Re: [RFC 2/3] vdpa/mlx5: conditionally delete cvq iotlb in destroy_mr
From: Eli Cohen
Date: Tue Jan 17 2023 - 02:08:11 EST
On 16/01/2023 20:03, Eugenio Perez Martin wrote:
On Mon, Jan 16, 2023 at 8:03 AM Eli Cohen <elic@xxxxxxxxxx> wrote:
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.
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.
I think mlx5_vdpa_destroy_mr() should get the asid as an argument and
have a logic such as:
if (asid == data asid)
destroy_data_mr
if (asid == ctrl_vq_asid)
prune_iotlb()
return
Since we have only two groups, one for data and for control, I don't
think we need to iterate.
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