| Message ID | 20190529070955.25565-2-xieyongji@baidu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio: fix some issues of "started" and "start_on_kick" flag | expand |
On Wed, 29 May 2019 15:09:51 +0800 elohimes@gmail.com wrote: > 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. > > Signed-off-by: Xie Yongji <xieyongji@baidu.com> > --- Maybe add a Fixes: badaf79cfdbd3 ? > hw/virtio/virtio.c | 25 +++++++++++++++---------- > 1 file changed, 15 insertions(+), 10 deletions(-) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 4805727b53..fc8fca81ad 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -1214,8 +1214,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 = false; > vdev->started = false; > vdev->broken = false; > vdev->guest_features = 0; > @@ -2069,14 +2068,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_host_has_feature(vdev, VIRTIO_F_VERSION_1) && > + !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + vdev->start_on_kick = true; > + } > } > return ret; > } > @@ -2331,8 +2337,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 = false; virtio_init() is called at realize on an object that was just allocated with g_malloc0(). You shouldn't need to set anything to 0 or false... or I'm missing something ? Anyway, I guess this doesn't hurt, so: Reviewed-by: Greg Kurz <groug@kaod.org> > vdev->started = false; > vdev->device_id = device_id; > vdev->status = 0;
On Tue, 4 Jun 2019 at 00:53, Greg Kurz <groug@kaod.org> wrote: > > On Wed, 29 May 2019 15:09:51 +0800 > elohimes@gmail.com wrote: > > > 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. > > > > Signed-off-by: Xie Yongji <xieyongji@baidu.com> > > --- > > Maybe add a Fixes: badaf79cfdbd3 ? > Will add it in v2. Thanks for review. Thanks, Yongji
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 4805727b53..fc8fca81ad 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -1214,8 +1214,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 = false; vdev->started = false; vdev->broken = false; vdev->guest_features = 0; @@ -2069,14 +2068,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_host_has_feature(vdev, VIRTIO_F_VERSION_1) && + !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { + vdev->start_on_kick = true; + } } return ret; } @@ -2331,8 +2337,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 = false; vdev->started = false; vdev->device_id = device_id; vdev->status = 0;