All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/4] usb attached scsi
@ 2012-06-20 12:41 Gerd Hoffmann
  2012-06-20 12:41 ` [Qemu-devel] [PATCH 1/4] ehci: fix ehci_qh_do_overlay Gerd Hoffmann
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2012-06-20 12:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

  Hi,

This patch series adds UAS (usb attached scsi) emulation to qemu.  UAS
is a protocol which uses usb3 streams and this is intended to be the
playground when adding full usb3 support to the xhci emulation.  It
turned out to also be a heavy stress test for the ehci emulation, so
this patch series includes a bunch of ehci bugfixes too.

Patch #4 is not ready for merge yet, I still have to think about a
better way to fix the issue described in the commit log.

Reviews & comments are welcome.

cheers,
  Gerd

Gerd Hoffmann (4):
  ehci: fix ehci_qh_do_overlay
  ehci: fix td writeback
  usb: add usb attached scsi emulation
  [wip] ehci: don't flush cache on dorbell rings.

 docs/usb-storage.txt |   38 +++
 hw/usb/Makefile.objs |    1 +
 hw/usb/dev-uas.c     |  792 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/usb/hcd-ehci.c    |   45 ++--
 trace-events         |   14 +
 5 files changed, 869 insertions(+), 21 deletions(-)
 create mode 100644 docs/usb-storage.txt
 create mode 100644 hw/usb/dev-uas.c

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 1/4] ehci: fix ehci_qh_do_overlay
  2012-06-20 12:41 [Qemu-devel] [PATCH 0/4] usb attached scsi Gerd Hoffmann
@ 2012-06-20 12:41 ` Gerd Hoffmann
  2012-06-20 12:41 ` [Qemu-devel] [PATCH 2/4] ehci: fix td writeback Gerd Hoffmann
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2012-06-20 12:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Use ehci_flush_qh to make sure we touch inly the fields the hc is
allowed to touch.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ehci.c |   37 ++++++++++++++++++-------------------
 1 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 6d2d549..f5fc008 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1246,6 +1246,23 @@ static inline int put_dwords(EHCIState *ehci, uint32_t addr,
     return 1;
 }
 
+/*
+ *  Write the qh back to guest physical memory.  This step isn't
+ *  in the EHCI spec but we need to do it since we don't share
+ *  physical memory with our guest VM.
+ *
+ *  The first three dwords are read-only for the EHCI, so skip them
+ *  when writing back the qh.
+ */
+static void ehci_flush_qh(EHCIQueue *q)
+{
+    uint32_t *qh = (uint32_t *) &q->qh;
+    uint32_t dwords = sizeof(EHCIqh) >> 2;
+    uint32_t addr = NLPTR_GET(q->qhaddr);
+
+    put_dwords(q->ehci, addr + 3 * sizeof(uint32_t), qh + 3, dwords - 3);
+}
+
 // 4.10.2
 
 static int ehci_qh_do_overlay(EHCIQueue *q)
@@ -1293,8 +1310,7 @@ static int ehci_qh_do_overlay(EHCIQueue *q)
     q->qh.bufptr[1] &= ~BUFPTR_CPROGMASK_MASK;
     q->qh.bufptr[2] &= ~BUFPTR_FRAMETAG_MASK;
 
-    put_dwords(q->ehci, NLPTR_GET(q->qhaddr), (uint32_t *) &q->qh,
-               sizeof(EHCIqh) >> 2);
+    ehci_flush_qh(q);
 
     return 0;
 }
@@ -1600,23 +1616,6 @@ static int ehci_process_itd(EHCIState *ehci,
 }
 
 
-/*
- *  Write the qh back to guest physical memory.  This step isn't
- *  in the EHCI spec but we need to do it since we don't share
- *  physical memory with our guest VM.
- *
- *  The first three dwords are read-only for the EHCI, so skip them
- *  when writing back the qh.
- */
-static void ehci_flush_qh(EHCIQueue *q)
-{
-    uint32_t *qh = (uint32_t *) &q->qh;
-    uint32_t dwords = sizeof(EHCIqh) >> 2;
-    uint32_t addr = NLPTR_GET(q->qhaddr);
-
-    put_dwords(q->ehci, addr + 3 * sizeof(uint32_t), qh + 3, dwords - 3);
-}
-
 /*  This state is the entry point for asynchronous schedule
  *  processing.  Entry here consitutes a EHCI start event state (4.8.5)
  */
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 2/4] ehci: fix td writeback
  2012-06-20 12:41 [Qemu-devel] [PATCH 0/4] usb attached scsi Gerd Hoffmann
  2012-06-20 12:41 ` [Qemu-devel] [PATCH 1/4] ehci: fix ehci_qh_do_overlay Gerd Hoffmann
@ 2012-06-20 12:41 ` Gerd Hoffmann
  2012-06-20 12:41 ` [Qemu-devel] [PATCH 3/4] usb: add usb attached scsi emulation Gerd Hoffmann
  2012-06-20 12:41 ` [Qemu-devel] [PATCH 4/4] [wip] ehci: don't flush cache on dorbell rings Gerd Hoffmann
  3 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2012-06-20 12:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Only write back the dwords the hc is supposed to update.  Should not
make a difference in theory as the guest must not touch the td while
it is active to avoid races.  But it is still more correct.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ehci.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index f5fc008..9fb6423 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2070,6 +2070,7 @@ out:
 static int ehci_state_writeback(EHCIQueue *q)
 {
     EHCIPacket *p = QTAILQ_FIRST(&q->packets);
+    uint32_t *qtd, addr;
     int again = 0;
 
     /*  Write back the QTD from the QH area */
@@ -2077,8 +2078,9 @@ static int ehci_state_writeback(EHCIQueue *q)
     assert(p->qtdaddr == q->qtdaddr);
 
     ehci_trace_qtd(q, NLPTR_GET(p->qtdaddr), (EHCIqtd *) &q->qh.next_qtd);
-    put_dwords(q->ehci, NLPTR_GET(p->qtdaddr), (uint32_t *) &q->qh.next_qtd,
-               sizeof(EHCIqtd) >> 2);
+    qtd = (uint32_t *) &q->qh.next_qtd;
+    addr = NLPTR_GET(p->qtdaddr);
+    put_dwords(q->ehci, addr + 2 * sizeof(uint32_t), qtd + 2, 2);
     ehci_free_packet(p);
 
     /*
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 3/4] usb: add usb attached scsi emulation
  2012-06-20 12:41 [Qemu-devel] [PATCH 0/4] usb attached scsi Gerd Hoffmann
  2012-06-20 12:41 ` [Qemu-devel] [PATCH 1/4] ehci: fix ehci_qh_do_overlay Gerd Hoffmann
  2012-06-20 12:41 ` [Qemu-devel] [PATCH 2/4] ehci: fix td writeback Gerd Hoffmann
@ 2012-06-20 12:41 ` Gerd Hoffmann
  2012-07-01 13:36   ` Paolo Bonzini
  2012-06-20 12:41 ` [Qemu-devel] [PATCH 4/4] [wip] ehci: don't flush cache on dorbell rings Gerd Hoffmann
  3 siblings, 1 reply; 11+ messages in thread
