[Qemu-devel] [virtio guest] vring_need_event() from virtqueue_kick_prepare()
What is the logic behind vring_need_event() when used with virtqueue_kick_prepare()? What does the keyword >>just<< refer to from the following context: /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */ /* Assuming a given event_idx value from the other size, if * we have just incremented index from old to new_idx, * should we trigger an event? */ ? I am sending 2 jobs, one after another, and the second one just does not want to kick, although the first one finished completely and the backend went back to interrupt mode, all because vring_need_event() returns false.
Re: [Qemu-devel] [virtio guest] vring_need_event() from virtqueue_kick_prepare()
On Tue, Jul 07, 2015 at 02:43:06PM +0300, Catalin Vasile wrote: > My vhost module respects the format vhost-net uses: > > /* summary */ > mutex_lock(&vq->mutex); > vhost_disable_notify(); > for (;;) { > head = vhost_get_vq_desc(); >if (head == vq->num) { > if (unlikely(vhost_enable_notify())) { > vhost_disable_notify(); > continue; > } > break; >} >vhost_add_used_and_signal(); > } > mutex_unlock(&vq->mutex); > /* */ > > I have made a lot of printk() calls and the first job gets processed > completely, and gets through all those calls: > 1. it goes into a first loop and processes the first job (get > descriptor, work with the descriptor, add used and signal). > 2. On the second loop it hits head == vq->num, and goes back to > listening to notifications (successfully, it does not get into the > fallback). > > Now in the guest: > 1. sends first job and the paramers used to call vring_need_event() are: > vring_avail_event=0, new=1, old=0 (which makes the function evaluate to "0 < > 1") > 2. the queue is kicked and vhost does its job. > 3. the guest driver reaches the end of the first job, and lets the > following job take its course, only this time vring_need_event() > receives the following parameters: > vring_avail_event=0, new=2, old=1 (which makes the function evaluate to "1 < > 1") This means you need to look at the vhost code because vring_avail_event shouldn't be 0. Why wasn't vhost_update_avail_event() called? Maybe the feature bit negotiation for your device is broken and vhost thinks VIRTIO_RING_F_EVENT_IDX is not set. Or does the used ring have the VRING_USED_F_NO_NOTIFY flag? pgpzb2CWe5TW_.pgp Description: PGP signature
Re: [Qemu-devel] [virtio guest] vring_need_event() from virtqueue_kick_prepare()
I have managed to deal with it. The thing is I was using one of the latest versions of qemu, but an old Linux Kernel version of 3.12. On Thu, Jul 9, 2015 at 1:43 PM, Stefan Hajnoczi wrote: > On Tue, Jul 07, 2015 at 02:43:06PM +0300, Catalin Vasile wrote: >> My vhost module respects the format vhost-net uses: >> >> /* summary */ >> mutex_lock(&vq->mutex); >> vhost_disable_notify(); >> for (;;) { >> head = vhost_get_vq_desc(); >>if (head == vq->num) { >> if (unlikely(vhost_enable_notify())) { >> vhost_disable_notify(); >> continue; >> } >> break; >>} >>vhost_add_used_and_signal(); >> } >> mutex_unlock(&vq->mutex); >> /* */ >> >> I have made a lot of printk() calls and the first job gets processed >> completely, and gets through all those calls: >> 1. it goes into a first loop and processes the first job (get >> descriptor, work with the descriptor, add used and signal). >> 2. On the second loop it hits head == vq->num, and goes back to >> listening to notifications (successfully, it does not get into the >> fallback). >> >> Now in the guest: >> 1. sends first job and the paramers used to call vring_need_event() are: >> vring_avail_event=0, new=1, old=0 (which makes the function evaluate to "0 < >> 1") >> 2. the queue is kicked and vhost does its job. >> 3. the guest driver reaches the end of the first job, and lets the >> following job take its course, only this time vring_need_event() >> receives the following parameters: >> vring_avail_event=0, new=2, old=1 (which makes the function evaluate to "1 < >> 1") > > This means you need to look at the vhost code because vring_avail_event > shouldn't be 0. > > Why wasn't vhost_update_avail_event() called? > > Maybe the feature bit negotiation for your device is broken and vhost > thinks VIRTIO_RING_F_EVENT_IDX is not set. > > Or does the used ring have the VRING_USED_F_NO_NOTIFY flag?
Re: [Qemu-devel] [virtio guest] vring_need_event() from virtqueue_kick_prepare()
On Thu, Jul 09, 2015 at 01:45:50PM +0300, Catalin Vasile wrote: > I have managed to deal with it. > The thing is I was using one of the latest versions of qemu, but an > old Linux Kernel version of 3.12. Old guest kernel or old host kernel? New QEMU should support old guest kernels. Maybe you have found a bug? pgpAhWoBaNKMQ.pgp Description: PGP signature
Re: [Qemu-devel] [virtio guest] vring_need_event() from virtqueue_kick_prepare()
Both. The compiled kernel was common for both. On Thu, Jul 16, 2015 at 3:52 PM, Stefan Hajnoczi wrote: > On Thu, Jul 09, 2015 at 01:45:50PM +0300, Catalin Vasile wrote: >> I have managed to deal with it. >> The thing is I was using one of the latest versions of qemu, but an >> old Linux Kernel version of 3.12. > > Old guest kernel or old host kernel? > > New QEMU should support old guest kernels. Maybe you have found a bug?
Re: [Qemu-devel] [virtio guest] vring_need_event() from virtqueue_kick_prepare()
On Thu, Jul 16, 2015 at 1:54 PM, Catalin Vasile wrote: > Both. The compiled kernel was common for both. Does vhost_net work with the old kernel + new QEMU combo? Stefan
Re: [Qemu-devel] [virtio guest] vring_need_event() from virtqueue_kick_prepare()
Do you mean vhost_net - old kernel, qemu - latest, guest - latest? On Thu, Jul 16, 2015 at 7:33 PM, Stefan Hajnoczi wrote: > On Thu, Jul 16, 2015 at 1:54 PM, Catalin Vasile > wrote: >> Both. The compiled kernel was common for both. > > Does vhost_net work with the old kernel + new QEMU combo? > > Stefan
Re: [Qemu-devel] [virtio guest] vring_need_event() from virtqueue_kick_prepare()
On Fri, Jul 17, 2015 at 09:30:03AM +0300, Catalin Vasile wrote: > Do you mean vhost_net - old kernel, qemu - latest, guest - latest? I mean whatever combination didn't work for your custom vhost module. If vhost_net is broken too there is probably a vhost/QEMU bug that should be fixed. Stefan pgpaYd2oKLC3H.pgp Description: PGP signature
Re: [Qemu-devel] [virtio guest] vring_need_event() from virtqueue_kick_prepare()
On Mon, Jul 06, 2015 at 06:13:29PM +0300, Catalin Vasile wrote: > What is the logic behind vring_need_event() when used with > virtqueue_kick_prepare()? > What does the keyword >>just<< refer to from the following context: > /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */ > /* Assuming a given event_idx value from the other size, if > * we have just incremented index from old to new_idx, > * should we trigger an event? */ > ? "just" means since the last time the host/guest-visible index field was changed. After avail or used rings have been processed, the index field for that ring is published to the host/guest. At that point a check is made whether the other side needs to be kicked. > I am sending 2 jobs, one after another, and the second one just does > not want to kick, although the first one finished completely and the > backend went back to interrupt mode, all because vring_need_event() > returns false. Maybe the vhost driver called vhost_disable_notify() and hasn't re-enabled notify yet? This could happen if the guest adds buffers to the virtqueue while the host is processing the virtqueue. Take a look at the vhost_net code for how to correctly disable and re-enable notify without race conditions on the host. The idea behind disabling notify is to eliminate unnecessary vmexits/notifications since the host is already processing the virtqueue and will see new buffers. It's like a polling vs interrupt mode. If the vhost driver on the host doesn't implement it correctly, then the device could stop responding to the avail ring. pgpkpaAQtaSRm.pgp Description: PGP signature
Re: [Qemu-devel] [virtio guest] vring_need_event() from virtqueue_kick_prepare()
My vhost module respects the format vhost-net uses: /* summary */ mutex_lock(&vq->mutex); vhost_disable_notify(); for (;;) { head = vhost_get_vq_desc(); if (head == vq->num) { if (unlikely(vhost_enable_notify())) { vhost_disable_notify(); continue; } break; } vhost_add_used_and_signal(); } mutex_unlock(&vq->mutex); /* */ I have made a lot of printk() calls and the first job gets processed completely, and gets through all those calls: 1. it goes into a first loop and processes the first job (get descriptor, work with the descriptor, add used and signal). 2. On the second loop it hits head == vq->num, and goes back to listening to notifications (successfully, it does not get into the fallback). Now in the guest: 1. sends first job and the paramers used to call vring_need_event() are: vring_avail_event=0, new=1, old=0 (which makes the function evaluate to "0 < 1") 2. the queue is kicked and vhost does its job. 3. the guest driver reaches the end of the first job, and lets the following job take its course, only this time vring_need_event() receives the following parameters: vring_avail_event=0, new=2, old=1 (which makes the function evaluate to "1 < 1") so a kick is not actually sent because vring_need_event() returns false. From what I see as the definition for vring_need_event(), it does not actually look at flags. "if (vq->event) {" evaluates to true in both cases, so it always verifies those indexes (it does not go on the branch which verifies flags). I am also pretty sure the jobs are serialized in the guest driver, and do not cross each other's path. One of the reasons is that every function that sends a job must hold a mutex that protects the virtqueue. The guest driver blocks awaiting an interrupt for the job being finished, but vhost does not get woken up to process the job in the first places, because a notification is not actually triggered because of what I have explained above. On Tue, Jul 7, 2015 at 1:17 PM, Stefan Hajnoczi wrote: > On Mon, Jul 06, 2015 at 06:13:29PM +0300, Catalin Vasile wrote: >> What is the logic behind vring_need_event() when used with >> virtqueue_kick_prepare()? >> What does the keyword >>just<< refer to from the following context: >> /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */ >> /* Assuming a given event_idx value from the other size, if >> * we have just incremented index from old to new_idx, >> * should we trigger an event? */ >> ? > > "just" means since the last time the host/guest-visible index field was > changed. After avail or used rings have been processed, the index field > for that ring is published to the host/guest. At that point a check is > made whether the other side needs to be kicked. > >> I am sending 2 jobs, one after another, and the second one just does >> not want to kick, although the first one finished completely and the >> backend went back to interrupt mode, all because vring_need_event() >> returns false. > > Maybe the vhost driver called vhost_disable_notify() and hasn't > re-enabled notify yet? > > This could happen if the guest adds buffers to the virtqueue while the > host is processing the virtqueue. Take a look at the vhost_net code for > how to correctly disable and re-enable notify without race conditions on > the host. > > The idea behind disabling notify is to eliminate unnecessary > vmexits/notifications since the host is already processing the virtqueue > and will see new buffers. It's like a polling vs interrupt mode. > > If the vhost driver on the host doesn't implement it correctly, then the > device could stop responding to the avail ring.