Re: [Qemu-devel] [PATCH 4/7] [s390] reset avail and used index on reboot

[Jens Freimann]

On Thu, Apr 26, 2012 at 03:16:00PM +0200, Alexander Graf wrote:
> 
> On 23.04.2012, at 11:52, Christian Borntraeger wrote:
> 
> > From: Jens Freimann <address@hidden>
> > 
> > reset the guest vring avail/used idx fields, otherwise it's possible
> > that old values remain in memory which would cause a reboot to fail
> > with a "Guest moved used index" message
> > 
> > Signed-off-by: Jens Freimann <address@hidden>
> > Signed-off-by: Christian Borntraeger <address@hidden>
> > ---
> > hw/s390-virtio-bus.c |    7 +++++++
> > hw/s390-virtio-bus.h |    2 ++
> > 2 files changed, 9 insertions(+)
> > 
> > diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
> > index 74419b3..084bac1 100644
> > --- a/hw/s390-virtio-bus.c
> > +++ b/hw/s390-virtio-bus.c
> > @@ -225,6 +225,7 @@ void s390_virtio_device_sync(VirtIOS390Device *dev)
> > {
> >     VirtIOS390Bus *bus = DO_UPCAST(VirtIOS390Bus, bus, 
> > dev->qdev.parent_bus);
> >     ram_addr_t cur_offs;
> > +    target_phys_addr_t idx_addr;
> >     uint8_t num_vq;
> >     int i;
> > 
> > @@ -250,6 +251,12 @@ void s390_virtio_device_sync(VirtIOS390Device *dev)
> >         vring = s390_virtio_next_ring(bus);
> >         virtio_queue_set_addr(dev->vdev, i, vring);
> >         virtio_queue_set_vector(dev->vdev, i, i);
> > +        idx_addr = virtio_queue_get_avail_addr(dev->vdev, i) +
> > +                     VIRTIO_VRING_AVAIL_IDX_OFFS;
> > +        stw_phys(idx_addr, 0);
> > +        idx_addr = virtio_queue_get_used_addr(dev->vdev, i) +
> > +                     VIRTIO_VRING_USED_IDX_OFFS;
> > +        stw_phys(idx_addr, 0);
> 
> Are you sure this is correct to do in the sync callback? The idea of "sync" 
> was to have a callback that can be called every time config information 
> changes.
> 
> That could even be something as simple as a byte changing in the virtio 
> device specific config space. But because we don't get callbacks when the 
> guest accesses them, we need to synchronize it with real memory.
 
It seemed to be the best place to do this, as right now it's called only in 
device init and the VIRTIO_RESET hypercall. If you have plans to change this
I guess we could do it in a reset handler as well. 

regards,
Jens