From: Gerd Hoffmann @ 2012-06-20 12:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

$subject says all.  First cut.

It's a pure UAS (usb attached scsi) emulation, without BOT (bulk-only
transport) compatibility.  If your guest can't handle it use usb-storage
instead.

The emulation works like any other scsi hba emulation (eps, lsi, virtio,
megasas, ...).  It provides just the HBA where you can attach scsi
devices as you like using '-device'.  A single scsi target with up to
256 luns is supported.

For now only usb 2.0 transport is supported.  This will change in the
future though as I plan to use this as playground when codeing up &
testing usb 3.0 transport and streams support in the qemu usb core and
the xhci emulation.

No migration support yet.  I'm planning to add usb 3.0 support first as
this probably requires saving additional state.

Special thanks go to Paolo for bringing the qemu scsi emulation into
shape, so this can be added nicely without having to touch a single line
of scsi code.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 docs/usb-storage.txt |   38 +++
 hw/usb/Makefile.objs |    1 +
 hw/usb/dev-uas.c     |  792 ++++++++++++++++++++++++++++++++++++++++++++++++++
 trace-events         |   14 +
 4 files changed, 845 insertions(+), 0 deletions(-)
 create mode 100644 docs/usb-storage.txt
 create mode 100644 hw/usb/dev-uas.c

diff --git a/docs/usb-storage.txt b/docs/usb-storage.txt
new file mode 100644
index 0000000..ff97559
--- /dev/null
+++ b/docs/usb-storage.txt
@@ -0,0 +1,38 @@
+
+qemu usb storage emulation
+--------------------------
+
+Qemu has two emulations for usb storage devices.
+
+Number one emulates the classic bulk-only transport protocol which is
+used by 99% of the usb sticks on the marked today and is called
+"usb-storage".  Usage (hooking up to xhci, other host controllers work
+too):
+
+  qemu ${other_vm_args}                                \
+       -drive if=none,id=stick,file=/path/to/file.img  \
+       -device nec-usb-xhci,id=xhci                    \
+       -device usb-storage,bus=xhci.0,drive=stick
+
+
+Number two is the newer usb attached scsi transport.  This one doesn't
+automagically create a scsi disk, so you have to explicitly attach one
+manually.  Multiple logical units are supported.  Here is an example
+with tree logical units:
+
+  qemu ${other_vm_args}                                                \
+       -drive if=none,id=uas-disk1,file=/path/to/file1.img             \
+       -drive if=none,id=uas-disk2,file=/path/to/file2.img             \
+       -drive if=none,id=uas-cdrom,media=cdrom,file=/path/to/image.iso \
+       -device nec-usb-xhci,id=xhci                                    \
+       -device usb-uas,id=uas,bus=xhci.0                               \
+       -device scsi-hd,bus=uas.0,scsi-id=0,lun=0,drive=uas-disk1       \
+       -device scsi-hd,bus=uas.0,scsi-id=0,lun=1,drive=uas-disk2       \
+       -device scsi-cd,bus=uas.0,scsi-id=0,lun=5,drive=uas-cdrom
+
+
+enjoy,
+  Gerd
+
+--
+Gerd Hoffmann <kraxel@redhat.com>
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 9c7ddf5..4225136 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -11,3 +11,4 @@ common-obj-y += core.o bus.o desc.o dev-hub.o
 common-obj-y += host-$(HOST_USB).o dev-bluetooth.o
 common-obj-y += dev-hid.o dev-storage.o dev-wacom.o
 common-obj-y += dev-serial.o dev-network.o dev-audio.o
