Hi Michael,
The following two patches add support for mlx5_vdpa to be driven by
virtio_vdpa. This requires the ability to create virtqueue resources
with uid equals zero.
The first patch makes use of uid 0 if supported.
The second one handles the memory registration.
Eli Cohen (2):
vdpa/mlx5: Support creating resources with uid == 0
vdpa/mlx5: Add support for running with virtio_vdpa
drivers/vdpa/mlx5/core/mlx5_vdpa.h | 1 +
drivers/vdpa/mlx5/core/mr.c | 81 ++++++++++++++++++++++++------
drivers/vdpa/mlx5/core/resources.c | 6 +++
drivers/vdpa/mlx5/net/mlx5_vnet.c | 13 ++++-
include/linux/mlx5/mlx5_ifc.h | 4 +-
5 files changed, 89 insertions(+), 16 deletions(-)
--
2.31.1
In order to support running vdpa using vritio_vdpa driver, we need to
create a different kind of MR, one that has 1:1 mapping, since the
addresses referring to virtqueues are dma addresses.
We create the 1:1 MR in mlx5_vdpa_dev_add() only in case firmware
supports the general capability umem_uid_0. The reason for that is that
1:1 MRs must be created with uid == 0 while virtqueue objects can be
created with uid == 0 only when the firmware capability is on.
If the set_map() callback is called with new translations provided
through iotlb, the driver will destroy the 1:1 MR and create a regular
one.
Signed-off-by: Eli Cohen <[email protected]>
---
drivers/vdpa/mlx5/core/mlx5_vdpa.h | 1 +
drivers/vdpa/mlx5/core/mr.c | 81 ++++++++++++++++++++++++------
drivers/vdpa/mlx5/net/mlx5_vnet.c | 13 ++++-
3 files changed, 80 insertions(+), 15 deletions(-)
diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
index b6cc53ba980c..09a16a3d1b2a 100644
--- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
+++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
@@ -35,6 +35,7 @@ struct mlx5_vdpa_mr {
/* serialize mkey creation and destruction */
struct mutex mkey_mtx;
+ bool user_mr;
};
struct mlx5_vdpa_resources {
diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
index 800cfd1967ad..020c0ce4d203 100644
--- a/drivers/vdpa/mlx5/core/mr.c
+++ b/drivers/vdpa/mlx5/core/mr.c
@@ -360,7 +360,7 @@ static int add_direct_chain(struct mlx5_vdpa_dev *mvdev, u64 start, u64 size, u8
* indirect memory key that provides access to the enitre address space given
* by iotlb.
*/
-static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
+static int create_user_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
{
struct mlx5_vdpa_mr *mr = &mvdev->mr;
struct mlx5_vdpa_direct_mr *dmr;
@@ -374,9 +374,6 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
int err = 0;
int nnuls;
- if (mr->initialized)
- return 0;
-
INIT_LIST_HEAD(&mr->head);
for (map = vhost_iotlb_itree_first(iotlb, start, last); map;
map = vhost_iotlb_itree_next(map, start, last)) {
@@ -414,7 +411,7 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
if (err)
goto err_chain;
- mr->initialized = true;
+ mr->user_mr = true;
return 0;
err_chain:
@@ -426,33 +423,89 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
return err;
}
-int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
+static int create_dma_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr)
+{
+ int inlen = MLX5_ST_SZ_BYTES(create_mkey_in);
+ void *mkc;
+ u32 *in;
+ int err;
+
+ in = kzalloc(inlen, GFP_KERNEL);
+ if (!in)
+ return -ENOMEM;
+
+ mkc = MLX5_ADDR_OF(create_mkey_in, in, memory_key_mkey_entry);
+
+ MLX5_SET(mkc, mkc, access_mode_1_0, MLX5_MKC_ACCESS_MODE_PA);
+ MLX5_SET(mkc, mkc, length64, 1);
+ MLX5_SET(mkc, mkc, lw, 1);
+ MLX5_SET(mkc, mkc, lr, 1);
+ MLX5_SET(mkc, mkc, pd, mvdev->res.pdn);
+ MLX5_SET(mkc, mkc, qpn, 0xffffff);
+
+ err = mlx5_vdpa_create_mkey(mvdev, &mr->mkey, in, inlen);
+ kfree(in);
+ return err;
+}
+
+static void destroy_dma_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr)
+{
+ mlx5_vdpa_destroy_mkey(mvdev, &mr->mkey);
+}
+
+static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
{
struct mlx5_vdpa_mr *mr = &mvdev->mr;
int err;
- mutex_lock(&mr->mkey_mtx);
+ if (mr->initialized)
+ return 0;
+
+ if (iotlb)
+ err = create_user_mr(mvdev, iotlb);
+ else
+ err = create_dma_mr(mvdev, mr);
+
+ mr->initialized = true;
+ return err;
+}
+
+int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
+{
+ int err;
+
+ mutex_lock(&mvdev->mr.mkey_mtx);
err = _mlx5_vdpa_create_mr(mvdev, iotlb);
- mutex_unlock(&mr->mkey_mtx);
+ mutex_unlock(&mvdev->mr.mkey_mtx);
return err;
}
-void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev)
+static void destroy_user_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr)
{
- struct mlx5_vdpa_mr *mr = &mvdev->mr;
struct mlx5_vdpa_direct_mr *dmr;
struct mlx5_vdpa_direct_mr *n;
- mutex_lock(&mr->mkey_mtx);
- if (!mr->initialized)
- goto out;
-
destroy_indirect_key(mvdev, mr);
list_for_each_entry_safe_reverse(dmr, n, &mr->head, list) {
list_del_init(&dmr->list);
unmap_direct_mr(mvdev, dmr);
kfree(dmr);
}
+}
+
+void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev)
+{
+ struct mlx5_vdpa_mr *mr = &mvdev->mr;
+
+ mutex_lock(&mr->mkey_mtx);
+ if (!mr->initialized)
+ goto out;
+
+ if (mr->user_mr)
+ destroy_user_mr(mvdev, mr);
+ else
+ destroy_dma_mr(mvdev, mr);
+
memset(mr, 0, sizeof(*mr));
mr->initialized = false;
out:
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index fdf3e74bffbd..f16756661c19 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1780,6 +1780,8 @@ static void mlx5_vdpa_set_status(struct vdpa_device *vdev, u8 status)
ndev->mvdev.status = 0;
ndev->mvdev.mlx_features = 0;
++mvdev->generation;
+ if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0))
+ mlx5_vdpa_create_mr(mvdev, NULL);
return;
}
@@ -1859,6 +1861,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_free_resources(&ndev->mvdev);
mutex_destroy(&ndev->reslock);
}
@@ -2023,9 +2026,15 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name)
if (err)
goto err_mtu;
+ if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0)) {
+ err = mlx5_vdpa_create_mr(mvdev, NULL);
+ if (err)
+ goto err_res;
+ }
+
err = alloc_resources(ndev);
if (err)
- goto err_res;
+ goto err_mr;
mvdev->vdev.mdev = &mgtdev->mgtdev;
err = _vdpa_register_device(&mvdev->vdev, 2 * mlx5_vdpa_max_qps(max_vqs));
@@ -2037,6 +2046,8 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name)
err_reg:
free_resources(ndev);
+err_mr:
+ mlx5_vdpa_destroy_mr(mvdev);
err_res:
mlx5_vdpa_free_resources(&ndev->mvdev);
err_mtu:
--
2.31.1
Currently all resources must be created with uid != 0 which is essential
userspace processes allocating virtquueue resources. Since this is a
kernel implementation, it is perfectly legal to open resources with
uid == 0.
In case frimware supports, avoid allocating user context.
Signed-off-by: Eli Cohen <[email protected]>
---
drivers/vdpa/mlx5/core/resources.c | 6 ++++++
include/linux/mlx5/mlx5_ifc.h | 4 +++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/vdpa/mlx5/core/resources.c b/drivers/vdpa/mlx5/core/resources.c
index 6521cbd0f5c2..836ab9ef0fa6 100644
--- a/drivers/vdpa/mlx5/core/resources.c
+++ b/drivers/vdpa/mlx5/core/resources.c
@@ -54,6 +54,9 @@ static int create_uctx(struct mlx5_vdpa_dev *mvdev, u16 *uid)
void *in;
int err;
+ if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0))
+ return 0;
+
/* 0 means not supported */
if (!MLX5_CAP_GEN(mvdev->mdev, log_max_uctx))
return -EOPNOTSUPP;
@@ -79,6 +82,9 @@ static void destroy_uctx(struct mlx5_vdpa_dev *mvdev, u32 uid)
u32 out[MLX5_ST_SZ_DW(destroy_uctx_out)] = {};
u32 in[MLX5_ST_SZ_DW(destroy_uctx_in)] = {};
+ if (!uid)
+ return;
+
MLX5_SET(destroy_uctx_in, in, opcode, MLX5_CMD_OP_DESTROY_UCTX);
MLX5_SET(destroy_uctx_in, in, uid, uid);
diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index 9c68b2da14c6..606d2aeacad4 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -1487,7 +1487,9 @@ struct mlx5_ifc_cmd_hca_cap_bits {
u8 uar_4k[0x1];
u8 reserved_at_241[0x9];
u8 uar_sz[0x6];
- u8 reserved_at_250[0x8];
+ u8 reserved_at_248[0x2];
+ u8 umem_uid_0[0x1];
+ u8 reserved_at_250[0x5];
u8 log_pg_sz[0x8];
u8 bf[0x1];
--
2.31.1
?? 2021/5/30 ????3:54, Eli Cohen д??:
> Currently all resources must be created with uid != 0 which is essential
> userspace processes allocating virtquueue resources. Since this is a
> kernel implementation, it is perfectly legal to open resources with
> uid == 0.
>
> In case frimware supports, avoid allocating user context.
Typo "frimware".
Otherwise.
Acked-by: Jason Wang <[email protected]>
(I don't see any code to check the firmware capability, is this intended?)
Thanks
>
> Signed-off-by: Eli Cohen <[email protected]>
> ---
> drivers/vdpa/mlx5/core/resources.c | 6 ++++++
> include/linux/mlx5/mlx5_ifc.h | 4 +++-
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/mlx5/core/resources.c b/drivers/vdpa/mlx5/core/resources.c
> index 6521cbd0f5c2..836ab9ef0fa6 100644
> --- a/drivers/vdpa/mlx5/core/resources.c
> +++ b/drivers/vdpa/mlx5/core/resources.c
> @@ -54,6 +54,9 @@ static int create_uctx(struct mlx5_vdpa_dev *mvdev, u16 *uid)
> void *in;
> int err;
>
> + if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0))
> + return 0;
> +
> /* 0 means not supported */
> if (!MLX5_CAP_GEN(mvdev->mdev, log_max_uctx))
> return -EOPNOTSUPP;
> @@ -79,6 +82,9 @@ static void destroy_uctx(struct mlx5_vdpa_dev *mvdev, u32 uid)
> u32 out[MLX5_ST_SZ_DW(destroy_uctx_out)] = {};
> u32 in[MLX5_ST_SZ_DW(destroy_uctx_in)] = {};
>
> + if (!uid)
> + return;
> +
> MLX5_SET(destroy_uctx_in, in, opcode, MLX5_CMD_OP_DESTROY_UCTX);
> MLX5_SET(destroy_uctx_in, in, uid, uid);
>
> diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
> index 9c68b2da14c6..606d2aeacad4 100644
> --- a/include/linux/mlx5/mlx5_ifc.h
> +++ b/include/linux/mlx5/mlx5_ifc.h
> @@ -1487,7 +1487,9 @@ struct mlx5_ifc_cmd_hca_cap_bits {
> u8 uar_4k[0x1];
> u8 reserved_at_241[0x9];
> u8 uar_sz[0x6];
> - u8 reserved_at_250[0x8];
> + u8 reserved_at_248[0x2];
> + u8 umem_uid_0[0x1];
> + u8 reserved_at_250[0x5];
> u8 log_pg_sz[0x8];
>
> u8 bf[0x1];
在 2021/5/31 上午11:02, Jason Wang 写道:
>
> 在 2021/5/30 下午3:54, Eli Cohen 写道:
>> Currently all resources must be created with uid != 0 which is essential
>> userspace processes allocating virtquueue resources. Since this is a
>> kernel implementation, it is perfectly legal to open resources with
>> uid == 0.
>>
>> In case frimware supports, avoid allocating user context.
>
>
> Typo "frimware".
>
> Otherwise.
>
> Acked-by: Jason Wang <[email protected]>
>
> (I don't see any code to check the firmware capability, is this
> intended?)
Speak too fast. I see the MLX5_CAP_GEN(), so it's fine.
Thanks
>
> Thanks
>
>
>>
>> Signed-off-by: Eli Cohen <[email protected]>
>> ---
>> drivers/vdpa/mlx5/core/resources.c | 6 ++++++
>> include/linux/mlx5/mlx5_ifc.h | 4 +++-
>> 2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/vdpa/mlx5/core/resources.c
>> b/drivers/vdpa/mlx5/core/resources.c
>> index 6521cbd0f5c2..836ab9ef0fa6 100644
>> --- a/drivers/vdpa/mlx5/core/resources.c
>> +++ b/drivers/vdpa/mlx5/core/resources.c
>> @@ -54,6 +54,9 @@ static int create_uctx(struct mlx5_vdpa_dev *mvdev,
>> u16 *uid)
>> void *in;
>> int err;
>> + if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0))
>> + return 0;
>> +
>> /* 0 means not supported */
>> if (!MLX5_CAP_GEN(mvdev->mdev, log_max_uctx))
>> return -EOPNOTSUPP;
>> @@ -79,6 +82,9 @@ static void destroy_uctx(struct mlx5_vdpa_dev
>> *mvdev, u32 uid)
>> u32 out[MLX5_ST_SZ_DW(destroy_uctx_out)] = {};
>> u32 in[MLX5_ST_SZ_DW(destroy_uctx_in)] = {};
>> + if (!uid)
>> + return;
>> +
>> MLX5_SET(destroy_uctx_in, in, opcode, MLX5_CMD_OP_DESTROY_UCTX);
>> MLX5_SET(destroy_uctx_in, in, uid, uid);
>> diff --git a/include/linux/mlx5/mlx5_ifc.h
>> b/include/linux/mlx5/mlx5_ifc.h
>> index 9c68b2da14c6..606d2aeacad4 100644
>> --- a/include/linux/mlx5/mlx5_ifc.h
>> +++ b/include/linux/mlx5/mlx5_ifc.h
>> @@ -1487,7 +1487,9 @@ struct mlx5_ifc_cmd_hca_cap_bits {
>> u8 uar_4k[0x1];
>> u8 reserved_at_241[0x9];
>> u8 uar_sz[0x6];
>> - u8 reserved_at_250[0x8];
>> + u8 reserved_at_248[0x2];
>> + u8 umem_uid_0[0x1];
>> + u8 reserved_at_250[0x5];
>> u8 log_pg_sz[0x8];
>> u8 bf[0x1];
?? 2021/5/30 ????3:54, Eli Cohen д??:
> In order to support running vdpa using vritio_vdpa driver, we need to
> create a different kind of MR, one that has 1:1 mapping, since the
> addresses referring to virtqueues are dma addresses.
>
> We create the 1:1 MR in mlx5_vdpa_dev_add() only in case firmware
> supports the general capability umem_uid_0. The reason for that is that
> 1:1 MRs must be created with uid == 0 while virtqueue objects can be
> created with uid == 0 only when the firmware capability is on.
>
> If the set_map() callback is called with new translations provided
> through iotlb, the driver will destroy the 1:1 MR and create a regular
> one.
>
> Signed-off-by: Eli Cohen <[email protected]>
> ---
> drivers/vdpa/mlx5/core/mlx5_vdpa.h | 1 +
> drivers/vdpa/mlx5/core/mr.c | 81 ++++++++++++++++++++++++------
> drivers/vdpa/mlx5/net/mlx5_vnet.c | 13 ++++-
> 3 files changed, 80 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> index b6cc53ba980c..09a16a3d1b2a 100644
> --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> @@ -35,6 +35,7 @@ struct mlx5_vdpa_mr {
>
> /* serialize mkey creation and destruction */
> struct mutex mkey_mtx;
> + bool user_mr;
> };
>
> struct mlx5_vdpa_resources {
> diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> index 800cfd1967ad..020c0ce4d203 100644
> --- a/drivers/vdpa/mlx5/core/mr.c
> +++ b/drivers/vdpa/mlx5/core/mr.c
> @@ -360,7 +360,7 @@ static int add_direct_chain(struct mlx5_vdpa_dev *mvdev, u64 start, u64 size, u8
> * indirect memory key that provides access to the enitre address space given
> * by iotlb.
> */
> -static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> +static int create_user_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> {
> struct mlx5_vdpa_mr *mr = &mvdev->mr;
> struct mlx5_vdpa_direct_mr *dmr;
> @@ -374,9 +374,6 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
> int err = 0;
> int nnuls;
>
> - if (mr->initialized)
> - return 0;
> -
> INIT_LIST_HEAD(&mr->head);
> for (map = vhost_iotlb_itree_first(iotlb, start, last); map;
> map = vhost_iotlb_itree_next(map, start, last)) {
> @@ -414,7 +411,7 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
> if (err)
> goto err_chain;
>
> - mr->initialized = true;
> + mr->user_mr = true;
> return 0;
>
> err_chain:
> @@ -426,33 +423,89 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
> return err;
> }
>
> -int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> +static int create_dma_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr)
> +{
> + int inlen = MLX5_ST_SZ_BYTES(create_mkey_in);
> + void *mkc;
> + u32 *in;
> + int err;
> +
> + in = kzalloc(inlen, GFP_KERNEL);
> + if (!in)
> + return -ENOMEM;
> +
> + mkc = MLX5_ADDR_OF(create_mkey_in, in, memory_key_mkey_entry);
> +
> + MLX5_SET(mkc, mkc, access_mode_1_0, MLX5_MKC_ACCESS_MODE_PA);
> + MLX5_SET(mkc, mkc, length64, 1);
> + MLX5_SET(mkc, mkc, lw, 1);
> + MLX5_SET(mkc, mkc, lr, 1);
> + MLX5_SET(mkc, mkc, pd, mvdev->res.pdn);
> + MLX5_SET(mkc, mkc, qpn, 0xffffff);
> +
> + err = mlx5_vdpa_create_mkey(mvdev, &mr->mkey, in, inlen);
> + kfree(in);
> + return err;
> +}
> +
> +static void destroy_dma_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr)
> +{
> + mlx5_vdpa_destroy_mkey(mvdev, &mr->mkey);
> +}
> +
> +static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> {
> struct mlx5_vdpa_mr *mr = &mvdev->mr;
> int err;
>
> - mutex_lock(&mr->mkey_mtx);
> + if (mr->initialized)
> + return 0;
> +
> + if (iotlb)
> + err = create_user_mr(mvdev, iotlb);
> + else
> + err = create_dma_mr(mvdev, mr);
Do we need to set user_mr to false here?
> +
> + mr->initialized = true;
> + return err;
> +}
> +
> +int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> +{
> + int err;
> +
> + mutex_lock(&mvdev->mr.mkey_mtx);
> err = _mlx5_vdpa_create_mr(mvdev, iotlb);
> - mutex_unlock(&mr->mkey_mtx);
> + mutex_unlock(&mvdev->mr.mkey_mtx);
> return err;
> }
>
> -void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev)
> +static void destroy_user_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr)
> {
> - struct mlx5_vdpa_mr *mr = &mvdev->mr;
> struct mlx5_vdpa_direct_mr *dmr;
> struct mlx5_vdpa_direct_mr *n;
>
> - mutex_lock(&mr->mkey_mtx);
> - if (!mr->initialized)
> - goto out;
> -
> destroy_indirect_key(mvdev, mr);
> list_for_each_entry_safe_reverse(dmr, n, &mr->head, list) {
> list_del_init(&dmr->list);
> unmap_direct_mr(mvdev, dmr);
> kfree(dmr);
> }
> +}
> +
> +void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev)
> +{
> + struct mlx5_vdpa_mr *mr = &mvdev->mr;
> +
> + mutex_lock(&mr->mkey_mtx);
> + if (!mr->initialized)
> + goto out;
> +
> + if (mr->user_mr)
> + destroy_user_mr(mvdev, mr);
> + else
> + destroy_dma_mr(mvdev, mr);
> +
> memset(mr, 0, sizeof(*mr));
> mr->initialized = false;
> out:
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index fdf3e74bffbd..f16756661c19 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -1780,6 +1780,8 @@ static void mlx5_vdpa_set_status(struct vdpa_device *vdev, u8 status)
> ndev->mvdev.status = 0;
> ndev->mvdev.mlx_features = 0;
> ++mvdev->generation;
> + if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0))
> + mlx5_vdpa_create_mr(mvdev, NULL);
I wonder if it's possible/worth to avoid the destroy and re-create of
dma MR here. (In the case of it has been used by us).
Thanks
> return;
> }
>
> @@ -1859,6 +1861,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_free_resources(&ndev->mvdev);
> mutex_destroy(&ndev->reslock);
> }
> @@ -2023,9 +2026,15 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name)
> if (err)
> goto err_mtu;
>
> + if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0)) {
> + err = mlx5_vdpa_create_mr(mvdev, NULL);
> + if (err)
> + goto err_res;
> + }
> +
> err = alloc_resources(ndev);
> if (err)
> - goto err_res;
> + goto err_mr;
>
> mvdev->vdev.mdev = &mgtdev->mgtdev;
> err = _vdpa_register_device(&mvdev->vdev, 2 * mlx5_vdpa_max_qps(max_vqs));
> @@ -2037,6 +2046,8 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name)
>
> err_reg:
> free_resources(ndev);
> +err_mr:
> + mlx5_vdpa_destroy_mr(mvdev);
> err_res:
> mlx5_vdpa_free_resources(&ndev->mvdev);
> err_mtu:
On Mon, May 31, 2021 at 11:02:21AM +0800, Jason Wang wrote:
>
> 在 2021/5/30 下午3:54, Eli Cohen 写道:
> > Currently all resources must be created with uid != 0 which is essential
> > userspace processes allocating virtquueue resources. Since this is a
> > kernel implementation, it is perfectly legal to open resources with
> > uid == 0.
> >
> > In case frimware supports, avoid allocating user context.
>
>
> Typo "frimware".
>
> Otherwise.
>
> Acked-by: Jason Wang <[email protected]>
>
> (I don't see any code to check the firmware capability, is this intended?)
>
> Thanks
>
>
> >
> > Signed-off-by: Eli Cohen <[email protected]>
> > ---
> > drivers/vdpa/mlx5/core/resources.c | 6 ++++++
> > include/linux/mlx5/mlx5_ifc.h | 4 +++-
> > 2 files changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/vdpa/mlx5/core/resources.c b/drivers/vdpa/mlx5/core/resources.c
> > index 6521cbd0f5c2..836ab9ef0fa6 100644
> > --- a/drivers/vdpa/mlx5/core/resources.c
> > +++ b/drivers/vdpa/mlx5/core/resources.c
> > @@ -54,6 +54,9 @@ static int create_uctx(struct mlx5_vdpa_dev *mvdev, u16 *uid)
> > void *in;
> > int err;
> > + if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0))
> > + return 0;
> > +
This checks that the "umem_uid_0" capability is set. If it is set, then
I don't create the user context which will result in uid being zero.
> > /* 0 means not supported */
> > if (!MLX5_CAP_GEN(mvdev->mdev, log_max_uctx))
> > return -EOPNOTSUPP;
> > @@ -79,6 +82,9 @@ static void destroy_uctx(struct mlx5_vdpa_dev *mvdev, u32 uid)
> > u32 out[MLX5_ST_SZ_DW(destroy_uctx_out)] = {};
> > u32 in[MLX5_ST_SZ_DW(destroy_uctx_in)] = {};
> > + if (!uid)
> > + return;
> > +
> > MLX5_SET(destroy_uctx_in, in, opcode, MLX5_CMD_OP_DESTROY_UCTX);
> > MLX5_SET(destroy_uctx_in, in, uid, uid);
> > diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
> > index 9c68b2da14c6..606d2aeacad4 100644
> > --- a/include/linux/mlx5/mlx5_ifc.h
> > +++ b/include/linux/mlx5/mlx5_ifc.h
> > @@ -1487,7 +1487,9 @@ struct mlx5_ifc_cmd_hca_cap_bits {
> > u8 uar_4k[0x1];
> > u8 reserved_at_241[0x9];
> > u8 uar_sz[0x6];
> > - u8 reserved_at_250[0x8];
> > + u8 reserved_at_248[0x2];
> > + u8 umem_uid_0[0x1];
> > + u8 reserved_at_250[0x5];
> > u8 log_pg_sz[0x8];
> > u8 bf[0x1];
>
On Mon, May 31, 2021 at 11:06:59AM +0800, Jason Wang wrote:
>
> 在 2021/5/31 上午11:02, Jason Wang 写道:
> >
> > 在 2021/5/30 下午3:54, Eli Cohen 写道:
> > > Currently all resources must be created with uid != 0 which is essential
> > > userspace processes allocating virtquueue resources. Since this is a
> > > kernel implementation, it is perfectly legal to open resources with
> > > uid == 0.
> > >
> > > In case frimware supports, avoid allocating user context.
> >
> >
> > Typo "frimware".
> >
> > Otherwise.
> >
> > Acked-by: Jason Wang <[email protected]>
> >
> > (I don't see any code to check the firmware capability, is this
> > intended?)
>
>
> Speak too fast. I see the MLX5_CAP_GEN(), so it's fine.
And I responded too fast :-)
>
> Thanks
>
>
> >
> > Thanks
> >
> >
> > >
> > > Signed-off-by: Eli Cohen <[email protected]>
> > > ---
> > > drivers/vdpa/mlx5/core/resources.c | 6 ++++++
> > > include/linux/mlx5/mlx5_ifc.h | 4 +++-
> > > 2 files changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/vdpa/mlx5/core/resources.c
> > > b/drivers/vdpa/mlx5/core/resources.c
> > > index 6521cbd0f5c2..836ab9ef0fa6 100644
> > > --- a/drivers/vdpa/mlx5/core/resources.c
> > > +++ b/drivers/vdpa/mlx5/core/resources.c
> > > @@ -54,6 +54,9 @@ static int create_uctx(struct mlx5_vdpa_dev
> > > *mvdev, u16 *uid)
> > > void *in;
> > > int err;
> > > + if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0))
> > > + return 0;
> > > +
> > > /* 0 means not supported */
> > > if (!MLX5_CAP_GEN(mvdev->mdev, log_max_uctx))
> > > return -EOPNOTSUPP;
> > > @@ -79,6 +82,9 @@ static void destroy_uctx(struct mlx5_vdpa_dev
> > > *mvdev, u32 uid)
> > > u32 out[MLX5_ST_SZ_DW(destroy_uctx_out)] = {};
> > > u32 in[MLX5_ST_SZ_DW(destroy_uctx_in)] = {};
> > > + if (!uid)
> > > + return;
> > > +
> > > MLX5_SET(destroy_uctx_in, in, opcode, MLX5_CMD_OP_DESTROY_UCTX);
> > > MLX5_SET(destroy_uctx_in, in, uid, uid);
> > > diff --git a/include/linux/mlx5/mlx5_ifc.h
> > > b/include/linux/mlx5/mlx5_ifc.h
> > > index 9c68b2da14c6..606d2aeacad4 100644
> > > --- a/include/linux/mlx5/mlx5_ifc.h
> > > +++ b/include/linux/mlx5/mlx5_ifc.h
> > > @@ -1487,7 +1487,9 @@ struct mlx5_ifc_cmd_hca_cap_bits {
> > > u8 uar_4k[0x1];
> > > u8 reserved_at_241[0x9];
> > > u8 uar_sz[0x6];
> > > - u8 reserved_at_250[0x8];
> > > + u8 reserved_at_248[0x2];
> > > + u8 umem_uid_0[0x1];
> > > + u8 reserved_at_250[0x5];
> > > u8 log_pg_sz[0x8];
> > > u8 bf[0x1];
>
On Mon, May 31, 2021 at 11:16:11AM +0800, Jason Wang wrote:
>
> 在 2021/5/30 下午3:54, Eli Cohen 写道:
> > In order to support running vdpa using vritio_vdpa driver, we need to
> > create a different kind of MR, one that has 1:1 mapping, since the
> > addresses referring to virtqueues are dma addresses.
> >
> > We create the 1:1 MR in mlx5_vdpa_dev_add() only in case firmware
> > supports the general capability umem_uid_0. The reason for that is that
> > 1:1 MRs must be created with uid == 0 while virtqueue objects can be
> > created with uid == 0 only when the firmware capability is on.
> >
> > If the set_map() callback is called with new translations provided
> > through iotlb, the driver will destroy the 1:1 MR and create a regular
> > one.
> >
> > Signed-off-by: Eli Cohen <[email protected]>
> > ---
> > drivers/vdpa/mlx5/core/mlx5_vdpa.h | 1 +
> > drivers/vdpa/mlx5/core/mr.c | 81 ++++++++++++++++++++++++------
> > drivers/vdpa/mlx5/net/mlx5_vnet.c | 13 ++++-
> > 3 files changed, 80 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > index b6cc53ba980c..09a16a3d1b2a 100644
> > --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > @@ -35,6 +35,7 @@ struct mlx5_vdpa_mr {
> > /* serialize mkey creation and destruction */
> > struct mutex mkey_mtx;
> > + bool user_mr;
> > };
> > struct mlx5_vdpa_resources {
> > diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> > index 800cfd1967ad..020c0ce4d203 100644
> > --- a/drivers/vdpa/mlx5/core/mr.c
> > +++ b/drivers/vdpa/mlx5/core/mr.c
> > @@ -360,7 +360,7 @@ static int add_direct_chain(struct mlx5_vdpa_dev *mvdev, u64 start, u64 size, u8
> > * indirect memory key that provides access to the enitre address space given
> > * by iotlb.
> > */
> > -static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> > +static int create_user_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> > {
> > struct mlx5_vdpa_mr *mr = &mvdev->mr;
> > struct mlx5_vdpa_direct_mr *dmr;
> > @@ -374,9 +374,6 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
> > int err = 0;
> > int nnuls;
> > - if (mr->initialized)
> > - return 0;
> > -
> > INIT_LIST_HEAD(&mr->head);
> > for (map = vhost_iotlb_itree_first(iotlb, start, last); map;
> > map = vhost_iotlb_itree_next(map, start, last)) {
> > @@ -414,7 +411,7 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
> > if (err)
> > goto err_chain;
> > - mr->initialized = true;
> > + mr->user_mr = true;
> > return 0;
> > err_chain:
> > @@ -426,33 +423,89 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
> > return err;
> > }
> > -int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> > +static int create_dma_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr)
> > +{
> > + int inlen = MLX5_ST_SZ_BYTES(create_mkey_in);
> > + void *mkc;
> > + u32 *in;
> > + int err;
> > +
> > + in = kzalloc(inlen, GFP_KERNEL);
> > + if (!in)
> > + return -ENOMEM;
> > +
> > + mkc = MLX5_ADDR_OF(create_mkey_in, in, memory_key_mkey_entry);
> > +
> > + MLX5_SET(mkc, mkc, access_mode_1_0, MLX5_MKC_ACCESS_MODE_PA);
> > + MLX5_SET(mkc, mkc, length64, 1);
> > + MLX5_SET(mkc, mkc, lw, 1);
> > + MLX5_SET(mkc, mkc, lr, 1);
> > + MLX5_SET(mkc, mkc, pd, mvdev->res.pdn);
> > + MLX5_SET(mkc, mkc, qpn, 0xffffff);
> > +
> > + err = mlx5_vdpa_create_mkey(mvdev, &mr->mkey, in, inlen);
> > + kfree(in);
> > + return err;
> > +}
> > +
> > +static void destroy_dma_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr)
> > +{
> > + mlx5_vdpa_destroy_mkey(mvdev, &mr->mkey);
> > +}
> > +
> > +static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> > {
> > struct mlx5_vdpa_mr *mr = &mvdev->mr;
> > int err;
> > - mutex_lock(&mr->mkey_mtx);
> > + if (mr->initialized)
> > + return 0;
> > +
> > + if (iotlb)
> > + err = create_user_mr(mvdev, iotlb);
> > + else
> > + err = create_dma_mr(mvdev, mr);
>
>
> Do we need to set user_mr to false here?
I think the right place to do this should be inside create_dma_mr(), the
same as we set it true in ().
>
>
> > +
> > + mr->initialized = true;
> > + return err;
> > +}
> > +
> > +int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> > +{
> > + int err;
> > +
> > + mutex_lock(&mvdev->mr.mkey_mtx);
> > err = _mlx5_vdpa_create_mr(mvdev, iotlb);
> > - mutex_unlock(&mr->mkey_mtx);
> > + mutex_unlock(&mvdev->mr.mkey_mtx);
> > return err;
> > }
> > -void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev)
> > +static void destroy_user_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr)
> > {
> > - struct mlx5_vdpa_mr *mr = &mvdev->mr;
> > struct mlx5_vdpa_direct_mr *dmr;
> > struct mlx5_vdpa_direct_mr *n;
> > - mutex_lock(&mr->mkey_mtx);
> > - if (!mr->initialized)
> > - goto out;
> > -
> > destroy_indirect_key(mvdev, mr);
> > list_for_each_entry_safe_reverse(dmr, n, &mr->head, list) {
> > list_del_init(&dmr->list);
> > unmap_direct_mr(mvdev, dmr);
> > kfree(dmr);
> > }
> > +}
> > +
> > +void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev)
> > +{
> > + struct mlx5_vdpa_mr *mr = &mvdev->mr;
> > +
> > + mutex_lock(&mr->mkey_mtx);
> > + if (!mr->initialized)
> > + goto out;
> > +
> > + if (mr->user_mr)
> > + destroy_user_mr(mvdev, mr);
> > + else
> > + destroy_dma_mr(mvdev, mr);
> > +
> > memset(mr, 0, sizeof(*mr));
> > mr->initialized = false;
> > out:
> > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > index fdf3e74bffbd..f16756661c19 100644
> > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > @@ -1780,6 +1780,8 @@ static void mlx5_vdpa_set_status(struct vdpa_device *vdev, u8 status)
> > ndev->mvdev.status = 0;
> > ndev->mvdev.mlx_features = 0;
> > ++mvdev->generation;
> > + if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0))
> > + mlx5_vdpa_create_mr(mvdev, NULL);
>
>
> I wonder if it's possible/worth to avoid the destroy and re-create of dma MR
> here. (In the case of it has been used by us).
If it is a user MR, you must destroy it since it is useless. If it's
DMA MR, you might be able to do it but since the DMA MR's are
lightweight, I don't think it's worth it.
>
> Thanks
>
>
> > return;
> > }
> > @@ -1859,6 +1861,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_free_resources(&ndev->mvdev);
> > mutex_destroy(&ndev->reslock);
> > }
> > @@ -2023,9 +2026,15 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name)
> > if (err)
> > goto err_mtu;
> > + if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0)) {
> > + err = mlx5_vdpa_create_mr(mvdev, NULL);
> > + if (err)
> > + goto err_res;
> > + }
> > +
> > err = alloc_resources(ndev);
> > if (err)
> > - goto err_res;
> > + goto err_mr;
> > mvdev->vdev.mdev = &mgtdev->mgtdev;
> > err = _vdpa_register_device(&mvdev->vdev, 2 * mlx5_vdpa_max_qps(max_vqs));
> > @@ -2037,6 +2046,8 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name)
> > err_reg:
> > free_resources(ndev);
> > +err_mr:
> > + mlx5_vdpa_destroy_mr(mvdev);
> > err_res:
> > mlx5_vdpa_free_resources(&ndev->mvdev);
> > err_mtu:
>
在 2021/5/31 下午12:59, Eli Cohen 写道:
> On Mon, May 31, 2021 at 11:06:59AM +0800, Jason Wang wrote:
>> 在 2021/5/31 上午11:02, Jason Wang 写道:
>>> 在 2021/5/30 下午3:54, Eli Cohen 写道:
>>>> Currently all resources must be created with uid != 0 which is essential
>>>> userspace processes allocating virtquueue resources. Since this is a
>>>> kernel implementation, it is perfectly legal to open resources with
>>>> uid == 0.
>>>>
>>>> In case frimware supports, avoid allocating user context.
>>>
>>> Typo "frimware".
>>>
>>> Otherwise.
>>>
>>> Acked-by: Jason Wang <[email protected]>
>>>
>>> (I don't see any code to check the firmware capability, is this
>>> intended?)
>>
>> Speak too fast. I see the MLX5_CAP_GEN(), so it's fine.
> And I responded too fast :-)
Right and thanks for the quick response :)
>> Thanks
>>
>>
>>> Thanks
>>>
>>>
>>>> Signed-off-by: Eli Cohen <[email protected]>
>>>> ---
>>>> drivers/vdpa/mlx5/core/resources.c | 6 ++++++
>>>> include/linux/mlx5/mlx5_ifc.h | 4 +++-
>>>> 2 files changed, 9 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/vdpa/mlx5/core/resources.c
>>>> b/drivers/vdpa/mlx5/core/resources.c
>>>> index 6521cbd0f5c2..836ab9ef0fa6 100644
>>>> --- a/drivers/vdpa/mlx5/core/resources.c
>>>> +++ b/drivers/vdpa/mlx5/core/resources.c
>>>> @@ -54,6 +54,9 @@ static int create_uctx(struct mlx5_vdpa_dev
>>>> *mvdev, u16 *uid)
>>>> void *in;
>>>> int err;
>>>> + if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0))
>>>> + return 0;
>>>> +
>>>> /* 0 means not supported */
>>>> if (!MLX5_CAP_GEN(mvdev->mdev, log_max_uctx))
>>>> return -EOPNOTSUPP;
>>>> @@ -79,6 +82,9 @@ static void destroy_uctx(struct mlx5_vdpa_dev
>>>> *mvdev, u32 uid)
>>>> u32 out[MLX5_ST_SZ_DW(destroy_uctx_out)] = {};
>>>> u32 in[MLX5_ST_SZ_DW(destroy_uctx_in)] = {};
>>>> + if (!uid)
>>>> + return;
>>>> +
>>>> MLX5_SET(destroy_uctx_in, in, opcode, MLX5_CMD_OP_DESTROY_UCTX);
>>>> MLX5_SET(destroy_uctx_in, in, uid, uid);
>>>> diff --git a/include/linux/mlx5/mlx5_ifc.h
>>>> b/include/linux/mlx5/mlx5_ifc.h
>>>> index 9c68b2da14c6..606d2aeacad4 100644
>>>> --- a/include/linux/mlx5/mlx5_ifc.h
>>>> +++ b/include/linux/mlx5/mlx5_ifc.h
>>>> @@ -1487,7 +1487,9 @@ struct mlx5_ifc_cmd_hca_cap_bits {
>>>> u8 uar_4k[0x1];
>>>> u8 reserved_at_241[0x9];
>>>> u8 uar_sz[0x6];
>>>> - u8 reserved_at_250[0x8];
>>>> + u8 reserved_at_248[0x2];
>>>> + u8 umem_uid_0[0x1];
>>>> + u8 reserved_at_250[0x5];
>>>> u8 log_pg_sz[0x8];
>>>> u8 bf[0x1];
在 2021/5/31 下午1:13, Eli Cohen 写道:
> On Mon, May 31, 2021 at 11:16:11AM +0800, Jason Wang wrote:
>> 在 2021/5/30 下午3:54, Eli Cohen 写道:
>>> In order to support running vdpa using vritio_vdpa driver, we need to
>>> create a different kind of MR, one that has 1:1 mapping, since the
>>> addresses referring to virtqueues are dma addresses.
>>>
>>> We create the 1:1 MR in mlx5_vdpa_dev_add() only in case firmware
>>> supports the general capability umem_uid_0. The reason for that is that
>>> 1:1 MRs must be created with uid == 0 while virtqueue objects can be
>>> created with uid == 0 only when the firmware capability is on.
>>>
>>> If the set_map() callback is called with new translations provided
>>> through iotlb, the driver will destroy the 1:1 MR and create a regular
>>> one.
>>>
>>> Signed-off-by: Eli Cohen <[email protected]>
>>> ---
>>> drivers/vdpa/mlx5/core/mlx5_vdpa.h | 1 +
>>> drivers/vdpa/mlx5/core/mr.c | 81 ++++++++++++++++++++++++------
>>> drivers/vdpa/mlx5/net/mlx5_vnet.c | 13 ++++-
>>> 3 files changed, 80 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
>>> index b6cc53ba980c..09a16a3d1b2a 100644
>>> --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
>>> +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
>>> @@ -35,6 +35,7 @@ struct mlx5_vdpa_mr {
>>> /* serialize mkey creation and destruction */
>>> struct mutex mkey_mtx;
>>> + bool user_mr;
>>> };
>>> struct mlx5_vdpa_resources {
>>> diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
>>> index 800cfd1967ad..020c0ce4d203 100644
>>> --- a/drivers/vdpa/mlx5/core/mr.c
>>> +++ b/drivers/vdpa/mlx5/core/mr.c
>>> @@ -360,7 +360,7 @@ static int add_direct_chain(struct mlx5_vdpa_dev *mvdev, u64 start, u64 size, u8
>>> * indirect memory key that provides access to the enitre address space given
>>> * by iotlb.
>>> */
>>> -static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
>>> +static int create_user_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
>>> {
>>> struct mlx5_vdpa_mr *mr = &mvdev->mr;
>>> struct mlx5_vdpa_direct_mr *dmr;
>>> @@ -374,9 +374,6 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
>>> int err = 0;
>>> int nnuls;
>>> - if (mr->initialized)
>>> - return 0;
>>> -
>>> INIT_LIST_HEAD(&mr->head);
>>> for (map = vhost_iotlb_itree_first(iotlb, start, last); map;
>>> map = vhost_iotlb_itree_next(map, start, last)) {
>>> @@ -414,7 +411,7 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
>>> if (err)
>>> goto err_chain;
>>> - mr->initialized = true;
>>> + mr->user_mr = true;
>>> return 0;
>>> err_chain:
>>> @@ -426,33 +423,89 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
>>> return err;
>>> }
>>> -int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
>>> +static int create_dma_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr)
>>> +{
>>> + int inlen = MLX5_ST_SZ_BYTES(create_mkey_in);
>>> + void *mkc;
>>> + u32 *in;
>>> + int err;
>>> +
>>> + in = kzalloc(inlen, GFP_KERNEL);
>>> + if (!in)
>>> + return -ENOMEM;
>>> +
>>> + mkc = MLX5_ADDR_OF(create_mkey_in, in, memory_key_mkey_entry);
>>> +
>>> + MLX5_SET(mkc, mkc, access_mode_1_0, MLX5_MKC_ACCESS_MODE_PA);
>>> + MLX5_SET(mkc, mkc, length64, 1);
>>> + MLX5_SET(mkc, mkc, lw, 1);
>>> + MLX5_SET(mkc, mkc, lr, 1);
>>> + MLX5_SET(mkc, mkc, pd, mvdev->res.pdn);
>>> + MLX5_SET(mkc, mkc, qpn, 0xffffff);
>>> +
>>> + err = mlx5_vdpa_create_mkey(mvdev, &mr->mkey, in, inlen);
>>> + kfree(in);
>>> + return err;
>>> +}
>>> +
>>> +static void destroy_dma_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr)
>>> +{
>>> + mlx5_vdpa_destroy_mkey(mvdev, &mr->mkey);
>>> +}
>>> +
>>> +static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
>>> {
>>> struct mlx5_vdpa_mr *mr = &mvdev->mr;
>>> int err;
>>> - mutex_lock(&mr->mkey_mtx);
>>> + if (mr->initialized)
>>> + return 0;
>>> +
>>> + if (iotlb)
>>> + err = create_user_mr(mvdev, iotlb);
>>> + else
>>> + err = create_dma_mr(mvdev, mr);
>>
>> Do we need to set user_mr to false here?
> I think the right place to do this should be inside create_dma_mr(), the
> same as we set it true in ().
Fine.
>>
>>> +
>>> + mr->initialized = true;
>>> + return err;
>>> +}
>>> +
>>> +int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
>>> +{
>>> + int err;
>>> +
>>> + mutex_lock(&mvdev->mr.mkey_mtx);
>>> err = _mlx5_vdpa_create_mr(mvdev, iotlb);
>>> - mutex_unlock(&mr->mkey_mtx);
>>> + mutex_unlock(&mvdev->mr.mkey_mtx);
>>> return err;
>>> }
>>> -void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev)
>>> +static void destroy_user_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr)
>>> {
>>> - struct mlx5_vdpa_mr *mr = &mvdev->mr;
>>> struct mlx5_vdpa_direct_mr *dmr;
>>> struct mlx5_vdpa_direct_mr *n;
>>> - mutex_lock(&mr->mkey_mtx);
>>> - if (!mr->initialized)
>>> - goto out;
>>> -
>>> destroy_indirect_key(mvdev, mr);
>>> list_for_each_entry_safe_reverse(dmr, n, &mr->head, list) {
>>> list_del_init(&dmr->list);
>>> unmap_direct_mr(mvdev, dmr);
>>> kfree(dmr);
>>> }
>>> +}
>>> +
>>> +void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev)
>>> +{
>>> + struct mlx5_vdpa_mr *mr = &mvdev->mr;
>>> +
>>> + mutex_lock(&mr->mkey_mtx);
>>> + if (!mr->initialized)
>>> + goto out;
>>> +
>>> + if (mr->user_mr)
>>> + destroy_user_mr(mvdev, mr);
>>> + else
>>> + destroy_dma_mr(mvdev, mr);
>>> +
>>> memset(mr, 0, sizeof(*mr));
>>> mr->initialized = false;
>>> out:
>>> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> index fdf3e74bffbd..f16756661c19 100644
>>> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> @@ -1780,6 +1780,8 @@ static void mlx5_vdpa_set_status(struct vdpa_device *vdev, u8 status)
>>> ndev->mvdev.status = 0;
>>> ndev->mvdev.mlx_features = 0;
>>> ++mvdev->generation;
>>> + if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0))
>>> + mlx5_vdpa_create_mr(mvdev, NULL);
>>
>> I wonder if it's possible/worth to avoid the destroy and re-create of dma MR
>> here. (In the case of it has been used by us).
> If it is a user MR, you must destroy it since it is useless. If it's
> DMA MR, you might be able to do it but since the DMA MR's are
> lightweight, I don't think it's worth it.
Yes, I agree.
Thanks
>
>> Thanks
>>
>>
>>> return;
>>> }
>>> @@ -1859,6 +1861,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_free_resources(&ndev->mvdev);
>>> mutex_destroy(&ndev->reslock);
>>> }
>>> @@ -2023,9 +2026,15 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name)
>>> if (err)
>>> goto err_mtu;
>>> + if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0)) {
>>> + err = mlx5_vdpa_create_mr(mvdev, NULL);
>>> + if (err)
>>> + goto err_res;
>>> + }
>>> +
>>> err = alloc_resources(ndev);
>>> if (err)
>>> - goto err_res;
>>> + goto err_mr;
>>> mvdev->vdev.mdev = &mgtdev->mgtdev;
>>> err = _vdpa_register_device(&mvdev->vdev, 2 * mlx5_vdpa_max_qps(max_vqs));
>>> @@ -2037,6 +2046,8 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name)
>>> err_reg:
>>> free_resources(ndev);
>>> +err_mr:
>>> + mlx5_vdpa_destroy_mr(mvdev);
>>> err_res:
>>> mlx5_vdpa_free_resources(&ndev->mvdev);
>>> err_mtu: