qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Michael Roth <mdroth@linux.vnet.ibm.com>
To: qemu-devel@nongnu.org
Cc: aliguori@us.ibm.com
Subject: [Qemu-devel] [PATCH 09/23] ehci: don't flush cache on doorbell rings.
Date: Tue, 21 Aug 2012 12:05:43 -0500	[thread overview]
Message-ID: <1345568757-14365-10-git-send-email-mdroth@linux.vnet.ibm.com> (raw)
In-Reply-To: <1345568757-14365-1-git-send-email-mdroth@linux.vnet.ibm.com>

From: Gerd Hoffmann <kraxel@redhat.com>

Commit 4be23939ab0d7019c7e59a37485b416fbbf0f073 makes ehci instantly
zap any unlinked queue heads when the guest rings the doorbell.

While hacking up uas support this turned out to be a problem.  The linux
kernel can unlink and instantly relink the very same queue head, thereby
killing any async packets in flight.  That alone isn't an issue yet, the
packet will canceled and resubmitted and everything is fine.  We'll run
into trouble though in case the async packet is completed already, so we
can't cancel it any more.  The transaction is simply lost then.

usb_ehci_qh_ptrs q (nil) - QH @ 39c4f000: next 39c4f122 qtds 00000000,00000001,39c50000
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95feba90a0 - QH @ 39c4f000: next 39c4f122 qtds 00000000,00000001,39c50000
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95fe515210 - QH @ 39c4f120: next 39c4f0c2 qtds 29dbce40,29dbc4e0,00000009
usb_ehci_qh_fields QH @ 39c4f120 - rl 4, mplen 512, eps 2, ep 1, dev 2
usb_ehci_packet_action q 0x7f95fe515210 p 0x7f95fdec32a0: alloc
usb_packet_state_change bus 0, port 2, ep 1, packet 0x7f95fdec32e0, state undef -> setup
usb_ehci_packet_action q 0x7f95fe515210 p 0x7f95fdec32a0: process
usb_uas_command dev 2, tag 0x2, lun 0, lun64 00000000-00000000
scsi_req_parsed target 0 lun 0 tag 2 command 42 dir 2 length 16384
scsi_req_parsed_lba target 0 lun 0 tag 2 command 42 lba 5933312
scsi_req_alloc target 0 lun 0 tag 2
scsi_req_continue target 0 lun 0 tag 2
scsi_req_data target 0 lun 0 tag 2 len 16384
usb_uas_scsi_data dev 2, tag 0x2, bytes 16384
usb_uas_write_ready dev 2, tag 0x2
usb_packet_state_change bus 0, port 2, ep 1, packet 0x7f95fdec32e0, state setup -> complete
usb_ehci_packet_action q 0x7f95fe515210 p 0x7f95fdec32a0: free
usb_ehci_qh_ptrs q 0x7f95fdec3210 - QH @ 39c4f0c0: next 39c4f002 qtds 29dbce40,00000001,00000009
usb_ehci_qh_fields QH @ 39c4f0c0 - rl 4, mplen 512, eps 2, ep 2, dev 2
usb_ehci_queue_action q 0x7f95fe5152a0: free
usb_packet_state_change bus 0, port 2, ep 2, packet 0x7f95feba9170, state async -> complete
^^^ async packets completes.
usb_ehci_packet_action q 0x7f95fdec3210 p 0x7f95feba9130: wakeup

usb_ehci_qh_ptrs q (nil) - QH @ 39c4f000: next 39c4f122 qtds 00000000,00000001,39c50000
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95feba90a0 - QH @ 39c4f000: next 39c4f122 qtds 00000000,00000001,39c50000
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95fe515210 - QH @ 39c4f120: next 39c4f002 qtds 29dbc4e0,29dbc8a0,00000009
usb_ehci_qh_fields QH @ 39c4f120 - rl 4, mplen 512, eps 2, ep 1, dev 2
usb_ehci_queue_action q 0x7f95fdec3210: free
usb_ehci_packet_action q 0x7f95fdec3210 p 0x7f95feba9130: free
^^^ endpoint #2 queue head removed from schedule, doorbell makes ehci zap the queue,
    the (completed) usb packet is freed too and gets lost.

