2022-06-13 08:11:14

by Eli Cohen

[permalink] [raw]
Subject: [PATCH 1/2] vdpa/mlx5: Update Control VQ callback information

The control VQ specific information is stored in the dedicated struct
mlx5_control_vq. When the callback is updated through
mlx5_vdpa_set_vq_cb(), make sure to update the control VQ struct.

Fixes: 5262912ef3cf ("vdpa/mlx5: Add support for control VQ and MAC setting")
Signed-off-by: Eli Cohen <[email protected]>
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 1b6d46b86f81..789c078ff1af 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1962,6 +1962,8 @@ static void mlx5_vdpa_set_vq_cb(struct vdpa_device *vdev, u16 idx, struct vdpa_c
struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);

ndev->event_cbs[idx] = *cb;
+ if (is_ctrl_vq_idx(mvdev, idx))
+ mvdev->cvq.event_cb = *cb;
}

static void mlx5_cvq_notify(struct vringh *vring)
--
2.35.1


2022-06-13 08:14:14

by Jason Wang

[permalink] [raw]
Subject: Re: [PATCH 1/2] vdpa/mlx5: Update Control VQ callback information

On Mon, Jun 13, 2022 at 4:00 PM Eli Cohen <[email protected]> wrote:
>
> The control VQ specific information is stored in the dedicated struct
> mlx5_control_vq. When the callback is updated through
> mlx5_vdpa_set_vq_cb(), make sure to update the control VQ struct.
>
> Fixes: 5262912ef3cf ("vdpa/mlx5: Add support for control VQ and MAC setting")
> Signed-off-by: Eli Cohen <[email protected]>
> ---
> drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 1b6d46b86f81..789c078ff1af 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -1962,6 +1962,8 @@ static void mlx5_vdpa_set_vq_cb(struct vdpa_device *vdev, u16 idx, struct vdpa_c
> struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
>
> ndev->event_cbs[idx] = *cb;
> + if (is_ctrl_vq_idx(mvdev, idx))
> + mvdev->cvq.event_cb = *cb;
> }
>

Acked-by: Jason Wang <[email protected])

In the future, I wonder if we can simply just use event_cbs[] since it
has took cvq into account:

struct vdpa_callback event_cbs[MLX5_MAX_SUPPORTED_VQS + 1];

Thanks

> static void mlx5_cvq_notify(struct vringh *vring)
> --
> 2.35.1
>

2022-06-13 08:27:05

by Eli Cohen

[permalink] [raw]
Subject: RE: [PATCH 1/2] vdpa/mlx5: Update Control VQ callback information



> -----Original Message-----
> From: Jason Wang <[email protected]>
> Sent: Monday, June 13, 2022 11:11 AM
> To: Eli Cohen <[email protected]>
> Cc: mst <[email protected]>; eperezma <[email protected]>; virtualization <[email protected]>; linux-
> kernel <[email protected]>; Si-Wei Liu <[email protected]>
> Subject: Re: [PATCH 1/2] vdpa/mlx5: Update Control VQ callback information
>
> On Mon, Jun 13, 2022 at 4:00 PM Eli Cohen <[email protected]> wrote:
> >
> > The control VQ specific information is stored in the dedicated struct
> > mlx5_control_vq. When the callback is updated through
> > mlx5_vdpa_set_vq_cb(), make sure to update the control VQ struct.
> >
> > Fixes: 5262912ef3cf ("vdpa/mlx5: Add support for control VQ and MAC setting")
> > Signed-off-by: Eli Cohen <[email protected]>
> > ---
> > drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > index 1b6d46b86f81..789c078ff1af 100644
> > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > @@ -1962,6 +1962,8 @@ static void mlx5_vdpa_set_vq_cb(struct vdpa_device *vdev, u16 idx, struct vdpa_c
> > struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
> >
> > ndev->event_cbs[idx] = *cb;
> > + if (is_ctrl_vq_idx(mvdev, idx))
> > + mvdev->cvq.event_cb = *cb;
> > }
> >
>
> Acked-by: Jason Wang <[email protected])
>
> In the future, I wonder if we can simply just use event_cbs[] since it
> has took cvq into account:
>
> struct vdpa_callback event_cbs[MLX5_MAX_SUPPORTED_VQS + 1];

Right. I will change this is some future patch.

>
> Thanks
>
> > static void mlx5_cvq_notify(struct vringh *vring)
> > --
> > 2.35.1
> >

2022-06-13 08:27:46

by Eli Cohen

[permalink] [raw]
Subject: [PATCH 2/2] vdpa/mlx5: Initializde CVQ vringh only once

Currently, CVQ vringh is initialized inside setup_virtqueues() which is
called every time a memory update is done. This is undesirable since it
resets all the context of the vring, including the available and used
indices.

Move the initialization to mlx5_vdpa_set_status() when
VIRTIO_CONFIG_S_DRIVER_OK is set.

Signed-off-by: Eli Cohen <[email protected]>
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 789c078ff1af..e85c1d71f4ed 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -2176,7 +2176,6 @@ static int verify_driver_features(struct mlx5_vdpa_dev *mvdev, u64 features)
static int setup_virtqueues(struct mlx5_vdpa_dev *mvdev)
{
struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
- struct mlx5_control_vq *cvq = &mvdev->cvq;
int err;
int i;

@@ -2186,16 +2185,6 @@ static int setup_virtqueues(struct mlx5_vdpa_dev *mvdev)
goto err_vq;
}