+common-obj-y += dev-uas.o
diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
new file mode 100644
index 0000000..21fc80e
--- /dev/null
+++ b/hw/usb/dev-uas.c
@@ -0,0 +1,792 @@
+/*
+ * UAS (USB Attached SCSI) emulation
+ *
+ * Copyright Red Hat, Inc. 2012
+ *
+ * Author: Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu-common.h"
+#include "qemu-option.h"
+#include "qemu-config.h"
+#include "trace.h"
+
+#include "hw/usb.h"
+#include "hw/usb/desc.h"
+#include "hw/scsi.h"
+#include "hw/scsi-defs.h"
+
+/* --------------------------------------------------------------------- */
+
+#define UAS_UI_COMMAND              0x01
+#define UAS_UI_SENSE                0x03
+#define UAS_UI_RESPONSE             0x04
+#define UAS_UI_TASK_MGMT            0x05
+#define UAS_UI_READ_READY           0x06
+#define UAS_UI_WRITE_READY          0x07
+
+#define UAS_RC_TMF_COMPLETE         0x00
+#define UAS_RC_INVALID_INFO_UNIT    0x02
+#define UAS_RC_TMF_NOT_SUPPORTED    0x04
+#define UAS_RC_TMF_FAILED           0x05
+#define UAS_RC_TMF_SUCCEEDED        0x08
+#define UAS_RC_INCORRECT_LUN        0x09
+#define UAS_RC_OVERLAPPED_TAG       0x0a
+
+#define UAS_TMF_ABORT_TASK          0x01
+#define UAS_TMF_ABORT_TASK_SET      0x02
+#define UAS_TMF_CLEAR_TASK_SET      0x04
+#define UAS_TMF_LOGICAL_UNIT_RESET  0x08
+#define UAS_TMF_I_T_NEXUS_RESET     0x10
+#define UAS_TMF_CLEAR_ACA           0x40
+#define UAS_TMF_QUERY_TASK          0x80
+#define UAS_TMF_QUERY_TASK_SET      0x81
+#define UAS_TMF_QUERY_ASYNC_EVENT   0x82
+
+#define UAS_PIPE_ID_COMMAND         0x01
+#define UAS_PIPE_ID_STATUS          0x02
+#define UAS_PIPE_ID_DATA_IN         0x03
+#define UAS_PIPE_ID_DATA_OUT        0x04
+
+typedef struct {
+    uint8_t    id;
+    uint8_t    reserved;
+    uint16_t   tag;
+} QEMU_PACKED  uas_ui_header;
+
+typedef struct {
+    uint8_t    prio_taskattr;   /* 6:3 priority, 2:0 task attribute   */
+    uint8_t    reserved_1;
+    uint8_t    add_cdb_length;  /* 7:2 additional adb length (dwords) */
+    uint8_t    reserved_2;
+    uint64_t   lun;
+    uint8_t    cdb[16];
+    uint8_t    add_cdb[];
+} QEMU_PACKED  uas_ui_command;
+
+typedef struct {
+    uint16_t   status_qualifier;
+    uint8_t    status;
+    uint8_t    reserved[7];
+    uint16_t   sense_length;
+    uint8_t    sense_data[18];
+} QEMU_PACKED  uas_ui_sense;
+
+typedef struct {
+    uint16_t   add_response_info;
+    uint8_t    response_code;
+} QEMU_PACKED  uas_ui_response;
+
+typedef struct {
+    uint8_t    function;
+    uint8_t    reserved;
+    uint16_t   task_tag;
+    uint64_t   lun;
+} QEMU_PACKED  uas_ui_task_mgmt;
+
+typedef struct {
+    uas_ui_header  hdr;
+    union {
+        uas_ui_command   command;
+        uas_ui_sense     sense;
+        uas_ui_task_mgmt task;
+        uas_ui_response  response;
+    };
+} QEMU_PACKED  uas_ui;
+
+/* --------------------------------------------------------------------- */
+
+typedef struct UASDevice UASDevice;
+typedef struct UASRequest UASRequest;
+typedef struct UASStatus UASStatus;
+
+struct UASDevice {
+    USBDevice                 dev;
+    SCSIBus                   bus;
+    UASRequest                *datain;
+    UASRequest                *dataout;
+    USBPacket                 *status;
+    QEMUBH                    *status_bh;
+    QTAILQ_HEAD(, UASStatus)  results;
+    QTAILQ_HEAD(, UASRequest) requests;
+};
+
+struct UASRequest {
+    uint16_t     tag;
+    uint64_t     lun;
+    UASDevice    *uas;
+    SCSIDevice   *dev;
+    SCSIRequest  *req;
+    USBPacket    *data;
+    bool         data_async;
+    bool         active;
+    uint32_t     buf_off;
+    uint32_t     buf_size;
+    uint32_t     data_off;
+    uint32_t     data_size;
+    uint32_t     refcount;
+    QTAILQ_ENTRY(UASRequest)  next;
+};
+
+struct UASStatus {
+    uas_ui                    status;
+    uint32_t                  length;
+    QTAILQ_ENTRY(UASStatus)   next;
+};
+
+/* --------------------------------------------------------------------- */
+
+enum {
+    STR_MANUFACTURER = 1,
+    STR_PRODUCT,
+    STR_SERIALNUMBER,
+    STR_CONFIG_HIGH,
+};
+
+static const USBDescStrings desc_strings = {
+    [STR_MANUFACTURER] = "QEMU",
+    [STR_PRODUCT]      = "USB Attached SCSI HBA",
+    [STR_SERIALNUMBER] = "27842",
+    [STR_CONFIG_HIGH]  = "High speed config (usb 2.0)",
+};
+
+static const USBDescIface desc_iface_high = {
+    .bInterfaceNumber              = 0,
+    .bNumEndpoints                 = 4,
+    .bInterfaceClass               = USB_CLASS_MASS_STORAGE,
+    .bInterfaceSubClass            = 0x06, /* SCSI */
+    .bInterfaceProtocol            = 0x62, /* UAS  */
+    .eps = (USBDescEndpoint[]) {
+        {
+            .bEndpointAddress      = USB_DIR_OUT | UAS_PIPE_ID_COMMAND,
+            .bmAttributes          = USB_ENDPOINT_XFER_BULK,
+            .wMaxPacketSize        = 512,
+            .extra = (uint8_t[]) {
+                0x04,  /*  u8  bLength */
+                0x24,  /*  u8  bDescriptorType */
+                UAS_PIPE_ID_COMMAND,
+                0x00,  /*  u8  bReserved */
+            },
+        },{
+            .bEndpointAddress      = USB_DIR_IN | UAS_PIPE_ID_STATUS,
+            .bmAttributes          = USB_ENDPOINT_XFER_BULK,
+            .wMaxPacketSize        = 512,
+            .extra = (uint8_t[]) {
+                0x04,  /*  u8  bLength */
+                0x24,  /*  u8  bDescriptorType */
+                UAS_PIPE_ID_STATUS,
+                0x00,  /*  u8  bReserved */
+            },
+        },{
+            .bEndpointAddress      = USB_DIR_IN | UAS_PIPE_ID_DATA_IN,
+            .bmAttributes          = USB_ENDPOINT_XFER_BULK,
+            .wMaxPacketSize        = 512,
+            .extra = (uint8_t[]) {
+                0x04,  /*  u8  bLength */
+                0x24,  /*  u8  bDescriptorType */
+                UAS_PIPE_ID_DATA_IN,
+                0x00,  /*  u8  bReserved */
+            },
+        },{
+            .bEndpointAddress      = USB_DIR_OUT | UAS_PIPE_ID_DATA_OUT,
+            .bmAttributes          = USB_ENDPOINT_XFER_BULK,
+            .wMaxPacketSize        = 512,
+            .extra = (uint8_t[]) {
+                0x04,  /*  u8  bLength */
+                0x24,  /*  u8  bDescriptorType */
+                UAS_PIPE_ID_DATA_OUT,
+                0x00,  /*  u8  bReserved */
+            },
+        },
+    }
+};
+
+static const USBDescDevice desc_device_high = {
+    .bcdUSB                        = 0x0200,
+    .bMaxPacketSize0               = 64,
+    .bNumConfigurations            = 1,
+    .confs = (USBDescConfig[]) {
+        {
+            .bNumInterfaces        = 1,
+            .bConfigurationValue   = 1,
+            .iConfiguration        = STR_CONFIG_HIGH,
+            .bmAttributes          = 0xc0,
+            .nif = 1,
+            .ifs = &desc_iface_high,
+        },
+    },
+};
+
+static const USBDesc desc = {
+    .id = {
+        .idVendor          = 0x46f4, /* CRC16() of "QEMU" */
+        .idProduct         = 0x0002,
+        .bcdDevice         = 0,
+        .iManufacturer     = STR_MANUFACTURER,
+        .iProduct          = STR_PRODUCT,
+        .iSerialNumber     = STR_SERIALNUMBER,
+    },
+    .high = &desc_device_high,
+    .str  = desc_strings,
+};
+
+/* --------------------------------------------------------------------- */
+
+static UASStatus *usb_uas_alloc_status(uint8_t id, uint16_t tag)
+{
+    UASStatus *st = g_new0(UASStatus, 1);
+
+    st->status.hdr.id = id;
+    st->status.hdr.tag = cpu_to_be16(tag);
+    st->length = sizeof(uas_ui_header);
+    return st;
+}
+
+static void usb_uas_send_status_bh(void *opaque)
+{
+    UASDevice *uas = opaque;
+    UASStatus *st = QTAILQ_FIRST(&uas->results);
+    USBPacket *p = uas->status;
+
+    assert(p != NULL);
+    assert(st != NULL);
+
+    uas->status = NULL;
+    usb_packet_copy(p, &st->status, st->length);
+    p->result = st->length;
+    QTAILQ_REMOVE(&uas->results, st, next);
+    g_free(st);
+
+    usb_packet_complete(&uas->dev, p);
+}
+
+static void usb_uas_queue_status(UASDevice *uas, UASStatus *st, int length)
+{
+    st->length += length;
+    QTAILQ_INSERT_TAIL(&uas->results, st, next);
+    if (uas->status) {
+        /*
+         * Just schedule bh make sure any in-flight data transaction
+         * is finished before completing (sending) the status packet.
+         */
+        qemu_bh_schedule(uas->status_bh);
+    } else {
+        USBEndpoint *ep = usb_ep_get(&uas->dev, USB_TOKEN_IN,
+                                     UAS_PIPE_ID_STATUS);
+        usb_wakeup(ep);
+    }
+}
+
+static void usb_uas_queue_response(UASDevice *uas, uint16_t tag,
+                                   uint8_t code, uint16_t add_info)
+{
+    UASStatus *st = usb_uas_alloc_status(UAS_UI_RESPONSE, tag);
+
+    trace_usb_uas_response(uas->dev.addr, tag, code);
+    st->status.response.response_code = code;
+    st->status.response.add_response_info = cpu_to_be16(add_info);
+    usb_uas_queue_status(uas, st, sizeof(uas_ui_response));
+}
+
+static void usb_uas_queue_sense(UASRequest *req, uint8_t status)
+{
+    UASStatus *st = usb_uas_alloc_status(UAS_UI_SENSE, req->tag);
+    int len, slen = 0;
+
+    trace_usb_uas_sense(req->uas->dev.addr, req->tag, status);
+    st->status.sense.status = status;
+    st->status.sense.status_qualifier = cpu_to_be16(0);
+    if (status != GOOD) {
+        slen = scsi_req_get_sense(req->req, st->status.sense.sense_data,
+                                  sizeof(st->status.sense.sense_data));
+        st->status.sense.sense_length = cpu_to_be16(slen);
+    }
+    len = sizeof(uas_ui_sense) - sizeof(st->status.sense.sense_data) + slen;
+    usb_uas_queue_status(req->uas, st, len);
+}
+
+static void usb_uas_queue_read_ready(UASRequest *req)
+{
+    UASStatus *st = usb_uas_alloc_status(UAS_UI_READ_READY, req->tag);
+
+    trace_usb_uas_read_ready(req->uas->dev.addr, req->tag);
+    usb_uas_queue_status(req->uas, st, 0);
+}
+
+static void usb_uas_queue_write_ready(UASRequest *req)
+{
+    UASStatus *st = usb_uas_alloc_status(UAS_UI_WRITE_READY, req->tag);
+
+    trace_usb_uas_write_ready(req->uas->dev.addr, req->tag);
+    usb_uas_queue_status(req->uas, st, 0);
+}
+
+/* --------------------------------------------------------------------- */
+
+static int usb_uas_get_lun(uint64_t lun64)
+{
+    return (lun64 >> 48) & 0xff;
+}
+
+static SCSIDevice *usb_uas_get_dev(UASDevice *uas, uint64_t lun64)
+{
+    if ((lun64 >> 56) != 0x00) {
+        return NULL;
+    }
+    return scsi_device_find(&uas->bus, 0, 0, usb_uas_get_lun(lun64));
+}
+
+static void usb_uas_complete_data_packet(UASRequest *req)
+{
+    USBPacket *p;
+
+    if (!req->data_async) {
+        return;
+    }
+    p = req->data;
+    req->data = NULL;
+    req->data_async = false;
+    usb_packet_complete(&req->uas->dev, p);
+}
+
+static void usb_uas_copy_data(UASRequest *req)
+{
+    uint32_t length;
+
+    length = MIN(req->buf_size - req->buf_off,
+                 req->data->iov.size - req->data->result);
+    trace_usb_uas_xfer_data(req->uas->dev.addr, req->tag, length,
+                            req->data->result, req->data->iov.size,
+                            req->buf_off, req->buf_size);
+    usb_packet_copy(req->data, scsi_req_get_buf(req->req) + req->buf_off,
+                    length);
+    req->buf_off += length;
+    req->data_off += length;
+
+    if (req->data->result == req->data->iov.size) {
+        usb_uas_complete_data_packet(req);
+    }
+    if (req->buf_size && req->buf_off == req->buf_size) {
+        req->buf_off = 0;
+        req->buf_size = 0;
+        scsi_req_continue(req->req);
+    }
+}
+
+static void usb_uas_start_next_transfer(UASDevice *uas)
+{
+    UASRequest *req;
+
+    QTAILQ_FOREACH(req, &uas->requests, next) {
+        if (req->active) {
+            continue;
+        }
+        if (req->req->cmd.mode == SCSI_XFER_FROM_DEV && uas->datain == NULL) {
+            uas->datain = req;
+            usb_uas_queue_read_ready(req);
+            req->active = true;
+            return;
+        }
+        if (req->req->cmd.mode == SCSI_XFER_TO_DEV && uas->dataout == NULL) {
+            uas->dataout = req;
+            usb_uas_queue_write_ready(req);
+            req->active = true;
+            return;
+        }
+    }
+}
+
+static UASRequest *usb_uas_alloc_request(UASDevice *uas, uas_ui *ui)
+{
+    UASRequest *req;
+
+    req = g_new0(UASRequest, 1);
+    req->uas = uas;
+    req->tag = be16_to_cpu(ui->hdr.tag);
+    req->lun = be64_to_cpu(ui->command.lun);
+    req->dev = usb_uas_get_dev(req->uas, req->lun);
+    req->refcount = 1;
+    return req;
+}
+
+static void usb_uas_release_request(UASRequest *req)
+{
+    UASDevice *uas = req->uas;
+
+    if (req == uas->datain) {
+        uas->datain = NULL;
+    }
+    if (req == uas->dataout) {
+        uas->dataout = NULL;
+    }
+    QTAILQ_REMOVE(&uas->requests, req, next);
+    g_free(req);
+}
+
+static UASRequest *usb_uas_find_request(UASDevice *uas, uint16_t tag)
+{
+    UASRequest *req;
+
+    QTAILQ_FOREACH(req, &uas->requests, next) {
+        if (req->tag == tag) {
+            return req;
+        }
+    }
+    return NULL;
+}
+
+static void usb_uas_ref_request(UASRequest *req)
+{
+    req->refcount++;
+}
+
+static void usb_uas_unref_request(UASRequest *req)
+{
+    req->refcount--;
+    if (req->refcount == 0) {
+        usb_uas_release_request(req);
+    }
+}
+
+static void usb_uas_scsi_transfer_data(SCSIRequest *r, uint32_t len)
+{
+    UASRequest *req = r->hba_private;
+
+    trace_usb_uas_scsi_data(req->uas->dev.addr, req->tag, len);
+    req->buf_off = 0;
+    req->buf_size = len;
+    if (req->data) {
+        usb_uas_copy_data(req);
+    } else {
+        usb_uas_start_next_transfer(req->uas);
+    }
+}
+
+static void usb_uas_scsi_command_complete(SCSIRequest *r,
+                                          uint32_t status, size_t resid)
+{
+    UASRequest *req = r->hba_private;
+    UASDevice *uas = req->uas;
+
+    trace_usb_uas_scsi_complete(req->uas->dev.addr, req->tag, status, resid);
+    if (req->data) {
+        usb_uas_complete_data_packet(req);
+    }
+    usb_uas_queue_sense(req, status);
+    scsi_req_unref(req->req);
+    req->req = NULL;
+    usb_uas_unref_request(req);
+    usb_uas_start_next_transfer(uas);
+}
+
+static void usb_uas_scsi_request_cancelled(SCSIRequest *r)
+{
+    UASRequest *req = r->hba_private;
+
+    /* FIXME: queue notification to status pipe? */
+    usb_uas_unref_request(req);
+}
+
+static const struct SCSIBusInfo usb_uas_scsi_info = {
+    .tcq = true,
+    .max_target = 0,
+    .max_lun = 255,
+
+    .transfer_data = usb_uas_scsi_transfer_data,
+    .complete = usb_uas_scsi_command_complete,
+    .cancel = usb_uas_scsi_request_cancelled,
+};
+
+/* --------------------------------------------------------------------- */
+
+static void usb_uas_handle_reset(USBDevice *dev)
+{
+    UASDevice *uas = DO_UPCAST(UASDevice, dev, dev);
+    UASRequest *req, *nreq;
+    UASStatus *st, *nst;
+
+    trace_usb_uas_reset(dev->addr);
+    QTAILQ_FOREACH_SAFE(req, &uas->requests, next, nreq) {
+        scsi_req_cancel(req->req);
+    }
+    QTAILQ_FOREACH_SAFE(st, &uas->results, next, nst) {
+        QTAILQ_REMOVE(&uas->results, st, next);
+        g_free(st);
+    }
+}
+
+static int usb_uas_handle_control(USBDevice *dev, USBPacket *p,
+               int request, int value, int index, int length, uint8_t *data)
+{
+    int ret;
+
+    ret = usb_desc_handle_control(dev, p, request, value, index, length, data);
+    if (ret >= 0) {
+        return ret;
+    }
+    fprintf(stderr, "%s: unhandled control request\n", __func__);
+    return USB_RET_STALL;
+}
+
+static void usb_uas_cancel_io(USBDevice *dev, USBPacket *p)
+{
+    UASDevice *uas = DO_UPCAST(UASDevice, dev, dev);
+    UASRequest *req, *nreq;
+
+    if (uas->status == p) {
+        uas->status = NULL;
+        qemu_bh_cancel(uas->status_bh);
+        return;
+    }
+    QTAILQ_FOREACH_SAFE(req, &uas->requests, next, nreq) {
+        if (req->data == p) {
+            req->data = NULL;
+            return;
+        }
+    }
+    assert(!"canceled usb packet not found");
+}
+
+static void usb_uas_command(UASDevice *uas, uas_ui *ui)
+{
+    UASRequest *req;
+    uint32_t len;
+
+    req = usb_uas_find_request(uas, be16_to_cpu(ui->hdr.tag));
+    if (req) {
+        goto overlapped_tag;
+    }
+    req = usb_uas_alloc_request(uas, ui);
+    if (req->dev == NULL) {
+        goto bad_target;
+    }
+
+    trace_usb_uas_command(uas->dev.addr, req->tag,
+                          usb_uas_get_lun(req->lun),
+                          req->lun >> 32, req->lun & 0xffffffff);
+    QTAILQ_INSERT_TAIL(&uas->requests, req, next);
+    req->req = scsi_req_new(req->dev, req->tag,
+                            usb_uas_get_lun(req->lun),
+                            ui->command.cdb, req);
+    len = scsi_req_enqueue(req->req);
+    if (len) {
+        req->data_size = len;
+        scsi_req_continue(req->req);
+    }
+    return;
+
+overlapped_tag:
+    usb_uas_queue_response(uas, req->tag, UAS_RC_OVERLAPPED_TAG, 0);
+    return;
+
+bad_target:
+    /*
+     * FIXME: Seems to upset linux, is this wrong?
+     * NOTE: Happens only with no scsi devices at the bus, not sure
+     *       this is a valid UAS setup in the first place.
+     */
+    usb_uas_queue_response(uas, req->tag, UAS_RC_INVALID_INFO_UNIT, 0);
+    g_free(req);
+    return;
+}
+
+static void usb_uas_task(UASDevice *uas, uas_ui *ui)
+{
+    uint16_t tag = be16_to_cpu(ui->hdr.tag);
+    uint64_t lun64 = be64_to_cpu(ui->task.lun);
+    SCSIDevice *dev = usb_uas_get_dev(uas, lun64);
+    int lun = usb_uas_get_lun(lun64);
+    UASRequest *req;
+    uint16_t task_tag;
+
+    req = usb_uas_find_request(uas, be16_to_cpu(ui->hdr.tag));
+    if (req) {
+        goto overlapped_tag;
+    }
+
+    switch (ui->task.function) {
+    case UAS_TMF_ABORT_TASK:
+        task_tag = be16_to_cpu(ui->task.task_tag);
+        trace_usb_uas_tmf_abort_task(uas->dev.addr, tag, task_tag);
+        if (dev == NULL) {
+            goto bad_target;
+        }
+        if (dev->lun != lun) {
+            goto incorrect_lun;
+        }
+        req = usb_uas_find_request(uas, task_tag);
+        if (req && req->dev == dev) {
+            scsi_req_cancel(req->req);
+        }
+        usb_uas_queue_response(uas, tag, UAS_RC_TMF_COMPLETE, 0);
+        break;
+
+    case UAS_TMF_LOGICAL_UNIT_RESET:
+        trace_usb_uas_tmf_logical_unit_reset(uas->dev.addr, tag, lun);
+        if (dev == NULL) {
+            goto bad_target;
+        }
+        if (dev->lun != lun) {
+            goto incorrect_lun;
+        }
+        qdev_reset_all(&dev->qdev);
+        usb_uas_queue_response(uas, tag, UAS_RC_TMF_COMPLETE, 0);
+        break;
+
+    default:
+        trace_usb_uas_tmf_unsupported(uas->dev.addr, tag, ui->task.function);
+        usb_uas_queue_response(uas, tag, UAS_RC_TMF_NOT_SUPPORTED, 0);
+        break;
+    }
+    return;
+
+overlapped_tag:
+    usb_uas_queue_response(uas, req->tag, UAS_RC_OVERLAPPED_TAG, 0);
+    return;
+
+bad_target:
+    /* FIXME: correct?  [see long comment in usb_uas_command()] */
+    usb_uas_queue_response(uas, tag, UAS_RC_INVALID_INFO_UNIT, 0);
+    return;
+
+incorrect_lun:
+    usb_uas_queue_response(uas, tag, UAS_RC_INCORRECT_LUN, 0);
+    return;
+}
+
+static int usb_uas_handle_data(USBDevice *dev, USBPacket *p)
+{
+    UASDevice *uas = DO_UPCAST(UASDevice, dev, dev);
+    uas_ui ui;
+    UASStatus *st;
+    UASRequest *req;
+    int length, ret = 0;
+
+    switch (p->ep->nr) {
+    case UAS_PIPE_ID_COMMAND:
+        length = MIN(sizeof(ui), p->iov.size);
+        usb_packet_copy(p, &ui, length);
+        switch (ui.hdr.id) {
+        case UAS_UI_COMMAND:
+            usb_uas_command(uas, &ui);
+            ret = length;
+            break;
+        case UAS_UI_TASK_MGMT:
+            usb_uas_task(uas, &ui);
+            ret = length;
+            break;
+        default:
+            fprintf(stderr, "%s: unknown command ui: id 0x%x\n",
+                    __func__, ui.hdr.id);
+            ret = USB_RET_STALL;
+            break;
+        }
+        break;
+    case UAS_PIPE_ID_STATUS:
+        st = QTAILQ_FIRST(&uas->results);
+        if (st == NULL) {
+            assert(uas->status == NULL);
+            uas->status = p;
+            ret = USB_RET_ASYNC;
+            break;
+        }
+        usb_packet_copy(p, &st->status, st->length);
+        ret = st->length;
+        QTAILQ_REMOVE(&uas->results, st, next);
+        g_free(st);
+        break;
+    case UAS_PIPE_ID_DATA_IN:
+    case UAS_PIPE_ID_DATA_OUT:
+        req = (p->ep->nr == UAS_PIPE_ID_DATA_IN) ? uas->datain : uas->dataout;
+        if (req == NULL) {
+            fprintf(stderr, "%s: no inflight request\n", __func__);
+            ret = USB_RET_STALL;
+            break;
+        }
+        usb_uas_ref_request(req);
+        req->data = p;
+        usb_uas_copy_data(req);
+        if (p->result == p->iov.size || req->req == NULL) {
+            req->data = NULL;
+            ret = p->result;
+        } else {
+            req->data_async = true;
+            ret = USB_RET_ASYNC;
+        }
+        usb_uas_unref_request(req);
+        usb_uas_start_next_transfer(uas);
+        break;
+    default:
+        fprintf(stderr, "%s: invalid endpoint %d\n", __func__, p->ep->nr);
+        ret = USB_RET_STALL;
+        break;
+    }
+    return ret;
+}
+
+static void usb_uas_handle_destroy(USBDevice *dev)
+{
+    UASDevice *uas = DO_UPCAST(UASDevice, dev, dev);
+
+    qemu_bh_delete(uas->status_bh);
+}
+
+static int usb_uas_init(USBDevice *dev)
+{
+    UASDevice *uas = DO_UPCAST(UASDevice, dev, dev);
+
+    usb_desc_create_serial(dev);
+    usb_desc_init(dev);
+
+    QTAILQ_INIT(&uas->results);
+    QTAILQ_INIT(&uas->requests);
+    uas->status_bh = qemu_bh_new(usb_uas_send_status_bh, uas);
+
+    scsi_bus_new(&uas->bus, &uas->dev.qdev, &usb_uas_scsi_info);
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_usb_uas = {
+    .name = "usb-uas",
+    .unmigratable = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_USB_DEVICE(dev, UASDevice),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void usb_uas_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
+
+    uc->init           = usb_uas_init;
+    uc->product_desc   = desc_strings[STR_PRODUCT];
+    uc->usb_desc       = &desc;
+    uc->cancel_packet  = usb_uas_cancel_io;
+    uc->handle_attach  = usb_desc_attach;
+    uc->handle_reset   = usb_uas_handle_reset;
+    uc->handle_control = usb_uas_handle_control;
+    uc->handle_data    = usb_uas_handle_data;
+    uc->handle_destroy = usb_uas_handle_destroy;
+    dc->fw_name = "storage";
+    dc->vmsd = &vmstate_usb_uas;
+}
+
+static TypeInfo uas_info = {
+    .name          = "usb-uas",
+    .parent        = TYPE_USB_DEVICE,
+    .instance_size = sizeof(UASDevice),
+    .class_init    = usb_uas_class_initfn,
+};
+
+static void usb_uas_register_types(void)
+{
+    type_register_static(&uas_info);
+}
+
+type_init(usb_uas_register_types)
diff --git a/trace-events b/trace-events
index c935ba2..4e7882c 100644
--- a/trace-events
+++ b/trace-events
@@ -347,6 +347,20 @@ usb_hub_clear_port_feature(int addr, int nr, const char *f) "dev %d, port %d, fe
 usb_hub_attach(int addr, int nr) "dev %d, port %d"
 usb_hub_detach(int addr, int nr) "dev %d, port %d"
 
+# hw/usb/dev-uas.c
+usb_uas_reset(int addr) "dev %d"
+usb_uas_command(int addr, uint16_t tag, int lun, uint32_t lun64_1, uint32_t lun64_2) "dev %d, tag 0x%x, lun %d, lun64 %08x-%08x"
+usb_uas_response(int addr, uint16_t tag, uint8_t code) "dev %d, tag 0x%x, code 0x%x"
+usb_uas_sense(int addr, uint16_t tag, uint8_t status) "dev %d, tag 0x%x, status 0x%x"
+usb_uas_read_ready(int addr, uint16_t tag) "dev %d, tag 0x%x"
+usb_uas_write_ready(int addr, uint16_t tag) "dev %d, tag 0x%x"
+usb_uas_xfer_data(int addr, uint16_t tag, uint32_t copy, uint32_t uoff, uint32_t usize, uint32_t soff, uint32_t ssize) "dev %d, tag 0x%x, copy %d, usb-pkt %d/%d, scsi-buf %d/%d"
+usb_uas_scsi_data(int addr, uint16_t tag, uint32_t bytes) "dev %d, tag 0x%x, bytes %d"
+usb_uas_scsi_complete(int addr, uint16_t tag, uint32_t status, uint32_t resid) "dev %d, tag 0x%x, status 0x%x, residue %d"
+usb_uas_tmf_abort_task(int addr, uint16_t tag, uint16_t task_tag) "dev %d, tag 0x%x, task-tag 0x%x"
+usb_uas_tmf_logical_unit_reset(int addr, uint16_t tag, int lun) "dev %d, tag 0x%x, lun %d"
+usb_uas_tmf_unsupported(int addr, uint16_t tag, uint32_t function) "dev %d, tag 0x%x, function 0x%x"
+
 # hw/usb/host-linux.c
 usb_host_open_started(int bus, int addr) "dev %d:%d"
 usb_host_open_success(int bus, int addr) "dev %d:%d"
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 4/4] [wip] ehci: don't flush cache on dorbell rings.
  2012-06-20 12:41 [Qemu-devel] [PATCH 0/4] usb attached scsi Gerd Hoffmann
                   ` (2 preceding siblings ...)
  2012-06-20 12:41 ` [Qemu-devel] [PATCH 3/4] usb: add usb attached scsi emulation Gerd Hoffmann