usb_ehci_qh_ptrs q (nil) - QH @ 39c4f000: next 39c4f0c2 qtds 00000000,00000001,39c50000
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95feba90a0 - QH @ 39c4f000: next 39c4f0c2 qtds 00000000,00000001,39c50000
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_queue_action q 0x7f9600dff570: alloc
usb_ehci_qh_ptrs q 0x7f9600dff570 - QH @ 39c4f0c0: next 39c4f122 qtds 29dbce40,00000001,00000009
usb_ehci_qh_fields QH @ 39c4f0c0 - rl 4, mplen 512, eps 2, ep 2, dev 2
usb_ehci_packet_action q 0x7f9600dff570 p 0x7f95feba9130: alloc
usb_packet_state_change bus 0, port 2, ep 2, packet 0x7f95feba9170, state undef -> setup
usb_ehci_packet_action q 0x7f9600dff570 p 0x7f95feba9130: process
usb_packet_state_change bus 0, port 2, ep 2, packet 0x7f95feba9170, state setup -> async
usb_ehci_packet_action q 0x7f9600dff570 p 0x7f95feba9130: async
^^^ linux kernel relinked the queue head, ehci creates a new usb packet,
    but we should have delivered the completed one instead.
usb_ehci_qh_ptrs q 0x7f95fe515210 - QH @ 39c4f120: next 39c4f002 qtds 29dbc4e0,29dbc8a0,00000009
usb_ehci_qh_fields QH @ 39c4f120 - rl 4, mplen 512, eps 2, ep 1, dev 2

So instead of instantly zapping the queue we'll set a flag that the
queue needs revalidation in case we'll see it again in the schedule.
ehci then checks that the queue head fields addressing / describing the
endpoint and the qtd pointer match the cached content before reusing it.

Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
(cherry picked from commit 9bc3a3a216e2689bfcdd36c3e079333bbdbf3ba0)

Conflicts:

	hw/usb/hcd-ehci.c

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/usb/hcd-ehci.c |   34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 5855c5d..ce8b405 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -348,6 +348,7 @@ struct EHCIQueue {
     QTAILQ_ENTRY(EHCIQueue) next;
     uint32_t seen;
     uint64_t ts;
+    int revalidate;
 
     /* cached data from guest - needs to be flushed
      * when guest removes an entry (doorbell, handshake sequence)
@@ -695,7 +696,18 @@ static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, uint32_t addr,
     return NULL;
 }
 
-static void ehci_queues_rip_unused(EHCIState *ehci, int async, int flush)
+static void ehci_queues_tag_unused_async(EHCIState *ehci)
+{
+    EHCIQueue *q;
+
+    QTAILQ_FOREACH(q, &ehci->aqueues, next) {
+        if (!q->seen) {
+            q->revalidate = 1;
+        }
+    }
+}
+
+static void ehci_queues_rip_unused(EHCIState *ehci, int async)
 {
     EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
     EHCIQueue *q, *tmp;
@@ -706,7 +718,7 @@ static void ehci_queues_rip_unused(EHCIState *ehci, int async, int flush)
             q->ts = ehci->last_run_ns;
             continue;
         }
-        if (!flush && ehci->last_run_ns < q->ts + 250000000) {
+        if (ehci->last_run_ns < q->ts + 250000000) {
             /* allow 0.25 sec idle */
             continue;
         }
@@ -1521,7 +1533,7 @@ static int ehci_state_waitlisthead(EHCIState *ehci,  int async)
         ehci_set_usbsts(ehci, USBSTS_REC);
     }
 
