* [Qemu-devel] [PATCH v3 0/5] virtio: fix some issues of "started" and "start_on_kick" flag
@ 2019-06-14 9:31 elohimes
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 1/5] virtio: add "use-started" property elohimes
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: elohimes @ 2019-06-14 9:31 UTC (permalink / raw)
To: mst, groug; +Cc: qemu-devel, Xie Yongji, dgilbert, ehabkost
From: Xie Yongji <xieyongji@baidu.com>
We introduced two flags "started" and "start_on_kick" to indicate virtio
device's state before. But there still are some problems with them. So
we try to fixup them in this patchset.
The patch 1 introduces a "use-started" property to avoid a migration
issue under Greg Kurz's suggestion [1].
The patch 2 set "start_on_kick" flag for legacy devices.
The patch 3 fixes a regression bug that old guest is not able to boot with
vhost-user-blk device.
The patch 4,5 fix some problems with "started" and "start_on_kick" flag.
[1] https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg06247.html
v3:
- change the order of patches
- Also disable "use-started" property by hw_compat_4_0
v2:
- Recalculate "start_on_kick" flag after migration instead of migrating
it
- Set "start_on_kick" flag for legacy devices
- Avoid setting "started" flag when "use_started" property is true
- Set "use_started" to false by hw_compat_4_0_1 instead of hw_compat_4_0
Xie Yongji (5):
virtio: add "use-started" property
virtio: Set "start_on_kick" for legacy devices
virtio: Set "start_on_kick" on virtio_set_features()
virtio: Make sure we get correct state of device on
handle_aio_output()
virtio: Don't change "started" flag on virtio_vmstate_change()
hw/block/vhost-user-blk.c | 4 +--
hw/core/machine.c | 8 ++++--
hw/virtio/virtio.c | 53 ++++++++++++++++++++++----------------
include/hw/virtio/virtio.h | 23 ++++++++++++++++-
4 files changed, 61 insertions(+), 27 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v3 1/5] virtio: add "use-started" property
2019-06-14 9:31 [Qemu-devel] [PATCH v3 0/5] virtio: fix some issues of "started" and "start_on_kick" flag elohimes
@ 2019-06-14 9:31 ` elohimes
2019-06-14 11:44 ` Greg Kurz
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 2/5] virtio: Set "start_on_kick" for legacy devices elohimes
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: elohimes @ 2019-06-14 9:31 UTC (permalink / raw)
To: mst, groug; +Cc: qemu-devel, Xie Yongji, dgilbert, ehabkost
From: Xie Yongji <xieyongji@baidu.com>
In order to avoid migration issues, we introduce a "use-started"
property to the base virtio device to indicate whether use
"started" flag or not. This property will be true by default and
set to false when machine type <= 4.0.1.
Suggested-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Xie Yongji <xieyongji@baidu.com>
---
hw/block/vhost-user-blk.c | 4 ++--
hw/core/machine.c | 8 ++++++--
hw/virtio/virtio.c | 18 +++++++-----------
include/hw/virtio/virtio.h | 21 +++++++++++++++++++++
4 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9cb61336a6..85bc4017e7 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -191,7 +191,7 @@ static void vhost_user_blk_stop(VirtIODevice *vdev)
static void vhost_user_blk_set_status(VirtIODevice *vdev, uint8_t status)
{
VHostUserBlk *s = VHOST_USER_BLK(vdev);
- bool should_start = vdev->started;
+ bool should_start = virtio_device_started(vdev, status);
int ret;
if (!vdev->vm_running) {
@@ -317,7 +317,7 @@ static int vhost_user_blk_connect(DeviceState *dev)
}
/* restore vhost state */
- if (vdev->started) {
+ if (virtio_device_started(vdev, vdev->status)) {
ret = vhost_user_blk_start(vdev);
if (ret < 0) {
error_report("vhost-user-blk: vhost start failed: %s",
diff --git a/hw/core/machine.c b/hw/core/machine.c
index f1a0f45f9c..60d1e27d2c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -24,10 +24,14 @@
#include "hw/pci/pci.h"
#include "hw/mem/nvdimm.h"
-GlobalProperty hw_compat_4_0_1[] = {};
+GlobalProperty hw_compat_4_0_1[] = {
+ { "virtio-device", "use-started", "false" },
+};
const size_t hw_compat_4_0_1_len = G_N_ELEMENTS(hw_compat_4_0_1);
-GlobalProperty hw_compat_4_0[] = {};
+GlobalProperty hw_compat_4_0[] = {
+ { "virtio-device", "use-started", "false" },
+};
const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
GlobalProperty hw_compat_3_1[] = {
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 07f4a64b48..19062fbb96 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1162,10 +1162,8 @@ int virtio_set_status(VirtIODevice *vdev, uint8_t val)
}
}
}
- vdev->started = val & VIRTIO_CONFIG_S_DRIVER_OK;
- if (unlikely(vdev->start_on_kick && vdev->started)) {
- vdev->start_on_kick = false;
- }
+
+ virtio_set_started(vdev, val & VIRTIO_CONFIG_S_DRIVER_OK);
if (k->set_status) {
k->set_status(vdev, val);
@@ -1536,8 +1534,7 @@ static bool virtio_queue_notify_aio_vq(VirtQueue *vq)
ret = vq->handle_aio_output(vdev, vq);
if (unlikely(vdev->start_on_kick)) {
- vdev->started = true;
- vdev->start_on_kick = false;
+ virtio_set_started(vdev, true);
}
}
@@ -1557,8 +1554,7 @@ static void virtio_queue_notify_vq(VirtQueue *vq)
vq->handle_output(vdev, vq);
if (unlikely(vdev->start_on_kick)) {
- vdev->started = true;
- vdev->start_on_kick = false;
+ virtio_set_started(vdev, true);
}
}
}
@@ -1579,8 +1575,7 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
}
if (unlikely(vdev->start_on_kick)) {
- vdev->started = true;
- vdev->start_on_kick = false;
+ virtio_set_started(vdev, true);
}
}
@@ -2291,7 +2286,7 @@ static void virtio_vmstate_change(void *opaque, int running, RunState state)
VirtIODevice *vdev = opaque;
BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
- bool backend_run = running && vdev->started;
+ bool backend_run = running && virtio_device_started(vdev, vdev->status);
vdev->vm_running = running;
if (backend_run) {
@@ -2669,6 +2664,7 @@ static void virtio_device_instance_finalize(Object *obj)
static Property virtio_properties[] = {
DEFINE_VIRTIO_COMMON_FEATURES(VirtIODevice, host_features),
+ DEFINE_PROP_BOOL("use-started", VirtIODevice, use_started, true),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 27c0efc3d0..15d5366939 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -105,6 +105,7 @@ struct VirtIODevice
uint16_t device_id;
bool vm_running;
bool broken; /* device in invalid state, needs reset */
+ bool use_started;
bool started;
bool start_on_kick; /* virtio 1.0 transitional devices support that */
VMChangeStateEntry *vmstate;
@@ -351,4 +352,24 @@ static inline bool virtio_is_big_endian(VirtIODevice *vdev)
/* Devices conforming to VIRTIO 1.0 or later are always LE. */
return false;
}
+
+static inline bool virtio_device_started(VirtIODevice *vdev, uint8_t status)
+{
+ if (vdev->use_started) {
+ return vdev->started;
+ }
+
+ return status & VIRTIO_CONFIG_S_DRIVER_OK;
+}
+
+static inline void virtio_set_started(VirtIODevice *vdev, bool started)
+{
+ if (started) {
+ vdev->start_on_kick = false;
+ }
+
+ if (vdev->use_started) {
+ vdev->started = started;
+ }
+}
#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v3 2/5] virtio: Set "start_on_kick" for legacy devices
2019-06-14 9:31 [Qemu-devel] [PATCH v3 0/5] virtio: fix some issues of "started" and "start_on_kick" flag elohimes
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 1/5] virtio: add "use-started" property elohimes
@ 2019-06-14 9:31 ` elohimes
2019-06-16 17:27 ` Greg Kurz
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 3/5] virtio: Set "start_on_kick" on virtio_set_features() elohimes
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: elohimes @ 2019-06-14 9:31 UTC (permalink / raw)
To: mst, groug; +Cc: qemu-devel, Xie Yongji, dgilbert, ehabkost
From: Xie Yongji <xieyongji@baidu.com>
Besides virtio 1.0 transitional devices, we should also
set "start_on_kick" flag for legacy devices (virtio 0.9).
Signed-off-by: Xie Yongji <xieyongji@baidu.com>
---
hw/virtio/virtio.c | 6 ++----
include/hw/virtio/virtio.h | 2 +-
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 19062fbb96..473881e9ec 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1212,8 +1212,7 @@ void virtio_reset(void *opaque)
k->reset(vdev);
}
- vdev->start_on_kick = (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
- !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1));
+ vdev->start_on_kick = !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
vdev->started = false;
vdev->broken = false;
vdev->guest_features = 0;
@@ -2325,8 +2324,7 @@ void virtio_init(VirtIODevice *vdev, const char *name,
g_malloc0(sizeof(*vdev->vector_queues) * nvectors);
}
- vdev->start_on_kick = (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
- !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1));
+ vdev->start_on_kick = !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
vdev->started = false;
vdev->device_id = device_id;
vdev->status = 0;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 15d5366939..b189788cb2 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -107,7 +107,7 @@ struct VirtIODevice
bool broken; /* device in invalid state, needs reset */
bool use_started;
bool started;
- bool start_on_kick; /* virtio 1.0 transitional devices support that */
+ bool start_on_kick; /* when virtio 1.0 feature has not been negotiated */
VMChangeStateEntry *vmstate;
char *bus_name;
uint8_t device_endian;
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v3 3/5] virtio: Set "start_on_kick" on virtio_set_features()
2019-06-14 9:31 [Qemu-devel] [PATCH v3 0/5] virtio: fix some issues of "started" and "start_on_kick" flag elohimes
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 1/5] virtio: add "use-started" property elohimes
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 2/5] virtio: Set "start_on_kick" for legacy devices elohimes
@ 2019-06-14 9:31 ` elohimes
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 4/5] virtio: Make sure we get correct state of device on handle_aio_output() elohimes
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 5/5] virtio: Don't change "started" flag on virtio_vmstate_change() elohimes
4 siblings, 0 replies; 13+ messages in thread
From: elohimes @ 2019-06-14 9:31 UTC (permalink / raw)
To: mst, groug; +Cc: qemu-devel, Xie Yongji, dgilbert, ehabkost
From: Xie Yongji <xieyongji@baidu.com>
The guest feature is not set correctly on virtio_reset() and
virtio_init(). So we should not use it to set "start_on_kick" at that
point. This patch set "start_on_kick" on virtio_set_features() instead.
Fixes: badaf79cfdbd3 ("virtio: Introduce started flag to VirtioDevice")
Signed-off-by: Xie Yongji <xieyongji@baidu.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
hw/virtio/virtio.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 473881e9ec..034320d277 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1212,7 +1212,7 @@ void virtio_reset(void *opaque)
k->reset(vdev);
}
- vdev->start_on_kick = !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
+ vdev->start_on_kick = false;
vdev->started = false;
vdev->broken = false;
vdev->guest_features = 0;
@@ -2063,14 +2063,21 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
return -EINVAL;
}
ret = virtio_set_features_nocheck(vdev, val);
- if (!ret && virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
- /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */
- int i;
- for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
- if (vdev->vq[i].vring.num != 0) {
- virtio_init_region_cache(vdev, i);
+ if (!ret) {
+ if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
+ /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */
+ int i;
+ for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
+ if (vdev->vq[i].vring.num != 0) {
+ virtio_init_region_cache(vdev, i);
+ }
}
}
+
+ if (!virtio_device_started(vdev, vdev->status) &&
+ !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+ vdev->start_on_kick = true;
+ }
}
return ret;
}
@@ -2222,6 +2229,11 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
}
}
+ if (!virtio_device_started(vdev, vdev->status) &&
+ !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+ vdev->start_on_kick = true;
+ }
+
rcu_read_lock();
for (i = 0; i < num; i++) {
if (vdev->vq[i].vring.desc) {
@@ -2324,7 +2336,7 @@ void virtio_init(VirtIODevice *vdev, const char *name,
g_malloc0(sizeof(*vdev->vector_queues) * nvectors);
}
- vdev->start_on_kick = !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
+ vdev->start_on_kick = false;
vdev->started = false;
vdev->device_id = device_id;
vdev->status = 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v3 4/5] virtio: Make sure we get correct state of device on handle_aio_output()
2019-06-14 9:31 [Qemu-devel] [PATCH v3 0/5] virtio: fix some issues of "started" and "start_on_kick" flag elohimes
` (2 preceding siblings ...)
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 3/5] virtio: Set "start_on_kick" on virtio_set_features() elohimes
@ 2019-06-14 9:31 ` elohimes
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 5/5] virtio: Don't change "started" flag on virtio_vmstate_change() elohimes
4 siblings, 0 replies; 13+ messages in thread
From: elohimes @ 2019-06-14 9:31 UTC (permalink / raw)
To: mst, groug; +Cc: qemu-devel, Xie Yongji, dgilbert, ehabkost
From: Xie Yongji <xieyongji@baidu.com>
We should set the flags: "start_on_kick" and "started" after we call
the kick functions (handle_aio_output() and handle_output()).
Signed-off-by: Xie Yongji <xieyongji@baidu.com>
---
hw/virtio/virtio.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 034320d277..b4301f9e02 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1571,10 +1571,10 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
event_notifier_set(&vq->host_notifier);
} else if (vq->handle_output) {
vq->handle_output(vdev, vq);
- }
- if (unlikely(vdev->start_on_kick)) {
- virtio_set_started(vdev, true);
+ if (unlikely(vdev->start_on_kick)) {
+ virtio_set_started(vdev, true);
+ }
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v3 5/5] virtio: Don't change "started" flag on virtio_vmstate_change()
2019-06-14 9:31 [Qemu-devel] [PATCH v3 0/5] virtio: fix some issues of "started" and "start_on_kick" flag elohimes
` (3 preceding siblings ...)
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 4/5] virtio: Make sure we get correct state of device on handle_aio_output() elohimes
@ 2019-06-14 9:31 ` elohimes
4 siblings, 0 replies; 13+ messages in thread
From: elohimes @ 2019-06-14 9:31 UTC (permalink / raw)
To: mst, groug; +Cc: qemu-devel, Xie Yongji, dgilbert, ehabkost
From: Xie Yongji <xieyongji@baidu.com>
We will call virtio_set_status() on virtio_vmstate_change().
The "started" flag should not be changed in this case. Otherwise,
we may get an incorrect value when we set "started" flag but
not set DRIVER_OK in source VM.
Signed-off-by: Xie Yongji <xieyongji@baidu.com>
---
hw/virtio/virtio.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index b4301f9e02..9af2e339af 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1163,7 +1163,10 @@ int virtio_set_status(VirtIODevice *vdev, uint8_t val)
}
}
- virtio_set_started(vdev, val & VIRTIO_CONFIG_S_DRIVER_OK);
+ if ((vdev->status & VIRTIO_CONFIG_S_DRIVER_OK) !=
+ (val & VIRTIO_CONFIG_S_DRIVER_OK)) {
+ virtio_set_started(vdev, val & VIRTIO_CONFIG_S_DRIVER_OK);
+ }
if (k->set_status) {
k->set_status(vdev, val);
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/5] virtio: add "use-started" property
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 1/5] virtio: add "use-started" property elohimes
@ 2019-06-14 11:44 ` Greg Kurz
2019-06-17 2:14 ` Yongji Xie
0 siblings, 1 reply; 13+ messages in thread
From: Greg Kurz @ 2019-06-14 11:44 UTC (permalink / raw)
To: elohimes; +Cc: ehabkost, mst, qemu-devel, dgilbert, Xie Yongji
On Fri, 14 Jun 2019 17:31:17 +0800
elohimes@gmail.com wrote:
> From: Xie Yongji <xieyongji@baidu.com>
>
> In order to avoid migration issues, we introduce a "use-started"
> property to the base virtio device to indicate whether use
> "started" flag or not. This property will be true by default and
> set to false when machine type <= 4.0.1.
>
> Suggested-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> ---
> hw/block/vhost-user-blk.c | 4 ++--
> hw/core/machine.c | 8 ++++++--
This patch conflicts with latest upstream changes to hw_compat_4_0_1[].
It seems you need to rebase. Also, I'm still not sure how we're supposed
to handle hw_compat_4_0_1[] versus hw_compat_4_0[]... nobody commented
on:
https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00637.html
https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00641.html
Maybe worth to sort that out before re-posting.
> hw/virtio/virtio.c | 18 +++++++-----------
> include/hw/virtio/virtio.h | 21 +++++++++++++++++++++
> 4 files changed, 36 insertions(+), 15 deletions(-)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 9cb61336a6..85bc4017e7 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -191,7 +191,7 @@ static void vhost_user_blk_stop(VirtIODevice *vdev)
> static void vhost_user_blk_set_status(VirtIODevice *vdev, uint8_t status)
> {
> VHostUserBlk *s = VHOST_USER_BLK(vdev);
> - bool should_start = vdev->started;
> + bool should_start = virtio_device_started(vdev, status);
> int ret;
>
> if (!vdev->vm_running) {
> @@ -317,7 +317,7 @@ static int vhost_user_blk_connect(DeviceState *dev)
> }
>
> /* restore vhost state */
> - if (vdev->started) {
> + if (virtio_device_started(vdev, vdev->status)) {
> ret = vhost_user_blk_start(vdev);
> if (ret < 0) {
> error_report("vhost-user-blk: vhost start failed: %s",
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index f1a0f45f9c..60d1e27d2c 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -24,10 +24,14 @@
> #include "hw/pci/pci.h"
> #include "hw/mem/nvdimm.h"
>
> -GlobalProperty hw_compat_4_0_1[] = {};
> +GlobalProperty hw_compat_4_0_1[] = {
> + { "virtio-device", "use-started", "false" },
> +};
> const size_t hw_compat_4_0_1_len = G_N_ELEMENTS(hw_compat_4_0_1);
>
> -GlobalProperty hw_compat_4_0[] = {};
> +GlobalProperty hw_compat_4_0[] = {
> + { "virtio-device", "use-started", "false" },
> +};
> const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
>
> GlobalProperty hw_compat_3_1[] = {
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 07f4a64b48..19062fbb96 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1162,10 +1162,8 @@ int virtio_set_status(VirtIODevice *vdev, uint8_t val)
> }
> }
> }
> - vdev->started = val & VIRTIO_CONFIG_S_DRIVER_OK;
> - if (unlikely(vdev->start_on_kick && vdev->started)) {
> - vdev->start_on_kick = false;
> - }
> +
> + virtio_set_started(vdev, val & VIRTIO_CONFIG_S_DRIVER_OK);
>
> if (k->set_status) {
> k->set_status(vdev, val);
> @@ -1536,8 +1534,7 @@ static bool virtio_queue_notify_aio_vq(VirtQueue *vq)
> ret = vq->handle_aio_output(vdev, vq);
>
> if (unlikely(vdev->start_on_kick)) {
> - vdev->started = true;
> - vdev->start_on_kick = false;
> + virtio_set_started(vdev, true);
> }
> }
>
> @@ -1557,8 +1554,7 @@ static void virtio_queue_notify_vq(VirtQueue *vq)
> vq->handle_output(vdev, vq);
>
> if (unlikely(vdev->start_on_kick)) {
> - vdev->started = true;
> - vdev->start_on_kick = false;
> + virtio_set_started(vdev, true);
> }
> }
> }
> @@ -1579,8 +1575,7 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
> }
>
> if (unlikely(vdev->start_on_kick)) {
> - vdev->started = true;
> - vdev->start_on_kick = false;
> + virtio_set_started(vdev, true);
> }
> }
>
> @@ -2291,7 +2286,7 @@ static void virtio_vmstate_change(void *opaque, int running, RunState state)
> VirtIODevice *vdev = opaque;
> BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> - bool backend_run = running && vdev->started;
> + bool backend_run = running && virtio_device_started(vdev, vdev->status);
> vdev->vm_running = running;
>
> if (backend_run) {
> @@ -2669,6 +2664,7 @@ static void virtio_device_instance_finalize(Object *obj)
>
> static Property virtio_properties[] = {
> DEFINE_VIRTIO_COMMON_FEATURES(VirtIODevice, host_features),
> + DEFINE_PROP_BOOL("use-started", VirtIODevice, use_started, true),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 27c0efc3d0..15d5366939 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -105,6 +105,7 @@ struct VirtIODevice
> uint16_t device_id;
> bool vm_running;
> bool broken; /* device in invalid state, needs reset */
> + bool use_started;
> bool started;
> bool start_on_kick; /* virtio 1.0 transitional devices support that */
> VMChangeStateEntry *vmstate;
> @@ -351,4 +352,24 @@ static inline bool virtio_is_big_endian(VirtIODevice *vdev)
> /* Devices conforming to VIRTIO 1.0 or later are always LE. */
> return false;
> }
> +
> +static inline bool virtio_device_started(VirtIODevice *vdev, uint8_t status)
> +{
> + if (vdev->use_started) {
> + return vdev->started;
> + }
> +
> + return status & VIRTIO_CONFIG_S_DRIVER_OK;
> +}
> +
> +static inline void virtio_set_started(VirtIODevice *vdev, bool started)
> +{
> + if (started) {
> + vdev->start_on_kick = false;
> + }
> +
> + if (vdev->use_started) {
> + vdev->started = started;
> + }
> +}
> #endif
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/5] virtio: Set "start_on_kick" for legacy devices
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 2/5] virtio: Set "start_on_kick" for legacy devices elohimes
@ 2019-06-16 17:27 ` Greg Kurz
0 siblings, 0 replies; 13+ messages in thread
From: Greg Kurz @ 2019-06-16 17:27 UTC (permalink / raw)
To: elohimes; +Cc: ehabkost, mst, qemu-devel, dgilbert, Xie Yongji
On Fri, 14 Jun 2019 17:31:18 +0800
elohimes@gmail.com wrote:
> From: Xie Yongji <xieyongji@baidu.com>
>
> Besides virtio 1.0 transitional devices, we should also
> set "start_on_kick" flag for legacy devices (virtio 0.9).
>
> Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/virtio/virtio.c | 6 ++----
> include/hw/virtio/virtio.h | 2 +-
> 2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 19062fbb96..473881e9ec 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1212,8 +1212,7 @@ void virtio_reset(void *opaque)
> k->reset(vdev);
> }
>
> - vdev->start_on_kick = (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
> - !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1));
> + vdev->start_on_kick = !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
> vdev->started = false;
> vdev->broken = false;
> vdev->guest_features = 0;
> @@ -2325,8 +2324,7 @@ void virtio_init(VirtIODevice *vdev, const char *name,
> g_malloc0(sizeof(*vdev->vector_queues) * nvectors);
> }
>
> - vdev->start_on_kick = (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
> - !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1));
> + vdev->start_on_kick = !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
> vdev->started = false;
> vdev->device_id = device_id;
> vdev->status = 0;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 15d5366939..b189788cb2 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -107,7 +107,7 @@ struct VirtIODevice
> bool broken; /* device in invalid state, needs reset */
> bool use_started;
> bool started;
> - bool start_on_kick; /* virtio 1.0 transitional devices support that */
> + bool start_on_kick; /* when virtio 1.0 feature has not been negotiated */
> VMChangeStateEntry *vmstate;
> char *bus_name;
> uint8_t device_endian;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/5] virtio: add "use-started" property
2019-06-14 11:44 ` Greg Kurz
@ 2019-06-17 2:14 ` Yongji Xie
2019-06-17 5:20 ` Greg Kurz
0 siblings, 1 reply; 13+ messages in thread
From: Yongji Xie @ 2019-06-17 2:14 UTC (permalink / raw)
To: Greg Kurz, Alex Williamson, pbonzini
Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel,
Dr. David Alan Gilbert, Xie Yongji
On Fri, 14 Jun 2019 at 19:45, Greg Kurz <groug@kaod.org> wrote:
>
> On Fri, 14 Jun 2019 17:31:17 +0800
> elohimes@gmail.com wrote:
>
> > From: Xie Yongji <xieyongji@baidu.com>
> >
> > In order to avoid migration issues, we introduce a "use-started"
> > property to the base virtio device to indicate whether use
> > "started" flag or not. This property will be true by default and
> > set to false when machine type <= 4.0.1.
> >
> > Suggested-by: Greg Kurz <groug@kaod.org>
> > Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> > ---
> > hw/block/vhost-user-blk.c | 4 ++--
> > hw/core/machine.c | 8 ++++++--
>
> This patch conflicts with latest upstream changes to hw_compat_4_0_1[].
>
> It seems you need to rebase. Also, I'm still not sure how we're supposed
> to handle hw_compat_4_0_1[] versus hw_compat_4_0[]... nobody commented
> on:
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00637.html
> https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00641.html
>
> Maybe worth to sort that out before re-posting.
>
If hw_compat_4_0_1[] is introduced only for q35, I think this patch
should be OK. If not, maybe we should handle hw_compat_4_0_1[] in
other machine types (i440fx, arm, ppc, s390)?
Hi Alex and Paolo,
Any comment for this?
Thanks,
Yongji
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/5] virtio: add "use-started" property
2019-06-17 2:14 ` Yongji Xie
@ 2019-06-17 5:20 ` Greg Kurz
2019-06-17 6:04 ` Yongji Xie
0 siblings, 1 reply; 13+ messages in thread
From: Greg Kurz @ 2019-06-17 5:20 UTC (permalink / raw)
To: Yongji Xie
Cc: Eduardo Habkost, Michael S. Tsirkin, Dr. David Alan Gilbert,
qemu-devel, Alex Williamson, pbonzini, Xie Yongji
On Mon, 17 Jun 2019 10:14:30 +0800
Yongji Xie <elohimes@gmail.com> wrote:
> On Fri, 14 Jun 2019 at 19:45, Greg Kurz <groug@kaod.org> wrote:
> >
> > On Fri, 14 Jun 2019 17:31:17 +0800
> > elohimes@gmail.com wrote:
> >
> > > From: Xie Yongji <xieyongji@baidu.com>
> > >
> > > In order to avoid migration issues, we introduce a "use-started"
> > > property to the base virtio device to indicate whether use
> > > "started" flag or not. This property will be true by default and
> > > set to false when machine type <= 4.0.1.
> > >
> > > Suggested-by: Greg Kurz <groug@kaod.org>
> > > Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> > > ---
> > > hw/block/vhost-user-blk.c | 4 ++--
> > > hw/core/machine.c | 8 ++++++--
> >
> > This patch conflicts with latest upstream changes to hw_compat_4_0_1[].
> >
> > It seems you need to rebase. Also, I'm still not sure how we're supposed
> > to handle hw_compat_4_0_1[] versus hw_compat_4_0[]... nobody commented
> > on:
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00637.html
> > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00641.html
> >
> > Maybe worth to sort that out before re-posting.
> >
>
> If hw_compat_4_0_1[] is introduced only for q35, I think this patch
> should be OK. If not, maybe we should handle hw_compat_4_0_1[] in
> other machine types (i440fx, arm, ppc, s390)?
>
It turns out that hw_compat_4_0_1[] isn't needed at all. Please see:
https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg03054.html
> Hi Alex and Paolo,
>
> Any comment for this?
>
> Thanks,
> Yongji
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/5] virtio: add "use-started" property
2019-06-17 5:20 ` Greg Kurz
@ 2019-06-17 6:04 ` Yongji Xie
2019-06-22 15:51 ` Greg Kurz
0 siblings, 1 reply; 13+ messages in thread
From: Yongji Xie @ 2019-06-17 6:04 UTC (permalink / raw)
To: Greg Kurz
Cc: Eduardo Habkost, Michael S. Tsirkin, Dr. David Alan Gilbert,
qemu-devel, Alex Williamson, pbonzini, Xie Yongji
On Mon, 17 Jun 2019 at 13:24, Greg Kurz <groug@kaod.org> wrote:
>
> On Mon, 17 Jun 2019 10:14:30 +0800
> Yongji Xie <elohimes@gmail.com> wrote:
>
> > On Fri, 14 Jun 2019 at 19:45, Greg Kurz <groug@kaod.org> wrote:
> > >
> > > On Fri, 14 Jun 2019 17:31:17 +0800
> > > elohimes@gmail.com wrote:
> > >
> > > > From: Xie Yongji <xieyongji@baidu.com>
> > > >
> > > > In order to avoid migration issues, we introduce a "use-started"
> > > > property to the base virtio device to indicate whether use
> > > > "started" flag or not. This property will be true by default and
> > > > set to false when machine type <= 4.0.1.
> > > >
> > > > Suggested-by: Greg Kurz <groug@kaod.org>
> > > > Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> > > > ---
> > > > hw/block/vhost-user-blk.c | 4 ++--
> > > > hw/core/machine.c | 8 ++++++--
> > >
> > > This patch conflicts with latest upstream changes to hw_compat_4_0_1[].
> > >
> > > It seems you need to rebase. Also, I'm still not sure how we're supposed
> > > to handle hw_compat_4_0_1[] versus hw_compat_4_0[]... nobody commented
> > > on:
> > >
> > > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00637.html
> > > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00641.html
> > >
> > > Maybe worth to sort that out before re-posting.
> > >
> >
> > If hw_compat_4_0_1[] is introduced only for q35, I think this patch
> > should be OK. If not, maybe we should handle hw_compat_4_0_1[] in
> > other machine types (i440fx, arm, ppc, s390)?
> >
>
> It turns out that hw_compat_4_0_1[] isn't needed at all. Please see:
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg03054.html
>
Oh, great! I will rebase my patch after this commit is merged.
Thanks,
Yongji
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/5] virtio: add "use-started" property
2019-06-17 6:04 ` Yongji Xie
@ 2019-06-22 15:51 ` Greg Kurz
2019-06-24 9:44 ` Yongji Xie
0 siblings, 1 reply; 13+ messages in thread
From: Greg Kurz @ 2019-06-22 15:51 UTC (permalink / raw)
To: Yongji Xie
Cc: Eduardo Habkost, Michael S. Tsirkin, Dr. David Alan Gilbert,
qemu-devel, Alex Williamson, pbonzini, Xie Yongji
On Mon, 17 Jun 2019 14:04:10 +0800
Yongji Xie <elohimes@gmail.com> wrote:
> On Mon, 17 Jun 2019 at 13:24, Greg Kurz <groug@kaod.org> wrote:
> >
> > On Mon, 17 Jun 2019 10:14:30 +0800
> > Yongji Xie <elohimes@gmail.com> wrote:
> >
> > > On Fri, 14 Jun 2019 at 19:45, Greg Kurz <groug@kaod.org> wrote:
> > > >
> > > > On Fri, 14 Jun 2019 17:31:17 +0800
> > > > elohimes@gmail.com wrote:
> > > >
> > > > > From: Xie Yongji <xieyongji@baidu.com>
> > > > >
> > > > > In order to avoid migration issues, we introduce a "use-started"
> > > > > property to the base virtio device to indicate whether use
> > > > > "started" flag or not. This property will be true by default and
> > > > > set to false when machine type <= 4.0.1.
> > > > >
> > > > > Suggested-by: Greg Kurz <groug@kaod.org>
> > > > > Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> > > > > ---
> > > > > hw/block/vhost-user-blk.c | 4 ++--
> > > > > hw/core/machine.c | 8 ++++++--
> > > >
> > > > This patch conflicts with latest upstream changes to hw_compat_4_0_1[].
> > > >
> > > > It seems you need to rebase. Also, I'm still not sure how we're supposed
> > > > to handle hw_compat_4_0_1[] versus hw_compat_4_0[]... nobody commented
> > > > on:
> > > >
> > > > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00637.html
> > > > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00641.html
> > > >
> > > > Maybe worth to sort that out before re-posting.
> > > >
> > >
> > > If hw_compat_4_0_1[] is introduced only for q35, I think this patch
> > > should be OK. If not, maybe we should handle hw_compat_4_0_1[] in
> > > other machine types (i440fx, arm, ppc, s390)?
> > >
> >
> > It turns out that hw_compat_4_0_1[] isn't needed at all. Please see:
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg03054.html
> >
>
> Oh, great! I will rebase my patch after this commit is merged.
>
> Thanks,
> Yongji
You can proceed.
https://git.qemu.org/?p=qemu.git;a=commit;h=8e8cbed09ad9d577955691b4c061b61b602406d1
Cheers,
--
Greg
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/5] virtio: add "use-started" property
2019-06-22 15:51 ` Greg Kurz
@ 2019-06-24 9:44 ` Yongji Xie
0 siblings, 0 replies; 13+ messages in thread
From: Yongji Xie @ 2019-06-24 9:44 UTC (permalink / raw)
To: Greg Kurz
Cc: Eduardo Habkost, Michael S. Tsirkin, Dr. David Alan Gilbert,
qemu-devel, Alex Williamson, pbonzini, Xie Yongji
On Sat, 22 Jun 2019 at 23:51, Greg Kurz <groug@kaod.org> wrote:
>
> On Mon, 17 Jun 2019 14:04:10 +0800
> Yongji Xie <elohimes@gmail.com> wrote:
>
> > On Mon, 17 Jun 2019 at 13:24, Greg Kurz <groug@kaod.org> wrote:
> > >
> > > On Mon, 17 Jun 2019 10:14:30 +0800
> > > Yongji Xie <elohimes@gmail.com> wrote:
> > >
> > > > On Fri, 14 Jun 2019 at 19:45, Greg Kurz <groug@kaod.org> wrote:
> > > > >
> > > > > On Fri, 14 Jun 2019 17:31:17 +0800
> > > > > elohimes@gmail.com wrote:
> > > > >
> > > > > > From: Xie Yongji <xieyongji@baidu.com>
> > > > > >
> > > > > > In order to avoid migration issues, we introduce a "use-started"
> > > > > > property to the base virtio device to indicate whether use
> > > > > > "started" flag or not. This property will be true by default and
> > > > > > set to false when machine type <= 4.0.1.
> > > > > >
> > > > > > Suggested-by: Greg Kurz <groug@kaod.org>
> > > > > > Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> > > > > > ---
> > > > > > hw/block/vhost-user-blk.c | 4 ++--
> > > > > > hw/core/machine.c | 8 ++++++--
> > > > >
> > > > > This patch conflicts with latest upstream changes to hw_compat_4_0_1[].
> > > > >
> > > > > It seems you need to rebase. Also, I'm still not sure how we're supposed
> > > > > to handle hw_compat_4_0_1[] versus hw_compat_4_0[]... nobody commented
> > > > > on:
> > > > >
> > > > > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00637.html
> > > > > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00641.html
> > > > >
> > > > > Maybe worth to sort that out before re-posting.
> > > > >
> > > >
> > > > If hw_compat_4_0_1[] is introduced only for q35, I think this patch
> > > > should be OK. If not, maybe we should handle hw_compat_4_0_1[] in
> > > > other machine types (i440fx, arm, ppc, s390)?
> > > >
> > >
> > > It turns out that hw_compat_4_0_1[] isn't needed at all. Please see:
> > >
> > > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg03054.html
> > >
> >
> > Oh, great! I will rebase my patch after this commit is merged.
> >
> > Thanks,
> > Yongji
>
> You can proceed.
>
> https://git.qemu.org/?p=qemu.git;a=commit;h=8e8cbed09ad9d577955691b4c061b61b602406d1
>
OK, Thank you.
Thanks,
Yongji
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-06-24 9:45 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-14 9:31 [Qemu-devel] [PATCH v3 0/5] virtio: fix some issues of "started" and "start_on_kick" flag elohimes
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 1/5] virtio: add "use-started" property elohimes
2019-06-14 11:44 ` Greg Kurz
2019-06-17 2:14 ` Yongji Xie
2019-06-17 5:20 ` Greg Kurz
2019-06-17 6:04 ` Yongji Xie
2019-06-22 15:51 ` Greg Kurz
2019-06-24 9:44 ` Yongji Xie
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 2/5] virtio: Set "start_on_kick" for legacy devices elohimes
2019-06-16 17:27 ` Greg Kurz
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 3/5] virtio: Set "start_on_kick" on virtio_set_features() elohimes
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 4/5] virtio: Make sure we get correct state of device on handle_aio_output() elohimes
2019-06-14 9:31 ` [Qemu-devel] [PATCH v3 5/5] virtio: Don't change "started" flag on virtio_vmstate_change() elohimes
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.