diff mbox series

[V3] vdpa/mlx5: preserve CVQ vringh index

Message ID	1699014387-194368-1-git-send-email-steven.sistare@oracle.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3; From: Steve Sistare <steven.sistare@oracle.com> To: virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org Cc: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>, Eli Cohen <elic@nvidia.com>, Si-Wei Liu <si-wei.liu@oracle.com>, Dragos Tatulea <dtatulea@nvidia.com>, Xuan Zhuo <xuanzhuo@linux.alibaba.com>, Eugenio Perez Martin <eperezma@redhat.com>, Steve Sistare <steven.sistare@oracle.com> Subject: [PATCH V3] vdpa/mlx5: preserve CVQ vringh index Date: Fri, 3 Nov 2023 05:26:27 -0700 Message-Id: <1699014387-194368-1-git-send-email-steven.sistare@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	[V3] vdpa/mlx5: preserve CVQ vringh index \| [V3] vdpa/mlx5: preserve CVQ vringh index

Commit Message

Steven Sistare Nov. 3, 2023, 12:26 p.m. UTC

  mlx5_vdpa does not preserve userland's view of vring base for the control
queue in the following sequence:

ioctl VHOST_SET_VRING_BASE
ioctl VHOST_VDPA_SET_STATUS VIRTIO_CONFIG_S_DRIVER_OK
  mlx5_vdpa_set_status()
    setup_cvq_vring()
      vringh_init_iotlb()
        vringh_init_kern()
          vrh->last_avail_idx = 0;
ioctl VHOST_GET_VRING_BASE

To fix, restore the value of cvq->vring.last_avail_idx after calling
vringh_init_iotlb.

Fixes: 5262912ef3cf ("vdpa/mlx5: Add support for control VQ and MAC setting")

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Acked-by: Eugenio Pérez <eperezma@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Steven Sistare Nov. 3, 2023, 12:27 p.m. UTC | #1

In V3 I added the fixes tag and the acks.  No other changes.

- Steve

On 11/3/2023 8:26 AM, Steve Sistare wrote:
> mlx5_vdpa does not preserve userland's view of vring base for the control
> queue in the following sequence:
> 
> ioctl VHOST_SET_VRING_BASE
> ioctl VHOST_VDPA_SET_STATUS VIRTIO_CONFIG_S_DRIVER_OK
>   mlx5_vdpa_set_status()
>     setup_cvq_vring()
>       vringh_init_iotlb()
>         vringh_init_kern()
>           vrh->last_avail_idx = 0;
> ioctl VHOST_GET_VRING_BASE
> 
> To fix, restore the value of cvq->vring.last_avail_idx after calling
> vringh_init_iotlb.
> 
> Fixes: 5262912ef3cf ("vdpa/mlx5: Add support for control VQ and MAC setting")
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> Acked-by: Eugenio Pérez <eperezma@redhat.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
> ---
>  drivers/vdpa/mlx5/net/mlx5_vnet.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 946488b8989f..ca972af3c89a 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -2795,13 +2795,18 @@ 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))
> +	if (mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ)) {
> +		u16 idx = cvq->vring.last_avail_idx;
> +
>  		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)
> +			cvq->vring.last_avail_idx = cvq->vring.last_used_idx = idx;
> +	}
>  	return err;
>  }
>

Steven Sistare Dec. 4, 2023, 1:33 p.m. UTC | #2

What's the decision here, pull it, or do not fix it?

- Steve

On 11/3/2023 8:26 AM, Steve Sistare wrote:
> mlx5_vdpa does not preserve userland's view of vring base for the control
> queue in the following sequence:
> 
> ioctl VHOST_SET_VRING_BASE
> ioctl VHOST_VDPA_SET_STATUS VIRTIO_CONFIG_S_DRIVER_OK
>   mlx5_vdpa_set_status()
>     setup_cvq_vring()
>       vringh_init_iotlb()
>         vringh_init_kern()
>           vrh->last_avail_idx = 0;
> ioctl VHOST_GET_VRING_BASE
> 
> To fix, restore the value of cvq->vring.last_avail_idx after calling
> vringh_init_iotlb.
> 
> Fixes: 5262912ef3cf ("vdpa/mlx5: Add support for control VQ and MAC setting")
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> Acked-by: Eugenio Pérez <eperezma@redhat.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
> ---
>  drivers/vdpa/mlx5/net/mlx5_vnet.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 946488b8989f..ca972af3c89a 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -2795,13 +2795,18 @@ 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))
> +	if (mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ)) {
> +		u16 idx = cvq->vring.last_avail_idx;
> +
>  		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)
> +			cvq->vring.last_avail_idx = cvq->vring.last_used_idx = idx;
> +	}
>  	return err;
>  }
>

