qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: Nicholas Piggin <npiggin@gmail.com>,
	qemu-devel@nongnu.org, Kevin Wolf <kwolf@redhat.com>
Subject: [PATCH v2 09/10] usb/msd: Permit a DATA-IN or CSW packet before CBW packet
Date: Fri, 11 Apr 2025 18:04:30 +1000	[thread overview]
Message-ID: <20250411080431.207579-10-npiggin@gmail.com> (raw)
In-Reply-To: <20250411080431.207579-1-npiggin@gmail.com>

The USB MSD protocol has 3 packets that make up a command, and only one
command may be active at any time.

- CBW to start a command (that contains a SCSI request).
- DATA (IN or OUT) to request data transfer between host and SCSI layer.
- CSW to return status and complete the command.

DATA is omitted if the request has no data.

The QEMU MSD model requires these packets to arrive in this order, CBW,
DATA, CSW. This is the way the state machine is generally described in
the MSD spec, and this must be how most USB stacks operate. Except AIX.

Universal Serial Bus Mass Storage Class Bulk-Only Transport 1.0 contains
one word in one sentence that permits the relaxed ordering:

  3.3 Host/Device Packet Transfer Order
  The host shall send the CBW before the associated Data-Out, and the
  device shall send Data-In after the associated CBW and before the
  associated CSW. The host may request Data-In or CSW before sending the
  associated CBW.

Complicating matters, DATA-IN and CSW are both input packets that arrive
in the same manner, so before a CBW it is impossible to determine if an
IN packet is for data or CSW.

So permit "unusually-ordered" packets by tracking them as an "unknown"
packet until the CBW arrives, then they are categorized into a DATA or
CSW packet.

It is not clear whether the spec permits multiple such packets before
the CBW. This implementation permits only one, which seems to be enough
for AIX.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 include/hw/usb/msd.h |  1 +
 hw/usb/dev-storage.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/include/hw/usb/msd.h b/include/hw/usb/msd.h
index c109544f632..2ed3664b31d 100644
--- a/include/hw/usb/msd.h
+++ b/include/hw/usb/msd.h
@@ -37,6 +37,7 @@ struct MSDState {
     /* For async completion.  */
     USBPacket *data_packet;
     USBPacket *csw_in_packet;
+    USBPacket *unknown_in_packet;
 
     /* usb-storage only */
     BlockConf conf;
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index ed6d9b70b96..654b9071d33 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -436,6 +436,8 @@ static void usb_msd_cancel_io(USBDevice *dev, USBPacket *p)
         }
     } else if (p == s->csw_in_packet) {
         s->csw_in_packet = NULL;
+    } else if (p == s->unknown_in_packet) {
+        s->unknown_in_packet = NULL;
     } else {
         g_assert_not_reached();
     }
@@ -499,6 +501,19 @@ static void usb_msd_handle_data_out(USBDevice *dev, USBPacket *p)
         } else {
             s->cbw_state = USB_MSD_CBW_DATAOUT;
         }
+        if (s->unknown_in_packet) {
+            if (s->cbw_state == USB_MSD_CBW_DATAIN) {
+                /* Must be a DATAIN packet */
+                s->data_packet = s->unknown_in_packet;
+            } else {
+                /* Must be the CSW packet */
+                if (!check_valid_csw(s->unknown_in_packet)) {
+                    goto fail;
+                }
+                s->csw_in_packet = s->unknown_in_packet;
+            }
+            s->unknown_in_packet = NULL;
+        }
         trace_usb_msd_cmd_submit(cbw.lun, tag, cbw.flags,
                                  cbw.cmd_len, s->data_len);
         assert(le32_to_cpu(s->csw.residue) == 0);
@@ -516,6 +531,11 @@ static void usb_msd_handle_data_out(USBDevice *dev, USBPacket *p)
 
     case USB_MSD_CBW_DATAOUT:
         trace_usb_msd_data_out(p->iov.size, s->data_len);
+        if (s->unknown_in_packet) {
+            error_report("usb-msd: unknown_in_packet in DATAOUT state");
+            goto fail;
+        }
+
         if (p->iov.size > s->data_len) {
             goto fail;
         }