-    ehci_queues_rip_unused(ehci, async, 0);
+    ehci_queues_rip_unused(ehci, async);
 
     /*  Find the head of the list (4.9.1.1) */
     for(i = 0; i < MAX_QH; i++) {
@@ -1605,6 +1617,7 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async)
 {
     uint32_t entry;
     EHCIQueue *q;
+    EHCIqh qh;
 
     entry = ehci_get_fetch_addr(ehci, async);
     q = ehci_find_queue_by_qh(ehci, entry, async);
@@ -1622,7 +1635,16 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async)
     }
 
     get_dwords(ehci, NLPTR_GET(q->qhaddr),
-               (uint32_t *) &q->qh, sizeof(EHCIqh) >> 2);
+               (uint32_t *) &qh, sizeof(EHCIqh) >> 2);
+    if (q->revalidate && (q->qh.epchar      != qh.epchar ||
+                          q->qh.epcap       != qh.epcap  ||
+                          q->qh.current_qtd != qh.current_qtd)) {
+        ehci_free_queue(q, async);
+        q = ehci_alloc_queue(ehci, async);
+        q->seen++;
+    }
+    q->qh = qh;
+    q->revalidate = 0;
     ehci_trace_qh(q, NLPTR_GET(q->qhaddr), &q->qh);
 
     if (q->async == EHCI_ASYNC_INFLIGHT) {
@@ -2047,7 +2069,7 @@ static void ehci_advance_async_state(EHCIState *ehci)
          */
         if (ehci->usbcmd & USBCMD_IAAD) {
             /* Remove all unseen qhs from the async qhs queue */
-            ehci_queues_rip_unused(ehci, async, 1);
+            ehci_queues_tag_unused_async(ehci);
             DPRINTF("ASYNC: doorbell request acknowledged\n");
             ehci->usbcmd &= ~USBCMD_IAAD;
             ehci_set_interrupt(ehci, USBSTS_IAA);
@@ -2102,7 +2124,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
         ehci_set_fetch_addr(ehci, async,entry);
         ehci_set_state(ehci, async, EST_FETCHENTRY);
         ehci_advance_state(ehci, async);
-        ehci_queues_rip_unused(ehci, async, 0);
+        ehci_queues_rip_unused(ehci, async);
         break;
 
     default:
-- 
1.7.9.5

  parent reply	other threads:[~2012-08-21 17:06 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-21 17:05 [Qemu-devel] [stable-1.1] Patch Round-up for stable 1.1.2 Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 01/23] qtest: fix infinite loop when QEMU aborts abruptly Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 02/23] configure: Don't override user's --cpu on MacOS and Solaris Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 03/23] ppc: Fix bug in handling of PAPR hypercall exits Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 04/23] s390: Fix error handling and condition code of service call Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 05/23] kvmvapic: Disable if there is insufficient memory Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 06/23] qdev: fix use-after-free in the error path of qdev_init_nofail Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 07/23] virtio-blk: fix use-after-free while handling scsi commands Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 08/23] ehci: fix reset Michael Roth
2012-08-21 17:05 ` Michael Roth [this message]
2012-08-21 17:05 ` [Qemu-devel] [PATCH 10/23] uhci: fix uhci_async_cancel_all Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 11/23] usb: restore USBDevice->attached on vmload Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 12/23] usb-redir: Correctly handle the usb_redir_babble usbredir status Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 13/23] usb-ehci: Fix an assert whenever isoc transfers are used Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 14/23] qlist: add qlist_size() Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 15/23] json-parser: don't replicate tokens at each level of recursion Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 16/23] check-qjson: add test for large JSON objects Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 17/23] slirp: Enforce host-side user of smb share Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 18/23] slirp: Ensure smbd and shared directory exist when enable smb Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 19/23] slirp: Improve error reporting of inaccessible smb directories Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 20/23] apic: Resolve potential endless loop around apic_update_irq Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 21/23] apic: Reevaluate pending interrupts on LVT_LINT0 changes Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 22/23] apic: Defer interrupt updates to VCPU thread Michael Roth
2012-08-21 17:05 ` [Qemu-devel] [PATCH 23/23] update VERSION for 1.1.2 Michael Roth

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=1345568757-14365-10-git-send-email-mdroth@linux.vnet.ibm.com \
    --to=mdroth@linux.vnet.ibm.com \
    --cc=aliguori@us.ibm.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).