@ 2012-06-20 12:41 ` Gerd Hoffmann
  2012-06-21  9:28   ` Gerd Hoffmann
  2012-06-21 10:04   ` Peter Maydell
  3 siblings, 2 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2012-06-20 12:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hans de Goede, Gerd Hoffmann

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

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

Simply not zapping queue heads on doorbell rings fixes the issue, but of course
re-introduces the risk of using cached but stale information.

Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ehci.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 9fb6423..6310630 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2227,8 +2227,10 @@ static void ehci_advance_async_state(EHCIState *ehci)
          * (section 4.8.2)
          */
         if (ehci->usbcmd & USBCMD_IAAD) {
+#if 0
             /* Remove all unseen qhs from the async qhs queue */
             ehci_queues_rip_unused(ehci, async, 1);
+#endif
             DPRINTF("ASYNC: doorbell request acknowledged\n");
             ehci->usbcmd &= ~USBCMD_IAAD;
             ehci_set_interrupt(ehci, USBSTS_IAA);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 4/4] [wip] ehci: don't flush cache on dorbell rings.
  2012-06-20 12:41 ` [Qemu-devel] [PATCH 4/4] [wip] ehci: don't flush cache on dorbell rings Gerd Hoffmann
@ 2012-06-21  9:28   ` Gerd Hoffmann
  2012-06-21 10:04   ` Peter Maydell
  1 sibling, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2012-06-21  9:28 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: Hans de Goede, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 377 bytes --]

