On Thu, Jun 09 2022, Paolo Bonzini <pbonz...@redhat.com> wrote:
> Make virtio_mmio_soft_reset reset the virtio device, which is performed by
> both the "soft" and the "hard" reset; and then call virtio_mmio_soft_reset
> from virtio_mmio_reset to emphasize that the latter is a superset of the
> former.
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> hw/virtio/virtio-mmio.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> index 6d81a26473..d240efef97 100644
> --- a/hw/virtio/virtio-mmio.c
> +++ b/hw/virtio/virtio-mmio.c
> @@ -72,12 +72,12 @@ static void virtio_mmio_soft_reset(VirtIOMMIOProxy *proxy)
> {
> int i;
>
> - if (proxy->legacy) {
> - return;
> - }
> + virtio_bus_reset(&proxy->bus);
>
> - for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
> - proxy->vqs[i].enabled = 0;
> + if (!proxy->legacy) {
> + for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
> + proxy->vqs[i].enabled = 0;
> + }
> }
> }
The more I look at this, the more confused I get.
The current code calls soft_reset when the driver sets the status to 0,
after already having called virtio_reset(). But doesn't virtio_reset()
ultimately already trigger the virtio-mmio reset routine, which sets
enabled to 0 for all queues? Why do that again? (And why is soft_reset a
"soft reset"?)
Maybe I'm missing something obvious, or it is simply -ENOCOFFEE on my side.
>
> @@ -376,7 +376,7 @@ static void virtio_mmio_write(void *opaque, hwaddr
> offset, uint64_t value,
> return;
> }
> if (value == 0) {
> - virtio_bus_reset(&vdev->bus);
> + virtio_mmio_soft_reset(proxy);
> } else {
> virtio_queue_set_addr(vdev, vdev->queue_sel,
> value << proxy->guest_page_shift);
> @@ -432,7 +432,6 @@ static void virtio_mmio_write(void *opaque, hwaddr
> offset, uint64_t value,
> }
>
> if (vdev->status == 0) {
> - virtio_reset(vdev);
> virtio_mmio_soft_reset(proxy);
> }
> break;
> @@ -627,7 +626,8 @@ static void virtio_mmio_reset(DeviceState *d)
> VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
> int i;
>
> - virtio_bus_reset(&proxy->bus);
> + virtio_mmio_soft_reset(proxy);
> +
> proxy->host_features_sel = 0;
> proxy->guest_features_sel = 0;
> proxy->guest_page_shift = 0;
> @@ -636,7 +636,6 @@ static void virtio_mmio_reset(DeviceState *d)
> proxy->guest_features[0] = proxy->guest_features[1] = 0;
>
> for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
> - proxy->vqs[i].enabled = 0;
> proxy->vqs[i].num = 0;
> proxy->vqs[i].desc[0] = proxy->vqs[i].desc[1] = 0;
> proxy->vqs[i].avail[0] = proxy->vqs[i].avail[1] = 0;