@@ -558,7 +578,22 @@ static void usb_msd_handle_data_in(USBDevice *dev, USBPacket *p)
     int len;
 
     switch (s->cbw_state) {
+    case USB_MSD_CBW_NONE:
+        if (s->unknown_in_packet) {
+            qemu_log_mask(LOG_GUEST_ERROR, "usb-msd: second IN packet was"
+                                           "received before CBW\n");
+            goto fail;
+        }
+        trace_usb_msd_packet_async();
+        s->unknown_in_packet = p;
+        p->status = USB_RET_ASYNC;
+        break;
+
     case USB_MSD_CBW_DATAOUT:
+        if (s->unknown_in_packet) {
+            error_report("usb-msd: unknown_in_packet in DATAOUT state");
+            goto fail;
+        }
         if (!check_valid_csw(p)) {
             goto fail;
         }
@@ -575,6 +610,10 @@ static void usb_msd_handle_data_in(USBDevice *dev, USBPacket *p)
         break;
 
     case USB_MSD_CBW_CSW:
+        if (s->unknown_in_packet) {
+            error_report("usb-msd: unknown_in_packet in DATAOUT state");
+            goto fail;
+        }
         if (!check_valid_csw(p)) {
             goto fail;
         }
@@ -592,6 +631,10 @@ static void usb_msd_handle_data_in(USBDevice *dev, USBPacket *p)
 
     case USB_MSD_CBW_DATAIN:
         trace_usb_msd_data_in(p->iov.size, s->data_len, s->scsi_len);
+        if (s->unknown_in_packet) {
+            error_report("usb-msd: unknown_in_packet in DATAIN state");
+            goto fail;
+        }
         if (s->scsi_len) {
             usb_msd_copy_data(s, p);
         }
-- 
2.47.1



  parent reply	other threads:[~2025-04-11  8:07 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-11  8:04 [PATCH v2 00/10] usb/msd: Permit relaxed ordering of IN packets Nicholas Piggin
2025-04-11  8:04 ` [PATCH v2 01/10] usb/msd: Split in and out packet handling Nicholas Piggin
2025-04-11  8:04 ` [PATCH v2 02/10] usb/msd: Ensure packet structure layout is correct Nicholas Piggin
2025-04-11 10:18   ` Philippe Mathieu-Daudé
2025-04-11 10:21   ` Philippe Mathieu-Daudé
2025-04-11 10:23     ` Philippe Mathieu-Daudé
2025-04-12  5:32     ` Nicholas Piggin
2025-04-11  8:04 ` [PATCH v2 03/10] usb/msd: Improved handling of mass storage reset Nicholas Piggin
2025-04-11 10:19   ` Philippe Mathieu-Daudé
2025-04-11  8:04 ` [PATCH v2 04/10] usb/msd: Improve packet validation error logging Nicholas Piggin
2025-04-11  8:04 ` [PATCH v2 05/10] usb/msd: Allow CBW packet size greater than 31 Nicholas Piggin
2025-04-11  8:04 ` [PATCH v2 06/10] usb/msd: Split async packet tracking into data and csw Nicholas Piggin
2025-04-11  8:04 ` [PATCH v2 07/10] usb/msd: Add some additional assertions Nicholas Piggin
2025-04-11 10:27   ` Philippe Mathieu-Daudé
2025-04-11  8:04 ` [PATCH v2 08/10] usb/msd: Rename mode to cbw_state, and tweak names Nicholas Piggin
2025-04-11 10:28   ` Philippe Mathieu-Daudé
2025-04-11 10:37   ` Philippe Mathieu-Daudé
2025-04-12  5:33     ` Nicholas Piggin
2025-04-11  8:04 ` Nicholas Piggin [this message]
2025-04-11  8:04 ` [PATCH v2 10/10] usb/msd: Add more tracing Nicholas Piggin
2025-04-11 10:36   ` Philippe Mathieu-Daudé
2025-04-12  5:33     ` Nicholas Piggin

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=20250411080431.207579-10-npiggin@gmail.com \
    --to=npiggin@gmail.com \
    --cc=kraxel@redhat.com \
    --cc=kwolf@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).