qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: Hans de Goede <hdegoede@redhat.com>, qemu-devel@nongnu.org
Subject: [Qemu-devel] [PATCH 15/17] uhci: Always mark a queue valid when we encounter it
Date: Wed, 24 Oct 2012 18:31:18 +0200	[thread overview]
Message-ID: <1351096280-9518-16-git-send-email-hdegoede@redhat.com> (raw)
In-Reply-To: <1351096280-9518-1-git-send-email-hdegoede@redhat.com>

Before this patch we would not mark a queue valid when its head was a
non-active td. This causes us to misbehave in the following scenario:

1) queue with multiple input transfers queued
2) We hit some latency issue, causing qemu to get behind processing frames
3) When qemu gets to run again, it notices the first transfer ends short,
   marking the head td non-active
4) It now processes 32+ frames in a row without giving the guest a chance
   to run since it is behind
5) valid is decreased to 0, causing the queue to get cancelled also cancelling
   already queued up further input transfers
6) guest gets to run, notices the inactive td, cleanups up further tds
   from the short transfer, and lets the queue continue at the first td of
   the next input transfer
7) we re-start the queue, issuing the second input transfer for the *second*
   time, and any data read by the first time we issued it has been lost

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/hcd-uhci.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 592ad8d..beeb3fd 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -183,6 +183,9 @@ static UHCIQueue *uhci_queue_new(UHCIState *s, uint32_t qh_addr, UHCI_TD *td,
     queue->ep = ep;
     QTAILQ_INIT(&queue->asyncs);
     QTAILQ_INSERT_HEAD(&s->queues, queue, next);
+    /* valid needs to be large enough to handle 10 frame delay
+     * for initial isochronous requests */
+    queue->valid = 32;
     trace_usb_uhci_queue_add(queue->token);
     return queue;
 }
@@ -819,6 +822,10 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
         }
     }
 
+    if (q) {
+        q->valid = 32;
+    }
+
     /* Is active ? */
     if (!(td->ctrl & TD_CTRL_ACTIVE)) {
         if (async) {
@@ -836,9 +843,6 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
     }
 
     if (async) {
-        /* Already submitted */
-        async->queue->valid = 32;
-
         if (!async->done)
             return TD_RESULT_ASYNC_CONT;
         if (queuing) {
@@ -860,11 +864,6 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
     }
     async = uhci_async_alloc(q, td_addr);
 
-    /* valid needs to be large enough to handle 10 frame delay
-     * for initial isochronous requests
-     */
-    async->queue->valid = 32;
-
     max_len = ((td->token >> 21) + 1) & 0x7ff;
     spd = (pid == USB_TOKEN_IN && (td->ctrl & TD_CTRL_SPD) != 0);
     usb_packet_setup(&async->packet, pid, q->ep, td_addr, spd,
-- 
1.7.12.1

  parent reply	other threads:[~2012-10-24 16:54 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-24 16:31 [Qemu-devel] uhci: Cleanups, fixes and improvements Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 01/17] usb: Enforce iso endpoints never returing USB_RET_ASYNC Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 02/17] uhci: No need to handle async completion of isoc packets Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 03/17] uhci: cleanup: Add an unlink call to uhci_async_cancel() Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 04/17] uhci: Don't retry on error Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 05/17] uhci: Drop unnecessary forward declaration of some static functions Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 06/17] uhci: Move emptying of the queue's asyncs' queue to uhci_queue_free Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 07/17] uhci: Rename UHCIAsync->td to UHCIAsync->td_addr Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 08/17] uhci: Add uhci_read_td() helper function Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 09/17] uhci: Make uhci_fill_queue() actually operate on an UHCIQueue Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 10/17] uhci: Store ep in UHCIQueue Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 11/17] uhci: Immediately free queues on device disconnect Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 12/17] uhci: Verify queue has not been changed by guest Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 13/17] uhci: Detect guest td re-use Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 14/17] uhci: When the guest marks a pending td non-active, cancel the queue Hans de Goede
2012-10-24 16:31 ` Hans de Goede [this message]
2012-10-24 16:31 ` [Qemu-devel] [PATCH 16/17] uhci: Retry to fill the queue while waiting for td completion Hans de Goede
2012-10-24 16:31 ` [Qemu-devel] [PATCH 17/17] uhci: Use only one queue for ctrl endpoints Hans de Goede
2012-10-25  7:14 ` [Qemu-devel] uhci: Cleanups, fixes and improvements Gerd Hoffmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1351096280-9518-16-git-send-email-hdegoede@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).