Michael S. Tsirkin Dec. 4, 2023, 1:35 p.m. UTC | #3

It's in my tee and I'm sending it to Linus.

On Mon, Dec 04, 2023 at 08:33:22AM -0500, Steven Sistare wrote:
> What's the decision here, pull it, or do not fix it?
> 
> - Steve
> 
> On 11/3/2023 8:26 AM, Steve Sistare wrote:
> > mlx5_vdpa does not preserve userland's view of vring base for the control
> > queue in the following sequence:
> > 
> > ioctl VHOST_SET_VRING_BASE
> > ioctl VHOST_VDPA_SET_STATUS VIRTIO_CONFIG_S_DRIVER_OK
> >   mlx5_vdpa_set_status()
> >     setup_cvq_vring()
> >       vringh_init_iotlb()
> >         vringh_init_kern()
> >           vrh->last_avail_idx = 0;
> > ioctl VHOST_GET_VRING_BASE
> > 
> > To fix, restore the value of cvq->vring.last_avail_idx after calling
> > vringh_init_iotlb.
> > 
> > Fixes: 5262912ef3cf ("vdpa/mlx5: Add support for control VQ and MAC setting")
> > 
> > Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> > Acked-by: Eugenio Pérez <eperezma@redhat.com>
> > Acked-by: Jason Wang <jasowang@redhat.com>
> > ---
> >  drivers/vdpa/mlx5/net/mlx5_vnet.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > index 946488b8989f..ca972af3c89a 100644
> > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > @@ -2795,13 +2795,18 @@ 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))
> > +	if (mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ)) {
> > +		u16 idx = cvq->vring.last_avail_idx;
> > +
> >  		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)
> > +			cvq->vring.last_avail_idx = cvq->vring.last_used_idx = idx;
> > +	}
> >  	return err;
> >  }
> >

Steven Sistare Dec. 4, 2023, 1:37 p.m. UTC | #4

Glad to hear it, thanks - Steve

On 12/4/2023 8:35 AM, Michael S. Tsirkin wrote:
> It's in my tee and I'm sending it to Linus.
> 
> On Mon, Dec 04, 2023 at 08:33:22AM -0500, Steven Sistare wrote:
>> What's the decision here, pull it, or do not fix it?
>>
>> - Steve
>>
>> On 11/3/2023 8:26 AM, Steve Sistare wrote:
>>> mlx5_vdpa does not preserve userland's view of vring base for the control
>>> queue in the following sequence:
>>>
>>> ioctl VHOST_SET_VRING_BASE
>>> ioctl VHOST_VDPA_SET_STATUS VIRTIO_CONFIG_S_DRIVER_OK
>>>   mlx5_vdpa_set_status()
>>>     setup_cvq_vring()
>>>       vringh_init_iotlb()
>>>         vringh_init_kern()
>>>           vrh->last_avail_idx = 0;
>>> ioctl VHOST_GET_VRING_BASE
>>>
>>> To fix, restore the value of cvq->vring.last_avail_idx after calling
>>> vringh_init_iotlb.
>>>
>>> Fixes: 5262912ef3cf ("vdpa/mlx5: Add support for control VQ and MAC setting")
>>>
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>> Acked-by: Eugenio Pérez <eperezma@redhat.com>
>>> Acked-by: Jason Wang <jasowang@redhat.com>
>>> ---
>>>  drivers/vdpa/mlx5/net/mlx5_vnet.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> index 946488b8989f..ca972af3c89a 100644
>>> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> @@ -2795,13 +2795,18 @@ 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))
>>> +	if (mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ)) {
>>> +		u16 idx = cvq->vring.last_avail_idx;
>>> +
>>>  		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)
>>> +			cvq->vring.last_avail_idx = cvq->vring.last_used_idx = idx;
>>> +	}
>>>  	return err;
>>>  }
>>>  
>

diff mbox series

Patch

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 946488b8989f..ca972af3c89a 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -2795,13 +2795,18 @@  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))
+	if (mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ)) {
+		u16 idx = cvq->vring.last_avail_idx;
+
 		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)
+			cvq->vring.last_avail_idx = cvq->vring.last_used_idx = idx;
+	}
 	return err;
 }