* [PATCH v2 1/4] kvm tools: Use virt_queue__trigger_irq() to trigger IRQ for virtio console
2011-05-08 13:09 [PATCH v2 0/4] Resend this patch series Asias He
@ 2011-05-08 13:09 ` Asias He
2011-05-08 13:09 ` [PATCH v2 2/4] kvm tools: Use virt_queue__trigger_irq() to trigger IRQ for virtio blk Asias He
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Asias He @ 2011-05-08 13:09 UTC (permalink / raw)
To: Pekka Enberg, Cyrill Gorcunov, Ingo Molnar, Sasha Levin, Prasad Joshi
Cc: kvm, Asias He
This patch uses IRQ injection mechanism introduced by
virt_queue__trigger_irq() which respect virtio IRQ status
and VRING_AVAIL_F_NO_INTERRUPT.
Signed-off-by: Asias He <asias.hejun@gmail.com>
---
tools/kvm/virtio/console.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/tools/kvm/virtio/console.c b/tools/kvm/virtio/console.c
index 188227b..f5449ba 100644
--- a/tools/kvm/virtio/console.c
+++ b/tools/kvm/virtio/console.c
@@ -49,6 +49,7 @@ struct con_dev {
u32 guest_features;
u16 config_vector;
u8 status;
+ u8 isr;
u16 queue_selector;
void *jobs[VIRTIO_CONSOLE_NUM_QUEUES];
@@ -85,7 +86,7 @@ static void virtio_console__inject_interrupt_callback(struct kvm *self, void *pa
head = virt_queue__get_iov(vq, iov, &out, &in, self);
len = term_getc_iov(CONSOLE_VIRTIO, iov, in);
virt_queue__set_used_elem(vq, head, len);
- kvm__irq_line(self, virtio_console_pci_device.irq_line, 1);
+ virt_queue__trigger_irq(vq, virtio_console_pci_device.irq_line, &cdev.isr, self);
}
mutex_unlock(&cdev.mutex);
@@ -139,8 +140,9 @@ static bool virtio_console_pci_io_in(struct kvm *self, u16 port, void *data, int
ioport__write8(data, cdev.status);
break;
case VIRTIO_PCI_ISR:
- ioport__write8(data, 0x1);
- kvm__irq_line(self, virtio_console_pci_device.irq_line, 0);
+ ioport__write8(data, cdev.isr);
+ kvm__irq_line(self, virtio_console_pci_device.irq_line, VIRTIO_IRQ_LOW);
+ cdev.isr = VIRTIO_IRQ_LOW;
break;
case VIRTIO_MSI_CONFIG_VECTOR:
ioport__write16(data, cdev.config_vector);
@@ -170,7 +172,7 @@ static void virtio_console_handle_callback(struct kvm *self, void *param)
virt_queue__set_used_elem(vq, head, len);
}
- kvm__irq_line(self, virtio_console_pci_device.irq_line, 1);
+ virt_queue__trigger_irq(vq, virtio_console_pci_device.irq_line, &cdev.isr, self);
}
static bool virtio_console_pci_io_out(struct kvm *self, u16 port, void *data, int size, u32 count)
--
1.7.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 2/4] kvm tools: Use virt_queue__trigger_irq() to trigger IRQ for virtio blk
2011-05-08 13:09 [PATCH v2 0/4] Resend this patch series Asias He
2011-05-08 13:09 ` [PATCH v2 1/4] kvm tools: Use virt_queue__trigger_irq() to trigger IRQ for virtio console Asias He
@ 2011-05-08 13:09 ` Asias He
2011-05-08 13:09 ` [PATCH v2 3/4] kvm tools: Use virt_queue__trigger_irq() to trigger IRQ for virtio rng Asias He
2011-05-08 13:09 ` [PATCH v2 4/4] kvm tools: Fix virtio console hangs by removing IRQ injection for tx path Asias He
3 siblings, 0 replies; 12+ messages in thread
From: Asias He @ 2011-05-08 13:09 UTC (permalink / raw)
To: Pekka Enberg, Cyrill Gorcunov, Ingo Molnar, Sasha Levin, Prasad Joshi
Cc: kvm, Asias He
This patch uses IRQ injection mechanism introduced by
virt_queue__trigger_irq() which respect virtio IRQ status
and VRING_AVAIL_F_NO_INTERRUPT.
Signed-off-by: Asias He <asias.hejun@gmail.com>
---
tools/kvm/virtio/blk.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
index cc3dc78..12c7029 100644
--- a/tools/kvm/virtio/blk.c
+++ b/tools/kvm/virtio/blk.c
@@ -38,6 +38,7 @@ struct blk_dev {
u32 guest_features;
u16 config_vector;
u8 status;
+ u8 isr;
u8 idx;
/* virtio queue */
@@ -102,8 +103,9 @@ static bool virtio_blk_pci_io_in(struct kvm *self, u16 port, void *data, int siz
ioport__write8(data, bdev->status);
break;
case VIRTIO_PCI_ISR:
- ioport__write8(data, 0x1);
- kvm__irq_line(self, bdev->pci_hdr.irq_line, 0);
+ ioport__write8(data, bdev->isr);
+ kvm__irq_line(self, bdev->pci_hdr.irq_line, VIRTIO_IRQ_LOW);
+ bdev->isr = VIRTIO_IRQ_LOW;
break;
case VIRTIO_MSI_CONFIG_VECTOR:
ioport__write16(data, bdev->config_vector);
@@ -167,7 +169,7 @@ static void virtio_blk_do_io(struct kvm *kvm, void *param)
while (virt_queue__available(vq))
virtio_blk_do_io_request(kvm, bdev, vq);
- kvm__irq_line(kvm, bdev->pci_hdr.irq_line, 1);
+ virt_queue__trigger_irq(vq, bdev->pci_hdr.irq_line, &bdev->isr, kvm);
}
static bool virtio_blk_pci_io_out(struct kvm *self, u16 port, void *data, int size, u32 count)
--
1.7.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 3/4] kvm tools: Use virt_queue__trigger_irq() to trigger IRQ for virtio rng
2011-05-08 13:09 [PATCH v2 0/4] Resend this patch series Asias He
2011-05-08 13:09 ` [PATCH v2 1/4] kvm tools: Use virt_queue__trigger_irq() to trigger IRQ for virtio console Asias He
2011-05-08 13:09 ` [PATCH v2 2/4] kvm tools: Use virt_queue__trigger_irq() to trigger IRQ for virtio blk Asias He
@ 2011-05-08 13:09 ` Asias He
2011-05-08 13:09 ` [PATCH v2 4/4] kvm tools: Fix virtio console hangs by removing IRQ injection for tx path Asias He
3 siblings, 0 replies; 12+ messages in thread
From: Asias He @ 2011-05-08 13:09 UTC (permalink / raw)
To: Pekka Enberg, Cyrill Gorcunov, Ingo Molnar, Sasha Levin, Prasad Joshi
Cc: kvm, Asias He
This patch uses IRQ injection mechanism introduced by
virt_queue__trigger_irq() which respect virtio IRQ status
and VRING_AVAIL_F_NO_INTERRUPT.
Signed-off-by: Asias He <asias.hejun@gmail.com>
---
tools/kvm/virtio/rng.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/kvm/virtio/rng.c b/tools/kvm/virtio/rng.c
index d355cf8..f692dfd 100644
--- a/tools/kvm/virtio/rng.c
+++ b/tools/kvm/virtio/rng.c
@@ -37,6 +37,7 @@ static struct pci_device_header virtio_rng_pci_device = {
struct rng_dev {
u8 status;
+ u8 isr;
u16 config_vector;
int fd;
@@ -72,8 +73,9 @@ static bool virtio_rng_pci_io_in(struct kvm *kvm, u16 port, void *data, int size
ioport__write8(data, rdev.status);
break;
case VIRTIO_PCI_ISR:
- ioport__write8(data, 0x1);
- kvm__irq_line(kvm, virtio_rng_pci_device.irq_line, 0);
+ ioport__write8(data, rdev.isr);
+ kvm__irq_line(kvm, virtio_rng_pci_device.irq_line, VIRTIO_IRQ_LOW);
+ rdev.isr = VIRTIO_IRQ_LOW;
break;
case VIRTIO_MSI_CONFIG_VECTOR:
ioport__write16(data, rdev.config_vector);
@@ -106,7 +108,7 @@ static void virtio_rng_do_io(struct kvm *kvm, void *param)
while (virt_queue__available(vq)) {
virtio_rng_do_io_request(kvm, vq);
- kvm__irq_line(kvm, virtio_rng_pci_device.irq_line, 1);
+ virt_queue__trigger_irq(vq, virtio_rng_pci_device.irq_line, &rdev.isr, kvm);
}
}
--
1.7.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 4/4] kvm tools: Fix virtio console hangs by removing IRQ injection for tx path
2011-05-08 13:09 [PATCH v2 0/4] Resend this patch series Asias He
` (2 preceding siblings ...)
2011-05-08 13:09 ` [PATCH v2 3/4] kvm tools: Use virt_queue__trigger_irq() to trigger IRQ for virtio rng Asias He
@ 2011-05-08 13:09 ` Asias He
2011-05-08 17:05 ` Pekka Enberg
3 siblings, 1 reply; 12+ messages in thread
From: Asias He @ 2011-05-08 13:09 UTC (permalink / raw)
To: Pekka Enberg, Cyrill Gorcunov, Ingo Molnar, Sasha Levin, Prasad Joshi
Cc: kvm, Asias He
As virtio spec says:
"""
Because this is high importance and low bandwidth, the current Linux
implementation polls for the buffer to be used, rather than waiting
for an interrupt, simplifying the implementation signicantly.
"""
drivers/char/virtio_console.c
send_buf() {
...
/* Tell Host to go! */
virtqueue_kick(out_vq);
...
while (!virtqueue_get_buf(out_vq, &len))
cpu_relax();
...
}
The console hangs can simply be reproduced by yes command which
gives tremendous console IOs and IRQs.
[ 16.786440] irq 4: nobody cared (try booting with the "irqpoll" option)
[ 16.786440] Pid: 1437, comm: yes Tainted: G W 2.6.39-rc6+ #56
[ 16.786440] Call Trace:
[ 16.786440] [<c16578eb>] __report_bad_irq+0x30/0x89
[ 16.786440] [<c10980e6>] note_interrupt+0x118/0x17a
[ 16.786440] [<c1096e7d>] handle_irq_event_percpu+0x168/0x179
[ 16.786440] [<c1096eba>] handle_irq_event+0x2c/0x46
[ 16.786440] [<c1098516>] ? unmask_irq+0x1e/0x1e
[ 16.786440] [<c1098566>] handle_level_irq+0x50/0x6e
[ 16.786440] <IRQ> [<c102fa69>] ? do_IRQ+0x35/0x7f
[ 16.786440] [<c1665ea9>] ? common_interrupt+0x29/0x30
[ 16.786440] [<c16610d6>] ? _raw_spin_unlock_irqrestore+0x7/0x28
[ 16.786440] [<c1364f65>] ? hvc_write+0x88/0x9e
[ 16.786440] [<c1355500>] ? do_output_char+0x88/0x18a
[ 16.786440] [<c1355631>] ? process_output+0x2f/0x42
[ 16.786440] [<c1355af6>] ? n_tty_write+0x211/0x2dc
[ 16.786440] [<c1059d77>] ? try_to_wake_up+0x226/0x226
[ 16.786440] [<c13534a4>] ? tty_write+0x15e/0x1d1
[ 16.786440] [<c12c1644>] ? security_file_permission+0x22/0x26
[ 16.786440] [<c13558e5>] ? process_echoes+0x241/0x241
[ 16.786440] [<c10dd9d2>] ? vfs_write+0x84/0xd7
[ 16.786440] [<c1353346>] ? tty_write_lock+0x3d/0x3d
[ 16.786440] [<c10ddb92>] ? sys_write+0x3b/0x5d
[ 16.786440] [<c166594c>] ? sysenter_do_call+0x12/0x22
[ 16.786440] handlers:
[ 16.786440] [<c1351397>] (vp_interrupt+0x0/0x3a)
[ 16.786440] Disabling IRQ #4
Signed-off-by: Asias He <asias.hejun@gmail.com>
---
tools/kvm/virtio/console.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/tools/kvm/virtio/console.c b/tools/kvm/virtio/console.c
index f5449ba..f9031cb 100644
--- a/tools/kvm/virtio/console.c
+++ b/tools/kvm/virtio/console.c
@@ -166,13 +166,18 @@ static void virtio_console_handle_callback(struct kvm *self, void *param)
vq = param;
+ /*
+ * The current Linux implementation polls for the buffer
+ * to be used, rather than waiting for an interrupt.
+ * So there is no need to inject an interrupt for the tx path.
+ */
+
while (virt_queue__available(vq)) {
head = virt_queue__get_iov(vq, iov, &out, &in, self);
len = term_putc_iov(CONSOLE_VIRTIO, iov, out);
virt_queue__set_used_elem(vq, head, len);
}
- virt_queue__trigger_irq(vq, virtio_console_pci_device.irq_line, &cdev.isr, self);
}
static bool virtio_console_pci_io_out(struct kvm *self, u16 port, void *data, int size, u32 count)
--
1.7.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 4/4] kvm tools: Fix virtio console hangs by removing IRQ injection for tx path
2011-05-08 13:09 ` [PATCH v2 4/4] kvm tools: Fix virtio console hangs by removing IRQ injection for tx path Asias He
@ 2011-05-08 17:05 ` Pekka Enberg
2011-05-08 17:21 ` Sasha Levin
0 siblings, 1 reply; 12+ messages in thread
From: Pekka Enberg @ 2011-05-08 17:05 UTC (permalink / raw)
To: Asias He; +Cc: Cyrill Gorcunov, Ingo Molnar, Sasha Levin, Prasad Joshi, kvm
On Sun, 2011-05-08 at 21:09 +0800, Asias He wrote:
> As virtio spec says:
>
> """
> Because this is high importance and low bandwidth, the current Linux
> implementation polls for the buffer to be used, rather than waiting
> for an interrupt, simplifying the implementation signicantly.
> """
>
> drivers/char/virtio_console.c
> send_buf() {
> ...
> /* Tell Host to go! */
> virtqueue_kick(out_vq);
> ...
> while (!virtqueue_get_buf(out_vq, &len))
> cpu_relax();
> ...
> }
>
> The console hangs can simply be reproduced by yes command which
> gives tremendous console IOs and IRQs.
Sasha, does this fix the hangs you were seeing? We should re-enable
virtio console unconditionally if this does - that increases test
coverage for virtio console.
Pekka
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 4/4] kvm tools: Fix virtio console hangs by removing IRQ injection for tx path
2011-05-08 17:05 ` Pekka Enberg
@ 2011-05-08 17:21 ` Sasha Levin
2011-05-08 17:28 ` Pekka Enberg
0 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2011-05-08 17:21 UTC (permalink / raw)
To: Pekka Enberg; +Cc: Asias He, Cyrill Gorcunov, Ingo Molnar, Prasad Joshi, kvm
On Sun, 2011-05-08 at 20:05 +0300, Pekka Enberg wrote:
> On Sun, 2011-05-08 at 21:09 +0800, Asias He wrote:
> > As virtio spec says:
> >
> > """
> > Because this is high importance and low bandwidth, the current Linux
> > implementation polls for the buffer to be used, rather than waiting
> > for an interrupt, simplifying the implementation signicantly.
> > """
> >
> > drivers/char/virtio_console.c
> > send_buf() {
> > ...
> > /* Tell Host to go! */
> > virtqueue_kick(out_vq);
> > ...
> > while (!virtqueue_get_buf(out_vq, &len))
> > cpu_relax();
> > ...
> > }
> >
> > The console hangs can simply be reproduced by yes command which
> > gives tremendous console IOs and IRQs.
>
> Sasha, does this fix the hangs you were seeing? We should re-enable
> virtio console unconditionally if this does - that increases test
> coverage for virtio console.
I'm seeing no more hangs, but why enable it unconditionally?
Maybe enable it by default, but we shouldn't force the activation of
virtio modules if the user doesn't want them.
--
Sasha.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 4/4] kvm tools: Fix virtio console hangs by removing IRQ injection for tx path
2011-05-08 17:21 ` Sasha Levin
@ 2011-05-08 17:28 ` Pekka Enberg
2011-05-08 17:29 ` Sasha Levin
0 siblings, 1 reply; 12+ messages in thread
From: Pekka Enberg @ 2011-05-08 17:28 UTC (permalink / raw)
To: Sasha Levin; +Cc: Asias He, Cyrill Gorcunov, Ingo Molnar, Prasad Joshi, kvm
On Sun, May 8, 2011 at 8:21 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> I'm seeing no more hangs, but why enable it unconditionally?
> Maybe enable it by default, but we shouldn't force the activation of
> virtio modules if the user doesn't want them.
I meant enabling the device on PCI bus like we did before.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] kvm tools: Fix virtio console hangs by removing IRQ injection for tx path
2011-05-08 17:28 ` Pekka Enberg
@ 2011-05-08 17:29 ` Sasha Levin
2011-05-08 17:34 ` Pekka Enberg
0 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2011-05-08 17:29 UTC (permalink / raw)
To: Pekka Enberg; +Cc: Asias He, Cyrill Gorcunov, Ingo Molnar, Prasad Joshi, kvm
On Sun, 2011-05-08 at 20:28 +0300, Pekka Enberg wrote:
> On Sun, May 8, 2011 at 8:21 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> > I'm seeing no more hangs, but why enable it unconditionally?
> > Maybe enable it by default, but we shouldn't force the activation of
> > virtio modules if the user doesn't want them.
>
> I meant enabling the device on PCI bus like we did before.
Thats what I've meant too. virtio-console is the only device which got
initialized even if it wasn't requested (even when '-c serial' was
passed specifically).
--
Sasha.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] kvm tools: Fix virtio console hangs by removing IRQ injection for tx path
2011-05-08 17:29 ` Sasha Levin
@ 2011-05-08 17:34 ` Pekka Enberg
2011-05-08 17:35 ` Sasha Levin
0 siblings, 1 reply; 12+ messages in thread
From: Pekka Enberg @ 2011-05-08 17:34 UTC (permalink / raw)
To: Sasha Levin; +Cc: Asias He, Cyrill Gorcunov, Ingo Molnar, Prasad Joshi, kvm
On Sun, May 8, 2011 at 8:29 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> On Sun, 2011-05-08 at 20:28 +0300, Pekka Enberg wrote:
>> On Sun, May 8, 2011 at 8:21 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
>> > I'm seeing no more hangs, but why enable it unconditionally?
>> > Maybe enable it by default, but we shouldn't force the activation of
>> > virtio modules if the user doesn't want them.
>>
>> I meant enabling the device on PCI bus like we did before.
>
> Thats what I've meant too. virtio-console is the only device which got
> initialized even if it wasn't requested (even when '-c serial' was
> passed specifically).
The more options we have, the more combinations we need to test.
What's the downside of enabling virtio console by default? The upside
is that it's less likely to break. Btw, we should probably do that for
virtio rng as well.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] kvm tools: Fix virtio console hangs by removing IRQ injection for tx path
2011-05-08 17:34 ` Pekka Enberg
@ 2011-05-08 17:35 ` Sasha Levin
2011-05-08 17:41 ` Pekka Enberg
0 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2011-05-08 17:35 UTC (permalink / raw)
To: Pekka Enberg; +Cc: Asias He, Cyrill Gorcunov, Ingo Molnar, Prasad Joshi, kvm
On Sun, 2011-05-08 at 20:34 +0300, Pekka Enberg wrote:
> On Sun, May 8, 2011 at 8:29 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> > On Sun, 2011-05-08 at 20:28 +0300, Pekka Enberg wrote:
> >> On Sun, May 8, 2011 at 8:21 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> >> > I'm seeing no more hangs, but why enable it unconditionally?
> >> > Maybe enable it by default, but we shouldn't force the activation of
> >> > virtio modules if the user doesn't want them.
> >>
> >> I meant enabling the device on PCI bus like we did before.
> >
> > Thats what I've meant too. virtio-console is the only device which got
> > initialized even if it wasn't requested (even when '-c serial' was
> > passed specifically).
>
> The more options we have, the more combinations we need to test.
> What's the downside of enabling virtio console by default? The upside
> is that it's less likely to break. Btw, we should probably do that for
> virtio rng as well.
I fully support enabling it by default, I'm against enabling it even if
the user asked to have it disabled.
--
Sasha.
^ permalink raw reply [flat|nested] 12+ messages in thread