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>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
	"Fabiano Rosas" <farosas@suse.de>,
	"Laurent Vivier" <lvivier@redhat.com>,
	"Phil Dennis-Jordan" <phil@philjordan.eu>,
	"Bernhard Beschow" <shentey@gmail.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>
Subject: [PATCH v4 22/22] usb/msd: Add more tracing
Date: Fri,  2 May 2025 13:30:46 +1000	[thread overview]
Message-ID: <20250502033047.102465-23-npiggin@gmail.com> (raw)
In-Reply-To: <20250502033047.102465-1-npiggin@gmail.com>

Add tracing for more received packet types, cbw_state changes, and
some more SCSI callbacks. These were useful in debugging relaxed
packet ordering support.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/usb/dev-storage.c | 61 +++++++++++++++++++++++++++++++++-----------
 hw/usb/trace-events  | 11 +++++++-
 2 files changed, 56 insertions(+), 16 deletions(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index f421bd1f8dd..79f857de599 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -185,6 +185,21 @@ static const USBDesc desc = {
     .str   = desc_strings,
 };
 
+
+static void usb_msd_change_cbw_state(MSDState *s, USBMSDCBWState cbw_state)
+{
+    g_assert(s->cbw_state != cbw_state);
+    s->cbw_state = cbw_state;
+    trace_usb_msd_cbw_state(s->cbw_state);
+}
+
+static void usb_msd_set_cbw_state(MSDState *s, USBMSDCBWState cbw_state)
+{
+    if (s->cbw_state != cbw_state) {
+        usb_msd_change_cbw_state(s, cbw_state);
+    }
+}
+
 static void usb_msd_data_packet_complete(MSDState *s, int status)
 {
     USBPacket *p = s->data_packet;
@@ -194,7 +209,7 @@ static void usb_msd_data_packet_complete(MSDState *s, int status)
      * because another request may be issued before usb_packet_complete
      * returns.
      */
-    trace_usb_msd_packet_complete();
+    trace_usb_msd_data_packet_complete();
     s->data_packet = NULL;
     p->status = status;
     usb_packet_complete(&s->dev, p);
@@ -209,7 +224,7 @@ static void usb_msd_csw_packet_complete(MSDState *s, int status)
      * because another request may be issued before usb_packet_complete
      * returns.
      */
-    trace_usb_msd_packet_complete();
+    trace_usb_msd_csw_packet_complete();
     s->csw_in_packet = NULL;
     p->status = status;
     usb_packet_complete(&s->dev, p);
@@ -238,7 +253,11 @@ static void usb_msd_fatal_error(MSDState *s)
 static void usb_msd_copy_data(MSDState *s, USBPacket *p)
 {
     uint32_t len;
+
     len = p->iov.size - p->actual_length;
+
+    trace_usb_msd_copy_data(s->req->tag, len);
+
     if (len > s->scsi_len)
         len = s->scsi_len;
     usb_packet_copy(p, scsi_req_get_buf(s->req) + s->scsi_off, len);
@@ -271,6 +290,8 @@ void usb_msd_transfer_data(SCSIRequest *req, uint32_t len)
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
     USBPacket *p = s->data_packet;
 
+    trace_usb_msd_transfer_data(req->tag, len);
+
     if (s->cbw_state == USB_MSD_CBW_DATAIN) {
         if (req->cmd.mode == SCSI_XFER_TO_DEV) {
             usb_msd_fatal_error(s);
@@ -333,19 +354,19 @@ void usb_msd_command_complete(SCSIRequest *req, size_t resid)
             s->data_len -= len;
         }
         if (s->data_len == 0) {
-            s->cbw_state = USB_MSD_CBW_CSW;
+            usb_msd_change_cbw_state(s, USB_MSD_CBW_CSW);
         }
         /* USB_RET_SUCCESS status clears previous ASYNC status */
         usb_msd_data_packet_complete(s, USB_RET_SUCCESS);
     } else if (s->data_len == 0) {
-        s->cbw_state = USB_MSD_CBW_CSW;
+        usb_msd_change_cbw_state(s, USB_MSD_CBW_CSW);
     }
 
     if (s->cbw_state == USB_MSD_CBW_CSW) {
         p = s->csw_in_packet;
         if (p) {
             usb_msd_send_status(s, p);
-            s->cbw_state = USB_MSD_CBW_NONE;
+            usb_msd_change_cbw_state(s, USB_MSD_CBW_NONE);
             /* USB_RET_SUCCESS status clears previous ASYNC status */
             usb_msd_csw_packet_complete(s, USB_RET_SUCCESS);
         }
@@ -388,7 +409,7 @@ void usb_msd_handle_reset(USBDevice *dev)
     }
 
     memset(&s->csw, 0, sizeof(s->csw));
-    s->cbw_state = USB_MSD_CBW_NONE;
+    usb_msd_set_cbw_state(s, USB_MSD_CBW_NONE);
 
     s->needs_reset = false;
 }
@@ -439,6 +460,8 @@ static void usb_msd_cancel_io(USBDevice *dev, USBPacket *p)
 {
     MSDState *s = USB_STORAGE_DEV(dev);
 
+    trace_usb_msd_cancel_io();
+
     if (p == s->data_packet) {
         s->data_packet = NULL;
         if (s->req) {
@@ -491,11 +514,14 @@ static void usb_msd_handle_data_out(USBDevice *dev, USBPacket *p)
     SCSIDevice *scsi_dev;
     int len;
 
+    trace_usb_msd_data_out(p->iov.size, s->data_len);
+
     switch (s->cbw_state) {
     case USB_MSD_CBW_NONE:
         if (!try_get_valid_cbw(p, &cbw)) {
             goto fail;
         }
+        trace_usb_msd_cbw_out();
         scsi_dev = scsi_device_find(&s->bus, 0, 0, cbw.lun);
         if (scsi_dev == NULL) {
             qemu_log_mask(LOG_GUEST_ERROR, "usb-msd: Bad CBW LUN %d\n",
@@ -505,11 +531,11 @@ static void usb_msd_handle_data_out(USBDevice *dev, USBPacket *p)
         tag = le32_to_cpu(cbw.tag);
         s->data_len = le32_to_cpu(cbw.data_len);
         if (s->data_len == 0) {
-            s->cbw_state = USB_MSD_CBW_NODATA;
+            usb_msd_change_cbw_state(s, USB_MSD_CBW_NODATA);
         } else if (cbw.flags & 0x80) {
-            s->cbw_state = USB_MSD_CBW_DATAIN;
+            usb_msd_change_cbw_state(s, USB_MSD_CBW_DATAIN);
         } else {
-            s->cbw_state = USB_MSD_CBW_DATAOUT;
+            usb_msd_change_cbw_state(s, USB_MSD_CBW_DATAOUT);
         }
         if (s->unknown_in_packet) {
             if (s->cbw_state == USB_MSD_CBW_DATAIN) {
@@ -540,7 +566,6 @@ static void usb_msd_handle_data_out(USBDevice *dev, USBPacket *p)
         break;
 
     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;
@@ -562,7 +587,7 @@ static void usb_msd_handle_data_out(USBDevice *dev, USBPacket *p)
                 }
                 s->data_len -= len;
                 if (s->data_len == 0) {
-                    s->cbw_state = USB_MSD_CBW_CSW;
+                    usb_msd_change_cbw_state(s, USB_MSD_CBW_CSW);
                 }
             }
         }
@@ -579,6 +604,7 @@ static void usb_msd_handle_data_out(USBDevice *dev, USBPacket *p)
     return;
 
 fail:
+    trace_usb_msd_bad_packet();
     p->status = USB_RET_STALL;
 }
 
@@ -587,8 +613,11 @@ static void usb_msd_handle_data_in(USBDevice *dev, USBPacket *p)
     MSDState *s = (MSDState *)dev;
     int len;
 
+    trace_usb_msd_data_in(p->iov.size, s->data_len, s->scsi_len);
+
     switch (s->cbw_state) {
     case USB_MSD_CBW_NONE:
+        trace_usb_msd_unknown_in(p->iov.size);
         if (s->unknown_in_packet) {
             qemu_log_mask(LOG_GUEST_ERROR, "usb-msd: second IN packet was"
                                            "received before CBW\n");
@@ -602,12 +631,13 @@ static void usb_msd_handle_data_in(USBDevice *dev, USBPacket *p)
     case USB_MSD_CBW_NODATA:
     case USB_MSD_CBW_DATAOUT:
         if (s->unknown_in_packet) {
-            error_report("usb-msd: unknown_in_packet in DATAOUT state");
+            error_report("usb-msd: unknown_in_packet in DATAOUT/NODATA state");
             goto fail;
         }
         if (!check_valid_csw(p)) {
             goto fail;
         }
+        trace_usb_msd_csw_in();
         if (s->data_len != 0) {
             qemu_log_mask(LOG_GUEST_ERROR, "usb-msd: CSW received before "
                                            "all data was sent\n");
@@ -621,6 +651,7 @@ static void usb_msd_handle_data_in(USBDevice *dev, USBPacket *p)
         break;
 
     case USB_MSD_CBW_CSW:
+        trace_usb_msd_csw_in();
         if (s->unknown_in_packet) {
             error_report("usb-msd: unknown_in_packet in CSW state");
             goto fail;
@@ -636,12 +667,11 @@ static void usb_msd_handle_data_in(USBDevice *dev, USBPacket *p)
             p->status = USB_RET_ASYNC;
         } else {
             usb_msd_send_status(s, p);
-            s->cbw_state = USB_MSD_CBW_NONE;
+            usb_msd_change_cbw_state(s, USB_MSD_CBW_NONE);
         }
         break;
 
     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;
@@ -658,7 +688,7 @@ static void usb_msd_handle_data_in(USBDevice *dev, USBPacket *p)
                 }
                 s->data_len -= len;
                 if (s->data_len == 0) {
-                    s->cbw_state = USB_MSD_CBW_CSW;
+                    usb_msd_change_cbw_state(s, USB_MSD_CBW_CSW);
                 }
             }
         }
@@ -676,6 +706,7 @@ static void usb_msd_handle_data_in(USBDevice *dev, USBPacket *p)
     return;
 
 fail:
+    trace_usb_msd_bad_packet();
     p->status = USB_RET_STALL;
 }
 
diff --git a/hw/usb/trace-events b/hw/usb/trace-events
index dd04f14add1..851ba9986c3 100644
--- a/hw/usb/trace-events
+++ b/hw/usb/trace-events
@@ -264,12 +264,21 @@ usb_msd_maxlun(unsigned maxlun) "%d"
 usb_msd_send_status(unsigned status, unsigned tag, size_t size) "status %d, tag 0x%x, len %zd"
 usb_msd_data_in(unsigned packet, unsigned remaining, unsigned total) "%d/%d (scsi %d)"
 usb_msd_data_out(unsigned packet, unsigned remaining) "%d/%d"
+usb_msd_unknown_in(unsigned packet) "%d"
+usb_msd_cbw_out(void) ""
+usb_msd_csw_in(void) ""
 usb_msd_packet_async(void) ""
-usb_msd_packet_complete(void) ""
+usb_msd_data_packet_complete(void) ""
+usb_msd_csw_packet_complete(void) ""
+usb_msd_bad_packet(void) ""
 usb_msd_cmd_submit(unsigned lun, unsigned tag, unsigned flags, unsigned len, unsigned data_len) "lun %u, tag 0x%x, flags 0x%08x, len %d, data-len %d"
 usb_msd_cmd_complete(unsigned status, unsigned tag) "status %d, tag 0x%x"
+usb_msd_copy_data(unsigned tag, unsigned len) "tag 0x%x len %d"
+usb_msd_transfer_data(unsigned tag, unsigned len) "tag 0x%x len %d"
 usb_msd_cmd_cancel(unsigned tag) "tag 0x%x"
+usb_msd_cancel_io(void) ""
 usb_msd_fatal_error(void) ""
+usb_msd_cbw_state(unsigned cbw_state) "cbw-state %d"
 
 # dev-uas.c
 usb_uas_reset(int addr) "dev %d"
-- 
2.47.1



  parent reply	other threads:[~2025-05-02  3:35 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-02  3:30 [PATCH v4 00/22] usb/xhci and usb/msd improvements and tests Nicholas Piggin
2025-05-02  3:30 ` [PATCH v4 01/22] hw/usb/xhci: Move HCD constants to a header and add register constants Nicholas Piggin
2025-05-12 12:25   ` Peter Maydell
2025-05-02  3:30 ` [PATCH v4 02/22] hw/usb/xhci: Rename and move HCD register region constants to header Nicholas Piggin
2025-05-12 12:29   ` Peter Maydell
2025-05-02  3:30 ` [PATCH v4 03/22] tests/qtest/xhci: test the qemu-xhci device Nicholas Piggin
2025-05-19 13:54   ` Fabiano Rosas
2025-05-02  3:30 ` [PATCH v4 04/22] tests/qtest/xhci: Add controller and device setup and ring tests Nicholas Piggin
2025-05-19 14:03   ` Fabiano Rosas
2025-05-02  3:30 ` [PATCH v4 05/22] tests/qtest/xhci: Add basic USB Mass Storage tests Nicholas Piggin
2025-05-19 14:44   ` Fabiano Rosas
2025-05-02  3:30 ` [PATCH v4 06/22] hw/usb/xhci: Support TR NOOP commands Nicholas Piggin
2025-05-12 13:06   ` Peter Maydell
2025-05-02  3:30 ` [PATCH v4 07/22] tests/qtest/xhci: add a test for " Nicholas Piggin
2025-05-19 14:54   ` Fabiano Rosas
2025-05-02  3:30 ` [PATCH v4 08/22] tests/qtest/usb-hcd-xhci: Deliver msix interrupts Nicholas Piggin
2025-05-02  8:24   ` Philippe Mathieu-Daudé
2025-05-05  1:05     ` Nicholas Piggin
2025-05-02  3:30 ` [PATCH v4 09/22] hw/usb/hcd-xhci-pci: Make PCI device more configurable Nicholas Piggin
2025-05-12 13:12   ` Peter Maydell
2025-05-02  3:30 ` [PATCH v4 10/22] hw/usb/hcd-xhci-pci: Add TI TUSB73X0 XHCI controller model Nicholas Piggin
2025-05-12 13:15   ` Peter Maydell
2025-05-02  3:30 ` [PATCH v4 11/22] usb/msd: Split in and out packet handling Nicholas Piggin
2025-05-05  9:22   ` Kevin Wolf
2025-05-02  3:30 ` [PATCH v4 12/22] usb/msd: Ensure packet structure layout is correct Nicholas Piggin
2025-05-05  9:30   ` Kevin Wolf
2025-05-02  3:30 ` [PATCH v4 13/22] usb/msd: Improved handling of mass storage reset Nicholas Piggin
2025-05-02  3:30 ` [PATCH v4 14/22] usb/msd: Improve packet validation error logging Nicholas Piggin
2025-05-05 10:26   ` Kevin Wolf
2025-05-02  3:30 ` [PATCH v4 15/22] usb/msd: Allow CBW packet size greater than 31 Nicholas Piggin
2025-05-05 10:50   ` Kevin Wolf
2025-05-02  3:30 ` [PATCH v4 16/22] usb/msd: Split async packet tracking into data and csw Nicholas Piggin
2025-05-05 13:05   ` Kevin Wolf
2025-05-05 14:04     ` Kevin Wolf
2025-05-02  3:30 ` [PATCH v4 17/22] usb/msd: Add some additional assertions Nicholas Piggin
2025-05-02  3:30 ` [PATCH v4 18/22] usb/msd: Rename mode to cbw_state, and tweak names Nicholas Piggin
2025-05-02  3:30 ` [PATCH v4 19/22] usb/msd: Add NODATA CBW state Nicholas Piggin
2025-05-02  3:30 ` [PATCH v4 20/22] usb/msd: Permit a DATA-IN or CSW packet before CBW packet Nicholas Piggin
2025-05-02  3:30 ` [PATCH v4 21/22] tests/qtest/xhci: Test USB Mass Storage relaxed CSW order Nicholas Piggin
2025-05-19 15:03   ` Fabiano Rosas
2025-05-02  3:30 ` Nicholas Piggin [this message]
2025-05-05  2:03 ` [PATCH v4 00/22] usb/xhci and usb/msd improvements and tests Nicholas Piggin
2025-05-05  9:02   ` Kevin Wolf
2025-05-12 13:20     ` Peter Maydell
2025-05-12 15:33       ` Fabiano Rosas

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=20250502033047.102465-23-npiggin@gmail.com \
    --to=npiggin@gmail.com \
    --cc=farosas@suse.de \
    --cc=kraxel@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=phil@philjordan.eu \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=shentey@gmail.com \
    /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).