- if (mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ)) {
- err = vringh_init_iotlb(&cvq->vring, mvdev->actual_features,
- MLX5_CVQ_MAX_ENT, false,
- (struct vring_desc *)(uintptr_t)cvq->desc_addr,
- (struct vring_avail *)(uintptr_t)cvq->driver_addr,
- (struct vring_used *)(uintptr_t)cvq->device_addr);
- if (err)
- goto err_vq;
- }
-
return 0;

err_vq:
@@ -2468,6 +2457,21 @@ static void clear_vqs_ready(struct mlx5_vdpa_net *ndev)
ndev->mvdev.cvq.ready = false;
}

+static int setup_cvq_vring(struct mlx5_vdpa_dev *mvdev)
+{
+ struct mlx5_control_vq *cvq = &mvdev->cvq;
+ int err = 0;
+
+ if (mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ))
+ err = vringh_init_iotlb(&cvq->vring, mvdev->actual_features,
+ MLX5_CVQ_MAX_ENT, false,
+ (struct vring_desc *)(uintptr_t)cvq->desc_addr,
+ (struct vring_avail *)(uintptr_t)cvq->driver_addr,
+ (struct vring_used *)(uintptr_t)cvq->device_addr);
+
+ return err;
+}
+
static void mlx5_vdpa_set_status(struct vdpa_device *vdev, u8 status)
{
struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
@@ -2480,6 +2484,11 @@ static void mlx5_vdpa_set_status(struct vdpa_device *vdev, u8 status)

if ((status ^ ndev->mvdev.status) & VIRTIO_CONFIG_S_DRIVER_OK) {
if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
+ err = setup_cvq_vring(mvdev);
+ if (err) {
+ mlx5_vdpa_warn(mvdev, "failed to setup control VQ vring\n");
+ goto err_setup;
+ }
err = setup_driver(mvdev);
if (err) {
mlx5_vdpa_warn(mvdev, "failed to setup driver\n");
--
2.35.1

2022-06-14 06:29:32

by Eugenio Perez Martin

[permalink] [raw]
Subject: Re: [PATCH 2/2] vdpa/mlx5: Initializde CVQ vringh only once

On Mon, Jun 13, 2022 at 10:00 AM Eli Cohen <[email protected]> wrote:
>
> Currently, CVQ vringh is initialized inside setup_virtqueues() which is
> called every time a memory update is done. This is undesirable since it
> resets all the context of the vring, including the available and used
> indices.
>
> Move the initialization to mlx5_vdpa_set_status() when
> VIRTIO_CONFIG_S_DRIVER_OK is set.
>
> Signed-off-by: Eli Cohen <[email protected]>

Acked-by: Eugenio PĂ©rez <[email protected]>

> ---
> drivers/vdpa/mlx5/net/mlx5_vnet.c | 31 ++++++++++++++++++++-----------
> 1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 789c078ff1af..e85c1d71f4ed 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -2176,7 +2176,6 @@ static int verify_driver_features(struct mlx5_vdpa_dev *mvdev, u64 features)
> static int setup_virtqueues(struct mlx5_vdpa_dev *mvdev)
> {
> struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
> - struct mlx5_control_vq *cvq = &mvdev->cvq;
> int err;
> int i;
>
> @@ -2186,16 +2185,6 @@ static int setup_virtqueues(struct mlx5_vdpa_dev *mvdev)
> goto err_vq;
> }
>
> - if (mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ)) {
> - err = vringh_init_iotlb(&cvq->vring, mvdev->actual_features,
> - MLX5_CVQ_MAX_ENT, false,
> - (struct vring_desc *)(uintptr_t)cvq->desc_addr,
> - (struct vring_avail *)(uintptr_t)cvq->driver_addr,
> - (struct vring_used *)(uintptr_t)cvq->device_addr);
> - if (err)
> - goto err_vq;
> - }
> -
> return 0;
>
> err_vq:
> @@ -2468,6 +2457,21 @@ static void clear_vqs_ready(struct mlx5_vdpa_net *ndev)
> ndev->mvdev.cvq.ready = false;
> }
>
> +static int setup_cvq_vring(struct mlx5_vdpa_dev *mvdev)
> +{
> + struct mlx5_control_vq *cvq = &mvdev->cvq;
> + int err = 0;
> +
> + if (mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ))
> + err = vringh_init_iotlb(&cvq->vring, mvdev->actual_features,
> + MLX5_CVQ_MAX_ENT, false,
> + (struct vring_desc *)(uintptr_t)cvq->desc_addr,
> + (struct vring_avail *)(uintptr_t)cvq->driver_addr,
> + (struct vring_used *)(uintptr_t)cvq->device_addr);
> +
> + return err;
> +}
> +
> static void mlx5_vdpa_set_status(struct vdpa_device *vdev, u8 status)
> {
> struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
> @@ -2480,6 +2484,11 @@ static void mlx5_vdpa_set_status(struct vdpa_device *vdev, u8 status)
>
> if ((status ^ ndev->mvdev.status) & VIRTIO_CONFIG_S_DRIVER_OK) {
> if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
> + err = setup_cvq_vring(mvdev);
> + if (err) {
> + mlx5_vdpa_warn(mvdev, "failed to setup control VQ vring\n");
> + goto err_setup;
> + }
> err = setup_driver(mvdev);
> if (err) {
> mlx5_vdpa_warn(mvdev, "failed to setup driver\n");
> --
> 2.35.1
>