* Re: [PATCH] virtio: Decrement avail idx on buffer detach
[not found] <a2fe5813ccecf1e762670702cba8ccb2a4cb8cfc.1300282928.git.amit.shah@redhat.com>
@ 2011-03-17 4:56 ` Rusty Russell
2011-03-17 12:26 ` Amit Shah
0 siblings, 1 reply; 6+ messages in thread
From: Rusty Russell @ 2011-03-17 4:56 UTC (permalink / raw)
To: Virtualization List; +Cc: Amit Shah, stable
On Wed, 16 Mar 2011 19:12:10 +0530, Amit Shah <amit.shah@redhat.com> wrote:
> When detaching a buffer from a vq, the avail.idx value should be
> decremented as well.
>
> This was noticed by hot-unplugging a virtio console port and then
> plugging in a new one on the same number (re-using the vqs which were
> just 'disowned'). qemu reported
>
> 'Guest moved used index from 0 to 256'
>
> when any IO was attempted on the new port.
Yech... detach_unused_buf cannot be used on a live virtqueue; it assumes
we will reset the vq (usually by resetting the entire device).
You've partially violated that assumption by reusing the vq after
calling detach_unused_buf. So I'm not entirely sure this is the only
bug lurking; safer would be to re-initialize the vq somehow when you
plug back in...
(Though this patch is minimal, and may be better -stable material).
Thanks,
Rusty.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] virtio: Decrement avail idx on buffer detach
2011-03-17 4:56 ` [PATCH] virtio: Decrement avail idx on buffer detach Rusty Russell
@ 2011-03-17 12:26 ` Amit Shah
2011-03-28 14:27 ` Amit Shah
0 siblings, 1 reply; 6+ messages in thread
From: Amit Shah @ 2011-03-17 12:26 UTC (permalink / raw)
To: Rusty Russell; +Cc: Virtualization List
On (Thu) 17 Mar 2011 [15:26:28], Rusty Russell wrote:
> On Wed, 16 Mar 2011 19:12:10 +0530, Amit Shah <amit.shah@redhat.com> wrote:
> > When detaching a buffer from a vq, the avail.idx value should be
> > decremented as well.
> >
> > This was noticed by hot-unplugging a virtio console port and then
> > plugging in a new one on the same number (re-using the vqs which were
> > just 'disowned'). qemu reported
> >
> > 'Guest moved used index from 0 to 256'
> >
> > when any IO was attempted on the new port.
>
> Yech... detach_unused_buf cannot be used on a live virtqueue; it assumes
> we will reset the vq (usually by resetting the entire device).
>
> You've partially violated that assumption by reusing the vq after
> calling detach_unused_buf. So I'm not entirely sure this is the only
> bug lurking; safer would be to re-initialize the vq somehow when you
> plug back in...
Right; and then that will need host changes too (re-init the vqs on
the host side), which then gets us into compat problems...
Amit
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] virtio: Decrement avail idx on buffer detach
2011-03-17 12:26 ` Amit Shah
@ 2011-03-28 14:27 ` Amit Shah
2011-04-04 6:34 ` Rusty Russell
0 siblings, 1 reply; 6+ messages in thread
From: Amit Shah @ 2011-03-28 14:27 UTC (permalink / raw)
To: Rusty Russell; +Cc: Virtualization List
On (Thu) 17 Mar 2011 [17:56:59], Amit Shah wrote:
> On (Thu) 17 Mar 2011 [15:26:28], Rusty Russell wrote:
> > On Wed, 16 Mar 2011 19:12:10 +0530, Amit Shah <amit.shah@redhat.com> wrote:
> > > When detaching a buffer from a vq, the avail.idx value should be
> > > decremented as well.
> > >
> > > This was noticed by hot-unplugging a virtio console port and then
> > > plugging in a new one on the same number (re-using the vqs which were
> > > just 'disowned'). qemu reported
> > >
> > > 'Guest moved used index from 0 to 256'
> > >
> > > when any IO was attempted on the new port.
> >
> > Yech... detach_unused_buf cannot be used on a live virtqueue; it assumes
> > we will reset the vq (usually by resetting the entire device).
> >
> > You've partially violated that assumption by reusing the vq after
> > calling detach_unused_buf. So I'm not entirely sure this is the only
> > bug lurking; safer would be to re-initialize the vq somehow when you
> > plug back in...
>
> Right; and then that will need host changes too (re-init the vqs on
> the host side), which then gets us into compat problems...
Rusty, any thoughts on this?
Amit
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] virtio: Decrement avail idx on buffer detach
2011-03-28 14:27 ` Amit Shah
@ 2011-04-04 6:34 ` Rusty Russell
2011-04-04 15:01 ` Amit Shah
0 siblings, 1 reply; 6+ messages in thread
From: Rusty Russell @ 2011-04-04 6:34 UTC (permalink / raw)
To: Amit Shah; +Cc: Virtualization List
On Mon, 28 Mar 2011 19:57:06 +0530, Amit Shah <amit.shah@redhat.com> wrote:
> On (Thu) 17 Mar 2011 [17:56:59], Amit Shah wrote:
> > On (Thu) 17 Mar 2011 [15:26:28], Rusty Russell wrote:
> > > On Wed, 16 Mar 2011 19:12:10 +0530, Amit Shah <amit.shah@redhat.com> wrote:
> > > > When detaching a buffer from a vq, the avail.idx value should be
> > > > decremented as well.
> > > >
> > > > This was noticed by hot-unplugging a virtio console port and then
> > > > plugging in a new one on the same number (re-using the vqs which were
> > > > just 'disowned'). qemu reported
> > > >
> > > > 'Guest moved used index from 0 to 256'
> > > >
> > > > when any IO was attempted on the new port.
> > >
> > > Yech... detach_unused_buf cannot be used on a live virtqueue; it assumes
> > > we will reset the vq (usually by resetting the entire device).
> > >
> > > You've partially violated that assumption by reusing the vq after
> > > calling detach_unused_buf. So I'm not entirely sure this is the only
> > > bug lurking; safer would be to re-initialize the vq somehow when you
> > > plug back in...
> >
> > Right; and then that will need host changes too (re-init the vqs on
> > the host side), which then gets us into compat problems...
>
> Rusty, any thoughts on this?
>
> Amit
>
Yes... I've applied your patch for the moment, and will send to Linus
with cc' stable.
Thanks,
Rusty.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] virtio: Decrement avail idx on buffer detach
2011-04-04 6:34 ` Rusty Russell
@ 2011-04-04 15:01 ` Amit Shah
0 siblings, 0 replies; 6+ messages in thread
From: Amit Shah @ 2011-04-04 15:01 UTC (permalink / raw)
To: Rusty Russell; +Cc: Virtualization List
On (Mon) 04 Apr 2011 [16:04:40], Rusty Russell wrote:
> On Mon, 28 Mar 2011 19:57:06 +0530, Amit Shah <amit.shah@redhat.com> wrote:
> > On (Thu) 17 Mar 2011 [17:56:59], Amit Shah wrote:
> > > On (Thu) 17 Mar 2011 [15:26:28], Rusty Russell wrote:
> > > > On Wed, 16 Mar 2011 19:12:10 +0530, Amit Shah <amit.shah@redhat.com> wrote:
> > > > > When detaching a buffer from a vq, the avail.idx value should be
> > > > > decremented as well.
> > > > >
> > > > > This was noticed by hot-unplugging a virtio console port and then
> > > > > plugging in a new one on the same number (re-using the vqs which were
> > > > > just 'disowned'). qemu reported
> > > > >
> > > > > 'Guest moved used index from 0 to 256'
> > > > >
> > > > > when any IO was attempted on the new port.
> > > >
> > > > Yech... detach_unused_buf cannot be used on a live virtqueue; it assumes
> > > > we will reset the vq (usually by resetting the entire device).
> > > >
> > > > You've partially violated that assumption by reusing the vq after
> > > > calling detach_unused_buf. So I'm not entirely sure this is the only
> > > > bug lurking; safer would be to re-initialize the vq somehow when you
> > > > plug back in...
> > >
> > > Right; and then that will need host changes too (re-init the vqs on
> > > the host side), which then gets us into compat problems...
> >
> > Rusty, any thoughts on this?
>
> Yes... I've applied your patch for the moment, and will send to Linus
> with cc' stable.
OK, thanks!
Amit
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] virtio: Decrement avail idx on buffer detach
@ 2011-03-16 13:42 Amit Shah
0 siblings, 0 replies; 6+ messages in thread
From: Amit Shah @ 2011-03-16 13:42 UTC (permalink / raw)
To: Virtualization List; +Cc: Amit Shah, stable
When detaching a buffer from a vq, the avail.idx value should be
decremented as well.
This was noticed by hot-unplugging a virtio console port and then
plugging in a new one on the same number (re-using the vqs which were
just 'disowned'). qemu reported
'Guest moved used index from 0 to 256'
when any IO was attempted on the new port.
CC: stable@kernel.org
Reported-by: juzhang <juzhang@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/virtio/virtio_ring.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cc2f73e..b0043fb 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -371,6 +371,7 @@ void *virtqueue_detach_unused_buf(struct virtqueue *_vq)
/* detach_buf clears data, so grab it now. */
buf = vq->data[i];
detach_buf(vq, i);
+ vq->vring.avail->idx--;
END_USE(vq);
return buf;
}
--
1.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-04-04 15:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <a2fe5813ccecf1e762670702cba8ccb2a4cb8cfc.1300282928.git.amit.shah@redhat.com>
2011-03-17 4:56 ` [PATCH] virtio: Decrement avail idx on buffer detach Rusty Russell
2011-03-17 12:26 ` Amit Shah
2011-03-28 14:27 ` Amit Shah
2011-04-04 6:34 ` Rusty Russell
2011-04-04 15:01 ` Amit Shah
2011-03-16 13:42 Amit Shah
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).