* [PATCH 1/3] vhost-user: do not send RESET_OWNER on device reset
2023-09-27 19:27 [PATCH 0/3] vhost: clean up device reset Stefan Hajnoczi
@ 2023-09-27 19:27 ` Stefan Hajnoczi
2023-09-28 3:01 ` Raphael Norwitz
2023-09-27 19:27 ` [PATCH 2/3] vhost-backend: remove vhost_kernel_reset_device() Stefan Hajnoczi
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2023-09-27 19:27 UTC (permalink / raw)
To: qemu-devel
Cc: hreitz, Raphael Norwitz, Fam Zheng, Michael S. Tsirkin, eperezma,
Paolo Bonzini, Stefan Hajnoczi
The VHOST_USER_RESET_OWNER message is deprecated in the spec:
This is no longer used. Used to be sent to request disabling all
rings, but some back-ends interpreted it to also discard connection
state (this interpretation would lead to bugs). It is recommended
that back-ends either ignore this message, or use it to disable all
rings.
The only caller of vhost_user_reset_device() is vhost_user_scsi_reset().
It checks that F_RESET_DEVICE was negotiated before calling it:
static void vhost_user_scsi_reset(VirtIODevice *vdev)
{
VHostSCSICommon *vsc = VHOST_SCSI_COMMON(vdev);
struct vhost_dev *dev = &vsc->dev;
/*
* Historically, reset was not implemented so only reset devices
* that are expecting it.
*/
if (!virtio_has_feature(dev->protocol_features,
VHOST_USER_PROTOCOL_F_RESET_DEVICE)) {
return;
}
if (dev->vhost_ops->vhost_reset_device) {
dev->vhost_ops->vhost_reset_device(dev);
}
}
Therefore VHOST_USER_RESET_OWNER is actually never sent by
vhost_user_reset_device(). Remove the dead code. This effectively moves
the vhost-user protocol specific code from vhost-user-scsi.c into
vhost-user.c where it belongs.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/scsi/vhost-user-scsi.c | 9 ---------
hw/virtio/vhost-user.c | 13 +++++++++----
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index ee99b19e7a..8582b2e8ab 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -71,15 +71,6 @@ static void vhost_user_scsi_reset(VirtIODevice *vdev)
VHostSCSICommon *vsc = VHOST_SCSI_COMMON(vdev);
struct vhost_dev *dev = &vsc->dev;
- /*
- * Historically, reset was not implemented so only reset devices
- * that are expecting it.
- */
- if (!virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_RESET_DEVICE)) {
- return;
- }
-
if (dev->vhost_ops->vhost_reset_device) {
dev->vhost_ops->vhost_reset_device(dev);
}
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 8dcf049d42..7bed9ad7d5 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1492,12 +1492,17 @@ static int vhost_user_reset_device(struct vhost_dev *dev)
{
VhostUserMsg msg = {
.hdr.flags = VHOST_USER_VERSION,
+ .hdr.request = VHOST_USER_RESET_DEVICE,
};
- msg.hdr.request = virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_RESET_DEVICE)
- ? VHOST_USER_RESET_DEVICE
- : VHOST_USER_RESET_OWNER;
+ /*
+ * Historically, reset was not implemented so only reset devices
+ * that are expecting it.
+ */
+ if (!virtio_has_feature(dev->protocol_features,
+ VHOST_USER_PROTOCOL_F_RESET_DEVICE)) {
+ return -ENOSYS;
+ }
return vhost_user_write(dev, &msg, NULL, 0);
}
--
2.41.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/3] vhost-user: do not send RESET_OWNER on device reset
2023-09-27 19:27 ` [PATCH 1/3] vhost-user: do not send RESET_OWNER on " Stefan Hajnoczi
@ 2023-09-28 3:01 ` Raphael Norwitz
0 siblings, 0 replies; 11+ messages in thread
From: Raphael Norwitz @ 2023-09-28 3:01 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: open list:All patches CC here, Hanna Reitz, Fam Zheng,
Michael S. Tsirkin, eperezma@redhat.com, Paolo Bonzini
> On Sep 27, 2023, at 3:27 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> The VHOST_USER_RESET_OWNER message is deprecated in the spec:
>
> This is no longer used. Used to be sent to request disabling all
> rings, but some back-ends interpreted it to also discard connection
> state (this interpretation would lead to bugs). It is recommended
> that back-ends either ignore this message, or use it to disable all
> rings.
>
> The only caller of vhost_user_reset_device() is vhost_user_scsi_reset().
> It checks that F_RESET_DEVICE was negotiated before calling it:
>
> static void vhost_user_scsi_reset(VirtIODevice *vdev)
> {
> VHostSCSICommon *vsc = VHOST_SCSI_COMMON(vdev);
> struct vhost_dev *dev = &vsc->dev;
>
> /*
> * Historically, reset was not implemented so only reset devices
> * that are expecting it.
> */
> if (!virtio_has_feature(dev->protocol_features,
> VHOST_USER_PROTOCOL_F_RESET_DEVICE)) {
> return;
> }
>
> if (dev->vhost_ops->vhost_reset_device) {
> dev->vhost_ops->vhost_reset_device(dev);
> }
> }
>
> Therefore VHOST_USER_RESET_OWNER is actually never sent by
> vhost_user_reset_device(). Remove the dead code. This effectively moves
> the vhost-user protocol specific code from vhost-user-scsi.c into
> vhost-user.c where it belongs.
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> hw/scsi/vhost-user-scsi.c | 9 ---------
> hw/virtio/vhost-user.c | 13 +++++++++----
> 2 files changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> index ee99b19e7a..8582b2e8ab 100644
> --- a/hw/scsi/vhost-user-scsi.c
> +++ b/hw/scsi/vhost-user-scsi.c
> @@ -71,15 +71,6 @@ static void vhost_user_scsi_reset(VirtIODevice *vdev)
> VHostSCSICommon *vsc = VHOST_SCSI_COMMON(vdev);
> struct vhost_dev *dev = &vsc->dev;
>
> - /*
> - * Historically, reset was not implemented so only reset devices
> - * that are expecting it.
> - */
> - if (!virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_RESET_DEVICE)) {
> - return;
> - }
> -
> if (dev->vhost_ops->vhost_reset_device) {
> dev->vhost_ops->vhost_reset_device(dev);
> }
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 8dcf049d42..7bed9ad7d5 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1492,12 +1492,17 @@ static int vhost_user_reset_device(struct vhost_dev *dev)
> {
> VhostUserMsg msg = {
> .hdr.flags = VHOST_USER_VERSION,
> + .hdr.request = VHOST_USER_RESET_DEVICE,
> };
>
> - msg.hdr.request = virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_RESET_DEVICE)
> - ? VHOST_USER_RESET_DEVICE
> - : VHOST_USER_RESET_OWNER;
> + /*
> + * Historically, reset was not implemented so only reset devices
> + * that are expecting it.
> + */
> + if (!virtio_has_feature(dev->protocol_features,
> + VHOST_USER_PROTOCOL_F_RESET_DEVICE)) {
> + return -ENOSYS;
> + }
>
> return vhost_user_write(dev, &msg, NULL, 0);
> }
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] vhost-backend: remove vhost_kernel_reset_device()
2023-09-27 19:27 [PATCH 0/3] vhost: clean up device reset Stefan Hajnoczi
2023-09-27 19:27 ` [PATCH 1/3] vhost-user: do not send RESET_OWNER on " Stefan Hajnoczi
@ 2023-09-27 19:27 ` Stefan Hajnoczi
2023-09-28 3:01 ` Raphael Norwitz
2023-09-27 19:27 ` [PATCH 3/3] virtio: call ->vhost_reset_device() during reset Stefan Hajnoczi
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2023-09-27 19:27 UTC (permalink / raw)
To: qemu-devel
Cc: hreitz, Raphael Norwitz, Fam Zheng, Michael S. Tsirkin, eperezma,
Paolo Bonzini, Stefan Hajnoczi
vhost_kernel_reset_device() invokes RESET_OWNER, which disassociates the
owner process from the device. The device is left non-operational since
SET_OWNER is only called once during startup in vhost_dev_init().
vhost_kernel_reset_device() is never called so this latent bug never
appears. Get rid of vhost_kernel_reset_device() for now. If someone
needs it in the future they'll need to implement it correctly.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/virtio/vhost-backend.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 8e581575c9..17f3fc6a08 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -197,11 +197,6 @@ static int vhost_kernel_set_owner(struct vhost_dev *dev)
return vhost_kernel_call(dev, VHOST_SET_OWNER, NULL);
}
-static int vhost_kernel_reset_device(struct vhost_dev *dev)
-{
- return vhost_kernel_call(dev, VHOST_RESET_OWNER, NULL);
-}
-
static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
{
assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
@@ -322,7 +317,6 @@ const VhostOps kernel_ops = {
.vhost_get_features = vhost_kernel_get_features,
.vhost_set_backend_cap = vhost_kernel_set_backend_cap,
.vhost_set_owner = vhost_kernel_set_owner,
- .vhost_reset_device = vhost_kernel_reset_device,
.vhost_get_vq_index = vhost_kernel_get_vq_index,
.vhost_vsock_set_guest_cid = vhost_kernel_vsock_set_guest_cid,
.vhost_vsock_set_running = vhost_kernel_vsock_set_running,
--
2.41.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/3] vhost-backend: remove vhost_kernel_reset_device()
2023-09-27 19:27 ` [PATCH 2/3] vhost-backend: remove vhost_kernel_reset_device() Stefan Hajnoczi
@ 2023-09-28 3:01 ` Raphael Norwitz
0 siblings, 0 replies; 11+ messages in thread
From: Raphael Norwitz @ 2023-09-28 3:01 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: open list:All patches CC here, Hanna Reitz, Fam Zheng,
Michael S. Tsirkin, eperezma@redhat.com, Paolo Bonzini
> On Sep 27, 2023, at 3:27 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> vhost_kernel_reset_device() invokes RESET_OWNER, which disassociates the
> owner process from the device. The device is left non-operational since
> SET_OWNER is only called once during startup in vhost_dev_init().
>
> vhost_kernel_reset_device() is never called so this latent bug never
> appears. Get rid of vhost_kernel_reset_device() for now. If someone
> needs it in the future they'll need to implement it correctly.
>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> hw/virtio/vhost-backend.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> index 8e581575c9..17f3fc6a08 100644
> --- a/hw/virtio/vhost-backend.c
> +++ b/hw/virtio/vhost-backend.c
> @@ -197,11 +197,6 @@ static int vhost_kernel_set_owner(struct vhost_dev *dev)
> return vhost_kernel_call(dev, VHOST_SET_OWNER, NULL);
> }
>
> -static int vhost_kernel_reset_device(struct vhost_dev *dev)
> -{
> - return vhost_kernel_call(dev, VHOST_RESET_OWNER, NULL);
> -}
> -
> static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
> {
> assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
> @@ -322,7 +317,6 @@ const VhostOps kernel_ops = {
> .vhost_get_features = vhost_kernel_get_features,
> .vhost_set_backend_cap = vhost_kernel_set_backend_cap,
> .vhost_set_owner = vhost_kernel_set_owner,
> - .vhost_reset_device = vhost_kernel_reset_device,
> .vhost_get_vq_index = vhost_kernel_get_vq_index,
> .vhost_vsock_set_guest_cid = vhost_kernel_vsock_set_guest_cid,
> .vhost_vsock_set_running = vhost_kernel_vsock_set_running,
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] virtio: call ->vhost_reset_device() during reset
2023-09-27 19:27 [PATCH 0/3] vhost: clean up device reset Stefan Hajnoczi
2023-09-27 19:27 ` [PATCH 1/3] vhost-user: do not send RESET_OWNER on " Stefan Hajnoczi
2023-09-27 19:27 ` [PATCH 2/3] vhost-backend: remove vhost_kernel_reset_device() Stefan Hajnoczi
@ 2023-09-27 19:27 ` Stefan Hajnoczi
2023-09-28 3:01 ` Raphael Norwitz
2023-09-28 10:38 ` [PATCH 0/3] vhost: clean up device reset Eugenio Perez Martin
2023-10-03 21:00 ` Michael S. Tsirkin
4 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2023-09-27 19:27 UTC (permalink / raw)
To: qemu-devel
Cc: hreitz, Raphael Norwitz, Fam Zheng, Michael S. Tsirkin, eperezma,
Paolo Bonzini, Stefan Hajnoczi
vhost-user-scsi has a VirtioDeviceClass->reset() function that calls
->vhost_reset_device(). The other vhost devices don't notify the vhost
device upon reset.
Stateful vhost devices may need to handle device reset in order to free
resources or prevent stale device state from interfering after reset.
Call ->vhost_device_reset() from virtio_reset() so that that vhost
devices are notified of device reset.
This patch affects behavior as follows:
- vhost-kernel: No change in behavior since ->vhost_reset_device() is
not implemented.
- vhost-user: back-ends that negotiate
VHOST_USER_PROTOCOL_F_RESET_DEVICE now receive a
VHOST_USER_DEVICE_RESET message upon device reset. Otherwise there is
no change in behavior. DPDK, SPDK, libvhost-user, and the
vhost-user-backend crate do not negotiate
VHOST_USER_PROTOCOL_F_RESET_DEVICE automatically.
- vhost-vdpa: an extra SET_STATUS 0 call is made during device reset.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/hw/virtio/vhost.h | 3 +++
hw/scsi/vhost-user-scsi.c | 11 -----------
hw/virtio/vhost.c | 9 +++++++++
hw/virtio/virtio.c | 4 ++++
4 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 6a173cb9fa..381fb51966 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -338,4 +338,7 @@ int vhost_dev_set_inflight(struct vhost_dev *dev,
int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
struct vhost_inflight *inflight);
bool vhost_dev_has_iommu(struct vhost_dev *dev);
+
+int vhost_reset_device(struct vhost_dev *hdev);
+
#endif
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 8582b2e8ab..6917a748bb 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -66,16 +66,6 @@ static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status)
}
}
-static void vhost_user_scsi_reset(VirtIODevice *vdev)
-{
- VHostSCSICommon *vsc = VHOST_SCSI_COMMON(vdev);
- struct vhost_dev *dev = &vsc->dev;
-
- if (dev->vhost_ops->vhost_reset_device) {
- dev->vhost_ops->vhost_reset_device(dev);
- }
-}
-
static void vhost_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq)
{
}
@@ -195,7 +185,6 @@ static void vhost_user_scsi_class_init(ObjectClass *klass, void *data)
vdc->get_features = vhost_scsi_common_get_features;
vdc->set_config = vhost_scsi_common_set_config;
vdc->set_status = vhost_user_scsi_set_status;
- vdc->reset = vhost_user_scsi_reset;
fwc->get_dev_path = vhost_scsi_common_get_fw_dev_path;
}
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index e2f6ffb446..6003e50e83 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -2087,3 +2087,12 @@ int vhost_net_set_backend(struct vhost_dev *hdev,
return -ENOSYS;
}
+
+int vhost_reset_device(struct vhost_dev *hdev)
+{
+ if (hdev->vhost_ops->vhost_reset_device) {
+ return hdev->vhost_ops->vhost_reset_device(hdev);
+ }
+
+ return -ENOSYS;
+}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 4577f3f5b3..d863ffd5d6 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2121,6 +2121,10 @@ void virtio_reset(void *opaque)
vdev->device_endian = virtio_default_endian();
}
+ if (vdev->vhost_started) {
+ vhost_reset_device(k->get_vhost(vdev));
+ }
+
if (k->reset) {
k->reset(vdev);
}
--
2.41.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/3] virtio: call ->vhost_reset_device() during reset
2023-09-27 19:27 ` [PATCH 3/3] virtio: call ->vhost_reset_device() during reset Stefan Hajnoczi
@ 2023-09-28 3:01 ` Raphael Norwitz
0 siblings, 0 replies; 11+ messages in thread
From: Raphael Norwitz @ 2023-09-28 3:01 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: open list:All patches CC here, Hanna Reitz, Fam Zheng,
Michael S. Tsirkin, eperezma@redhat.com, Paolo Bonzini
> On Sep 27, 2023, at 3:27 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> vhost-user-scsi has a VirtioDeviceClass->reset() function that calls
> ->vhost_reset_device(). The other vhost devices don't notify the vhost
> device upon reset.
>
> Stateful vhost devices may need to handle device reset in order to free
> resources or prevent stale device state from interfering after reset.
>
> Call ->vhost_device_reset() from virtio_reset() so that that vhost
> devices are notified of device reset.
>
> This patch affects behavior as follows:
> - vhost-kernel: No change in behavior since ->vhost_reset_device() is
> not implemented.
> - vhost-user: back-ends that negotiate
> VHOST_USER_PROTOCOL_F_RESET_DEVICE now receive a
> VHOST_USER_DEVICE_RESET message upon device reset. Otherwise there is
> no change in behavior. DPDK, SPDK, libvhost-user, and the
> vhost-user-backend crate do not negotiate
> VHOST_USER_PROTOCOL_F_RESET_DEVICE automatically.
> - vhost-vdpa: an extra SET_STATUS 0 call is made during device reset.
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> include/hw/virtio/vhost.h | 3 +++
> hw/scsi/vhost-user-scsi.c | 11 -----------
> hw/virtio/vhost.c | 9 +++++++++
> hw/virtio/virtio.c | 4 ++++
> 4 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 6a173cb9fa..381fb51966 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -338,4 +338,7 @@ int vhost_dev_set_inflight(struct vhost_dev *dev,
> int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
> struct vhost_inflight *inflight);
> bool vhost_dev_has_iommu(struct vhost_dev *dev);
> +
> +int vhost_reset_device(struct vhost_dev *hdev);
> +
> #endif
> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> index 8582b2e8ab..6917a748bb 100644
> --- a/hw/scsi/vhost-user-scsi.c
> +++ b/hw/scsi/vhost-user-scsi.c
> @@ -66,16 +66,6 @@ static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status)
> }
> }
>
> -static void vhost_user_scsi_reset(VirtIODevice *vdev)
> -{
> - VHostSCSICommon *vsc = VHOST_SCSI_COMMON(vdev);
> - struct vhost_dev *dev = &vsc->dev;
> -
> - if (dev->vhost_ops->vhost_reset_device) {
> - dev->vhost_ops->vhost_reset_device(dev);
> - }
> -}
> -
> static void vhost_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq)
> {
> }
> @@ -195,7 +185,6 @@ static void vhost_user_scsi_class_init(ObjectClass *klass, void *data)
> vdc->get_features = vhost_scsi_common_get_features;
> vdc->set_config = vhost_scsi_common_set_config;
> vdc->set_status = vhost_user_scsi_set_status;
> - vdc->reset = vhost_user_scsi_reset;
> fwc->get_dev_path = vhost_scsi_common_get_fw_dev_path;
> }
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index e2f6ffb446..6003e50e83 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -2087,3 +2087,12 @@ int vhost_net_set_backend(struct vhost_dev *hdev,
>
> return -ENOSYS;
> }
> +
> +int vhost_reset_device(struct vhost_dev *hdev)
> +{
> + if (hdev->vhost_ops->vhost_reset_device) {
> + return hdev->vhost_ops->vhost_reset_device(hdev);
> + }
> +
> + return -ENOSYS;
> +}
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 4577f3f5b3..d863ffd5d6 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2121,6 +2121,10 @@ void virtio_reset(void *opaque)
> vdev->device_endian = virtio_default_endian();
> }
>
> + if (vdev->vhost_started) {
> + vhost_reset_device(k->get_vhost(vdev));
> + }
> +
> if (k->reset) {
> k->reset(vdev);
> }
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] vhost: clean up device reset
2023-09-27 19:27 [PATCH 0/3] vhost: clean up device reset Stefan Hajnoczi
` (2 preceding siblings ...)
2023-09-27 19:27 ` [PATCH 3/3] virtio: call ->vhost_reset_device() during reset Stefan Hajnoczi
@ 2023-09-28 10:38 ` Eugenio Perez Martin
2023-10-02 14:11 ` Lei Yang
2023-10-03 21:00 ` Michael S. Tsirkin
4 siblings, 1 reply; 11+ messages in thread
From: Eugenio Perez Martin @ 2023-09-28 10:38 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel, hreitz, Raphael Norwitz, Fam Zheng,
Michael S. Tsirkin, Paolo Bonzini
On Wed, Sep 27, 2023 at 9:27 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> Stateful vhost devices may need to free resources or clear device state upon
> device reset. The vhost-user protocol has a VHOST_USER_RESET_DEVICE message for
> this and vDPA has SET_STATUS 0, but only QEMU's vhost-user-scsi device actually
> implements this today.
>
> This patch series performs device reset across all device types. When
> virtio_reset() is called, the associated vhost_dev's ->vhost_reset_device() is
> called. vhost-user-scsi's one-off implementation is obsoleted and removed.
>
> This patch affects behavior as follows:
> - vhost-kernel: no change in behavior. No ioctl calls are made.
> - vhost-user: back-ends that negotiate
> VHOST_USER_PROTOCOL_F_RESET_DEVICE now receive a
> VHOST_USER_DEVICE_RESET message upon device reset. Otherwise there is
> no change in behavior. DPDK, SPDK, libvhost-user, and the
> vhost-user-backend crate do not negotiate
> VHOST_USER_PROTOCOL_F_RESET_DEVICE automatically.
> - vhost-vdpa: an extra SET_STATUS 0 call is made during device reset.
>
> I have tested this series with vhost-net (kernel), vhost-user-blk, and
> vhost-user-fs (both Rust and legacy C).
>
Acked-by: Eugenio Pérez <eperezma@redhat.com>
> Stefan Hajnoczi (3):
> vhost-user: do not send RESET_OWNER on device reset
> vhost-backend: remove vhost_kernel_reset_device()
> virtio: call ->vhost_reset_device() during reset
>
> include/hw/virtio/vhost.h | 3 +++
> hw/scsi/vhost-user-scsi.c | 20 --------------------
> hw/virtio/vhost-backend.c | 6 ------
> hw/virtio/vhost-user.c | 13 +++++++++----
> hw/virtio/vhost.c | 9 +++++++++
> hw/virtio/virtio.c | 4 ++++
> 6 files changed, 25 insertions(+), 30 deletions(-)
>
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 0/3] vhost: clean up device reset
2023-09-28 10:38 ` [PATCH 0/3] vhost: clean up device reset Eugenio Perez Martin
@ 2023-10-02 14:11 ` Lei Yang
0 siblings, 0 replies; 11+ messages in thread
From: Lei Yang @ 2023-10-02 14:11 UTC (permalink / raw)
To: Eugenio Perez Martin, Stefan Hajnoczi
Cc: qemu-devel, hreitz, Raphael Norwitz, Fam Zheng,
Michael S. Tsirkin, Paolo Bonzini
QE tested a regression testing on this series with vhost-vdpa device,
everything is working fine.
Tested-by: Lei Yang <Leiyang@redhat.com>
On Thu, Sep 28, 2023 at 6:40 PM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Wed, Sep 27, 2023 at 9:27 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > Stateful vhost devices may need to free resources or clear device state upon
> > device reset. The vhost-user protocol has a VHOST_USER_RESET_DEVICE message for
> > this and vDPA has SET_STATUS 0, but only QEMU's vhost-user-scsi device actually
> > implements this today.
> >
> > This patch series performs device reset across all device types. When
> > virtio_reset() is called, the associated vhost_dev's ->vhost_reset_device() is
> > called. vhost-user-scsi's one-off implementation is obsoleted and removed.
> >
> > This patch affects behavior as follows:
> > - vhost-kernel: no change in behavior. No ioctl calls are made.
> > - vhost-user: back-ends that negotiate
> > VHOST_USER_PROTOCOL_F_RESET_DEVICE now receive a
> > VHOST_USER_DEVICE_RESET message upon device reset. Otherwise there is
> > no change in behavior. DPDK, SPDK, libvhost-user, and the
> > vhost-user-backend crate do not negotiate
> > VHOST_USER_PROTOCOL_F_RESET_DEVICE automatically.
> > - vhost-vdpa: an extra SET_STATUS 0 call is made during device reset.
> >
> > I have tested this series with vhost-net (kernel), vhost-user-blk, and
> > vhost-user-fs (both Rust and legacy C).
> >
>
> Acked-by: Eugenio Pérez <eperezma@redhat.com>
>
> > Stefan Hajnoczi (3):
> > vhost-user: do not send RESET_OWNER on device reset
> > vhost-backend: remove vhost_kernel_reset_device()
> > virtio: call ->vhost_reset_device() during reset
> >
> > include/hw/virtio/vhost.h | 3 +++
> > hw/scsi/vhost-user-scsi.c | 20 --------------------
> > hw/virtio/vhost-backend.c | 6 ------
> > hw/virtio/vhost-user.c | 13 +++++++++----
> > hw/virtio/vhost.c | 9 +++++++++
> > hw/virtio/virtio.c | 4 ++++
> > 6 files changed, 25 insertions(+), 30 deletions(-)
> >
> > --
> > 2.41.0
> >
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] vhost: clean up device reset
2023-09-27 19:27 [PATCH 0/3] vhost: clean up device reset Stefan Hajnoczi
` (3 preceding siblings ...)
2023-09-28 10:38 ` [PATCH 0/3] vhost: clean up device reset Eugenio Perez Martin
@ 2023-10-03 21:00 ` Michael S. Tsirkin
2023-10-03 21:09 ` Stefan Hajnoczi
4 siblings, 1 reply; 11+ messages in thread
From: Michael S. Tsirkin @ 2023-10-03 21:00 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel, hreitz, Raphael Norwitz, Fam Zheng, eperezma,
Paolo Bonzini
On Wed, Sep 27, 2023 at 03:27:34PM -0400, Stefan Hajnoczi wrote:
> Stateful vhost devices may need to free resources or clear device state upon
> device reset. The vhost-user protocol has a VHOST_USER_RESET_DEVICE message for
> this and vDPA has SET_STATUS 0, but only QEMU's vhost-user-scsi device actually
> implements this today.
>
> This patch series performs device reset across all device types. When
> virtio_reset() is called, the associated vhost_dev's ->vhost_reset_device() is
> called. vhost-user-scsi's one-off implementation is obsoleted and removed.
>
> This patch affects behavior as follows:
> - vhost-kernel: no change in behavior. No ioctl calls are made.
> - vhost-user: back-ends that negotiate
> VHOST_USER_PROTOCOL_F_RESET_DEVICE now receive a
> VHOST_USER_DEVICE_RESET message upon device reset. Otherwise there is
> no change in behavior. DPDK, SPDK, libvhost-user, and the
> vhost-user-backend crate do not negotiate
> VHOST_USER_PROTOCOL_F_RESET_DEVICE automatically.
> - vhost-vdpa: an extra SET_STATUS 0 call is made during device reset.
>
> I have tested this series with vhost-net (kernel), vhost-user-blk, and
> vhost-user-fs (both Rust and legacy C).
>
> Stefan Hajnoczi (3):
> vhost-user: do not send RESET_OWNER on device reset
> vhost-backend: remove vhost_kernel_reset_device()
> virtio: call ->vhost_reset_device() during reset
Build failure:
https://gitlab.com/mstredhat/qemu/-/jobs/5215049540
> include/hw/virtio/vhost.h | 3 +++
> hw/scsi/vhost-user-scsi.c | 20 --------------------
> hw/virtio/vhost-backend.c | 6 ------
> hw/virtio/vhost-user.c | 13 +++++++++----
> hw/virtio/vhost.c | 9 +++++++++
> hw/virtio/virtio.c | 4 ++++
> 6 files changed, 25 insertions(+), 30 deletions(-)
>
> --
> 2.41.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 0/3] vhost: clean up device reset
2023-10-03 21:00 ` Michael S. Tsirkin
@ 2023-10-03 21:09 ` Stefan Hajnoczi
0 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2023-10-03 21:09 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Stefan Hajnoczi, qemu-devel, hreitz, Raphael Norwitz, Fam Zheng,
eperezma, Paolo Bonzini
On Tue, 3 Oct 2023 at 17:01, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Sep 27, 2023 at 03:27:34PM -0400, Stefan Hajnoczi wrote:
> > Stateful vhost devices may need to free resources or clear device state upon
> > device reset. The vhost-user protocol has a VHOST_USER_RESET_DEVICE message for
> > this and vDPA has SET_STATUS 0, but only QEMU's vhost-user-scsi device actually
> > implements this today.
> >
> > This patch series performs device reset across all device types. When
> > virtio_reset() is called, the associated vhost_dev's ->vhost_reset_device() is
> > called. vhost-user-scsi's one-off implementation is obsoleted and removed.
> >
> > This patch affects behavior as follows:
> > - vhost-kernel: no change in behavior. No ioctl calls are made.
> > - vhost-user: back-ends that negotiate
> > VHOST_USER_PROTOCOL_F_RESET_DEVICE now receive a
> > VHOST_USER_DEVICE_RESET message upon device reset. Otherwise there is
> > no change in behavior. DPDK, SPDK, libvhost-user, and the
> > vhost-user-backend crate do not negotiate
> > VHOST_USER_PROTOCOL_F_RESET_DEVICE automatically.
> > - vhost-vdpa: an extra SET_STATUS 0 call is made during device reset.
> >
> > I have tested this series with vhost-net (kernel), vhost-user-blk, and
> > vhost-user-fs (both Rust and legacy C).
> >
> > Stefan Hajnoczi (3):
> > vhost-user: do not send RESET_OWNER on device reset
> > vhost-backend: remove vhost_kernel_reset_device()
> > virtio: call ->vhost_reset_device() during reset
>
> Build failure:
> https://gitlab.com/mstredhat/qemu/-/jobs/5215049540
Sorry about that, will fix.
Stefan
> > include/hw/virtio/vhost.h | 3 +++
> > hw/scsi/vhost-user-scsi.c | 20 --------------------
> > hw/virtio/vhost-backend.c | 6 ------
> > hw/virtio/vhost-user.c | 13 +++++++++----
> > hw/virtio/vhost.c | 9 +++++++++
> > hw/virtio/virtio.c | 4 ++++
> > 6 files changed, 25 insertions(+), 30 deletions(-)
> >
> > --
> > 2.41.0
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread