All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] vdpa: Block vq property change in DRIVER_OK
@ 2023-12-25 13:42 Dragos Tatulea
  2023-12-25 13:42 ` [PATCH 1/2] vdpa: Track device suspended state Dragos Tatulea
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Dragos Tatulea @ 2023-12-25 13:42 UTC (permalink / raw
  To: Michael S . Tsirkin, Jason Wang, Eugenio Perez Martin, Si-Wei Liu,
	virtualization, Gal Pressman
  Cc: Dragos Tatulea, kvm, linux-kernel, Parav Pandit, Xuan Zhuo

This series prevents the change of virtqueue address or state when a
device is in DRIVER_OK and not suspended. The virtio spec doesn't
allow changing virtqueue addresses and state in DRIVER_OK, but some devices
do support this operation when the device is suspended. The series
leaves the door open for these devices.

The series was suggested while discussing the addition of resumable
virtuque support in the mlx5_vdpa driver [0].

[0] https://lore.kernel.org/virtualization/20231219180858.120898-1-dtatulea@nvidia.com/T/#m044ddf540d98a6b025f81bffa058ca647a3d013e

Dragos Tatulea (2):
  vdpa: Track device suspended state
  vdpa: Block vq property changes in DRIVER_OK

 drivers/vhost/vdpa.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] vdpa: Track device suspended state
  2023-12-25 13:42 [PATCH 0/2] vdpa: Block vq property change in DRIVER_OK Dragos Tatulea
@ 2023-12-25 13:42 ` Dragos Tatulea
  2023-12-25 13:42 ` [PATCH 2/2] vdpa: Block vq property changes in DRIVER_OK Dragos Tatulea
  2023-12-25 14:40 ` [PATCH 0/2] vdpa: Block vq property change " Michael S. Tsirkin
  2 siblings, 0 replies; 5+ messages in thread