On 06/20/12 14:41, Gerd Hoffmann wrote:
> Commit 4be23939ab0d7019c7e59a37485b416fbbf0f073 makes ehci instantly
> zap any unlinked queue heads when the guest rings the dorbell.

[ ... ]

> Simply not zapping queue heads on doorbell rings fixes the issue, but of course
> re-introduces the risk of using cached but stale information.

Improved version attached.

cheers,
  Gerd


[-- Attachment #2: 0001-wip-ehci-don-t-flush-cache-on-dorbell-rings.patch --]
[-- Type: text/plain, Size: 8315 bytes --]

>From 27c3356deb29f7cd5189aee8fd84058129812e28 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Wed, 20 Jun 2012 13:14:08 +0200
Subject: [PATCH v2] ehci: don't flush cache on dorbell rings.

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

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>
---
 hw/usb/hcd-ehci.c |   35 +++++++++++++++++++++++++++++------
 1 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 9fb6423..890692a 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -365,6 +365,7 @@ struct EHCIQueue {
     uint32_t seen;
     uint64_t ts;
     int async;
+    int revalidate;
 
     /* cached data from guest - needs to be flushed
      * when guest removes an entry (doorbell, handshake sequence)
@@ -775,7 +776,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;
     uint64_t maxage = FRAME_TIMER_NS * ehci->maxframes * 4;
@@ -787,7 +799,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 + maxage) {
+        if (ehci->last_run_ns < q->ts + maxage) {
             continue;
         }
         ehci_free_queue(q);
@@ -1631,7 +1643,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++) {
@@ -1716,6 +1728,7 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async)
     EHCIPacket *p;
     uint32_t entry, devaddr;
     EHCIQueue *q;
+    EHCIqh qh;
 
     entry = ehci_get_fetch_addr(ehci, async);
     q = ehci_find_queue_by_qh(ehci, entry, async);
@@ -1733,7 +1746,17 @@ 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);
+        q = ehci_alloc_queue(ehci, entry, async);
+        q->seen++;
+        p = NULL;
+    }
+    q->qh = qh;
+    q->revalidate = 0;
     ehci_trace_qh(q, NLPTR_GET(q->qhaddr), &q->qh);
 
     devaddr = get_field(q->qh.epchar, QH_EPCHAR_DEVADDR);
@@ -2228,7 +2251,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);
@@ -2281,7 +2304,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.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 4/4] [wip] ehci: don't flush cache on dorbell rings.
  2012-06-20 12:41 ` [Qemu-devel] [PATCH 4/4] [wip] ehci: don't flush cache on dorbell rings Gerd Hoffmann
  2012-06-21  9:28   ` Gerd Hoffmann
@ 2012-06-21 10:04   ` Peter Maydell
  2012-06-21 10:27     ` Andreas Färber
  1 sibling, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2012-06-21 10:04 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: Hans de Goede, qemu-devel

On 20 June 2012 13:41, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Commit 4be23939ab0d7019c7e59a37485b416fbbf0f073 makes ehci instantly
> zap any unlinked queue heads when the guest rings the dorbell.

Should be "doorbell" here and in the Subject, worth fixing up when
you do a next version / pullreq.

-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 4/4] [wip] ehci: don't flush cache on dorbell rings.
  2012-06-21 10:04   ` Peter Maydell
@ 2012-06-21 10:27     ` Andreas Färber
  2012-06-21 11:28       ` Dor Laor
  0 siblings, 1 reply; 11+ messages in thread
From: Andreas Färber @ 2012-06-21 10:27 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Hans de Goede, Dor Laor, Gerd Hoffmann, qemu-devel

Am 21.06.2012 12:04, schrieb Peter Maydell:
> On 20 June 2012 13:41, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> Commit 4be23939ab0d7019c7e59a37485b416fbbf0f073 makes ehci instantly
>> zap any unlinked queue heads when the guest rings the dorbell.
> 
> Should be "doorbell" here and in the Subject, worth fixing up when
> you do a next version / pullreq.

Maybe Dor has some special bell as ringtone. :)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 4/4] [wip] ehci: don't flush cache on dorbell rings.
  2012-06-21 10:27     ` Andreas Färber
@ 2012-06-21 11:28       ` Dor Laor
  0 siblings, 0 replies; 11+ messages in thread
From: Dor Laor @ 2012-06-21 11:28 UTC (permalink / raw)
  To: Andreas Färber
  Cc: qemu-devel, Peter Maydell, Gerd Hoffmann, Hans de Goede

On 06/21/2012 01:27 PM, Andreas Färber wrote:
> Am 21.06.2012 12:04, schrieb Peter Maydell:
>> On 20 June 2012 13:41, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>> Commit 4be23939ab0d7019c7e59a37485b416fbbf0f073 makes ehci instantly
>>> zap any unlinked queue heads when the guest rings the dorbell.
>>
>> Should be "doorbell" here and in the Subject, worth fixing up when
>> you do a next version / pullreq.
>
> Maybe Dor has some special bell as ringtone. :)

Luckily Bell is not my last name :)

> Andreas
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 3/4] usb: add usb attached scsi emulation
  2012-06-20 12:41 ` [Qemu-devel] [PATCH 3/4] usb: add usb attached scsi emulation Gerd Hoffmann
@ 2012-07-01 13:36   ` Paolo Bonzini
  2012-07-02 11:20     ` Gerd Hoffmann
  0 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2012-07-01 13:36 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

Il 20/06/2012 14:41, Gerd Hoffmann ha scritto:
> Special thanks go to Paolo for bringing the qemu scsi emulation into
> shape, so this can be added nicely without having to touch a single line
> of scsi code.

But we can touch it and make it even better. :)

Do you need a release_req SCSIBusInfo method that is called with the
hba_private when a request refcount goes to zero, so that you can remove
usb_uas_ref_request and usb_uas_unref_request?  (and replace it with
scsi_req_ref/unref in usb_uas_handle_data, I think).

Paolo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 3/4] usb: add usb attached scsi emulation
  2012-07-01 13:36   ` Paolo Bonzini
@ 2012-07-02 11:20     ` Gerd Hoffmann
  0 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2012-07-02 11:20 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On 07/01/12 15:36, Paolo Bonzini wrote:
> Il 20/06/2012 14:41, Gerd Hoffmann ha scritto:
>> Special thanks go to Paolo for bringing the qemu scsi emulation into
>> shape, so this can be added nicely without having to touch a single line
>> of scsi code.
> 
> But we can touch it and make it even better. :)
> 
> Do you need a release_req SCSIBusInfo method that is called with the
> hba_private when a request refcount goes to zero, so that you can remove
> usb_uas_ref_request and usb_uas_unref_request?  (and replace it with
> scsi_req_ref/unref in usb_uas_handle_data, I think).

Good idea.  Yes, that would indeed remove the need to do my own
reference counting.

cheers,
  Gerd

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2012-07-02 11:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-20 12:41 [Qemu-devel] [PATCH 0/4] usb attached scsi Gerd Hoffmann
2012-06-20 12:41 ` [Qemu-devel] [PATCH 1/4] ehci: fix ehci_qh_do_overlay Gerd Hoffmann
2012-06-20 12:41 ` [Qemu-devel] [PATCH 2/4] ehci: fix td writeback Gerd Hoffmann
2012-06-20 12:41 ` [Qemu-devel] [PATCH 3/4] usb: add usb attached scsi emulation Gerd Hoffmann
2012-07-01 13:36   ` Paolo Bonzini
2012-07-02 11:20     ` Gerd Hoffmann
2012-06-20 12:41 ` [Qemu-devel] [PATCH 4/4] [wip] ehci: don't flush cache on dorbell rings Gerd Hoffmann
2012-06-21  9:28   ` Gerd Hoffmann
2012-06-21 10:04   ` Peter Maydell
2012-06-21 10:27     ` Andreas Färber
2012-06-21 11:28       ` Dor Laor

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.