From: Dragos Tatulea @ 2023-12-25 13:42 UTC (permalink / raw
  To: Michael S . Tsirkin, Jason Wang, Eugenio Perez Martin, Si-Wei Liu,
	virtualization, Gal Pressman
  Cc: Dragos Tatulea, kvm, linux-kernel, Parav Pandit, Xuan Zhuo

Set vdpa device suspended state on successful suspend. Clear it on
successful resume and reset.

The state will be locked by the vhost_vdpa mutex. The mutex is taken
during suspend, resume and reset in vhost_vdpa_unlocked_ioctl. The
exception is vhost_vdpa_open which does a device reset but that should
be safe because it can only happen before the other ops.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Suggested-by: Eugenio Pérez <eperezma@redhat.com>
---
 drivers/vhost/vdpa.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index da7ec77cdaff..4c422be7d1e7 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -59,6 +59,7 @@ struct vhost_vdpa {
 	int in_batch;
 	struct vdpa_iova_range range;
 	u32 batch_asid;
+	bool suspended;
 };
 
 static DEFINE_IDA(vhost_vdpa_ida);
@@ -232,6 +233,8 @@ static int _compat_vdpa_reset(struct vhost_vdpa *v)
 	struct vdpa_device *vdpa = v->vdpa;
 	u32 flags = 0;
 
+	v->suspended = false;
+
 	if (v->vdev.vqs) {
 		flags |= !vhost_backend_has_feature(v->vdev.vqs[0],
 						    VHOST_BACKEND_F_IOTLB_PERSIST) ?
@@ -590,11 +593,16 @@ static long vhost_vdpa_suspend(struct vhost_vdpa *v)
 {
 	struct vdpa_device *vdpa = v->vdpa;
 	const struct vdpa_config_ops *ops = vdpa->config;
+	int ret;
 
 	if (!ops->suspend)
 		return -EOPNOTSUPP;
 
-	return ops->suspend(vdpa);
+	ret = ops->suspend(vdpa);
+	if (!ret)
+		v->suspended = true;
+
+	return ret;
 }
 
 /* After a successful return of this ioctl the device resumes processing
@@ -605,11 +613,16 @@ static long vhost_vdpa_resume(struct vhost_vdpa *v)
 {
 	struct vdpa_device *vdpa = v->vdpa;
 	const struct vdpa_config_ops *ops = vdpa->config;
+	int ret;
 
 	if (!ops->resume)
 		return -EOPNOTSUPP;
 
-	return ops->resume(vdpa);
+	ret = ops->resume(vdpa);
+	if (!ret)
+		v->suspended = false;
+
+	return ret;
 }
 
 static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] vdpa: Block vq property changes in DRIVER_OK
  2023-12-25 13:42 [PATCH 0/2] vdpa: Block vq property change in DRIVER_OK Dragos Tatulea
  2023-12-25 13:42 ` [PATCH 1/2] vdpa: Track device suspended state Dragos Tatulea
@ 2023-12-25 13:42 ` Dragos Tatulea
  2023-12-25 14:40 ` [PATCH 0/2] vdpa: Block vq property change " Michael S. Tsirkin
  2 siblings, 0 replies; 5+ messages in thread
From: Dragos Tatulea @ 2023-12-25 13:42 UTC (permalink / raw
  To: Michael S . Tsirkin, Jason Wang, Eugenio Perez Martin, Si-Wei Liu,
	virtualization, Gal Pressman
  Cc: Dragos Tatulea, kvm, linux-kernel, Parav Pandit, Xuan Zhuo

The virtio standard doesn't allow for virtqueue address and state
changes when the device is in DRIVER_OK. Return an error in such cases
unless the device is suspended.

The suspended device exception is needed because some devices support
virtqueue changes when the device is suspended.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Suggested-by: Eugenio Pérez <eperezma@redhat.com>
---
 drivers/vhost/vdpa.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 4c422be7d1e7..620073383d15 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -703,6 +703,9 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
 
 	switch (cmd) {
 	case VHOST_SET_VRING_ADDR:
+		if ((ops->get_status(vdpa) & VIRTIO_CONFIG_S_DRIVER_OK) && !v->suspended)
+			return -EINVAL;
+
 		if (ops->set_vq_address(vdpa, idx,
 					(u64)(uintptr_t)vq->desc,
 					(u64)(uintptr_t)vq->avail,
@@ -711,6 +714,9 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
 		break;
 
 	case VHOST_SET_VRING_BASE:
+		if ((ops->get_status(vdpa) & VIRTIO_CONFIG_S_DRIVER_OK) && !v->suspended)
+			return -EINVAL;
+
 		if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED)) {
 			vq_state.packed.last_avail_idx = vq->last_avail_idx & 0x7fff;
 			vq_state.packed.last_avail_counter = !!(vq->last_avail_idx & 0x8000);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] vdpa: Block vq property change in DRIVER_OK
  2023-12-25 13:42 [PATCH 0/2] vdpa: Block vq property change in DRIVER_OK Dragos Tatulea
  2023-12-25 13:42 ` [PATCH 1/2] vdpa: Track device suspended state Dragos Tatulea
  2023-12-25 13:42 ` [PATCH 2/2] vdpa: Block vq property changes in DRIVER_OK Dragos Tatulea
@ 2023-12-25 14:40 ` Michael S. Tsirkin
  2023-12-25 15:09   ` Dragos Tatulea
  2 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2023-12-25 14:40 UTC (permalink / raw
  To: Dragos Tatulea
  Cc: Jason Wang, Eugenio Perez Martin, Si-Wei Liu, virtualization,
	Gal Pressman, kvm, linux-kernel, Parav Pandit, Xuan Zhuo

On Mon, Dec 25, 2023 at 03:42:08PM +0200, Dragos Tatulea wrote:
> This series prevents the change of virtqueue address or state when a
> device is in DRIVER_OK and not suspended. The virtio spec doesn't
> allow changing virtqueue addresses and state in DRIVER_OK, but some devices
> do support this operation when the device is suspended. The series
> leaves the door open for these devices.
> 
> The series was suggested while discussing the addition of resumable
> virtuque support in the mlx5_vdpa driver [0].


I am confused. Isn't this also included in
 vdpa/mlx5: Add support for resumable vqs

do you now want this merged separately?

> [0] https://lore.kernel.org/virtualization/20231219180858.120898-1-dtatulea@nvidia.com/T/#m044ddf540d98a6b025f81bffa058ca647a3d013e
> 
> Dragos Tatulea (2):
>   vdpa: Track device suspended state
>   vdpa: Block vq property changes in DRIVER_OK
> 
>  drivers/vhost/vdpa.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> -- 
> 2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] vdpa: Block vq property change in DRIVER_OK
  2023-12-25 14:40 ` [PATCH 0/2] vdpa: Block vq property change " Michael S. Tsirkin
@ 2023-12-25 15:09   ` Dragos Tatulea
  0 siblings, 0 replies; 5+ messages in thread
From: Dragos Tatulea @ 2023-12-25 15:09 UTC (permalink / raw
  To: mst@redhat.com
  Cc: xuanzhuo@linux.alibaba.com, Parav Pandit, Gal Pressman,
	eperezma@redhat.com, virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, si-wei.liu@oracle.com,
	jasowang@redhat.com, kvm@vger.kernel.org

On Mon, 2023-12-25 at 09:40 -0500, Michael S. Tsirkin wrote:
> On Mon, Dec 25, 2023 at 03:42:08PM +0200, Dragos Tatulea wrote:
> > This series prevents the change of virtqueue address or state when a
> > device is in DRIVER_OK and not suspended. The virtio spec doesn't
> > allow changing virtqueue addresses and state in DRIVER_OK, but some devices
> > do support this operation when the device is suspended. The series
> > leaves the door open for these devices.
> > 
> > The series was suggested while discussing the addition of resumable
> > virtuque support in the mlx5_vdpa driver [0].
> 
> 
> I am confused. Isn't this also included in
>  vdpa/mlx5: Add support for resumable vqs
> 
> do you now want this merged separately?
The discussion in the linked thread lead to having 2 series that are
independent: this series and the original v2 of of the "vdpa/mlx5: Add support
for resumable vqs" series (for which I will send a v5 now that is same as v2 +
Acked-by tags).

> 
> > [0] https://lore.kernel.org/virtualization/20231219180858.120898-1-dtatulea@nvidia.com/T/#m044ddf540d98a6b025f81bffa058ca647a3d013e
> > 
> > Dragos Tatulea (2):
> >   vdpa: Track device suspended state
> >   vdpa: Block vq property changes in DRIVER_OK
> > 
> >  drivers/vhost/vdpa.c | 23 +++++++++++++++++++++--
> >  1 file changed, 21 insertions(+), 2 deletions(-)
> > 
> > -- 
> > 2.43.0
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-12-25 15:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-25 13:42 [PATCH 0/2] vdpa: Block vq property change in DRIVER_OK Dragos Tatulea
2023-12-25 13:42 ` [PATCH 1/2] vdpa: Track device suspended state Dragos Tatulea
2023-12-25 13:42 ` [PATCH 2/2] vdpa: Block vq property changes in DRIVER_OK Dragos Tatulea
2023-12-25 14:40 ` [PATCH 0/2] vdpa: Block vq property change " Michael S. Tsirkin
2023-12-25 15:09   ` Dragos Tatulea

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.